Re: [PATCH] drm/amdgpu: Add Fine Grain Clock Gating

[Huang Rui <ray.huang@amd.com>]

On Tue, Nov 03, 2020 at 10:14:01AM +0800, Su, Jinzhou (Joe) wrote:
> 1.Enable Fine Grain Clock Gating - SRAM(SW control) for Vangogh
> 2.Add FGCG flags on amdgpu_pm_info debugfs
> 
> Change-Id: I839a623fcc1a444c880d644035531435c0b0eeb6
> Signed-off-by: Jinzhou.Su <Jinzhou.Su@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 45 ++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/nv.c          |  1 +
>  drivers/gpu/drm/amd/include/amd_shared.h |  1 +
>  drivers/gpu/drm/amd/pm/amdgpu_pm.c       |  1 +
>  4 files changed, 48 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index e4c69125805d..1ef738c35b4a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -7657,12 +7657,50 @@ static void gfx_v10_0_update_coarse_grain_clock_gating(struct amdgpu_device *ade
>  	}
>  }
>  
> +static void gfx_v10_0_update_fine_grain_clock_gating(struct amdgpu_device *adev,
> +						      bool enable)
> +{
> +	uint32_t def, data;
> +
> +	if (enable && (adev->cg_flags & AMD_CG_SUPPORT_GFX_FGCG)) {
> +		def = data = RREG32_SOC15(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE);
> +		/* unset FGCG override */
> +		data &= ~RLC_CGTT_MGCG_OVERRIDE__GFXIP_FGCG_OVERRIDE_MASK;
> +		/* update FGCG override bits */
> +		if (def != data)
> +			WREG32_SOC15(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE, data);
> +
> +		def = data = RREG32_SOC15(GC, 0, mmRLC_CLK_CNTL);
> +		/* unset RLC SRAM CLK GATER override */
> +		data &= ~RLC_CLK_CNTL__RLC_SRAM_CLK_GATER_OVERRIDE_MASK;
> +		/* update RLC SRAM CLK GATER override bits */
> +		if (def != data)
> +			WREG32_SOC15(GC, 0, mmRLC_CLK_CNTL, data);
> +	} else {
> +		def = data = RREG32_SOC15(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE);
> +		/* reset FGCG bits */
> +		data |= RLC_CGTT_MGCG_OVERRIDE__GFXIP_FGCG_OVERRIDE_MASK;
> +		/* disable FGCG*/
> +		if (def != data)
> +			WREG32_SOC15(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE, data);
> +
> +		def = data = RREG32_SOC15(GC, 0, mmRLC_CLK_CNTL);
> +		/* reset RLC SRAM CLK GATER bits */
> +		data |= RLC_CLK_CNTL__RLC_SRAM_CLK_GATER_OVERRIDE_MASK;
> +		/* disable RLC SRAM CLK*/
> +		if (def != data)
> +			WREG32_SOC15(GC, 0, mmRLC_CLK_CNTL, data);
> +	}
> +}
> +
>  static int gfx_v10_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>  					    bool enable)
>  {
>  	amdgpu_gfx_rlc_enter_safe_mode(adev);
>  
>  	if (enable) {
> +		/* enable FGCG firstly*/
> +		gfx_v10_0_update_fine_grain_clock_gating(adev, enable);
>  		/* CGCG/CGLS should be enabled after MGCG/MGLS
>  		 * ===  MGCG + MGLS ===
>  		 */
> @@ -7680,6 +7718,8 @@ static int gfx_v10_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>  		gfx_v10_0_update_3d_clock_gating(adev, enable);
>  		/* ===  MGCG + MGLS === */
>  		gfx_v10_0_update_medium_grain_clock_gating(adev, enable);
> +		/* disable fgcg at last*/
> +		gfx_v10_0_update_fine_grain_clock_gating(adev, enable);
>  	}
>  
>  	if (adev->cg_flags &
> @@ -7848,6 +7888,11 @@ static void gfx_v10_0_get_clockgating_state(void *handle, u32 *flags)
>  	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>  	int data;
>  
> +	/* AMD_CG_SUPPORT_GFX_FGCG */
> +	data = RREG32_KIQ(SOC15_REG_OFFSET(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE));
> +	if (!(data & RLC_CGTT_MGCG_OVERRIDE__GFXIP_FGCG_OVERRIDE_MASK))
> +		*flags |= AMD_CG_SUPPORT_GFX_FGCG;
> +
>  	/* AMD_CG_SUPPORT_GFX_MGCG */
>  	data = RREG32_KIQ(SOC15_REG_OFFSET(GC, 0, mmRLC_CGTT_MGCG_OVERRIDE));
>  	if (!(data & RLC_CGTT_MGCG_OVERRIDE__GFXIP_MGCG_OVERRIDE_MASK))

The whole implementation can be slipped at 3 patches:

The FGCG programming can be put at the 2nd patch.

> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c b/drivers/gpu/drm/amd/amdgpu/nv.c
> index 1f8659a1a4cf..e33d8022cc32 100644
> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> @@ -953,6 +953,7 @@ static int nv_common_early_init(void *handle)
>  			AMD_CG_SUPPORT_GFX_3D_CGLS |
>  			AMD_CG_SUPPORT_MC_MGCG |
>  			AMD_CG_SUPPORT_MC_LS |
> +			AMD_CG_SUPPORT_GFX_FGCG |
>  			AMD_CG_SUPPORT_VCN_MGCG |
>  			AMD_CG_SUPPORT_JPEG_MGCG;
>  		adev->pg_flags = AMD_PG_SUPPORT_GFX_PG |

The FGCG feature enabling for VanGogh can be put at the 3rd patch.

> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
> index 412602d84f71..e59501bde5d2 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -144,6 +144,7 @@ enum amd_powergating_state {
>  #define AMD_CG_SUPPORT_ATHUB_LS			(1 << 28)
>  #define AMD_CG_SUPPORT_ATHUB_MGCG		(1 << 29)
>  #define AMD_CG_SUPPORT_JPEG_MGCG		(1 << 30)
> +#define AMD_CG_SUPPORT_GFX_FGCG         (1 << 31)
>  /* PG flags */
>  #define AMD_PG_SUPPORT_GFX_PG			(1 << 0)
>  #define AMD_PG_SUPPORT_GFX_SMG			(1 << 1)

For introducing a new flag, we would better use a separate patch at first.
If more asic uses this feature in future, that's able to avoid git-bisect
issue.

> diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> index 080af05724ed..e57153d1fa24 100644
> --- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
> @@ -39,6 +39,7 @@
>  #include "hwmgr.h"
>  
>  static const struct cg_flag_name clocks[] = {
> +	{AMD_CG_SUPPORT_GFX_FGCG, "Graphics Fine Grain Clock Gating"},
>  	{AMD_CG_SUPPORT_GFX_MGCG, "Graphics Medium Grain Clock Gating"},
>  	{AMD_CG_SUPPORT_GFX_MGLS, "Graphics Medium Grain memory Light Sleep"},
>  	{AMD_CG_SUPPORT_GFX_CGCG, "Graphics Coarse Grain Clock Gating"},

This part can be with AMD_CG_SUPPORT_GFX_FGCG definition on the first
patch.

Others look good for me.

Thanks,
Ray

> -- 
> 2.17.1
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx