On 5/19/26 13:18, Sunil Khatri wrote:
> Add per queue array to store userq vas and keep
> size to accommodate vas of all types of queues
> i.e gfx, compute and sdma.

Yeah that's a start, but I think we should go a bit further.

> 
> Signed-off-by: Sunil Khatri <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 36 ++++++++---------------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |  4 ++-
>  2 files changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 7354c51ae83d..9ac7f18c903f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -218,18 +218,11 @@ void amdgpu_userq_process_fence_irq(struct 
> amdgpu_device *adev, u32 doorbell)
>  static int amdgpu_userq_buffer_va_list_add(struct amdgpu_usermode_queue 
> *queue,
>                                          struct amdgpu_bo_va_mapping *va_map, 
> u64 addr)
>  {

Completely nuke that function and it's caller.

> -     struct amdgpu_userq_va_cursor *va_cursor;
> -     struct userq_va_list;
> -
> -     va_cursor = kzalloc(sizeof(*va_cursor), GFP_KERNEL);
> -     if (!va_cursor)
> +     if (queue->userq_va_count >= ARRAY_SIZE(queue->userq_va))
>               return -ENOMEM;
>  
> -     INIT_LIST_HEAD(&va_cursor->list);
> -     va_cursor->gpu_addr = addr;
> +     queue->userq_va[queue->userq_va_count++] = addr;
>       va_map->bo_va->userq_va_mapped = true;
> -     list_add(&va_cursor->list, &queue->userq_va_list);
> -
>       return 0;
>  }
>  
> @@ -284,14 +277,13 @@ static bool amdgpu_userq_buffer_va_mapped(struct 
> amdgpu_vm *vm, u64 addr)
>  
>  static bool amdgpu_userq_buffer_vas_mapped(struct amdgpu_usermode_queue 
> *queue)
>  {
> -     struct amdgpu_userq_va_cursor *va_cursor, *tmp;
> -     int r = 0;
> +     int i, r = 0;
>  
> -     list_for_each_entry_safe(va_cursor, tmp, &queue->userq_va_list, list) {
> -             r += amdgpu_userq_buffer_va_mapped(queue->vm, 
> va_cursor->gpu_addr);
> +     for (i = 0; i < queue->userq_va_count; i++) {
> +             r += amdgpu_userq_buffer_va_mapped(queue->vm, 
> queue->userq_va[i]);
>               dev_dbg(queue->userq_mgr->adev->dev,
>                       "validate the userq mapping:%p va:%llx r:%d\n",
> -                     queue, va_cursor->gpu_addr, r);
> +                     queue, queue->userq_va[i], r);
>       }
>  
>       if (r != 0)
> @@ -303,19 +295,19 @@ static bool amdgpu_userq_buffer_vas_mapped(struct 
> amdgpu_usermode_queue *queue)
>  static void amdgpu_userq_buffer_vas_list_cleanup(struct amdgpu_device *adev,
>                                                struct amdgpu_usermode_queue 
> *queue)

Drop that whole function.

>  {
> -     struct amdgpu_userq_va_cursor *va_cursor, *tmp;
>       struct amdgpu_bo_va_mapping *mapping;
> +     int i;
>  
>       /* Caller must hold vm->root.bo reservation */
>       dma_resv_assert_held(queue->vm->root.bo->tbo.base.resv);
>  
> -     list_for_each_entry_safe(va_cursor, tmp, &queue->userq_va_list, list) {
> -             mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, 
> va_cursor->gpu_addr);
> -             if (mapping)
> +     for (i = 0; i < queue->userq_va_count; i++) {
> +             mapping = amdgpu_vm_bo_lookup_mapping(queue->vm, 
> queue->userq_va[i]);
> +             if (mapping) {
> +                     mapping->bo_va->userq_va_mapped = false;
>                       dev_dbg(adev->dev, "delete the userq:%p va:%llx\n",
> -                             queue, va_cursor->gpu_addr);
> -             list_del(&va_cursor->list);
> -             kfree(va_cursor);
> +                             queue, queue->userq_va[i]);
> +                     }
>       }
>  }
>  
> @@ -633,7 +625,6 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr, 
> struct amdgpu_usermode_que
>       amdgpu_bo_reserve(vm->root.bo, true);
>       amdgpu_userq_buffer_vas_list_cleanup(adev, queue);
>       amdgpu_bo_unreserve(vm->root.bo);
> -     list_del(&queue->userq_va_list);
>       queue->userq_mgr = NULL;
>  
>       amdgpu_bo_reserve(queue->db_obj.obj, true);
> @@ -738,7 +729,6 @@ amdgpu_userq_create(struct drm_file *filp, union 
> drm_amdgpu_userq *args)
>       }
>  
>       kref_init(&queue->refcount);
> -     INIT_LIST_HEAD(&queue->userq_va_list);
>       queue->doorbell_handle = args->in.doorbell_handle;
>       queue->queue_type = args->in.ip_type;
>       queue->vm = &fpriv->vm;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> index 033b8a0de6b1..fdf4d878c894 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> @@ -93,7 +93,9 @@ struct amdgpu_usermode_queue {
>       struct delayed_work     hang_detect_work;
>       struct kref             refcount;
>  
> -     struct list_head        userq_va_list;
> +     /* User to store core bo's va addresses */
> +     u64                     userq_va[5];
> +     int                     userq_va_count;

Make that look like this:

union {
        struct {
                u64 queue_rb;
                u64 wptr;
                u64 rptr;
                ....
        } va_names;
        u64 va_array[];
};

Unused entries should simply be zero.

After amdgpu_userq_create() fills in the different VAs we just call 
amdgpu_userq_buffer_vas_mapped() to double check that they are valid before 
mapping the queue.

Regards,
Christian.

>  };
>  
>  struct amdgpu_userq_funcs {

Reply via email to