* [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL
@ 2024-09-30 11:58 Arunpravin Paneer Selvam
2024-09-30 11:58 ` [PATCH v3 02/09] drm/amdgpu: screen freeze and userq driver crash Arunpravin Paneer Selvam
` (8 more replies)
0 siblings, 9 replies; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:58 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
This patch introduces new IOCTL for userqueue secure semaphore.
The signal IOCTL called from userspace application creates a drm
syncobj and array of bo GEM handles and passed in as parameter to
the driver to install the fence into it.
The wait IOCTL gets an array of drm syncobjs, finds the fences
attached to the drm syncobjs and obtain the array of
memory_address/fence_value combintion which are returned to
userspace.
v2: (Christian)
- Install fence into GEM BO object.
- Lock all BO's using the dma resv subsystem
- Reorder the sequence in signal IOCTL function.
- Get write pointer from the shadow wptr
- use userq_fence to fetch the va/value in wait IOCTL.
v3: (Christian)
- Use drm_exec helper for the proper BO drm reserve and avoid BO
lock/unlock issues.
- fence/fence driver reference count logic for signal/wait IOCTLs.
v4: (Christian)
- Fixed the drm_exec calling sequence
- use dma_resv_for_each_fence_unlock if BO's are not locked
- Modified the fence_info array storing logic.
v5: (Christian)
- Keep fence_drv until wait queue execution.
- Add dma_fence_wait for other fences.
- Lock BO's using drm_exec as the number of fences in them could
change.
- Install signaled fences as well into BO/Syncobj.
- Move Syncobj fence installation code after the drm_exec_prepare_array.
- Directly add dma_resv_usage_rw(args->bo_flags....
- remove unnecessary dma_fence_put.
v6: (Christian)
- Add xarray stuff to store the fence_drv
- Implement a function to iterate over the xarray and drop
the fence_drv references.
- Add drm_exec_until_all_locked() wrapper
- Add a check that if we haven't exceeded the user allocated num_fences
before adding dma_fence to the fences array.
v7: (Christian)
- Use memdup_user() for kmalloc_array + copy_from_user
- Move the fence_drv references from the xarray into the newly created fence
and drop the fence_drv references when we signal this fence.
- Move this locking of BOs before the "if (!wait_info->num_fences)",
this way you need this code block only once.
- Merge the error handling code and the cleanup + return 0 code.
- Initializing the xa should probably be done in the userq code.
- Remove the userq back pointer stored in fence_drv.
- Pass xarray as parameter in amdgpu_userq_walk_and_drop_fence_drv()
v8: (Christian)
- Move fence_drv references must come before adding the fence to the list.
- Use xa_lock_irqsave_nested for nested spinlock operations.
- userq_mgr should be per fpriv and not one per device.
- Restructure the interrupt process code for the early exit of the loop.
- The reference acquired in the syncobj fence replace code needs to be
kept around.
- Modify the dma_fence acquire placement in wait IOCTL.
- Move USERQ_BO_WRITE flag to UAPI header file.
- drop the fence drv reference after telling the hw to stop accessing it.
- Add multi sync object support to userq signal IOCTL.
V9: (Christian)
- Store all the fence_drv ref to other drivers and not ourself.
- Remove the userq fence xa implementation and replace with
kvmalloc_array.
v10: (Christian)
- Add a comment for the userq_xa xarray
- drop the if check of userq_fence->fence_drv_array
- use the i variable to initialize userq_fence->fence_drv_array_count
- drop the fence reference before you free the array in the error handling,
otherwise it could be that some references leaked.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Suggested-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 446 +++++++++++++++++-
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 7 +
drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 29 +-
.../gpu/drm/amd/include/amdgpu_userqueue.h | 1 +
6 files changed, 483 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a31f6c92a755..6178a1cda223 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1043,6 +1043,12 @@ struct amdgpu_device {
struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM];
const struct amdgpu_userq_funcs *userq_funcs[AMDGPU_HW_IP_NUM];
+ /* xarray used to retrieve the user queue fence driver reference
+ * in the EOP interrupt handler to signal the particular user
+ * queue fence.
+ */
+ struct xarray userq_xa;
+
/* df */
struct amdgpu_df df;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6e51b27b833d..70cb3b794a8a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2878,6 +2878,8 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
+ DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
};
static const struct drm_driver amdgpu_kms_driver = {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index f7baea2c67ab..95f232ad4a3d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -25,6 +25,7 @@
#include <linux/kref.h>
#include <linux/slab.h>
+#include <drm/drm_exec.h>
#include <drm/drm_syncobj.h>
#include "amdgpu.h"
@@ -92,6 +93,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
spin_lock_init(&fence_drv->fence_list_lock);
fence_drv->adev = adev;
+ fence_drv->uq_fence_drv_xa_ref = &userq->uq_fence_drv_xa;
fence_drv->context = dma_fence_context_alloc(1);
get_task_comm(fence_drv->timeline_name, current);
@@ -105,6 +107,7 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d
struct amdgpu_userq_fence *userq_fence, *tmp;
struct dma_fence *fence;
u64 rptr;
+ int i;
if (!fence_drv)
return;
@@ -115,14 +118,16 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d
list_for_each_entry_safe(userq_fence, tmp, &fence_drv->fences, link) {
fence = &userq_fence->base;
- if (rptr >= fence->seqno) {
- dma_fence_signal(fence);
- list_del(&userq_fence->link);
-
- dma_fence_put(fence);
- } else {
+ if (rptr < fence->seqno)
break;
- }
+
+ dma_fence_signal(fence);
+
+ for (i = 0; i < userq_fence->fence_drv_array_count; i++)
+ amdgpu_userq_fence_driver_put(userq_fence->fence_drv_array[i]);
+
+ list_del(&userq_fence->link);
+ dma_fence_put(fence);
}
spin_unlock(&fence_drv->fence_list_lock);
}
@@ -132,8 +137,11 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
struct amdgpu_userq_fence_driver,
refcount);
+ struct amdgpu_userq_fence_driver *xa_fence_drv;
struct amdgpu_device *adev = fence_drv->adev;
struct amdgpu_userq_fence *fence, *tmp;
+ struct xarray *xa = &adev->userq_xa;
+ unsigned long index;
struct dma_fence *f;
spin_lock(&fence_drv->fence_list_lock);
@@ -150,6 +158,12 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
}
spin_unlock(&fence_drv->fence_list_lock);
+ xa_lock(xa);
+ xa_for_each(xa, index, xa_fence_drv)
+ if (xa_fence_drv == fence_drv)
+ __xa_erase(xa, index);
+ xa_unlock(xa);
+
/* Free seq64 memory */
amdgpu_seq64_free(adev, fence_drv->gpu_addr);
kfree(fence_drv);
@@ -191,6 +205,33 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
amdgpu_userq_fence_driver_get(fence_drv);
dma_fence_get(fence);
+ if (!xa_empty(&userq->uq_fence_drv_xa)) {
+ struct amdgpu_userq_fence_driver *stored_fence_drv;
+ unsigned long index, count = 0;
+ int i = 0;
+
+ xa_for_each(&userq->uq_fence_drv_xa, index, stored_fence_drv)
+ count++;
+
+ userq_fence->fence_drv_array =
+ kvmalloc_array(count,
+ sizeof(struct amdgpu_userq_fence_driver *),
+ GFP_KERNEL);
+
+ if (userq_fence->fence_drv_array) {
+ xa_for_each(&userq->uq_fence_drv_xa, index, stored_fence_drv) {
+ userq_fence->fence_drv_array[i] = stored_fence_drv;
+ xa_erase(&userq->uq_fence_drv_xa, index);
+ i++;
+ }
+ }
+
+ userq_fence->fence_drv_array_count = i;
+ } else {
+ userq_fence->fence_drv_array = NULL;
+ userq_fence->fence_drv_array_count = 0;
+ }
+
spin_lock(&fence_drv->fence_list_lock);
/* Check if hardware has already processed the job */
if (!dma_fence_is_signaled(fence))
@@ -240,6 +281,8 @@ static void amdgpu_userq_fence_free(struct rcu_head *rcu)
/* Release the fence driver reference */
amdgpu_userq_fence_driver_put(fence_drv);
+
+ kvfree(userq_fence->fence_drv_array);
kmem_cache_free(amdgpu_userq_fence_slab, userq_fence);
}
@@ -255,3 +298,392 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
.signaled = amdgpu_userq_fence_signaled,
.release = amdgpu_userq_fence_release,
};
+
+/**
+ * amdgpu_userq_fence_read_wptr - Read the userq wptr value
+ *
+ * @filp: drm file private data structure
+ * @queue: user mode queue structure pointer
+ * @wptr: write pointer value
+ *
+ * Read the wptr value from userq's MQD. The userq signal IOCTL
+ * creates a dma_fence for the shared buffers that expects the
+ * RPTR value written to seq64 memory >= WPTR.
+ *
+ * Returns wptr value on success, error on failure.
+ */
+static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
+ struct amdgpu_usermode_queue *queue,
+ u64 *wptr)
+{
+ struct amdgpu_fpriv *fpriv = filp->driver_priv;
+ struct amdgpu_bo_va_mapping *mapping;
+ struct amdgpu_vm *vm = &fpriv->vm;
+ struct amdgpu_bo *bo;
+ u64 addr, *ptr;
+ int r;
+
+ addr = queue->userq_prop->wptr_gpu_addr;
+ addr &= AMDGPU_GMC_HOLE_MASK;
+
+ mapping = amdgpu_vm_bo_lookup_mapping(vm, addr >> PAGE_SHIFT);
+ if (!mapping)
+ return -EINVAL;
+
+ bo = mapping->bo_va->base.bo;
+ r = amdgpu_bo_reserve(bo, true);
+ if (r) {
+ DRM_ERROR("Failed to reserve userqueue wptr bo");
+ return r;
+ }
+
+ r = amdgpu_bo_kmap(bo, (void **)&ptr);
+ if (r) {
+ DRM_ERROR("Failed mapping the userqueue wptr bo");
+ goto map_error;
+ }
+
+ *wptr = le64_to_cpu(*ptr);
+
+ amdgpu_bo_kunmap(bo);
+ amdgpu_bo_unreserve(bo);
+
+ return 0;
+
+map_error:
+ amdgpu_bo_unreserve(bo);
+ return r;
+}
+
+int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+ struct amdgpu_fpriv *fpriv = filp->driver_priv;
+ struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
+ struct drm_amdgpu_userq_signal *args = data;
+ struct amdgpu_usermode_queue *queue;
+ struct drm_gem_object **gobj = NULL;
+ struct drm_syncobj **syncobj = NULL;
+ u32 *syncobj_handles, num_syncobj_handles;
+ u32 *bo_handles, num_bo_handles;
+ int r, i, entry, boentry;
+ struct dma_fence *fence;
+ struct drm_exec exec;
+ u64 wptr;
+
+ /* Array of syncobj handles */
+ num_syncobj_handles = args->num_syncobj_handles;
+ syncobj_handles = memdup_user(u64_to_user_ptr(args->syncobj_handles_array),
+ sizeof(u32) * num_syncobj_handles);
+ if (IS_ERR(syncobj_handles))
+ return PTR_ERR(syncobj_handles);
+
+ /* Array of pointers to the looked up syncobjs */
+ syncobj = kmalloc_array(num_syncobj_handles, sizeof(*syncobj), GFP_KERNEL);
+ if (!syncobj) {
+ r = -ENOMEM;
+ goto free_syncobj_handles;
+ }
+
+ for (entry = 0; entry < num_syncobj_handles; entry++) {
+ syncobj[entry] = drm_syncobj_find(filp, syncobj_handles[entry]);
+ if (!syncobj[entry]) {
+ r = -ENOENT;
+ goto free_syncobj;
+ }
+ }
+
+ /* Array of bo handles */
+ num_bo_handles = args->num_bo_handles;
+ bo_handles = memdup_user(u64_to_user_ptr(args->bo_handles_array),
+ sizeof(u32) * num_bo_handles);
+ if (IS_ERR(bo_handles))
+ goto free_syncobj;
+
+ /* Array of pointers to the GEM objects */
+ gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
+ if (!gobj) {
+ r = -ENOMEM;
+ goto free_bo_handles;
+ }
+
+ for (boentry = 0; boentry < num_bo_handles; boentry++) {
+ gobj[boentry] = drm_gem_object_lookup(filp, bo_handles[boentry]);
+ if (!gobj[boentry]) {
+ r = -ENOENT;
+ goto put_gobj;
+ }
+ }
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
+ drm_exec_until_all_locked(&exec) {
+ r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 1);
+ drm_exec_retry_on_contention(&exec);
+ if (r)
+ goto exec_fini;
+ }
+
+ /*Retrieve the user queue */
+ queue = idr_find(&userq_mgr->userq_idr, args->queue_id);
+ if (!queue) {
+ r = -ENOENT;
+ goto exec_fini;
+ }
+
+ r = amdgpu_userq_fence_read_wptr(filp, queue, &wptr);
+ if (r)
+ goto exec_fini;
+
+ /* Create a new fence */
+ r = amdgpu_userq_fence_create(queue, wptr, &fence);
+ if (r)
+ goto exec_fini;
+
+ for (i = 0; i < num_bo_handles; i++)
+ dma_resv_add_fence(gobj[i]->resv, fence,
+ dma_resv_usage_rw(args->bo_flags &
+ AMDGPU_USERQ_BO_WRITE));
+
+ /* Add the created fence to syncobj/BO's */
+ for (i = 0; i < num_syncobj_handles; i++)
+ drm_syncobj_replace_fence(syncobj[i], fence);
+
+ /* drop the reference acquired in fence creation function */
+ dma_fence_put(fence);
+
+exec_fini:
+ drm_exec_fini(&exec);
+put_gobj:
+ while (boentry-- > 0)
+ drm_gem_object_put(gobj[boentry]);
+ kfree(gobj);
+free_bo_handles:
+ kfree(bo_handles);
+free_syncobj:
+ while (entry-- > 0)
+ if (syncobj[entry])
+ drm_syncobj_put(syncobj[entry]);
+ kfree(syncobj);
+free_syncobj_handles:
+ kfree(syncobj_handles);
+
+ return r;
+}
+
+int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+ struct drm_amdgpu_userq_fence_info *fence_info = NULL;
+ struct drm_amdgpu_userq_wait *wait_info = data;
+ u32 *syncobj_handles, *bo_handles;
+ struct dma_fence **fences = NULL;
+ u32 num_syncobj, num_bo_handles;
+ struct drm_gem_object **gobj;
+ struct drm_exec exec;
+ int r, i, entry, cnt;
+ u64 num_fences = 0;
+
+ num_bo_handles = wait_info->num_bo_handles;
+ bo_handles = memdup_user(u64_to_user_ptr(wait_info->bo_handles_array),
+ sizeof(u32) * num_bo_handles);
+ if (IS_ERR(bo_handles))
+ return PTR_ERR(bo_handles);
+
+ num_syncobj = wait_info->num_syncobj_handles;
+ syncobj_handles = memdup_user(u64_to_user_ptr(wait_info->syncobj_handles_array),
+ sizeof(u32) * num_syncobj);
+ if (IS_ERR(syncobj_handles)) {
+ r = PTR_ERR(syncobj_handles);
+ goto free_bo_handles;
+ }
+
+ /* Array of GEM object handles */
+ gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
+ if (!gobj) {
+ r = -ENOMEM;
+ goto free_syncobj_handles;
+ }
+
+ for (entry = 0; entry < num_bo_handles; entry++) {
+ gobj[entry] = drm_gem_object_lookup(filp, bo_handles[entry]);
+ if (!gobj[entry]) {
+ r = -ENOENT;
+ goto put_gobj;
+ }
+ }
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
+ drm_exec_until_all_locked(&exec) {
+ r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 0);
+ drm_exec_retry_on_contention(&exec);
+ if (r) {
+ drm_exec_fini(&exec);
+ goto put_gobj;
+ }
+ }
+
+ if (!wait_info->num_fences) {
+ /* Count syncobj's fence */
+ for (i = 0; i < num_syncobj; i++) {
+ struct dma_fence *fence;
+
+ r = drm_syncobj_find_fence(filp, syncobj_handles[i],
+ 0, 0, &fence);
+ if (r)
+ goto exec_fini;
+
+ num_fences++;
+ dma_fence_put(fence);
+ }
+
+ /* Count GEM objects fence */
+ for (i = 0; i < num_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
+
+ dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
+ dma_resv_usage_rw(wait_info->bo_wait_flags &
+ AMDGPU_USERQ_BO_WRITE), fence)
+ num_fences++;
+ }
+
+ /*
+ * Passing num_fences = 0 means that userspace doesn't want to
+ * retrieve userq_fence_info. If num_fences = 0 we skip filling
+ * userq_fence_info and return the actual number of fences on
+ * args->num_fences.
+ */
+ wait_info->num_fences = num_fences;
+ } else {
+ /* Array of fence info */
+ fence_info = kmalloc_array(wait_info->num_fences, sizeof(*fence_info), GFP_KERNEL);
+ if (!fence_info) {
+ r = -ENOMEM;
+ goto exec_fini;
+ }
+
+ /* Array of fences */
+ fences = kmalloc_array(wait_info->num_fences, sizeof(*fences), GFP_KERNEL);
+ if (!fences) {
+ r = -ENOMEM;
+ goto free_fence_info;
+ }
+
+ /* Retrieve GEM objects fence */
+ for (i = 0; i < num_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
+
+ dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
+ dma_resv_usage_rw(wait_info->bo_wait_flags &
+ AMDGPU_USERQ_BO_WRITE), fence) {
+ if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
+ r = -EINVAL;
+ goto free_fences;
+ }
+
+ fences[num_fences++] = fence;
+ dma_fence_get(fence);
+ }
+ }
+
+ /* Retrieve syncobj's fence */
+ for (i = 0; i < num_syncobj; i++) {
+ struct dma_fence *fence;
+
+ r = drm_syncobj_find_fence(filp, syncobj_handles[i],
+ 0, 0, &fence);
+ if (r)
+ goto free_fences;
+
+ if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
+ r = -EINVAL;
+ goto free_fences;
+ }
+
+ fences[num_fences++] = fence;
+ }
+
+ for (i = 0, cnt = 0; i < wait_info->num_fences; i++) {
+ struct amdgpu_userq_fence_driver *fence_drv;
+ struct amdgpu_userq_fence *userq_fence;
+ u32 index;
+
+ userq_fence = to_amdgpu_userq_fence(fences[i]);
+ if (!userq_fence) {
+ /*
+ * Just waiting on other driver fences should
+ * be good for now
+ */
+ dma_fence_wait(fences[i], false);
+ dma_fence_put(fences[i]);
+
+ continue;
+ }
+
+ fence_drv = userq_fence->fence_drv;
+ /*
+ * We need to make sure the user queue release their reference
+ * to the fence drivers at some point before queue destruction.
+ * Otherwise, we would gather those references until we don't
+ * have any more space left and crash.
+ */
+ if (fence_drv->uq_fence_drv_xa_ref) {
+ r = xa_alloc(fence_drv->uq_fence_drv_xa_ref, &index, fence_drv,
+ xa_limit_32b, GFP_KERNEL);
+ if (r)
+ goto free_fences;
+
+ amdgpu_userq_fence_driver_get(fence_drv);
+ }
+
+ /* Store drm syncobj's gpu va address and value */
+ fence_info[cnt].va = fence_drv->gpu_addr;
+ fence_info[cnt].value = fences[i]->seqno;
+
+ dma_fence_put(fences[i]);
+ /* Increment the actual userq fence count */
+ cnt++;
+ }
+
+ wait_info->num_fences = cnt;
+ /* Copy userq fence info to user space */
+ if (copy_to_user(u64_to_user_ptr(wait_info->userq_fence_info),
+ fence_info, wait_info->num_fences * sizeof(*fence_info))) {
+ r = -EFAULT;
+ goto free_fences;
+ }
+
+ kfree(fences);
+ kfree(fence_info);
+ }
+
+ drm_exec_fini(&exec);
+ for (i = 0; i < num_bo_handles; i++)
+ drm_gem_object_put(gobj[i]);
+ kfree(gobj);
+
+ kfree(syncobj_handles);
+ kfree(bo_handles);
+
+ return 0;
+
+free_fences:
+ while (num_fences-- > 0)
+ dma_fence_put(fences[num_fences]);
+ kfree(fences);
+free_fence_info:
+ kfree(fence_info);
+exec_fini:
+ drm_exec_fini(&exec);
+put_gobj:
+ while (entry-- > 0)
+ drm_gem_object_put(gobj[entry]);
+ kfree(gobj);
+free_syncobj_handles:
+ kfree(syncobj_handles);
+free_bo_handles:
+ kfree(bo_handles);
+
+ return r;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
index c3e04cdbb9e7..f72424248cc5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
@@ -37,7 +37,9 @@ struct amdgpu_userq_fence {
*/
spinlock_t lock;
struct list_head link;
+ unsigned long fence_drv_array_count;
struct amdgpu_userq_fence_driver *fence_drv;
+ struct amdgpu_userq_fence_driver **fence_drv_array;
};
struct amdgpu_userq_fence_driver {
@@ -52,6 +54,7 @@ struct amdgpu_userq_fence_driver {
spinlock_t fence_list_lock;
struct list_head fences;
struct amdgpu_device *adev;
+ struct xarray *uq_fence_drv_xa_ref;
char timeline_name[TASK_COMM_LEN];
};
@@ -65,5 +68,9 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
struct amdgpu_usermode_queue *userq);
void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv);
void amdgpu_userq_fence_driver_destroy(struct kref *ref);
+int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp);
+int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp);
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
index 7f6495466975..ba986d55ceeb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
@@ -27,6 +27,32 @@
#include "amdgpu_userqueue.h"
#include "amdgpu_userq_fence.h"
+static void amdgpu_userq_walk_and_drop_fence_drv(struct xarray *xa)
+{
+ struct amdgpu_userq_fence_driver *fence_drv;
+ unsigned long index;
+
+ if (xa_empty(xa))
+ return;
+
+ xa_lock(xa);
+ xa_for_each(xa, index, fence_drv) {
+ __xa_erase(xa, index);
+ amdgpu_userq_fence_driver_put(fence_drv);
+ }
+
+ xa_unlock(xa);
+}
+
+static void
+amdgpu_userq_fence_driver_free(struct amdgpu_usermode_queue *userq)
+{
+ amdgpu_userq_walk_and_drop_fence_drv(&userq->uq_fence_drv_xa);
+ xa_destroy(&userq->uq_fence_drv_xa);
+ /* Drop the fence_drv reference held by user queue */
+ amdgpu_userq_fence_driver_put(userq->fence_drv);
+}
+
static void
amdgpu_userqueue_cleanup(struct amdgpu_userq_mgr *uq_mgr,
struct amdgpu_usermode_queue *queue,
@@ -36,7 +62,7 @@ amdgpu_userqueue_cleanup(struct amdgpu_userq_mgr *uq_mgr,
const struct amdgpu_userq_funcs *uq_funcs = adev->userq_funcs[queue->queue_type];
uq_funcs->mqd_destroy(uq_mgr, queue);
- amdgpu_userq_fence_driver_put(queue->fence_drv);
+ amdgpu_userq_fence_driver_free(queue);
idr_remove(&uq_mgr->userq_idr, queue_id);
kfree(queue);
}
@@ -320,6 +346,7 @@ amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq *args)
}
queue->doorbell_index = index;
+ xa_init_flags(&queue->uq_fence_drv_xa, XA_FLAGS_ALLOC);
r = amdgpu_userq_fence_driver_alloc(adev, queue);
if (r) {
DRM_ERROR("Failed to alloc fence driver\n");
diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
index 5fbffde48999..77a33f9e37f8 100644
--- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
+++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
@@ -47,6 +47,7 @@ struct amdgpu_usermode_queue {
struct amdgpu_userq_obj db_obj;
struct amdgpu_userq_obj fw_obj;
struct amdgpu_userq_obj wptr_obj;
+ struct xarray uq_fence_drv_xa;
struct amdgpu_userq_fence_driver *fence_drv;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 02/09] drm/amdgpu: screen freeze and userq driver crash
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
@ 2024-09-30 11:58 ` Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 03/09] drm/amdgpu: Add wait IOCTL timeline syncobj support Arunpravin Paneer Selvam
` (7 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:58 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Screen freeze and userq fence driver crash while playing Xonotic
v2: (Christian)
- There is change that fence might signal in between testing
and grabbing the lock. Hence we can move the lock above the
if..else check and use the dma_fence_is_signaled_locked().
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 95f232ad4a3d..8f9d2427d380 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -185,6 +185,7 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
struct amdgpu_userq_fence_driver *fence_drv;
struct amdgpu_userq_fence *userq_fence;
struct dma_fence *fence;
+ unsigned long flags;
fence_drv = userq->fence_drv;
if (!fence_drv)
@@ -232,14 +233,14 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
userq_fence->fence_drv_array_count = 0;
}
- spin_lock(&fence_drv->fence_list_lock);
/* Check if hardware has already processed the job */
- if (!dma_fence_is_signaled(fence))
+ spin_lock_irqsave(&fence_drv->fence_list_lock, flags);
+ if (!dma_fence_is_signaled_locked(fence))
list_add_tail(&userq_fence->link, &fence_drv->fences);
else
dma_fence_put(fence);
- spin_unlock(&fence_drv->fence_list_lock);
+ spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags);
*f = fence;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 03/09] drm/amdgpu: Add wait IOCTL timeline syncobj support
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
2024-09-30 11:58 ` [PATCH v3 02/09] drm/amdgpu: screen freeze and userq driver crash Arunpravin Paneer Selvam
@ 2024-09-30 11:59 ` Arunpravin Paneer Selvam
2024-09-30 12:21 ` Christian König
2024-09-30 11:59 ` [PATCH v3 04/09] drm/amdgpu: Enable userq fence interrupt support Arunpravin Paneer Selvam
` (6 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:59 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Add user fence wait IOCTL timeline syncobj support.
v2:(Christian)
- handle dma_fence_wait() return value.
- shorten the variable name syncobj_timeline_points a bit.
- move num_points up to avoid padding issues.
v3:(Christian)
- Handle timeline drm_syncobj_find_fence() call error
handling
- Use dma_fence_unwrap_for_each() in timeline fence as
there could be more than one fence.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 99 +++++++++++++++++--
include/uapi/drm/amdgpu_drm.h | 16 ++-
2 files changed, 107 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 8f9d2427d380..76552eed6d86 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -24,6 +24,7 @@
#include <linux/kref.h>
#include <linux/slab.h>
+#include <linux/dma-fence-unwrap.h>
#include <drm/drm_exec.h>
#include <drm/drm_syncobj.h>
@@ -474,11 +475,11 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
+ u32 *syncobj_handles, *timeline_points, *timeline_handles, *bo_handles;
+ u32 num_syncobj, num_bo_handles, num_points;
struct drm_amdgpu_userq_fence_info *fence_info = NULL;
struct drm_amdgpu_userq_wait *wait_info = data;
- u32 *syncobj_handles, *bo_handles;
struct dma_fence **fences = NULL;
- u32 num_syncobj, num_bo_handles;
struct drm_gem_object **gobj;
struct drm_exec exec;
int r, i, entry, cnt;
@@ -498,11 +499,26 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
goto free_bo_handles;
}
+ num_points = wait_info->num_points;
+ timeline_handles = memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_handles),
+ sizeof(u32) * num_points);
+ if (IS_ERR(timeline_handles)) {
+ r = PTR_ERR(timeline_handles);
+ goto free_syncobj_handles;
+ }
+
+ timeline_points = memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_points),
+ sizeof(u32) * num_points);
+ if (IS_ERR(timeline_points)) {
+ r = PTR_ERR(timeline_points);
+ goto free_timeline_handles;
+ }
+
/* Array of GEM object handles */
gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
if (!gobj) {
r = -ENOMEM;
- goto free_syncobj_handles;
+ goto free_timeline_points;
}
for (entry = 0; entry < num_bo_handles; entry++) {
@@ -524,15 +540,40 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
if (!wait_info->num_fences) {
+ if (num_points) {
+ struct dma_fence_unwrap iter;
+ struct dma_fence *fence;
+ struct dma_fence *f;
+
+ for (i = 0; i < num_points; i++) {
+ r = drm_syncobj_find_fence(filp, timeline_handles[i],
+ timeline_points[i],
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r)
+ goto exec_fini;
+
+ num_fences++;
+
+ dma_fence_unwrap_for_each(f, &iter, fence)
+ num_fences++;
+
+ dma_fence_put(fence);
+ }
+ }
+
/* Count syncobj's fence */
for (i = 0; i < num_syncobj; i++) {
struct dma_fence *fence;
r = drm_syncobj_find_fence(filp, syncobj_handles[i],
- 0, 0, &fence);
+ 0,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
if (r)
goto exec_fini;
+ dma_fence_put(fence);
num_fences++;
dma_fence_put(fence);
}
@@ -588,12 +629,46 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
}
+ if (num_points) {
+ struct dma_fence_unwrap iter;
+ struct dma_fence *fence;
+ struct dma_fence *f;
+
+ for (i = 0; i < num_points; i++) {
+ r = drm_syncobj_find_fence(filp, timeline_handles[i],
+ timeline_points[i],
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r)
+ goto free_fences;
+
+ if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
+ r = -EINVAL;
+ goto free_fences;
+ }
+
+ fences[num_fences++] = fence;
+
+ dma_fence_unwrap_for_each(f, &iter, fence) {
+ if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
+ r = -EINVAL;
+ goto free_fences;
+ }
+
+ dma_fence_get(f);
+ fences[num_fences++] = f;
+ }
+ }
+ }
+
/* Retrieve syncobj's fence */
for (i = 0; i < num_syncobj; i++) {
struct dma_fence *fence;
r = drm_syncobj_find_fence(filp, syncobj_handles[i],
- 0, 0, &fence);
+ 0,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
if (r)
goto free_fences;
@@ -616,9 +691,13 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
* Just waiting on other driver fences should
* be good for now
*/
- dma_fence_wait(fences[i], false);
- dma_fence_put(fences[i]);
+ r = dma_fence_wait(fences[i], true);
+ if (r) {
+ dma_fence_put(fences[i]);
+ goto free_fences;
+ }
+ dma_fence_put(fences[i]);
continue;
}
@@ -664,6 +743,8 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
drm_gem_object_put(gobj[i]);
kfree(gobj);
+ kfree(timeline_points);
+ kfree(timeline_handles);
kfree(syncobj_handles);
kfree(bo_handles);
@@ -681,6 +762,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
while (entry-- > 0)
drm_gem_object_put(gobj[entry]);
kfree(gobj);
+free_timeline_points:
+ kfree(timeline_points);
+free_timeline_handles:
+ kfree(timeline_handles);
free_syncobj_handles:
kfree(syncobj_handles);
free_bo_handles:
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index af42798e901d..3b24e0cb1b51 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -521,12 +521,26 @@ struct drm_amdgpu_userq_wait {
* matching fence wait info pair in @userq_fence_info.
*/
__u32 bo_wait_flags;
- __u32 pad;
+ /**
+ * @num_points: A count that represents the number of timeline syncobj handles in
+ * syncobj_handles_array.
+ */
+ __u32 num_points;
/**
* @syncobj_handles_array: An array of syncobj handles defined to get the
* fence wait information of every syncobj handles in the array.
*/
__u64 syncobj_handles_array;
+ /**
+ * @syncobj_timeline_handles: An array of timeline syncobj handles defined to get the
+ * fence wait information of every timeline syncobj handles in the array.
+ */
+ __u64 syncobj_timeline_handles;
+ /**
+ * @syncobj_timeline_points: An array of timeline syncobj points defined to get the
+ * fence wait points of every timeline syncobj handles in the syncobj_handles_array.
+ */
+ __u64 syncobj_timeline_points;
/**
* @bo_handles_array: An array of GEM BO handles defined to fetch the fence
* wait information of every BO handles in the array.
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 04/09] drm/amdgpu: Enable userq fence interrupt support
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
2024-09-30 11:58 ` [PATCH v3 02/09] drm/amdgpu: screen freeze and userq driver crash Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 03/09] drm/amdgpu: Add wait IOCTL timeline syncobj support Arunpravin Paneer Selvam
@ 2024-09-30 11:59 ` Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 05/09] drm/amdgpu: Remove the MES self test Arunpravin Paneer Selvam
` (5 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:59 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Add support to handle the userqueue protected fence signal hardware
interrupt.
Create a xarray which maps the doorbell index to the fence driver address.
This would help to retrieve the fence driver information when an userq fence
interrupt is triggered. Firmware sends the doorbell offset value and
this info is compared with the queue's mqd doorbell offset value.
If they are same, we process the userq fence interrupt.
v1:(Christian):
- use xa_load to extract the fence driver.
- move the amdgpu_userq_fence_driver_process call within the xa_lock
as there is a chance that fence_drv might be freed.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 6 +++++
drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 25 +++++++++----------
3 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 94bdb5fa6ebc..2da895f91e4d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3967,6 +3967,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
spin_lock_init(&adev->audio_endpt_idx_lock);
spin_lock_init(&adev->mm_stats.lock);
+ xa_init_flags(&adev->userq_xa, XA_FLAGS_LOCK_IRQ);
+
INIT_LIST_HEAD(&adev->shadow_list);
mutex_init(&adev->shadow_list_lock);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 76552eed6d86..dad26165df4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -71,6 +71,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
struct amdgpu_usermode_queue *userq)
{
struct amdgpu_userq_fence_driver *fence_drv;
+ unsigned long flags;
int r;
fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL);
@@ -98,6 +99,11 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
fence_drv->context = dma_fence_context_alloc(1);
get_task_comm(fence_drv->timeline_name, current);
+ xa_lock_irqsave(&adev->userq_xa, flags);
+ __xa_store(&adev->userq_xa, userq->doorbell_index,
+ fence_drv, GFP_KERNEL);
+ xa_unlock_irqrestore(&adev->userq_xa, flags);
+
userq->fence_drv = fence_drv;
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index 508e0acebb0d..68f908cafb6f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -50,6 +50,7 @@
#include "nbio_v4_3.h"
#include "mes_v11_0.h"
#include "mes_v11_0_userqueue.h"
+#include "amdgpu_userq_fence.h"
#define GFX11_NUM_GFX_RINGS 1
#define GFX11_MEC_HPD_SIZE 2048
@@ -5865,25 +5866,23 @@ static int gfx_v11_0_eop_irq(struct amdgpu_device *adev,
struct amdgpu_irq_src *source,
struct amdgpu_iv_entry *entry)
{
- int i;
+ u32 doorbell_offset = entry->src_data[0];
u8 me_id, pipe_id, queue_id;
struct amdgpu_ring *ring;
- uint32_t mes_queue_id = entry->src_data[0];
+ int i;
DRM_DEBUG("IH: CP EOP\n");
- if (adev->enable_mes && (mes_queue_id & AMDGPU_FENCE_MES_QUEUE_FLAG)) {
- struct amdgpu_mes_queue *queue;
+ if (adev->enable_mes && doorbell_offset) {
+ struct amdgpu_userq_fence_driver *fence_drv = NULL;
+ struct xarray *xa = &adev->userq_xa;
+ unsigned long flags;
- mes_queue_id &= AMDGPU_FENCE_MES_QUEUE_ID_MASK;
-
- spin_lock(&adev->mes.queue_id_lock);
- queue = idr_find(&adev->mes.queue_id_idr, mes_queue_id);
- if (queue) {
- DRM_DEBUG("process mes queue id = %d\n", mes_queue_id);
- amdgpu_fence_process(queue->ring);
- }
- spin_unlock(&adev->mes.queue_id_lock);
+ xa_lock_irqsave(xa, flags);
+ fence_drv = xa_load(xa, doorbell_offset);
+ if (fence_drv)
+ amdgpu_userq_fence_driver_process(fence_drv);
+ xa_unlock_irqrestore(xa, flags);
} else {
me_id = (entry->ring_id & 0x0c) >> 2;
pipe_id = (entry->ring_id & 0x03) >> 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 05/09] drm/amdgpu: Remove the MES self test
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
` (2 preceding siblings ...)
2024-09-30 11:59 ` [PATCH v3 04/09] drm/amdgpu: Enable userq fence interrupt support Arunpravin Paneer Selvam
@ 2024-09-30 11:59 ` Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 06/09] drm/amdgpu: Few optimization and fixes for userq fence driver Arunpravin Paneer Selvam
` (4 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:59 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Remove MES self test as this conflicts the userqueue fence
interrupts.
v2:(Christian)
- remove the amdgpu_mes_self_test() function and any now unused code.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 -
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 169 ---------------------
drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h | 2 -
drivers/gpu/drm/amd/amdgpu/mes_v10_1.c | 12 +-
drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 14 +-
5 files changed, 2 insertions(+), 198 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 2da895f91e4d..5ec6cb237c81 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4700,9 +4700,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool fbcon)
}
adev->in_suspend = false;
- if (adev->enable_mes)
- amdgpu_mes_self_test(adev);
-
if (amdgpu_acpi_smart_shift_update(dev, AMDGPU_SS_DEV_D0))
DRM_WARN("smart shift update failed\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
index da48b6da0107..dbe7cf4b322d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
@@ -1297,175 +1297,6 @@ int amdgpu_mes_ctx_unmap_meta_data(struct amdgpu_device *adev,
return r;
}
-static int amdgpu_mes_test_create_gang_and_queues(struct amdgpu_device *adev,
- int pasid, int *gang_id,
- int queue_type, int num_queue,
- struct amdgpu_ring **added_rings,
- struct amdgpu_mes_ctx_data *ctx_data)
-{
- struct amdgpu_ring *ring;
- struct amdgpu_mes_gang_properties gprops = {0};
- int r, j;
-
- /* create a gang for the process */
- gprops.priority = AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
- gprops.gang_quantum = adev->mes.default_gang_quantum;
- gprops.inprocess_gang_priority = AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
- gprops.priority_level = AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
- gprops.global_priority_level = AMDGPU_MES_PRIORITY_LEVEL_NORMAL;
-
- r = amdgpu_mes_add_gang(adev, pasid, &gprops, gang_id);
- if (r) {
- DRM_ERROR("failed to add gang\n");
- return r;
- }
-
- /* create queues for the gang */
- for (j = 0; j < num_queue; j++) {
- r = amdgpu_mes_add_ring(adev, *gang_id, queue_type, j,
- ctx_data, &ring);
- if (r) {
- DRM_ERROR("failed to add ring\n");
- break;
- }
-
- DRM_INFO("ring %s was added\n", ring->name);
- added_rings[j] = ring;
- }
-
- return 0;
-}
-
-static int amdgpu_mes_test_queues(struct amdgpu_ring **added_rings)
-{
- struct amdgpu_ring *ring;
- int i, r;
-
- for (i = 0; i < AMDGPU_MES_CTX_MAX_RINGS; i++) {
- ring = added_rings[i];
- if (!ring)
- continue;
-
- r = amdgpu_ring_test_helper(ring);
- if (r)
- return r;
-
- r = amdgpu_ring_test_ib(ring, 1000 * 10);
- if (r) {
- DRM_DEV_ERROR(ring->adev->dev,
- "ring %s ib test failed (%d)\n",
- ring->name, r);
- return r;
- } else
- DRM_INFO("ring %s ib test pass\n", ring->name);
- }
-
- return 0;
-}
-
-int amdgpu_mes_self_test(struct amdgpu_device *adev)
-{
- struct amdgpu_vm *vm = NULL;
- struct amdgpu_mes_ctx_data ctx_data = {0};
- struct amdgpu_ring *added_rings[AMDGPU_MES_CTX_MAX_RINGS] = { NULL };
- int gang_ids[3] = {0};
- int queue_types[][2] = { { AMDGPU_RING_TYPE_GFX, 1 },
- { AMDGPU_RING_TYPE_COMPUTE, 1 },
- { AMDGPU_RING_TYPE_SDMA, 1} };
- int i, r, pasid, k = 0;
-
- pasid = amdgpu_pasid_alloc(16);
- if (pasid < 0) {
- dev_warn(adev->dev, "No more PASIDs available!");
- pasid = 0;
- }
-
- vm = kzalloc(sizeof(*vm), GFP_KERNEL);
- if (!vm) {
- r = -ENOMEM;
- goto error_pasid;
- }
-
- r = amdgpu_vm_init(adev, vm, -1);
- if (r) {
- DRM_ERROR("failed to initialize vm\n");
- goto error_pasid;
- }
-
- r = amdgpu_mes_ctx_alloc_meta_data(adev, &ctx_data);
- if (r) {
- DRM_ERROR("failed to alloc ctx meta data\n");
- goto error_fini;
- }
-
- ctx_data.meta_data_gpu_addr = AMDGPU_VA_RESERVED_SIZE;
- r = amdgpu_mes_ctx_map_meta_data(adev, vm, &ctx_data);
- if (r) {
- DRM_ERROR("failed to map ctx meta data\n");
- goto error_vm;
- }
-
- r = amdgpu_mes_create_process(adev, pasid, vm);
- if (r) {
- DRM_ERROR("failed to create MES process\n");
- goto error_vm;
- }
-
- for (i = 0; i < ARRAY_SIZE(queue_types); i++) {
- /* On GFX v10.3, fw hasn't supported to map sdma queue. */
- if (amdgpu_ip_version(adev, GC_HWIP, 0) >=
- IP_VERSION(10, 3, 0) &&
- amdgpu_ip_version(adev, GC_HWIP, 0) <
- IP_VERSION(11, 0, 0) &&
- queue_types[i][0] == AMDGPU_RING_TYPE_SDMA)
- continue;
-
- r = amdgpu_mes_test_create_gang_and_queues(adev, pasid,
- &gang_ids[i],
- queue_types[i][0],
- queue_types[i][1],
- &added_rings[k],
- &ctx_data);
- if (r)
- goto error_queues;
-
- k += queue_types[i][1];
- }
-
- /* start ring test and ib test for MES queues */
- amdgpu_mes_test_queues(added_rings);
-
-error_queues:
- /* remove all queues */
- for (i = 0; i < ARRAY_SIZE(added_rings); i++) {
- if (!added_rings[i])
- continue;
- amdgpu_mes_remove_ring(adev, added_rings[i]);
- }
-
- for (i = 0; i < ARRAY_SIZE(gang_ids); i++) {
- if (!gang_ids[i])
- continue;
- amdgpu_mes_remove_gang(adev, gang_ids[i]);
- }
-
- amdgpu_mes_destroy_process(adev, pasid);
-
-error_vm:
- amdgpu_mes_ctx_unmap_meta_data(adev, &ctx_data);
-
-error_fini:
- amdgpu_vm_fini(adev, vm);
-
-error_pasid:
- if (pasid)
- amdgpu_pasid_free(pasid);
-
- amdgpu_mes_ctx_free_meta_data(&ctx_data);
- kfree(vm);
- return 0;
-}
-
int amdgpu_mes_init_microcode(struct amdgpu_device *adev, int pipe)
{
const struct mes_firmware_header_v1_0 *mes_hdr;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
index 7d4f93fea937..e7af28834766 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.h
@@ -396,8 +396,6 @@ int amdgpu_mes_ctx_map_meta_data(struct amdgpu_device *adev,
int amdgpu_mes_ctx_unmap_meta_data(struct amdgpu_device *adev,
struct amdgpu_mes_ctx_data *ctx_data);
-int amdgpu_mes_self_test(struct amdgpu_device *adev);
-
int amdgpu_mes_doorbell_process_slice(struct amdgpu_device *adev);
/*
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
index 1e5ad1e08d2a..1b2745670d4c 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
@@ -1156,20 +1156,10 @@ static int mes_v10_0_early_init(void *handle)
return 0;
}
-static int mes_v10_0_late_init(void *handle)
-{
- struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
- if (!amdgpu_in_reset(adev))
- amdgpu_mes_self_test(adev);
-
- return 0;
-}
-
static const struct amd_ip_funcs mes_v10_1_ip_funcs = {
.name = "mes_v10_1",
.early_init = mes_v10_0_early_init,
- .late_init = mes_v10_0_late_init,
+ .late_init = NULL,
.sw_init = mes_v10_1_sw_init,
.sw_fini = mes_v10_1_sw_fini,
.hw_init = mes_v10_1_hw_init,
diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index 26d71a22395d..38e35c0a2876 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -1257,22 +1257,10 @@ static int mes_v11_0_early_init(void *handle)
return 0;
}
-static int mes_v11_0_late_init(void *handle)
-{
- struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-
- /* it's only intended for use in mes_self_test case, not for s0ix and reset */
- if (!amdgpu_in_reset(adev) && !adev->in_s0ix && !adev->in_suspend &&
- (amdgpu_ip_version(adev, GC_HWIP, 0) != IP_VERSION(11, 0, 3)))
- amdgpu_mes_self_test(adev);
-
- return 0;
-}
-
static const struct amd_ip_funcs mes_v11_0_ip_funcs = {
.name = "mes_v11_0",
.early_init = mes_v11_0_early_init,
- .late_init = mes_v11_0_late_init,
+ .late_init = NULL,
.sw_init = mes_v11_0_sw_init,
.sw_fini = mes_v11_0_sw_fini,
.hw_init = mes_v11_0_hw_init,
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 06/09] drm/amdgpu: Few optimization and fixes for userq fence driver
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
` (3 preceding siblings ...)
2024-09-30 11:59 ` [PATCH v3 05/09] drm/amdgpu: Remove the MES self test Arunpravin Paneer Selvam
@ 2024-09-30 11:59 ` Arunpravin Paneer Selvam
2025-04-09 18:45 ` Kees Bakker
2024-09-30 11:59 ` [PATCH v3 07/09] drm/amdgpu: Add the missing error handling for xa_store() call Arunpravin Paneer Selvam
` (3 subsequent siblings)
8 siblings, 1 reply; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:59 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Few optimization and fixes for userq fence driver.
v1:(Christian):
- Remove unnecessary comments.
- In drm_exec_init call give num_bo_handles as last parameter it would
making allocation of the array more efficient
- Handle return value of __xa_store() and improve the error handling of
amdgpu_userq_fence_driver_alloc().
v2:(Christian):
- Revert userq_xa xarray init to XA_FLAGS_LOCK_IRQ.
- move the xa_unlock before the error check of the call xa_err(__xa_store())
and moved this change to a separate patch as this is adding a missing error
handling.
- Removed the unnecessary comments.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 44 ++++++++++++-------
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 6 +--
.../gpu/drm/amd/include/amdgpu_userqueue.h | 2 +-
4 files changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index ca8f01b2bd96..56bd870ff15d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -77,7 +77,8 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL);
if (!fence_drv) {
DRM_ERROR("Failed to allocate memory for fence driver\n");
- return -ENOMEM;
+ r = -ENOMEM;
+ goto free_fence_drv;
}
/* Acquire seq64 memory */
@@ -85,7 +86,8 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
&fence_drv->cpu_addr);
if (r) {
kfree(fence_drv);
- return -ENOMEM;
+ r = -ENOMEM;
+ goto free_seq64;
}
memset(fence_drv->cpu_addr, 0, sizeof(u64));
@@ -95,7 +97,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
spin_lock_init(&fence_drv->fence_list_lock);
fence_drv->adev = adev;
- fence_drv->uq_fence_drv_xa_ref = &userq->uq_fence_drv_xa;
+ fence_drv->fence_drv_xa_ptr = &userq->fence_drv_xa;
fence_drv->context = dma_fence_context_alloc(1);
get_task_comm(fence_drv->timeline_name, current);
@@ -107,6 +109,13 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
userq->fence_drv = fence_drv;
return 0;
+
+free_seq64:
+ amdgpu_seq64_free(adev, fence_drv->gpu_addr);
+free_fence_drv:
+ kfree(fence_drv);
+
+ return r;
}
void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv)
@@ -148,7 +157,7 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
struct amdgpu_device *adev = fence_drv->adev;
struct amdgpu_userq_fence *fence, *tmp;
struct xarray *xa = &adev->userq_xa;
- unsigned long index;
+ unsigned long index, flags;
struct dma_fence *f;
spin_lock(&fence_drv->fence_list_lock);
@@ -165,11 +174,11 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
}
spin_unlock(&fence_drv->fence_list_lock);
- xa_lock(xa);
+ xa_lock_irqsave(xa, flags);
xa_for_each(xa, index, xa_fence_drv)
if (xa_fence_drv == fence_drv)
__xa_erase(xa, index);
- xa_unlock(xa);
+ xa_unlock_irqrestore(xa, flags);
/* Free seq64 memory */
amdgpu_seq64_free(adev, fence_drv->gpu_addr);
@@ -213,12 +222,12 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
amdgpu_userq_fence_driver_get(fence_drv);
dma_fence_get(fence);
- if (!xa_empty(&userq->uq_fence_drv_xa)) {
+ if (!xa_empty(&userq->fence_drv_xa)) {
struct amdgpu_userq_fence_driver *stored_fence_drv;
unsigned long index, count = 0;
int i = 0;
- xa_for_each(&userq->uq_fence_drv_xa, index, stored_fence_drv)
+ xa_for_each(&userq->fence_drv_xa, index, stored_fence_drv)
count++;
userq_fence->fence_drv_array =
@@ -227,9 +236,9 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
GFP_KERNEL);
if (userq_fence->fence_drv_array) {
- xa_for_each(&userq->uq_fence_drv_xa, index, stored_fence_drv) {
+ xa_for_each(&userq->fence_drv_xa, index, stored_fence_drv) {
userq_fence->fence_drv_array[i] = stored_fence_drv;
- xa_erase(&userq->uq_fence_drv_xa, index);
+ xa_erase(&userq->fence_drv_xa, index);
i++;
}
}
@@ -379,7 +388,6 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
struct drm_exec exec;
u64 wptr;
- /* Array of syncobj handles */
num_syncobj_handles = args->num_syncobj_handles;
syncobj_handles = memdup_user(u64_to_user_ptr(args->syncobj_handles_array),
sizeof(u32) * num_syncobj_handles);
@@ -401,7 +409,6 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
}
}
- /* Array of bo handles */
num_bo_handles = args->num_bo_handles;
bo_handles = memdup_user(u64_to_user_ptr(args->bo_handles_array),
sizeof(u32) * num_bo_handles);
@@ -423,7 +430,9 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
}
}
- drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, num_bo_handles);
+
+ /* Lock all BOs with retry handling */
drm_exec_until_all_locked(&exec) {
r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 1);
drm_exec_retry_on_contention(&exec);
@@ -528,7 +537,6 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
goto free_timeline_handles;
}
- /* Array of GEM object handles */
gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
if (!gobj) {
r = -ENOMEM;
@@ -543,7 +551,9 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
}
- drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, num_bo_handles);
+
+ /* Lock all BOs with retry handling */
drm_exec_until_all_locked(&exec) {
r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 0);
drm_exec_retry_on_contention(&exec);
@@ -722,8 +732,8 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
* Otherwise, we would gather those references until we don't
* have any more space left and crash.
*/
- if (fence_drv->uq_fence_drv_xa_ref) {
- r = xa_alloc(fence_drv->uq_fence_drv_xa_ref, &index, fence_drv,
+ if (fence_drv->fence_drv_xa_ptr) {
+ r = xa_alloc(fence_drv->fence_drv_xa_ptr, &index, fence_drv,
xa_limit_32b, GFP_KERNEL);
if (r)
goto free_fences;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
index f72424248cc5..89c82ba38b50 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
@@ -54,7 +54,7 @@ struct amdgpu_userq_fence_driver {
spinlock_t fence_list_lock;
struct list_head fences;
struct amdgpu_device *adev;
- struct xarray *uq_fence_drv_xa_ref;
+ struct xarray *fence_drv_xa_ptr;
char timeline_name[TASK_COMM_LEN];
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
index e7f7354e0c0e..9b24218f7ad2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
@@ -48,8 +48,8 @@ static void amdgpu_userq_walk_and_drop_fence_drv(struct xarray *xa)
static void
amdgpu_userq_fence_driver_free(struct amdgpu_usermode_queue *userq)
{
- amdgpu_userq_walk_and_drop_fence_drv(&userq->uq_fence_drv_xa);
- xa_destroy(&userq->uq_fence_drv_xa);
+ amdgpu_userq_walk_and_drop_fence_drv(&userq->fence_drv_xa);
+ xa_destroy(&userq->fence_drv_xa);
/* Drop the fence_drv reference held by user queue */
amdgpu_userq_fence_driver_put(userq->fence_drv);
}
@@ -348,7 +348,7 @@ amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq *args)
}
queue->doorbell_index = index;
- xa_init_flags(&queue->uq_fence_drv_xa, XA_FLAGS_ALLOC);
+ xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
r = amdgpu_userq_fence_driver_alloc(adev, queue);
if (r) {
DRM_ERROR("Failed to alloc fence driver\n");
diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
index d035b5c2b14b..6eb094a54f4b 100644
--- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
+++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
@@ -52,7 +52,7 @@ struct amdgpu_usermode_queue {
struct amdgpu_userq_obj db_obj;
struct amdgpu_userq_obj fw_obj;
struct amdgpu_userq_obj wptr_obj;
- struct xarray uq_fence_drv_xa;
+ struct xarray fence_drv_xa;
struct amdgpu_userq_fence_driver *fence_drv;
struct dma_fence *last_fence;
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 07/09] drm/amdgpu: Add the missing error handling for xa_store() call
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
` (4 preceding siblings ...)
2024-09-30 11:59 ` [PATCH v3 06/09] drm/amdgpu: Few optimization and fixes for userq fence driver Arunpravin Paneer Selvam
@ 2024-09-30 11:59 ` Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 08/09] drm/amdgpu: add vm root BO lock before accessing the vm Arunpravin Paneer Selvam
` (2 subsequent siblings)
8 siblings, 0 replies; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:59 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Add the missing error handling for xa_store() call in the function
amdgpu_userq_fence_driver_alloc().
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 56bd870ff15d..9ca1910d7a62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -102,9 +102,11 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
get_task_comm(fence_drv->timeline_name, current);
xa_lock_irqsave(&adev->userq_xa, flags);
- __xa_store(&adev->userq_xa, userq->doorbell_index,
- fence_drv, GFP_KERNEL);
+ r = xa_err(__xa_store(&adev->userq_xa, userq->doorbell_index,
+ fence_drv, GFP_KERNEL));
xa_unlock_irqrestore(&adev->userq_xa, flags);
+ if (r)
+ goto free_seq64;
userq->fence_drv = fence_drv;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 08/09] drm/amdgpu: add vm root BO lock before accessing the vm
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
` (5 preceding siblings ...)
2024-09-30 11:59 ` [PATCH v3 07/09] drm/amdgpu: Add the missing error handling for xa_store() call Arunpravin Paneer Selvam
@ 2024-09-30 11:59 ` Arunpravin Paneer Selvam
2024-09-30 12:50 ` Christian König
2024-09-30 11:59 ` [PATCH v3 09/09] drm/amdgpu: Add separate array of read and write for BO handles Arunpravin Paneer Selvam
2024-09-30 12:09 ` [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Christian König
8 siblings, 1 reply; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:59 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Add a vm root BO lock before accessing the userqueue VM.
v1:(Christian)
- Keep the VM locked until you are done with the mapping.
- Grab a temporary BO reference, drop the VM lock and acquire the BO.
When you are done with everything just drop the BO lock and
then the temporary BO reference.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 24 ++++++++++++-------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 9ca1910d7a62..0725f28f3484 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -321,7 +321,6 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
/**
* amdgpu_userq_fence_read_wptr - Read the userq wptr value
*
- * @filp: drm file private data structure
* @queue: user mode queue structure pointer
* @wptr: write pointer value
*
@@ -331,25 +330,29 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
*
* Returns wptr value on success, error on failure.
*/
-static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
- struct amdgpu_usermode_queue *queue,
+static int amdgpu_userq_fence_read_wptr(struct amdgpu_usermode_queue *queue,
u64 *wptr)
{
- struct amdgpu_fpriv *fpriv = filp->driver_priv;
struct amdgpu_bo_va_mapping *mapping;
- struct amdgpu_vm *vm = &fpriv->vm;
struct amdgpu_bo *bo;
u64 addr, *ptr;
int r;
+ r = amdgpu_bo_reserve(queue->vm->root.bo, false);
+ if (r)
+ return r;
+
addr = queue->userq_prop->wptr_gpu_addr;
addr &= AMDGPU_GMC_HOLE_MASK;
- mapping = amdgpu_vm_bo_lookup_mapping(vm, addr >> PAGE_SHIFT);
- if (!mapping)
+ mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
+ if (!mapping) {
+ DRM_ERROR("Failed to lookup amdgpu_bo_va_mapping\n");
return -EINVAL;
+ }
- bo = mapping->bo_va->base.bo;
+ bo = amdgpu_bo_ref(mapping->bo_va->base.bo);
+ amdgpu_bo_unreserve(queue->vm->root.bo);
r = amdgpu_bo_reserve(bo, true);
if (r) {
DRM_ERROR("Failed to reserve userqueue wptr bo");
@@ -366,11 +369,14 @@ static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
amdgpu_bo_kunmap(bo);
amdgpu_bo_unreserve(bo);
+ amdgpu_bo_unref(&bo);
return 0;
map_error:
amdgpu_bo_unreserve(bo);
+ amdgpu_bo_unref(&bo);
+
return r;
}
@@ -449,7 +455,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
goto exec_fini;
}
- r = amdgpu_userq_fence_read_wptr(filp, queue, &wptr);
+ r = amdgpu_userq_fence_read_wptr(queue, &wptr);
if (r)
goto exec_fini;
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 09/09] drm/amdgpu: Add separate array of read and write for BO handles
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
` (6 preceding siblings ...)
2024-09-30 11:59 ` [PATCH v3 08/09] drm/amdgpu: add vm root BO lock before accessing the vm Arunpravin Paneer Selvam
@ 2024-09-30 11:59 ` Arunpravin Paneer Selvam
2024-09-30 12:54 ` Christian König
2024-09-30 12:09 ` [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Christian König
8 siblings, 1 reply; 14+ messages in thread
From: Arunpravin Paneer Selvam @ 2024-09-30 11:59 UTC (permalink / raw)
To: amd-gfx; +Cc: christian.koenig, alexander.deucher, Arunpravin Paneer Selvam
Drop AMDGPU_USERQ_BO_WRITE as this should not be a global option
of the IOCTL, It should be option per buffer. Hence adding separate
array for read and write BO handles.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 238 +++++++++++++-----
include/uapi/drm/amdgpu_drm.h | 48 ++--
2 files changed, 206 insertions(+), 80 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 0725f28f3484..aa794f342036 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -386,12 +386,14 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
struct amdgpu_fpriv *fpriv = filp->driver_priv;
struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
struct drm_amdgpu_userq_signal *args = data;
+ struct drm_gem_object **gobj_write = NULL;
+ struct drm_gem_object **gobj_read = NULL;
struct amdgpu_usermode_queue *queue;
- struct drm_gem_object **gobj = NULL;
struct drm_syncobj **syncobj = NULL;
+ u32 *bo_handles_write, num_write_bo_handles;
u32 *syncobj_handles, num_syncobj_handles;
- u32 *bo_handles, num_bo_handles;
- int r, i, entry, boentry;
+ u32 *bo_handles_read, num_read_bo_handles;
+ int r, i, entry, rentry, wentry;
struct dma_fence *fence;
struct drm_exec exec;
u64 wptr;
@@ -417,32 +419,59 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
}
}
- num_bo_handles = args->num_bo_handles;
- bo_handles = memdup_user(u64_to_user_ptr(args->bo_handles_array),
- sizeof(u32) * num_bo_handles);
- if (IS_ERR(bo_handles))
+ num_read_bo_handles = args->num_read_bo_handles;
+ bo_handles_read = memdup_user(u64_to_user_ptr(args->bo_handles_read_array),
+ sizeof(u32) * num_read_bo_handles);
+ if (IS_ERR(bo_handles_read))
goto free_syncobj;
- /* Array of pointers to the GEM objects */
- gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
- if (!gobj) {
+ /* Array of pointers to the GEM read objects */
+ gobj_read = kmalloc_array(num_read_bo_handles, sizeof(*gobj_read), GFP_KERNEL);
+ if (!gobj_read) {
r = -ENOMEM;
- goto free_bo_handles;
+ goto free_bo_handles_read;
}
- for (boentry = 0; boentry < num_bo_handles; boentry++) {
- gobj[boentry] = drm_gem_object_lookup(filp, bo_handles[boentry]);
- if (!gobj[boentry]) {
+ for (rentry = 0; rentry < num_read_bo_handles; rentry++) {
+ gobj_read[rentry] = drm_gem_object_lookup(filp, bo_handles_read[rentry]);
+ if (!gobj_read[rentry]) {
r = -ENOENT;
- goto put_gobj;
+ goto put_gobj_read;
}
}
- drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, num_bo_handles);
+ num_write_bo_handles = args->num_write_bo_handles;
+ bo_handles_write = memdup_user(u64_to_user_ptr(args->bo_handles_write_array),
+ sizeof(u32) * num_write_bo_handles);
+ if (IS_ERR(bo_handles_write))
+ goto put_gobj_read;
+
+ /* Array of pointers to the GEM write objects */
+ gobj_write = kmalloc_array(num_write_bo_handles, sizeof(*gobj_write), GFP_KERNEL);
+ if (!gobj_write) {
+ r = -ENOMEM;
+ goto free_bo_handles_write;
+ }
+
+ for (wentry = 0; wentry < num_write_bo_handles; wentry++) {
+ gobj_write[wentry] = drm_gem_object_lookup(filp, bo_handles_write[wentry]);
+ if (!gobj_write[wentry]) {
+ r = -ENOENT;
+ goto put_gobj_write;
+ }
+ }
+
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT,
+ (num_read_bo_handles + num_write_bo_handles));
/* Lock all BOs with retry handling */
drm_exec_until_all_locked(&exec) {
- r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 1);
+ r = drm_exec_prepare_array(&exec, gobj_read, num_read_bo_handles, 1);
+ drm_exec_retry_on_contention(&exec);
+ if (r)
+ goto exec_fini;
+
+ r = drm_exec_prepare_array(&exec, gobj_write, num_write_bo_handles, 1);
drm_exec_retry_on_contention(&exec);
if (r)
goto exec_fini;
@@ -468,13 +497,20 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
dma_fence_put(queue->last_fence);
queue->last_fence = dma_fence_get(fence);
- for (i = 0; i < num_bo_handles; i++) {
- if (!gobj || !gobj[i]->resv)
+ for (i = 0; i < num_read_bo_handles; i++) {
+ if (!gobj_read || !gobj_read[i]->resv)
continue;
- dma_resv_add_fence(gobj[i]->resv, fence,
- dma_resv_usage_rw(args->bo_flags &
- AMDGPU_USERQ_BO_WRITE));
+ dma_resv_add_fence(gobj_read[i]->resv, fence,
+ DMA_RESV_USAGE_READ);
+ }
+
+ for (i = 0; i < num_write_bo_handles; i++) {
+ if (!gobj_write || !gobj_write[i]->resv)
+ continue;
+
+ dma_resv_add_fence(gobj_read[i]->resv, fence,
+ DMA_RESV_USAGE_WRITE);
}
/* Add the created fence to syncobj/BO's */
@@ -486,12 +522,18 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
exec_fini:
drm_exec_fini(&exec);
-put_gobj:
- while (boentry-- > 0)
- drm_gem_object_put(gobj[boentry]);
- kfree(gobj);
-free_bo_handles:
- kfree(bo_handles);
+put_gobj_write:
+ while (wentry-- > 0)
+ drm_gem_object_put(gobj_write[wentry]);
+ kfree(gobj_write);
+free_bo_handles_write:
+ kfree(bo_handles_write);
+put_gobj_read:
+ while (rentry-- > 0)
+ drm_gem_object_put(gobj_read[rentry]);
+ kfree(gobj_read);
+free_bo_handles_read:
+ kfree(bo_handles_read);
free_syncobj:
while (entry-- > 0)
if (syncobj[entry])
@@ -506,28 +548,35 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
- u32 *syncobj_handles, *timeline_points, *timeline_handles, *bo_handles;
- u32 num_syncobj, num_bo_handles, num_points;
+ u32 *syncobj_handles, *timeline_points, *timeline_handles, *bo_handles_read, *bo_handles_write;
+ u32 num_syncobj, num_read_bo_handles, num_write_bo_handles, num_points;
struct drm_amdgpu_userq_fence_info *fence_info = NULL;
struct drm_amdgpu_userq_wait *wait_info = data;
+ struct drm_gem_object **gobj_write;
+ struct drm_gem_object **gobj_read;
struct dma_fence **fences = NULL;
- struct drm_gem_object **gobj;
+ int r, i, rentry, wentry, cnt;
struct drm_exec exec;
- int r, i, entry, cnt;
u64 num_fences = 0;
- num_bo_handles = wait_info->num_bo_handles;
- bo_handles = memdup_user(u64_to_user_ptr(wait_info->bo_handles_array),
- sizeof(u32) * num_bo_handles);
- if (IS_ERR(bo_handles))
- return PTR_ERR(bo_handles);
+ num_read_bo_handles = wait_info->num_read_bo_handles;
+ bo_handles_read = memdup_user(u64_to_user_ptr(wait_info->bo_handles_read_array),
+ sizeof(u32) * num_read_bo_handles);
+ if (IS_ERR(bo_handles_read))
+ return PTR_ERR(bo_handles_read);
+
+ num_write_bo_handles = wait_info->num_write_bo_handles;
+ bo_handles_write = memdup_user(u64_to_user_ptr(wait_info->bo_handles_write_array),
+ sizeof(u32) * num_write_bo_handles);
+ if (IS_ERR(bo_handles_write))
+ goto free_bo_handles_read;
num_syncobj = wait_info->num_syncobj_handles;
syncobj_handles = memdup_user(u64_to_user_ptr(wait_info->syncobj_handles_array),
sizeof(u32) * num_syncobj);
if (IS_ERR(syncobj_handles)) {
r = PTR_ERR(syncobj_handles);
- goto free_bo_handles;
+ goto free_bo_handles_write;
}
num_points = wait_info->num_points;
@@ -545,29 +594,51 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
goto free_timeline_handles;
}
- gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
- if (!gobj) {
+ gobj_read = kmalloc_array(num_read_bo_handles, sizeof(*gobj_read), GFP_KERNEL);
+ if (!gobj_read) {
r = -ENOMEM;
goto free_timeline_points;
}
- for (entry = 0; entry < num_bo_handles; entry++) {
- gobj[entry] = drm_gem_object_lookup(filp, bo_handles[entry]);
- if (!gobj[entry]) {
+ for (rentry = 0; rentry < num_read_bo_handles; rentry++) {
+ gobj_read[rentry] = drm_gem_object_lookup(filp, bo_handles_read[rentry]);
+ if (!gobj_read[rentry]) {
+ r = -ENOENT;
+ goto put_gobj_read;
+ }
+ }
+
+ gobj_write = kmalloc_array(num_write_bo_handles, sizeof(*gobj_write), GFP_KERNEL);
+ if (!gobj_write) {
+ r = -ENOMEM;
+ goto put_gobj_read;
+ }
+
+ for (wentry = 0; wentry < num_write_bo_handles; wentry++) {
+ gobj_write[wentry] = drm_gem_object_lookup(filp, bo_handles_write[wentry]);
+ if (!gobj_write[wentry]) {
r = -ENOENT;
- goto put_gobj;
+ goto put_gobj_write;
}
}
- drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, num_bo_handles);
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT,
+ (num_read_bo_handles + num_write_bo_handles));
/* Lock all BOs with retry handling */
drm_exec_until_all_locked(&exec) {
- r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 0);
+ r = drm_exec_prepare_array(&exec, gobj_read, num_read_bo_handles, 0);
+ drm_exec_retry_on_contention(&exec);
+ if (r) {
+ drm_exec_fini(&exec);
+ goto put_gobj_write;
+ }
+
+ r = drm_exec_prepare_array(&exec, gobj_write, num_write_bo_handles, 0);
drm_exec_retry_on_contention(&exec);
if (r) {
drm_exec_fini(&exec);
- goto put_gobj;
+ goto put_gobj_write;
}
}
@@ -611,13 +682,21 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
/* Count GEM objects fence */
- for (i = 0; i < num_bo_handles; i++) {
+ for (i = 0; i < num_read_bo_handles; i++) {
struct dma_resv_iter resv_cursor;
struct dma_fence *fence;
- dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
- dma_resv_usage_rw(wait_info->bo_wait_flags &
- AMDGPU_USERQ_BO_WRITE), fence)
+ dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
+ DMA_RESV_USAGE_READ, fence)
+ num_fences++;
+ }
+
+ for (i = 0; i < num_write_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
+
+ dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
+ DMA_RESV_USAGE_WRITE, fence)
num_fences++;
}
@@ -643,14 +722,30 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
goto free_fence_info;
}
- /* Retrieve GEM objects fence */
- for (i = 0; i < num_bo_handles; i++) {
+ /* Retrieve GEM read objects fence */
+ for (i = 0; i < num_read_bo_handles; i++) {
struct dma_resv_iter resv_cursor;
struct dma_fence *fence;
- dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
- dma_resv_usage_rw(wait_info->bo_wait_flags &
- AMDGPU_USERQ_BO_WRITE), fence) {
+ dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
+ DMA_RESV_USAGE_READ, fence) {
+ if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
+ r = -EINVAL;
+ goto free_fences;
+ }
+
+ fences[num_fences++] = fence;
+ dma_fence_get(fence);
+ }
+ }
+
+ /* Retrieve GEM write objects fence */
+ for (i = 0; i < num_write_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
+
+ dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
+ DMA_RESV_USAGE_WRITE, fence) {
if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
r = -EINVAL;
goto free_fences;
@@ -771,14 +866,19 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
drm_exec_fini(&exec);
- for (i = 0; i < num_bo_handles; i++)
- drm_gem_object_put(gobj[i]);
- kfree(gobj);
+ for (i = 0; i < num_read_bo_handles; i++)
+ drm_gem_object_put(gobj_read[i]);
+ kfree(gobj_read);
+
+ for (i = 0; i < num_write_bo_handles; i++)
+ drm_gem_object_put(gobj_write[i]);
+ kfree(gobj_write);
kfree(timeline_points);
kfree(timeline_handles);
kfree(syncobj_handles);
- kfree(bo_handles);
+ kfree(bo_handles_write);
+ kfree(bo_handles_read);
return 0;
@@ -790,18 +890,24 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
kfree(fence_info);
exec_fini:
drm_exec_fini(&exec);
-put_gobj:
- while (entry-- > 0)
- drm_gem_object_put(gobj[entry]);
- kfree(gobj);
+put_gobj_write:
+ while (wentry-- > 0)
+ drm_gem_object_put(gobj_write[wentry]);
+ kfree(gobj_write);
+put_gobj_read:
+ while (rentry-- > 0)
+ drm_gem_object_put(gobj_read[rentry]);
+ kfree(gobj_read);
free_timeline_points:
kfree(timeline_points);
free_timeline_handles:
kfree(timeline_handles);
free_syncobj_handles:
kfree(syncobj_handles);
-free_bo_handles:
- kfree(bo_handles);
+free_bo_handles_write:
+ kfree(bo_handles_write);
+free_bo_handles_read:
+ kfree(bo_handles_read);
return r;
}
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index eeb345ddbf57..8d21e765094b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -443,9 +443,6 @@ struct drm_amdgpu_userq_mqd_compute_gfx_v11 {
__u64 eop_va;
};
-/* dma_resv usage flag */
-#define AMDGPU_USERQ_BO_WRITE 1
-
/* userq signal/wait ioctl */
struct drm_amdgpu_userq_signal {
/**
@@ -475,20 +472,32 @@ struct drm_amdgpu_userq_signal {
*/
__u64 syncobj_point;
/**
- * @bo_handles_array: An array of GEM BO handles used by the userq fence creation
- * IOCTL to install the created dma_fence object which can be utilized by
+ * @bo_handles_read_array: An array of GEM read BO handles used by the userq fence
+ * creation IOCTL to install the created dma_fence object which can be utilized by
+ * userspace to synchronize the BO usage between user processes.
+ */
+ __u64 bo_handles_read_array;
+ /**
+ * @bo_handles_write_array: An array of GEM write BO handles used by the userq fence
+ * creation IOCTL to install the created dma_fence object which can be utilized by
* userspace to synchronize the BO usage between user processes.
*/
- __u64 bo_handles_array;
+ __u64 bo_handles_write_array;
+ /**
+ * @num_read_bo_handles: A count that represents the number of GEM read BO handles in
+ * @bo_handles_read_array.
+ */
+ __u32 num_read_bo_handles;
/**
- * @num_bo_handles: A count that represents the number of GEM BO handles in
- * @bo_handles_array.
+ * @num_write_bo_handles: A count that represents the number of GEM write BO handles in
+ * @bo_handles_write_array.
*/
- __u32 num_bo_handles;
+ __u32 num_write_bo_handles;
/**
* @bo_flags: flags to indicate BOs synchronize for READ or WRITE
*/
__u32 bo_flags;
+ __u32 pad;
};
struct drm_amdgpu_userq_fence_info {
@@ -542,20 +551,31 @@ struct drm_amdgpu_userq_wait {
*/
__u64 syncobj_timeline_points;
/**
- * @bo_handles_array: An array of GEM BO handles defined to fetch the fence
+ * @bo_handles_read_array: An array of GEM read BO handles defined to fetch the fence
* wait information of every BO handles in the array.
*/
- __u64 bo_handles_array;
+ __u64 bo_handles_read_array;
+ /**
+ * @bo_handles_write_array: An array of GEM write BO handles defined to fetch the fence
+ * wait information of every BO handles in the array.
+ */
+ __u64 bo_handles_write_array;
/**
* @num_syncobj_handles: A count that represents the number of syncobj handles in
* @syncobj_handles_array.
*/
__u32 num_syncobj_handles;
/**
- * @num_bo_handles: A count that represents the number of GEM BO handles in
- * @bo_handles_array.
+ * @num_read_bo_handles: A count that represents the number of GEM BO handles in
+ * @bo_handles_read_array.
+ */
+ __u32 num_read_bo_handles;
+ /**
+ * @num_write_bo_handles: A count that represents the number of GEM BO handles in
+ * @bo_handles_write_array.
*/
- __u32 num_bo_handles;
+ __u32 num_write_bo_handles;
+ __u32 pad;
/**
* @userq_fence_info: An array of fence information (va and value) pair of each
* objects stored in @syncobj_handles_array and @bo_handles_array.
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
` (7 preceding siblings ...)
2024-09-30 11:59 ` [PATCH v3 09/09] drm/amdgpu: Add separate array of read and write for BO handles Arunpravin Paneer Selvam
@ 2024-09-30 12:09 ` Christian König
8 siblings, 0 replies; 14+ messages in thread
From: Christian König @ 2024-09-30 12:09 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 30.09.24 um 13:58 schrieb Arunpravin Paneer Selvam:
> This patch introduces new IOCTL for userqueue secure semaphore.
>
> The signal IOCTL called from userspace application creates a drm
> syncobj and array of bo GEM handles and passed in as parameter to
> the driver to install the fence into it.
>
> The wait IOCTL gets an array of drm syncobjs, finds the fences
> attached to the drm syncobjs and obtain the array of
> memory_address/fence_value combintion which are returned to
> userspace.
>
> v2: (Christian)
> - Install fence into GEM BO object.
> - Lock all BO's using the dma resv subsystem
> - Reorder the sequence in signal IOCTL function.
> - Get write pointer from the shadow wptr
> - use userq_fence to fetch the va/value in wait IOCTL.
>
> v3: (Christian)
> - Use drm_exec helper for the proper BO drm reserve and avoid BO
> lock/unlock issues.
> - fence/fence driver reference count logic for signal/wait IOCTLs.
>
> v4: (Christian)
> - Fixed the drm_exec calling sequence
> - use dma_resv_for_each_fence_unlock if BO's are not locked
> - Modified the fence_info array storing logic.
>
> v5: (Christian)
> - Keep fence_drv until wait queue execution.
> - Add dma_fence_wait for other fences.
> - Lock BO's using drm_exec as the number of fences in them could
> change.
> - Install signaled fences as well into BO/Syncobj.
> - Move Syncobj fence installation code after the drm_exec_prepare_array.
> - Directly add dma_resv_usage_rw(args->bo_flags....
> - remove unnecessary dma_fence_put.
>
> v6: (Christian)
> - Add xarray stuff to store the fence_drv
> - Implement a function to iterate over the xarray and drop
> the fence_drv references.
> - Add drm_exec_until_all_locked() wrapper
> - Add a check that if we haven't exceeded the user allocated num_fences
> before adding dma_fence to the fences array.
>
> v7: (Christian)
> - Use memdup_user() for kmalloc_array + copy_from_user
> - Move the fence_drv references from the xarray into the newly created fence
> and drop the fence_drv references when we signal this fence.
> - Move this locking of BOs before the "if (!wait_info->num_fences)",
> this way you need this code block only once.
> - Merge the error handling code and the cleanup + return 0 code.
> - Initializing the xa should probably be done in the userq code.
> - Remove the userq back pointer stored in fence_drv.
> - Pass xarray as parameter in amdgpu_userq_walk_and_drop_fence_drv()
>
> v8: (Christian)
> - Move fence_drv references must come before adding the fence to the list.
> - Use xa_lock_irqsave_nested for nested spinlock operations.
> - userq_mgr should be per fpriv and not one per device.
> - Restructure the interrupt process code for the early exit of the loop.
> - The reference acquired in the syncobj fence replace code needs to be
> kept around.
> - Modify the dma_fence acquire placement in wait IOCTL.
> - Move USERQ_BO_WRITE flag to UAPI header file.
> - drop the fence drv reference after telling the hw to stop accessing it.
> - Add multi sync object support to userq signal IOCTL.
>
> V9: (Christian)
> - Store all the fence_drv ref to other drivers and not ourself.
> - Remove the userq fence xa implementation and replace with
> kvmalloc_array.
>
> v10: (Christian)
> - Add a comment for the userq_xa xarray
> - drop the if check of userq_fence->fence_drv_array
> - use the i variable to initialize userq_fence->fence_drv_array_count
> - drop the fence reference before you free the array in the error handling,
> otherwise it could be that some references leaked.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> Suggested-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 6 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 446 +++++++++++++++++-
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 7 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 29 +-
> .../gpu/drm/amd/include/amdgpu_userqueue.h | 1 +
> 6 files changed, 483 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a31f6c92a755..6178a1cda223 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1043,6 +1043,12 @@ struct amdgpu_device {
> struct amdgpu_mqd mqds[AMDGPU_HW_IP_NUM];
> const struct amdgpu_userq_funcs *userq_funcs[AMDGPU_HW_IP_NUM];
>
> + /* xarray used to retrieve the user queue fence driver reference
> + * in the EOP interrupt handler to signal the particular user
> + * queue fence.
> + */
> + struct xarray userq_xa;
> +
> /* df */
> struct amdgpu_df df;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 6e51b27b833d..70cb3b794a8a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2878,6 +2878,8 @@ const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
> DRM_IOCTL_DEF_DRV(AMDGPU_GEM_OP, amdgpu_gem_op_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(AMDGPU_GEM_USERPTR, amdgpu_gem_userptr_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(AMDGPU_USERQ, amdgpu_userq_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_SIGNAL, amdgpu_userq_signal_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(AMDGPU_USERQ_WAIT, amdgpu_userq_wait_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
> };
>
> static const struct drm_driver amdgpu_kms_driver = {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index f7baea2c67ab..95f232ad4a3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -25,6 +25,7 @@
> #include <linux/kref.h>
> #include <linux/slab.h>
>
> +#include <drm/drm_exec.h>
> #include <drm/drm_syncobj.h>
>
> #include "amdgpu.h"
> @@ -92,6 +93,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> spin_lock_init(&fence_drv->fence_list_lock);
>
> fence_drv->adev = adev;
> + fence_drv->uq_fence_drv_xa_ref = &userq->uq_fence_drv_xa;
> fence_drv->context = dma_fence_context_alloc(1);
> get_task_comm(fence_drv->timeline_name, current);
>
> @@ -105,6 +107,7 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d
> struct amdgpu_userq_fence *userq_fence, *tmp;
> struct dma_fence *fence;
> u64 rptr;
> + int i;
>
> if (!fence_drv)
> return;
> @@ -115,14 +118,16 @@ void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_d
> list_for_each_entry_safe(userq_fence, tmp, &fence_drv->fences, link) {
> fence = &userq_fence->base;
>
> - if (rptr >= fence->seqno) {
> - dma_fence_signal(fence);
> - list_del(&userq_fence->link);
> -
> - dma_fence_put(fence);
> - } else {
> + if (rptr < fence->seqno)
> break;
> - }
> +
> + dma_fence_signal(fence);
> +
> + for (i = 0; i < userq_fence->fence_drv_array_count; i++)
> + amdgpu_userq_fence_driver_put(userq_fence->fence_drv_array[i]);
> +
> + list_del(&userq_fence->link);
> + dma_fence_put(fence);
> }
> spin_unlock(&fence_drv->fence_list_lock);
> }
> @@ -132,8 +137,11 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
> struct amdgpu_userq_fence_driver *fence_drv = container_of(ref,
> struct amdgpu_userq_fence_driver,
> refcount);
> + struct amdgpu_userq_fence_driver *xa_fence_drv;
> struct amdgpu_device *adev = fence_drv->adev;
> struct amdgpu_userq_fence *fence, *tmp;
> + struct xarray *xa = &adev->userq_xa;
> + unsigned long index;
> struct dma_fence *f;
>
> spin_lock(&fence_drv->fence_list_lock);
> @@ -150,6 +158,12 @@ void amdgpu_userq_fence_driver_destroy(struct kref *ref)
> }
> spin_unlock(&fence_drv->fence_list_lock);
>
> + xa_lock(xa);
> + xa_for_each(xa, index, xa_fence_drv)
> + if (xa_fence_drv == fence_drv)
> + __xa_erase(xa, index);
> + xa_unlock(xa);
> +
> /* Free seq64 memory */
> amdgpu_seq64_free(adev, fence_drv->gpu_addr);
> kfree(fence_drv);
> @@ -191,6 +205,33 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
> amdgpu_userq_fence_driver_get(fence_drv);
> dma_fence_get(fence);
>
> + if (!xa_empty(&userq->uq_fence_drv_xa)) {
> + struct amdgpu_userq_fence_driver *stored_fence_drv;
> + unsigned long index, count = 0;
> + int i = 0;
> +
> + xa_for_each(&userq->uq_fence_drv_xa, index, stored_fence_drv)
> + count++;
> +
> + userq_fence->fence_drv_array =
> + kvmalloc_array(count,
> + sizeof(struct amdgpu_userq_fence_driver *),
> + GFP_KERNEL);
> +
> + if (userq_fence->fence_drv_array) {
> + xa_for_each(&userq->uq_fence_drv_xa, index, stored_fence_drv) {
> + userq_fence->fence_drv_array[i] = stored_fence_drv;
> + xa_erase(&userq->uq_fence_drv_xa, index);
> + i++;
> + }
> + }
> +
> + userq_fence->fence_drv_array_count = i;
> + } else {
> + userq_fence->fence_drv_array = NULL;
> + userq_fence->fence_drv_array_count = 0;
> + }
> +
> spin_lock(&fence_drv->fence_list_lock);
> /* Check if hardware has already processed the job */
> if (!dma_fence_is_signaled(fence))
> @@ -240,6 +281,8 @@ static void amdgpu_userq_fence_free(struct rcu_head *rcu)
>
> /* Release the fence driver reference */
> amdgpu_userq_fence_driver_put(fence_drv);
> +
> + kvfree(userq_fence->fence_drv_array);
> kmem_cache_free(amdgpu_userq_fence_slab, userq_fence);
> }
>
> @@ -255,3 +298,392 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> .signaled = amdgpu_userq_fence_signaled,
> .release = amdgpu_userq_fence_release,
> };
> +
> +/**
> + * amdgpu_userq_fence_read_wptr - Read the userq wptr value
> + *
> + * @filp: drm file private data structure
> + * @queue: user mode queue structure pointer
> + * @wptr: write pointer value
> + *
> + * Read the wptr value from userq's MQD. The userq signal IOCTL
> + * creates a dma_fence for the shared buffers that expects the
> + * RPTR value written to seq64 memory >= WPTR.
> + *
> + * Returns wptr value on success, error on failure.
> + */
> +static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
> + struct amdgpu_usermode_queue *queue,
> + u64 *wptr)
> +{
> + struct amdgpu_fpriv *fpriv = filp->driver_priv;
> + struct amdgpu_bo_va_mapping *mapping;
> + struct amdgpu_vm *vm = &fpriv->vm;
> + struct amdgpu_bo *bo;
> + u64 addr, *ptr;
> + int r;
> +
> + addr = queue->userq_prop->wptr_gpu_addr;
> + addr &= AMDGPU_GMC_HOLE_MASK;
> +
> + mapping = amdgpu_vm_bo_lookup_mapping(vm, addr >> PAGE_SHIFT);
> + if (!mapping)
> + return -EINVAL;
> +
> + bo = mapping->bo_va->base.bo;
> + r = amdgpu_bo_reserve(bo, true);
> + if (r) {
> + DRM_ERROR("Failed to reserve userqueue wptr bo");
> + return r;
> + }
> +
> + r = amdgpu_bo_kmap(bo, (void **)&ptr);
> + if (r) {
> + DRM_ERROR("Failed mapping the userqueue wptr bo");
> + goto map_error;
> + }
> +
> + *wptr = le64_to_cpu(*ptr);
> +
> + amdgpu_bo_kunmap(bo);
> + amdgpu_bo_unreserve(bo);
> +
> + return 0;
> +
> +map_error:
> + amdgpu_bo_unreserve(bo);
> + return r;
> +}
> +
> +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> +{
> + struct amdgpu_fpriv *fpriv = filp->driver_priv;
> + struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
> + struct drm_amdgpu_userq_signal *args = data;
> + struct amdgpu_usermode_queue *queue;
> + struct drm_gem_object **gobj = NULL;
> + struct drm_syncobj **syncobj = NULL;
> + u32 *syncobj_handles, num_syncobj_handles;
> + u32 *bo_handles, num_bo_handles;
> + int r, i, entry, boentry;
> + struct dma_fence *fence;
> + struct drm_exec exec;
> + u64 wptr;
> +
> + /* Array of syncobj handles */
> + num_syncobj_handles = args->num_syncobj_handles;
> + syncobj_handles = memdup_user(u64_to_user_ptr(args->syncobj_handles_array),
> + sizeof(u32) * num_syncobj_handles);
> + if (IS_ERR(syncobj_handles))
> + return PTR_ERR(syncobj_handles);
> +
> + /* Array of pointers to the looked up syncobjs */
> + syncobj = kmalloc_array(num_syncobj_handles, sizeof(*syncobj), GFP_KERNEL);
> + if (!syncobj) {
> + r = -ENOMEM;
> + goto free_syncobj_handles;
> + }
> +
> + for (entry = 0; entry < num_syncobj_handles; entry++) {
> + syncobj[entry] = drm_syncobj_find(filp, syncobj_handles[entry]);
> + if (!syncobj[entry]) {
> + r = -ENOENT;
> + goto free_syncobj;
> + }
> + }
> +
> + /* Array of bo handles */
> + num_bo_handles = args->num_bo_handles;
> + bo_handles = memdup_user(u64_to_user_ptr(args->bo_handles_array),
> + sizeof(u32) * num_bo_handles);
> + if (IS_ERR(bo_handles))
> + goto free_syncobj;
> +
> + /* Array of pointers to the GEM objects */
> + gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
> + if (!gobj) {
> + r = -ENOMEM;
> + goto free_bo_handles;
> + }
> +
> + for (boentry = 0; boentry < num_bo_handles; boentry++) {
> + gobj[boentry] = drm_gem_object_lookup(filp, bo_handles[boentry]);
> + if (!gobj[boentry]) {
> + r = -ENOENT;
> + goto put_gobj;
> + }
> + }
> +
> + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
> + drm_exec_until_all_locked(&exec) {
> + r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 1);
> + drm_exec_retry_on_contention(&exec);
> + if (r)
> + goto exec_fini;
> + }
> +
> + /*Retrieve the user queue */
> + queue = idr_find(&userq_mgr->userq_idr, args->queue_id);
> + if (!queue) {
> + r = -ENOENT;
> + goto exec_fini;
> + }
> +
> + r = amdgpu_userq_fence_read_wptr(filp, queue, &wptr);
> + if (r)
> + goto exec_fini;
> +
> + /* Create a new fence */
> + r = amdgpu_userq_fence_create(queue, wptr, &fence);
> + if (r)
> + goto exec_fini;
> +
> + for (i = 0; i < num_bo_handles; i++)
> + dma_resv_add_fence(gobj[i]->resv, fence,
> + dma_resv_usage_rw(args->bo_flags &
> + AMDGPU_USERQ_BO_WRITE));
> +
> + /* Add the created fence to syncobj/BO's */
> + for (i = 0; i < num_syncobj_handles; i++)
> + drm_syncobj_replace_fence(syncobj[i], fence);
> +
> + /* drop the reference acquired in fence creation function */
> + dma_fence_put(fence);
> +
> +exec_fini:
> + drm_exec_fini(&exec);
> +put_gobj:
> + while (boentry-- > 0)
> + drm_gem_object_put(gobj[boentry]);
> + kfree(gobj);
> +free_bo_handles:
> + kfree(bo_handles);
> +free_syncobj:
> + while (entry-- > 0)
> + if (syncobj[entry])
> + drm_syncobj_put(syncobj[entry]);
> + kfree(syncobj);
> +free_syncobj_handles:
> + kfree(syncobj_handles);
> +
> + return r;
> +}
> +
> +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp)
> +{
> + struct drm_amdgpu_userq_fence_info *fence_info = NULL;
> + struct drm_amdgpu_userq_wait *wait_info = data;
> + u32 *syncobj_handles, *bo_handles;
> + struct dma_fence **fences = NULL;
> + u32 num_syncobj, num_bo_handles;
> + struct drm_gem_object **gobj;
> + struct drm_exec exec;
> + int r, i, entry, cnt;
> + u64 num_fences = 0;
> +
> + num_bo_handles = wait_info->num_bo_handles;
> + bo_handles = memdup_user(u64_to_user_ptr(wait_info->bo_handles_array),
> + sizeof(u32) * num_bo_handles);
> + if (IS_ERR(bo_handles))
> + return PTR_ERR(bo_handles);
> +
> + num_syncobj = wait_info->num_syncobj_handles;
> + syncobj_handles = memdup_user(u64_to_user_ptr(wait_info->syncobj_handles_array),
> + sizeof(u32) * num_syncobj);
> + if (IS_ERR(syncobj_handles)) {
> + r = PTR_ERR(syncobj_handles);
> + goto free_bo_handles;
> + }
> +
> + /* Array of GEM object handles */
> + gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
> + if (!gobj) {
> + r = -ENOMEM;
> + goto free_syncobj_handles;
> + }
> +
> + for (entry = 0; entry < num_bo_handles; entry++) {
> + gobj[entry] = drm_gem_object_lookup(filp, bo_handles[entry]);
> + if (!gobj[entry]) {
> + r = -ENOENT;
> + goto put_gobj;
> + }
> + }
> +
> + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, 0);
> + drm_exec_until_all_locked(&exec) {
> + r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 0);
> + drm_exec_retry_on_contention(&exec);
> + if (r) {
> + drm_exec_fini(&exec);
> + goto put_gobj;
> + }
> + }
> +
> + if (!wait_info->num_fences) {
> + /* Count syncobj's fence */
> + for (i = 0; i < num_syncobj; i++) {
> + struct dma_fence *fence;
> +
> + r = drm_syncobj_find_fence(filp, syncobj_handles[i],
> + 0, 0, &fence);
> + if (r)
> + goto exec_fini;
> +
> + num_fences++;
> + dma_fence_put(fence);
> + }
> +
> + /* Count GEM objects fence */
> + for (i = 0; i < num_bo_handles; i++) {
> + struct dma_resv_iter resv_cursor;
> + struct dma_fence *fence;
> +
> + dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
> + dma_resv_usage_rw(wait_info->bo_wait_flags &
> + AMDGPU_USERQ_BO_WRITE), fence)
> + num_fences++;
> + }
> +
> + /*
> + * Passing num_fences = 0 means that userspace doesn't want to
> + * retrieve userq_fence_info. If num_fences = 0 we skip filling
> + * userq_fence_info and return the actual number of fences on
> + * args->num_fences.
> + */
> + wait_info->num_fences = num_fences;
> + } else {
> + /* Array of fence info */
> + fence_info = kmalloc_array(wait_info->num_fences, sizeof(*fence_info), GFP_KERNEL);
> + if (!fence_info) {
> + r = -ENOMEM;
> + goto exec_fini;
> + }
> +
> + /* Array of fences */
> + fences = kmalloc_array(wait_info->num_fences, sizeof(*fences), GFP_KERNEL);
> + if (!fences) {
> + r = -ENOMEM;
> + goto free_fence_info;
> + }
> +
> + /* Retrieve GEM objects fence */
> + for (i = 0; i < num_bo_handles; i++) {
> + struct dma_resv_iter resv_cursor;
> + struct dma_fence *fence;
> +
> + dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
> + dma_resv_usage_rw(wait_info->bo_wait_flags &
> + AMDGPU_USERQ_BO_WRITE), fence) {
> + if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
> + r = -EINVAL;
> + goto free_fences;
> + }
> +
> + fences[num_fences++] = fence;
> + dma_fence_get(fence);
> + }
> + }
> +
> + /* Retrieve syncobj's fence */
> + for (i = 0; i < num_syncobj; i++) {
> + struct dma_fence *fence;
> +
> + r = drm_syncobj_find_fence(filp, syncobj_handles[i],
> + 0, 0, &fence);
> + if (r)
> + goto free_fences;
> +
> + if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
> + r = -EINVAL;
> + goto free_fences;
> + }
> +
> + fences[num_fences++] = fence;
> + }
> +
> + for (i = 0, cnt = 0; i < wait_info->num_fences; i++) {
> + struct amdgpu_userq_fence_driver *fence_drv;
> + struct amdgpu_userq_fence *userq_fence;
> + u32 index;
> +
> + userq_fence = to_amdgpu_userq_fence(fences[i]);
> + if (!userq_fence) {
> + /*
> + * Just waiting on other driver fences should
> + * be good for now
> + */
> + dma_fence_wait(fences[i], false);
> + dma_fence_put(fences[i]);
> +
> + continue;
> + }
> +
> + fence_drv = userq_fence->fence_drv;
> + /*
> + * We need to make sure the user queue release their reference
> + * to the fence drivers at some point before queue destruction.
> + * Otherwise, we would gather those references until we don't
> + * have any more space left and crash.
> + */
> + if (fence_drv->uq_fence_drv_xa_ref) {
> + r = xa_alloc(fence_drv->uq_fence_drv_xa_ref, &index, fence_drv,
> + xa_limit_32b, GFP_KERNEL);
> + if (r)
> + goto free_fences;
> +
> + amdgpu_userq_fence_driver_get(fence_drv);
> + }
> +
> + /* Store drm syncobj's gpu va address and value */
> + fence_info[cnt].va = fence_drv->gpu_addr;
> + fence_info[cnt].value = fences[i]->seqno;
> +
> + dma_fence_put(fences[i]);
> + /* Increment the actual userq fence count */
> + cnt++;
> + }
> +
> + wait_info->num_fences = cnt;
> + /* Copy userq fence info to user space */
> + if (copy_to_user(u64_to_user_ptr(wait_info->userq_fence_info),
> + fence_info, wait_info->num_fences * sizeof(*fence_info))) {
> + r = -EFAULT;
> + goto free_fences;
> + }
> +
> + kfree(fences);
> + kfree(fence_info);
> + }
> +
> + drm_exec_fini(&exec);
> + for (i = 0; i < num_bo_handles; i++)
> + drm_gem_object_put(gobj[i]);
> + kfree(gobj);
> +
> + kfree(syncobj_handles);
> + kfree(bo_handles);
> +
> + return 0;
> +
> +free_fences:
> + while (num_fences-- > 0)
> + dma_fence_put(fences[num_fences]);
> + kfree(fences);
> +free_fence_info:
> + kfree(fence_info);
> +exec_fini:
> + drm_exec_fini(&exec);
> +put_gobj:
> + while (entry-- > 0)
> + drm_gem_object_put(gobj[entry]);
> + kfree(gobj);
> +free_syncobj_handles:
> + kfree(syncobj_handles);
> +free_bo_handles:
> + kfree(bo_handles);
> +
> + return r;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> index c3e04cdbb9e7..f72424248cc5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
> @@ -37,7 +37,9 @@ struct amdgpu_userq_fence {
> */
> spinlock_t lock;
> struct list_head link;
> + unsigned long fence_drv_array_count;
> struct amdgpu_userq_fence_driver *fence_drv;
> + struct amdgpu_userq_fence_driver **fence_drv_array;
> };
>
> struct amdgpu_userq_fence_driver {
> @@ -52,6 +54,7 @@ struct amdgpu_userq_fence_driver {
> spinlock_t fence_list_lock;
> struct list_head fences;
> struct amdgpu_device *adev;
> + struct xarray *uq_fence_drv_xa_ref;
> char timeline_name[TASK_COMM_LEN];
> };
>
> @@ -65,5 +68,9 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> struct amdgpu_usermode_queue *userq);
> void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv);
> void amdgpu_userq_fence_driver_destroy(struct kref *ref);
> +int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp);
> +int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *filp);
>
> #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> index 7f6495466975..ba986d55ceeb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
> @@ -27,6 +27,32 @@
> #include "amdgpu_userqueue.h"
> #include "amdgpu_userq_fence.h"
>
> +static void amdgpu_userq_walk_and_drop_fence_drv(struct xarray *xa)
> +{
> + struct amdgpu_userq_fence_driver *fence_drv;
> + unsigned long index;
> +
> + if (xa_empty(xa))
> + return;
> +
> + xa_lock(xa);
> + xa_for_each(xa, index, fence_drv) {
> + __xa_erase(xa, index);
> + amdgpu_userq_fence_driver_put(fence_drv);
> + }
> +
> + xa_unlock(xa);
> +}
> +
> +static void
> +amdgpu_userq_fence_driver_free(struct amdgpu_usermode_queue *userq)
> +{
> + amdgpu_userq_walk_and_drop_fence_drv(&userq->uq_fence_drv_xa);
> + xa_destroy(&userq->uq_fence_drv_xa);
> + /* Drop the fence_drv reference held by user queue */
> + amdgpu_userq_fence_driver_put(userq->fence_drv);
> +}
> +
> static void
> amdgpu_userqueue_cleanup(struct amdgpu_userq_mgr *uq_mgr,
> struct amdgpu_usermode_queue *queue,
> @@ -36,7 +62,7 @@ amdgpu_userqueue_cleanup(struct amdgpu_userq_mgr *uq_mgr,
> const struct amdgpu_userq_funcs *uq_funcs = adev->userq_funcs[queue->queue_type];
>
> uq_funcs->mqd_destroy(uq_mgr, queue);
> - amdgpu_userq_fence_driver_put(queue->fence_drv);
> + amdgpu_userq_fence_driver_free(queue);
> idr_remove(&uq_mgr->userq_idr, queue_id);
> kfree(queue);
> }
> @@ -320,6 +346,7 @@ amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq *args)
> }
> queue->doorbell_index = index;
>
> + xa_init_flags(&queue->uq_fence_drv_xa, XA_FLAGS_ALLOC);
> r = amdgpu_userq_fence_driver_alloc(adev, queue);
> if (r) {
> DRM_ERROR("Failed to alloc fence driver\n");
> diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
> index 5fbffde48999..77a33f9e37f8 100644
> --- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
> +++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
> @@ -47,6 +47,7 @@ struct amdgpu_usermode_queue {
> struct amdgpu_userq_obj db_obj;
> struct amdgpu_userq_obj fw_obj;
> struct amdgpu_userq_obj wptr_obj;
> + struct xarray uq_fence_drv_xa;
> struct amdgpu_userq_fence_driver *fence_drv;
> };
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 03/09] drm/amdgpu: Add wait IOCTL timeline syncobj support
2024-09-30 11:59 ` [PATCH v3 03/09] drm/amdgpu: Add wait IOCTL timeline syncobj support Arunpravin Paneer Selvam
@ 2024-09-30 12:21 ` Christian König
0 siblings, 0 replies; 14+ messages in thread
From: Christian König @ 2024-09-30 12:21 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 30.09.24 um 13:59 schrieb Arunpravin Paneer Selvam:
> Add user fence wait IOCTL timeline syncobj support.
>
> v2:(Christian)
> - handle dma_fence_wait() return value.
> - shorten the variable name syncobj_timeline_points a bit.
> - move num_points up to avoid padding issues.
>
> v3:(Christian)
> - Handle timeline drm_syncobj_find_fence() call error
> handling
> - Use dma_fence_unwrap_for_each() in timeline fence as
> there could be more than one fence.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 99 +++++++++++++++++--
> include/uapi/drm/amdgpu_drm.h | 16 ++-
> 2 files changed, 107 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 8f9d2427d380..76552eed6d86 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -24,6 +24,7 @@
>
> #include <linux/kref.h>
> #include <linux/slab.h>
> +#include <linux/dma-fence-unwrap.h>
>
> #include <drm/drm_exec.h>
> #include <drm/drm_syncobj.h>
> @@ -474,11 +475,11 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp)
> {
> + u32 *syncobj_handles, *timeline_points, *timeline_handles, *bo_handles;
> + u32 num_syncobj, num_bo_handles, num_points;
> struct drm_amdgpu_userq_fence_info *fence_info = NULL;
> struct drm_amdgpu_userq_wait *wait_info = data;
> - u32 *syncobj_handles, *bo_handles;
> struct dma_fence **fences = NULL;
> - u32 num_syncobj, num_bo_handles;
> struct drm_gem_object **gobj;
> struct drm_exec exec;
> int r, i, entry, cnt;
> @@ -498,11 +499,26 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> goto free_bo_handles;
> }
>
> + num_points = wait_info->num_points;
> + timeline_handles = memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_handles),
> + sizeof(u32) * num_points);
> + if (IS_ERR(timeline_handles)) {
> + r = PTR_ERR(timeline_handles);
> + goto free_syncobj_handles;
> + }
> +
> + timeline_points = memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_points),
> + sizeof(u32) * num_points);
> + if (IS_ERR(timeline_points)) {
> + r = PTR_ERR(timeline_points);
> + goto free_timeline_handles;
> + }
> +
> /* Array of GEM object handles */
> gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
> if (!gobj) {
> r = -ENOMEM;
> - goto free_syncobj_handles;
> + goto free_timeline_points;
> }
>
> for (entry = 0; entry < num_bo_handles; entry++) {
> @@ -524,15 +540,40 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> }
>
> if (!wait_info->num_fences) {
> + if (num_points) {
> + struct dma_fence_unwrap iter;
> + struct dma_fence *fence;
> + struct dma_fence *f;
> +
> + for (i = 0; i < num_points; i++) {
> + r = drm_syncobj_find_fence(filp, timeline_handles[i],
> + timeline_points[i],
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + &fence);
> + if (r)
> + goto exec_fini;
> +
> + num_fences++;
> +
> + dma_fence_unwrap_for_each(f, &iter, fence)
> + num_fences++;
This duplicates the num_fences increment, just drop the first one since
fence is always included in the iteration as well.
> +
> + dma_fence_put(fence);
> + }
> + }
> +
> /* Count syncobj's fence */
> for (i = 0; i < num_syncobj; i++) {
> struct dma_fence *fence;
>
> r = drm_syncobj_find_fence(filp, syncobj_handles[i],
> - 0, 0, &fence);
> + 0,
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + &fence);
> if (r)
> goto exec_fini;
>
> + dma_fence_put(fence);
> num_fences++;
> dma_fence_put(fence);
> }
> @@ -588,12 +629,46 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> }
> }
>
> + if (num_points) {
> + struct dma_fence_unwrap iter;
> + struct dma_fence *fence;
> + struct dma_fence *f;
> +
> + for (i = 0; i < num_points; i++) {
> + r = drm_syncobj_find_fence(filp, timeline_handles[i],
> + timeline_points[i],
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + &fence);
> + if (r)
> + goto free_fences;
> +
> + if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
> + r = -EINVAL;
> + goto free_fences;
> + }
> +
> + fences[num_fences++] = fence;
Same here.
Apart from that looks good to me. We could cleanup the code a bit more,
but that can come later on.
Regards,
Christian.
> +
> + dma_fence_unwrap_for_each(f, &iter, fence) {
> + if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
> + r = -EINVAL;
> + goto free_fences;
> + }
> +
> + dma_fence_get(f);
> + fences[num_fences++] = f;
> + }
> + }
> + }
> +
> /* Retrieve syncobj's fence */
> for (i = 0; i < num_syncobj; i++) {
> struct dma_fence *fence;
>
> r = drm_syncobj_find_fence(filp, syncobj_handles[i],
> - 0, 0, &fence);
> + 0,
> + DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
> + &fence);
> if (r)
> goto free_fences;
>
> @@ -616,9 +691,13 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> * Just waiting on other driver fences should
> * be good for now
> */
> - dma_fence_wait(fences[i], false);
> - dma_fence_put(fences[i]);
> + r = dma_fence_wait(fences[i], true);
> + if (r) {
> + dma_fence_put(fences[i]);
> + goto free_fences;
> + }
>
> + dma_fence_put(fences[i]);
> continue;
> }
>
> @@ -664,6 +743,8 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> drm_gem_object_put(gobj[i]);
> kfree(gobj);
>
> + kfree(timeline_points);
> + kfree(timeline_handles);
> kfree(syncobj_handles);
> kfree(bo_handles);
>
> @@ -681,6 +762,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> while (entry-- > 0)
> drm_gem_object_put(gobj[entry]);
> kfree(gobj);
> +free_timeline_points:
> + kfree(timeline_points);
> +free_timeline_handles:
> + kfree(timeline_handles);
> free_syncobj_handles:
> kfree(syncobj_handles);
> free_bo_handles:
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index af42798e901d..3b24e0cb1b51 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -521,12 +521,26 @@ struct drm_amdgpu_userq_wait {
> * matching fence wait info pair in @userq_fence_info.
> */
> __u32 bo_wait_flags;
> - __u32 pad;
> + /**
> + * @num_points: A count that represents the number of timeline syncobj handles in
> + * syncobj_handles_array.
> + */
> + __u32 num_points;
> /**
> * @syncobj_handles_array: An array of syncobj handles defined to get the
> * fence wait information of every syncobj handles in the array.
> */
> __u64 syncobj_handles_array;
> + /**
> + * @syncobj_timeline_handles: An array of timeline syncobj handles defined to get the
> + * fence wait information of every timeline syncobj handles in the array.
> + */
> + __u64 syncobj_timeline_handles;
> + /**
> + * @syncobj_timeline_points: An array of timeline syncobj points defined to get the
> + * fence wait points of every timeline syncobj handles in the syncobj_handles_array.
> + */
> + __u64 syncobj_timeline_points;
> /**
> * @bo_handles_array: An array of GEM BO handles defined to fetch the fence
> * wait information of every BO handles in the array.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 08/09] drm/amdgpu: add vm root BO lock before accessing the vm
2024-09-30 11:59 ` [PATCH v3 08/09] drm/amdgpu: add vm root BO lock before accessing the vm Arunpravin Paneer Selvam
@ 2024-09-30 12:50 ` Christian König
0 siblings, 0 replies; 14+ messages in thread
From: Christian König @ 2024-09-30 12:50 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 30.09.24 um 13:59 schrieb Arunpravin Paneer Selvam:
> Add a vm root BO lock before accessing the userqueue VM.
>
> v1:(Christian)
> - Keep the VM locked until you are done with the mapping.
> - Grab a temporary BO reference, drop the VM lock and acquire the BO.
> When you are done with everything just drop the BO lock and
> then the temporary BO reference.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 24 ++++++++++++-------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 9ca1910d7a62..0725f28f3484 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -321,7 +321,6 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> /**
> * amdgpu_userq_fence_read_wptr - Read the userq wptr value
> *
> - * @filp: drm file private data structure
> * @queue: user mode queue structure pointer
> * @wptr: write pointer value
> *
> @@ -331,25 +330,29 @@ static const struct dma_fence_ops amdgpu_userq_fence_ops = {
> *
> * Returns wptr value on success, error on failure.
> */
> -static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
> - struct amdgpu_usermode_queue *queue,
> +static int amdgpu_userq_fence_read_wptr(struct amdgpu_usermode_queue *queue,
> u64 *wptr)
> {
> - struct amdgpu_fpriv *fpriv = filp->driver_priv;
> struct amdgpu_bo_va_mapping *mapping;
> - struct amdgpu_vm *vm = &fpriv->vm;
> struct amdgpu_bo *bo;
> u64 addr, *ptr;
> int r;
>
> + r = amdgpu_bo_reserve(queue->vm->root.bo, false);
> + if (r)
> + return r;
> +
> addr = queue->userq_prop->wptr_gpu_addr;
> addr &= AMDGPU_GMC_HOLE_MASK;
>
> - mapping = amdgpu_vm_bo_lookup_mapping(vm, addr >> PAGE_SHIFT);
> - if (!mapping)
> + mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, addr >> PAGE_SHIFT);
> + if (!mapping) {
> + DRM_ERROR("Failed to lookup amdgpu_bo_va_mapping\n");
> return -EINVAL;
> + }
>
> - bo = mapping->bo_va->base.bo;
> + bo = amdgpu_bo_ref(mapping->bo_va->base.bo);
> + amdgpu_bo_unreserve(queue->vm->root.bo);
> r = amdgpu_bo_reserve(bo, true);
> if (r) {
> DRM_ERROR("Failed to reserve userqueue wptr bo");
> @@ -366,11 +369,14 @@ static int amdgpu_userq_fence_read_wptr(struct drm_file *filp,
>
> amdgpu_bo_kunmap(bo);
> amdgpu_bo_unreserve(bo);
> + amdgpu_bo_unref(&bo);
>
> return 0;
>
> map_error:
> amdgpu_bo_unreserve(bo);
> + amdgpu_bo_unref(&bo);
> +
> return r;
> }
>
> @@ -449,7 +455,7 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> goto exec_fini;
> }
>
> - r = amdgpu_userq_fence_read_wptr(filp, queue, &wptr);
> + r = amdgpu_userq_fence_read_wptr(queue, &wptr);
> if (r)
> goto exec_fini;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 09/09] drm/amdgpu: Add separate array of read and write for BO handles
2024-09-30 11:59 ` [PATCH v3 09/09] drm/amdgpu: Add separate array of read and write for BO handles Arunpravin Paneer Selvam
@ 2024-09-30 12:54 ` Christian König
0 siblings, 0 replies; 14+ messages in thread
From: Christian König @ 2024-09-30 12:54 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: alexander.deucher
Am 30.09.24 um 13:59 schrieb Arunpravin Paneer Selvam:
> Drop AMDGPU_USERQ_BO_WRITE as this should not be a global option
> of the IOCTL, It should be option per buffer. Hence adding separate
> array for read and write BO handles.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
Still some potential to de-duplicate code, but for now it should work.
Acked-by: Christian König <christian.koenig@amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 238 +++++++++++++-----
> include/uapi/drm/amdgpu_drm.h | 48 ++--
> 2 files changed, 206 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index 0725f28f3484..aa794f342036 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -386,12 +386,14 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> struct amdgpu_fpriv *fpriv = filp->driver_priv;
> struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
> struct drm_amdgpu_userq_signal *args = data;
> + struct drm_gem_object **gobj_write = NULL;
> + struct drm_gem_object **gobj_read = NULL;
> struct amdgpu_usermode_queue *queue;
> - struct drm_gem_object **gobj = NULL;
> struct drm_syncobj **syncobj = NULL;
> + u32 *bo_handles_write, num_write_bo_handles;
> u32 *syncobj_handles, num_syncobj_handles;
> - u32 *bo_handles, num_bo_handles;
> - int r, i, entry, boentry;
> + u32 *bo_handles_read, num_read_bo_handles;
> + int r, i, entry, rentry, wentry;
> struct dma_fence *fence;
> struct drm_exec exec;
> u64 wptr;
> @@ -417,32 +419,59 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> }
> }
>
> - num_bo_handles = args->num_bo_handles;
> - bo_handles = memdup_user(u64_to_user_ptr(args->bo_handles_array),
> - sizeof(u32) * num_bo_handles);
> - if (IS_ERR(bo_handles))
> + num_read_bo_handles = args->num_read_bo_handles;
> + bo_handles_read = memdup_user(u64_to_user_ptr(args->bo_handles_read_array),
> + sizeof(u32) * num_read_bo_handles);
> + if (IS_ERR(bo_handles_read))
> goto free_syncobj;
>
> - /* Array of pointers to the GEM objects */
> - gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
> - if (!gobj) {
> + /* Array of pointers to the GEM read objects */
> + gobj_read = kmalloc_array(num_read_bo_handles, sizeof(*gobj_read), GFP_KERNEL);
> + if (!gobj_read) {
> r = -ENOMEM;
> - goto free_bo_handles;
> + goto free_bo_handles_read;
> }
>
> - for (boentry = 0; boentry < num_bo_handles; boentry++) {
> - gobj[boentry] = drm_gem_object_lookup(filp, bo_handles[boentry]);
> - if (!gobj[boentry]) {
> + for (rentry = 0; rentry < num_read_bo_handles; rentry++) {
> + gobj_read[rentry] = drm_gem_object_lookup(filp, bo_handles_read[rentry]);
> + if (!gobj_read[rentry]) {
> r = -ENOENT;
> - goto put_gobj;
> + goto put_gobj_read;
> }
> }
>
> - drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, num_bo_handles);
> + num_write_bo_handles = args->num_write_bo_handles;
> + bo_handles_write = memdup_user(u64_to_user_ptr(args->bo_handles_write_array),
> + sizeof(u32) * num_write_bo_handles);
> + if (IS_ERR(bo_handles_write))
> + goto put_gobj_read;
> +
> + /* Array of pointers to the GEM write objects */
> + gobj_write = kmalloc_array(num_write_bo_handles, sizeof(*gobj_write), GFP_KERNEL);
> + if (!gobj_write) {
> + r = -ENOMEM;
> + goto free_bo_handles_write;
> + }
> +
> + for (wentry = 0; wentry < num_write_bo_handles; wentry++) {
> + gobj_write[wentry] = drm_gem_object_lookup(filp, bo_handles_write[wentry]);
> + if (!gobj_write[wentry]) {
> + r = -ENOENT;
> + goto put_gobj_write;
> + }
> + }
> +
> + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT,
> + (num_read_bo_handles + num_write_bo_handles));
>
> /* Lock all BOs with retry handling */
> drm_exec_until_all_locked(&exec) {
> - r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 1);
> + r = drm_exec_prepare_array(&exec, gobj_read, num_read_bo_handles, 1);
> + drm_exec_retry_on_contention(&exec);
> + if (r)
> + goto exec_fini;
> +
> + r = drm_exec_prepare_array(&exec, gobj_write, num_write_bo_handles, 1);
> drm_exec_retry_on_contention(&exec);
> if (r)
> goto exec_fini;
> @@ -468,13 +497,20 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> dma_fence_put(queue->last_fence);
> queue->last_fence = dma_fence_get(fence);
>
> - for (i = 0; i < num_bo_handles; i++) {
> - if (!gobj || !gobj[i]->resv)
> + for (i = 0; i < num_read_bo_handles; i++) {
> + if (!gobj_read || !gobj_read[i]->resv)
> continue;
>
> - dma_resv_add_fence(gobj[i]->resv, fence,
> - dma_resv_usage_rw(args->bo_flags &
> - AMDGPU_USERQ_BO_WRITE));
> + dma_resv_add_fence(gobj_read[i]->resv, fence,
> + DMA_RESV_USAGE_READ);
> + }
> +
> + for (i = 0; i < num_write_bo_handles; i++) {
> + if (!gobj_write || !gobj_write[i]->resv)
> + continue;
> +
> + dma_resv_add_fence(gobj_read[i]->resv, fence,
> + DMA_RESV_USAGE_WRITE);
> }
>
> /* Add the created fence to syncobj/BO's */
> @@ -486,12 +522,18 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
>
> exec_fini:
> drm_exec_fini(&exec);
> -put_gobj:
> - while (boentry-- > 0)
> - drm_gem_object_put(gobj[boentry]);
> - kfree(gobj);
> -free_bo_handles:
> - kfree(bo_handles);
> +put_gobj_write:
> + while (wentry-- > 0)
> + drm_gem_object_put(gobj_write[wentry]);
> + kfree(gobj_write);
> +free_bo_handles_write:
> + kfree(bo_handles_write);
> +put_gobj_read:
> + while (rentry-- > 0)
> + drm_gem_object_put(gobj_read[rentry]);
> + kfree(gobj_read);
> +free_bo_handles_read:
> + kfree(bo_handles_read);
> free_syncobj:
> while (entry-- > 0)
> if (syncobj[entry])
> @@ -506,28 +548,35 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
> int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> struct drm_file *filp)
> {
> - u32 *syncobj_handles, *timeline_points, *timeline_handles, *bo_handles;
> - u32 num_syncobj, num_bo_handles, num_points;
> + u32 *syncobj_handles, *timeline_points, *timeline_handles, *bo_handles_read, *bo_handles_write;
> + u32 num_syncobj, num_read_bo_handles, num_write_bo_handles, num_points;
> struct drm_amdgpu_userq_fence_info *fence_info = NULL;
> struct drm_amdgpu_userq_wait *wait_info = data;
> + struct drm_gem_object **gobj_write;
> + struct drm_gem_object **gobj_read;
> struct dma_fence **fences = NULL;
> - struct drm_gem_object **gobj;
> + int r, i, rentry, wentry, cnt;
> struct drm_exec exec;
> - int r, i, entry, cnt;
> u64 num_fences = 0;
>
> - num_bo_handles = wait_info->num_bo_handles;
> - bo_handles = memdup_user(u64_to_user_ptr(wait_info->bo_handles_array),
> - sizeof(u32) * num_bo_handles);
> - if (IS_ERR(bo_handles))
> - return PTR_ERR(bo_handles);
> + num_read_bo_handles = wait_info->num_read_bo_handles;
> + bo_handles_read = memdup_user(u64_to_user_ptr(wait_info->bo_handles_read_array),
> + sizeof(u32) * num_read_bo_handles);
> + if (IS_ERR(bo_handles_read))
> + return PTR_ERR(bo_handles_read);
> +
> + num_write_bo_handles = wait_info->num_write_bo_handles;
> + bo_handles_write = memdup_user(u64_to_user_ptr(wait_info->bo_handles_write_array),
> + sizeof(u32) * num_write_bo_handles);
> + if (IS_ERR(bo_handles_write))
> + goto free_bo_handles_read;
>
> num_syncobj = wait_info->num_syncobj_handles;
> syncobj_handles = memdup_user(u64_to_user_ptr(wait_info->syncobj_handles_array),
> sizeof(u32) * num_syncobj);
> if (IS_ERR(syncobj_handles)) {
> r = PTR_ERR(syncobj_handles);
> - goto free_bo_handles;
> + goto free_bo_handles_write;
> }
>
> num_points = wait_info->num_points;
> @@ -545,29 +594,51 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> goto free_timeline_handles;
> }
>
> - gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
> - if (!gobj) {
> + gobj_read = kmalloc_array(num_read_bo_handles, sizeof(*gobj_read), GFP_KERNEL);
> + if (!gobj_read) {
> r = -ENOMEM;
> goto free_timeline_points;
> }
>
> - for (entry = 0; entry < num_bo_handles; entry++) {
> - gobj[entry] = drm_gem_object_lookup(filp, bo_handles[entry]);
> - if (!gobj[entry]) {
> + for (rentry = 0; rentry < num_read_bo_handles; rentry++) {
> + gobj_read[rentry] = drm_gem_object_lookup(filp, bo_handles_read[rentry]);
> + if (!gobj_read[rentry]) {
> + r = -ENOENT;
> + goto put_gobj_read;
> + }
> + }
> +
> + gobj_write = kmalloc_array(num_write_bo_handles, sizeof(*gobj_write), GFP_KERNEL);
> + if (!gobj_write) {
> + r = -ENOMEM;
> + goto put_gobj_read;
> + }
> +
> + for (wentry = 0; wentry < num_write_bo_handles; wentry++) {
> + gobj_write[wentry] = drm_gem_object_lookup(filp, bo_handles_write[wentry]);
> + if (!gobj_write[wentry]) {
> r = -ENOENT;
> - goto put_gobj;
> + goto put_gobj_write;
> }
> }
>
> - drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT, num_bo_handles);
> + drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT,
> + (num_read_bo_handles + num_write_bo_handles));
>
> /* Lock all BOs with retry handling */
> drm_exec_until_all_locked(&exec) {
> - r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 0);
> + r = drm_exec_prepare_array(&exec, gobj_read, num_read_bo_handles, 0);
> + drm_exec_retry_on_contention(&exec);
> + if (r) {
> + drm_exec_fini(&exec);
> + goto put_gobj_write;
> + }
> +
> + r = drm_exec_prepare_array(&exec, gobj_write, num_write_bo_handles, 0);
> drm_exec_retry_on_contention(&exec);
> if (r) {
> drm_exec_fini(&exec);
> - goto put_gobj;
> + goto put_gobj_write;
> }
> }
>
> @@ -611,13 +682,21 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> }
>
> /* Count GEM objects fence */
> - for (i = 0; i < num_bo_handles; i++) {
> + for (i = 0; i < num_read_bo_handles; i++) {
> struct dma_resv_iter resv_cursor;
> struct dma_fence *fence;
>
> - dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
> - dma_resv_usage_rw(wait_info->bo_wait_flags &
> - AMDGPU_USERQ_BO_WRITE), fence)
> + dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
> + DMA_RESV_USAGE_READ, fence)
> + num_fences++;
> + }
> +
> + for (i = 0; i < num_write_bo_handles; i++) {
> + struct dma_resv_iter resv_cursor;
> + struct dma_fence *fence;
> +
> + dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
> + DMA_RESV_USAGE_WRITE, fence)
> num_fences++;
> }
>
> @@ -643,14 +722,30 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> goto free_fence_info;
> }
>
> - /* Retrieve GEM objects fence */
> - for (i = 0; i < num_bo_handles; i++) {
> + /* Retrieve GEM read objects fence */
> + for (i = 0; i < num_read_bo_handles; i++) {
> struct dma_resv_iter resv_cursor;
> struct dma_fence *fence;
>
> - dma_resv_for_each_fence(&resv_cursor, gobj[i]->resv,
> - dma_resv_usage_rw(wait_info->bo_wait_flags &
> - AMDGPU_USERQ_BO_WRITE), fence) {
> + dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
> + DMA_RESV_USAGE_READ, fence) {
> + if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
> + r = -EINVAL;
> + goto free_fences;
> + }
> +
> + fences[num_fences++] = fence;
> + dma_fence_get(fence);
> + }
> + }
> +
> + /* Retrieve GEM write objects fence */
> + for (i = 0; i < num_write_bo_handles; i++) {
> + struct dma_resv_iter resv_cursor;
> + struct dma_fence *fence;
> +
> + dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
> + DMA_RESV_USAGE_WRITE, fence) {
> if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
> r = -EINVAL;
> goto free_fences;
> @@ -771,14 +866,19 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> }
>
> drm_exec_fini(&exec);
> - for (i = 0; i < num_bo_handles; i++)
> - drm_gem_object_put(gobj[i]);
> - kfree(gobj);
> + for (i = 0; i < num_read_bo_handles; i++)
> + drm_gem_object_put(gobj_read[i]);
> + kfree(gobj_read);
> +
> + for (i = 0; i < num_write_bo_handles; i++)
> + drm_gem_object_put(gobj_write[i]);
> + kfree(gobj_write);
>
> kfree(timeline_points);
> kfree(timeline_handles);
> kfree(syncobj_handles);
> - kfree(bo_handles);
> + kfree(bo_handles_write);
> + kfree(bo_handles_read);
>
> return 0;
>
> @@ -790,18 +890,24 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
> kfree(fence_info);
> exec_fini:
> drm_exec_fini(&exec);
> -put_gobj:
> - while (entry-- > 0)
> - drm_gem_object_put(gobj[entry]);
> - kfree(gobj);
> +put_gobj_write:
> + while (wentry-- > 0)
> + drm_gem_object_put(gobj_write[wentry]);
> + kfree(gobj_write);
> +put_gobj_read:
> + while (rentry-- > 0)
> + drm_gem_object_put(gobj_read[rentry]);
> + kfree(gobj_read);
> free_timeline_points:
> kfree(timeline_points);
> free_timeline_handles:
> kfree(timeline_handles);
> free_syncobj_handles:
> kfree(syncobj_handles);
> -free_bo_handles:
> - kfree(bo_handles);
> +free_bo_handles_write:
> + kfree(bo_handles_write);
> +free_bo_handles_read:
> + kfree(bo_handles_read);
>
> return r;
> }
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index eeb345ddbf57..8d21e765094b 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -443,9 +443,6 @@ struct drm_amdgpu_userq_mqd_compute_gfx_v11 {
> __u64 eop_va;
> };
>
> -/* dma_resv usage flag */
> -#define AMDGPU_USERQ_BO_WRITE 1
> -
> /* userq signal/wait ioctl */
> struct drm_amdgpu_userq_signal {
> /**
> @@ -475,20 +472,32 @@ struct drm_amdgpu_userq_signal {
> */
> __u64 syncobj_point;
> /**
> - * @bo_handles_array: An array of GEM BO handles used by the userq fence creation
> - * IOCTL to install the created dma_fence object which can be utilized by
> + * @bo_handles_read_array: An array of GEM read BO handles used by the userq fence
> + * creation IOCTL to install the created dma_fence object which can be utilized by
> + * userspace to synchronize the BO usage between user processes.
> + */
> + __u64 bo_handles_read_array;
> + /**
> + * @bo_handles_write_array: An array of GEM write BO handles used by the userq fence
> + * creation IOCTL to install the created dma_fence object which can be utilized by
> * userspace to synchronize the BO usage between user processes.
> */
> - __u64 bo_handles_array;
> + __u64 bo_handles_write_array;
> + /**
> + * @num_read_bo_handles: A count that represents the number of GEM read BO handles in
> + * @bo_handles_read_array.
> + */
> + __u32 num_read_bo_handles;
> /**
> - * @num_bo_handles: A count that represents the number of GEM BO handles in
> - * @bo_handles_array.
> + * @num_write_bo_handles: A count that represents the number of GEM write BO handles in
> + * @bo_handles_write_array.
> */
> - __u32 num_bo_handles;
> + __u32 num_write_bo_handles;
> /**
> * @bo_flags: flags to indicate BOs synchronize for READ or WRITE
> */
> __u32 bo_flags;
> + __u32 pad;
> };
>
> struct drm_amdgpu_userq_fence_info {
> @@ -542,20 +551,31 @@ struct drm_amdgpu_userq_wait {
> */
> __u64 syncobj_timeline_points;
> /**
> - * @bo_handles_array: An array of GEM BO handles defined to fetch the fence
> + * @bo_handles_read_array: An array of GEM read BO handles defined to fetch the fence
> * wait information of every BO handles in the array.
> */
> - __u64 bo_handles_array;
> + __u64 bo_handles_read_array;
> + /**
> + * @bo_handles_write_array: An array of GEM write BO handles defined to fetch the fence
> + * wait information of every BO handles in the array.
> + */
> + __u64 bo_handles_write_array;
> /**
> * @num_syncobj_handles: A count that represents the number of syncobj handles in
> * @syncobj_handles_array.
> */
> __u32 num_syncobj_handles;
> /**
> - * @num_bo_handles: A count that represents the number of GEM BO handles in
> - * @bo_handles_array.
> + * @num_read_bo_handles: A count that represents the number of GEM BO handles in
> + * @bo_handles_read_array.
> + */
> + __u32 num_read_bo_handles;
> + /**
> + * @num_write_bo_handles: A count that represents the number of GEM BO handles in
> + * @bo_handles_write_array.
> */
> - __u32 num_bo_handles;
> + __u32 num_write_bo_handles;
> + __u32 pad;
> /**
> * @userq_fence_info: An array of fence information (va and value) pair of each
> * objects stored in @syncobj_handles_array and @bo_handles_array.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 06/09] drm/amdgpu: Few optimization and fixes for userq fence driver
2024-09-30 11:59 ` [PATCH v3 06/09] drm/amdgpu: Few optimization and fixes for userq fence driver Arunpravin Paneer Selvam
@ 2025-04-09 18:45 ` Kees Bakker
0 siblings, 0 replies; 14+ messages in thread
From: Kees Bakker @ 2025-04-09 18:45 UTC (permalink / raw)
To: Arunpravin Paneer Selvam, amd-gfx; +Cc: christian.koenig, alexander.deucher
Op 30-09-2024 om 13:59 schreef Arunpravin Paneer Selvam:
> Few optimization and fixes for userq fence driver.
>
> v1:(Christian):
> - Remove unnecessary comments.
> - In drm_exec_init call give num_bo_handles as last parameter it would
> making allocation of the array more efficient
> - Handle return value of __xa_store() and improve the error handling of
> amdgpu_userq_fence_driver_alloc().
>
> v2:(Christian):
> - Revert userq_xa xarray init to XA_FLAGS_LOCK_IRQ.
> - move the xa_unlock before the error check of the call xa_err(__xa_store())
> and moved this change to a separate patch as this is adding a missing error
> handling.
> - Removed the unnecessary comments.
>
> Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
> Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 44 ++++++++++++-------
> .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h | 2 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 6 +--
> .../gpu/drm/amd/include/amdgpu_userqueue.h | 2 +-
> 4 files changed, 32 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> index ca8f01b2bd96..56bd870ff15d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
> @@ -77,7 +77,8 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL);
> if (!fence_drv) {
> DRM_ERROR("Failed to allocate memory for fence driver\n");
> - return -ENOMEM;
> + r = -ENOMEM;
> + goto free_fence_drv;
> }
>
> /* Acquire seq64 memory */
> @@ -85,7 +86,8 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> &fence_drv->cpu_addr);
> if (r) {
> kfree(fence_drv);
> - return -ENOMEM;
> + r = -ENOMEM;
> + goto free_seq64;
Sorry to come back to this old patch.
Can I still ask you to take a closer look? The erroneous
code is still present in today's linux-next.
You've replaced a `return -ENOMEM` by setting `r` and
jumping to `free_seq64` where the freed pointer
is used again (use-after-free). And it is doing another
`kfree` with the same pointer.
> }
>
> memset(fence_drv->cpu_addr, 0, sizeof(u64));
> @@ -95,7 +97,7 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> spin_lock_init(&fence_drv->fence_list_lock);
>
> fence_drv->adev = adev;
> - fence_drv->uq_fence_drv_xa_ref = &userq->uq_fence_drv_xa;
> + fence_drv->fence_drv_xa_ptr = &userq->fence_drv_xa;
> fence_drv->context = dma_fence_context_alloc(1);
> get_task_comm(fence_drv->timeline_name, current);
>
> @@ -107,6 +109,13 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
> userq->fence_drv = fence_drv;
>
> return 0;
> +
> +free_seq64:
> + amdgpu_seq64_free(adev, fence_drv->gpu_addr);
> +free_fence_drv:
> + kfree(fence_drv);
> +
> + return r;
> }
>
> void amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv)
> [...]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2025-04-10 13:09 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-30 11:58 [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Arunpravin Paneer Selvam
2024-09-30 11:58 ` [PATCH v3 02/09] drm/amdgpu: screen freeze and userq driver crash Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 03/09] drm/amdgpu: Add wait IOCTL timeline syncobj support Arunpravin Paneer Selvam
2024-09-30 12:21 ` Christian König
2024-09-30 11:59 ` [PATCH v3 04/09] drm/amdgpu: Enable userq fence interrupt support Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 05/09] drm/amdgpu: Remove the MES self test Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 06/09] drm/amdgpu: Few optimization and fixes for userq fence driver Arunpravin Paneer Selvam
2025-04-09 18:45 ` Kees Bakker
2024-09-30 11:59 ` [PATCH v3 07/09] drm/amdgpu: Add the missing error handling for xa_store() call Arunpravin Paneer Selvam
2024-09-30 11:59 ` [PATCH v3 08/09] drm/amdgpu: add vm root BO lock before accessing the vm Arunpravin Paneer Selvam
2024-09-30 12:50 ` Christian König
2024-09-30 11:59 ` [PATCH v3 09/09] drm/amdgpu: Add separate array of read and write for BO handles Arunpravin Paneer Selvam
2024-09-30 12:54 ` Christian König
2024-09-30 12:09 ` [PATCH v3 01/09] drm/amdgpu: Implement userqueue signal/wait IOCTL Christian König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox