[drm/amdgpu]: user queue doorbell allocation for IP reqs
Saleemkhan Jamadar
sjamadar at amd.com
Fri Jan 3 06:34:35 UTC 2025
Hi Shashank,
Replied inline [Saleem]
Regards,
Salem
On 02/01/25 18:58, Sharma, Shashank wrote:
> + (amd-gfx)
>
> On 01/01/2025 07:03, Saleemkhan Jamadar wrote:
>> #resending patch
>>
>> From 79cd62f882197505dbf9c489ead2b0bcab98209f Mon Sep 17 00:00:00 2001
>> From: Saleemkhan Jamadar <saleemkhan.jamadar at amd.com>
>> Date: Wed, 18 Dec 2024 19:30:22 +0530
>> Subject: [PATCH] drm/amdgpu: user queue doorbell allocation for IP reqs
>>
>> Currenlty doorbell allocation handles only 64 bit db size.
>>
>> In case of VCN we need to allocated AGDB and per instance
>> doorbell.VCN/UMSCH doorbell size is 32 bit and offset
>> calculated is from specific range from the allocated page.
>>
>> With these changes individual IP can provide specific reqs
>> for db allocation.
>>
>> Signed-off-by: Saleemkhan Jamadar <saleemkhan.jamadar at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 41 ++++++++++++++-----
>> .../gpu/drm/amd/include/amdgpu_userqueue.h | 12 ++++++
>> 2 files changed, 42 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> index cba51bdf2e2c..4fff15e0d838 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c
>> @@ -163,18 +163,17 @@ void amdgpu_userqueue_destroy_object(struct
>> amdgpu_userq_mgr *uq_mgr,
>> amdgpu_bo_unref(&userq_obj->obj);
>> }
>> -static uint64_t
>> +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 amdgpu_db_info *db_info,
>> + struct drm_file *filp)
>> {
>> uint64_t index;
>> struct drm_gem_object *gobj;
>> - struct amdgpu_userq_obj *db_obj = &queue->db_obj;
>> - int r;
>> + struct amdgpu_userq_obj *db_obj = db_info->db_obj;
>> + int r, db_size;
>> - gobj = drm_gem_object_lookup(filp, queue->doorbell_handle);
>> + gobj = drm_gem_object_lookup(filp, db_info->doorbell_handle);
>> if (gobj == NULL) {
>> DRM_ERROR("Can't find GEM object for doorbell\n");
>> return -EINVAL;
>> @@ -196,8 +195,23 @@ amdgpu_userqueue_get_doorbell_index(struct
>> amdgpu_userq_mgr *uq_mgr,
>> goto unpin_bo;
>> }
>> + switch (db_info->queue_type) {
>> + case AMDGPU_HW_IP_GFX:
>> + case AMDGPU_HW_IP_COMPUTE:
>> + case AMDGPU_HW_IP_DMA:
>> + db_size = sizeof(u64);
>> + break;
>> + case AMDGPU_HW_IP_VCN_ENC:
>> + db_size = sizeof(u32);
>> + db_info->doorbell_offset += AMDGPU_NAVI10_DOORBELL64_VCN0_1
>> << 1;
>> + break;
>> + default:
>> + DRM_WARN("User queues not supported for IP (%d )\n",
>> db_info->queue_type);
>> + return -EINVAL;
>> + }
>> +
>> index = amdgpu_doorbell_index_on_bar(uq_mgr->adev, db_obj->obj,
>> - doorbell_offset, sizeof(u64));
>> + db_info->doorbell_offset, db_size);
>> DRM_DEBUG_DRIVER("[Usermode queues] doorbell index=%lld\n",
>> index);
>> amdgpu_bo_unreserve(db_obj->obj);
>> return index;
>> @@ -242,6 +256,7 @@ amdgpu_userqueue_create(struct drm_file *filp,
>> union drm_amdgpu_userq *args)
>> struct amdgpu_device *adev = uq_mgr->adev;
>> const struct amdgpu_userq_funcs *uq_funcs;
>> struct amdgpu_usermode_queue *queue;
>> + struct amdgpu_db_info db_info;
>> uint64_t index;
>> int qid, r = 0;
>> @@ -269,19 +284,23 @@ amdgpu_userqueue_create(struct drm_file
>> *filp, union drm_amdgpu_userq *args)
>> goto unlock;
>> }
>> queue->doorbell_handle = args->in.doorbell_handle;
>> - queue->doorbell_index = args->in.doorbell_offset;
> Nack, this is going to break mapping/unmapping for GFX UQ. Nothing
> should be removed from the queue structure, as all of it is accounted
> for.
[Saleem]:This assignment is not used anywhere, moreover it gets
overwritten with mapped doorbell index below.
>> queue->queue_type = args->in.ip_type;
>> queue->vm = &fpriv->vm;
>> + db_info.queue_type = queue->queue_type;
>> + db_info.doorbell_handle = queue->doorbell_handle;
>> + db_info.db_obj = &queue->db_obj;
>> + db_info.doorbell_offset = args->in.doorbell_offset;
>> +
> I can see that all the information you are adding in struct db_info is
> coming from queue only, then why to add a new structure here at all ?
> Instead, you can just do this in function
> [Saleem]: In previous impl you can see that db_obj is fetched form
> queue structure. In case of vcn we have mulpitle doorbell to be
> allocated, to accomodate this and to avoid the another level of check
> to identify which doorbell is being allocated this db_info (structure
> used locally) helps. db_info shares data for which allocation needs to
> done.
>
> amdgpu_userqueue_get_doorbell_index():
>
> db_size = sizeof(u62);
>
> switch(queue->queue_type) {
> case VCN:
> queue->doorbell_offset += AMDGPU_NAVI10_DOORBELL64_VCN0_1 << 1;
> db_size = sizeof(u32);
> break;
> }
> [Saleem]: This change I can make.
> <same doorbell offset calculation as usual>
>
> - Shashank
>
>> /* Convert relative doorbell offset into absolute doorbell
>> index */
>> - index = amdgpu_userqueue_get_doorbell_index(uq_mgr, queue, filp,
>> args->in.doorbell_offset);
>> + index = amdgpu_userqueue_get_doorbell_index(uq_mgr, &db_info,
>> filp);
>> if (index == (uint64_t)-EINVAL) {
>> DRM_ERROR("Failed to get doorbell for queue\n");
>> kfree(queue);
>> goto unlock;
>> }
>> - queue->doorbell_index = index;
>> + queue->doorbell_index = index;
>> xa_init_flags(&queue->fence_drv_xa, XA_FLAGS_ALLOC);
>> r = amdgpu_userq_fence_driver_alloc(adev, queue);
>> if (r) {
>> diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> index 2bf28f3454cb..3d54601c6a24 100644
>> --- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> +++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> @@ -39,6 +39,13 @@ struct amdgpu_userq_obj {
>> struct amdgpu_bo *obj;
>> };
>> +struct amdgpu_db_info {
>> + uint64_t doorbell_handle;
>> + uint32_t queue_type;
>> + uint32_t doorbell_offset;
>> + struct amdgpu_userq_obj *db_obj;
>> +};
>> +
>> struct amdgpu_usermode_queue {
>> int queue_type;
>> uint8_t queue_active;
>> @@ -93,4 +100,9 @@ void amdgpu_userqueue_destroy_object(struct
>> amdgpu_userq_mgr *uq_mgr,
>> void amdgpu_userqueue_suspend(struct amdgpu_userq_mgr *uq_mgr);
>> int amdgpu_userqueue_active(struct amdgpu_userq_mgr *uq_mgr);
>> +
>> +uint64_t amdgpu_userqueue_get_doorbell_index(struct amdgpu_userq_mgr
>> *uq_mgr,
>> + struct amdgpu_db_info *db_info,
>> + struct drm_file *filp);
>> +
>> #endif
More information about the amd-gfx
mailing list