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

AMD General

> -----Original Message-----
> From: Alex Deucher <[email protected]>
> Sent: Monday, August 17, 2026 9:27 PM
> To: Wang, Yang(Kevin) <[email protected]>
> Cc: [email protected]; Deucher, Alexander
> <[email protected]>; Zhang, Hawking
> <[email protected]>; Feng, Kenneth <[email protected]>
> Subject: 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?

No, the unload message is required. Without it, the device will remain in a high‑power state.
Also, the current solution triggers a post-tmr pmfw reload, and this step is necessary for this issue, but this behavior differs from other ASICs,
so, I'm trying to rework this fix if a better approach becomes available.

Best Regards,
Kevin
>
> >
> > 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
> >