mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems
@ 2026-06-05 10:26 Marco Scardovi
  2026-06-05 10:26 ` [PATCH v3 1/1] " Marco Scardovi
  2026-06-06  6:57 ` [PATCH v4 0/1] " Marco Scardovi
  0 siblings, 2 replies; 21+ messages in thread
From: Marco Scardovi @ 2026-06-05 10:26 UTC (permalink / raw)
  To: Mario Limonciello, K Prateek Nayak
  Cc: linux-kernel, linux-pm, perry.yuan, rafael, ray.huang,
	stuartmeckle, viresh.kumar, wyes.karny

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v3 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems
  2026-06-05 10:26 [PATCH v3 0/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems Marco Scardovi
@ 2026-06-05 10:26 ` Marco Scardovi
  2026-06-05 18:24   ` Mario Limonciello
  2026-06-06  6:57 ` [PATCH v4 0/1] " Marco Scardovi
  1 sibling, 1 reply; 21+ messages in thread
From: Marco Scardovi @ 2026-06-05 10:26 UTC (permalink / raw)
  To: Mario Limonciello, K Prateek Nayak
  Cc: linux-kernel, linux-pm, perry.yuan, rafael, ray.huang,
	stuartmeckle, viresh.kumar, wyes.karny

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. Cache the firmware programmed default EPP value in cppc_req_cached
   during CPU EPP initialization.
2. Explicitly toggle the AUTO_SEL_ENABLE register to 1 during EPP CPU
   initialization for shared memory systems, independent of whether the EPP
   write is skipped due to a cache match.

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

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8d55e2be825b..8e0099eba512 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1877,6 +1877,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 	struct amd_cpudata *cpudata;
 	union perf_cached perf;
 	struct device *dev;
+	s16 default_epp;
 	int ret;
 
 	/*
@@ -1926,6 +1927,27 @@ 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);
+
+	/*
+	 * Shared memory based systems may require the AUTO_SEL_ENABLE register
+	 * to be toggled on to function correctly. Since the first call to
+	 * amd_pstate_set_epp() may bail out early if the desired EPP is
+	 * same as the one configured by the firmware, attempt to toggle the
+	 * AUTO_SEL_ENABLE here, independent of EPP programming.
+	 */
+	if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
+		ret = cppc_set_auto_sel(policy->cpu, 1);
+		if (ret)
+			pr_warn("failed to enable auto_sel for cpu %d: %d\n", policy->cpu, ret);
+	}
+
 	/*
 	 * Set the policy to provide a valid fallback value in case
 	 * the default cpufreq governor is neither powersave nor performance.
@@ -1933,7 +1955,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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v3 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems
  2026-06-05 10:26 ` [PATCH v3 1/1] " Marco Scardovi
@ 2026-06-05 18:24   ` Mario Limonciello
  0 siblings, 0 replies; 21+ messages in thread
From: Mario Limonciello @ 2026-06-05 18:24 UTC (permalink / raw)
  To: Marco Scardovi, K Prateek Nayak
  Cc: linux-kernel, linux-pm, perry.yuan, rafael, ray.huang,
	stuartmeckle, viresh.kumar, wyes.karny



On 6/5/26 05:26, Marco Scardovi wrote:
> 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. Cache the firmware programmed default EPP value in cppc_req_cached
>     during CPU EPP initialization.
> 2. Explicitly toggle the AUTO_SEL_ENABLE register to 1 during EPP CPU
>     initialization for shared memory systems, independent of whether the EPP
>     write is skipped due to a cache match.
> 
> Fixes: ffa5096a7c33 ("cpufreq: amd-pstate: implement Pstate EPP support for the AMD processors")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221473
> Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
> Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>
> ---
>   drivers/cpufreq/amd-pstate.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 8d55e2be825b..8e0099eba512 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1877,6 +1877,7 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>   	struct amd_cpudata *cpudata;
>   	union perf_cached perf;
>   	struct device *dev;
> +	s16 default_epp;

Reviewing this code I realize we have a problem with the return types.

Both possible functions return u8:

static u8 msr_get_epp(struct amd_cpudata *cpudata)
static u8 shmem_get_epp(struct amd_cpudata *cpudata)

The static call returns s16:
static inline s16 amd_pstate_get_epp(struct amd_cpudata *cpudata)

But then both shmem_get_epp and msr_get_epp can return negative integers 
on failure.

So the return type for all of them should be changed to be an integer to 
support this change.

Als othe variable used in amd_pstate_epp_cpu_init needs to be an integer 
too.

>   	int ret;
>   
>   	/*
> @@ -1926,6 +1927,27 @@ 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);
> +
> +	/*
> +	 * Shared memory based systems may require the AUTO_SEL_ENABLE register
> +	 * to be toggled on to function correctly. Since the first call to
> +	 * amd_pstate_set_epp() may bail out early if the desired EPP is
> +	 * same as the one configured by the firmware, attempt to toggle the
> +	 * AUTO_SEL_ENABLE here, independent of EPP programming.
> +	 */
> +	if (!cpu_feature_enabled(X86_FEATURE_CPPC)) {
> +		ret = cppc_set_auto_sel(policy->cpu, 1);
> +		if (ret)
> +			pr_warn("failed to enable auto_sel for cpu %d: %d\n", policy->cpu, ret);
> +	}

This doesn't really make sense to me.  I think it's actually pointing to 
a logic error in shmem_init_perf().

I think you can just drop the call to bail in active mode and get the 
same result.

> +
>   	/*
>   	 * Set the policy to provide a valid fallback value in case
>   	 * the default cpufreq governor is neither powersave nor performance.
> @@ -1933,7 +1955,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;


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 0/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems
  2026-06-05 10:26 [PATCH v3 0/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems Marco Scardovi
  2026-06-05 10:26 ` [PATCH v3 1/1] " Marco Scardovi
@ 2026-06-06  6:57 ` Marco Scardovi
  2026-06-06  6:57   ` [PATCH v4 1/1] " Marco Scardovi
  1 sibling, 1 reply; 21+ messages in thread
From: Marco Scardovi @ 2026-06-06  6:57 UTC (permalink / raw)
  To: scardracs
  Cc: kprateek.nayak, linux-kernel, linux-pm, mario.limonciello,
	perry.yuan, rafael, ray.huang, stuartmeckle, viresh.kumar,
	wyes.karny

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v4 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems
  2026-06-06  6:57 ` [PATCH v4 0/1] " Marco Scardovi
@ 2026-06-06  6:57   ` Marco Scardovi
  2026-06-08  4:21     ` K Prateek Nayak
  0 siblings, 1 reply; 21+ messages in thread
From: Marco Scardovi @ 2026-06-06  6:57 UTC (permalink / raw)
  To: scardracs
  Cc: kprateek.nayak, linux-kernel, linux-pm, mario.limonciello,
	perry.yuan, rafael, ray.huang, stuartmeckle, viresh.kumar,
	wyes.karny

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems
  2026-06-06  6:57   ` [PATCH v4 1/1] " Marco Scardovi
@ 2026-06-08  4:21     ` K Prateek Nayak
  2026-06-08  7:31       ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Marco Scardovi
  2026-06-08 16:06       ` [PATCH v4 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems Mario Limonciello
  0 siblings, 2 replies; 21+ messages in thread
From: K Prateek Nayak @ 2026-06-08  4:21 UTC (permalink / raw)
  To: Marco Scardovi
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, stuartmeckle, viresh.kumar, wyes.karny

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization
  2026-06-08  4:21     ` K Prateek Nayak
@ 2026-06-08  7:31       ` Marco Scardovi
  2026-06-08  7:31         ` [PATCH v5 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
                           ` (3 more replies)
  2026-06-08 16:06       ` [PATCH v4 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems Mario Limonciello
  1 sibling, 4 replies; 21+ messages in thread
From: Marco Scardovi @ 2026-06-08  7:31 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

Hi Mario, Prateek,

I don't mind splitting in more patches as far as it works and make things
easier for you on backporting and verification.

Changes in v5:
- Split the unified patch into a 3-patch series:
  - Patch 1: Change EPP helper return types to signed int and add error handling
    in amd_pstate_epp_cpu_init().
  - Patch 2: Remove the active-mode early return check in shmem_init_perf() to
    ensure auto_sel is properly toggled on shared memory systems.
  - Patch 3: Cache the initial firmware programmed EPP value in cppc_req_cached.

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 (3):
  cpufreq/amd-pstate: Fix EPP return type and handle errors during
    initialization
  cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory
    systems
  cpufreq/amd-pstate: Cache the firmware programmed EPP value

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

-- 
2.54.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization
  2026-06-08  7:31       ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Marco Scardovi
@ 2026-06-08  7:31         ` Marco Scardovi
  2026-06-09  4:45           ` K Prateek Nayak
  2026-06-08  7:31         ` [PATCH v5 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems Marco Scardovi
                           ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: Marco Scardovi @ 2026-06-08  7:31 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

Currently, the EPP getter helper functions (msr_get_epp, shmem_get_epp, and
the static call wrapper amd_pstate_get_epp) return u8 or s16. This makes it
difficult to correctly propagate negative error values returned by the
underlying MSR read or CPPC helpers (such as rdmsrq_on_cpu or
cppc_get_epp_perf).

Modify the return type of these functions to int, allowing them to return
negative error codes properly.

Additionally, in amd_pstate_epp_cpu_init(), fetch the firmware-programmed
default EPP value and validate it before assigning it to the EPP variables.
If amd_pstate_get_epp() returns an error code, propagate the error and abort
the CPU initialization to prevent subsequent configuration failures.

Fixes: 555bbe67a622 ("cpufreq/amd-pstate: Convert all perf values to u8")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/cpufreq/amd-pstate.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 8d55e2be825b..77261c31d81d 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;
@@ -1877,6 +1877,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 +1927,13 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	policy->boost_supported = READ_ONCE(cpudata->boost_supported);
 
+	/* Fetch the firmware programmed default EPP value */
+	default_epp = amd_pstate_get_epp(cpudata);
+	if (default_epp < 0) {
+		ret = default_epp;
+		goto free_cpudata1;
+	}
+
 	/*
 	 * Set the policy to provide a valid fallback value in case
 	 * the default cpufreq governor is neither powersave nor performance.
@@ -1933,7 +1941,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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems
  2026-06-08  7:31       ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Marco Scardovi
  2026-06-08  7:31         ` [PATCH v5 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
@ 2026-06-08  7:31         ` Marco Scardovi
  2026-06-09  4:47           ` K Prateek Nayak
  2026-06-08  7:31         ` [PATCH v5 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value Marco Scardovi
  2026-06-09  7:18         ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization K Prateek Nayak
  3 siblings, 1 reply; 21+ messages in thread
From: Marco Scardovi @ 2026-06-08  7:31 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

On shared memory systems, the EPP configuration path (handled via
cppc_set_epp_perf()) is responsible for toggling on the CPPC autonomous
selection register (auto_sel).

Currently, shmem_init_perf() returns early without doing any of the auto_sel
configuration steps if cppc_state is AMD_PSTATE_ACTIVE. This skips enabling
auto_sel, leaving the CPU in non-autonomous mode.

Remove the early return check in shmem_init_perf() when cppc_state is
AMD_PSTATE_ACTIVE. Toggling auto_sel is necessary for the active mode on
shared memory systems to function based on the ACPI spec for CPPC v2 and
below.

Fixes: 2dd6d0ebf740 ("cpufreq: amd-pstate: Add guided autonomous mode")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/cpufreq/amd-pstate.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 77261c31d81d..7982f9d0f634 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -526,9 +526,6 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
 	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);
-- 
2.54.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v5 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value
  2026-06-08  7:31       ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Marco Scardovi
  2026-06-08  7:31         ` [PATCH v5 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
  2026-06-08  7:31         ` [PATCH v5 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems Marco Scardovi
@ 2026-06-08  7:31         ` Marco Scardovi
  2026-06-09  4:50           ` K Prateek Nayak
  2026-06-09  7:18         ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization K Prateek Nayak
  3 siblings, 1 reply; 21+ messages in thread
From: Marco Scardovi @ 2026-06-08  7:31 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

At CPU EPP 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, skipping the EPP write consequently skips
enabling auto_sel, leaving the CPU in non-autonomous mode.

Cache the firmware-programmed default EPP value in cppc_req_cached during
CPU EPP initialization. This saves on an unnecessary reprogramming later
when the EPP is first set.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/cpufreq/amd-pstate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 7982f9d0f634..d1ed53a0f119 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1924,12 +1924,13 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	policy->boost_supported = READ_ONCE(cpudata->boost_supported);
 
-	/* Fetch the firmware programmed default EPP value */
+	/* 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
-- 
2.54.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v4 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems
  2026-06-08  4:21     ` K Prateek Nayak
  2026-06-08  7:31       ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Marco Scardovi
@ 2026-06-08 16:06       ` Mario Limonciello
  1 sibling, 0 replies; 21+ messages in thread
From: Mario Limonciello @ 2026-06-08 16:06 UTC (permalink / raw)
  To: K Prateek Nayak, Marco Scardovi
  Cc: linux-kernel, linux-pm, perry.yuan, rafael, ray.huang,
	stuartmeckle, viresh.kumar, wyes.karny



On 6/7/26 23:21, K Prateek Nayak wrote:
> Hello Marco,
> 
> I don't mind the changes, except that it can be broken down into
> two patches as Mario suggested for easier backporting :-)

Totally agree. All the technical stuff I'm happy with too, just split it up.

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization
  2026-06-08  7:31         ` [PATCH v5 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
@ 2026-06-09  4:45           ` K Prateek Nayak
  0 siblings, 0 replies; 21+ messages in thread
From: K Prateek Nayak @ 2026-06-09  4:45 UTC (permalink / raw)
  To: Marco Scardovi
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, stuartmeckle, viresh.kumar

Hello Marco,

On 6/8/2026 1:01 PM, Marco Scardovi wrote:
> Currently, the EPP getter helper functions (msr_get_epp, shmem_get_epp, and
> the static call wrapper amd_pstate_get_epp) return u8 or s16. This makes it
> difficult to correctly propagate negative error values returned by the
> underlying MSR read or CPPC helpers (such as rdmsrq_on_cpu or
> cppc_get_epp_perf).
> 
> Modify the return type of these functions to int, allowing them to return
> negative error codes properly.
> 
> Additionally, in amd_pstate_epp_cpu_init(), fetch the firmware-programmed
> default EPP value and validate it before assigning it to the EPP variables.
> If amd_pstate_get_epp() returns an error code, propagate the error and abort
> the CPU initialization to prevent subsequent configuration failures.
> 
> Fixes: 555bbe67a622 ("cpufreq/amd-pstate: Convert all perf values to u8")
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>

Thank you for your patience during the debug and helping chase these
issues down. Feel free to include:

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

-- 
Thanks and Regards,
Prateek


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems
  2026-06-08  7:31         ` [PATCH v5 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems Marco Scardovi
@ 2026-06-09  4:47           ` K Prateek Nayak
  0 siblings, 0 replies; 21+ messages in thread
From: K Prateek Nayak @ 2026-06-09  4:47 UTC (permalink / raw)
  To: Marco Scardovi
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, stuartmeckle, viresh.kumar

Hello Marco,

On 6/8/2026 1:01 PM, Marco Scardovi wrote:
> On shared memory systems, the EPP configuration path (handled via
> cppc_set_epp_perf()) is responsible for toggling on the CPPC autonomous
> selection register (auto_sel).
> 
> Currently, shmem_init_perf() returns early without doing any of the auto_sel
> configuration steps if cppc_state is AMD_PSTATE_ACTIVE. This skips enabling
> auto_sel, leaving the CPU in non-autonomous mode.
> 
> Remove the early return check in shmem_init_perf() when cppc_state is
> AMD_PSTATE_ACTIVE. Toggling auto_sel is necessary for the active mode on
> shared memory systems to function based on the ACPI spec for CPPC v2 and
> below.
> 
> Fixes: 2dd6d0ebf740 ("cpufreq: amd-pstate: Add guided autonomous mode")
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>

Feel free to include:

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

-- 
Thanks and Regards,
Prateek


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value
  2026-06-08  7:31         ` [PATCH v5 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value Marco Scardovi
@ 2026-06-09  4:50           ` K Prateek Nayak
  0 siblings, 0 replies; 21+ messages in thread
From: K Prateek Nayak @ 2026-06-09  4:50 UTC (permalink / raw)
  To: Marco Scardovi
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, stuartmeckle, viresh.kumar

Hello Marco,

On 6/8/2026 1:01 PM, Marco Scardovi wrote:
> At CPU EPP 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, skipping the EPP write consequently skips
> enabling auto_sel, leaving the CPU in non-autonomous mode.

nit. this paragraph is unnecessary since you've fixed it in previous
patch but Mario can drop it when he is including it in his tree so
no biggie.

> 
> Cache the firmware-programmed default EPP value in cppc_req_cached during
> CPU EPP initialization. This saves on an unnecessary reprogramming later
> when the EPP is first set.
> 
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Marco Scardovi <scardracs@disroot.org>

Feel free to include:

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

-- 
Thanks and Regards,
Prateek


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization
  2026-06-08  7:31       ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Marco Scardovi
                           ` (2 preceding siblings ...)
  2026-06-08  7:31         ` [PATCH v5 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value Marco Scardovi
@ 2026-06-09  7:18         ` K Prateek Nayak
  2026-06-09  7:29           ` [PATCH v6 " Marco Scardovi
  3 siblings, 1 reply; 21+ messages in thread
From: K Prateek Nayak @ 2026-06-09  7:18 UTC (permalink / raw)
  To: Marco Scardovi
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, stuartmeckle, viresh.kumar

Hello Marco,

On 6/8/2026 1:01 PM, Marco Scardovi wrote:
> Hi Mario, Prateek,
> 
> I don't mind splitting in more patches as far as it works and make things
> easier for you on backporting and verification.
> 
> Changes in v5:
> - Split the unified patch into a 3-patch series:
>   - Patch 1: Change EPP helper return types to signed int and add error handling
>     in amd_pstate_epp_cpu_init().
>   - Patch 2: Remove the active-mode early return check in shmem_init_perf() to
>     ensure auto_sel is properly toggled on shared memory systems.
>   - Patch 3: Cache the initial firmware programmed EPP value in cppc_req_cached.

Thank you! I just tested this on a Zen2 system and everything looks
great. For the whole series:

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

At least this time I didn't copypasta my email ;-)

-- 
Thanks and Regards,
Prateek


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization
  2026-06-09  7:18         ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization K Prateek Nayak
@ 2026-06-09  7:29           ` Marco Scardovi
  2026-06-09  7:29             ` [PATCH v6 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
                               ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Marco Scardovi @ 2026-06-09  7:29 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

Hi Mario, Prateek,

no big deal making a v6 with your suggestions. I'm glad this series come to an end
and everything works as expected.

Here is v6 of the series addressing the feedback on EPP and auto_sel initialization.
This version updates the metadata with Reviewed-by and Tested-by tags and applies
a small commit message cleanup.

Changes in v6:
- Add K Prateek Nayak's Reviewed-by and Tested-by tags to all patches.
- Drop the redundant paragraph from the commit message of Patch 3.

Changes in v5:
- Split the single unified patch into a 3-patch series:
  - Patch 1: Change EPP helper return types to signed int and add error handling
    in amd_pstate_epp_cpu_init().
  - Patch 2: Remove the active-mode early return check in shmem_init_perf() to
    ensure auto_sel is properly toggled on shared memory systems.
  - Patch 3: Cache the initial firmware programmed EPP value in cppc_req_cached.

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 (3):
  cpufreq/amd-pstate: Fix EPP return type and handle errors during
    initialization
  cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory
    systems
  cpufreq/amd-pstate: Cache the firmware programmed EPP value

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

-- 
2.54.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization
  2026-06-09  7:29           ` [PATCH v6 " Marco Scardovi
@ 2026-06-09  7:29             ` Marco Scardovi
  2026-06-09  7:29             ` [PATCH v6 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems Marco Scardovi
                               ` (2 subsequent siblings)
  3 siblings, 0 replies; 21+ messages in thread
From: Marco Scardovi @ 2026-06-09  7:29 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

Currently, the EPP getter helper functions (msr_get_epp, shmem_get_epp, and
the static call wrapper amd_pstate_get_epp) return u8 or s16. This makes it
difficult to correctly propagate negative error values returned by the
underlying MSR read or CPPC helpers (such as rdmsrq_on_cpu or
cppc_get_epp_perf).

Modify the return type of these functions to int, allowing them to return
negative error codes properly.

Additionally, in amd_pstate_epp_cpu_init(), fetch the firmware-programmed
default EPP value and validate it before assigning it to the EPP variables.
If amd_pstate_get_epp() returns an error code, propagate the error and abort
the CPU initialization to prevent subsequent configuration failures.

Fixes: 555bbe67a622 ("cpufreq/amd-pstate: Convert all perf values to u8")
Assisted-by: Antigravity:gemini-3.5-flash
Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/cpufreq/amd-pstate.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index afb040c04f38..a04ff2bad4dd 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;
@@ -1877,6 +1877,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 +1927,13 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	policy->boost_supported = READ_ONCE(cpudata->boost_supported);
 
+	/* Fetch the firmware programmed default EPP value */
+	default_epp = amd_pstate_get_epp(cpudata);
+	if (default_epp < 0) {
+		ret = default_epp;
+		goto free_cpudata1;
+	}
+
 	/*
 	 * Set the policy to provide a valid fallback value in case
 	 * the default cpufreq governor is neither powersave nor performance.
@@ -1933,7 +1941,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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems
  2026-06-09  7:29           ` [PATCH v6 " Marco Scardovi
  2026-06-09  7:29             ` [PATCH v6 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
@ 2026-06-09  7:29             ` Marco Scardovi
  2026-06-09  7:29             ` [PATCH v6 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value Marco Scardovi
  2026-06-09 13:48             ` [PATCH v6 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Mario Limonciello
  3 siblings, 0 replies; 21+ messages in thread
From: Marco Scardovi @ 2026-06-09  7:29 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

On shared memory systems, the EPP configuration path (handled via
cppc_set_epp_perf()) is responsible for toggling on the CPPC autonomous
selection register (auto_sel).

Currently, shmem_init_perf() returns early without doing any of the auto_sel
configuration steps if cppc_state is AMD_PSTATE_ACTIVE. This skips enabling
auto_sel, leaving the CPU in non-autonomous mode.

Remove the early return check in shmem_init_perf() when cppc_state is
AMD_PSTATE_ACTIVE. Toggling auto_sel is necessary for the active mode on
shared memory systems to function based on the ACPI spec for CPPC v2 and
below.

Fixes: 2dd6d0ebf740 ("cpufreq: amd-pstate: Add guided autonomous mode")
Assisted-by: Antigravity:gemini-3.5-flash
Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/cpufreq/amd-pstate.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index a04ff2bad4dd..c67e02f77059 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -526,9 +526,6 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
 	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);
-- 
2.54.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v6 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value
  2026-06-09  7:29           ` [PATCH v6 " Marco Scardovi
  2026-06-09  7:29             ` [PATCH v6 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
  2026-06-09  7:29             ` [PATCH v6 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems Marco Scardovi
@ 2026-06-09  7:29             ` Marco Scardovi
  2026-06-09 13:48             ` [PATCH v6 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Mario Limonciello
  3 siblings, 0 replies; 21+ messages in thread
From: Marco Scardovi @ 2026-06-09  7:29 UTC (permalink / raw)
  To: kprateek.nayak
  Cc: linux-kernel, linux-pm, mario.limonciello, perry.yuan, rafael,
	ray.huang, scardracs, stuartmeckle, viresh.kumar

At CPU EPP 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.

Cache the firmware-programmed default EPP value in cppc_req_cached during
CPU EPP initialization. This saves on an unnecessary reprogramming later
when the EPP is first set.

Assisted-by: Antigravity:gemini-3.5-flash
Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Marco Scardovi <scardracs@disroot.org>
---
 drivers/cpufreq/amd-pstate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index c67e02f77059..a30f6fff72a0 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -1924,12 +1924,13 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	policy->boost_supported = READ_ONCE(cpudata->boost_supported);
 
-	/* Fetch the firmware programmed default EPP value */
+	/* 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
-- 
2.54.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization
  2026-06-09  7:29           ` [PATCH v6 " Marco Scardovi
                               ` (2 preceding siblings ...)
  2026-06-09  7:29             ` [PATCH v6 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value Marco Scardovi
@ 2026-06-09 13:48             ` Mario Limonciello
  2026-06-10  7:09               ` Marco Scardovi
  3 siblings, 1 reply; 21+ messages in thread
From: Mario Limonciello @ 2026-06-09 13:48 UTC (permalink / raw)
  To: Marco Scardovi, kprateek.nayak
  Cc: linux-kernel, linux-pm, perry.yuan, rafael, ray.huang,
	stuartmeckle, viresh.kumar



On 6/9/26 02:29, Marco Scardovi wrote:
> Hi Mario, Prateek,
> 
> no big deal making a v6 with your suggestions. I'm glad this series come to an end
> and everything works as expected.
> 
> Here is v6 of the series addressing the feedback on EPP and auto_sel initialization.
> This version updates the metadata with Reviewed-by and Tested-by tags and applies
> a small commit message cleanup.
> 
> Changes in v6:
> - Add K Prateek Nayak's Reviewed-by and Tested-by tags to all patches.
> - Drop the redundant paragraph from the commit message of Patch 3.
> 
> Changes in v5:
> - Split the single unified patch into a 3-patch series:
>    - Patch 1: Change EPP helper return types to signed int and add error handling
>      in amd_pstate_epp_cpu_init().
>    - Patch 2: Remove the active-mode early return check in shmem_init_perf() to
>      ensure auto_sel is properly toggled on shared memory systems.
>    - Patch 3: Cache the initial firmware programmed EPP value in cppc_req_cached.
> 
> 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 (3):
>    cpufreq/amd-pstate: Fix EPP return type and handle errors during
>      initialization
>    cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory
>      systems
>    cpufreq/amd-pstate: Cache the firmware programmed EPP value
> 
>   drivers/cpufreq/amd-pstate.c | 20 +++++++++++++-------
>   1 file changed, 13 insertions(+), 7 deletions(-)
> 

Thanks, this all looks good now.  Appreciate the patience, and thanks 
for taking all the feedback into account.

I will add it to my tree and it will go for next cycle (7.3).

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v6 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization
  2026-06-09 13:48             ` [PATCH v6 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Mario Limonciello
@ 2026-06-10  7:09               ` Marco Scardovi
  0 siblings, 0 replies; 21+ messages in thread
From: Marco Scardovi @ 2026-06-10  7:09 UTC (permalink / raw)
  To: kprateek.nayak, Mario Limonciello
  Cc: linux-kernel, linux-pm, perry.yuan, rafael, ray.huang,
	stuartmeckle, viresh.kumar

In data martedì 9 giugno 2026 15:48:57 Ora legale dell’Europa centrale, Mario 
Limonciello ha scritto:
> On 6/9/26 02:29, Marco Scardovi wrote:
> > Hi Mario, Prateek,
> > 
> > no big deal making a v6 with your suggestions. I'm glad this series come
> > to an end and everything works as expected.
> > 
> > Here is v6 of the series addressing the feedback on EPP and auto_sel
> > initialization. This version updates the metadata with Reviewed-by and
> > Tested-by tags and applies a small commit message cleanup.
> > 
> > Changes in v6:
> > - Add K Prateek Nayak's Reviewed-by and Tested-by tags to all patches.
> > - Drop the redundant paragraph from the commit message of Patch 3.
> > 
> > Changes in v5:
> > 
> > - Split the single unified patch into a 3-patch series:
> >    - Patch 1: Change EPP helper return types to signed int and add error
> >    handling>    
> >      in amd_pstate_epp_cpu_init().
> >    
> >    - Patch 2: Remove the active-mode early return check in
> >    shmem_init_perf() to>    
> >      ensure auto_sel is properly toggled on shared memory systems.
> >    
> >    - Patch 3: Cache the initial firmware programmed EPP value in
> >    cppc_req_cached.> 
> > 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 (3):
> >    cpufreq/amd-pstate: Fix EPP return type and handle errors during
> >    
> >      initialization
> >    
> >    cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory
> >    
> >      systems
> >    
> >    cpufreq/amd-pstate: Cache the firmware programmed EPP value
> >   
> >   drivers/cpufreq/amd-pstate.c | 20 +++++++++++++-------
> >   1 file changed, 13 insertions(+), 7 deletions(-)
> 
> Thanks, this all looks good now.  Appreciate the patience, and thanks
> for taking all the feedback into account.
> 
> I will add it to my tree and it will go for next cycle (7.3).

Well, thank both of you in first place for the patience in helping me figuring 
it out and testing my patches.




^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2026-06-10  7:09 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-05 10:26 [PATCH v3 0/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems Marco Scardovi
2026-06-05 10:26 ` [PATCH v3 1/1] " Marco Scardovi
2026-06-05 18:24   ` Mario Limonciello
2026-06-06  6:57 ` [PATCH v4 0/1] " Marco Scardovi
2026-06-06  6:57   ` [PATCH v4 1/1] " Marco Scardovi
2026-06-08  4:21     ` K Prateek Nayak
2026-06-08  7:31       ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Marco Scardovi
2026-06-08  7:31         ` [PATCH v5 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
2026-06-09  4:45           ` K Prateek Nayak
2026-06-08  7:31         ` [PATCH v5 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems Marco Scardovi
2026-06-09  4:47           ` K Prateek Nayak
2026-06-08  7:31         ` [PATCH v5 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value Marco Scardovi
2026-06-09  4:50           ` K Prateek Nayak
2026-06-09  7:18         ` [PATCH v5 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization K Prateek Nayak
2026-06-09  7:29           ` [PATCH v6 " Marco Scardovi
2026-06-09  7:29             ` [PATCH v6 1/3] cpufreq/amd-pstate: Fix EPP return type and handle errors during initialization Marco Scardovi
2026-06-09  7:29             ` [PATCH v6 2/3] cpufreq/amd-pstate: Toggle auto_sel in active mode on shared memory systems Marco Scardovi
2026-06-09  7:29             ` [PATCH v6 3/3] cpufreq/amd-pstate: Cache the firmware programmed EPP value Marco Scardovi
2026-06-09 13:48             ` [PATCH v6 0/3] cpufreq/amd-pstate: Fix EPP and auto_sel initialization Mario Limonciello
2026-06-10  7:09               ` Marco Scardovi
2026-06-08 16:06       ` [PATCH v4 1/1] cpufreq/amd-pstate: Fix EPP initialization for shared memory systems Mario Limonciello

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox