* [PATCH] drm/amdgpu: enable userqueue support for GFX12 @ 2024-10-10 18:08 Shashank Sharma 2024-10-14 20:29 ` Deucher, Alexander 0 siblings, 1 reply; 7+ messages in thread From: Shashank Sharma @ 2024-10-10 18:08 UTC (permalink / raw) To: amd-gfx Cc: Somalapuram Amaranath, Alex Deucher, Christian Koenig, Arvind Yadav, Shashank Sharma 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. 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH] drm/amdgpu: enable userqueue support for GFX12 2024-10-10 18:08 [PATCH] drm/amdgpu: enable userqueue support for GFX12 Shashank Sharma @ 2024-10-14 20:29 ` Deucher, Alexander 2024-10-15 9:59 ` Sharma, Shashank 0 siblings, 1 reply; 7+ messages in thread From: Deucher, Alexander @ 2024-10-14 20:29 UTC (permalink / raw) To: Sharma, Shashank, amd-gfx@lists.freedesktop.org Cc: Somalapuram, Amaranath, Koenig, Christian, Yadav, Arvind [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() 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: enable userqueue support for GFX12 2024-10-14 20:29 ` Deucher, Alexander @ 2024-10-15 9:59 ` Sharma, Shashank 2024-10-15 14:58 ` Alex Deucher 0 siblings, 1 reply; 7+ messages in thread From: Sharma, Shashank @ 2024-10-15 9:59 UTC (permalink / raw) To: Deucher, Alexander, amd-gfx@lists.freedesktop.org Cc: Somalapuram, Amaranath, Koenig, Christian, Yadav, Arvind [-- 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 --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: enable userqueue support for GFX12 2024-10-15 9:59 ` Sharma, Shashank @ 2024-10-15 14:58 ` Alex Deucher 2024-10-15 16:37 ` Sharma, Shashank 0 siblings, 1 reply; 7+ messages in thread From: Alex Deucher @ 2024-10-15 14:58 UTC (permalink / raw) To: Sharma, Shashank Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Somalapuram, Amaranath, Koenig, Christian, Yadav, Arvind On Tue, Oct 15, 2024 at 6:13 AM Sharma, Shashank <shashank.sharma@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@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. 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. > > 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 > > - 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: enable userqueue support for GFX12 2024-10-15 14:58 ` Alex Deucher @ 2024-10-15 16:37 ` Sharma, Shashank 2024-10-18 19:18 ` Alex Deucher 0 siblings, 1 reply; 7+ messages in thread From: Sharma, Shashank @ 2024-10-15 16:37 UTC (permalink / raw) To: Alex Deucher Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Somalapuram, Amaranath, Koenig, Christian, Yadav, Arvind On 15/10/2024 16:58, Alex Deucher wrote: > On Tue, Oct 15, 2024 at 6:13 AM Sharma, Shashank > <shashank.sharma@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@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. > 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@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: enable userqueue support for GFX12 2024-10-15 16:37 ` Sharma, Shashank @ 2024-10-18 19:18 ` Alex Deucher 2024-10-22 16:39 ` Sharma, Shashank 0 siblings, 1 reply; 7+ messages in thread From: Alex Deucher @ 2024-10-18 19:18 UTC (permalink / raw) To: Sharma, Shashank Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Somalapuram, Amaranath, Koenig, Christian, Yadav, Arvind [-- Attachment #1: Type: text/plain, Size: 6452 bytes --] On Tue, Oct 15, 2024 at 12:37 PM Sharma, Shashank <shashank.sharma@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@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@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. > > 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@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: 0008-drm-amdgpu-enable-userqueue-support-for-GFX12.patch --] [-- Type: text/x-patch, Size: 2455 bytes --] From be97efd971c50e19c951dff075343d3e7ad0ae31 Mon Sep 17 00:00:00 2001 From: Somalapuram Amaranath <Amaranath.Somalapuram@amd.com> Date: Thu, 10 Oct 2024 20:08:06 +0200 Subject: [PATCH 8/8] drm/amdgpu: enable userqueue support for GFX12 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. v2: rebase on proposed changes (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> Signed-off-by: Alex Deucher <alexander.deucher@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 2c42af225696..fe31ff9e425d 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_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_funcs; + adev->userq_funcs[AMDGPU_HW_IP_COMPUTE] = &userq_mes_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 c1dbb12843f6..2cbf995c7799 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_userqueue.h" MODULE_FIRMWARE("amdgpu/sdma_7_0_0.bin"); MODULE_FIRMWARE("amdgpu/sdma_7_0_1.bin"); @@ -1320,6 +1321,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_funcs; +#endif + + return r; } -- 2.46.2 [-- Attachment #3: 0002-drm-amdgpu-gfx11-update-mqd-init-for-UQ.patch --] [-- Type: text/x-patch, Size: 1198 bytes --] From 5478434578c7448ac3f4edf7956994982086970d Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Fri, 18 Oct 2024 14:14:34 -0400 Subject: [PATCH 2/8] drm/amdgpu/gfx11: update mqd init for UQ Set the addresses for the UQ metadata. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c index 5aff8f72de9c..a4f051275e1c 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c @@ -4008,6 +4008,14 @@ static int gfx_v11_0_gfx_mqd_init(struct amdgpu_device *adev, void *m, /* active the queue */ mqd->cp_gfx_hqd_active = 1; + /* set gfx UQ items */ + mqd->shadow_base_lo = lower_32_bits(prop->shadow_addr); + mqd->shadow_base_hi = upper_32_bits(prop->shadow_addr); + mqd->gds_bkup_base_lo = lower_32_bits(prop->gds_bkup_addr); + mqd->gds_bkup_base_hi = upper_32_bits(prop->gds_bkup_addr); + mqd->fw_work_area_base_lo = lower_32_bits(prop->csa_addr); + mqd->fw_work_area_base_hi = upper_32_bits(prop->csa_addr); + return 0; } -- 2.46.2 [-- Attachment #4: 0003-drm-amdgpu-gfx12-update-mqd-init-for-UQ.patch --] [-- Type: text/x-patch, Size: 1072 bytes --] From fd0173b5d6733bec9e3b27fc37d16f25958bdb88 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Fri, 18 Oct 2024 14:15:12 -0400 Subject: [PATCH 3/8] drm/amdgpu/gfx12: update mqd init for UQ Set the addresses for the UQ metadata. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 6 ++++++ 1 file changed, 6 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..2c42af225696 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c @@ -2929,6 +2929,12 @@ static int gfx_v12_0_gfx_mqd_init(struct amdgpu_device *adev, void *m, /* active the queue */ mqd->cp_gfx_hqd_active = 1; + /* set gfx UQ items */ + mqd->shadow_base_lo = lower_32_bits(prop->shadow_addr); + mqd->shadow_base_hi = upper_32_bits(prop->shadow_addr); + mqd->fw_work_area_base_lo = lower_32_bits(prop->csa_addr); + mqd->fw_work_area_base_hi = upper_32_bits(prop->csa_addr); + return 0; } -- 2.46.2 [-- Attachment #5: 0004-drm-amdgpu-sdma6-update-mqd-init-for-UQ.patch --] [-- Type: text/x-patch, Size: 998 bytes --] From debbbded9809e59b6b322b5dc999550bce3f9085 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Fri, 18 Oct 2024 14:15:31 -0400 Subject: [PATCH 4/8] drm/amdgpu/sdma6: update mqd init for UQ Set the addresses for the UQ metadata. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c index 4856a093e23f..538a97e2e2a4 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c @@ -891,6 +891,9 @@ static int sdma_v6_0_mqd_init(struct amdgpu_device *adev, void *mqd, m->sdmax_rlcx_rb_aql_cntl = regSDMA0_QUEUE0_RB_AQL_CNTL_DEFAULT; m->sdmax_rlcx_dummy_reg = regSDMA0_QUEUE0_DUMMY_REG_DEFAULT; + m->sdmax_rlcx_csa_addr_lo = lower_32_bits(prop->csa_addr); + m->sdmax_rlcx_csa_addr_hi = upper_32_bits(prop->csa_addr); + return 0; } -- 2.46.2 [-- Attachment #6: 0001-drm-amdgpu-add-some-additional-members-to-amdgpu_mqd.patch --] [-- Type: text/x-patch, Size: 852 bytes --] From c70120d8c727b42384ddcd3b414136fb522d2a14 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Fri, 18 Oct 2024 13:58:23 -0400 Subject: [PATCH 1/8] drm/amdgpu: add some additional members to amdgpu_mqd_prop These are needed for user queues. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 48c9b9b06905..d3ee1fbf1e07 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -813,6 +813,9 @@ struct amdgpu_mqd_prop { uint32_t hqd_queue_priority; bool allow_tunneling; bool hqd_active; + uint64_t shadow_addr; + uint64_t gds_bkup_addr; + uint64_t csa_addr; }; struct amdgpu_mqd { -- 2.46.2 [-- Attachment #7: 0006-drm-amdgpu-uq-remove-gfx11-specifics-from-UQ-setup.patch --] [-- Type: text/x-patch, Size: 4235 bytes --] From a7a5efe8f8897869c5fac643f7a10b788a671ab9 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Fri, 18 Oct 2024 14:34:18 -0400 Subject: [PATCH 6/8] drm/amdgpu/uq: remove gfx11 specifics from UQ setup This can all be handled by in the IP specific mpd init code. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- .../gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c | 84 ++++++++----------- 1 file changed, 36 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c index e70b8e429e9c..40cc295862ec 100644 --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c @@ -23,8 +23,6 @@ */ #include "amdgpu.h" #include "amdgpu_gfx.h" -#include "v11_structs.h" -#include "mes_v11_0.h" #include "mes_v11_0_userqueue.h" #define AMDGPU_USERQ_PROC_CTX_SZ PAGE_SIZE @@ -180,52 +178,6 @@ static int mes_v11_0_userq_create_ctx_space(struct amdgpu_userq_mgr *uq_mgr, return r; } - /* Shadow, GDS and CSA objects come directly from userspace */ - if (mqd_user->ip_type == AMDGPU_HW_IP_GFX) { - struct v11_gfx_mqd *mqd = queue->mqd.cpu_ptr; - struct drm_amdgpu_userq_mqd_gfx11 *mqd_gfx_v11; - - if (mqd_user->mqd_size != sizeof(*mqd_gfx_v11) || !mqd_user->mqd) { - DRM_ERROR("Invalid GFX MQD\n"); - return -EINVAL; - } - - mqd_gfx_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size); - if (IS_ERR(mqd_gfx_v11)) { - DRM_ERROR("Failed to read user MQD\n"); - amdgpu_userqueue_destroy_object(uq_mgr, ctx); - return -ENOMEM; - } - - mqd->shadow_base_lo = mqd_gfx_v11->shadow_va & 0xFFFFFFFC; - mqd->shadow_base_hi = upper_32_bits(mqd_gfx_v11->shadow_va); - - mqd->gds_bkup_base_lo = mqd_gfx_v11->gds_va & 0xFFFFFFFC; - mqd->gds_bkup_base_hi = upper_32_bits(mqd_gfx_v11->gds_va); - - mqd->fw_work_area_base_lo = mqd_gfx_v11->csa_va & 0xFFFFFFFC; - mqd->fw_work_area_base_hi = upper_32_bits(mqd_gfx_v11->csa_va); - kfree(mqd_gfx_v11); - } else if (mqd_user->ip_type == AMDGPU_HW_IP_DMA) { - struct v11_sdma_mqd *mqd = queue->mqd.cpu_ptr; - struct drm_amdgpu_userq_mqd_sdma_gfx11 *mqd_sdma_v11; - - if (mqd_user->mqd_size != sizeof(*mqd_sdma_v11) || !mqd_user->mqd) { - DRM_ERROR("Invalid SDMA MQD\n"); - return -EINVAL; - } - - mqd_sdma_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size); - if (IS_ERR(mqd_sdma_v11)) { - DRM_ERROR("Failed to read sdma user MQD\n"); - amdgpu_userqueue_destroy_object(uq_mgr, ctx); - return -ENOMEM; - } - - mqd->sdmax_rlcx_csa_addr_lo = mqd_sdma_v11->csa_va & 0xFFFFFFFC; - mqd->sdmax_rlcx_csa_addr_hi = upper_32_bits(mqd_sdma_v11->csa_va); - } - return 0; } @@ -289,6 +241,42 @@ static int mes_v11_0_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, userq_props->hqd_queue_priority = AMDGPU_GFX_QUEUE_PRIORITY_MINIMUM; userq_props->hqd_active = false; kfree(compute_mqd); + } else if (queue->queue_type == AMDGPU_HW_IP_GFX) { + struct drm_amdgpu_userq_mqd_gfx11 *mqd_gfx_v11; + + if (mqd_user->mqd_size != sizeof(*mqd_gfx_v11) || !mqd_user->mqd) { + DRM_ERROR("Invalid GFX MQD\n"); + return -EINVAL; + } + + mqd_gfx_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size); + if (IS_ERR(mqd_gfx_v11)) { + DRM_ERROR("Failed to read user MQD\n"); + amdgpu_userqueue_destroy_object(uq_mgr, ctx); + return -ENOMEM; + } + + userq_props->shadow_addr = mqd_gfx_v11->shadow_va; + userq_props->gds_bkup_addr = mqd_gfx_v11->gds_va; + userq_props->csa_addr = mqd_gfx_v11->csa_va; + kfree(mqd_gfx_v11); + } else if (queue->queue_type == AMDGPU_HW_IP_DMA) { + struct drm_amdgpu_userq_mqd_sdma_gfx11 *mqd_sdma_v11; + + if (mqd_user->mqd_size != sizeof(*mqd_sdma_v11) || !mqd_user->mqd) { + DRM_ERROR("Invalid SDMA MQD\n"); + return -EINVAL; + } + + mqd_sdma_v11 = memdup_user(u64_to_user_ptr(mqd_user->mqd), mqd_user->mqd_size); + if (IS_ERR(mqd_sdma_v11)) { + DRM_ERROR("Failed to read sdma user MQD\n"); + amdgpu_userqueue_destroy_object(uq_mgr, ctx); + return -ENOMEM; + } + + userq_props->csa_addr = mqd_sdma_v11->csa_va; + kfree(mqd_sdma_v11); } queue->userq_prop = userq_props; -- 2.46.2 [-- Attachment #8: 0007-drm-amdgpu-uq-make-MES-UQ-setup-generic.patch --] [-- Type: text/x-patch, Size: 9380 bytes --] From 52021be575c5f06503986e664f69352c47eb55d7 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Fri, 18 Oct 2024 15:03:03 -0400 Subject: [PATCH 7/8] drm/amdgpu/uq: make MES UQ setup generic Now that all of the IP specific code has been moved into the IP specific functions, we can make this code generic. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/Makefile | 2 +- drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 10 ++-- ...{mes_v11_0_userqueue.c => mes_userqueue.c} | 49 +++++++++---------- ...{mes_v11_0_userqueue.h => mes_userqueue.h} | 6 +-- drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 4 +- 5 files changed, 35 insertions(+), 36 deletions(-) rename drivers/gpu/drm/amd/amdgpu/{mes_v11_0_userqueue.c => mes_userqueue.c} (88%) rename drivers/gpu/drm/amd/amdgpu/{mes_v11_0_userqueue.h => mes_userqueue.h} (91%) diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile index f82649b1d4ab..4cc709c7d868 100644 --- a/drivers/gpu/drm/amd/amdgpu/Makefile +++ b/drivers/gpu/drm/amd/amdgpu/Makefile @@ -176,7 +176,7 @@ amdgpu-y += \ mes_v12_0.o \ # add GFX userqueue support -amdgpu-$(CONFIG_DRM_AMDGPU_NAVI3X_USERQ) += mes_v11_0_userqueue.o +amdgpu-$(CONFIG_DRM_AMDGPU_NAVI3X_USERQ) += mes_userqueue.o # add UVD block amdgpu-y += \ diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c index a4f051275e1c..b9aff8191524 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c @@ -49,7 +49,7 @@ #include "gfx_v11_0_3.h" #include "nbio_v4_3.h" #include "mes_v11_0.h" -#include "mes_v11_0_userqueue.h" +#include "mes_userqueue.h" #define GFX11_NUM_GFX_RINGS 1 #define GFX11_MEC_HPD_SIZE 2048 @@ -1557,8 +1557,8 @@ static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block) adev->gfx.mec.num_pipe_per_mec = 4; 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; + adev->userq_funcs[AMDGPU_HW_IP_GFX] = &userq_mes_funcs; + adev->userq_funcs[AMDGPU_HW_IP_COMPUTE] = &userq_mes_funcs; #endif break; case IP_VERSION(11, 0, 1): @@ -1573,8 +1573,8 @@ static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block) adev->gfx.mec.num_pipe_per_mec = 4; adev->gfx.mec.num_queue_per_pipe = 4; #ifdef CONFIG_DRM_AMD_USERQ_GFX - adev->userq_funcs[AMDGPU_HW_IP_GFX] = &userq_mes_v11_0_funcs; - adev->userq_funcs[AMDGPU_HW_IP_COMPUTE] = &userq_mes_v11_0_funcs; + adev->userq_funcs[AMDGPU_HW_IP_GFX] = &userq_mes_funcs; + adev->userq_funcs[AMDGPU_HW_IP_COMPUTE] = &userq_mes_funcs; #endif break; default: diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c similarity index 88% rename from drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c rename to drivers/gpu/drm/amd/amdgpu/mes_userqueue.c index 40cc295862ec..b782bc31fd62 100644 --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.c +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c @@ -23,13 +23,13 @@ */ #include "amdgpu.h" #include "amdgpu_gfx.h" -#include "mes_v11_0_userqueue.h" +#include "mes_userqueue.h" #define AMDGPU_USERQ_PROC_CTX_SZ PAGE_SIZE #define AMDGPU_USERQ_GANG_CTX_SZ PAGE_SIZE static int -mes_v11_0_map_gtt_bo_to_gart(struct amdgpu_bo *bo) +mes_userq_map_gtt_bo_to_gart(struct amdgpu_bo *bo) { int ret; @@ -57,7 +57,7 @@ mes_v11_0_map_gtt_bo_to_gart(struct amdgpu_bo *bo) } static int -mes_v11_0_create_wptr_mapping(struct amdgpu_userq_mgr *uq_mgr, +mes_userq_create_wptr_mapping(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_queue *queue, uint64_t wptr) { @@ -85,7 +85,7 @@ mes_v11_0_create_wptr_mapping(struct amdgpu_userq_mgr *uq_mgr, return -EINVAL; } - ret = mes_v11_0_map_gtt_bo_to_gart(wptr_obj->obj); + ret = mes_userq_map_gtt_bo_to_gart(wptr_obj->obj); if (ret) { DRM_ERROR("Failed to map wptr bo to GART\n"); return ret; @@ -95,9 +95,9 @@ mes_v11_0_create_wptr_mapping(struct amdgpu_userq_mgr *uq_mgr, return 0; } -static int mes_v11_0_userq_map(struct amdgpu_userq_mgr *uq_mgr, - struct amdgpu_usermode_queue *queue, - struct amdgpu_mqd_prop *userq_props) +static int mes_userq_map(struct amdgpu_userq_mgr *uq_mgr, + struct amdgpu_usermode_queue *queue, + struct amdgpu_mqd_prop *userq_props) { struct amdgpu_device *adev = uq_mgr->adev; struct amdgpu_userq_obj *ctx = &queue->fw_obj; @@ -140,8 +140,8 @@ static int mes_v11_0_userq_map(struct amdgpu_userq_mgr *uq_mgr, return 0; } -static void mes_v11_0_userq_unmap(struct amdgpu_userq_mgr *uq_mgr, - struct amdgpu_usermode_queue *queue) +static void mes_userq_unmap(struct amdgpu_userq_mgr *uq_mgr, + struct amdgpu_usermode_queue *queue) { struct amdgpu_device *adev = uq_mgr->adev; struct mes_remove_queue_input queue_input; @@ -159,9 +159,9 @@ static void mes_v11_0_userq_unmap(struct amdgpu_userq_mgr *uq_mgr, DRM_ERROR("Failed to unmap queue in HW, err (%d)\n", r); } -static int mes_v11_0_userq_create_ctx_space(struct amdgpu_userq_mgr *uq_mgr, - struct amdgpu_usermode_queue *queue, - struct drm_amdgpu_userq_in *mqd_user) +static int mes_userq_create_ctx_space(struct amdgpu_userq_mgr *uq_mgr, + struct amdgpu_usermode_queue *queue, + struct drm_amdgpu_userq_in *mqd_user) { struct amdgpu_userq_obj *ctx = &queue->fw_obj; int r, size; @@ -181,9 +181,9 @@ static int mes_v11_0_userq_create_ctx_space(struct amdgpu_userq_mgr *uq_mgr, return 0; } -static int mes_v11_0_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, - struct drm_amdgpu_userq_in *args_in, - struct amdgpu_usermode_queue *queue) +static int mes_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, + struct drm_amdgpu_userq_in *args_in, + struct amdgpu_usermode_queue *queue) { struct amdgpu_device *adev = uq_mgr->adev; struct amdgpu_mqd *mqd_hw_default = &adev->mqds[queue->queue_type]; @@ -288,21 +288,21 @@ static int mes_v11_0_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, } /* Create BO for FW operations */ - r = mes_v11_0_userq_create_ctx_space(uq_mgr, queue, mqd_user); + r = mes_userq_create_ctx_space(uq_mgr, queue, mqd_user); if (r) { DRM_ERROR("Failed to allocate BO for userqueue (%d)", r); goto free_mqd; } /* FW expects WPTR BOs to be mapped into GART */ - r = mes_v11_0_create_wptr_mapping(uq_mgr, queue, userq_props->wptr_gpu_addr); + r = mes_userq_create_wptr_mapping(uq_mgr, queue, userq_props->wptr_gpu_addr); if (r) { DRM_ERROR("Failed to create WPTR mapping\n"); goto free_ctx; } /* Map userqueue into FW using MES */ - r = mes_v11_0_userq_map(uq_mgr, queue, userq_props); + r = mes_userq_map(uq_mgr, queue, userq_props); if (r) { DRM_ERROR("Failed to init MQD\n"); goto free_ctx; @@ -322,18 +322,17 @@ static int mes_v11_0_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, return r; } -static void -mes_v11_0_userq_mqd_destroy(struct amdgpu_userq_mgr *uq_mgr, - struct amdgpu_usermode_queue *queue) +static void mes_userq_mqd_destroy(struct amdgpu_userq_mgr *uq_mgr, + struct amdgpu_usermode_queue *queue) { - mes_v11_0_userq_unmap(uq_mgr, queue); + mes_userq_unmap(uq_mgr, queue); amdgpu_bo_unref(&queue->wptr_obj.obj); amdgpu_userqueue_destroy_object(uq_mgr, &queue->fw_obj); kfree(queue->userq_prop); amdgpu_userqueue_destroy_object(uq_mgr, &queue->mqd); } -const struct amdgpu_userq_funcs userq_mes_v11_0_funcs = { - .mqd_create = mes_v11_0_userq_mqd_create, - .mqd_destroy = mes_v11_0_userq_mqd_destroy, +const struct amdgpu_userq_funcs userq_mes_funcs = { + .mqd_create = mes_userq_mqd_create, + .mqd_destroy = mes_userq_mqd_destroy, }; diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.h b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.h similarity index 91% rename from drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.h rename to drivers/gpu/drm/amd/amdgpu/mes_userqueue.h index 2c102361ca82..d0a521312ad4 100644 --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0_userqueue.h +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.h @@ -22,9 +22,9 @@ * */ -#ifndef MES_V11_0_USERQ_H -#define MES_V11_0_USERQ_H +#ifndef MES_USERQ_H +#define MES_USERQ_H #include "amdgpu_userqueue.h" -extern const struct amdgpu_userq_funcs userq_mes_v11_0_funcs; +extern const struct amdgpu_userq_funcs userq_mes_funcs; #endif diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c index 538a97e2e2a4..f3dda87cf124 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c @@ -43,7 +43,7 @@ #include "sdma_common.h" #include "sdma_v6_0.h" #include "v11_structs.h" -#include "mes_v11_0_userqueue.h" +#include "mes_userqueue.h" MODULE_FIRMWARE("amdgpu/sdma_6_0_0.bin"); MODULE_FIRMWARE("amdgpu/sdma_6_0_1.bin"); @@ -1367,7 +1367,7 @@ static int sdma_v6_0_sw_init(struct amdgpu_ip_block *ip_block) 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; + adev->userq_funcs[AMDGPU_HW_IP_DMA] = &userq_mes_funcs; #endif return r; -- 2.46.2 [-- Attachment #9: 0005-drm-amdgpu-sdma7-update-mqd-init-for-UQ.patch --] [-- Type: text/x-patch, Size: 1015 bytes --] From 8bd1b33f9887f57dea046c9cd0c8136750670894 Mon Sep 17 00:00:00 2001 From: Alex Deucher <alexander.deucher@amd.com> Date: Fri, 18 Oct 2024 14:15:51 -0400 Subject: [PATCH 5/8] drm/amdgpu/sdma7: update mqd init for UQ Set the addresses for the UQ metadata. Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c index 24f24974ac1d..c1dbb12843f6 100644 --- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c @@ -881,6 +881,9 @@ static int sdma_v7_0_mqd_init(struct amdgpu_device *adev, void *mqd, m->sdmax_rlcx_rb_aql_cntl = 0x4000; //regSDMA0_QUEUE0_RB_AQL_CNTL_DEFAULT; m->sdmax_rlcx_dummy_reg = 0xf; //regSDMA0_QUEUE0_DUMMY_REG_DEFAULT; + m->sdmax_rlcx_csa_addr_lo = lower_32_bits(prop->csa_addr); + m->sdmax_rlcx_csa_addr_hi = upper_32_bits(prop->csa_addr); + return 0; } -- 2.46.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/amdgpu: enable userqueue support for GFX12 2024-10-18 19:18 ` Alex Deucher @ 2024-10-22 16:39 ` Sharma, Shashank 0 siblings, 0 replies; 7+ messages in thread From: Sharma, Shashank @ 2024-10-22 16:39 UTC (permalink / raw) To: Alex Deucher Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org, Somalapuram, Amaranath, Koenig, Christian, Yadav, Arvind [-- Attachment #1: Type: text/plain, Size: 7201 bytes --] [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@gmail.com> Sent: Friday, October 18, 2024 9:18 PM To: Sharma, Shashank <Shashank.Sharma@amd.com> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Somalapuram, Amaranath <Amaranath.Somalapuram@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Yadav, Arvind <Arvind.Yadav@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@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@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@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. > > 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@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: 11657 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-10-22 16:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-10 18:08 [PATCH] drm/amdgpu: enable userqueue support for GFX12 Shashank Sharma 2024-10-14 20:29 ` Deucher, Alexander 2024-10-15 9:59 ` Sharma, Shashank 2024-10-15 14:58 ` Alex Deucher 2024-10-15 16:37 ` Sharma, Shashank 2024-10-18 19:18 ` Alex Deucher 2024-10-22 16:39 ` Sharma, Shashank
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox