[PATCH] drm/amd/pm: Snapshot PPT limits for suspend and reset recovery

Yang Wang <[email protected]>
Newsgroups org.freedesktop.lists.amd-gfx
Message-ID <[email protected]>
The DPM user profile records driver-programmed policy, not the effective
PMFW limits. Replaying it after recovery can restore stale policy.

Maintain a PMFW PPT snapshot:
- Query supported PPT controllers before suspend and mode1 reset.
- Save successful queries with the current OS AC/DC source.
- Clear the selected source mask before capture to prevent stale replay.
- Refresh the OS power source before recovery.
- Synchronize the current source with PMFW before restoring PPT limits.
- Restore PPT limits only when the snapshot source still matches.
- Do not replay PPT policy on normal AC/DC changes.

Capture reset state before DPM and PMFW teardown.
Add debug logging for the snapshot, restore order, and PMFW AC/DC sync.

Signed-off-by: Yang Wang <[email protected]>
---
 drivers/gpu/drm/amd/pm/amdgpu_dpm.c           |   2 +-
 drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c     | 152 +++++++++++++-----
 drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |   8 +-
 3 files changed, 117 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
index ce526db4d24a..c3688b3b12cc 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_dpm.c
@@ -508,7 +508,7 @@ void amdgpu_pm_acpi_event_handler(struct amdgpu_device *adev)
 			amdgpu_dpm_notify_ac_dc(adev);
 
 		if (is_support_sw_smu(adev))
-			smu_set_ac_dc(adev->powerplay.pp_handle, true);
+			smu_set_ac_dc(adev->powerplay.pp_handle);
 
 		mutex_unlock(&adev->pm.mutex);
 	}
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index bfe2c0bf426d..e5cb8d0420ab 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -489,44 +489,122 @@ static void smu_set_user_clk_dependencies(struct smu_context *smu, enum smu_clk_
 		return;
 }
 
-static void smu_restore_ppt_limits(struct smu_context *smu,
-				   bool restore_defaults)
+static void smu_save_ppt_limits(struct smu_context *smu)
 {
+	struct amdgpu_device *adev = smu->adev;
 	enum smu_power_src_type power_source;
-	struct smu_ppt_limit_range *range;
-	uint32_t restore_mask;
-	uint32_t limit;
+	u32 limit;
 	int i, ret;
 
-	power_source = smu->adev->pm.ac_power ?
+	/* AC/DC is maintained by the OS and driver power-source events. */
+	smu->ppt_limits.saved_ac_power = adev->pm.ac_power;
+	power_source = adev->pm.ac_power ?
 		SMU_POWER_SOURCE_AC : SMU_POWER_SOURCE_DC;
-	restore_mask = smu->user_dpm_profile.ppt_limit_user_mask[power_source] &
-		smu->ppt_limits.supported_mask;
-	if (!restore_mask && !restore_defaults)
-		return;
-
-	smu->user_dpm_profile.flags |= SMU_DPM_USER_PROFILE_RESTORE;
+	smu->ppt_limits.saved_mask[power_source] = 0;
+	dev_dbg(adev->dev, "PPT snapshot: source=%s\n",
+		adev->pm.ac_power ? "AC" : "DC");
 
 	for (i = SMU_PPT_LIMIT_PPT0; i < SMU_LIMIT_TYPE_COUNT; i++) {
 		if (!(smu->ppt_limits.supported_mask & BIT(i)))
 			continue;
 
-		if (restore_mask & BIT(i)) {
-			limit = smu->user_dpm_profile.ppt_limits[power_source][i];
-		} else if (restore_defaults) {
-			range = &smu->ppt_limits.range[power_source][i];
-			limit = range->default_value;
-		} else {
+		ret = smu_get_asic_ppt_limit(smu, i, &limit);
+		if (ret) {
+			dev_err(adev->dev,
+				"Failed to save PPT%d limit: %d\n", i, ret);
 			continue;
 		}
 
+		smu->ppt_limits.saved_values[power_source][i] = limit;
+		smu->ppt_limits.saved_mask[power_source] |= BIT(i);
+		dev_dbg(adev->dev, "PPT%d snapshot: source=%s limit=%u\n",
+			i, adev->pm.ac_power ? "AC" : "DC", limit);
+	}
+
+	dev_dbg(adev->dev, "PPT snapshot complete: source=%s mask=%#x\n",
+		adev->pm.ac_power ? "AC" : "DC",
+		smu->ppt_limits.saved_mask[power_source]);
+}
+
+static int smu_restore_ac_dc(struct smu_context *smu, u32 *restore_mask)
+{
+	struct amdgpu_device *adev = smu->adev;
+	enum smu_power_src_type current_source;
+	enum smu_power_src_type saved_source;
+
+	*restore_mask = 0;
+
+	if (adev->in_suspend || amdgpu_reset_in_recovery(adev)) {
+		/* The power source may change while the device is suspended. */
+		adev->pm.ac_power = power_supply_is_system_supplied() > 0;
+		current_source = adev->pm.ac_power ?
+			SMU_POWER_SOURCE_AC : SMU_POWER_SOURCE_DC;
+		saved_source = smu->ppt_limits.saved_ac_power ?
+			SMU_POWER_SOURCE_AC : SMU_POWER_SOURCE_DC;
+		*restore_mask = smu->ppt_limits.saved_mask[saved_source] &
+			smu->ppt_limits.supported_mask;
+		dev_dbg(adev->dev,
+			"PPT restore: saved source=%s mask=%#x, current source=%s\n",
+			smu->ppt_limits.saved_ac_power ? "AC" : "DC",
+			*restore_mask, adev->pm.ac_power ? "AC" : "DC");
+		/*
+		 * A snapshot is valid only for the source under which it was
+		 * captured. Do not replay an AC snapshot after switching to DC,
+		 * or vice versa. The current OS power source takes precedence.
+		 */
+		if (current_source != saved_source) {
+			dev_dbg(adev->dev,
+				"Discard PPT snapshot after source change: %s to %s\n",
+				smu->ppt_limits.saved_ac_power ? "AC" : "DC",
+				adev->pm.ac_power ? "AC" : "DC");
+			*restore_mask = 0;
+		}
+	}
+
+	/*
+	 * NOTE: Synchronize PMFW with the OS AC/DC source before restoring PPT
+	 * limits so both sides use the same power policy. GPIO-controlled
+	 * platforms perform the synchronization in PMFW.
+	 */
+	return smu_set_ac_dc(smu);
+}
+
+static int smu_restore_ppt_limits(struct smu_context *smu)
+{
+	struct amdgpu_device *adev = smu->adev;
+	enum smu_power_src_type power_source;
+	u32 restore_mask;
+	u32 limit;
+	int i, ret;
+
+	ret = smu_restore_ac_dc(smu, &restore_mask);
+	if (ret)
+		return ret;
+	if (!restore_mask)
+		return 0;
+
+	power_source = adev->pm.ac_power ?
+		SMU_POWER_SOURCE_AC : SMU_POWER_SOURCE_DC;
+
+	dev_dbg(adev->dev, "PPT restore: source=%s mask=%#x\n",
+		adev->pm.ac_power ? "AC" : "DC", restore_mask);
+
+	for (i = SMU_PPT_LIMIT_PPT0; i < SMU_LIMIT_TYPE_COUNT; i++) {
+		if (!(restore_mask & BIT(i)))
+			continue;
+
+		limit = smu->ppt_limits.saved_values[power_source][i];
 		ret = smu_set_ppt_limit(smu, i, limit);
 		if (ret)
-			dev_err(smu->adev->dev,
+			dev_err(adev->dev,
 				"Failed to restore PPT%d limit: %d\n", i, ret);
+		else
+			dev_dbg(adev->dev,
+				"PPT%d restored: source=%s limit=%u\n", i,
+				adev->pm.ac_power ? "AC" : "DC", limit);
 	}
 
-	smu->user_dpm_profile.flags &= ~SMU_DPM_USER_PROFILE_RESTORE;
+	return 0;
 }
 
 /**
@@ -954,15 +1032,9 @@ static int smu_late_init(struct amdgpu_ip_block *ip_block)
 		return ret;
 	}
 
-	/*
-	 * Explicitly notify PMFW the power mode the system in. Since
-	 * the PMFW may boot the ASIC with a different mode.
-	 * For those supporting ACDC switch via gpio, PMFW will
-	 * handle the switch automatically. Driver involvement
-	 * is unnecessary.
-	 */
-	adev->pm.ac_power = power_supply_is_system_supplied() > 0;
-	smu_set_ac_dc(smu, false);
+	ret = smu_restore_ppt_limits(smu);
+	if (ret)
+		return ret;
 
 	if ((amdgpu_ip_version(adev, MP1_HWIP, 0) == IP_VERSION(13, 0, 1)) ||
 	    (amdgpu_ip_version(adev, MP1_HWIP, 0) == IP_VERSION(13, 0, 3)))
@@ -997,8 +1069,6 @@ static int smu_late_init(struct amdgpu_ip_block *ip_block)
 		return ret;
 	}
 
-	if (adev->in_suspend || amdgpu_reset_in_recovery(adev))
-		smu_restore_ppt_limits(smu, false);
 	smu_restore_dpm_user_profile(smu);
 
 	return 0;
@@ -2202,6 +2272,9 @@ static int smu_hw_fini(struct amdgpu_ip_block *ip_block)
 	if (!smu->pm_enabled)
 		return 0;
 
+	if (amdgpu_in_reset(adev))
+		smu_save_ppt_limits(smu);
+
 	adev->pm.dpm_enabled = false;
 
 	ret = smu_smc_hw_cleanup(smu);
@@ -2263,6 +2336,8 @@ static int smu_suspend(struct amdgpu_ip_block *ip_block)
 	if (!smu->pm_enabled)
 		return 0;
 
+	smu_save_ppt_limits(smu);
+
 	adev->pm.dpm_enabled = false;
 
 	ret = smu_smc_hw_cleanup(smu);
@@ -2778,7 +2853,7 @@ static int smu_set_watermarks_for_clock_ranges(void *handle,
 	return smu_set_watermarks_table(smu, clock_ranges);
 }
 
-int smu_set_ac_dc(struct smu_context *smu, bool restore_ppt_policy)
+int smu_set_ac_dc(struct smu_context *smu)
 {
 	int ret = 0;
 
@@ -2796,11 +2871,12 @@ int smu_set_ac_dc(struct smu_context *smu, bool restore_ppt_policy)
 				smu->adev->pm.ac_power ? "AC" : "DC");
 			return ret;
 		}
+	} else {
+		dev_dbg(smu->adev->dev,
+			"PMFW controls %s mode through GPIO\n",
+			smu->adev->pm.ac_power ? "AC" : "DC");
 	}
 
-	if (restore_ppt_policy)
-		smu_restore_ppt_limits(smu, true);
-
 	return 0;
 }
 
@@ -3057,12 +3133,6 @@ static int smu_set_ppt_limit(void *handle, uint32_t limit_type, uint32_t limit)
 	ret = smu->ppt_funcs->set_ppt_limit(smu, limit_type, limit);
 	if (ret)
 		return ret;
-	if (!(smu->user_dpm_profile.flags & SMU_DPM_USER_PROFILE_RESTORE)) {
-		smu->user_dpm_profile.ppt_limits[power_source][limit_type] = limit;
-		smu->user_dpm_profile.ppt_limit_user_mask[power_source] |=
-			BIT(limit_type);
-	}
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
index d30951121e1f..9f85b302060e 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
+++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
@@ -238,6 +238,10 @@ struct smu_ppt_limit_range {
 struct smu_ppt_limit_context {
 	struct smu_ppt_limit_range
 		range[SMU_POWER_SOURCE_COUNT][SMU_LIMIT_TYPE_COUNT];
+	/* PMFW state saved before suspend or mode1 reset. */
+	u32 saved_values[SMU_POWER_SOURCE_COUNT][SMU_LIMIT_TYPE_COUNT];
+	u32 saved_mask[SMU_POWER_SOURCE_COUNT];
+	bool saved_ac_power;
 	uint32_t supported_mask;
 };
 
@@ -251,8 +255,6 @@ enum smu_memory_pool_size {
 
 struct smu_user_dpm_profile {
 	uint32_t fan_mode;
-	uint32_t ppt_limits[SMU_POWER_SOURCE_COUNT][SMU_LIMIT_TYPE_COUNT];
-	uint32_t ppt_limit_user_mask[SMU_POWER_SOURCE_COUNT];
 	uint32_t fan_speed_pwm;
 	uint32_t fan_speed_rpm;
 	uint32_t flags;
@@ -1954,7 +1956,7 @@ int smu_set_soft_freq_range(struct smu_context *smu, enum pp_clock_type clk_type
 
 int smu_set_gfx_power_up_by_imu(struct smu_context *smu);
 
-int smu_set_ac_dc(struct smu_context *smu, bool restore_ppt_policy);
+int smu_set_ac_dc(struct smu_context *smu);
 
 int smu_set_xgmi_plpd_mode(struct smu_context *smu,
 			   enum pp_xgmi_plpd_mode mode);
-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.