Re: [PATCH] cpufreq: intel_pstate: Restore P-state request on resume without HWP

[Rafael J. Wysocki (Intel)]

On Tue, Oct 27, 2026 at 3:39 PM Zak <zakbrightonknight@xxxxxxxxx> wrote:
>
> From: Zak Brighton Knight <zakbrightonknight@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Zak Brighton Knight <zakbrightonknight@xxxxxxxxx>
> ---
> 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
>