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 > >
