Add few optimizations to 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(). Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@xxxxxxx> --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +- .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 64 ++++++++++++------- .../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 +- 5 files changed, 48 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 5ec6cb237c81..3c4568929d12 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -3967,7 +3967,7 @@ 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); + xa_init_flags(&adev->userq_xa, XA_FLAGS_ALLOC); 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 c6d201abf5ec..a30b8abe8a2f 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c @@ -76,7 +76,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 */ @@ -84,7 +85,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)); @@ -94,18 +96,30 @@ 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); 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)); + if (r) + goto xa_err; + xa_unlock_irqrestore(&adev->userq_xa, flags); userq->fence_drv = fence_drv; return 0; + +xa_err: + xa_unlock_irqrestore(&adev->userq_xa, flags); +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) @@ -149,7 +163,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); @@ -166,11 +180,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); @@ -214,12 +228,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 = @@ -229,9 +243,9 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq, userq_fence->fence_drv_array_count = count; 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++; } } @@ -377,14 +391,12 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data, int r, i, entry; 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 syncobj object handles */ syncobj = kmalloc_array(num_syncobj_handles, sizeof(*syncobj), GFP_KERNEL); if (!syncobj) { r = -ENOMEM; @@ -399,14 +411,12 @@ 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); if (IS_ERR(bo_handles)) goto put_syncobj_handles; - /* Array of GEM object handles */ gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL); if (!gobj) { r = -ENOMEM; @@ -421,7 +431,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); @@ -526,7 +538,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; @@ -541,7 +552,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); @@ -703,9 +716,16 @@ 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, - xa_limit_32b, GFP_KERNEL); + if (fence_drv->fence_drv_xa_ptr) { + /* + * Store the fence_drv reference into the corresponding + * userq fence_drv_xa. + */ + 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