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

On Tue, Aug 18, 2026 at 11:13 AM Feng, Kenneth <[email protected]> wrote:
>
> AMD General
>
> Hi Alex,
>
> Mp1 unload is not expected to be used on S3 suspend/resume scenarios originally since there is a mode1 reset during S3 suspend.
> The mode1 reset erases everything so psp will reload mp1 fw anyway during S3 resume, no need to ask amdgpu driver to 'remind the psp to reload mp1 fw' by sending the unload message to mp1.
> Kevin's patch addresses this issue: when S3 suspend is triggered, the GPU happens to be in runtime pm suspended state already. Then GPU does runtime pm resume first, then continue the S3 suspend -> S3 resume.
> In this case, during runtime pm resume, psp might mark this as 'no need to load MP1 fw again during reinitialization'. Then even a latter mode1 reset is executed, psp may keep the mark somehow.
> So MP1 unload is added in S3 suspend for this scenario.
> But this is not verified on all dGPUs.

I suspect the mode1 reset in S3 resume was probably a workaround.  It
sounds like doing mp1unload is perhaps a better approach overall?

Alex

> Thanks.
>
>
>
>
>
> -----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?
>
> >
> > 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
> >