RE: [PATCH] drm/amd/pm: Track current PPT limit for restore
"Wang, Yang(Kevin)" <[email protected]> Thu, 6 Aug 2026 07:33:48 +0000
| Newsgroups | org.freedesktop.lists.amd-gfx |
|---|---|
| Message-ID | <BN9PR12MB52110CD725737790A39A3E8682D22@BN9PR12MB5211.namprd12.prod.outlook.com> |
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 an observation; it must not implicitly create or update driver-owned persistent policy. This separation is intentional: the driver keeps immutable capabilities from the PPTable and restores only limits that it successfully programmed itself. Caching a value returned by GetPptLimit would turn a potentially transient or out-of-band PMFW state into a value that the driver replays after suspend or reset. There is no ownership, notification, or synchronization mechanism that makes such a cache authoritative, so it can only become stale. and the existing user_dpm_profile policy *CACHE ALREADY* handles the supported driver-owned update path. If no bug fix or feature improve, please drop this patch. Best 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