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

[Zak]

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);
+		}
 
 		mutex_unlock(&intel_pstate_limits_lock);
 	}

base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
-- 
2.56.0