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

AMD General

This issue happens in runpm resume, so need to remove the comments:
amdgpu_pmops_resume()
`-- amdgpu_device_resume()
    `-- amdgpu_device_ip_resume()
        `-- smu_resume()
            `-- smu_smc_hw_setup()
                `-- smu_run_btc()
                    `-- RunDcBtc fails

And loading fw twice is a workaround, might need a root cause in the hardware in the future. But the workaround is fine.

Reviewed-by: Kenneth Feng <[email protected]>


-----Original Message-----
From: Wang, Yang(Kevin) <[email protected]>
Sent: Tuesday, August 18, 2026 12:20 PM
To: [email protected]
Cc: Deucher, Alexander <[email protected]>; Zhang, Hawking <[email protected]>; Feng, Kenneth <[email protected]>
Subject: [PATCH v2] drm/amd/pm: Fix SMU messages after system resume for smu 11.0.x

SMU 11.0.x can retain an unfinished command across system suspend, causing SMU messages such as RunDcBtc to fail during resume.

The original failure path is:

amdgpu_pmops_resume()
`-- amdgpu_device_resume()
    `-- amdgpu_device_ip_resume()
        `-- smu_resume()
            `-- smu_smc_hw_setup()
                `-- smu_run_btc()
                    `-- RunDcBtc fails

v1:
- Prepare MP1 for unload for an active system suspend.
- Restore PMFW context after the normal PSP resume load. The initial
  centralized load preserves PMFW-before-TMR ordering; a successful MP1
  unload additionally requires the post-TMR context reload.

v2:
- Snapshot the runtime PM state at system-suspend entry.
- Exclude runtime-suspended BACO entries from the MP1 unload and PMFW
  recovery transaction.
- Mark the successful MP1 unload in PSP to order the post-TMR reload.

Closes: https://gitlab.freedesktop.org/drm/amd/-/work_items/5620
Signed-off-by: Yang Wang <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 11 ++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c   | 11 +++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h   |  2 ++
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 40 ++++++++++++++++++++---
 5 files changed, 58 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 070986b03fb8..71606dd2d6fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -860,6 +860,8 @@ struct amdgpu_device {

        /* s3/s4 mask */
        bool                            in_suspend;
+       /* runtime PM state snapshot at system-suspend entry */
+       bool                            in_runpm_at_suspend;
        bool                            in_s3;
        bool                            in_s4;
        bool                            in_s0ix;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 845160d2c516..8c7df6b4e168 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2803,10 +2803,15 @@ static int amdgpu_pmops_prepare(struct device *dev)
        struct drm_device *drm_dev = dev_get_drvdata(dev);
        struct amdgpu_device *adev = drm_to_adev(drm_dev);

+       adev->in_runpm_at_suspend = false;
+
        /* device maybe not resumed here, return immediately in this case */
        if (adev->in_s4 && adev->in_suspend)
                return 0;

+       /* System PM can change runtime PM state after this callback. */
+       adev->in_runpm_at_suspend = pm_runtime_suspended(dev);
+
        /* Return a positive number here so
         * DPM_FLAG_SMART_SUSPEND works properly
         */
@@ -2825,7 +2830,11 @@ static int amdgpu_pmops_prepare(struct device *dev)

 static void amdgpu_pmops_complete(struct device *dev)  {
-       amdgpu_device_complete(dev_get_drvdata(dev));
+       struct drm_device *drm_dev = dev_get_drvdata(dev);
+       struct amdgpu_device *adev = drm_to_adev(drm_dev);
+
+       amdgpu_device_complete(drm_dev);
+       adev->in_runpm_at_suspend = false;
 }

 static int amdgpu_pmops_suspend(struct device *dev) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 04f6ebf31cca..e252733a6f3b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -3512,11 +3512,18 @@ 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) {
+       /*
+        * NOTE: The centralized PMFW load in psp_hw_start() must precede
+        * TMR setup. A successful MP1 unload also requires PMFW context
+        * recovery after TMR, so reload it for this transaction here.
+        */
+       if (psp->smu_fw_post_tmr_reload ||
+           (psp->autoload_supported &&
+            !psp->pmfw_centralized_cstate_management)) {
                ret = psp_load_smu_fw(psp);
                if (ret)
                        return ret;
+               psp->smu_fw_post_tmr_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..540077b9f070 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 after TMR during system resume */
+       bool                            smu_fw_post_tmr_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..8ff1cb1efc5b 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2182,12 +2182,38 @@ 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):
+               /*
+                * Prepare MP1 only for an active system-suspend entry.
+                * BACO retains the SMU firmware across runtime suspend.
+                */
+               needs_mp1_unload = adev->in_suspend &&
+                       !adev->in_runpm_at_suspend &&
+                       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_post_tmr_reload = true;

        return ret;
 }
@@ -2279,6 +2305,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