[PATCH] drm/amdgpu: enable userqueue support for GFX12
Sharma, Shashank
Shashank.Sharma at amd.com
Tue Oct 22 16:39:11 UTC 2024
[AMD Official Use Only - AMD Internal Distribution Only]
Thanks Alex,
I have created an integration branch for GFX12, and cleaned up and ported these patches. We will finish testing and start the code review of these patches soon.
Regards
Shashank
________________________________
From: Alex Deucher <alexdeucher at gmail.com>
Sent: Friday, October 18, 2024 9:18 PM
To: Sharma, Shashank <Shashank.Sharma at amd.com>
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>; Somalapuram, Amaranath <Amaranath.Somalapuram at amd.com>; Koenig, Christian <Christian.Koenig at amd.com>; Yadav, Arvind <Arvind.Yadav at amd.com>
Subject: Re: [PATCH] drm/amdgpu: enable userqueue support for GFX12
On Tue, Oct 15, 2024 at 12:37 PM Sharma, Shashank
<shashank.sharma at amd.com> wrote:
>
>
> 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.
A took a longer look at the changes and it turns out I could move all
of the IP specific stuff into the IP specific code. I hacked together
the attached untested patches to clean this up.
Alex
>
> - 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20241022/85288bcb/attachment-0001.htm>
More information about the amd-gfx
mailing list