[PATCH] drm/amdgpu: correct the wrong cached state for GMC on PICASSO

Lazar, Lijo lijo.lazar at amd.com
Tue Dec 14 06:12:50 UTC 2021



On 12/14/2021 7:04 AM, Evan Quan wrote:
> Pair the operations did in GMC ->hw_init and ->hw_fini. That
> can help to maintain correct cached state for GMC and avoid
> unintention gate operation dropping due to wrong cached state.
> 
> BUG: https://gitlab.freedesktop.org/drm/amd/-/issues/1828
> 
> Signed-off-by: Evan Quan <evan.quan at amd.com>
> Change-Id: I9976672a64464b86bb45eed0c25c9599d3bb4c06
> ---
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c            | 8 ++++++++
>   drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c          | 8 ++++----
>   drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 7 ++++++-
>   3 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index db2ec84f7237..c7492db3e189 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1809,6 +1809,14 @@ static int gmc_v9_0_hw_fini(void *handle)
>   		return 0;
>   	}
>   
> +	/*
> +	 * Pair the operations did in gmc_v9_0_hw_init and thus maintain
> +	 * a correct cached state for GMC. Otherwise, the "gate" again
> +	 * operation on S3 resuming will fail due to wrong cached state.
> +	 */
> +	if (adev->mmhub.funcs->update_power_gating)
> +		adev->mmhub.funcs->update_power_gating(adev, false);
> +
>   	amdgpu_irq_put(adev, &adev->gmc.ecc_irq, 0);
>   	amdgpu_irq_put(adev, &adev->gmc.vm_fault, 0);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> index b3bede1dc41d..1da2ec692057 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
> @@ -301,10 +301,10 @@ static void mmhub_v1_0_update_power_gating(struct amdgpu_device *adev,
>   	if (amdgpu_sriov_vf(adev))
>   		return;
>   
> -	if (enable && adev->pg_flags & AMD_PG_SUPPORT_MMHUB) {
> -		amdgpu_dpm_set_powergating_by_smu(adev, AMD_IP_BLOCK_TYPE_GMC, true);
> -
> -	}
> +	if (adev->pg_flags & AMD_PG_SUPPORT_MMHUB)
> +		amdgpu_dpm_set_powergating_by_smu(adev,
> +						  AMD_IP_BLOCK_TYPE_GMC,
> +						  enable);
>   }
>   
>   static int mmhub_v1_0_gart_enable(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> index 3656a77baea4..9953a77cb987 100644
> --- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
> @@ -1167,7 +1167,12 @@ static int pp_set_powergating_by_smu(void *handle,
>   		pp_dpm_powergate_vce(handle, gate);
>   		break;
>   	case AMD_IP_BLOCK_TYPE_GMC:
> -		pp_dpm_powergate_mmhub(handle);
> +		/*
> +		 * For now, this is only used on PICASSO.
> +		 * And only "gate" operation is supported.
> +		 */
> +		if (gate)
> +			pp_dpm_powergate_mmhub(handle);

This is a generic entry point. Would rather keep PG enable or disable 
supported in mmhub_v1_0 rather than than here. And this comment also 
should be in mmhub_v1_0.

Thanks,
Lijo

>   		break;
>   	case AMD_IP_BLOCK_TYPE_GFX:
>   		ret = pp_dpm_powergate_gfx(handle, gate);
> 


More information about the amd-gfx mailing list