Hi Prateek, Mario,

Following discussion, I have dropped the previous second patch (EPP capability
checks) since EPP is supported on all Zen CPUs that support CPPC. The
frequency capping on Zen 2 systems was purely caused by a false cache hit
during driver initialization.

Thus, we consolidate the series to a single patch that fixes the false EPP
cache hit at boot and explicitly toggles AUTO_SEL_ENABLE on shared memory
systems.

If you have a znver2 or 1 under hand please test them as I don't own them.

Changes in v3:
- Patch 1: Cache the firmware-programmed default EPP value at CPU EPP
  initialization (resolving the boot-time false cache hit) and explicitly
  toggle the AUTO_SEL_ENABLE register to 1 on shared memory systems,
  rather than utilizing a state-tracking flag as proposed in v2.
- Patch 2: Dropped as CPPC systems universally support EPP.

Changes in v2:
- Patch 1: Rename `epp_initialized` to `epp_hw_programmed` and add a comment
  documenting the EPP cache guard optimization behavior.
- Patch 2: Add comments explaining the uniform CPU capability check on x86,
  handle EPP capability check errors robustly (only treat -EOPNOTSUPP as
  unsupported, warn and assume supported for other errors to avoid false
  negatives), and reject runtime active mode transitions at sysfs store time
  (preventing the driver from being left in an unregistered state).

Changes in v1:
- Fix the boot-time CPPC EPP/auto_sel initialization regression in
  shmem_set_epp() using a state tracking flag while preserving runtime
  cache optimization.
- Add an EPP capability check helper during initialization.
- Fall back to passive mode at boot if EPP is not supported, and reject
  transitions to active mode at runtime if EPP is not supported.


Marco Scardovi (1):
  cpufreq/amd-pstate: Fix EPP initialization for shared memory systems

 drivers/cpufreq/amd-pstate.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

-- 
2.54.0

Hi Mario, Prateek,

I have submitted v4 of this series to address your feedback:

Changes in v4:
- Refactor EPP getter helper functions (msr_get_epp, shmem_get_epp, and
  amd_pstate_get_epp) to return int, conforming to standard kernel practice
  for value-or-negative-errno helpers.
- Clean up commit message description to link error propagation directly to
  EPP caching.
- Execute the remaining shared-memory initialization path even when booting
  in active mode, rather than bypassing it through an early return.

Changes in v3:
- Patch 1: Cache the firmware-programmed default EPP value at CPU EPP
  initialization (resolving the boot-time false cache hit) and explicitly
  toggle the AUTO_SEL_ENABLE register to 1 on shared memory systems,
  rather than utilizing a state-tracking flag as proposed in v2.
- Patch 2: Dropped as CPPC systems universally support EPP.

Changes in v2:
- Patch 1: Rename `epp_initialized` to `epp_hw_programmed` and add a comment
  documenting the EPP cache guard optimization behavior.
- Patch 2: Add comments explaining the uniform CPU capability check on x86,
  handle EPP capability check errors robustly (only treat -EOPNOTSUPP as
  unsupported, warn and assume supported for other errors to avoid false
  negatives), and reject runtime active mode transitions at sysfs store time
  (preventing the driver from being left in an unregistered state).

Changes in v1:
- Fix the boot-time CPPC EPP/auto_sel initialization regression in
  shmem_set_epp() using a state tracking flag while preserving runtime
  cache optimization.
- Add an EPP capability check helper during initialization.
- Fall back to passive mode at boot if EPP is not supported, and reject
  transitions to active mode at runtime if EPP is not supported.


Marco Scardovi (1):
  cpufreq/amd-pstate: Fix EPP initialization for shared memory systems

 drivers/cpufreq/amd-pstate.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

-- 
2.54.0

At CPU initialization, the private cpudata structure is allocated via
kzalloc, which means cpudata->cppc_req_cached is initialized to 0. This
makes the default cached EPP value 0 (AMD_CPPC_EPP_PERFORMANCE).

When initializing a system that defaults to performance EPP, the driver
attempts to configure the EPP via amd_pstate_set_epp(). Because the
requested EPP (0) matches the uninitialized cached value (0), the cache
guard check triggers, and the driver skips writing to the hardware.

On shared memory systems, the EPP write via cppc_set_epp_perf() is also
responsible for toggling on the autonomous selection register (auto_sel).
Skipping the EPP write consequently skips enabling auto_sel, leaving the
CPU in non-autonomous mode. This prevents the hardware from boosting and
leaves the CPU frequency stuck at the lowest non-linear frequency (1.7GHz).

Fix this by:
1. Caching the firmware programmed default EPP value in cppc_req_cached
   during CPU EPP initialization.
2. Propagating negative error codes from amd_pstate_get_epp() correctly
   while caching the firmware EPP value. Change msr_get_epp() and
   shmem_get_epp() to return signed int instead of u8. Also change
   amd_pstate_get_epp() return type and the local default_epp variable
   in amd_pstate_epp_cpu_init() to int.
3. Executing the remaining shared-memory initialization path even when
   booting in active mode, rather than bypassing it through an early
   return.

Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221473
Assisted-by: Antigravity:gemini-3.5-flash
Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/cpufreq/amd-pstate.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8d55e2be825b..a35cf126e335 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -199,7 +199,7 @@ static inline int get_mode_idx_from_str(const char *str, size_t size)
 
 static DEFINE_MUTEX(amd_pstate_driver_lock);
 
-static u8 msr_get_epp(struct amd_cpudata *cpudata)
+static int msr_get_epp(struct amd_cpudata *cpudata)
 {
 	u64 value;
 	int ret;
@@ -215,12 +215,12 @@ static u8 msr_get_epp(struct amd_cpudata *cpudata)
 
 DEFINE_STATIC_CALL(amd_pstate_get_epp, msr_get_epp);
 
-static inline s16 amd_pstate_get_epp(struct amd_cpudata *cpudata)
+static inline int amd_pstate_get_epp(struct amd_cpudata *cpudata)
 {
 	return static_call(amd_pstate_get_epp)(cpudata);
 }
 
-static u8 shmem_get_epp(struct amd_cpudata *cpudata)
+static int shmem_get_epp(struct amd_cpudata *cpudata)
 {
 	u64 epp;
 	int ret;
@@ -525,10 +525,6 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
 	perf.lowest_perf = cppc_perf.lowest_perf;
 	WRITE_ONCE(cpudata->perf, perf);
 	WRITE_ONCE(cpudata->prefcore_ranking, cppc_perf.highest_perf);
-
-	if (cppc_state == AMD_PSTATE_ACTIVE)
-		return 0;
-
 	ret = cppc_get_auto_sel(cpudata->cpu, &auto_sel);
 	if (ret) {
 		pr_warn("failed to get auto_sel, ret: %d\n", ret);
@@ -1877,6 +1873,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 	struct amd_cpudata *cpudata;
 	union perf_cached perf;
 	struct device *dev;
+	int default_epp;
 	int ret;
 
 	/*
@@ -1926,6 +1923,14 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	policy->boost_supported = READ_ONCE(cpudata->boost_supported);
 
+	/* Cache the firmware programmed EPP */
+	default_epp = amd_pstate_get_epp(cpudata);
+	if (default_epp < 0) {
+		ret = default_epp;
+		goto free_cpudata1;
+	}
+	FIELD_MODIFY(AMD_CPPC_EPP_PERF_MASK, &cpudata->cppc_req_cached, default_epp);
+
 	/*
 	 * Set the policy to provide a valid fallback value in case
 	 * the default cpufreq governor is neither powersave nor performance.
@@ -1933,7 +1938,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 	if (amd_pstate_acpi_pm_profile_server() ||
 	    amd_pstate_acpi_pm_profile_undefined()) {
 		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
-		cpudata->epp_default_ac = cpudata->epp_default_dc = amd_pstate_get_epp(cpudata);
+		cpudata->epp_default_ac = cpudata->epp_default_dc = default_epp;
 		cpudata->current_profile = PLATFORM_PROFILE_PERFORMANCE;
 	} else {
 		policy->policy = CPUFREQ_POLICY_POWERSAVE;
-- 
2.54.0

Hello Marco,

I don't mind the changes, except that it can be broken down into
two patches as Mario suggested for easier backporting :-)

Some notes below ...

On 6/6/2026 12:27 PM, Marco Scardovi wrote:
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 8d55e2be825b..a35cf126e335 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -199,7 +199,7 @@ static inline int get_mode_idx_from_str(const char *str, size_t size)
>  
>  static DEFINE_MUTEX(amd_pstate_driver_lock);
>  
> -static u8 msr_get_epp(struct amd_cpudata *cpudata)
> +static int msr_get_epp(struct amd_cpudata *cpudata)
>  {
>  	u64 value;
>  	int ret;
> @@ -215,12 +215,12 @@ static u8 msr_get_epp(struct amd_cpudata *cpudata)
>  
>  DEFINE_STATIC_CALL(amd_pstate_get_epp, msr_get_epp);
>  
> -static inline s16 amd_pstate_get_epp(struct amd_cpudata *cpudata)
> +static inline int amd_pstate_get_epp(struct amd_cpudata *cpudata)

The change to return type and the amd_pstate_epp_cpu_init() bits
should be Patch 1/2 with:

Fixes: 555bbe67a622 ("cpufreq/amd-pstate: Convert all perf values to u8")

and commit message stating the necessary error handling that is
required if amd_pstate_get_epp() fails.

>  {
>  	return static_call(amd_pstate_get_epp)(cpudata);
>  }
>  
> -static u8 shmem_get_epp(struct amd_cpudata *cpudata)
> +static int shmem_get_epp(struct amd_cpudata *cpudata)
>  {
>  	u64 epp;
>  	int ret;
> @@ -525,10 +525,6 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
>  	perf.lowest_perf = cppc_perf.lowest_perf;
>  	WRITE_ONCE(cpudata->perf, perf);
>  	WRITE_ONCE(cpudata->prefcore_ranking, cppc_perf.highest_perf);
> -
> -	if (cppc_state == AMD_PSTATE_ACTIVE)
> -		return 0;
> -

And this should be Patch 2/2 with:

Fixes: 2dd6d0ebf740 ("cpufreq: amd-pstate: Add guided autonomous mode")

and a commit message that reads it is necessary for AMD_PSTATE_ACTIVE mode
on shared memory system to toggle on AUTO_SEL_ENABLE based on the
ACPI spec for CPPC v2 and below.

>  	ret = cppc_get_auto_sel(cpudata->cpu, &auto_sel);
>  	if (ret) {
>  		pr_warn("failed to get auto_sel, ret: %d\n", ret);
> @@ -1877,6 +1873,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  	struct amd_cpudata *cpudata;
>  	union perf_cached perf;
>  	struct device *dev;
> +	int default_epp;
>  	int ret;
>  
>  	/*
> @@ -1926,6 +1923,14 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  
>  	policy->boost_supported = READ_ONCE(cpudata->boost_supported);
>  
> +	/* Cache the firmware programmed EPP */
> +	default_epp = amd_pstate_get_epp(cpudata);
> +	if (default_epp < 0) {
> +		ret = default_epp;
> +		goto free_cpudata1;
> +	}
> +	FIELD_MODIFY(AMD_CPPC_EPP_PERF_MASK, &cpudata->cppc_req_cached, default_epp);

I would actually prefer this "cppc_req_cached" change to be a third
patch that says caching the initial EPP saves on an unnecessary
reprogramming later when the EPP is first set but I don't mind it
being part of Patch 1 with a small note in the commit message.

> +
>  	/*
>  	 * Set the policy to provide a valid fallback value in case
>  	 * the default cpufreq governor is neither powersave nor performance.
> @@ -1933,7 +1938,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  	if (amd_pstate_acpi_pm_profile_server() ||
>  	    amd_pstate_acpi_pm_profile_undefined()) {
>  		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
> -		cpudata->epp_default_ac = cpudata->epp_default_dc = amd_pstate_get_epp(cpudata);
> +		cpudata->epp_default_ac = cpudata->epp_default_dc = default_epp;
>  		cpudata->current_profile = PLATFORM_PROFILE_PERFORMANCE;
>  	} else {
>  		policy->policy = CPUFREQ_POLICY_POWERSAVE;

-- 
Thanks and Regards,
Prateek