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

["Doug Smythies" <dsmythies@telus.net>]

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