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
