Linux Power Management development
 help / color / mirror / Atom feed
* [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP
@ 2026-10-27 14:37 Zak
  2026-10-27 15:56 ` Rafael J. Wysocki (Intel)
  2026-10-27 16:36 ` Doug Smythies
  0 siblings, 2 replies; 8+ messages in thread
From: Zak @ 2026-10-27 14:37 UTC (permalink / raw)
  To: linux-pm, Rafael J. Wysocki, Srinivas Pandruvada
  Cc: Len Brown, Viresh Kumar, linux-kernel, Zak Brighton Knight,
	stable

From: Zak Brighton Knight <zakbrightonknight@gmail.com>

On a ThinkPad T520 with an i7-3840QM (Ivy Bridge) running coreboot,
the CPUs stop using turbo after every suspend to RAM when intel_pstate
is in passive mode with the performance governor: they stay at the
2.8 GHz base frequency instead of boosting up to 3.8 GHz, until the
policy limits change or the system is rebooted.

During resume, the firmware sets each CPU's P-state request register,
MSR_IA32_PERF_CTL, to the base ratio (28), and the kernel never writes
the requested turbo ratio (38) again.  Without hardware-managed
P-states (HWP), intel_pstate caches the last P-state it wrote and
skips writes that would not change it.  In passive mode the cpufreq
core also skips requests for the frequency it believes is already set
(policy->cur), because intel_cpufreq sets CPUFREQ_NEED_UPDATE_LIMITS
only with HWP.  Neither cached value is updated across suspend and
resume, so as long as the governor keeps requesting the same P-state,
as the performance governor always does, the register is never
rewritten.  Governors that change the request often, such as
schedutil, mostly hide the problem.

Fix this by writing the cached P-state back to the register in
intel_pstate_resume() when HWP is not used, as is already done for
the HWP request when it is.

The problem was found and root-caused, and this patch was drafted, with
the help of an AI coding assistant (Claude, by Anthropic) during a
performance review of the machine described above.

Fixes: 001c76f05b01 ("cpufreq: intel_pstate: Generic governors support")
Cc: stable@vger.kernel.org
Signed-off-by: Zak Brighton Knight <zakbrightonknight@gmail.com>
---
Tested on the T520 (i7-3840QM, no HWP, coreboot 24.08 with SeaBIOS)
with v7.3 plus this patch and a temporary module parameter, not part
of this patch, to turn the fix off.  intel_pstate.c is the same in
v7.3 and in the base commit below.

Each row is one suspend to RAM and resume (rtcwake -m mem -s 25).
"Request" is the P-state read back from PERF_CTL on all eight CPUs
5 and 17 seconds after resume; "clock" is the measured speed of a
single-threaded busy loop.  Before every suspend, all CPUs requested
38 and the clock was about 3.8 GHz.

  mode     governor     fix  request after resume  clock
  passive  performance  on   38 on all CPUs        3.8 GHz
  passive  performance  off  28 on all CPUs        2.8 GHz
  active   performance  off  38 on all CPUs        3.8 GHz
  active   performance  on   38 on all CPUs        3.8 GHz

The passive-mode failure was also reproduced twice on Fedora's 7.2.11
kernel.  In active mode the request was back at 38 after resume even
without the fix, so on this machine the fix only makes a difference in
passive mode, the default without HWP.

Not tested: systems with HWP (their code path is unchanged, only
restructured), other machines and firmware, and suspend-to-idle.  The
patch adds no warnings with W=1; sparse was not run.  The tests were
scripted and run by the same AI assistant.

 drivers/cpufreq/intel_pstate.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 17158cb..53db91a 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1371,6 +1371,7 @@ static void set_power_ctl_ee_state(bool input)
 }
 
 static void intel_pstate_hwp_enable(struct cpudata *cpudata);
+static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate);
 
 static void intel_pstate_hwp_reenable(struct cpudata *cpu)
 {
@@ -1404,11 +1405,20 @@ static int intel_pstate_resume(struct cpufreq_policy *policy)
 	else if (power_ctl_ee_state == POWER_CTL_EE_DISABLE)
 		set_power_ctl_ee_state(false);
 
-	if (cpu->suspended && hwp_active) {
+	if (cpu->suspended) {
 		mutex_lock(&intel_pstate_limits_lock);
 
-		/* Re-enable HWP, because "online" has not done that. */
-		intel_pstate_hwp_reenable(cpu);
+		if (hwp_active) {
+			/* Re-enable HWP, because "online" has not done that. */
+			intel_pstate_hwp_reenable(cpu);
+		} else {
+			/*
+			 * PERF_CTL may have been changed by the platform
+			 * firmware, but it is only written when the requested
+			 * P-state changes, so restore it explicitly.
+			 */
+			intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate);
+		}
 
 		mutex_unlock(&intel_pstate_limits_lock);
 	}

base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
-- 
2.56.0


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

* Re: [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP
  2026-10-27 14:37 [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP Zak
@ 2026-10-27 15:56 ` Rafael J. Wysocki (Intel)
  2026-10-27 16:36 ` Doug Smythies
  1 sibling, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-27 15:56 UTC (permalink / raw)
  To: Zak
  Cc: linux-pm, Rafael J. Wysocki, Srinivas Pandruvada, Len Brown,
	Viresh Kumar, linux-kernel, stable

On Tue, Oct 27, 2026 at 3:39 PM Zak <zakbrightonknight@gmail.com> wrote:
>
> From: Zak Brighton Knight <zakbrightonknight@gmail.com>
>
> On a ThinkPad T520 with an i7-3840QM (Ivy Bridge) running coreboot,
> the CPUs stop using turbo after every suspend to RAM when intel_pstate
> is in passive mode with the performance governor: they stay at the
> 2.8 GHz base frequency instead of boosting up to 3.8 GHz, until the
> policy limits change or the system is rebooted.
>
> During resume, the firmware sets each CPU's P-state request register,
> MSR_IA32_PERF_CTL, to the base ratio (28), and the kernel never writes
> the requested turbo ratio (38) again.  Without hardware-managed
> P-states (HWP), intel_pstate caches the last P-state it wrote and
> skips writes that would not change it.  In passive mode the cpufreq
> core also skips requests for the frequency it believes is already set
> (policy->cur), because intel_cpufreq sets CPUFREQ_NEED_UPDATE_LIMITS
> only with HWP.  Neither cached value is updated across suspend and
> resume, so as long as the governor keeps requesting the same P-state,
> as the performance governor always does, the register is never
> rewritten.  Governors that change the request often, such as
> schedutil, mostly hide the problem.
>
> Fix this by writing the cached P-state back to the register in
> intel_pstate_resume() when HWP is not used, as is already done for
> the HWP request when it is.
>
> The problem was found and root-caused, and this patch was drafted, with
> the help of an AI coding assistant (Claude, by Anthropic) during a
> performance review of the machine described above.
>
> Fixes: 001c76f05b01 ("cpufreq: intel_pstate: Generic governors support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zak Brighton Knight <zakbrightonknight@gmail.com>
> ---
> Tested on the T520 (i7-3840QM, no HWP, coreboot 24.08 with SeaBIOS)
> with v7.3 plus this patch and a temporary module parameter, not part
> of this patch, to turn the fix off.  intel_pstate.c is the same in
> v7.3 and in the base commit below.
>
> Each row is one suspend to RAM and resume (rtcwake -m mem -s 25).
> "Request" is the P-state read back from PERF_CTL on all eight CPUs
> 5 and 17 seconds after resume; "clock" is the measured speed of a
> single-threaded busy loop.  Before every suspend, all CPUs requested
> 38 and the clock was about 3.8 GHz.
>
>   mode     governor     fix  request after resume  clock
>   passive  performance  on   38 on all CPUs        3.8 GHz
>   passive  performance  off  28 on all CPUs        2.8 GHz
>   active   performance  off  38 on all CPUs        3.8 GHz
>   active   performance  on   38 on all CPUs        3.8 GHz
>
> The passive-mode failure was also reproduced twice on Fedora's 7.2.11
> kernel.  In active mode the request was back at 38 after resume even
> without the fix, so on this machine the fix only makes a difference in
> passive mode, the default without HWP.
>
> Not tested: systems with HWP (their code path is unchanged, only
> restructured), other machines and firmware, and suspend-to-idle.  The
> patch adds no warnings with W=1; sparse was not run.  The tests were
> scripted and run by the same AI assistant.
>
>  drivers/cpufreq/intel_pstate.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 17158cb..53db91a 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1371,6 +1371,7 @@ static void set_power_ctl_ee_state(bool input)
>  }
>
>  static void intel_pstate_hwp_enable(struct cpudata *cpudata);
> +static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate);
>
>  static void intel_pstate_hwp_reenable(struct cpudata *cpu)
>  {
> @@ -1404,11 +1405,20 @@ static int intel_pstate_resume(struct cpufreq_policy *policy)
>         else if (power_ctl_ee_state == POWER_CTL_EE_DISABLE)
>                 set_power_ctl_ee_state(false);
>
> -       if (cpu->suspended && hwp_active) {
> +       if (cpu->suspended) {
>                 mutex_lock(&intel_pstate_limits_lock);
>
> -               /* Re-enable HWP, because "online" has not done that. */
> -               intel_pstate_hwp_reenable(cpu);
> +               if (hwp_active) {
> +                       /* Re-enable HWP, because "online" has not done that. */
> +                       intel_pstate_hwp_reenable(cpu);
> +               } else {
> +                       /*
> +                        * PERF_CTL may have been changed by the platform
> +                        * firmware, but it is only written when the requested
> +                        * P-state changes, so restore it explicitly.
> +                        */
> +                       intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate);

According to Sashiko, this introduces a possible race condition with
concurrent P-state updates on cpu->cpu:

https://sashiko.dev/#/patchset/20261027143725.3763827-1-zakbrightonknight%40gmail.com

It would take some more work to avoid this race AFAICS.

> +               }
>
>                 mutex_unlock(&intel_pstate_limits_lock);
>         }
>
> base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
> --
> 2.56.0
>

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

* RE: [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP
  2026-10-27 14:37 [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP Zak
  2026-10-27 15:56 ` Rafael J. Wysocki (Intel)
@ 2026-10-27 16:36 ` Doug Smythies
  2026-10-27 20:32   ` Rafael J. Wysocki (Intel)
  1 sibling, 1 reply; 8+ messages in thread
From: Doug Smythies @ 2026-10-27 16:36 UTC (permalink / raw)
  To: 'Zak', linux-pm, 'Rafael J. Wysocki',
	'Srinivas Pandruvada'
  Cc: 'Len Brown', 'Viresh Kumar', linux-kernel, stable,
	Doug Smythies

On 2026.10.27 07:37 Zak Brighton Knight wrote:

> From: Zak Brighton Knight <zakbrightonknight@gmail.com>
>
> On a ThinkPad T520 with an i7-3840QM (Ivy Bridge) running coreboot,
> the CPUs stop using turbo after every suspend to RAM when intel_pstate
> is in passive mode with the performance governor: they stay at the
> 2.8 GHz base frequency instead of boosting up to 3.8 GHz, until the
> policy limits change or the system is rebooted.
> 
> During resume, the firmware sets each CPU's P-state request register,
> MSR_IA32_PERF_CTL, to the base ratio (28), and the kernel never writes
> the requested turbo ratio (38) again.  Without hardware-managed
> P-states (HWP), intel_pstate caches the last P-state it wrote and
> skips writes that would not change it.  In passive mode the cpufreq
> core also skips requests for the frequency it believes is already set
> (policy->cur), because intel_cpufreq sets CPUFREQ_NEED_UPDATE_LIMITS
> only with HWP.  Neither cached value is updated across suspend and
> resume, so as long as the governor keeps requesting the same P-state,
> as the performance governor always does, the register is never
> rewritten.  Governors that change the request often, such as
> schedutil, mostly hide the problem.
>
> Fix this by writing the cached P-state back to the register in
> intel_pstate_resume() when HWP is not used, as is already done for
> the HWP request when it is.
>
> The problem was found and root-caused, and this patch was drafted, with
> the help of an AI coding assistant (Claude, by Anthropic) during a
> performance review of the machine described above.
>
> Fixes: 001c76f05b01 ("cpufreq: intel_pstate: Generic governors support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zak Brighton Knight <zakbrightonknight@gmail.com>
> ---
> Tested on the T520 (i7-3840QM, no HWP, coreboot 24.08 with SeaBIOS)
> with v7.3 plus this patch and a temporary module parameter, not part
> of this patch, to turn the fix off.  intel_pstate.c is the same in
> v7.3 and in the base commit below.
>
> Each row is one suspend to RAM and resume (rtcwake -m mem -s 25).
> "Request" is the P-state read back from PERF_CTL on all eight CPUs
> 5 and 17 seconds after resume; "clock" is the measured speed of a
> single-threaded busy loop.  Before every suspend, all CPUs requested
> 38 and the clock was about 3.8 GHz.
>
>  mode     governor     fix  request after resume  clock
>  passive  performance  on   38 on all CPUs        3.8 GHz
>  passive  performance  off  28 on all CPUs        2.8 GHz
>  active   performance  off  38 on all CPUs        3.8 GHz
>  active   performance  on   38 on all CPUs        3.8 GHz
>
> The passive-mode failure was also reproduced twice on Fedora's 7.2.11
> kernel.  In active mode the request was back at 38 after resume even
> without the fix, so on this machine the fix only makes a difference in
> passive mode, the default without HWP.
>
> Not tested: systems with HWP (their code path is unchanged, only
> restructured), other machines and firmware, and suspend-to-idle.  The
> patch adds no warnings with W=1; sparse was not run.  The tests were
> scripted and run by the same AI assistant.
>
> drivers/cpufreq/intel_pstate.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 17158cb..53db91a 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -1371,6 +1371,7 @@ static void set_power_ctl_ee_state(bool input)
>  }
> 
>  static void intel_pstate_hwp_enable(struct cpudata *cpudata);
> +static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate);
> 
>  static void intel_pstate_hwp_reenable(struct cpudata *cpu)
>  {
> @@ -1404,11 +1405,20 @@ static int intel_pstate_resume(struct cpufreq_policy *policy)
>  	else if (power_ctl_ee_state == POWER_CTL_EE_DISABLE)
>  		set_power_ctl_ee_state(false);
> 
> -	if (cpu->suspended && hwp_active) {
> +	if (cpu->suspended) {
> 		mutex_lock(&intel_pstate_limits_lock);
> 
> -		/* Re-enable HWP, because "online" has not done that. */
> -		intel_pstate_hwp_reenable(cpu);
> +		if (hwp_active) {
> +			/* Re-enable HWP, because "online" has not done that. */
> +			intel_pstate_hwp_reenable(cpu);
> +		} else {
> +			/*
> +			 * PERF_CTL may have been changed by the platform
> +			 * firmware, but it is only written when the requested
> +			 * P-state changes, so restore it explicitly.
> +			 */
> +			intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate);
> +		}
> 
> 		mutex_unlock(&intel_pstate_limits_lock);
> 	}
>
> base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
> -- 
> 2.56.0

I confirm your findings and tested your proposed patch. It all worked fine.
I had not seen Rafaels's reply about a possible race condition until now.

Test computer: i5-10600K, 6 cores, 12 CPUs, HWP disabled.

... Doug



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

* Re: [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP
  2026-10-27 16:36 ` Doug Smythies
@ 2026-10-27 20:32   ` Rafael J. Wysocki (Intel)
  2026-10-27 21:46     ` Doug Smythies
  2026-10-28  9:46     ` [PATCH v2] " Zak Brighton Knight
  0 siblings, 2 replies; 8+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-27 20:32 UTC (permalink / raw)
  To: Doug Smythies, Zak
  Cc: linux-pm, Srinivas Pandruvada, Viresh Kumar, linux-kernel

On Tue, Oct 27, 2026 at 5:36 PM Doug Smythies <dsmythies@telus.net> wrote:
>
> On 2026.10.27 07:37 Zak Brighton Knight wrote:
>
> > From: Zak Brighton Knight <zakbrightonknight@gmail.com>
> >
> > On a ThinkPad T520 with an i7-3840QM (Ivy Bridge) running coreboot,
> > the CPUs stop using turbo after every suspend to RAM when intel_pstate
> > is in passive mode with the performance governor: they stay at the
> > 2.8 GHz base frequency instead of boosting up to 3.8 GHz, until the
> > policy limits change or the system is rebooted.
> >
> > During resume, the firmware sets each CPU's P-state request register,
> > MSR_IA32_PERF_CTL, to the base ratio (28), and the kernel never writes
> > the requested turbo ratio (38) again.

Well, not quite.

If this is suspend-to-RAM, all CPUs except for CPU0 are taken offline
during suspend and brought back online during resume.

So what happens is that cpufreq_suspend() runs first and calls
intel_pstate_suspend() for each CPU.  Without HWP, this just sets the
cpu->suspended flag.  In addition, in the passive mode,
cpufreq_suspend() stops the governor for all CPUs (which does nothing
for the performance governor).

When CPU offline takes place subsequently, intel_pstate_cpu_offline()
or intel_cpufreq_cpu_offline() is called for the CPU going offline.
The former clears the util update hook and invokes the latter.  The
latter sees the cpu->suspended flag set and bails out.

During resume, CPU online takes place first (except for CPU0) and
cpufreq_online() runs for each of the affected CPUs.  Each of the
involved policies is inactive at that time, so cpufreq_policy_online()
runs.  It invokes intel_pstate_cpu_online() which doesn't do anything
interesting without HWP, and then cpufreq_init_policy() is called and
it ends up calling cpufreq_set_policy().  That one calls either
intel_pstate_set_policy(), in the active mode, or the governor
->limits() callback that for the performance governor ends up calling
intel_cpufreq_target(), in the passive mode.

intel_pstate_set_policy() does the right thing and sets the P-state to
the max in the "performance" case, but intel_cpufreq_target() doesn't
update the PERF_CTL register if the new P-state value is equal to the
cached one.  That only happens for the performance governor though.

Subsequently, cpufreq_resume() runs.  It calls intel_pstate_resume()
for all CPUs (that doesn't do anything interesting without HWP) and
starts the governor for each of them in the passive mode, but that
does nothing for the performance governor.

This means a couple of things.

First of all, the fix is not needed in the active mode for CPUs other
than CPU0 because "online" takes care of them properly.  It may be
needed for CPU0 in that case because its PERF_CTL is never updated
throughout suspend-resume.  However, in the active mode, if the
performance governor is used, it is actually fine to access
pstate.current_pstate and call intel_pstate_set_pstate() without
synchronization because the update util hook is not set in that case.
If the powersave governor is used in the active mode, the update util
hook would need to be cleared at suspend time and restored at resume
time (modulo the possible offline and online in between).

Second, in the passive mode the fix is generally needed, but it is
safe to do in that case because the governor has been stopped during
suspend and will be re-started only after calling
intel_pstate_resume() (IOW, the needed synchronization is already
there).

> > Without hardware-managed
> > P-states (HWP), intel_pstate caches the last P-state it wrote and
> > skips writes that would not change it.  In passive mode the cpufreq
> > core also skips requests for the frequency it believes is already set
> > (policy->cur), because intel_cpufreq sets CPUFREQ_NEED_UPDATE_LIMITS
> > only with HWP.  Neither cached value is updated across suspend and
> > resume, so as long as the governor keeps requesting the same P-state,
> > as the performance governor always does, the register is never
> > rewritten.

Not entirely the case as you can see from the above.

> > Governors that change the request often, such as schedutil, mostly hide the problem.

Well, the question is whether or not it makes sense to restore a
pre-suspend value that's going to be overwritten immediately.  Maybe
not.

Since in the passive mode it is always safe to carry out that
restoration, it is kind of straightforward to do it always then.  In
the active mode though, it's probably better to just do it only in the
"performance" case and only on CPU0 (that doesn't undergo "offline"
and "online", but since it appears to work for you anyway, fixing the
passive mode may be sufficient for now.

I'll cut a patch for this unless someone beats me to it.

> >
> > Fix this by writing the cached P-state back to the register in
> > intel_pstate_resume() when HWP is not used, as is already done for
> > the HWP request when it is.
> >
> > The problem was found and root-caused, and this patch was drafted, with
> > the help of an AI coding assistant (Claude, by Anthropic) during a
> > performance review of the machine described above.
> >
> > Fixes: 001c76f05b01 ("cpufreq: intel_pstate: Generic governors support")

I'm not sure about this.  Maybe some other change related to "offline"
and "online" support.

> > Cc: stable@vger.kernel.org
> > Signed-off-by: Zak Brighton Knight <zakbrightonknight@gmail.com>
> > ---
> > Tested on the T520 (i7-3840QM, no HWP, coreboot 24.08 with SeaBIOS)
> > with v7.3 plus this patch and a temporary module parameter, not part
> > of this patch, to turn the fix off.  intel_pstate.c is the same in
> > v7.3 and in the base commit below.
> >
> > Each row is one suspend to RAM and resume (rtcwake -m mem -s 25).
> > "Request" is the P-state read back from PERF_CTL on all eight CPUs
> > 5 and 17 seconds after resume; "clock" is the measured speed of a
> > single-threaded busy loop.  Before every suspend, all CPUs requested
> > 38 and the clock was about 3.8 GHz.
> >
> >  mode     governor     fix  request after resume  clock
> >  passive  performance  on   38 on all CPUs        3.8 GHz
> >  passive  performance  off  28 on all CPUs        2.8 GHz
> >  active   performance  off  38 on all CPUs        3.8 GHz

Had the changelog been accurate, this would have been 28.

> >  active   performance  on   38 on all CPUs        3.8 GHz
> >
> > The passive-mode failure was also reproduced twice on Fedora's 7.2.11
> > kernel.  In active mode the request was back at 38 after resume even
> > without the fix, so on this machine the fix only makes a difference in
> > passive mode, the default without HWP.
> >
> > Not tested: systems with HWP (their code path is unchanged, only
> > restructured), other machines and firmware, and suspend-to-idle.  The
> > patch adds no warnings with W=1; sparse was not run.  The tests were
> > scripted and run by the same AI assistant.
> >
> > drivers/cpufreq/intel_pstate.c | 16 +++++++++++++---
> > 1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index 17158cb..53db91a 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -1371,6 +1371,7 @@ static void set_power_ctl_ee_state(bool input)
> >  }
> >
> >  static void intel_pstate_hwp_enable(struct cpudata *cpudata);
> > +static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate);
> >
> >  static void intel_pstate_hwp_reenable(struct cpudata *cpu)
> >  {
> > @@ -1404,11 +1405,20 @@ static int intel_pstate_resume(struct cpufreq_policy *policy)
> >       else if (power_ctl_ee_state == POWER_CTL_EE_DISABLE)
> >               set_power_ctl_ee_state(false);
> >
> > -     if (cpu->suspended && hwp_active) {
> > +     if (cpu->suspended) {
> >               mutex_lock(&intel_pstate_limits_lock);
> >
> > -             /* Re-enable HWP, because "online" has not done that. */
> > -             intel_pstate_hwp_reenable(cpu);
> > +             if (hwp_active) {
> > +                     /* Re-enable HWP, because "online" has not done that. */
> > +                     intel_pstate_hwp_reenable(cpu);
> > +             } else {
> > +                     /*
> > +                      * PERF_CTL may have been changed by the platform
> > +                      * firmware, but it is only written when the requested
> > +                      * P-state changes, so restore it explicitly.
> > +                      */
> > +                     intel_pstate_set_pstate(cpu, cpu->pstate.current_pstate);
> > +             }
> >
> >               mutex_unlock(&intel_pstate_limits_lock);
> >       }
> >
> > base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
> > --
> > 2.56.0
>
> I confirm your findings and tested your proposed patch. It all worked fine.
> I had not seen Rafaels's reply about a possible race condition until now.
>
> Test computer: i5-10600K, 6 cores, 12 CPUs, HWP disabled.

Passive or active mode?

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

* RE: [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP
  2026-10-27 20:32   ` Rafael J. Wysocki (Intel)
@ 2026-10-27 21:46     ` Doug Smythies
  2026-10-28 15:54       ` Rafael J. Wysocki (Intel)
  2026-10-28  9:46     ` [PATCH v2] " Zak Brighton Knight
  1 sibling, 1 reply; 8+ messages in thread
From: Doug Smythies @ 2026-10-27 21:46 UTC (permalink / raw)
  To: 'Rafael J. Wysocki (Intel)', 'Zak'
  Cc: linux-pm, 'Srinivas Pandruvada', 'Viresh Kumar',
	linux-kernel, Doug Smythies

On 2026.10.27 Rafael wrote:
> On Tue, Oct 27, 2026 at 5:36 PM Doug Smythies <dsmythies@telus.net> wrote:>
>> On 2026.10.27 07:37 Zak Brighton Knight wrote:

...

>> I confirm your findings and tested your proposed patch. It all worked fine.
>> I had not seen Rafaels's reply about a possible race condition until now.
>>
>> Test computer: i5-10600K, 6 cores, 12 CPUs, HWP disabled.
>
> Passive or active mode?

Apologies for the incomplete report.

Passive mode has the issue. Active mode was fine without the need for the patch.

... Doug



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

* [PATCH v2] cpufreq: intel_pstate: Restore P-state request on resume without HWP
  2026-10-27 20:32   ` Rafael J. Wysocki (Intel)
  2026-10-27 21:46     ` Doug Smythies
@ 2026-10-28  9:46     ` Zak Brighton Knight
  2026-10-28 15:52       ` Rafael J. Wysocki (Intel)
  1 sibling, 1 reply; 8+ messages in thread
From: Zak Brighton Knight @ 2026-10-28  9:46 UTC (permalink / raw)
  To: linux-pm, Rafael J. Wysocki, Srinivas Pandruvada
  Cc: Len Brown, Viresh Kumar, Doug Smythies, linux-kernel,
	Zak Brighton Knight, stable

On a ThinkPad T520 with an i7-3840QM (Ivy Bridge) running coreboot,
the CPUs stop using turbo after every suspend to RAM when intel_pstate
is in passive mode with the performance governor: they stay at the
2.8 GHz base frequency instead of boosting up to 3.8 GHz, until the
policy limits change or the system is rebooted.

During resume, the platform firmware sets each CPU's P-state request
register, MSR_IA32_PERF_CTL, to the base ratio (28).  Without
hardware-managed P-states (HWP), nothing in the passive mode writes it
again afterwards:

 - When the nonboot CPUs go offline during suspend,
   intel_cpufreq_cpu_offline() bails out because cpu->suspended is
   set, so the cached P-state (cpu->pstate.current_pstate) and
   policy->cur keep the values from before suspend.

 - When they come back online, cpufreq_set_policy() does not start the
   governor, because cpufreq_suspended is still set at that point.

 - cpufreq_resume() then calls intel_pstate_resume(), which does not
   touch the register without HWP, and starts the governor.  The
   performance governor asks for policy->max, which is equal to
   policy->cur, so __cpufreq_driver_target() returns without calling
   the driver.  Even if it did, intel_cpufreq_update_pstate() would
   skip the write, because the requested P-state matches the cached
   one.

Governors that change the request often, like schedutil, write the
register again shortly after resume, so they mostly hide the problem.
In the active mode, intel_pstate_set_policy() writes the register for
the nonboot CPUs when they come back online.

Fix this by adding a ->resume() callback to intel_cpufreq that writes
the cached P-state back to the register when HWP is not used, before
calling intel_pstate_resume().  The governor of the policy is not
running at that point, and the write is done under the policy rwsem,
so it cannot race with updates of the policy limits either.

The problem was found and root-caused, and this patch was drafted, with
the help of an AI coding assistant (Claude, by Anthropic) during a
performance review of the machine described above.

Fixes: 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and ->online callbacks")
Cc: stable@vger.kernel.org
Signed-off-by: Zak Brighton Knight <zakbrightonknight@gmail.com>
---
Changes in v2:
- Only restore the request in the passive mode, from a new ->resume()
  callback of intel_cpufreq, and do it there unconditionally, as
  suggested by Rafael.  v1 did it in intel_pstate_resume() for both
  modes, where it could race with the active-mode util update hook, as
  reported by Sashiko.
- Hold the policy rwsem around the write.  The governor of the policy
  is stopped at that point, but cpufreq_resume() has already cleared
  cpufreq_suspended, so a policy->update work item (handle_update() ->
  refresh_frequency_limits() -> cpufreq_set_policy() ->
  cpufreq_governor_limits() -> ->limits()) can still get to
  intel_cpufreq_target() for the same CPU, for example after a limit
  change during resume, and its write could then be overwritten with
  the stale request.  This is the second case in Sashiko's report;
  please drop the lock if that cannot happen there.
- Describe the suspend/resume flow in the changelog, based on Rafael's
  explanation.  When the nonboot CPUs come back online during resume,
  cpufreq_init_governor() and cpufreq_start_governor() return early
  because cpufreq_suspended is still set, so the request is only
  dropped later, by the target_freq == policy->cur check in
  __cpufreq_driver_target() (see the debug messages below).
- Fixes: 4adcf2e5829f, after which the nonboot CPUs no longer go
  through ->init on online.  Before it, intel_cpufreq_cpu_init() reset
  PERF_CTL and policy->cur to the minimum when they came back, so the
  request after resume was written.

v1: https://lore.kernel.org/linux-pm/20261027143725.3763827-1-zakbrightonknight@gmail.com/

Tested on the T520 (i7-3840QM, no HWP, coreboot 24.08 with SeaBIOS)
with v7.3 plus this patch and a temporary module parameter, not part
of this patch, to turn the new write off, built with PROVE_LOCKING.
intel_pstate.c and the cpufreq core functions involved are the same
in v7.3 and in the base commit below.  Passive mode, performance
governor, one suspend to RAM and resume per row (rtcwake -m mem
-s 25), PERF_CTL read on all eight CPUs:

  fix  before resume   5 and 17 s after resume   single-thread clock
  off  38 on all       28 on all                 2.8 GHz
  on   28 on all       38 on all                 3.8 GHz
  on   38 on all       38 on all                 3.8 GHz

The second row started from the state the first one left: the restore
writes the cached request, which was still 38.  No lockdep reports,
and debug_locks was still 1 after the three cycles.

With dynamic debug enabled for cpufreq.c, cpufreq_performance.c and
intel_pstate.c, the online part of resume logged "governor switch" and
"governor change" for CPUs 1-7, but no cpufreq_init_governor() or
cpufreq_start_governor() messages and no frequency requests.  After
"Resuming Governors", every CPU logged cpufreq_start_governor(),
"setting to 3800000 kHz" and "CPU n: cur 3800000 kHz -> target
3800000 kHz", and there were no transition notifications at all, so
intel_cpufreq_target() was not called during resume.

In v1's testing, the active mode was back at 38 on all CPUs after
resume on this machine even without a restore.  Doug Smythies saw the
same on an i5-10600K with HWP disabled (passive mode affected, active
mode fine) and tested v1 there; v2 has not been tested on that
machine yet.  sparse was not run.
The tests were scripted and run by the same AI assistant.

 drivers/cpufreq/intel_pstate.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 17158cb..67a3015 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -3408,6 +3408,27 @@ static int intel_cpufreq_suspend(struct cpufreq_policy *policy)
 	return 0;
 }
 
+static int intel_cpufreq_resume(struct cpufreq_policy *policy)
+{
+	struct cpudata *cpu = all_cpu_data[policy->cpu];
+
+	/*
+	 * Without HWP, PERF_CTL is only written when the requested P-state
+	 * changes, but the platform firmware may have changed it during
+	 * resume, so write the last request back.  The governor is not
+	 * running yet, and holding the policy rwsem serializes this with
+	 * updates of the policy limits.
+	 */
+	if (!hwp_active && cpu->suspended) {
+		guard(cpufreq_policy_write)(policy);
+
+		intel_cpufreq_perf_ctl_update(cpu, cpu->pstate.current_pstate,
+					      false);
+	}
+
+	return intel_pstate_resume(policy);
+}
+
 static struct cpufreq_driver intel_cpufreq = {
 	.flags		= CPUFREQ_CONST_LOOPS,
 	.verify		= intel_cpufreq_verify_policy,
@@ -3418,7 +3439,7 @@ static struct cpufreq_driver intel_cpufreq = {
 	.offline	= intel_cpufreq_cpu_offline,
 	.online		= intel_pstate_cpu_online,
 	.suspend	= intel_cpufreq_suspend,
-	.resume		= intel_pstate_resume,
+	.resume		= intel_cpufreq_resume,
 	.update_limits	= intel_pstate_update_limits,
 	.scale_freq_ref = intel_pstate_scale_freq_ref,
 	.name		= "intel_cpufreq",

base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
-- 
2.56.0


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

* Re: [PATCH v2] cpufreq: intel_pstate: Restore P-state request on resume without HWP
  2026-10-28  9:46     ` [PATCH v2] " Zak Brighton Knight
@ 2026-10-28 15:52       ` Rafael J. Wysocki (Intel)
  0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-28 15:52 UTC (permalink / raw)
  To: Zak Brighton Knight
  Cc: linux-pm, Rafael J. Wysocki, Srinivas Pandruvada, Len Brown,
	Viresh Kumar, Doug Smythies, linux-kernel, stable

[-- Attachment #1: Type: text/plain, Size: 4525 bytes --]

On Wed, Oct 28, 2026 at 10:48 AM Zak Brighton Knight
<zakbrightonknight@gmail.com> wrote:
>
> On a ThinkPad T520 with an i7-3840QM (Ivy Bridge) running coreboot,
> the CPUs stop using turbo after every suspend to RAM when intel_pstate
> is in passive mode with the performance governor: they stay at the
> 2.8 GHz base frequency instead of boosting up to 3.8 GHz, until the
> policy limits change or the system is rebooted.
>
> During resume, the platform firmware sets each CPU's P-state request
> register, MSR_IA32_PERF_CTL, to the base ratio (28).  Without
> hardware-managed P-states (HWP), nothing in the passive mode writes it
> again afterwards:
>
>  - When the nonboot CPUs go offline during suspend,
>    intel_cpufreq_cpu_offline() bails out because cpu->suspended is
>    set, so the cached P-state (cpu->pstate.current_pstate) and
>    policy->cur keep the values from before suspend.
>
>  - When they come back online, cpufreq_set_policy() does not start the
>    governor, because cpufreq_suspended is still set at that point.
>
>  - cpufreq_resume() then calls intel_pstate_resume(), which does not
>    touch the register without HWP, and starts the governor.  The
>    performance governor asks for policy->max, which is equal to
>    policy->cur, so __cpufreq_driver_target() returns without calling
>    the driver.  Even if it did, intel_cpufreq_update_pstate() would
>    skip the write, because the requested P-state matches the cached
>    one.
>
> Governors that change the request often, like schedutil, write the
> register again shortly after resume, so they mostly hide the problem.
> In the active mode, intel_pstate_set_policy() writes the register for
> the nonboot CPUs when they come back online.
>
> Fix this by adding a ->resume() callback to intel_cpufreq that writes
> the cached P-state back to the register when HWP is not used, before
> calling intel_pstate_resume().  The governor of the policy is not
> running at that point, and the write is done under the policy rwsem,
> so it cannot race with updates of the policy limits either.
>
> The problem was found and root-caused, and this patch was drafted, with
> the help of an AI coding assistant (Claude, by Anthropic) during a
> performance review of the machine described above.
>
> Fixes: 4adcf2e5829f ("cpufreq: intel_pstate: Add ->offline and ->online callbacks")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zak Brighton Knight <zakbrightonknight@gmail.com>

This patch would correct the observed behavior with intel_pstate, but
it has drawbacks.

First, it papers over issues in the cpufreq core that may affect more
drivers in principle.

Second, it adds IPI overhead to the resume path that is entirely
avoidable AFAICS.

The first core issue related to it that should be addressed in general
is suspend/resume synchronization in it that is insufficient because
the cpufreq_suspended global variable is accessed locklessly, so its
updates are subject to reordering with respect to other operations and
may be missed by concurrent threads running on other CPUs.
Fortunately, this can be done in a straightforward way by replacing
cpufreq_suspended with a per-policy "suspended" flag updated under the
policy rwsem.

Another related core issue is that policy->cur is retained after
stopping the governor during suspend, even though it may be stale
during the subsequent resume, which generally doesn't affect drivers
implementing the .get() callback because of the
cpufreq_verify_current_freq() call in cpufreq_start_governor().
intel_pstate doesn't implement that callback though and so it allows
the stale policy->cur to be used past the governor restart during
resume and there may be other drivers that don't implement .get() in
principle.

After addressing the previous issue, fixing this one is a matter of
clearing policy->cur at suspend time.

The intel_pstate-specific issue, which is that
cpu->pstate.current_pstate is retained across suspend-resume, even
though it may be stale during resume, can also be addressed by
clearing that field at suspend time.  Doing so doesn't require any
special synchronization after fixing the first core issue above
because the core will prevent the frequency from being updated and so
there will be no other cpu->pstate.current_pstate updates till resume.

So please test the attached patch (that needs to be split before
submitting etc) and let me know if it works for you.

Thanks!

[-- Attachment #2: cpufreq-intel-pstate-suspend-resume.patch --]
[-- Type: application/x-patch, Size: 5969 bytes --]

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

* Re: [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP
  2026-10-27 21:46     ` Doug Smythies
@ 2026-10-28 15:54       ` Rafael J. Wysocki (Intel)
  0 siblings, 0 replies; 8+ messages in thread
From: Rafael J. Wysocki (Intel) @ 2026-10-28 15:54 UTC (permalink / raw)
  To: Doug Smythies
  Cc: Rafael J. Wysocki (Intel), Zak, linux-pm, Srinivas Pandruvada,
	Viresh Kumar, linux-kernel

On Tue, Oct 27, 2026 at 10:46 PM Doug Smythies <dsmythies@telus.net> wrote:
>
> On 2026.10.27 Rafael wrote:
> > On Tue, Oct 27, 2026 at 5:36 PM Doug Smythies <dsmythies@telus.net> wrote:>
> >> On 2026.10.27 07:37 Zak Brighton Knight wrote:
>
> ...
>
> >> I confirm your findings and tested your proposed patch. It all worked fine.
> >> I had not seen Rafaels's reply about a possible race condition until now.
> >>
> >> Test computer: i5-10600K, 6 cores, 12 CPUs, HWP disabled.
> >
> > Passive or active mode?
>
> Apologies for the incomplete report.

No worries.

> Passive mode has the issue. Active mode was fine without the need for the patch.

Thanks for the update.

Please check the patch I've just posted if poss.

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

end of thread, other threads:[~2026-10-28 15:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 14:37 [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP Zak
2026-10-27 15:56 ` Rafael J. Wysocki (Intel)
2026-10-27 16:36 ` Doug Smythies
2026-10-27 20:32   ` Rafael J. Wysocki (Intel)
2026-10-27 21:46     ` Doug Smythies
2026-10-28 15:54       ` Rafael J. Wysocki (Intel)
2026-10-28  9:46     ` [PATCH v2] " Zak Brighton Knight
2026-10-28 15:52       ` Rafael J. Wysocki (Intel)

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