[PATCH v5 08/10] drm/amdgpu: generate doorbell index for userqueue

[Christian König]

Am 07.07.23 um 09:39 schrieb Shashank Sharma:
>
> On 07/07/2023 09:15, Christian König wrote:
>> Am 06.07.23 um 14:36 schrieb Shashank Sharma:
>>> The userspace sends us the doorbell object and the relative doobell
>>> index in the object to be used for the usermode queue, but the FW
>>> expects the absolute doorbell index on the PCI BAR in the MQD. This
>>> patch adds a function to convert this relative doorbell index to
>>> absolute doorbell index.
>>>
>>> This patch is dependent on the doorbell manager series:
>>> Link: https://patchwork.freedesktop.org/series/115802/
>>>
>>> V5: Fix the db object reference leak (Christian)
>>>
>>> Cc: Alex Deucher <alexander.deucher at amd.com>
>>> Cc: Christian Koenig <christian.koenig at amd.com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 34 
>>> +++++++++++++++++++
>>>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c        |  1 +
>>>   2 files changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>>> index bb774144c372..61064266c4f8 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>>> @@ -32,6 +32,31 @@ amdgpu_userqueue_find(struct amdgpu_userq_mgr 
>>> *uq_mgr, int qid)
>>>       return idr_find(&uq_mgr->userq_idr, qid);
>>>   }
>>>   +static uint64_t
>>> +amdgpu_userqueue_get_doorbell_index(struct amdgpu_userq_mgr *uq_mgr,
>>> +                     struct amdgpu_usermode_queue *queue,
>>> +                     struct drm_file *filp,
>>> +                     uint32_t doorbell_offset)
>>> +{
>>> +    struct drm_gem_object *gobj;
>>> +    struct amdgpu_bo *db_bo;
>>> +    uint64_t index;
>>> +
>>> +    gobj = drm_gem_object_lookup(filp, queue->doorbell_handle);
>>> +    if (gobj == NULL) {
>>> +        DRM_ERROR("Can't find GEM object for doorbell\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    db_bo = amdgpu_bo_ref(gem_to_amdgpu_bo(gobj));
>>> +    drm_gem_object_put(gobj);
>>> +
>>> +    index = amdgpu_doorbell_index_on_bar(uq_mgr->adev, db_bo, 
>>> doorbell_offset);
>>
>> This can only be done with the doorbell BO locked and as soon as you 
>> unlock it the value becomes invalid unless you pin the BO.
>>
> Which means I need to use create_bo_kernel() for doorbell BO's or 
> specifically pin it while creating it ?

For now I think you need to pin it when amdgpu_userqueue_create() is 
called and unpin it when the userqueue is destroyed again.

It's probably a good idea to not use amdgpu_bo_create_kernel() for the 
MQD and context BO either, but rather explicitly pin it during queue 
create as well.

Christian.

>
> - Shashank
>
>> Regards,
>> Christian.
>>
>>> +    amdgpu_bo_unref(&db_bo);
>>> +    DRM_DEBUG_DRIVER("[Usermode queues] doorbell index=%lld\n", 
>>> index);
>>> +    return index;
>>> +}
>>> +
>>>   static int
>>>   amdgpu_userqueue_destroy(struct drm_file *filp, int queue_id)
>>>   {
>>> @@ -64,6 +89,7 @@ amdgpu_userqueue_create(struct drm_file *filp, 
>>> union drm_amdgpu_userq *args)
>>>       struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
>>>       const struct amdgpu_userq_funcs *uq_funcs;
>>>       struct amdgpu_usermode_queue *queue;
>>> +    uint64_t index;
>>>       int qid, r = 0;
>>>         mutex_lock(&uq_mgr->userq_mutex);
>>> @@ -87,6 +113,14 @@ amdgpu_userqueue_create(struct drm_file *filp, 
>>> union drm_amdgpu_userq *args)
>>>       queue->flags = args->in.flags;
>>>       queue->vm = &fpriv->vm;
>>>   +    /* Convert relative doorbell offset into absolute doorbell 
>>> index */
>>> +    index = amdgpu_userqueue_get_doorbell_index(uq_mgr, queue, 
>>> filp, args->in.doorbell_offset);
>>> +    if (index == (uint64_t)-EINVAL) {
>>> +        DRM_ERROR("Failed to get doorbell for queue\n");
>>> +        goto unlock;
>>> +    }
>>> +    queue->doorbell_index = index;
>>> +
>>>       r = uq_funcs->mqd_create(uq_mgr, &args->in, queue);
>>>       if (r) {
>>>           DRM_ERROR("Failed to create Queue\n");
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>>> index afaeecb9940a..8edb020683a1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>>> @@ -6719,6 +6719,7 @@ static int gfx_v11_0_userq_mqd_create(struct 
>>> amdgpu_userq_mgr *uq_mgr,
>>>       userq_props.queue_size = mqd_user.queue_size;
>>>       userq_props.hqd_base_gpu_addr = mqd_user.queue_va;
>>>       userq_props.mqd_gpu_addr = queue->mqd.gpu_addr;
>>> +    userq_props.doorbell_index = queue->doorbell_index;
>>>       userq_props.use_doorbell = true;
>>>         r = mqd_gfx_generic->init_mqd(adev, (void 
>>> *)queue->mqd.cpu_ptr, &userq_props);
>>