Re: [PATCH] drm/amdgpu: enable userqueue support for GFX12

["Sharma, Shashank" <shashank.sharma@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 4301 bytes --]

Hello Alex,

On 14/10/2024 22:29, Deucher, Alexander wrote:
> [AMD Official Use Only - AMD Internal Distribution Only]
>
>> -----Original Message-----
>> From: Sharma, Shashank<Shashank.Sharma@amd.com>
>> Sent: Thursday, October 10, 2024 2:08 PM
>> To:amd-gfx@lists.freedesktop.org
>> Cc: Somalapuram, Amaranath<Amaranath.Somalapuram@amd.com>; Deucher,
>> Alexander<Alexander.Deucher@amd.com>; Koenig, Christian
>> <Christian.Koenig@amd.com>; Yadav, Arvind<Arvind.Yadav@amd.com>; Sharma,
>> Shashank<Shashank.Sharma@amd.com>
>> Subject: [PATCH] drm/amdgpu: enable userqueue support for GFX12
>>
>> From: Somalapuram Amaranath<Amaranath.Somalapuram@amd.com>
>>
>> This patch enables Usermode queue support across GFX, Compute and SDMA IPs
>> on GFX12/SDMA7. It typically reuses Navi3X userqueue IP functions to create and
>> destroy MQDs.
> I would like to make this more explicit.  In mes_v11_0_userqueue.c, I would suggest splitting out any non-gfx11 specific code into some new helpers in mes_userqueue.c.  E.g.,
>
> mes_v11_0_map_gtt_bo_to_gart() -> mes_userq_map_gtt_bo_to_gart()
> mes_v11_0_create_wptr_mapping() -> mes_userq_create_wptr_mapping()
> mes_v11_0_userq_map() -> mes_userq_map()
> mes_v11_0_userq_unmap() -> mes_userq_unmap()
> mes_v11_0_userq_mqd_destroy() -> mes_userq_mqd_destroy()

Initial patch sets had all these functions named similar to 'mes_userq_* 
' only, but later you recommended that we should have mention of _v11_0 
to indicate the IP specific stuff, as there might be IP specific way of 
mapping/unmapping/creating and destroying the queues. So with this 
comment we might be going back to the same version.

As of now, v12 UQ was tested using the the same methods as V11 UQs, and 
found it functional. We might need more information before taking this 
step.

- Shashank

>
> However, mes_v11_userq_create_ctx_space() uses the v11 mqd structures which may not match the v12 structures.  We should add a v12 implementation for any functions that use the v12 structures.
> Alex
>
>> Cc: Alex Deucher<alexander.deucher@amd.com>
>> Cc: Christian Koenig<christian.koenig@amd.com>
>> Cc: Arvind Yadav<arvind.yadav@amd.com>
>> Signed-off-by: Somalapuram Amaranath<Amaranath.Somalapuram@amd.com>
>> Signed-off-by: Shashank Sharma<shashank.sharma@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 5 +++++
>> drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 6 ++++++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
>> index 9fec28d8a5fc..d511996c374d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
>> @@ -46,6 +46,7 @@
>>   #include "gfx_v12_0.h"
>>   #include "nbif_v6_3_1.h"
>>   #include "mes_v12_0.h"
>> +#include "mes_v11_0_userqueue.h"
>>
>>   #define GFX12_NUM_GFX_RINGS  1
>>   #define GFX12_MEC_HPD_SIZE   2048
>> @@ -1335,6 +1336,10 @@ static int gfx_v12_0_sw_init(struct amdgpu_ip_block
>> *ip_block)
>>                adev->gfx.mec.num_mec = 2;
>>                adev->gfx.mec.num_pipe_per_mec = 2;
>>                adev->gfx.mec.num_queue_per_pipe = 4;
>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>> +             adev->userq_funcs[AMDGPU_HW_IP_GFX] =
>> &userq_mes_v11_0_funcs;
>> +             adev->userq_funcs[AMDGPU_HW_IP_COMPUTE] =
>> &userq_mes_v11_0_funcs;
>> +#endif
>>                break;
>>        default:
>>                adev->gfx.me.num_me = 1;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
>> b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
>> index 24f24974ac1d..badcf38bb8b6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
>> @@ -42,6 +42,7 @@
>>   #include "sdma_common.h"
>>   #include "sdma_v7_0.h"
>>   #include "v12_structs.h"
>> +#include "mes_v11_0_userqueue.h"
>>
>>   MODULE_FIRMWARE("amdgpu/sdma_7_0_0.bin");
>>   MODULE_FIRMWARE("amdgpu/sdma_7_0_1.bin");
>> @@ -1317,6 +1318,11 @@ static int sdma_v7_0_sw_init(struct amdgpu_ip_block
>> *ip_block)
>>        else
>>                DRM_ERROR("Failed to allocated memory for SDMA IP Dump\n");
>>
>> +#ifdef CONFIG_DRM_AMDGPU_NAVI3X_USERQ
>> +     adev->userq_funcs[AMDGPU_HW_IP_DMA] = &userq_mes_v11_0_funcs;
>> #endif
>> +
>> +
>>        return r;
>>   }
>>
>> --
>> 2.46.2

[-- Attachment #2: Type: text/html, Size: 6460 bytes --]