Re: [PATCH] drm/amd/pm: fix smu messages after system resume for smu 11.0.x

On Mon, Aug 17, 2026 at 3:45 AM Yang Wang <[email protected]> wrote:
>
> SMU 11.0.x can retain an unfinished command across system
> suspend, causing SMU messages such as RunDcBtc to fail during resume.
>
> Prepare MP1 for unload during system suspend and record successful completion,
> reload SMC firmware after PSP restart before resume sends further SMU messages,
> then clear the reload state.

Acked-by: Alex Deucher <[email protected]>

Is there any reason not to do mp1 unload for all dGPUs in this case?

>
> Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/5620
>
> Signed-off-by: Yang Wang <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   |  9 ++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h   |  2 ++
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 38 ++++++++++++++++++++---
>  3 files changed, 42 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 04f6ebf31cca..5c00683c1edd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -3512,11 +3512,16 @@ static int psp_load_non_psp_fw(struct psp_context *psp)
>         struct amdgpu_firmware_info *ucode;
>         struct amdgpu_device *adev = psp->adev;
>
> -       if (psp->autoload_supported &&
> -           !psp->pmfw_centralized_cstate_management) {
> +       if (psp->smu_fw_needs_reload ||
> +           (psp->autoload_supported &&
> +            !psp->pmfw_centralized_cstate_management)) {
> +               if (psp->smu_fw_needs_reload)
> +                       dev_dbg(adev->dev,
> +                               "Reloading SMU firmware after MP1 unload\n");
>                 ret = psp_load_smu_fw(psp);
>                 if (ret)
>                         return ret;
> +               psp->smu_fw_needs_reload = false;
>         }
>
>         /* Load P2S table first if it's available */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index d80c85793e3b..20338caa4921 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -443,6 +443,8 @@ struct psp_context {
>         bool                            boot_time_tmr;
>         /* flag to mark whether df cstate management centralized to PMFW */
>         bool                            pmfw_centralized_cstate_management;
> +       /* SMC firmware reload required during resume */
> +       bool                            smu_fw_needs_reload;
>
>         /* xgmi ta firmware and buffer */
>         const struct firmware           *ta_fw;
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 559afdc5815d..8d499d9a86df 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -2182,12 +2182,36 @@ static int smu_smc_hw_cleanup(struct smu_context *smu)
>  static int smu_reset_mp1_state(struct smu_context *smu)
>  {
>         struct amdgpu_device *adev = smu->adev;
> -       int ret = 0;
> +       u32 mp1_version = amdgpu_ip_version(adev, MP1_HWIP, 0);
> +       bool needs_mp1_unload;
> +       int ret;
>
> -       if ((!adev->in_runpm) && (!adev->in_suspend) &&
> -               (!amdgpu_in_reset(adev)) && !smu->is_apu &&
> -                       amdgpu_ip_version(adev, MP1_HWIP, 0) >= IP_VERSION(13, 0, 0))
> -               ret = smu_set_mp1_state(smu, PP_MP1_STATE_UNLOAD);
> +       if (adev->in_runpm || amdgpu_in_reset(adev) || smu->is_apu)
> +               return 0;
> +
> +       switch (mp1_version) {
> +       case IP_VERSION(11, 0, 7):
> +       case IP_VERSION(11, 0, 11):
> +       case IP_VERSION(11, 0, 12):
> +       case IP_VERSION(11, 0, 13):
> +               /*
> +                * These SMU versions require MP1 unload before system suspend.
> +                * The shutdown path already requests it through mp1_state.
> +                */
> +               needs_mp1_unload = adev->mp1_state != PP_MP1_STATE_UNLOAD;
> +               break;
> +       default:
> +               needs_mp1_unload = !adev->in_suspend &&
> +                       mp1_version >= IP_VERSION(13, 0, 0);
> +               break;
> +       }
> +
> +       if (!needs_mp1_unload)
> +               return 0;
> +
> +       ret = smu_set_mp1_state(smu, PP_MP1_STATE_UNLOAD);
> +       if (!ret && adev->in_suspend)
> +               adev->psp.smu_fw_needs_reload = true;
>
>         return ret;
>  }
> @@ -2279,6 +2303,10 @@ static int smu_suspend(struct amdgpu_ip_block *ip_block)
>         if (ret)
>                 return ret;
>
> +       ret = smu_reset_mp1_state(smu);
> +       if (ret)
> +               return ret;
> +
>         smu->watermarks_bitmap &= ~(WATERMARKS_LOADED);
>
>         smu_set_gfx_cgpg(smu, false);
> --
> 2.54.0
>