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

[Sharma, Shashank]

On 15/10/2024 16:58, Alex Deucher wrote:
> On Tue, Oct 15, 2024 at 6:13 AM Sharma, Shashank
> <shashank.sharma at amd.com> wrote:
>> 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 at amd.com>
>> Sent: Thursday, October 10, 2024 2:08 PM
>> To: amd-gfx at lists.freedesktop.org
>> Cc: Somalapuram, Amaranath <Amaranath.Somalapuram at amd.com>; Deucher,
>> Alexander <Alexander.Deucher at amd.com>; Koenig, Christian
>> <Christian.Koenig at amd.com>; Yadav, Arvind <Arvind.Yadav at amd.com>; Sharma,
>> Shashank <Shashank.Sharma at amd.com>
>> Subject: [PATCH] drm/amdgpu: enable userqueue support for GFX12
>>
>> From: Somalapuram Amaranath <Amaranath.Somalapuram at 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.
> Well that was before gfx12 support was on our radar.  Generally, you
> develop for the first generation and if there are things that you can
> pull out into common code and share across generations, then you
> should do that when you add support for the next generation.
Noted,
>> 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.
> You would need to verify that the V11 and V12 MQDs are the same.
> EIther way, I would recommend making v11 and v12 variants the the
> functions which populate the MQDs that the firmware uses.  There is
> alays a chance that the firmware may repurpose some of the fields for
> different things that can lead to subtle bugs.  At the end of the day,
> I think we'll end up with a bunch of helpers in mes_userqueue.c and
> then a function or two in mes_v11_0_userqueue.c and
> mes_v12_0_userqueue.c.  Alternatively, you could just put the helpers
> in amdgpu_mes.c and the gfx11 and gfx12 specific functions in
> mes_v11_0.c and mes_v12_0.c since it will probably only be a function
> or two.  You could even add a callback for the MQD specific changes
> and add a wrapper like the other functions in amdgpu_mes.c and the
> generic functions in mes_v11_0_userqueue.c.  That would make
> everything symmetric for mes managed queues.
>
> Alex

Thanks, Noted, I would do the recommended changes.

- Shashank

>> - 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 at amd.com>
>> Cc: Christian Koenig <christian.koenig at amd.com>
>> Cc: Arvind Yadav <arvind.yadav at amd.com>
>> Signed-off-by: Somalapuram Amaranath <Amaranath.Somalapuram at amd.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma at 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