[PATCH] drm/amd/pm: Track current PPT limit for restore

[PATCH] drm/amd/pm: Track current PPT limit for restore

[Lijo Lazar]

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 <lijo.lazar@amd.com>
---
 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

RE: [PATCH] drm/amd/pm: Track current PPT limit for restore

[Wang, Yang(Kevin)]

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 <Lijo.Lazar@amd.com>
> Sent: Thursday, August 6, 2026 2:14 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>;
> Wang, Yang(Kevin) <KevinYang.Wang@amd.com>
> 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 <lijo.lazar@amd.com>
> ---
>  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

Re: [PATCH] drm/amd/pm: Track current PPT limit for restore

[Lazar, Lijo]

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 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.
> 

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,
Lijo

> 
> Best Regards,
> Kevin
> 
>> -----Original Message-----
>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>> Sent: Thursday, August 6, 2026 2:14 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>;
>> Wang, Yang(Kevin) <KevinYang.Wang@amd.com>
>> 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 <lijo.lazar@amd.com>
>> ---
>>   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
> 

RE: [PATCH] drm/amd/pm: Track current PPT limit for restore

[Wang, Yang(Kevin)]

AMD General

> 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.

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.

So What issue you have facing now ?

Best Regards,
Kevin

> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, August 6, 2026 3:48 PM
> To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>
> 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 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.
> >
>
> 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,
> Lijo
>
> >
> > Best Regards,
> > Kevin
> >
> >> -----Original Message-----
> >> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> >> Sent: Thursday, August 6, 2026 2:14 PM
> >> To: amd-gfx@lists.freedesktop.org
> >> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
> >> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>;
> Wang,
> >> Yang(Kevin) <KevinYang.Wang@amd.com>
> >> 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 <lijo.lazar@amd.com>
> >> ---
> >>   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
> >

Re: [PATCH] drm/amd/pm: Track current PPT limit for restore

[Lazar, Lijo]

On 06-Aug-26 1:26 PM, Wang, Yang(Kevin) wrote:
> AMD General
> 
>> 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.
> 
> 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 <Lijo.Lazar@amd.com>
>> Sent: Thursday, August 6, 2026 3:48 PM
>> To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-
>> gfx@lists.freedesktop.org
>> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>
>> 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 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.
>>>
>>
>> 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,
>> Lijo
>>
>>>
>>> Best Regards,
>>> Kevin
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>> Sent: Thursday, August 6, 2026 2:14 PM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
>>>> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>;
>> Wang,
>>>> Yang(Kevin) <KevinYang.Wang@amd.com>
>>>> 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 <lijo.lazar@amd.com>
>>>> ---
>>>>    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
>>>
> 

Re: [PATCH] drm/amd/pm: Track current PPT limit for restore

[Lazar, Lijo]

On 06-Aug-26 1:26 PM, Wang, Yang(Kevin) wrote:
> AMD General
> 
>> 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.
> 
> 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 <Lijo.Lazar@amd.com>
>> Sent: Thursday, August 6, 2026 3:48 PM
>> To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-
>> gfx@lists.freedesktop.org
>> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>
>> 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 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.
>>>
>>
>> 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,
>> Lijo
>>
>>>
>>> Best Regards,
>>> Kevin
>>>
>>>> -----Original Message-----
>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>> Sent: Thursday, August 6, 2026 2:14 PM
>>>> To: amd-gfx@lists.freedesktop.org
>>>> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
>>>> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>;
>> Wang,
>>>> Yang(Kevin) <KevinYang.Wang@amd.com>
>>>> 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 <lijo.lazar@amd.com>
>>>> ---
>>>>    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
>>>
> 

Re: [PATCH] drm/amd/pm: Track current PPT limit for restore

[Lazar, Lijo]

On 06-Aug-26 1:32 PM, Lazar, Lijo wrote:
> 
> 
> On 06-Aug-26 1:26 PM, Wang, Yang(Kevin) wrote:
>> AMD General
>>
>>> 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.
>>
>> 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.
> 

Hi Kevin,

Does this address your concern?

Thanks,
Lijo

>>
>> 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 <Lijo.Lazar@amd.com>
>>> Sent: Thursday, August 6, 2026 3:48 PM
>>> To: Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; amd-
>>> gfx@lists.freedesktop.org
>>> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
>>> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>
>>> 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 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.
>>>>
>>>
>>> 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,
>>> Lijo
>>>
>>>>
>>>> Best Regards,
>>>> Kevin
>>>>
>>>>> -----Original Message-----
>>>>> From: Lazar, Lijo <Lijo.Lazar@amd.com>
>>>>> Sent: Thursday, August 6, 2026 2:14 PM
>>>>> To: amd-gfx@lists.freedesktop.org
>>>>> Cc: Zhang, Hawking <Hawking.Zhang@amd.com>; Deucher, Alexander
>>>>> <Alexander.Deucher@amd.com>; Kamal, Asad <Asad.Kamal@amd.com>;
>>> Wang,
>>>>> Yang(Kevin) <KevinYang.Wang@amd.com>
>>>>> 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 <lijo.lazar@amd.com>
>>>>> ---
>>>>>    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
>>>>
>>
>