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