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

[Shashank Sharma]

On 07/07/2023 09:57, Christian König wrote:
> 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.


Noted, will do that.

- Shashank

>
> 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);
>>>
>