On 06-Aug-26 1:26 PM, Wang, Yang(Kevin) wrote:
AMD GeneralDriver triggers resets which could relaod the whole FW. In such cases driver remains responsible for restoring the last known power limit. The current method doesn't do that.No, the original code already contains this restore logic, include suspend/resume + gpu recovery, and save/restore logic has been verified on Navi21 and Navi48 ASICs. And the if no user edit power limit from user side, the driver will skip restore power limit, in this case, the driver will use default value after firmware reloading. btw, your change will break gpu od + power limit logic in navi3x/navi4x.
This patch doesn't break that. Whatever user has set as the last limit will be the current limit in Navi3x/Navi4x case. That will be restored. If user hasn't edited limit, current limit will be equivalent to default limit and then it's not set again.
So What issue you have facing now ?
Driver remains responsible for restoring the state triggered by inband actions like suspend/resume or reset as OOB doesn't monitor everything. In such cases, driver being the inband owner needs to restore the proper state after the event.
Thanks, Lijo
Best Regards, Kevin-----Original Message----- From: Lazar, Lijo <[email protected]> Sent: Thursday, August 6, 2026 3:48 PM To: Wang, Yang(Kevin) <[email protected]>; amd- [email protected] Cc: Zhang, Hawking <[email protected]>; Deucher, Alexander <[email protected]>; Kamal, Asad <[email protected]> Subject: Re: [PATCH] drm/amd/pm: Track current PPT limit for restore On 06-Aug-26 1:03 PM, Wang, Yang(Kevin) wrote:AMD General I do not agree with adding a read-to-cache path for the current PPT limit. The current limit is PMFW runtime state. A read of that state must remain anobservation; it must not implicitly create or update driver-owned persistent policy.This separation is intentional: the driver keeps immutable capabilities fromthe PPTable and restores only limits that it successfully programmed itself.Caching a value returned by GetPptLimit would turn a potentially transientor out-of-band PMFW state into a value that the driver replays after suspend or reset.There is no ownership, notification, or synchronization mechanism thatmakes such a cache authoritative, so it can only become stale.and the existing user_dpm_profile policy *CACHE ALREADY* handles thesupported driver-owned update path.The existing path also doesn't have any synchronization with out-of-band, so the same argument holds. i.e., it overwrites whatever set by external source.If no bug fix or feature improve, please drop this patch.Driver triggers resets which could relaod the whole FW. In such cases driver remains responsible for restoring the last known power limit. The current method doesn't do that. I will add a bug fix tag as the previous patch doesn't restore the current limit properly. Thanks, LijoBest Regards, Kevin-----Original Message----- From: Lazar, Lijo <[email protected]> Sent: Thursday, August 6, 2026 2:14 PM To: [email protected] Cc: Zhang, Hawking <[email protected]>; Deucher, Alexander <[email protected]>; Kamal, Asad <[email protected]>;Wang,Yang(Kevin) <[email protected]> Subject: [PATCH] drm/amd/pm: Track current PPT limit for restore Cache the applied PPT limit in the ppt limit range and refresh it on every get and set. Initialize with the default value on fresh load; a value equal to default is treated as unset. On suspend or reset recovery, restore the cached current limit when it differs from the default, falling back to the default otherwise. This replaces the user- mask based restore, covering limits changed outside the user profile path as well. Signed-off-by: Lijo Lazar <[email protected]> --- drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 39+++++++++++++------ drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 1 + 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c index f45cd4e31415..7edd919a7832 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c @@ -495,16 +495,11 @@ static void smu_restore_ppt_limits(struct smu_context *smu, { enum smu_power_src_type power_source; struct smu_ppt_limit_range *range; - uint32_t restore_mask; uint32_t limit; int i, ret; power_source = smu->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; @@ -512,14 +507,14 @@ static void smu_restore_ppt_limits(struct smu_context *smu, 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]; + range = &smu->ppt_limits.range[power_source][i]; + if (range->current_value && + range->current_value != range->default_value) + limit = range->current_value; + else if (restore_defaults) limit = range->default_value; - } else { + else continue; - } ret = smu_set_ppt_limit(smu, i, limit); if (ret) @@ -875,6 +870,21 @@ static int smu_early_init(struct amdgpu_ip_block *ip_block) return smu_init_microcode(smu); } +static void smu_init_ppt_limits_current(struct smu_context *smu) { + struct smu_ppt_limit_range *range; + int i, j; + + for (i = SMU_POWER_SOURCE_AC; i < SMU_POWER_SOURCE_COUNT; i++) { + for (j = SMU_PPT_LIMIT_PPT0; j < SMU_LIMIT_TYPE_COUNT; j++) { + if (!(smu->ppt_limits.supported_mask & BIT(j))) + continue; + range = &smu->ppt_limits.range[i][j]; + range->current_value = range->default_value; + } + } +} + static int smu_set_default_dpm_table(struct smu_context *smu) { struct amdgpu_device *adev = smu->adev; @@ -1911,6 +1921,9 @@ static int smu_smc_hw_setup(struct smu_context *smu) if (ret) dev_err(adev->dev, "Error during wbrf init call\n"); + if (!adev->in_suspend && !amdgpu_reset_in_recovery(adev)) + smu_init_ppt_limits_current(smu); + return ret; } @@ -2999,6 +3012,9 @@ int smu_get_ppt_limit(void *handle, switch (limit_level) { case SMU_PPT_LIMIT_CURRENT: ret = smu_get_asic_ppt_limit(smu, limit_type, limit); + if (!ret) + smu-ppt_limits.range[power_source][limit_type].current_value =+ *limit; break; case SMU_PPT_LIMIT_DEFAULT: *limit = smu-ppt_limits.range[power_source][limit_type].default_value;@@ -3061,6 +3077,7 @@ 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; + range->current_value = limit; 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] |= 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 92658eb3886d..5222b48eba25 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h @@ -229,6 +229,7 @@ enum smu_ppt_limit_level { struct smu_ppt_limit_range { uint32_t default_value; + uint32_t current_value; uint32_t min; uint32_t max; uint32_t od_min; -- 2.49.0
