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

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

[Mario Limonciello]

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@amd.com>
---
 drivers/cpufreq/amd-pstate-ut.c | 17 +----------------
 drivers/cpufreq/amd-pstate.c    | 20 ++++++++++++++++----
 drivers/cpufreq/amd-pstate.h    |  4 ++--
 3 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
index 506d7bb2e6609..21313b724945d 100644
--- a/drivers/cpufreq/amd-pstate-ut.c
+++ b/drivers/cpufreq/amd-pstate-ut.c
@@ -392,26 +392,11 @@ static int amd_pstate_ut_epp(u32 index)
 		strreplace(buf, '\n', '\0');
 		/*
 		 * "dynamic" mode reports the EPP as "dynamic(profile:X)"
-		 * Trim at "(" and just compare tie the epp string.
+		 * Trim at "(" and just compare the EPP string.
 		 */
 		strreplace(buf, '(', '\0');
 
 		if (strcmp(buf, energy_perf_strings[epp_modes[i]])) {
-			u8 epp = FIELD_GET(AMD_CPPC_EPP_PERF_MASK, cpudata->cppc_req_cached);
-
-			/*
-			 * The preference read back may legitimately differ from the one
-			 * written: some platforms (e.g. Zen6) program the same raw EPP
-			 * value for more than one named preference, so show() reports the
-			 * first name that matches that value.
-			 *
-			 * If strings mismatch, confirm the CPU is programmed with the correct
-			 * EPP value for the mode. This verifies hardware programming while
-			 * tolerating such aliasing in driver.
-			 */
-			if (epp == amd_pstate_cpu_epp_values(cpudata->cpu_type)[epp_modes[i]])
-				continue;
-
 			pr_err("String EPP value mismatch: %s != %s\n",
 			       buf, energy_perf_strings[epp_modes[i]]);
 			ret = -EINVAL;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 4bedfbfc18b66..318365585b4e0 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -118,7 +118,7 @@ static_assert(ARRAY_SIZE(epp_values) == TOPO_CPU_TYPE_LOW_POWER + 1,
  * Non-hybrid systems use TOPO_CPU_TYPE_ANY; hybrid systems use the CPU's
  * actual type (PERFORMANCE/EFFICIENCY/LOW_POWER).
  */
-u8 *amd_pstate_cpu_epp_values(enum x86_topology_cpu_type cpu_type)
+static inline u8 *amd_pstate_cpu_epp_values(enum x86_topology_cpu_type cpu_type)
 {
 	switch (cpu_type) {
 	case TOPO_CPU_TYPE_PERFORMANCE:
@@ -129,7 +129,6 @@ u8 *amd_pstate_cpu_epp_values(enum x86_topology_cpu_type cpu_type)
 		return epp_values[TOPO_CPU_TYPE_ANY];
 	}
 }
-EXPORT_SYMBOL_FOR_PSTATE_UT(amd_pstate_cpu_epp_values);
 
 /**
  * struct amd_pstate_epp_values - EPP values for the four named preferences
@@ -1474,6 +1473,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
 	u8 *values = amd_pstate_cpu_epp_values(cpudata->cpu_type);
+	u8 epp_index = EPP_INDEX_MAX;
 	ssize_t ret;
 	bool raw_epp = false;
 	u8 epp;
@@ -1507,10 +1507,12 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 			return count;
 		}
 
-		if (ret)
+		if (ret) {
 			epp = values[ret];
-		else
+			epp_index = ret;
+		} else {
 			epp = cpudata->epp_default_dc;
+		}
 	}
 
 	if (epp > 0 && epp != values[EPP_INDEX_PERFORMANCE] &&
@@ -1531,6 +1533,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 		return ret;
 
 	cpudata->raw_epp = raw_epp;
+	cpudata->epp_index = epp_index;
 
 	return count;
 }
@@ -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])
+		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;
 
 	ret = amd_pstate_init_perf(cpudata);
 	if (ret)
diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
index 422188c639b1e..e42ea811d581e 100644
--- a/drivers/cpufreq/amd-pstate.h
+++ b/drivers/cpufreq/amd-pstate.h
@@ -101,6 +101,7 @@ struct amd_aperf_mperf {
  * @dynamic_epp: Whether dynamic EPP is enabled
  * @raw_epp: Whether the last EPP write was a raw numeric value rather than a
  *	     named preference
+ * @epp_index: Index of the last static named EPP preference
  * @power_nb: Notifier block for power events
  * @current_profile: Currently selected platform profile option
  * @ppdev: Device registered with the platform profile handler
@@ -142,6 +143,7 @@ struct amd_cpudata {
 	u8	epp_default_dc;
 	bool	dynamic_epp;
 	bool	raw_epp;
+	u8	epp_index;
 	struct notifier_block power_nb;
 
 	/* platform profile */
@@ -204,8 +206,6 @@ static const char * const energy_perf_strings[] = {
 };
 static_assert(ARRAY_SIZE(energy_perf_strings) == EPP_INDEX_MAX);
 
-u8 *amd_pstate_cpu_epp_values(enum x86_topology_cpu_type cpu_type);
-
 static inline u8 freq_to_perf(union perf_cached perf, u32 nominal_freq, unsigned int freq_val)
 {
 	u32 perf_val = DIV_ROUND_UP_ULL((u64)freq_val * perf.nominal_perf, nominal_freq);

base-commit: 19c2ab5ae8aa9fe92c396a089f67baca85b5f117
-- 
2.43.0

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

Feel free to include:

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

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