In the "active" mode of the amd-pstate driver with performance
governor, the CPPC.min_perf is expected to be the nominal_perf.

However after commit a9b9b4c2a4cd ("cpufreq/amd-pstate: Drop min and
max cached frequencies"), this is not the case when the governor is
switched from performance to powersave and back to performance, and
the CPPC.min_perf will be equal to the scaling_min_freq that was set
for the powersave governor.

This is because prior to commit a9b9b4c2a4cd ("cpufreq/amd-pstate:
Drop min and max cached frequencies"), amd_pstate_epp_update_limit()
would unconditionally call amd_pstate_update_min_max_limit() and the
latter function would enforce the CPPC.min_perf constraint when the
governor is performance.

However, after the aforementioned commit,
amd_pstate_update_min_max_limit() is called by
amd_pstate_epp_update_limit() only when either the
scaling_{min/max}_freq is different from the cached value of
cpudata->{min/max}_limit_freq, which wouldn't have changed on a
governor transition from powersave to performance, thus missing out on
enforcing the CPPC.min_perf constraint for the performance governor.

Fix this by invoking amd_pstate_epp_udpate_limit() not only when the
{min/max} limits have changed from the cached values, but also when
the policy itself has changed.

Fixes: a9b9b4c2a4cd ("cpufreq/amd-pstate: Drop min and max cached frequencies")
Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>
---
 drivers/cpufreq/amd-pstate.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index f3477ab37742..bbb8e18a6e2b 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1547,13 +1547,15 @@ static void amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy)
 	pr_debug("CPU %d exiting\n", policy->cpu);
 }
 
-static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy)
+static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy, bool policy_change)
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
 	union perf_cached perf;
 	u8 epp;
 
-	if (policy->min != cpudata->min_limit_freq || policy->max != cpudata->max_limit_freq)
+	if (policy_change ||
+	    policy->min != cpudata->min_limit_freq ||
+	    policy->max != cpudata->max_limit_freq)
 		amd_pstate_update_min_max_limit(policy);
 
 	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
@@ -1577,7 +1579,7 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
 
 	cpudata->policy = policy->policy;
 
-	ret = amd_pstate_epp_update_limit(policy);
+	ret = amd_pstate_epp_update_limit(policy, true);
 	if (ret)
 		return ret;
 
@@ -1651,7 +1653,7 @@ static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
 		int ret;
 
 		/* enable amd pstate from suspend state*/
-		ret = amd_pstate_epp_update_limit(policy);
+		ret = amd_pstate_epp_update_limit(policy, false);
 		if (ret)
 			return ret;
 
-- 
2.34.1

On 8/20/25 11:26 PM, Gautham R. Shenoy wrote:
> In the "active" mode of the amd-pstate driver with performance
> governor, the CPPC.min_perf is expected to be the nominal_perf.
> 
> However after commit a9b9b4c2a4cd ("cpufreq/amd-pstate: Drop min and
> max cached frequencies"), this is not the case when the governor is
> switched from performance to powersave and back to performance, and
> the CPPC.min_perf will be equal to the scaling_min_freq that was set
> for the powersave governor.
> 
> This is because prior to commit a9b9b4c2a4cd ("cpufreq/amd-pstate:
> Drop min and max cached frequencies"), amd_pstate_epp_update_limit()
> would unconditionally call amd_pstate_update_min_max_limit() and the
> latter function would enforce the CPPC.min_perf constraint when the
> governor is performance.
> 
> However, after the aforementioned commit,
> amd_pstate_update_min_max_limit() is called by
> amd_pstate_epp_update_limit() only when either the
> scaling_{min/max}_freq is different from the cached value of
> cpudata->{min/max}_limit_freq, which wouldn't have changed on a
> governor transition from powersave to performance, thus missing out on
> enforcing the CPPC.min_perf constraint for the performance governor.
> 
> Fix this by invoking amd_pstate_epp_udpate_limit() not only when the
> {min/max} limits have changed from the cached values, but also when
> the policy itself has changed.
> 
> Fixes: a9b9b4c2a4cd ("cpufreq/amd-pstate: Drop min and max cached frequencies")
> Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

I'll add to my testing tree, and if all looks good will send out for 
-fixes later this cycle.

> ---
>   drivers/cpufreq/amd-pstate.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index f3477ab37742..bbb8e18a6e2b 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1547,13 +1547,15 @@ static void amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy)
>   	pr_debug("CPU %d exiting\n", policy->cpu);
>   }
>   
> -static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy)
> +static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy, bool policy_change)
>   {
>   	struct amd_cpudata *cpudata = policy->driver_data;
>   	union perf_cached perf;
>   	u8 epp;
>   
> -	if (policy->min != cpudata->min_limit_freq || policy->max != cpudata->max_limit_freq)
> +	if (policy_change ||
> +	    policy->min != cpudata->min_limit_freq ||
> +	    policy->max != cpudata->max_limit_freq)
>   		amd_pstate_update_min_max_limit(policy);
>   
>   	if (cpudata->policy == CPUFREQ_POLICY_PERFORMANCE)
> @@ -1577,7 +1579,7 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
>   
>   	cpudata->policy = policy->policy;
>   
> -	ret = amd_pstate_epp_update_limit(policy);
> +	ret = amd_pstate_epp_update_limit(policy, true);
>   	if (ret)
>   		return ret;
>   
> @@ -1651,7 +1653,7 @@ static int amd_pstate_epp_resume(struct cpufreq_policy *policy)
>   		int ret;
>   
>   		/* enable amd pstate from suspend state*/
> -		ret = amd_pstate_epp_update_limit(policy);
> +		ret = amd_pstate_epp_update_limit(policy, false);
>   		if (ret)
>   			return ret;
>