[PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Jeffrey Hugo]

A BO for a QAIC device has two states -
1. Allocated
2. Sliced

A BO can be allocated at any time, and is initialized in the allocated state.
A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
BO for use with an active workload.  Currently a BO in the sliced state can
only be used with a single workload, and will only transition back to the
allocated state once the workload is deactivated.

Userspace would like the ability to trigger a BO transition from the sliced
state to the allocated state. This would support the usecase of a userspace
client that has two active workloads, where the output of the first workload
becomes the input of the second workload. Currently, the client would need
two BOs, once for each workload, and copy from one BO to the other.

To support this usecase, we create the detach slice concept which is the
inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
DETACH_SLICE_BO ioctl that allows userspace to perform this operation.

Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
a decent amount of code. This series starts with restructuring the common code
for use in both ioctls before finally adding the DETACH_SLICE_BO.

Pranjal Ramajor Asha Kanojiya (7):
  accel/qaic: Remove ->size field from struct qaic_bo
  accel/qaic: Update BO metadata in a central location
  accel/qaic: Declare BO 'sliced' after all the operations are complete
  accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
  accel/qaic: Clean up BO during flushing of transfer list
  accel/qaic: Create a function to initialize BO
  accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

 Documentation/accel/qaic/qaic.rst |  10 ++
 drivers/accel/qaic/qaic.h         |   6 +-
 drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
 drivers/accel/qaic/qaic_drv.c     |   1 +
 include/uapi/drm/qaic_accel.h     |  24 +++-
 5 files changed, 175 insertions(+), 53 deletions(-)

-- 
2.40.1

[PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Jeffrey Hugo]

A BO for a QAIC device has two states -
1. Allocated
2. Sliced

A BO can be allocated at any time, and is initialized in the allocated state.
A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
BO for use with an active workload.  Currently a BO in the sliced state can
only be used with a single workload, and will only transition back to the
allocated state once the workload is deactivated.

Userspace would like the ability to trigger a BO transition from the sliced
state to the allocated state. This would support the usecase of a userspace
client that has two active workloads, where the output of the first workload
becomes the input of the second workload. Currently, the client would need
two BOs, once for each workload, and copy from one BO to the other.

To support this usecase, we create the detach slice concept which is the
inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
DETACH_SLICE_BO ioctl that allows userspace to perform this operation.

Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
a decent amount of code. This series starts with restructuring the common code
for use in both ioctls before finally adding the DETACH_SLICE_BO.

Pranjal Ramajor Asha Kanojiya (7):
  accel/qaic: Remove ->size field from struct qaic_bo
  accel/qaic: Update BO metadata in a central location
  accel/qaic: Declare BO 'sliced' after all the operations are complete
  accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
  accel/qaic: Clean up BO during flushing of transfer list
  accel/qaic: Create a function to initialize BO
  accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

 Documentation/accel/qaic/qaic.rst |  10 ++
 drivers/accel/qaic/qaic.h         |   6 +-
 drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
 drivers/accel/qaic/qaic_drv.c     |   1 +
 include/uapi/drm/qaic_accel.h     |  24 +++-
 5 files changed, 175 insertions(+), 53 deletions(-)

-- 
2.40.1

[PATCH 1/7] accel/qaic: Remove ->size field from struct qaic_bo

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

->size field in struct qaic_bo stores user requested buffer size for
allocate path or size of the dmabuf(PRIME). Now for allocate path driver
allocates a BO of size which is PAGE_SIZE aligned, this size is already
stored in base BO structure (struct drm_gem_object).

So difference is ->size of struct qaic_bo stores the raw value coming from
user and ->size in struct drm_gem_object stores the PAGE_SZIE aligned size.

Do not use ->size from struct qaic_bo for any validation or operation
instead use ->size from struct drm_gem_object since we already have
allocated that much memory then why not use it. Only validate if user
is trying to use more then the BO size. This make the driver more flexible.

After this change ->size field of struct qaic_bo becomes redundant. Remove
it.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic.h      |  2 --
 drivers/accel/qaic/qaic_data.c | 10 +++-------
 include/uapi/drm/qaic_accel.h  | 12 ++++++------
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index f2bd637a0d4e..27cf66dbd5a5 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -158,8 +158,6 @@ struct qaic_bo {
 	struct drm_gem_object	base;
 	/* Scatter/gather table for allocate/imported BO */
 	struct sg_table		*sgt;
-	/* BO size requested by user. GEM object might be bigger in size. */
-	u64			size;
 	/* Head in list of slices of this BO */
 	struct list_head	slices;
 	/* Total nents, for all slices of this BO */
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index a90b64b325b4..09b5c6a52cb3 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -579,7 +579,7 @@ static void qaic_gem_print_info(struct drm_printer *p, unsigned int indent,
 {
 	struct qaic_bo *bo = to_qaic_bo(obj);
 
-	drm_printf_indent(p, indent, "user requested size=%llu\n", bo->size);
+	drm_printf_indent(p, indent, "BO DMA direction %d\n", bo->dir);
 }
 
 static const struct vm_operations_struct drm_vm_ops = {
@@ -695,8 +695,6 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi
 	if (ret)
 		goto free_bo;
 
-	bo->size = args->size;
-
 	ret = drm_gem_handle_create(file_priv, obj, &args->handle);
 	if (ret)
 		goto free_sgt;
@@ -828,7 +826,6 @@ static int qaic_prepare_import_bo(struct qaic_bo *bo, struct qaic_attach_slice_h
 	}
 
 	bo->sgt = sgt;
-	bo->size = hdr->size;
 
 	return 0;
 }
@@ -838,7 +835,7 @@ static int qaic_prepare_export_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 {
 	int ret;
 
-	if (bo->size != hdr->size)
+	if (bo->base.size < hdr->size)
 		return -EINVAL;
 
 	ret = dma_map_sgtable(&qdev->pdev->dev, bo->sgt, hdr->dir, 0);
@@ -868,7 +865,6 @@ static void qaic_unprepare_import_bo(struct qaic_bo *bo)
 {
 	dma_buf_unmap_attachment(bo->base.import_attach, bo->sgt, bo->dir);
 	bo->sgt = NULL;
-	bo->size = 0;
 }
 
 static void qaic_unprepare_export_bo(struct qaic_device *qdev, struct qaic_bo *bo)
@@ -1190,7 +1186,7 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 			goto failed_to_send_bo;
 		}
 
-		if (is_partial && pexec[i].resize > bo->size) {
+		if (is_partial && pexec[i].resize > bo->base.size) {
 			ret = -EINVAL;
 			goto failed_to_send_bo;
 		}
diff --git a/include/uapi/drm/qaic_accel.h b/include/uapi/drm/qaic_accel.h
index 2d348744a853..f89880b7bfb6 100644
--- a/include/uapi/drm/qaic_accel.h
+++ b/include/uapi/drm/qaic_accel.h
@@ -242,12 +242,12 @@ struct qaic_attach_slice_entry {
  * @dbc_id: In. Associate the sliced BO with this DBC.
  * @handle: In. GEM handle of the BO to slice.
  * @dir: In. Direction of data flow. 1 = DMA_TO_DEVICE, 2 = DMA_FROM_DEVICE
- * @size: In. Total length of the BO.
- *	  If BO is imported (DMABUF/PRIME) then this size
- *	  should not exceed the size of DMABUF provided.
- *	  If BO is allocated using DRM_IOCTL_QAIC_CREATE_BO
- *	  then this size should be exactly same as the size
- *	  provided during DRM_IOCTL_QAIC_CREATE_BO.
+ * @size: In. Total length of BO being used. This should not exceed base
+ *	  size of BO (struct drm_gem_object.base)
+ *	  For BOs being allocated using DRM_IOCTL_QAIC_CREATE_BO, size of
+ *	  BO requested is PAGE_SIZE aligned then allocated hence allocated
+ *	  BO size maybe bigger. This size should not exceed the new
+ *	  PAGE_SIZE aligned BO size.
  * @dev_addr: In. Device address this slice pushes to or pulls from.
  * @db_addr: In. Address of the doorbell to ring.
  * @db_data: In. Data to write to the doorbell.
-- 
2.40.1

[PATCH 1/7] accel/qaic: Remove ->size field from struct qaic_bo

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

->size field in struct qaic_bo stores user requested buffer size for
allocate path or size of the dmabuf(PRIME). Now for allocate path driver
allocates a BO of size which is PAGE_SIZE aligned, this size is already
stored in base BO structure (struct drm_gem_object).

So difference is ->size of struct qaic_bo stores the raw value coming from
user and ->size in struct drm_gem_object stores the PAGE_SZIE aligned size.

Do not use ->size from struct qaic_bo for any validation or operation
instead use ->size from struct drm_gem_object since we already have
allocated that much memory then why not use it. Only validate if user
is trying to use more then the BO size. This make the driver more flexible.

After this change ->size field of struct qaic_bo becomes redundant. Remove
it.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic.h      |  2 --
 drivers/accel/qaic/qaic_data.c | 10 +++-------
 include/uapi/drm/qaic_accel.h  | 12 ++++++------
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index f2bd637a0d4e..27cf66dbd5a5 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -158,8 +158,6 @@ struct qaic_bo {
 	struct drm_gem_object	base;
 	/* Scatter/gather table for allocate/imported BO */
 	struct sg_table		*sgt;
-	/* BO size requested by user. GEM object might be bigger in size. */
-	u64			size;
 	/* Head in list of slices of this BO */
 	struct list_head	slices;
 	/* Total nents, for all slices of this BO */
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index a90b64b325b4..09b5c6a52cb3 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -579,7 +579,7 @@ static void qaic_gem_print_info(struct drm_printer *p, unsigned int indent,
 {
 	struct qaic_bo *bo = to_qaic_bo(obj);
 
-	drm_printf_indent(p, indent, "user requested size=%llu\n", bo->size);
+	drm_printf_indent(p, indent, "BO DMA direction %d\n", bo->dir);
 }
 
 static const struct vm_operations_struct drm_vm_ops = {
@@ -695,8 +695,6 @@ int qaic_create_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *fi
 	if (ret)
 		goto free_bo;
 
-	bo->size = args->size;
-
 	ret = drm_gem_handle_create(file_priv, obj, &args->handle);
 	if (ret)
 		goto free_sgt;
@@ -828,7 +826,6 @@ static int qaic_prepare_import_bo(struct qaic_bo *bo, struct qaic_attach_slice_h
 	}
 
 	bo->sgt = sgt;
-	bo->size = hdr->size;
 
 	return 0;
 }
@@ -838,7 +835,7 @@ static int qaic_prepare_export_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 {
 	int ret;
 
-	if (bo->size != hdr->size)
+	if (bo->base.size < hdr->size)
 		return -EINVAL;
 
 	ret = dma_map_sgtable(&qdev->pdev->dev, bo->sgt, hdr->dir, 0);
@@ -868,7 +865,6 @@ static void qaic_unprepare_import_bo(struct qaic_bo *bo)
 {
 	dma_buf_unmap_attachment(bo->base.import_attach, bo->sgt, bo->dir);
 	bo->sgt = NULL;
-	bo->size = 0;
 }
 
 static void qaic_unprepare_export_bo(struct qaic_device *qdev, struct qaic_bo *bo)
@@ -1190,7 +1186,7 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 			goto failed_to_send_bo;
 		}
 
-		if (is_partial && pexec[i].resize > bo->size) {
+		if (is_partial && pexec[i].resize > bo->base.size) {
 			ret = -EINVAL;
 			goto failed_to_send_bo;
 		}
diff --git a/include/uapi/drm/qaic_accel.h b/include/uapi/drm/qaic_accel.h
index 2d348744a853..f89880b7bfb6 100644
--- a/include/uapi/drm/qaic_accel.h
+++ b/include/uapi/drm/qaic_accel.h
@@ -242,12 +242,12 @@ struct qaic_attach_slice_entry {
  * @dbc_id: In. Associate the sliced BO with this DBC.
  * @handle: In. GEM handle of the BO to slice.
  * @dir: In. Direction of data flow. 1 = DMA_TO_DEVICE, 2 = DMA_FROM_DEVICE
- * @size: In. Total length of the BO.
- *	  If BO is imported (DMABUF/PRIME) then this size
- *	  should not exceed the size of DMABUF provided.
- *	  If BO is allocated using DRM_IOCTL_QAIC_CREATE_BO
- *	  then this size should be exactly same as the size
- *	  provided during DRM_IOCTL_QAIC_CREATE_BO.
+ * @size: In. Total length of BO being used. This should not exceed base
+ *	  size of BO (struct drm_gem_object.base)
+ *	  For BOs being allocated using DRM_IOCTL_QAIC_CREATE_BO, size of
+ *	  BO requested is PAGE_SIZE aligned then allocated hence allocated
+ *	  BO size maybe bigger. This size should not exceed the new
+ *	  PAGE_SIZE aligned BO size.
  * @dev_addr: In. Device address this slice pushes to or pulls from.
  * @db_addr: In. Address of the doorbell to ring.
  * @db_data: In. Data to write to the doorbell.
-- 
2.40.1

[PATCH 2/7] accel/qaic: Update BO metadata in a central location

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Update/Clean up BO metadata in a central location, this will help maintain
the code and looks cleaner.

Use qaic_unprepare_bo() to cleanup release_dbc()

Next few patches will be implementing detach IOCTL which will leverage
this patch.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 09b5c6a52cb3..710bd355ed8e 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -854,9 +854,9 @@ static int qaic_prepare_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 		ret = qaic_prepare_import_bo(bo, hdr);
 	else
 		ret = qaic_prepare_export_bo(qdev, bo, hdr);
-
-	if (ret == 0)
-		bo->dir = hdr->dir;
+	bo->dir = hdr->dir;
+	bo->dbc = &qdev->dbc[hdr->dbc_id];
+	bo->nr_slice = hdr->count;
 
 	return ret;
 }
@@ -880,6 +880,8 @@ static void qaic_unprepare_bo(struct qaic_device *qdev, struct qaic_bo *bo)
 		qaic_unprepare_export_bo(qdev, bo);
 
 	bo->dir = 0;
+	bo->dbc = NULL;
+	bo->nr_slice = 0;
 }
 
 static void qaic_free_slices_bo(struct qaic_bo *bo)
@@ -904,14 +906,13 @@ static int qaic_attach_slicing_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 		}
 	}
 
-	if (bo->total_slice_nents > qdev->dbc[hdr->dbc_id].nelem) {
+	if (bo->total_slice_nents > bo->dbc->nelem) {
 		qaic_free_slices_bo(bo);
 		return -ENOSPC;
 	}
 
 	bo->sliced = true;
-	bo->nr_slice = hdr->count;
-	list_add_tail(&bo->bo_list, &qdev->dbc[hdr->dbc_id].bo_lists);
+	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
 
 	return 0;
 }
@@ -1014,7 +1015,6 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	if (args->hdr.dir == DMA_TO_DEVICE)
 		dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, args->hdr.dir);
 
-	bo->dbc = dbc;
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
 	drm_gem_object_put(obj);
 	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
@@ -1870,14 +1870,12 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	dbc->usr = NULL;
 
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
+		qaic_unprepare_bo(qdev, bo);
 		list_for_each_entry_safe(slice, slice_temp, &bo->slices, slice)
 			kref_put(&slice->ref_count, free_slice);
 		bo->sliced = false;
 		INIT_LIST_HEAD(&bo->slices);
 		bo->total_slice_nents = 0;
-		bo->dir = 0;
-		bo->dbc = NULL;
-		bo->nr_slice = 0;
 		bo->nr_slice_xfer_done = 0;
 		bo->queued = false;
 		bo->req_id = 0;
-- 
2.40.1

[PATCH 2/7] accel/qaic: Update BO metadata in a central location

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Update/Clean up BO metadata in a central location, this will help maintain
the code and looks cleaner.

Use qaic_unprepare_bo() to cleanup release_dbc()

Next few patches will be implementing detach IOCTL which will leverage
this patch.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 09b5c6a52cb3..710bd355ed8e 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -854,9 +854,9 @@ static int qaic_prepare_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 		ret = qaic_prepare_import_bo(bo, hdr);
 	else
 		ret = qaic_prepare_export_bo(qdev, bo, hdr);
-
-	if (ret == 0)
-		bo->dir = hdr->dir;
+	bo->dir = hdr->dir;
+	bo->dbc = &qdev->dbc[hdr->dbc_id];
+	bo->nr_slice = hdr->count;
 
 	return ret;
 }
@@ -880,6 +880,8 @@ static void qaic_unprepare_bo(struct qaic_device *qdev, struct qaic_bo *bo)
 		qaic_unprepare_export_bo(qdev, bo);
 
 	bo->dir = 0;
+	bo->dbc = NULL;
+	bo->nr_slice = 0;
 }
 
 static void qaic_free_slices_bo(struct qaic_bo *bo)
@@ -904,14 +906,13 @@ static int qaic_attach_slicing_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 		}
 	}
 
-	if (bo->total_slice_nents > qdev->dbc[hdr->dbc_id].nelem) {
+	if (bo->total_slice_nents > bo->dbc->nelem) {
 		qaic_free_slices_bo(bo);
 		return -ENOSPC;
 	}
 
 	bo->sliced = true;
-	bo->nr_slice = hdr->count;
-	list_add_tail(&bo->bo_list, &qdev->dbc[hdr->dbc_id].bo_lists);
+	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
 
 	return 0;
 }
@@ -1014,7 +1015,6 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	if (args->hdr.dir == DMA_TO_DEVICE)
 		dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, args->hdr.dir);
 
-	bo->dbc = dbc;
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
 	drm_gem_object_put(obj);
 	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
@@ -1870,14 +1870,12 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	dbc->usr = NULL;
 
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
+		qaic_unprepare_bo(qdev, bo);
 		list_for_each_entry_safe(slice, slice_temp, &bo->slices, slice)
 			kref_put(&slice->ref_count, free_slice);
 		bo->sliced = false;
 		INIT_LIST_HEAD(&bo->slices);
 		bo->total_slice_nents = 0;
-		bo->dir = 0;
-		bo->dbc = NULL;
-		bo->nr_slice = 0;
 		bo->nr_slice_xfer_done = 0;
 		bo->queued = false;
 		bo->req_id = 0;
-- 
2.40.1

[PATCH 3/7] accel/qaic: Declare BO 'sliced' after all the operations are complete

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Once the BO is declared 'sliced' by setting bo->sliced to true we can
perform DMA (QAIC_EXECUTE_BO) operation on that BO. Hence we should
declare a BO sliced after completing all the operations.

Adding BO to its respective DBC list in qaic_attach_slicing_bo() seems
out of place as qaic_attach_slicing_bo() should just update BO with
slicing configuration.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 710bd355ed8e..6a802497834c 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -911,9 +911,6 @@ static int qaic_attach_slicing_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 		return -ENOSPC;
 	}
 
-	bo->sliced = true;
-	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
-
 	return 0;
 }
 
@@ -1015,6 +1012,8 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	if (args->hdr.dir == DMA_TO_DEVICE)
 		dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, args->hdr.dir);
 
+	bo->sliced = true;
+	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
 	drm_gem_object_put(obj);
 	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
-- 
2.40.1

[PATCH 3/7] accel/qaic: Declare BO 'sliced' after all the operations are complete

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Once the BO is declared 'sliced' by setting bo->sliced to true we can
perform DMA (QAIC_EXECUTE_BO) operation on that BO. Hence we should
declare a BO sliced after completing all the operations.

Adding BO to its respective DBC list in qaic_attach_slicing_bo() seems
out of place as qaic_attach_slicing_bo() should just update BO with
slicing configuration.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 710bd355ed8e..6a802497834c 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -911,9 +911,6 @@ static int qaic_attach_slicing_bo(struct qaic_device *qdev, struct qaic_bo *bo,
 		return -ENOSPC;
 	}
 
-	bo->sliced = true;
-	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
-
 	return 0;
 }
 
@@ -1015,6 +1012,8 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	if (args->hdr.dir == DMA_TO_DEVICE)
 		dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, args->hdr.dir);
 
+	bo->sliced = true;
+	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
 	drm_gem_object_put(obj);
 	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
-- 
2.40.1

[PATCH 4/7] accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

qaic_attach_slicing_bo() updates slicing config on BO. Use the existing
function qaic_free_slices_bo() to remove slicing config done in
qaic_attach_slicing_bo().

Use qaic_free_slices_bo() to cleanup release_dbc()

This would be helpful when we introduce a new IOCTL to detach slicing
configuration onto a BO.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 6a802497834c..c4b8b4bf0200 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -154,6 +154,7 @@ static void free_slice(struct kref *kref)
 {
 	struct bo_slice *slice = container_of(kref, struct bo_slice, ref_count);
 
+	slice->bo->total_slice_nents -= slice->nents;
 	list_del(&slice->slice);
 	drm_gem_object_put(&slice->bo->base);
 	sg_free_table(slice->sgt);
@@ -890,6 +891,9 @@ static void qaic_free_slices_bo(struct qaic_bo *bo)
 
 	list_for_each_entry_safe(slice, temp, &bo->slices, slice)
 		kref_put(&slice->ref_count, free_slice);
+	if (WARN_ON_ONCE(bo->total_slice_nents != 0))
+		bo->total_slice_nents = 0;
+	bo->nr_slice = 0;
 }
 
 static int qaic_attach_slicing_bo(struct qaic_device *qdev, struct qaic_bo *bo,
@@ -1851,7 +1855,6 @@ void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id)
 
 void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 {
-	struct bo_slice *slice, *slice_temp;
 	struct qaic_bo *bo, *bo_temp;
 	struct dma_bridge_chan *dbc;
 
@@ -1869,12 +1872,10 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	dbc->usr = NULL;
 
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
+		qaic_free_slices_bo(bo);
 		qaic_unprepare_bo(qdev, bo);
-		list_for_each_entry_safe(slice, slice_temp, &bo->slices, slice)
-			kref_put(&slice->ref_count, free_slice);
 		bo->sliced = false;
 		INIT_LIST_HEAD(&bo->slices);
-		bo->total_slice_nents = 0;
 		bo->nr_slice_xfer_done = 0;
 		bo->queued = false;
 		bo->req_id = 0;
-- 
2.40.1

[PATCH 4/7] accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

qaic_attach_slicing_bo() updates slicing config on BO. Use the existing
function qaic_free_slices_bo() to remove slicing config done in
qaic_attach_slicing_bo().

Use qaic_free_slices_bo() to cleanup release_dbc()

This would be helpful when we introduce a new IOCTL to detach slicing
configuration onto a BO.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 6a802497834c..c4b8b4bf0200 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -154,6 +154,7 @@ static void free_slice(struct kref *kref)
 {
 	struct bo_slice *slice = container_of(kref, struct bo_slice, ref_count);
 
+	slice->bo->total_slice_nents -= slice->nents;
 	list_del(&slice->slice);
 	drm_gem_object_put(&slice->bo->base);
 	sg_free_table(slice->sgt);
@@ -890,6 +891,9 @@ static void qaic_free_slices_bo(struct qaic_bo *bo)
 
 	list_for_each_entry_safe(slice, temp, &bo->slices, slice)
 		kref_put(&slice->ref_count, free_slice);
+	if (WARN_ON_ONCE(bo->total_slice_nents != 0))
+		bo->total_slice_nents = 0;
+	bo->nr_slice = 0;
 }
 
 static int qaic_attach_slicing_bo(struct qaic_device *qdev, struct qaic_bo *bo,
@@ -1851,7 +1855,6 @@ void wakeup_dbc(struct qaic_device *qdev, u32 dbc_id)
 
 void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 {
-	struct bo_slice *slice, *slice_temp;
 	struct qaic_bo *bo, *bo_temp;
 	struct dma_bridge_chan *dbc;
 
@@ -1869,12 +1872,10 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	dbc->usr = NULL;
 
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
+		qaic_free_slices_bo(bo);
 		qaic_unprepare_bo(qdev, bo);
-		list_for_each_entry_safe(slice, slice_temp, &bo->slices, slice)
-			kref_put(&slice->ref_count, free_slice);
 		bo->sliced = false;
 		INIT_LIST_HEAD(&bo->slices);
-		bo->total_slice_nents = 0;
 		bo->nr_slice_xfer_done = 0;
 		bo->queued = false;
 		bo->req_id = 0;
-- 
2.40.1

[PATCH 5/7] accel/qaic: Clean up BO during flushing of transfer list

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Variables that are set while adding the corresponding BO in transfer list
should be cleaned when flushing them out of transfer list prematurely.

After this patch we do not need some of the cleanup done in release_dbc()

This patch would also pave the way to have a central location to clean BO,
during an undesired situation.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index c4b8b4bf0200..6e44e00937af 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1808,6 +1808,12 @@ static void empty_xfer_list(struct qaic_device *qdev, struct dma_bridge_chan *db
 		bo->queued = false;
 		list_del(&bo->xfer_list);
 		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+		bo->nr_slice_xfer_done = 0;
+		bo->req_id = 0;
+		bo->perf_stats.req_received_ts = 0;
+		bo->perf_stats.req_submit_ts = 0;
+		bo->perf_stats.req_processed_ts = 0;
+		bo->perf_stats.queue_level_before = 0;
 		dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
 		complete_all(&bo->xfer_done);
 		drm_gem_object_put(&bo->base);
@@ -1876,16 +1882,8 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 		qaic_unprepare_bo(qdev, bo);
 		bo->sliced = false;
 		INIT_LIST_HEAD(&bo->slices);
-		bo->nr_slice_xfer_done = 0;
-		bo->queued = false;
-		bo->req_id = 0;
 		init_completion(&bo->xfer_done);
-		complete_all(&bo->xfer_done);
 		list_del(&bo->bo_list);
-		bo->perf_stats.req_received_ts = 0;
-		bo->perf_stats.req_submit_ts = 0;
-		bo->perf_stats.req_processed_ts = 0;
-		bo->perf_stats.queue_level_before = 0;
 	}
 
 	dbc->in_use = false;
-- 
2.40.1

[PATCH 5/7] accel/qaic: Clean up BO during flushing of transfer list

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Variables that are set while adding the corresponding BO in transfer list
should be cleaned when flushing them out of transfer list prematurely.

After this patch we do not need some of the cleanup done in release_dbc()

This patch would also pave the way to have a central location to clean BO,
during an undesired situation.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index c4b8b4bf0200..6e44e00937af 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -1808,6 +1808,12 @@ static void empty_xfer_list(struct qaic_device *qdev, struct dma_bridge_chan *db
 		bo->queued = false;
 		list_del(&bo->xfer_list);
 		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+		bo->nr_slice_xfer_done = 0;
+		bo->req_id = 0;
+		bo->perf_stats.req_received_ts = 0;
+		bo->perf_stats.req_submit_ts = 0;
+		bo->perf_stats.req_processed_ts = 0;
+		bo->perf_stats.queue_level_before = 0;
 		dma_sync_sgtable_for_cpu(&qdev->pdev->dev, bo->sgt, bo->dir);
 		complete_all(&bo->xfer_done);
 		drm_gem_object_put(&bo->base);
@@ -1876,16 +1882,8 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 		qaic_unprepare_bo(qdev, bo);
 		bo->sliced = false;
 		INIT_LIST_HEAD(&bo->slices);
-		bo->nr_slice_xfer_done = 0;
-		bo->queued = false;
-		bo->req_id = 0;
 		init_completion(&bo->xfer_done);
-		complete_all(&bo->xfer_done);
 		list_del(&bo->bo_list);
-		bo->perf_stats.req_received_ts = 0;
-		bo->perf_stats.req_submit_ts = 0;
-		bo->perf_stats.req_processed_ts = 0;
-		bo->perf_stats.queue_level_before = 0;
 	}
 
 	dbc->in_use = false;
-- 
2.40.1

[PATCH 6/7] accel/qaic: Create a function to initialize BO

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

This makes sure that we have a single place to initialize and
re-initialize BO.

Use this new API to cleanup release_dbc()

We will need this for next patch to detach slicing to a BO.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 6e44e00937af..2acb9dbac88b 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -635,6 +635,18 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
 	.vm_ops = &drm_vm_ops,
 };
 
+static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
+{
+	if (reinit) {
+		bo->sliced = false;
+		reinit_completion(&bo->xfer_done);
+	} else {
+		init_completion(&bo->xfer_done);
+	}
+	complete_all(&bo->xfer_done);
+	INIT_LIST_HEAD(&bo->slices);
+}
+
 static struct qaic_bo *qaic_alloc_init_bo(void)
 {
 	struct qaic_bo *bo;
@@ -643,9 +655,7 @@ static struct qaic_bo *qaic_alloc_init_bo(void)
 	if (!bo)
 		return ERR_PTR(-ENOMEM);
 
-	INIT_LIST_HEAD(&bo->slices);
-	init_completion(&bo->xfer_done);
-	complete_all(&bo->xfer_done);
+	qaic_init_bo(bo, false);
 
 	return bo;
 }
@@ -1880,9 +1890,7 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
 		qaic_free_slices_bo(bo);
 		qaic_unprepare_bo(qdev, bo);
-		bo->sliced = false;
-		INIT_LIST_HEAD(&bo->slices);
-		init_completion(&bo->xfer_done);
+		qaic_init_bo(bo, true);
 		list_del(&bo->bo_list);
 	}
 
-- 
2.40.1

[PATCH 6/7] accel/qaic: Create a function to initialize BO

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

This makes sure that we have a single place to initialize and
re-initialize BO.

Use this new API to cleanup release_dbc()

We will need this for next patch to detach slicing to a BO.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/accel/qaic/qaic_data.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 6e44e00937af..2acb9dbac88b 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -635,6 +635,18 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
 	.vm_ops = &drm_vm_ops,
 };
 
+static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
+{
+	if (reinit) {
+		bo->sliced = false;
+		reinit_completion(&bo->xfer_done);
+	} else {
+		init_completion(&bo->xfer_done);
+	}
+	complete_all(&bo->xfer_done);
+	INIT_LIST_HEAD(&bo->slices);
+}
+
 static struct qaic_bo *qaic_alloc_init_bo(void)
 {
 	struct qaic_bo *bo;
@@ -643,9 +655,7 @@ static struct qaic_bo *qaic_alloc_init_bo(void)
 	if (!bo)
 		return ERR_PTR(-ENOMEM);
 
-	INIT_LIST_HEAD(&bo->slices);
-	init_completion(&bo->xfer_done);
-	complete_all(&bo->xfer_done);
+	qaic_init_bo(bo, false);
 
 	return bo;
 }
@@ -1880,9 +1890,7 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
 		qaic_free_slices_bo(bo);
 		qaic_unprepare_bo(qdev, bo);
-		bo->sliced = false;
-		INIT_LIST_HEAD(&bo->slices);
-		init_completion(&bo->xfer_done);
+		qaic_init_bo(bo, true);
 		list_del(&bo->bo_list);
 	}
 
-- 
2.40.1

Re: [PATCH 6/7] accel/qaic: Create a function to initialize BO

[Stanislaw Gruszka]

On Fri, Sep 01, 2023 at 11:22:46AM -0600, Jeffrey Hugo wrote:
> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> 
> This makes sure that we have a single place to initialize and
> re-initialize BO.
> 
> Use this new API to cleanup release_dbc()
> 
> We will need this for next patch to detach slicing to a BO.
> 
> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> ---
>  drivers/accel/qaic/qaic_data.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index 6e44e00937af..2acb9dbac88b 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
> @@ -635,6 +635,18 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
>  	.vm_ops = &drm_vm_ops,
>  };
>  
> +static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
> +{
> +	if (reinit) {
> +		bo->sliced = false;
> +		reinit_completion(&bo->xfer_done);
> +	} else {
> +		init_completion(&bo->xfer_done);
> +	}
> +	complete_all(&bo->xfer_done);
Why do you need complete_all() here ? 

Regards
Stanislaw

Re: [PATCH 6/7] accel/qaic: Create a function to initialize BO

[Stanislaw Gruszka]

On Fri, Sep 01, 2023 at 11:22:46AM -0600, Jeffrey Hugo wrote:
> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> 
> This makes sure that we have a single place to initialize and
> re-initialize BO.
> 
> Use this new API to cleanup release_dbc()
> 
> We will need this for next patch to detach slicing to a BO.
> 
> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> ---
>  drivers/accel/qaic/qaic_data.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index 6e44e00937af..2acb9dbac88b 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
> @@ -635,6 +635,18 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
>  	.vm_ops = &drm_vm_ops,
>  };
>  
> +static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
> +{
> +	if (reinit) {
> +		bo->sliced = false;
> +		reinit_completion(&bo->xfer_done);
> +	} else {
> +		init_completion(&bo->xfer_done);
> +	}
> +	complete_all(&bo->xfer_done);
Why do you need complete_all() here ? 

Regards
Stanislaw

Re: [PATCH 6/7] accel/qaic: Create a function to initialize BO

[Jeffrey Hugo]

On 9/17/2023 2:48 AM, Stanislaw Gruszka wrote:
> On Fri, Sep 01, 2023 at 11:22:46AM -0600, Jeffrey Hugo wrote:
>> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>>
>> This makes sure that we have a single place to initialize and
>> re-initialize BO.
>>
>> Use this new API to cleanup release_dbc()
>>
>> We will need this for next patch to detach slicing to a BO.
>>
>> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> ---
>>   drivers/accel/qaic/qaic_data.c | 20 ++++++++++++++------
>>   1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
>> index 6e44e00937af..2acb9dbac88b 100644
>> --- a/drivers/accel/qaic/qaic_data.c
>> +++ b/drivers/accel/qaic/qaic_data.c
>> @@ -635,6 +635,18 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
>>   	.vm_ops = &drm_vm_ops,
>>   };
>>   
>> +static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
>> +{
>> +	if (reinit) {
>> +		bo->sliced = false;
>> +		reinit_completion(&bo->xfer_done);
>> +	} else {
>> +		init_completion(&bo->xfer_done);
>> +	}
>> +	complete_all(&bo->xfer_done);
> Why do you need complete_all() here ?

This is moved from qaic_alloc_init_bo().

This puts the BO in a state where the wait_exec ioctl will fall through 
and return if userspace immediately calls it after allocating the BO, 
prior to submitting the BO to hardware.  Otherwise we need a special, 
one off check to see that the BO was never submitted to the hardware and 
handle that edge case.

-Jeff

Re: [PATCH 6/7] accel/qaic: Create a function to initialize BO

[Jeffrey Hugo]

On 9/17/2023 2:48 AM, Stanislaw Gruszka wrote:
> On Fri, Sep 01, 2023 at 11:22:46AM -0600, Jeffrey Hugo wrote:
>> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>>
>> This makes sure that we have a single place to initialize and
>> re-initialize BO.
>>
>> Use this new API to cleanup release_dbc()
>>
>> We will need this for next patch to detach slicing to a BO.
>>
>> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> ---
>>   drivers/accel/qaic/qaic_data.c | 20 ++++++++++++++------
>>   1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
>> index 6e44e00937af..2acb9dbac88b 100644
>> --- a/drivers/accel/qaic/qaic_data.c
>> +++ b/drivers/accel/qaic/qaic_data.c
>> @@ -635,6 +635,18 @@ static const struct drm_gem_object_funcs qaic_gem_funcs = {
>>   	.vm_ops = &drm_vm_ops,
>>   };
>>   
>> +static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
>> +{
>> +	if (reinit) {
>> +		bo->sliced = false;
>> +		reinit_completion(&bo->xfer_done);
>> +	} else {
>> +		init_completion(&bo->xfer_done);
>> +	}
>> +	complete_all(&bo->xfer_done);
> Why do you need complete_all() here ?

This is moved from qaic_alloc_init_bo().

This puts the BO in a state where the wait_exec ioctl will fall through 
and return if userspace immediately calls it after allocating the BO, 
prior to submitting the BO to hardware.  Otherwise we need a special, 
one off check to see that the BO was never submitted to the hardware and 
handle that edge case.

-Jeff

[PATCH 7/7] accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Once a BO is attached with slicing configuration that BO can only be used
for that particular setting. With this new feature user can detach slicing
configuration off an already sliced BO and attach new slicing configuration
using QAIC_ATTACH_SLICE_BO.

This will support BO recycling.

detach_slice_bo() detaches slicing configuration from a BO. This new
helper function can also be used in release_dbc() as we are doing the
exact same thing.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
[jhugo: add documentation for new ioctl]
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 Documentation/accel/qaic/qaic.rst |  10 +++
 drivers/accel/qaic/qaic.h         |   4 +-
 drivers/accel/qaic/qaic_data.c    | 119 +++++++++++++++++++++++++++---
 drivers/accel/qaic/qaic_drv.c     |   1 +
 include/uapi/drm/qaic_accel.h     |  12 +++
 5 files changed, 135 insertions(+), 11 deletions(-)

diff --git a/Documentation/accel/qaic/qaic.rst b/Documentation/accel/qaic/qaic.rst
index 72a70ab6e3a8..c88502383136 100644
--- a/Documentation/accel/qaic/qaic.rst
+++ b/Documentation/accel/qaic/qaic.rst
@@ -123,6 +123,16 @@ DRM_IOCTL_QAIC_PART_DEV
   AIC100 device and can be used for limiting a process to some subset of
   resources.
 
+DRM_IOCTL_QAIC_DETACH_SLICE_BO
+  This IOCTL allows userspace to remove the slicing information from a BO that
+  was originally provided by a call to DRM_IOCTL_QAIC_ATTACH_SLICE_BO. This
+  is the inverse of DRM_IOCTL_QAIC_ATTACH_SLICE_BO. The BO must be idle for
+  DRM_IOCTL_QAIC_DETACH_SLICE_BO to be called. After a successful detach slice
+  operation the BO may have new slicing information attached with a new call
+  to DRM_IOCTL_QAIC_ATTACH_SLICE_BO. After detach slice, the BO cannot be
+  executed until after a new attach slice operation. Combining attach slice
+  and detach slice calls allows userspace to use a BO with multiple workloads.
+
 Userspace Client Isolation
 ==========================
 
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 27cf66dbd5a5..28f1e81a1465 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -219,7 +219,8 @@ struct qaic_bo {
 		 */
 		u32		queue_level_before;
 	} perf_stats;
-
+	/* Synchronizes BO operations */
+	struct mutex		lock;
 };
 
 struct bo_slice {
@@ -275,6 +276,7 @@ int qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *f
 int qaic_partial_execute_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
 int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
 int qaic_perf_stats_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
+int qaic_detach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
 void irq_polling_work(struct work_struct *work);
 
 #endif /* _QAIC_H_ */
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 2acb9dbac88b..c90fa6a430f6 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -624,6 +624,7 @@ static void qaic_free_object(struct drm_gem_object *obj)
 		qaic_free_sgt(bo->sgt);
 	}
 
+	mutex_destroy(&bo->lock);
 	drm_gem_object_release(obj);
 	kfree(bo);
 }
@@ -641,6 +642,7 @@ static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
 		bo->sliced = false;
 		reinit_completion(&bo->xfer_done);
 	} else {
+		mutex_init(&bo->lock);
 		init_completion(&bo->xfer_done);
 	}
 	complete_all(&bo->xfer_done);
@@ -1002,10 +1004,13 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	}
 
 	bo = to_qaic_bo(obj);
+	ret = mutex_lock_interruptible(&bo->lock);
+	if (ret)
+		goto put_bo;
 
 	if (bo->sliced) {
 		ret = -EINVAL;
-		goto put_bo;
+		goto unlock_bo;
 	}
 
 	dbc = &qdev->dbc[args->hdr.dbc_id];
@@ -1029,7 +1034,7 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	bo->sliced = true;
 	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
-	drm_gem_object_put(obj);
+	mutex_unlock(&bo->lock);
 	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
 	srcu_read_unlock(&usr->qddev_lock, usr_rcu_id);
 
@@ -1039,6 +1044,8 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	qaic_unprepare_bo(qdev, bo);
 unlock_ch_srcu:
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
+unlock_bo:
+	mutex_unlock(&bo->lock);
 put_bo:
 	drm_gem_object_put(obj);
 free_slice_ent:
@@ -1193,15 +1200,18 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 		}
 
 		bo = to_qaic_bo(obj);
+		ret = mutex_lock_interruptible(&bo->lock);
+		if (ret)
+			goto failed_to_send_bo;
 
 		if (!bo->sliced) {
 			ret = -EINVAL;
-			goto failed_to_send_bo;
+			goto unlock_bo;
 		}
 
 		if (is_partial && pexec[i].resize > bo->base.size) {
 			ret = -EINVAL;
-			goto failed_to_send_bo;
+			goto unlock_bo;
 		}
 
 		spin_lock_irqsave(&dbc->xfer_lock, flags);
@@ -1210,7 +1220,7 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 		if (queued) {
 			spin_unlock_irqrestore(&dbc->xfer_lock, flags);
 			ret = -EINVAL;
-			goto failed_to_send_bo;
+			goto unlock_bo;
 		}
 
 		bo->req_id = dbc->next_req_id++;
@@ -1241,17 +1251,20 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 			if (ret) {
 				bo->queued = false;
 				spin_unlock_irqrestore(&dbc->xfer_lock, flags);
-				goto failed_to_send_bo;
+				goto unlock_bo;
 			}
 		}
 		reinit_completion(&bo->xfer_done);
 		list_add_tail(&bo->xfer_list, &dbc->xfer_list);
 		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
 		dma_sync_sgtable_for_device(&qdev->pdev->dev, bo->sgt, bo->dir);
+		mutex_unlock(&bo->lock);
 	}
 
 	return 0;
 
+unlock_bo:
+	mutex_unlock(&bo->lock);
 failed_to_send_bo:
 	if (likely(obj))
 		drm_gem_object_put(obj);
@@ -1807,6 +1820,91 @@ int qaic_perf_stats_bo_ioctl(struct drm_device *dev, void *data, struct drm_file
 	return ret;
 }
 
+static void detach_slice_bo(struct qaic_device *qdev, struct qaic_bo *bo)
+{
+	qaic_free_slices_bo(bo);
+	qaic_unprepare_bo(qdev, bo);
+	qaic_init_bo(bo, true);
+	list_del(&bo->bo_list);
+	drm_gem_object_put(&bo->base);
+}
+
+int qaic_detach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv)
+{
+	struct qaic_detach_slice *args = data;
+	int rcu_id, usr_rcu_id, qdev_rcu_id;
+	struct dma_bridge_chan *dbc;
+	struct drm_gem_object *obj;
+	struct qaic_device *qdev;
+	struct qaic_user *usr;
+	unsigned long flags;
+	struct qaic_bo *bo;
+	int ret;
+
+	if (args->pad != 0)
+		return -EINVAL;
+
+	usr = file_priv->driver_priv;
+	usr_rcu_id = srcu_read_lock(&usr->qddev_lock);
+	if (!usr->qddev) {
+		ret = -ENODEV;
+		goto unlock_usr_srcu;
+	}
+
+	qdev = usr->qddev->qdev;
+	qdev_rcu_id = srcu_read_lock(&qdev->dev_lock);
+	if (qdev->in_reset) {
+		ret = -ENODEV;
+		goto unlock_dev_srcu;
+	}
+
+	obj = drm_gem_object_lookup(file_priv, args->handle);
+	if (!obj) {
+		ret = -ENOENT;
+		goto unlock_dev_srcu;
+	}
+
+	bo = to_qaic_bo(obj);
+	ret = mutex_lock_interruptible(&bo->lock);
+	if (ret)
+		goto put_bo;
+
+	if (!bo->sliced) {
+		ret = -EINVAL;
+		goto unlock_bo;
+	}
+
+	dbc = bo->dbc;
+	rcu_id = srcu_read_lock(&dbc->ch_lock);
+	if (dbc->usr != usr) {
+		ret = -EINVAL;
+		goto unlock_ch_srcu;
+	}
+
+	/* Check if BO is committed to H/W for DMA */
+	spin_lock_irqsave(&dbc->xfer_lock, flags);
+	if (bo->queued) {
+		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+		ret = -EBUSY;
+		goto unlock_ch_srcu;
+	}
+	spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+
+	detach_slice_bo(qdev, bo);
+
+unlock_ch_srcu:
+	srcu_read_unlock(&dbc->ch_lock, rcu_id);
+unlock_bo:
+	mutex_unlock(&bo->lock);
+put_bo:
+	drm_gem_object_put(obj);
+unlock_dev_srcu:
+	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
+unlock_usr_srcu:
+	srcu_read_unlock(&usr->qddev_lock, usr_rcu_id);
+	return ret;
+}
+
 static void empty_xfer_list(struct qaic_device *qdev, struct dma_bridge_chan *dbc)
 {
 	unsigned long flags;
@@ -1888,10 +1986,11 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	dbc->usr = NULL;
 
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
-		qaic_free_slices_bo(bo);
-		qaic_unprepare_bo(qdev, bo);
-		qaic_init_bo(bo, true);
-		list_del(&bo->bo_list);
+		drm_gem_object_get(&bo->base);
+		mutex_lock(&bo->lock);
+		detach_slice_bo(qdev, bo);
+		mutex_unlock(&bo->lock);
+		drm_gem_object_put(&bo->base);
 	}
 
 	dbc->in_use = false;
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index b5de82e6eb4d..e2bfb4eaf852 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -150,6 +150,7 @@ static const struct drm_ioctl_desc qaic_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(QAIC_PARTIAL_EXECUTE_BO, qaic_partial_execute_bo_ioctl, 0),
 	DRM_IOCTL_DEF_DRV(QAIC_WAIT_BO, qaic_wait_bo_ioctl, 0),
 	DRM_IOCTL_DEF_DRV(QAIC_PERF_STATS_BO, qaic_perf_stats_bo_ioctl, 0),
+	DRM_IOCTL_DEF_DRV(QAIC_DETACH_SLICE_BO, qaic_detach_slice_bo_ioctl, 0),
 };
 
 static const struct drm_driver qaic_accel_driver = {
diff --git a/include/uapi/drm/qaic_accel.h b/include/uapi/drm/qaic_accel.h
index f89880b7bfb6..43ac5d864512 100644
--- a/include/uapi/drm/qaic_accel.h
+++ b/include/uapi/drm/qaic_accel.h
@@ -372,6 +372,16 @@ struct qaic_perf_stats_entry {
 	__u32 pad;
 };
 
+/**
+ * struct qaic_detach_slice - Detaches slicing configuration from BO.
+ * @handle: In. GEM handle of the BO to detach slicing configuration.
+ * @pad: Structure padding. Must be 0.
+ */
+struct qaic_detach_slice {
+	__u32 handle;
+	__u32 pad;
+};
+
 #define DRM_QAIC_MANAGE				0x00
 #define DRM_QAIC_CREATE_BO			0x01
 #define DRM_QAIC_MMAP_BO			0x02
@@ -380,6 +390,7 @@ struct qaic_perf_stats_entry {
 #define DRM_QAIC_PARTIAL_EXECUTE_BO		0x05
 #define DRM_QAIC_WAIT_BO			0x06
 #define DRM_QAIC_PERF_STATS_BO			0x07
+#define DRM_QAIC_DETACH_SLICE_BO		0x08
 
 #define DRM_IOCTL_QAIC_MANAGE			DRM_IOWR(DRM_COMMAND_BASE + DRM_QAIC_MANAGE, struct qaic_manage_msg)
 #define DRM_IOCTL_QAIC_CREATE_BO		DRM_IOWR(DRM_COMMAND_BASE + DRM_QAIC_CREATE_BO,	struct qaic_create_bo)
@@ -389,6 +400,7 @@ struct qaic_perf_stats_entry {
 #define DRM_IOCTL_QAIC_PARTIAL_EXECUTE_BO	DRM_IOW(DRM_COMMAND_BASE + DRM_QAIC_PARTIAL_EXECUTE_BO,	struct qaic_execute)
 #define DRM_IOCTL_QAIC_WAIT_BO			DRM_IOW(DRM_COMMAND_BASE + DRM_QAIC_WAIT_BO, struct qaic_wait)
 #define DRM_IOCTL_QAIC_PERF_STATS_BO		DRM_IOWR(DRM_COMMAND_BASE + DRM_QAIC_PERF_STATS_BO, struct qaic_perf_stats)
+#define DRM_IOCTL_QAIC_DETACH_SLICE_BO		DRM_IOW(DRM_COMMAND_BASE + DRM_QAIC_DETACH_SLICE_BO, struct qaic_detach_slice)
 
 #if defined(__cplusplus)
 }
-- 
2.40.1

[PATCH 7/7] accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

[Jeffrey Hugo]

From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>

Once a BO is attached with slicing configuration that BO can only be used
for that particular setting. With this new feature user can detach slicing
configuration off an already sliced BO and attach new slicing configuration
using QAIC_ATTACH_SLICE_BO.

This will support BO recycling.

detach_slice_bo() detaches slicing configuration from a BO. This new
helper function can also be used in release_dbc() as we are doing the
exact same thing.

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
[jhugo: add documentation for new ioctl]
Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 Documentation/accel/qaic/qaic.rst |  10 +++
 drivers/accel/qaic/qaic.h         |   4 +-
 drivers/accel/qaic/qaic_data.c    | 119 +++++++++++++++++++++++++++---
 drivers/accel/qaic/qaic_drv.c     |   1 +
 include/uapi/drm/qaic_accel.h     |  12 +++
 5 files changed, 135 insertions(+), 11 deletions(-)

diff --git a/Documentation/accel/qaic/qaic.rst b/Documentation/accel/qaic/qaic.rst
index 72a70ab6e3a8..c88502383136 100644
--- a/Documentation/accel/qaic/qaic.rst
+++ b/Documentation/accel/qaic/qaic.rst
@@ -123,6 +123,16 @@ DRM_IOCTL_QAIC_PART_DEV
   AIC100 device and can be used for limiting a process to some subset of
   resources.
 
+DRM_IOCTL_QAIC_DETACH_SLICE_BO
+  This IOCTL allows userspace to remove the slicing information from a BO that
+  was originally provided by a call to DRM_IOCTL_QAIC_ATTACH_SLICE_BO. This
+  is the inverse of DRM_IOCTL_QAIC_ATTACH_SLICE_BO. The BO must be idle for
+  DRM_IOCTL_QAIC_DETACH_SLICE_BO to be called. After a successful detach slice
+  operation the BO may have new slicing information attached with a new call
+  to DRM_IOCTL_QAIC_ATTACH_SLICE_BO. After detach slice, the BO cannot be
+  executed until after a new attach slice operation. Combining attach slice
+  and detach slice calls allows userspace to use a BO with multiple workloads.
+
 Userspace Client Isolation
 ==========================
 
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 27cf66dbd5a5..28f1e81a1465 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -219,7 +219,8 @@ struct qaic_bo {
 		 */
 		u32		queue_level_before;
 	} perf_stats;
-
+	/* Synchronizes BO operations */
+	struct mutex		lock;
 };
 
 struct bo_slice {
@@ -275,6 +276,7 @@ int qaic_execute_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *f
 int qaic_partial_execute_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
 int qaic_wait_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
 int qaic_perf_stats_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
+int qaic_detach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);
 void irq_polling_work(struct work_struct *work);
 
 #endif /* _QAIC_H_ */
diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
index 2acb9dbac88b..c90fa6a430f6 100644
--- a/drivers/accel/qaic/qaic_data.c
+++ b/drivers/accel/qaic/qaic_data.c
@@ -624,6 +624,7 @@ static void qaic_free_object(struct drm_gem_object *obj)
 		qaic_free_sgt(bo->sgt);
 	}
 
+	mutex_destroy(&bo->lock);
 	drm_gem_object_release(obj);
 	kfree(bo);
 }
@@ -641,6 +642,7 @@ static void qaic_init_bo(struct qaic_bo *bo, bool reinit)
 		bo->sliced = false;
 		reinit_completion(&bo->xfer_done);
 	} else {
+		mutex_init(&bo->lock);
 		init_completion(&bo->xfer_done);
 	}
 	complete_all(&bo->xfer_done);
@@ -1002,10 +1004,13 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	}
 
 	bo = to_qaic_bo(obj);
+	ret = mutex_lock_interruptible(&bo->lock);
+	if (ret)
+		goto put_bo;
 
 	if (bo->sliced) {
 		ret = -EINVAL;
-		goto put_bo;
+		goto unlock_bo;
 	}
 
 	dbc = &qdev->dbc[args->hdr.dbc_id];
@@ -1029,7 +1034,7 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	bo->sliced = true;
 	list_add_tail(&bo->bo_list, &bo->dbc->bo_lists);
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
-	drm_gem_object_put(obj);
+	mutex_unlock(&bo->lock);
 	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
 	srcu_read_unlock(&usr->qddev_lock, usr_rcu_id);
 
@@ -1039,6 +1044,8 @@ int qaic_attach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_fi
 	qaic_unprepare_bo(qdev, bo);
 unlock_ch_srcu:
 	srcu_read_unlock(&dbc->ch_lock, rcu_id);
+unlock_bo:
+	mutex_unlock(&bo->lock);
 put_bo:
 	drm_gem_object_put(obj);
 free_slice_ent:
@@ -1193,15 +1200,18 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 		}
 
 		bo = to_qaic_bo(obj);
+		ret = mutex_lock_interruptible(&bo->lock);
+		if (ret)
+			goto failed_to_send_bo;
 
 		if (!bo->sliced) {
 			ret = -EINVAL;
-			goto failed_to_send_bo;
+			goto unlock_bo;
 		}
 
 		if (is_partial && pexec[i].resize > bo->base.size) {
 			ret = -EINVAL;
-			goto failed_to_send_bo;
+			goto unlock_bo;
 		}
 
 		spin_lock_irqsave(&dbc->xfer_lock, flags);
@@ -1210,7 +1220,7 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 		if (queued) {
 			spin_unlock_irqrestore(&dbc->xfer_lock, flags);
 			ret = -EINVAL;
-			goto failed_to_send_bo;
+			goto unlock_bo;
 		}
 
 		bo->req_id = dbc->next_req_id++;
@@ -1241,17 +1251,20 @@ static int send_bo_list_to_device(struct qaic_device *qdev, struct drm_file *fil
 			if (ret) {
 				bo->queued = false;
 				spin_unlock_irqrestore(&dbc->xfer_lock, flags);
-				goto failed_to_send_bo;
+				goto unlock_bo;
 			}
 		}
 		reinit_completion(&bo->xfer_done);
 		list_add_tail(&bo->xfer_list, &dbc->xfer_list);
 		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
 		dma_sync_sgtable_for_device(&qdev->pdev->dev, bo->sgt, bo->dir);
+		mutex_unlock(&bo->lock);
 	}
 
 	return 0;
 
+unlock_bo:
+	mutex_unlock(&bo->lock);
 failed_to_send_bo:
 	if (likely(obj))
 		drm_gem_object_put(obj);
@@ -1807,6 +1820,91 @@ int qaic_perf_stats_bo_ioctl(struct drm_device *dev, void *data, struct drm_file
 	return ret;
 }
 
+static void detach_slice_bo(struct qaic_device *qdev, struct qaic_bo *bo)
+{
+	qaic_free_slices_bo(bo);
+	qaic_unprepare_bo(qdev, bo);
+	qaic_init_bo(bo, true);
+	list_del(&bo->bo_list);
+	drm_gem_object_put(&bo->base);
+}
+
+int qaic_detach_slice_bo_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv)
+{
+	struct qaic_detach_slice *args = data;
+	int rcu_id, usr_rcu_id, qdev_rcu_id;
+	struct dma_bridge_chan *dbc;
+	struct drm_gem_object *obj;
+	struct qaic_device *qdev;
+	struct qaic_user *usr;
+	unsigned long flags;
+	struct qaic_bo *bo;
+	int ret;
+
+	if (args->pad != 0)
+		return -EINVAL;
+
+	usr = file_priv->driver_priv;
+	usr_rcu_id = srcu_read_lock(&usr->qddev_lock);
+	if (!usr->qddev) {
+		ret = -ENODEV;
+		goto unlock_usr_srcu;
+	}
+
+	qdev = usr->qddev->qdev;
+	qdev_rcu_id = srcu_read_lock(&qdev->dev_lock);
+	if (qdev->in_reset) {
+		ret = -ENODEV;
+		goto unlock_dev_srcu;
+	}
+
+	obj = drm_gem_object_lookup(file_priv, args->handle);
+	if (!obj) {
+		ret = -ENOENT;
+		goto unlock_dev_srcu;
+	}
+
+	bo = to_qaic_bo(obj);
+	ret = mutex_lock_interruptible(&bo->lock);
+	if (ret)
+		goto put_bo;
+
+	if (!bo->sliced) {
+		ret = -EINVAL;
+		goto unlock_bo;
+	}
+
+	dbc = bo->dbc;
+	rcu_id = srcu_read_lock(&dbc->ch_lock);
+	if (dbc->usr != usr) {
+		ret = -EINVAL;
+		goto unlock_ch_srcu;
+	}
+
+	/* Check if BO is committed to H/W for DMA */
+	spin_lock_irqsave(&dbc->xfer_lock, flags);
+	if (bo->queued) {
+		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+		ret = -EBUSY;
+		goto unlock_ch_srcu;
+	}
+	spin_unlock_irqrestore(&dbc->xfer_lock, flags);
+
+	detach_slice_bo(qdev, bo);
+
+unlock_ch_srcu:
+	srcu_read_unlock(&dbc->ch_lock, rcu_id);
+unlock_bo:
+	mutex_unlock(&bo->lock);
+put_bo:
+	drm_gem_object_put(obj);
+unlock_dev_srcu:
+	srcu_read_unlock(&qdev->dev_lock, qdev_rcu_id);
+unlock_usr_srcu:
+	srcu_read_unlock(&usr->qddev_lock, usr_rcu_id);
+	return ret;
+}
+
 static void empty_xfer_list(struct qaic_device *qdev, struct dma_bridge_chan *dbc)
 {
 	unsigned long flags;
@@ -1888,10 +1986,11 @@ void release_dbc(struct qaic_device *qdev, u32 dbc_id)
 	dbc->usr = NULL;
 
 	list_for_each_entry_safe(bo, bo_temp, &dbc->bo_lists, bo_list) {
-		qaic_free_slices_bo(bo);
-		qaic_unprepare_bo(qdev, bo);
-		qaic_init_bo(bo, true);
-		list_del(&bo->bo_list);
+		drm_gem_object_get(&bo->base);
+		mutex_lock(&bo->lock);
+		detach_slice_bo(qdev, bo);
+		mutex_unlock(&bo->lock);
+		drm_gem_object_put(&bo->base);
 	}
 
 	dbc->in_use = false;
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index b5de82e6eb4d..e2bfb4eaf852 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -150,6 +150,7 @@ static const struct drm_ioctl_desc qaic_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(QAIC_PARTIAL_EXECUTE_BO, qaic_partial_execute_bo_ioctl, 0),
 	DRM_IOCTL_DEF_DRV(QAIC_WAIT_BO, qaic_wait_bo_ioctl, 0),
 	DRM_IOCTL_DEF_DRV(QAIC_PERF_STATS_BO, qaic_perf_stats_bo_ioctl, 0),
+	DRM_IOCTL_DEF_DRV(QAIC_DETACH_SLICE_BO, qaic_detach_slice_bo_ioctl, 0),
 };
 
 static const struct drm_driver qaic_accel_driver = {
diff --git a/include/uapi/drm/qaic_accel.h b/include/uapi/drm/qaic_accel.h
index f89880b7bfb6..43ac5d864512 100644
--- a/include/uapi/drm/qaic_accel.h
+++ b/include/uapi/drm/qaic_accel.h
@@ -372,6 +372,16 @@ struct qaic_perf_stats_entry {
 	__u32 pad;
 };
 
+/**
+ * struct qaic_detach_slice - Detaches slicing configuration from BO.
+ * @handle: In. GEM handle of the BO to detach slicing configuration.
+ * @pad: Structure padding. Must be 0.
+ */
+struct qaic_detach_slice {
+	__u32 handle;
+	__u32 pad;
+};
+
 #define DRM_QAIC_MANAGE				0x00
 #define DRM_QAIC_CREATE_BO			0x01
 #define DRM_QAIC_MMAP_BO			0x02
@@ -380,6 +390,7 @@ struct qaic_perf_stats_entry {
 #define DRM_QAIC_PARTIAL_EXECUTE_BO		0x05
 #define DRM_QAIC_WAIT_BO			0x06
 #define DRM_QAIC_PERF_STATS_BO			0x07
+#define DRM_QAIC_DETACH_SLICE_BO		0x08
 
 #define DRM_IOCTL_QAIC_MANAGE			DRM_IOWR(DRM_COMMAND_BASE + DRM_QAIC_MANAGE, struct qaic_manage_msg)
 #define DRM_IOCTL_QAIC_CREATE_BO		DRM_IOWR(DRM_COMMAND_BASE + DRM_QAIC_CREATE_BO,	struct qaic_create_bo)
@@ -389,6 +400,7 @@ struct qaic_perf_stats_entry {
 #define DRM_IOCTL_QAIC_PARTIAL_EXECUTE_BO	DRM_IOW(DRM_COMMAND_BASE + DRM_QAIC_PARTIAL_EXECUTE_BO,	struct qaic_execute)
 #define DRM_IOCTL_QAIC_WAIT_BO			DRM_IOW(DRM_COMMAND_BASE + DRM_QAIC_WAIT_BO, struct qaic_wait)
 #define DRM_IOCTL_QAIC_PERF_STATS_BO		DRM_IOWR(DRM_COMMAND_BASE + DRM_QAIC_PERF_STATS_BO, struct qaic_perf_stats)
+#define DRM_IOCTL_QAIC_DETACH_SLICE_BO		DRM_IOW(DRM_COMMAND_BASE + DRM_QAIC_DETACH_SLICE_BO, struct qaic_detach_slice)
 
 #if defined(__cplusplus)
 }
-- 
2.40.1

Re: [PATCH 7/7] accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

[Stanislaw Gruszka]

On Fri, Sep 01, 2023 at 11:22:47AM -0600, Jeffrey Hugo wrote:
> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> 
> Once a BO is attached with slicing configuration that BO can only be used
> for that particular setting. With this new feature user can detach slicing
> configuration off an already sliced BO and attach new slicing configuration
> using QAIC_ATTACH_SLICE_BO.
> 
> This will support BO recycling.
> 
> detach_slice_bo() detaches slicing configuration from a BO. This new
> helper function can also be used in release_dbc() as we are doing the
> exact same thing.
> 
> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> [jhugo: add documentation for new ioctl]
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
<snip>

> +	/* Check if BO is committed to H/W for DMA */
> +	spin_lock_irqsave(&dbc->xfer_lock, flags);
> +	if (bo->queued) {
> +		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
> +		ret = -EBUSY;
> +		goto unlock_ch_srcu;
> +	}
> +	spin_unlock_irqrestore(&dbc->xfer_lock, flags);

This looks like race condition. If some other thread will take the xfer_lock
and set bo->queued (HERE just after _unlock())  we will not return -EBUSY.
Something seems to be missing here or xfer_lock is not needed to protect
bo->queued.

Regards
Stanislaw

Re: [PATCH 7/7] accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

[Stanislaw Gruszka]

On Fri, Sep 01, 2023 at 11:22:47AM -0600, Jeffrey Hugo wrote:
> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> 
> Once a BO is attached with slicing configuration that BO can only be used
> for that particular setting. With this new feature user can detach slicing
> configuration off an already sliced BO and attach new slicing configuration
> using QAIC_ATTACH_SLICE_BO.
> 
> This will support BO recycling.
> 
> detach_slice_bo() detaches slicing configuration from a BO. This new
> helper function can also be used in release_dbc() as we are doing the
> exact same thing.
> 
> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> [jhugo: add documentation for new ioctl]
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
<snip>

> +	/* Check if BO is committed to H/W for DMA */
> +	spin_lock_irqsave(&dbc->xfer_lock, flags);
> +	if (bo->queued) {
> +		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
> +		ret = -EBUSY;
> +		goto unlock_ch_srcu;
> +	}
> +	spin_unlock_irqrestore(&dbc->xfer_lock, flags);

This looks like race condition. If some other thread will take the xfer_lock
and set bo->queued (HERE just after _unlock())  we will not return -EBUSY.
Something seems to be missing here or xfer_lock is not needed to protect
bo->queued.

Regards
Stanislaw

Re: [PATCH 7/7] accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

[Jeffrey Hugo]

On 9/17/2023 2:56 AM, Stanislaw Gruszka wrote:
> On Fri, Sep 01, 2023 at 11:22:47AM -0600, Jeffrey Hugo wrote:
>> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>>
>> Once a BO is attached with slicing configuration that BO can only be used
>> for that particular setting. With this new feature user can detach slicing
>> configuration off an already sliced BO and attach new slicing configuration
>> using QAIC_ATTACH_SLICE_BO.
>>
>> This will support BO recycling.
>>
>> detach_slice_bo() detaches slicing configuration from a BO. This new
>> helper function can also be used in release_dbc() as we are doing the
>> exact same thing.
>>
>> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> [jhugo: add documentation for new ioctl]
>> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> <snip>
> 
>> +	/* Check if BO is committed to H/W for DMA */
>> +	spin_lock_irqsave(&dbc->xfer_lock, flags);
>> +	if (bo->queued) {
>> +		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
>> +		ret = -EBUSY;
>> +		goto unlock_ch_srcu;
>> +	}
>> +	spin_unlock_irqrestore(&dbc->xfer_lock, flags);
> 
> This looks like race condition. If some other thread will take the xfer_lock
> and set bo->queued (HERE just after _unlock())  we will not return -EBUSY.
> Something seems to be missing here or xfer_lock is not needed to protect
> bo->queued.

The other thread would also need to take the bo->lock, which is held 
here and not released until after detach_slice_bo().  xfer_lock actually 
protects xfer_list, but bo->queued is a quick check to see if the bo is 
in the list, rather than iterating the list.  I can see how this is 
misleading.  I will ponder how to improve it.

-Jeff

Re: [PATCH 7/7] accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL

[Jeffrey Hugo]

On 9/17/2023 2:56 AM, Stanislaw Gruszka wrote:
> On Fri, Sep 01, 2023 at 11:22:47AM -0600, Jeffrey Hugo wrote:
>> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>>
>> Once a BO is attached with slicing configuration that BO can only be used
>> for that particular setting. With this new feature user can detach slicing
>> configuration off an already sliced BO and attach new slicing configuration
>> using QAIC_ATTACH_SLICE_BO.
>>
>> This will support BO recycling.
>>
>> detach_slice_bo() detaches slicing configuration from a BO. This new
>> helper function can also be used in release_dbc() as we are doing the
>> exact same thing.
>>
>> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
>> [jhugo: add documentation for new ioctl]
>> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> <snip>
> 
>> +	/* Check if BO is committed to H/W for DMA */
>> +	spin_lock_irqsave(&dbc->xfer_lock, flags);
>> +	if (bo->queued) {
>> +		spin_unlock_irqrestore(&dbc->xfer_lock, flags);
>> +		ret = -EBUSY;
>> +		goto unlock_ch_srcu;
>> +	}
>> +	spin_unlock_irqrestore(&dbc->xfer_lock, flags);
> 
> This looks like race condition. If some other thread will take the xfer_lock
> and set bo->queued (HERE just after _unlock())  we will not return -EBUSY.
> Something seems to be missing here or xfer_lock is not needed to protect
> bo->queued.

The other thread would also need to take the bo->lock, which is held 
here and not released until after detach_slice_bo().  xfer_lock actually 
protects xfer_list, but bo->queued is a quick check to see if the bo is 
in the list, rather than iterating the list.  I can see how this is 
misleading.  I will ponder how to improve it.

-Jeff

Re: [PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Stanislaw Gruszka]

On Fri, Sep 01, 2023 at 11:22:40AM -0600, Jeffrey Hugo wrote:
> A BO for a QAIC device has two states -
> 1. Allocated
> 2. Sliced
> 
> A BO can be allocated at any time, and is initialized in the allocated state.
> A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
> BO for use with an active workload.  Currently a BO in the sliced state can
> only be used with a single workload, and will only transition back to the
> allocated state once the workload is deactivated.
> 
> Userspace would like the ability to trigger a BO transition from the sliced
> state to the allocated state. This would support the usecase of a userspace
> client that has two active workloads, where the output of the first workload
> becomes the input of the second workload. Currently, the client would need
> two BOs, once for each workload, and copy from one BO to the other.
> 
> To support this usecase, we create the detach slice concept which is the
> inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
> DETACH_SLICE_BO ioctl that allows userspace to perform this operation.
> 
> Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
> a decent amount of code. This series starts with restructuring the common code
> for use in both ioctls before finally adding the DETACH_SLICE_BO.
> 
> Pranjal Ramajor Asha Kanojiya (7):
>   accel/qaic: Remove ->size field from struct qaic_bo
>   accel/qaic: Update BO metadata in a central location
>   accel/qaic: Declare BO 'sliced' after all the operations are complete
>   accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
>   accel/qaic: Clean up BO during flushing of transfer list
>   accel/qaic: Create a function to initialize BO
>   accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL
> 
>  Documentation/accel/qaic/qaic.rst |  10 ++
>  drivers/accel/qaic/qaic.h         |   6 +-
>  drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
>  drivers/accel/qaic/qaic_drv.c     |   1 +
>  include/uapi/drm/qaic_accel.h     |  24 +++-
>  5 files changed, 175 insertions(+), 53 deletions(-)


Do not see any serious issues with the set.

Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> for the whole series.

Re: [PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Stanislaw Gruszka]

On Fri, Sep 01, 2023 at 11:22:40AM -0600, Jeffrey Hugo wrote:
> A BO for a QAIC device has two states -
> 1. Allocated
> 2. Sliced
> 
> A BO can be allocated at any time, and is initialized in the allocated state.
> A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
> BO for use with an active workload.  Currently a BO in the sliced state can
> only be used with a single workload, and will only transition back to the
> allocated state once the workload is deactivated.
> 
> Userspace would like the ability to trigger a BO transition from the sliced
> state to the allocated state. This would support the usecase of a userspace
> client that has two active workloads, where the output of the first workload
> becomes the input of the second workload. Currently, the client would need
> two BOs, once for each workload, and copy from one BO to the other.
> 
> To support this usecase, we create the detach slice concept which is the
> inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
> DETACH_SLICE_BO ioctl that allows userspace to perform this operation.
> 
> Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
> a decent amount of code. This series starts with restructuring the common code
> for use in both ioctls before finally adding the DETACH_SLICE_BO.
> 
> Pranjal Ramajor Asha Kanojiya (7):
>   accel/qaic: Remove ->size field from struct qaic_bo
>   accel/qaic: Update BO metadata in a central location
>   accel/qaic: Declare BO 'sliced' after all the operations are complete
>   accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
>   accel/qaic: Clean up BO during flushing of transfer list
>   accel/qaic: Create a function to initialize BO
>   accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL
> 
>  Documentation/accel/qaic/qaic.rst |  10 ++
>  drivers/accel/qaic/qaic.h         |   6 +-
>  drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
>  drivers/accel/qaic/qaic_drv.c     |   1 +
>  include/uapi/drm/qaic_accel.h     |  24 +++-
>  5 files changed, 175 insertions(+), 53 deletions(-)


Do not see any serious issues with the set.

Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> for the whole series.

Re: [PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Jeffrey Hugo]

On 9/17/2023 2:58 AM, Stanislaw Gruszka wrote:
> On Fri, Sep 01, 2023 at 11:22:40AM -0600, Jeffrey Hugo wrote:
>> A BO for a QAIC device has two states -
>> 1. Allocated
>> 2. Sliced
>>
>> A BO can be allocated at any time, and is initialized in the allocated state.
>> A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
>> BO for use with an active workload.  Currently a BO in the sliced state can
>> only be used with a single workload, and will only transition back to the
>> allocated state once the workload is deactivated.
>>
>> Userspace would like the ability to trigger a BO transition from the sliced
>> state to the allocated state. This would support the usecase of a userspace
>> client that has two active workloads, where the output of the first workload
>> becomes the input of the second workload. Currently, the client would need
>> two BOs, once for each workload, and copy from one BO to the other.
>>
>> To support this usecase, we create the detach slice concept which is the
>> inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
>> DETACH_SLICE_BO ioctl that allows userspace to perform this operation.
>>
>> Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
>> a decent amount of code. This series starts with restructuring the common code
>> for use in both ioctls before finally adding the DETACH_SLICE_BO.
>>
>> Pranjal Ramajor Asha Kanojiya (7):
>>    accel/qaic: Remove ->size field from struct qaic_bo
>>    accel/qaic: Update BO metadata in a central location
>>    accel/qaic: Declare BO 'sliced' after all the operations are complete
>>    accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
>>    accel/qaic: Clean up BO during flushing of transfer list
>>    accel/qaic: Create a function to initialize BO
>>    accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL
>>
>>   Documentation/accel/qaic/qaic.rst |  10 ++
>>   drivers/accel/qaic/qaic.h         |   6 +-
>>   drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
>>   drivers/accel/qaic/qaic_drv.c     |   1 +
>>   include/uapi/drm/qaic_accel.h     |  24 +++-
>>   5 files changed, 175 insertions(+), 53 deletions(-)
> 
> 
> Do not see any serious issues with the set.
> 
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> for the whole series.

Thanks!

-Jeff

Re: [PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Jeffrey Hugo]

On 9/17/2023 2:58 AM, Stanislaw Gruszka wrote:
> On Fri, Sep 01, 2023 at 11:22:40AM -0600, Jeffrey Hugo wrote:
>> A BO for a QAIC device has two states -
>> 1. Allocated
>> 2. Sliced
>>
>> A BO can be allocated at any time, and is initialized in the allocated state.
>> A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
>> BO for use with an active workload.  Currently a BO in the sliced state can
>> only be used with a single workload, and will only transition back to the
>> allocated state once the workload is deactivated.
>>
>> Userspace would like the ability to trigger a BO transition from the sliced
>> state to the allocated state. This would support the usecase of a userspace
>> client that has two active workloads, where the output of the first workload
>> becomes the input of the second workload. Currently, the client would need
>> two BOs, once for each workload, and copy from one BO to the other.
>>
>> To support this usecase, we create the detach slice concept which is the
>> inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
>> DETACH_SLICE_BO ioctl that allows userspace to perform this operation.
>>
>> Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
>> a decent amount of code. This series starts with restructuring the common code
>> for use in both ioctls before finally adding the DETACH_SLICE_BO.
>>
>> Pranjal Ramajor Asha Kanojiya (7):
>>    accel/qaic: Remove ->size field from struct qaic_bo
>>    accel/qaic: Update BO metadata in a central location
>>    accel/qaic: Declare BO 'sliced' after all the operations are complete
>>    accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
>>    accel/qaic: Clean up BO during flushing of transfer list
>>    accel/qaic: Create a function to initialize BO
>>    accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL
>>
>>   Documentation/accel/qaic/qaic.rst |  10 ++
>>   drivers/accel/qaic/qaic.h         |   6 +-
>>   drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
>>   drivers/accel/qaic/qaic_drv.c     |   1 +
>>   include/uapi/drm/qaic_accel.h     |  24 +++-
>>   5 files changed, 175 insertions(+), 53 deletions(-)
> 
> 
> Do not see any serious issues with the set.
> 
> Reviewed-by: Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> for the whole series.

Thanks!

-Jeff

Re: [PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Jeffrey Hugo]

On 9/1/2023 11:22 AM, Jeffrey Hugo wrote:
> A BO for a QAIC device has two states -
> 1. Allocated
> 2. Sliced
> 
> A BO can be allocated at any time, and is initialized in the allocated state.
> A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
> BO for use with an active workload.  Currently a BO in the sliced state can
> only be used with a single workload, and will only transition back to the
> allocated state once the workload is deactivated.
> 
> Userspace would like the ability to trigger a BO transition from the sliced
> state to the allocated state. This would support the usecase of a userspace
> client that has two active workloads, where the output of the first workload
> becomes the input of the second workload. Currently, the client would need
> two BOs, once for each workload, and copy from one BO to the other.
> 
> To support this usecase, we create the detach slice concept which is the
> inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
> DETACH_SLICE_BO ioctl that allows userspace to perform this operation.
> 
> Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
> a decent amount of code. This series starts with restructuring the common code
> for use in both ioctls before finally adding the DETACH_SLICE_BO.
> 
> Pranjal Ramajor Asha Kanojiya (7):
>    accel/qaic: Remove ->size field from struct qaic_bo
>    accel/qaic: Update BO metadata in a central location
>    accel/qaic: Declare BO 'sliced' after all the operations are complete
>    accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
>    accel/qaic: Clean up BO during flushing of transfer list
>    accel/qaic: Create a function to initialize BO
>    accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL
> 
>   Documentation/accel/qaic/qaic.rst |  10 ++
>   drivers/accel/qaic/qaic.h         |   6 +-
>   drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
>   drivers/accel/qaic/qaic_drv.c     |   1 +
>   include/uapi/drm/qaic_accel.h     |  24 +++-
>   5 files changed, 175 insertions(+), 53 deletions(-)
> 

Pushed to drm-misc-next

-Jeff

Re: [PATCH 0/7] accel/qaic: Extend uAPI to support undoing ATTACH_SLICE_BO

[Jeffrey Hugo]

On 9/1/2023 11:22 AM, Jeffrey Hugo wrote:
> A BO for a QAIC device has two states -
> 1. Allocated
> 2. Sliced
> 
> A BO can be allocated at any time, and is initialized in the allocated state.
> A BO can transition to the sliced state via ATTACH_SLICE_BO.  This prepares the
> BO for use with an active workload.  Currently a BO in the sliced state can
> only be used with a single workload, and will only transition back to the
> allocated state once the workload is deactivated.
> 
> Userspace would like the ability to trigger a BO transition from the sliced
> state to the allocated state. This would support the usecase of a userspace
> client that has two active workloads, where the output of the first workload
> becomes the input of the second workload. Currently, the client would need
> two BOs, once for each workload, and copy from one BO to the other.
> 
> To support this usecase, we create the detach slice concept which is the
> inverse operation of ATTACH_SLICE_BO.  We extend the uAPI with a new
> DETACH_SLICE_BO ioctl that allows userspace to perform this operation.
> 
> Since ATTACH_SLICE_BO and DETACH_SLICE_BO are related operations, they share
> a decent amount of code. This series starts with restructuring the common code
> for use in both ioctls before finally adding the DETACH_SLICE_BO.
> 
> Pranjal Ramajor Asha Kanojiya (7):
>    accel/qaic: Remove ->size field from struct qaic_bo
>    accel/qaic: Update BO metadata in a central location
>    accel/qaic: Declare BO 'sliced' after all the operations are complete
>    accel/qaic: Undo slicing setup done in qaic_attach_slicing_bo()
>    accel/qaic: Clean up BO during flushing of transfer list
>    accel/qaic: Create a function to initialize BO
>    accel/qaic: Add QAIC_DETACH_SLICE_BO IOCTL
> 
>   Documentation/accel/qaic/qaic.rst |  10 ++
>   drivers/accel/qaic/qaic.h         |   6 +-
>   drivers/accel/qaic/qaic_data.c    | 187 +++++++++++++++++++++++-------
>   drivers/accel/qaic/qaic_drv.c     |   1 +
>   include/uapi/drm/qaic_accel.h     |  24 +++-
>   5 files changed, 175 insertions(+), 53 deletions(-)
> 

Pushed to drm-misc-next

-Jeff