Re: [PATCH] cpufreq: amd-pstate: Preserve names for aliased EPP values

[K Prateek Nayak]

Hello Mario,

On 10/13/2026 2:25 AM, Mario Limonciello wrote:
> Zen6 low-power cores use the same numeric EPP value for the
> balance_performance, balance_power, and power preferences. The show
> callback currently scans the numeric values and returns the first matching
> name, so writing balance_power or power reads back as
> balance_performance.
> 
> Cache the index of the last successfully written static named preference.
> Return that name while the cached numeric EPP still agrees, retaining the
> numeric lookup as a fallback for boot, firmware, default, and dynamic
> states.
> 
> Make the unit test require exact string round-tripping again.
> 
> Fixes: 52763c00945a ("cpufreq: amd-pstate: Update Zen6 client EPP tuning values")
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>

Feel free to include:

Reviewed-by: K Prateek Nayak <kprateek.nayak@xxxxxxx>
Tested-by: K Prateek Nayak <kprateek.nayak@xxxxxxx>

One small nit. below:


> @@ -1548,6 +1551,14 @@ ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *
>  	if (!cpudata->dynamic_epp && cpudata->raw_epp)
>  		return sysfs_emit(buf, "%u\n", epp);
>  
> +	/* Several named preferences may share the same numeric EPP value. */
> +	if (!cpudata->dynamic_epp &&
> +	    cpudata->epp_index >= EPP_INDEX_PERFORMANCE &&
> +	    cpudata->epp_index <= EPP_INDEX_POWERSAVE &&
> +	    epp == values[cpudata->epp_index])

That checks seems overly defensive. And I have a hunch ...

> +		return sysfs_emit(buf, "%s\n",
> +				  energy_perf_strings[cpudata->epp_index]);
> +
>  	/*
>  	 * Map the cached EPP value back to a named preference. Skip the
>  	 * "default" slot (index 0) so an EPP of 0 reports as "performance".
> @@ -1980,6 +1991,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  
>  	cpudata->cpu = policy->cpu;
>  	cpudata->cpu_type = cpu_data(policy->cpu).topo.cpu_type;
> +	cpudata->epp_index = EPP_INDEX_MAX;

... it is mostly to handle this case. Can we instead initialize it to
the correct index and say if epp_index != EPP_INDEX_MAX, use the
index to derive string always?

Something like below squashed into your patch:

  (Tested with amd-pstate-ut on top of your patch)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8eebba922219..1f64726348bc 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1504,7 +1504,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 			if (ret)
 				return ret;
 
-			return count;
+			goto success;
 		}
 
 		if (ret) {
@@ -1533,6 +1533,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 		return ret;
 
 	cpudata->raw_epp = raw_epp;
+success:
 	cpudata->epp_index = epp_index;
 
 	return count;
@@ -1552,10 +1553,7 @@ ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *
 		return sysfs_emit(buf, "%u\n", epp);
 
 	/* Several named preferences may share the same numeric EPP value. */
-	if (!cpudata->dynamic_epp &&
-	    cpudata->epp_index >= EPP_INDEX_PERFORMANCE &&
-	    cpudata->epp_index <= EPP_INDEX_POWERSAVE &&
-	    epp == values[cpudata->epp_index])
+	if (cpudata->epp_index != EPP_INDEX_MAX)
 		return sysfs_emit(buf, "%s\n",
 				  energy_perf_strings[cpudata->epp_index]);
 
@@ -1980,7 +1978,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	cpudata->cpu = policy->cpu;
 	cpudata->cpu_type = cpu_data(policy->cpu).topo.cpu_type;
-	cpudata->epp_index = EPP_INDEX_MAX;
 
 	ret = amd_pstate_init_perf(cpudata);
 	if (ret)
@@ -2030,6 +2027,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
 		cpudata->epp_default_ac = cpudata->epp_default_dc = default_epp;
 		cpudata->current_profile = PLATFORM_PROFILE_PERFORMANCE;
+		cpudata->epp_index = EPP_INDEX_PERFORMANCE;
 	} else {
 		u8 *values = amd_pstate_cpu_epp_values(cpudata->cpu_type);
 
@@ -2037,6 +2035,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 		cpudata->epp_default_ac = values[EPP_INDEX_PERFORMANCE];
 		cpudata->epp_default_dc = values[EPP_INDEX_BALANCE_PERFORMANCE];
 		cpudata->current_profile = PLATFORM_PROFILE_BALANCED;
+		cpudata->epp_index = EPP_INDEX_BALANCE_PERFORMANCE;
 	}
 
 	ret = amd_pstate_set_epp(policy, cpudata->epp_default_dc);
-- 
Thanks and Regards,
Prateek