drm/amdgpu: Few optimization and fixes for userq fence driver

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>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
This commit is contained in:
Arunpravin Paneer Selvam 2024-10-30 10:51:58 +05:30 committed by Alex Deucher
parent ac4a1f7f13
commit e7cf21fbb2
4 changed files with 32 additions and 22 deletions

View File

@ -77,7 +77,8 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL); fence_drv = kzalloc(sizeof(*fence_drv), GFP_KERNEL);
if (!fence_drv) { if (!fence_drv) {
DRM_ERROR("Failed to allocate memory for fence driver\n"); DRM_ERROR("Failed to allocate memory for fence driver\n");
return -ENOMEM; r = -ENOMEM;
goto free_fence_drv;
} }
/* Acquire seq64 memory */ /* Acquire seq64 memory */
@ -85,7 +86,8 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
&fence_drv->cpu_addr); &fence_drv->cpu_addr);
if (r) { if (r) {
kfree(fence_drv); kfree(fence_drv);
return -ENOMEM; r = -ENOMEM;
goto free_seq64;
} }
memset(fence_drv->cpu_addr, 0, sizeof(u64)); 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); spin_lock_init(&fence_drv->fence_list_lock);
fence_drv->adev = adev; 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); fence_drv->context = dma_fence_context_alloc(1);
get_task_comm(fence_drv->timeline_name, current); 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; userq->fence_drv = fence_drv;
return 0; 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) 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_device *adev = fence_drv->adev;
struct amdgpu_userq_fence *fence, *tmp; struct amdgpu_userq_fence *fence, *tmp;
struct xarray *xa = &adev->userq_xa; struct xarray *xa = &adev->userq_xa;
unsigned long index; unsigned long index, flags;
struct dma_fence *f; struct dma_fence *f;
spin_lock(&fence_drv->fence_list_lock); 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); spin_unlock(&fence_drv->fence_list_lock);
xa_lock(xa); xa_lock_irqsave(xa, flags);
xa_for_each(xa, index, xa_fence_drv) xa_for_each(xa, index, xa_fence_drv)
if (xa_fence_drv == fence_drv) if (xa_fence_drv == fence_drv)
__xa_erase(xa, index); __xa_erase(xa, index);
xa_unlock(xa); xa_unlock_irqrestore(xa, flags);
/* Free seq64 memory */ /* Free seq64 memory */
amdgpu_seq64_free(adev, fence_drv->gpu_addr); 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); amdgpu_userq_fence_driver_get(fence_drv);
dma_fence_get(fence); 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; struct amdgpu_userq_fence_driver *stored_fence_drv;
unsigned long index, count = 0; unsigned long index, count = 0;
int i = 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++; count++;
userq_fence->fence_drv_array = userq_fence->fence_drv_array =
@ -227,9 +236,9 @@ int amdgpu_userq_fence_create(struct amdgpu_usermode_queue *userq,
GFP_KERNEL); GFP_KERNEL);
if (userq_fence->fence_drv_array) { 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; 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++; i++;
} }
} }
@ -379,7 +388,6 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
struct drm_exec exec; struct drm_exec exec;
u64 wptr; u64 wptr;
/* Array of syncobj handles */
num_syncobj_handles = args->num_syncobj_handles; num_syncobj_handles = args->num_syncobj_handles;
syncobj_handles = memdup_user(u64_to_user_ptr(args->syncobj_handles_array), syncobj_handles = memdup_user(u64_to_user_ptr(args->syncobj_handles_array),
sizeof(u32) * num_syncobj_handles); 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; num_bo_handles = args->num_bo_handles;
bo_handles = memdup_user(u64_to_user_ptr(args->bo_handles_array), bo_handles = memdup_user(u64_to_user_ptr(args->bo_handles_array),
sizeof(u32) * num_bo_handles); 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) { drm_exec_until_all_locked(&exec) {
r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 1); r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 1);
drm_exec_retry_on_contention(&exec); drm_exec_retry_on_contention(&exec);
@ -520,7 +529,6 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
goto free_timeline_handles; goto free_timeline_handles;
} }
/* Array of GEM object handles */
gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL); gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
if (!gobj) { if (!gobj) {
r = -ENOMEM; r = -ENOMEM;
@ -535,7 +543,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) { drm_exec_until_all_locked(&exec) {
r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 0); r = drm_exec_prepare_array(&exec, gobj, num_bo_handles, 0);
drm_exec_retry_on_contention(&exec); drm_exec_retry_on_contention(&exec);
@ -706,8 +716,8 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
* Otherwise, we would gather those references until we don't * Otherwise, we would gather those references until we don't
* have any more space left and crash. * have any more space left and crash.
*/ */
if (fence_drv->uq_fence_drv_xa_ref) { if (fence_drv->fence_drv_xa_ptr) {
r = xa_alloc(fence_drv->uq_fence_drv_xa_ref, &index, fence_drv, r = xa_alloc(fence_drv->fence_drv_xa_ptr, &index, fence_drv,
xa_limit_32b, GFP_KERNEL); xa_limit_32b, GFP_KERNEL);
if (r) if (r)
goto free_fences; goto free_fences;

View File

@ -54,7 +54,7 @@ struct amdgpu_userq_fence_driver {
spinlock_t fence_list_lock; spinlock_t fence_list_lock;
struct list_head fences; struct list_head fences;
struct amdgpu_device *adev; struct amdgpu_device *adev;
struct xarray *uq_fence_drv_xa_ref; struct xarray *fence_drv_xa_ptr;
char timeline_name[TASK_COMM_LEN]; char timeline_name[TASK_COMM_LEN];
}; };

View File

@ -47,8 +47,8 @@ static void amdgpu_userq_walk_and_drop_fence_drv(struct xarray *xa)
static void static void
amdgpu_userq_fence_driver_free(struct amdgpu_usermode_queue *userq) amdgpu_userq_fence_driver_free(struct amdgpu_usermode_queue *userq)
{ {
amdgpu_userq_walk_and_drop_fence_drv(&userq->uq_fence_drv_xa); amdgpu_userq_walk_and_drop_fence_drv(&userq->fence_drv_xa);
xa_destroy(&userq->uq_fence_drv_xa); xa_destroy(&userq->fence_drv_xa);
/* Drop the fence_drv reference held by user queue */ /* Drop the fence_drv reference held by user queue */
amdgpu_userq_fence_driver_put(userq->fence_drv); amdgpu_userq_fence_driver_put(userq->fence_drv);
} }
@ -260,7 +260,7 @@ amdgpu_userqueue_create(struct drm_file *filp, union drm_amdgpu_userq *args)
} }
queue->doorbell_index = index; 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); r = amdgpu_userq_fence_driver_alloc(adev, queue);
if (r) { if (r) {
DRM_ERROR("Failed to alloc fence driver\n"); DRM_ERROR("Failed to alloc fence driver\n");

View File

@ -47,7 +47,7 @@ struct amdgpu_usermode_queue {
struct amdgpu_userq_obj db_obj; struct amdgpu_userq_obj db_obj;
struct amdgpu_userq_obj fw_obj; struct amdgpu_userq_obj fw_obj;
struct amdgpu_userq_obj wptr_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 amdgpu_userq_fence_driver *fence_drv;
}; };