[PATCH] drm/amdgpu: Driver needn't request RLC safe mode for gfx MGCG

[Liang, Prike]

[AMD Official Use Only - AMD Internal Distribution Only]

> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Wednesday, December 18, 2024 4:36 PM
> To: Liang, Prike <Prike.Liang at amd.com>; amd-gfx at lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher at amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Driver needn't request RLC safe mode for gfx
> MGCG
>
>
>
> On 12/18/2024 1:01 PM, Prike Liang wrote:
> > In accordance with the MGCG HW sequence, there is no need for the
> > driver to request safe mode before enabling GFX MGCG. For
> > GFX10 and later versions, maintaining safe mode is acceptable for GFX
> > MGCG; otherwise, there will be an increased overhead during safe mode
> > entry and exit when enabling other GFX clock gating. So now this
> > change only apply gfx9 and gfx8, and without polling safe mode, the
> > time required for GFX MGCG entry and exit will be reduced on GFX9 and
> > GFX8 systems.
> >
>
> The only place these get called is *update_gfx_clock_gating(). So it's better to keep
> a single safe mode/entry exit for that rather than keeping it separately for each
> function call within that.
>
Yes, polling the safe mode status respectively will have overhead on the GFX clock gating. Even though mgcg after gfx9 does not require safe mode, *update_gfx_clock_gating() can uniformly enter safe mode before all clock gating for the sake of code simplicity and reducing the time waiting for safe Mode by other clock gating.

> For gfx_v8.0, somehow reference sequences use RLC safe mode entry/exit for
> MGCG.
>
Thank you for bringing this up. I went back and double-checked the GFX8 MGCG enable sequence reference code and seems that it requires safe mode entry as well. I'll follow up with the RLC team to confirm this.

Regards,
    Prike

> Thanks,
> Lijo
>
> > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 4 ----
> > drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 4 ----
> >  2 files changed, 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index af73f85527b7..690235dafec1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -5639,8 +5639,6 @@ static void
> > gfx_v8_0_update_medium_grain_clock_gating(struct amdgpu_device *adev  {
> >     uint32_t temp, data;
> >
> > -   amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
> > -
> >     /* It is disabled by HW by default */
> >     if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
> >             if (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGLS) { @@ -
> 5734,8 +5732,6
> > @@ static void gfx_v8_0_update_medium_grain_clock_gating(struct
> amdgpu_device *adev
> >             /* 7- wait for RLC_SERDES_CU_MASTER &
> RLC_SERDES_NONCU_MASTER idle */
> >             gfx_v8_0_wait_for_rlc_serdes(adev);
> >     }
> > -
> > -   amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
> >  }
> >
> >  static void gfx_v8_0_update_coarse_grain_clock_gating(struct
> > amdgpu_device *adev, diff --git
> > a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > index 4b5006dc3d34..6dec5383ba17 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> > @@ -4964,8 +4964,6 @@ static void
> > gfx_v9_0_update_medium_grain_clock_gating(struct amdgpu_device *adev  {
> >     uint32_t data, def;
> >
> > -   amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
> > -
> >     /* It is disabled by HW by default */
> >     if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_MGCG)) {
> >             /* 1 - RLC_CGTT_MGCG_OVERRIDE */
> > @@ -5030,8 +5028,6 @@ static void
> gfx_v9_0_update_medium_grain_clock_gating(struct amdgpu_device *adev
> >                     WREG32_SOC15(GC, 0, mmCP_MEM_SLP_CNTL, data);
> >             }
> >     }
> > -
> > -   amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
> >  }
> >
> >  static void gfx_v9_0_update_3d_clock_gating(struct amdgpu_device
> > *adev,