[N11] Update permission and buffer sequence

Log skipping of output buffers
Check all buffers/processing is complete before changing modes
Using memory given by user directly has been removed, and added memcpy for all cases
Minor bug fixes

Signed-off-by: Parichay Kapoor <pk.kapoor@samsung.com>

diff --git a/src/core/npu-engine/ip/plugin-ip-comm.c b/src/core/npu-engine/ip/plugin-ip-comm.c
index 07c417d..2258de5 100644 (file)
--- a/src/core/npu-engine/ip/plugin-ip-comm.c
+++ b/src/core/npu-engine/ip/plugin-ip-comm.c
@@ -72,11 +72,9 @@ typedef struct {
   int num_models;                         /**< num of models loaded */
   model_private *models[MAX_NUM_MODELS];  /**< private for each model */
   int activated_model_id;                 /**< currently activated model */
-  uint64_t sequence;                      /**< increasing number for running */
+  uint64_t sequence_in;                   /**< increasing number for running input */
+  uint64_t sequence_out;                  /**< increasing number for running output */
   buffer_types buffer_type;               /**< inout buffer type */
-  void *mapped_ptr;                       /**< mapped ptr if input is fd */
-  size_t mapped_size;                     /**< mapped ptr size if input is fd */
-  size_t mapped_offset;                   /**< mapped ptr offset if input is fd */
   pthread_mutex_t mutex;                  /**< mutex for locking */
   pthread_cond_t cond;                    /**< for syncing running the device */
 } npu_device;
@@ -229,10 +227,10 @@ int getNPUdevice(npudev_h *dev, uint32_t id)
     }
     npu_dev->device_id = id;
     npu_dev->num_models = 0;
-    npu_dev->sequence = 1;
+    npu_dev->sequence_in = 0;
+    npu_dev->sequence_out = 0;
     npu_dev->activated_model_id = -1;
     npu_dev->buffer_type = BUFFER_UNDEFINED;
-    npu_dev->mapped_ptr = NULL;
     for (idx = 0; idx < MAX_NUM_MODELS; idx ++) {
       npu_dev->models[idx] = NULL;
     }
@@ -300,69 +298,45 @@ static int setup_hwmem (npu_device *npu_dev, const generic_buffer *in_buf, hwmem
     return -EINVAL;
   }
 
-  if (in_buf->type == BUFFER_CONT) {
-    if (in_buf->buf == NULL) {
-      logerr (TAG, "Output buffer is NULL\n");
-      return -EINVAL;
-    }
+  if ((err = hwmem_activate (hwmem_ptr)) < 0) {
+    logerr (TAG, "Fail to activate hwmem\n");
+    return err;
+  }
+  if ((err = hwmem_get_data (hwmem_ptr, &data_ptr)) < 0) {
+    logerr (TAG, "Error setting data in hwmem, errno: %d\n", err);
+    goto out;
+  }
 
-    if ((err = hwmem_set_data (hwmem_ptr, in_buf->buf)) < 0) {
-      logerr (TAG, "Error setting data in hwmem, errno: %d\n", err);
-      return err;
-    }
-  } else if (in_buf->type == BUFFER_NON_CONT) {
+  if (in_buf->type == BUFFER_NON_CONT || in_buf->type == BUFFER_CONT) {
     if (in_buf->buf == NULL) {
       logerr (TAG, "Output buffer is NULL\n");
-      return -EINVAL;
-    }
-
-    if ((err = hwmem_activate (hwmem_ptr)) < 0) {
-      logerr (TAG, "Fail to activate hwmem\n");
-      return err;
-    }
-
-    if ((err = hwmem_get_data (hwmem_ptr, &data_ptr)) < 0) {
-      logerr (TAG, "Error setting data in hwmem, errno: %d\n", err);
-      return err;
+      err = -EINVAL;
+      goto out;
     }
     memcpy (data_ptr, in_buf->buf, in_buf->size);
   } else if (in_buf->type == BUFFER_FD) {
-    /** this is munmap when the buffer is returned in callback */
-    if ((err = mmap_wrapper (PROT_READ, in_buf, &npu_dev->mapped_ptr)) < 0)
-      return -errno;
-    npu_dev->mapped_size = in_buf->size;
-    npu_dev->mapped_offset = in_buf->offset;
-
-    if ((err = hwmem_set_data (hwmem_ptr, npu_dev->mapped_ptr)) < 0) {
-      logerr (TAG, "Error setting data in hwmem, errno: %d\n", err);
-      if (munmap(npu_dev->mapped_ptr - npu_dev->mapped_offset,
-            npu_dev->mapped_size + npu_dev->mapped_offset) == -1)
-        logerr (TAG, "Munmap failed\n");
-      npu_dev->mapped_ptr = NULL;
-      return err;
-    }
+    void *mapped_ptr;
+
+    if ((err = mmap_wrapper (PROT_READ, in_buf, &mapped_ptr)) < 0)
+      goto out;
+
+    memcpy (data_ptr, mapped_ptr, in_buf->size);
+
+    err = munmap(mapped_ptr - in_buf->offset, in_buf->size + in_buf->offset);
   } else if (in_buf->type == BUFFER_FILE) {
     FILE *fp;
     size_t read_size;
 
     if (in_buf->filepath == NULL) {
       logerr (TAG, "Input buffer is NULL\n");
-      return -EINVAL;
-    }
-
-    if ((err = hwmem_activate (hwmem_ptr)) < 0) {
-      logerr (TAG, "Fail to activate hwmem\n");
-      return err;
-    }
-
-    if ((err = hwmem_get_data (hwmem_ptr, &data_ptr)) < 0) {
-      logerr (TAG, "Error setting data in hwmem, errno: %d\n", err);
-      return err;
+      err = -EINVAL;
+      goto out;
     }
 
     fp = fopen (in_buf->filepath, "r");
     if (fp == NULL) {
-      return -errno;
+      err = -errno;
+      goto out;
     }
 
     /** Read the data from the file */
@@ -370,15 +344,23 @@ static int setup_hwmem (npu_device *npu_dev, const generic_buffer *in_buf, hwmem
       logerr (TAG, "Failed to read all data, only read %d out of %d\n",
           read_size, in_buf->size);
       err = -ENOMEM;
+      fclose (fp);
+      goto out;
     }
 
     fclose (fp);
   } else {
     logerr (TAG, "Unknown output buffer type: %d\n", in_buf->type);
     err = -EINVAL;
+    goto out;
   }
 
   return err;
+
+out:
+  if ((err = hwmem_deactivate (hwmem_ptr)) < 0)
+    logerr (TAG, "Error deactivating hwmem, errno: %d\n", err);
+  return err;
 }
 
 /**
@@ -459,22 +441,15 @@ int registerNPUmodel(npudev_h dev, generic_buffer *model, uint32_t *modelid)
   }
 
   data_size = model->size;
-  if (model->type == BUFFER_NON_CONT || model->type == BUFFER_FILE) {
-    /** Get memory for the NPU model file */
-    if (libnpupriv.host_handle->getAvailableModelMemory() < data_size) {
-      logerr (TAG, "No availabl memory size\n");
-      return -ENOMEM;
-    }
+  /** Get memory for the NPU model file */
+  if (libnpupriv.host_handle->getAvailableModelMemory() < data_size) {
+    logerr (TAG, "No availabl memory size\n");
+    return -ENOMEM;
+  }
 
-    if ((err = GET_MEM()->alloc (data_size, &hwmem_ptr)) != 0) {
-      logerr (TAG, "Fail to allocate hwmem with size %lu\n", data_size);
-      return err;
-    }
-  } else {
-    if ((err = GET_MEM()->alloc_no_mem (data_size, &hwmem_ptr)) != 0) {
-      logerr (TAG, "Fail to allocate hwmem with size %lu\n", data_size);
-      return err;
-    }
+  if ((err = GET_MEM()->alloc (data_size, &hwmem_ptr)) != 0) {
+    logerr (TAG, "Fail to allocate hwmem with size %lu\n", data_size);
+    return err;
   }
 
   /** handles all the buffer types */
@@ -507,7 +482,7 @@ out_free:
 int unregisterNPUmodel(npudev_h dev, uint32_t modelid)
 {
   npu_device *npu_dev;
-  int err;
+  int err = 0;
 
   if ((err = model_dev_validity_check (dev, modelid)) < 0) {
     return err;
@@ -517,34 +492,38 @@ int unregisterNPUmodel(npudev_h dev, uint32_t modelid)
   npu_dev = dev;
   DEVICE_LOCK();
 
-  err = libnpupriv.host_handle->setOpMode (NPUINPUT_STOP, DEFAULT_FORCE_STOP,
-      npu_dev->models[modelid]->model_id,
-      npu_dev->models[modelid]->model_version, NULL, NULL);
-  if (err < 0) {
-    return err;
+  if (npu_dev->activated_model_id == modelid) {
+    err = libnpupriv.host_handle->setOpMode (NPUINPUT_STOP, DEFAULT_FORCE_STOP,
+        npu_dev->models[modelid]->model_id,
+        npu_dev->models[modelid]->model_version, NULL, NULL);
+    if (err < 0) {
+      goto out;
+    }
   }
 
   err = libnpupriv.host_handle->unregisterModel(
       npu_dev->models[modelid]->model_id,
       npu_dev->models[modelid]->model_version);
   if (err < 0) {
-    return err;
+    goto out;
   }
 
   npu_dev->num_models -= 1;
+  npu_dev->sequence_out = npu_dev->sequence_in;
   free (npu_dev->models[modelid]);
   npu_dev->models[modelid] = NULL;
   if (npu_dev->activated_model_id == modelid) {
     npu_dev->activated_model_id = -1;
   }
 
-  DEVICE_UNLOCK();
   if (npu_dev->num_models == 0) {
     pthread_mutex_destroy (&npu_dev->mutex);
     pthread_cond_destroy (&npu_dev->cond);
   }
 
-  return 0;
+out:
+  DEVICE_UNLOCK();
+  return err;
 }
 
 /**
@@ -691,11 +670,11 @@ static void npu_callback (buffer *buffer_ptr, uint64_t offset, uint64_t size,
   /** wake the sync process, if any */
   DEVICE_WAKEUP();
 
-  if (npu_dev->mapped_ptr != NULL) {
-    munmap(npu_dev->mapped_ptr - npu_dev->mapped_offset,
-        npu_dev->mapped_size + npu_dev->mapped_offset);
-    npu_dev->mapped_ptr = NULL;
+  if (buffer_ptr->sequence != npu_dev->sequence_out + 1) {
+    logwarn (TAG, "Output sequence num from %d to %d have been skipped\n",
+        npu_dev->sequence_out + 1, buffer_ptr->sequence - 1);
   }
+  npu_dev->sequence_out = buffer_ptr->sequence;
   DEVICE_UNLOCK();
 
   if (cb != NULL && npu_dev->buffer_type != BUFFER_UNDEFINED) {
@@ -784,6 +763,7 @@ static int activateNPUmodel(npudev_h dev, uint32_t modelid, npuOutputNotify *cb,
     return err;
   }
 
+  npu_dev->sequence_out = npu_dev->sequence_in;
   npu_dev->activated_model_id = modelid;
   return 0;
 }
@@ -810,7 +790,7 @@ static int runNPU_async_util(npudev_h dev, uint32_t modelid, const input_buffers
   hwmem *hwmem_ptr;
 
   if ((err = model_dev_validity_check (dev, modelid)) < 0) {
-    goto out;
+    return err;
   }
 
   npu_dev = dev;
@@ -834,21 +814,22 @@ static int runNPU_async_util(npudev_h dev, uint32_t modelid, const input_buffers
 
   if ((err = buffer_get_hwmem (buffer_ptr, &hwmem_ptr)) < 0) {
     logerr (TAG, "Error getting hwmem from buffer, errno: %d\n", err);
-    return err;
+    goto out;
   }
 
   if ((err = setup_hwmem (npu_dev, in_buf, hwmem_ptr)) < 0) {
     goto out;
   }
 
-  buffer_ptr->sequence = npu_dev->sequence;
+  buffer_ptr->sequence = npu_dev->sequence_in + 1;
   if ((err = libnpupriv.host_handle->validateBuffer (buffer_ptr)) < 0) {
-    hwmem_deactivate (hwmem_ptr);
+    if ((err = hwmem_deactivate (hwmem_ptr)) < 0)
+      logerr (TAG, "Error deactivating hwmem, errno: %d\n", err);
     goto out;
   }
 
-  npu_dev->sequence += 1;
-  *sequence = npu_dev->sequence;
+  npu_dev->sequence_in += 1;
+  *sequence = npu_dev->sequence_in;
 
 out:
   DEVICE_UNLOCK();
@@ -915,9 +896,12 @@ int runNPU_sync(npudev_h dev, uint32_t modelid, const input_buffers *input,
  *
  * @notes buffer_type cannot be changed while running an input. all the inputs,
  *        models and outputs will follow this type.
+ * @todo remove this function later as inconsistency between input buffer types
+ *       is already handled
  */
 int setInoutBufferType(npudev_h dev, buffer_types buffer_type)
 {
+  int err = 0;
   npu_device *npu_dev;
 
   if (dev == NULL || buffer_type == BUFFER_UNDEFINED) {
@@ -929,13 +913,17 @@ int setInoutBufferType(npudev_h dev, buffer_types buffer_type)
     return -EINVAL;
   }
 
-  npu_dev->buffer_type = buffer_type;
-  if (buffer_type == BUFFER_FD || buffer_type == BUFFER_CONT) {
-    GET_MEM()->set_mode (DOUBLE_BUFFERING);
+  DEVICE_LOCK ();
+
+  if (npu_dev->sequence_in == npu_dev->sequence_out) {
+    npu_dev->buffer_type = buffer_type;
   } else {
-    GET_MEM()->set_mode (TRIPLE_BUFFERING);
+    logerr (TAG, "Cannot set permission while processing data\n");
+    err = -EPERM;
   }
-  return 0;
+
+  DEVICE_UNLOCK ();
+  return err;
 }
 
 /**