Linux Tegra architecture development
 help / color / mirror / Atom feed
From: Sumit Gupta <sumitg@nvidia.com>
To: <rafael@kernel.org>, <viresh.kumar@linaro.org>,
	<pierre.gondois@arm.com>, <christian.loehle@arm.com>,
	<ionela.voinescu@arm.com>, <zhenglifeng1@huawei.com>,
	<zhanjie9@hisilicon.com>, <lenb@kernel.org>, <ray.huang@amd.com>,
	<mario.limonciello@amd.com>, <perry.yuan@amd.com>,
	<kprateek.nayak@amd.com>, <maciej.wieczor-retman@intel.com>,
	<pawel.chmielewski@intel.com>, <linux-pm@vger.kernel.org>,
	<linux-acpi@vger.kernel.org>, <acpica-devel@lists.linux.dev>,
	<linux-kernel@vger.kernel.org>
Cc: <linux-tegra@vger.kernel.org>, <treding@nvidia.com>,
	<jonathanh@nvidia.com>, <vsethi@nvidia.com>,
	<ksitaraman@nvidia.com>, <sanjayc@nvidia.com>, <mochs@nvidia.com>,
	<bbasu@nvidia.com>, <sumitg@nvidia.com>
Subject: [PATCH v6 5/5] cpufreq: CPPC: Preserve OSPM-set registers across suspend/resume
Date: Tue, 20 Oct 2026 17:48:46 +0530	[thread overview]
Message-ID: <20261020121846.691218-6-sumitg@nvidia.com> (raw)
In-Reply-To: <20261020121846.691218-1-sumitg@nvidia.com>

The driver preserves the OSPM-set registers across CPU hotplug, but system
suspend/resume is a separate path. On platforms that reset those registers
or the performance controls across suspend, the values are lost.

The hotplug callbacks cannot cover suspend on their own. offline() does
not run for every policy, as the boot CPU stays up during suspend-to-RAM
and no CPU goes offline during suspend-to-idle. The cpufreq core calls the
driver's suspend() for every active policy from cpufreq_suspend(), before
any CPU goes offline.

Reuse the same save/restore mechanism for suspend/resume:

  - suspend() saves the current OSPM-set values, restores the firmware
    ones and sets a per-policy flag, suspend_regs_handled, to record that.
    It also stops the frequency invariance updates, so that no sample
    spans the suspend window.
  - offline() skips the save and restore while the flag is set, as
    suspend() has already done both. Saving again would capture the
    firmware values that suspend() wrote back and lose what the OS set.
    It still requests the lowest performance.
  - online() clears the flag, so that a later offline() takes a fresh
    snapshot. It also restarts the frequency invariance updates. resume()
    no longer marks the cached fast-path limits stale, as online() already
    does.
  - resume() calls online() for a policy that still has the flag set. CPUs
    offlined during suspend come back before the core calls resume(), so
    online() has already run for their policies and cleared the flag.

Also make fie_init() cover related_cpus, as fie_exit() does. Some CPUs
may still be offline when resume() calls online(). online() is not
called again as they come up, so their frequency invariance updates
would never restart.

Suggested-by: Christian Loehle <christian.loehle@arm.com>
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 81 ++++++++++++++++++++++++++++------
 1 file changed, 68 insertions(+), 13 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 1b733f15d641..aa4429c615e0 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -85,6 +85,12 @@ enum cppc_saved_type {
  */
 struct cppc_policy_state {
 	u64 regs[CPPC_NR_SAVED_REGS][CPPC_NR_SAVED_TYPES];
+	/*
+	 * Set by suspend() after it saves the OSPM-set values and restores the
+	 * firmware ones, so a later offline() does not save the firmware ones
+	 * over them. Cleared at init() and by online().
+	 */
+	bool suspend_regs_handled;
 };
 
 static DEFINE_PER_CPU(struct cppc_policy_state, cppc_policy_state);
@@ -313,6 +319,7 @@ static struct scale_freq_data cppc_sftd_pcc = {
 
 static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
 {
+	const struct cpumask *cpus = policy->related_cpus;
 	struct scale_freq_data *sftd;
 	struct cppc_freq_invariance *cppc_fi;
 	int cpu, ret;
@@ -320,7 +327,17 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
 	if (fie_disabled)
 		return;
 
-	for_each_cpu(cpu, policy->cpus) {
+	/*
+	 * Use related_cpus, as cppc_cpufreq_cpu_fie_exit() does. When
+	 * cppc_cpufreq_cpu_resume() calls online(), policy->cpus holds only
+	 * the online CPUs. online() is not called again as the offline CPUs
+	 * come up, so their updates would never start. During init(),
+	 * related_cpus is empty and policy->cpus holds every CPU.
+	 */
+	if (cpumask_empty(cpus))
+		cpus = policy->cpus;
+
+	for_each_cpu(cpu, cpus) {
 		cppc_fi = &per_cpu(cppc_freq_inv, cpu);
 		cppc_fi->cpu = cpu;
 		cppc_fi->cpu_data = policy->driver_data;
@@ -335,10 +352,7 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
 
 		ret = cppc_get_perf_ctrs(cpu, &cppc_fi->prev_perf_fb_ctrs);
 
-		/*
-		 * Don't abort as the CPU was offline while the driver was
-		 * getting registered.
-		 */
+		/* Don't abort for a CPU that is offline. */
 		if (ret && cpu_online(cpu)) {
 			pr_debug("%s: failed to read perf counters for cpu:%d: %d\n",
 				__func__, cpu, ret);
@@ -347,7 +361,7 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy)
 	}
 
 	/* A shared policy may contain both PCC and non-PCC counters. */
-	for_each_cpu(cpu, policy->cpus) {
+	for_each_cpu(cpu, cpus) {
 		cppc_fi = &per_cpu(cppc_freq_inv, cpu);
 		if (cppc_fi->pcc_work_initialized)
 			sftd = &cppc_sftd_pcc;
@@ -960,6 +974,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	policy->cur = cppc_perf_to_khz(caps, desired_perf);
 	cpu_data->perf_ctrls.desired_perf = desired_perf;
 
+	cppc_cpufreq_policy_state(policy)->suspend_regs_handled = false;
 	cppc_cpufreq_save_regs(policy, CPPC_SAVED_FIRMWARE);
 
 	ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
@@ -1023,6 +1038,8 @@ static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
 	unsigned int cpu = policy->cpu;
 	int ret;
 
+	cppc_cpufreq_policy_state(policy)->suspend_regs_handled = false;
+
 	/*
 	 * The platform may have reset the controls, so drop the limits cached
 	 * for the fast path until the restore below reprograms them.
@@ -1081,9 +1098,14 @@ static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
 	unsigned int cpu = policy->cpu;
 	int ret;
 
-	/* Save what the OS set, and leave the platform in its pre-driver state. */
-	cppc_cpufreq_save_regs(policy, CPPC_SAVED_REQUESTED);
-	cppc_cpufreq_apply_saved_regs(policy, CPPC_SAVED_FIRMWARE);
+	/*
+	 * Save what the OS set and leave the platform in its pre-driver state,
+	 * unless suspend() already did so earlier in this suspend cycle.
+	 */
+	if (!cppc_cpufreq_policy_state(policy)->suspend_regs_handled) {
+		cppc_cpufreq_save_regs(policy, CPPC_SAVED_REQUESTED);
+		cppc_cpufreq_apply_saved_regs(policy, CPPC_SAVED_FIRMWARE);
+	}
 
 	/*
 	 * Stop the frequency invariance updates and cancel the pending work, so
@@ -1109,16 +1131,48 @@ static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
 	return 0;
 }
 
-static int cppc_cpufreq_cpu_resume(struct cpufreq_policy *policy)
+/*
+ * Run for every active policy when the system suspends, before any CPU goes
+ * offline.
+ *
+ * Save the OSPM-set values and restore the firmware values. Doing it here
+ * covers a policy whose CPUs stay online, for which offline() never runs.
+ * offline() skips the save for the rest of the suspend cycle, as it would
+ * capture the firmware values written here and lose what the OS set.
+ *
+ * Stop the frequency invariance updates too, so that no sample spans the
+ * suspend window. online() restarts them on the way back, from hotplug or
+ * from resume().
+ */
+static int cppc_cpufreq_cpu_suspend(struct cpufreq_policy *policy)
 {
-	struct cppc_cpudata *cpu_data = policy->driver_data;
+	cppc_cpufreq_save_regs(policy, CPPC_SAVED_REQUESTED);
+	cppc_cpufreq_apply_saved_regs(policy, CPPC_SAVED_FIRMWARE);
+	cppc_cpufreq_policy_state(policy)->suspend_regs_handled = true;
 
-	/* The platform may have reset the controls during suspend. */
-	WRITE_ONCE(cpu_data->perf_limits_valid, false);
+	cppc_cpufreq_cpu_fie_exit(policy);
 
 	return 0;
 }
 
+/*
+ * Run the online() restore for a policy whose CPUs stayed online through
+ * suspend.
+ *
+ * CPUs offlined during suspend come back before the core calls resume(), so
+ * online() has already run for their policies and cleared the flag. Only a
+ * policy that still has it set needs online() here. Either way online() has
+ * reprogrammed the controls by the time resume() returns, so the limits
+ * cached for the fast path are up to date again.
+ */
+static int cppc_cpufreq_cpu_resume(struct cpufreq_policy *policy)
+{
+	if (!cppc_cpufreq_policy_state(policy)->suspend_regs_handled)
+		return 0;
+
+	return cppc_cpufreq_cpu_online(policy);
+}
+
 static inline u64 get_delta(u64 t1, u64 t0)
 {
 	if (t1 > t0 || t0 > ~(u32)0)
@@ -1423,6 +1477,7 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
 	.exit = cppc_cpufreq_cpu_exit,
 	.online = cppc_cpufreq_cpu_online,
 	.offline = cppc_cpufreq_cpu_offline,
+	.suspend = cppc_cpufreq_cpu_suspend,
 	.resume = cppc_cpufreq_cpu_resume,
 	.set_boost = cppc_cpufreq_set_boost,
 	.attr = cppc_cpufreq_attr,
-- 
2.34.1


      parent reply	other threads:[~2026-10-20 12:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-20 12:18 [PATCH v6 0/5] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload Sumit Gupta
2026-10-20 12:18 ` [PATCH v6 1/5] ACPI: CPPC: Order performance-control updates Sumit Gupta
2026-10-20 12:18 ` [PATCH v6 2/5] cpufreq: CPPC: Keep the policy across CPU hotplug Sumit Gupta
2026-10-20 12:18 ` [PATCH v6 3/5] ACPI: CPPC: Make autonomous selection helpers take a u64 Sumit Gupta
2026-10-20 12:18 ` [PATCH v6 4/5] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload Sumit Gupta
2026-10-20 12:18 ` Sumit Gupta [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261020121846.691218-6-sumitg@nvidia.com \
    --to=sumitg@nvidia.com \
    --cc=acpica-devel@lists.linux.dev \
    --cc=bbasu@nvidia.com \
    --cc=christian.loehle@arm.com \
    --cc=ionela.voinescu@arm.com \
    --cc=jonathanh@nvidia.com \
    --cc=kprateek.nayak@amd.com \
    --cc=ksitaraman@nvidia.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=maciej.wieczor-retman@intel.com \
    --cc=mario.limonciello@amd.com \
    --cc=mochs@nvidia.com \
    --cc=pawel.chmielewski@intel.com \
    --cc=perry.yuan@amd.com \
    --cc=pierre.gondois@arm.com \
    --cc=rafael@kernel.org \
    --cc=ray.huang@amd.com \
    --cc=sanjayc@nvidia.com \
    --cc=treding@nvidia.com \
    --cc=viresh.kumar@linaro.org \
    --cc=vsethi@nvidia.com \
    --cc=zhanjie9@hisilicon.com \
    --cc=zhenglifeng1@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox