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

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Chen, Guchun <guchun.c...@amd.com>
> Sent: Monday, December 13, 2021 21:49
> To: Quan, Evan <evan.q...@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Quan, Evan
> <evan.q...@amd.com>; Limonciello, Mario <mario.limoncie...@amd.com>
> Subject: RE: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on
> PICASSO
> 
> [Public]
> 
> Acked-by: Guchun Chen <guchun.c...@amd.com>
> 
> Regards,
> Guchun
> 
> -----Original Message-----
> From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of Evan
> Quan
> Sent: Tuesday, December 14, 2021 9:34 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <alexander.deuc...@amd.com>; Quan, Evan
> <evan.q...@amd.com>; Limonciello, Mario <mario.limoncie...@amd.com>
> Subject: [PATCH] drm/amdgpu: correct the wrong cached state for GMC on
> PICASSO
> 
> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.fr
> eedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1828&amp;data=04%7C01%7Cguchun.chen%40amd.com%7C42b
> 00d7e1c4e44c0762908d9bea1ef53%7C3dd8961fe4884e608e11a82d994e183d%
> 7C0%7C0%7C637750424983319967%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&a
> mp;sdata=VgBDAbcKN%2FqUz8iBQby9YP8PsG2y93VlnDVhXVaGNBo%3D&amp;r
> eserved=0
> 

The syntax here should be BugLink: <url>

Otherwise
Reviewed-by: Mario Limonciello <mario.limoncie...@amd.com>

> Signed-off-by: Evan Quan <evan.q...@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);
>               break;
>       case AMD_IP_BLOCK_TYPE_GFX:
>               ret = pp_dpm_powergate_gfx(handle, gate);
> --
> 2.29.0