[AMD Official Use Only - General]
> -----Original Message-----
> From: Sharma, Shashank <Shashank.Sharma@xxxxxxx>
> Sent: Friday, April 5, 2024 11:37 AM
> To: Joshi, Mukul <Mukul.Joshi@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Koenig, Christian <Christian.Koenig@xxxxxxx>; Deucher, Alexander
> <Alexander.Deucher@xxxxxxx>; Yadav, Arvind <Arvind.Yadav@xxxxxxx>
> Subject: Re: [PATCH] drm/amdgpu: fix MES HQD masks
>
>
> On 05/04/2024 17:26, Joshi, Mukul wrote:
> > [AMD Official Use Only - General]
> >
> >> -----Original Message-----
> >> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> >> Shashank Sharma
> >> Sent: Friday, April 5, 2024 10:36 AM
> >> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> >> Cc: Sharma, Shashank <Shashank.Sharma@xxxxxxx>; Koenig, Christian
> >> <Christian.Koenig@xxxxxxx>; Deucher, Alexander
> >> <Alexander.Deucher@xxxxxxx>; Yadav, Arvind <Arvind.Yadav@xxxxxxx>
> >> Subject: [PATCH] drm/amdgpu: fix MES HQD masks
> >>
> >> Caution: This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >>
> >> This patch fixes the existing HQD masks prepared during the MES
> initialization.
> >> These existing masks values were causing problems when we try to
> >> enable GFX oversubscription.
> >>
> >> Cc: Christian König <Christian.Koenig@xxxxxxx>
> >> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
> >> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxx>
> >> Signed-off-by: Arvind Yadav <arvind.yadav@xxxxxxx>
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c | 3 ---
> >> drivers/gpu/drm/amd/amdgpu/mes_v10_1.c | 15 ++++++++++++++-
> >> drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 15 ++++++++++++++-
> >> 3 files changed, 28 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> >> index da48b6da0107..7db80ffda33f 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mes.c
> >> @@ -148,9 +148,6 @@ int amdgpu_mes_init(struct amdgpu_device
> *adev)
> >> adev->mes.compute_hqd_mask[i] = 0xc;
> >> }
> >>
> >> - for (i = 0; i < AMDGPU_MES_MAX_GFX_PIPES; i++)
> >> - adev->mes.gfx_hqd_mask[i] = i ? 0 : 0xfffffffe;
> >> -
> >> for (i = 0; i < AMDGPU_MES_MAX_SDMA_PIPES; i++) {
> >> if (amdgpu_ip_version(adev, SDMA0_HWIP, 0) <
> >> IP_VERSION(6, 0, 0)) diff --git
> >> a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> >> b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> >> index 1e5ad1e08d2a..9217914f824d 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> >> @@ -266,6 +266,19 @@ static int mes_v10_1_query_sched_status(struct
> >> amdgpu_mes *mes)
> >> offsetof(union MESAPI__QUERY_MES_STATUS,
> >> api_status)); }
> >>
> >> +static inline uint32_t mes_v10_get_gfx_hqd_mask(int pipe_index) {
> >> + /* Pipe 1 can't be used for MES due to HW limitation */
> >> + if (pipe_index == 1)
> >> + return 0;
> >> +
> >> + /*
> >> + * GFX V10 supports 2 queues, but we want to keep queue 0
> >> + * reserved for kernel, so enable only queue 1 (1<<1) for MES.
> >> + */
> >> + return 0x2;
> >> +}
> >> +
> >> static int mes_v10_1_set_hw_resources(struct amdgpu_mes *mes) {
> >> int i;
> >> @@ -291,7 +304,7 @@ static int mes_v10_1_set_hw_resources(struct
> >> amdgpu_mes *mes)
> >> mes->compute_hqd_mask[i];
> >>
> >> for (i = 0; i < MAX_GFX_PIPES; i++)
> >> - mes_set_hw_res_pkt.gfx_hqd_mask[i] = mes->gfx_hqd_mask[i];
> >> + mes_set_hw_res_pkt.gfx_hqd_mask[i] =
> >> + mes_v10_get_gfx_hqd_mask(i);
> >>
> >> for (i = 0; i < MAX_SDMA_PIPES; i++)
> >> mes_set_hw_res_pkt.sdma_hqd_mask[i] =
> >> mes->sdma_hqd_mask[i]; diff --git
> >> a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> >> b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> >> index 26d71a22395d..b7dcd936afc8 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> >> @@ -360,6 +360,19 @@ static int mes_v11_0_misc_op(struct
> amdgpu_mes
> >> *mes,
> >> offsetof(union MESAPI__MISC, api_status));
> >> }
> >>
> >> +static inline uint32_t mes_v11_get_gfx_hqd_mask(int pipe_index) {
> >> + /* Pipe 1 can't be used for MES due to HW limitation */
> >> + if (pipe_index == 1)
> >> + return 0;
> >> +
> >> + /*
> >> + * GFX V10 supports 2 queues, but we want to keep queue 0
> >> + * reserved for kernel, so enable only queue 1 (1<<1) for MES.
> >> + */
> >> + return 0x2;
> >> +}
> > There is no difference between this function and the corresponding function
> written for GFX10.
> > Why not make this a common function instead?
>
> This is deliberate, to indicate that the HQD mask can be different for each GFX
> IP version, so as the number of pipes and queue per pipe. Also the limitation
> on pipe 1 will not be there for future versions.
>
But for now this is common for both GFX10 and GFX11. When we have a case where this changes in future, we can implement a new
function specific for that GFX IP version. For now, this should be a common function.
Regards,
Mukul
> - Shashank
>
> > Regards,
> > Mukul
> >
> >> +
> >> static int mes_v11_0_set_hw_resources(struct amdgpu_mes *mes) {
> >> int i;
> >> @@ -385,7 +398,7 @@ static int mes_v11_0_set_hw_resources(struct
> >> amdgpu_mes *mes)
> >> mes->compute_hqd_mask[i];
> >>
> >> for (i = 0; i < MAX_GFX_PIPES; i++)
> >> - mes_set_hw_res_pkt.gfx_hqd_mask[i] = mes->gfx_hqd_mask[i];
> >> + mes_set_hw_res_pkt.gfx_hqd_mask[i] =
> >> + mes_v11_get_gfx_hqd_mask(i);
> >>
> >> for (i = 0; i < MAX_SDMA_PIPES; i++)
> >> mes_set_hw_res_pkt.sdma_hqd_mask[i] =
> >> mes->sdma_hqd_mask[i];
> >> --
> >> 2.43.2
