* [PATCH v6 1/5] ACPI: CPPC: Order performance-control updates
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 ` Sumit Gupta
2026-10-20 12:18 ` [PATCH v6 2/5] cpufreq: CPPC: Keep the policy across CPU hotplug Sumit Gupta
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Sumit Gupta @ 2026-10-20 12:18 UTC (permalink / raw)
To: rafael, viresh.kumar, pierre.gondois, christian.loehle,
ionela.voinescu, zhenglifeng1, zhanjie9, lenb, ray.huang,
mario.limonciello, perry.yuan, kprateek.nayak,
maciej.wieczor-retman, pawel.chmielewski, linux-pm, linux-acpi,
acpica-devel, linux-kernel
Cc: linux-tegra, treding, jonathanh, vsethi, ksitaraman, sanjayc,
mochs, bbasu, sumitg
From: Christian Loehle <christian.loehle@arm.com>
The ACPI specification requires Minimum Performance to remain below
Maximum Performance. When autonomous selection is disabled, Desired
Performance must also remain within those limits.
cppc_set_perf() writes the three controls in a fixed order. This can expose
an invalid tuple after firmware resets the controls or while policy bounds
move across the current desired value.
Read the live limits. Apply non-atomic updates in three phases: widen the
interval, update Desired Performance, then narrow it. Submit PCC controls
in each phase together. Keep the existing single-command path when the
complete tuple is in PCC, reject invalid requests, and stop after the first
failed access.
Make cppc_cpufreq fast switches include the limits only when they differ
from the last successful request. Invalidate the cache across resume and
after a failed request, since firmware may have reset the controls or an
earlier phase of a failed update may have reached the platform. Publish
slow-path limits only after the complete request succeeds.
At policy teardown, lower Minimum Performance together with Desired
Performance so the final request remains valid. On initialization, clamp
Desired Performance to the live interval left by firmware or an earlier
driver instance.
Fixes: 76531df5e13b ("ACPI: CPPC: Add min and max perf register writing support")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
[sumitg:
- In cppc_read_perf_limits(), skip an unimplemented MIN or MAX register,
and do not fail when the live MIN is above MAX, as the requested limits
replace it.
- In init(), request the full range when the live MAX is zero or below
MIN, to avoid a cppc_cpufreq probe failure.
- Clamp Desired to MIN and MAX in set_target(), fast_switch() and
store_auto_select(), as cppc_set_perf() rejects it outside them.
cppc_khz_to_perf() can round the minimum frequency to lowest_perf - 1.]
Link: https://lore.kernel.org/all/01d7f07e-839c-428b-833d-713ed2128f4d@nvidia.com/
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
drivers/acpi/cppc_acpi.c | 294 ++++++++++++++++++++++++++-------
drivers/cpufreq/cppc_cpufreq.c | 118 +++++++++++--
include/acpi/cppc_acpi.h | 1 +
3 files changed, 337 insertions(+), 76 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 80e2e6b32ce3..8d38924e0750 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -3471,11 +3471,212 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
}
EXPORT_SYMBOL_GPL(cppc_get_perf);
+enum cppc_perf_update {
+ CPPC_UPDATE_DESIRED = BIT(0),
+ CPPC_UPDATE_MIN = BIT(1),
+ CPPC_UPDATE_MAX = BIT(2),
+};
+
+/* The caller holds pcc_lock for write if either limit is in PCC. */
+static int cppc_read_perf_limits(int cpu, u32 *min_perf, u32 *max_perf)
+{
+ struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+ struct cpc_register_resource *min_reg = &cpc_desc->cpc_regs[MIN_PERF];
+ struct cpc_register_resource *max_reg = &cpc_desc->cpc_regs[MAX_PERF];
+ u64 min = 0, max = U32_MAX;
+ int ret;
+
+ if (!cpc_is_readable(min_reg) || !cpc_is_readable(max_reg))
+ return -EOPNOTSUPP;
+
+ /* Firmware may have reset the controls, so read the live limits. */
+ if (CPC_IN_PCC(min_reg) || CPC_IN_PCC(max_reg)) {
+ ret = send_pcc_cmd(per_cpu(cpu_pcc_subspace_idx, cpu), CMD_READ);
+ if (ret)
+ return ret;
+ }
+
+ /* An absent optional limit leaves that side of the interval open. */
+ if (CPC_SUPPORTED(min_reg)) {
+ ret = cpc_read(cpu, min_reg, &min);
+ if (ret)
+ return ret;
+ }
+ if (CPC_SUPPORTED(max_reg)) {
+ ret = cpc_read(cpu, max_reg, &max);
+ if (ret)
+ return ret;
+ }
+ if (min > U32_MAX || max > U32_MAX)
+ return -EFAULT;
+ if (min > max)
+ pr_debug("CPU:%d live MIN_PERF %llu above MAX_PERF %llu\n",
+ cpu, min, max);
+
+ *min_perf = min;
+ *max_perf = max;
+ return 0;
+}
+
+static bool cppc_perf_updates_in_pcc(int cpu, unsigned int updates)
+{
+ struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+
+ return ((updates & CPPC_UPDATE_DESIRED) &&
+ CPC_IN_PCC(&cpc_desc->cpc_regs[DESIRED_PERF])) ||
+ ((updates & CPPC_UPDATE_MIN) &&
+ CPC_IN_PCC(&cpc_desc->cpc_regs[MIN_PERF])) ||
+ ((updates & CPPC_UPDATE_MAX) &&
+ CPC_IN_PCC(&cpc_desc->cpc_regs[MAX_PERF]));
+}
+
+/* The caller holds pcc_lock for write if an updated control is in PCC. */
+static int cppc_write_perf_controls(int cpu,
+ struct cppc_perf_ctrls *perf_ctrls,
+ unsigned int updates, bool sync_desired)
+{
+ struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+ struct cpc_register_resource *desired_reg =
+ &cpc_desc->cpc_regs[DESIRED_PERF];
+ struct cpc_register_resource *min_reg = &cpc_desc->cpc_regs[MIN_PERF];
+ struct cpc_register_resource *max_reg = &cpc_desc->cpc_regs[MAX_PERF];
+ struct cppc_pcc_data *pcc_ss_data;
+ int pcc_ss_id, ret;
+ bool pcc_update = false;
+
+ /* Direct writes precede the PCC command in each ordered phase. */
+ if ((updates & CPPC_UPDATE_DESIRED) && !CPC_IN_PCC(desired_reg)) {
+ ret = cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
+ if (ret)
+ return ret;
+ }
+ if ((updates & CPPC_UPDATE_MIN) && !CPC_IN_PCC(min_reg)) {
+ ret = cpc_write(cpu, min_reg, perf_ctrls->min_perf);
+ if (ret)
+ return ret;
+ }
+ if ((updates & CPPC_UPDATE_MAX) && !CPC_IN_PCC(max_reg)) {
+ ret = cpc_write(cpu, max_reg, perf_ctrls->max_perf);
+ if (ret)
+ return ret;
+ }
+
+ /* Keep Desired Performance valid in every intermediate PCC command. */
+ if (((updates & CPPC_UPDATE_DESIRED) || sync_desired) &&
+ CPC_IN_PCC(desired_reg)) {
+ ret = cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
+ if (ret)
+ return ret;
+ pcc_update = true;
+ }
+ if ((updates & CPPC_UPDATE_MIN) && CPC_IN_PCC(min_reg)) {
+ ret = cpc_write(cpu, min_reg, perf_ctrls->min_perf);
+ if (ret)
+ return ret;
+ pcc_update = true;
+ }
+ if ((updates & CPPC_UPDATE_MAX) && CPC_IN_PCC(max_reg)) {
+ ret = cpc_write(cpu, max_reg, perf_ctrls->max_perf);
+ if (ret)
+ return ret;
+ pcc_update = true;
+ }
+ if (!pcc_update)
+ return 0;
+
+ pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
+ pcc_ss_data = pcc_data[pcc_ss_id];
+ WRITE_ONCE(pcc_ss_data->pending_pcc_write_cmd, true);
+ cpc_desc->write_cmd_id = pcc_ss_data->pcc_write_cnt;
+ cpc_desc->write_cmd_status = 0;
+ return send_pcc_cmd(pcc_ss_id, CMD_WRITE);
+}
+
+/*
+ * Move from the live interval to the requested interval without exposing an
+ * invalid control tuple: widen, update Desired Performance, then narrow.
+ */
+static int cppc_write_ordered_perf(int cpu,
+ struct cppc_perf_ctrls *perf_ctrls,
+ bool desired_update, bool min_update,
+ bool max_update)
+{
+ struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+ struct cpc_register_resource *desired_reg =
+ &cpc_desc->cpc_regs[DESIRED_PERF];
+ unsigned int widen = 0, narrow = 0;
+ u32 current_min, current_max;
+ u32 target_min, target_max;
+ bool desired_done = false;
+ bool sync_desired;
+ int ret;
+
+ ret = cppc_read_perf_limits(cpu, ¤t_min, ¤t_max);
+ if (ret)
+ return ret;
+
+ target_min = min_update ? perf_ctrls->min_perf : current_min;
+ target_max = max_update ? perf_ctrls->max_perf : current_max;
+
+ /*
+ * Check only the new interval. If the live one is inverted, this
+ * update fixes it.
+ */
+ if (target_min > target_max)
+ return -EINVAL;
+
+ if (desired_update && perf_ctrls->desired_perf &&
+ (perf_ctrls->desired_perf < target_min ||
+ perf_ctrls->desired_perf > target_max))
+ return -EINVAL;
+
+ if (min_update) {
+ if (perf_ctrls->min_perf < current_min)
+ widen |= CPPC_UPDATE_MIN;
+ else
+ narrow |= CPPC_UPDATE_MIN;
+ }
+ if (max_update) {
+ if (perf_ctrls->max_perf > current_max)
+ widen |= CPPC_UPDATE_MAX;
+ else
+ narrow |= CPPC_UPDATE_MAX;
+ }
+
+ if (widen) {
+ sync_desired = desired_update && CPC_IN_PCC(desired_reg) &&
+ cppc_perf_updates_in_pcc(cpu, widen);
+ ret = cppc_write_perf_controls(cpu, perf_ctrls, widen,
+ sync_desired);
+ if (ret)
+ return ret;
+ desired_done = sync_desired;
+ }
+
+ if (desired_update && !desired_done) {
+ ret = cppc_write_perf_controls(cpu, perf_ctrls,
+ CPPC_UPDATE_DESIRED, false);
+ if (ret)
+ return ret;
+ }
+
+ if (!narrow)
+ return 0;
+
+ sync_desired = desired_update && CPC_IN_PCC(desired_reg) &&
+ cppc_perf_updates_in_pcc(cpu, narrow);
+ return cppc_write_perf_controls(cpu, perf_ctrls, narrow, sync_desired);
+}
+
/**
* cppc_set_perf - Set a CPU's performance controls.
* @cpu: CPU for which to set performance controls.
* @perf_ctrls: ptr to cppc_perf_ctrls. See cppc_acpi.h
*
+ * Callers must serialize updates to the same CPU's performance controls.
+ * Limit updates preserve MIN <= MAX, including intermediate states on
+ * directly accessed registers. An error may leave an earlier write applied.
+ *
* Return: 0 for success, -ERRNO otherwise.
*/
int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
@@ -3502,10 +3703,22 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
(perf_ctrls->min_perf || perf_ctrls->min_perf_valid);
max_update = cpc_is_writable(max_perf_reg) &&
perf_ctrls->max_perf;
+
+ if (min_update && max_update &&
+ perf_ctrls->min_perf > perf_ctrls->max_perf)
+ return -EINVAL;
+
+ if (desired_update && perf_ctrls->desired_perf &&
+ min_update && max_update &&
+ (perf_ctrls->desired_perf < perf_ctrls->min_perf ||
+ perf_ctrls->desired_perf > perf_ctrls->max_perf))
+ return -EINVAL;
+
desired_pcc = desired_update && CPC_IN_PCC(desired_reg);
min_pcc = min_update && CPC_IN_PCC(min_perf_reg);
max_pcc = max_update && CPC_IN_PCC(max_perf_reg);
pcc_update = desired_pcc || min_pcc || max_pcc;
+
pcc_layout = (cpc_is_writable(desired_reg) && CPC_IN_PCC(desired_reg)) ||
(cpc_is_writable(min_perf_reg) && CPC_IN_PCC(min_perf_reg)) ||
(cpc_is_writable(max_perf_reg) && CPC_IN_PCC(max_perf_reg));
@@ -3531,9 +3744,10 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
* A mixed layout cannot batch fallible direct writes safely: another
* CPU's staged PCC values may no longer match if a direct write fails.
* Serialize the complete mixed transaction and drain an older batch
- * before changing a direct control.
+ * before changing a direct control. A single PCC limit update also
+ * needs the exclusive lock to read and check the unchanged limit.
*/
- if (mixed_layout) {
+ if (mixed_layout || (pcc_update && min_update != max_update)) {
down_write(&pcc_ss_data->pcc_lock);
if (pcc_ss_data->pending_pcc_write_cmd) {
ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
@@ -3547,50 +3761,14 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
goto out_mixed_unlock;
}
- if (desired_update && !desired_pcc) {
- ret = cpc_write(cpu, desired_reg,
- perf_ctrls->desired_perf);
- if (ret)
- goto out_mixed_unlock;
- }
- if (min_update && !min_pcc) {
- ret = cpc_write(cpu, min_perf_reg,
- perf_ctrls->min_perf);
- if (ret)
- goto out_mixed_unlock;
- }
- if (max_update && !max_pcc) {
- ret = cpc_write(cpu, max_perf_reg,
- perf_ctrls->max_perf);
- if (ret)
- goto out_mixed_unlock;
- }
-
- if (desired_pcc) {
- ret = cpc_write(cpu, desired_reg,
- perf_ctrls->desired_perf);
- if (ret)
- goto out_mixed_unlock;
- }
- if (min_pcc) {
- ret = cpc_write(cpu, min_perf_reg,
- perf_ctrls->min_perf);
- if (ret)
- goto out_mixed_unlock;
- }
- if (max_pcc) {
- ret = cpc_write(cpu, max_perf_reg,
- perf_ctrls->max_perf);
- if (ret)
- goto out_mixed_unlock;
- }
-
- if (pcc_update) {
- WRITE_ONCE(pcc_ss_data->pending_pcc_write_cmd, true);
- cpc_desc->write_cmd_id = pcc_ss_data->pcc_write_cnt;
- cpc_desc->write_cmd_status = 0;
- ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
- }
+ if (min_update || max_update)
+ ret = cppc_write_ordered_perf(cpu, perf_ctrls,
+ desired_update,
+ min_update, max_update);
+ else if (desired_update)
+ ret = cppc_write_perf_controls(cpu, perf_ctrls,
+ CPPC_UPDATE_DESIRED,
+ false);
out_mixed_unlock:
up_write(&pcc_ss_data->pcc_lock);
@@ -3599,22 +3777,14 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
/* A request without PCC updates has no payload to coordinate. */
if (!pcc_update) {
- if (desired_update) {
- ret = cpc_write(cpu, desired_reg,
- perf_ctrls->desired_perf);
- if (ret)
- return ret;
- }
- if (min_update) {
- ret = cpc_write(cpu, min_perf_reg,
- perf_ctrls->min_perf);
- if (ret)
- return ret;
- }
- if (max_update)
- ret = cpc_write(cpu, max_perf_reg,
- perf_ctrls->max_perf);
- return ret;
+ if (min_update || max_update)
+ return cppc_write_ordered_perf(cpu, perf_ctrls,
+ desired_update,
+ min_update, max_update);
+ if (desired_update)
+ return cpc_write(cpu, desired_reg,
+ perf_ctrls->desired_perf);
+ return 0;
}
down_read(&pcc_ss_data->pcc_lock); /* BEGIN Phase-I */
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 4ea444ff889e..709363e6d98d 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -335,47 +335,92 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
unsigned int relation)
{
struct cppc_cpudata *cpu_data = policy->driver_data;
+ struct cppc_perf_ctrls perf_ctrls = {};
unsigned int cpu = policy->cpu;
struct cpufreq_freqs freqs;
+ u32 desired_perf;
int ret = 0;
- cpu_data->perf_ctrls.desired_perf =
- cppc_khz_to_perf(&cpu_data->perf_caps, target_freq);
- cppc_cpufreq_update_perf_limits(cpu_data, policy);
+ desired_perf = cppc_khz_to_perf(&cpu_data->perf_caps, target_freq);
+ cppc_cpufreq_get_perf_limits(cpu_data, policy, &perf_ctrls.min_perf,
+ &perf_ctrls.max_perf);
+ perf_ctrls.min_perf_valid = true;
+
+ /*
+ * cppc_set_perf() rejects a Desired value outside [MIN, MAX].
+ * Rounding can put it just outside at the ends of the range. The
+ * policy limits may also change after the governor picked
+ * target_freq.
+ */
+ perf_ctrls.desired_perf = clamp(desired_perf, perf_ctrls.min_perf,
+ perf_ctrls.max_perf);
freqs.old = policy->cur;
freqs.new = target_freq;
cpufreq_freq_transition_begin(policy, &freqs);
- ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
+ ret = cppc_set_perf(cpu, &perf_ctrls);
cpufreq_freq_transition_end(policy, &freqs, ret != 0);
- if (ret)
+ if (ret) {
+ WRITE_ONCE(cpu_data->perf_limits_valid, false);
pr_debug("Failed to set target on CPU:%d. ret:%d\n",
cpu, ret);
+ return ret;
+ }
- return ret;
+ WRITE_ONCE(cpu_data->perf_ctrls.desired_perf,
+ perf_ctrls.desired_perf);
+ WRITE_ONCE(cpu_data->perf_ctrls.min_perf, perf_ctrls.min_perf);
+ WRITE_ONCE(cpu_data->perf_ctrls.max_perf, perf_ctrls.max_perf);
+ WRITE_ONCE(cpu_data->perf_limits_valid, true);
+
+ return 0;
}
static unsigned int cppc_cpufreq_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
{
struct cppc_cpudata *cpu_data = policy->driver_data;
+ struct cppc_perf_ctrls perf_ctrls = {};
unsigned int cpu = policy->cpu;
- u32 desired_perf;
+ u32 desired_perf, min_perf, max_perf;
+ bool update_limits;
int ret;
desired_perf = cppc_khz_to_perf(&cpu_data->perf_caps, target_freq);
- cpu_data->perf_ctrls.desired_perf = desired_perf;
- cppc_cpufreq_update_perf_limits(cpu_data, policy);
+ cppc_cpufreq_get_perf_limits(cpu_data, policy, &min_perf, &max_perf);
- ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
+ /*
+ * cppc_set_perf() rejects a Desired value outside [MIN, MAX], and
+ * rounding or a policy limit change can put it there.
+ */
+ desired_perf = clamp(desired_perf, min_perf, max_perf);
+ perf_ctrls.desired_perf = desired_perf;
+ update_limits = !READ_ONCE(cpu_data->perf_limits_valid) ||
+ min_perf != READ_ONCE(cpu_data->perf_ctrls.min_perf) ||
+ max_perf != READ_ONCE(cpu_data->perf_ctrls.max_perf);
+ if (update_limits) {
+ perf_ctrls.min_perf = min_perf;
+ perf_ctrls.max_perf = max_perf;
+ perf_ctrls.min_perf_valid = true;
+ }
+
+ ret = cppc_set_perf(cpu, &perf_ctrls);
if (ret) {
+ WRITE_ONCE(cpu_data->perf_limits_valid, false);
pr_debug("Failed to set target on CPU:%d. ret:%d\n",
cpu, ret);
return 0;
}
+ WRITE_ONCE(cpu_data->perf_ctrls.desired_perf, desired_perf);
+ if (update_limits) {
+ WRITE_ONCE(cpu_data->perf_ctrls.min_perf, min_perf);
+ WRITE_ONCE(cpu_data->perf_ctrls.max_perf, max_perf);
+ WRITE_ONCE(cpu_data->perf_limits_valid, true);
+ }
+
return target_freq;
}
@@ -670,6 +715,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
unsigned int cpu = policy->cpu;
struct cppc_cpudata *cpu_data;
struct cppc_perf_caps *caps;
+ u32 desired_perf;
int ret;
cpu_data = cppc_cpufreq_get_cpu_data(cpu);
@@ -728,16 +774,39 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
if (caps->highest_perf > caps->nominal_perf)
policy->boost_supported = true;
- /* Set policy->cur to max now. The governors will adjust later. */
- policy->cur = cppc_perf_to_khz(caps, caps->highest_perf);
- cpu_data->perf_ctrls.desired_perf = caps->highest_perf;
+ /*
+ * The request starts from the live limits, which may be unusable.
+ * Firmware may leave them zeroed. An interrupted earlier update may
+ * have left MIN above MAX. cppc_set_perf() would reject either, so
+ * request the full range instead.
+ */
+ if (!cpu_data->perf_ctrls.max_perf ||
+ cpu_data->perf_ctrls.min_perf > cpu_data->perf_ctrls.max_perf) {
+ cpu_data->perf_ctrls.min_perf = caps->lowest_perf;
+ cpu_data->perf_ctrls.min_perf_valid = true;
+ cpu_data->perf_ctrls.max_perf = caps->highest_perf;
+ }
+
+ /*
+ * Keep the initial request within controls left by an earlier driver
+ * instance or firmware. The governor will update the full tuple later.
+ */
+ desired_perf = caps->highest_perf;
+ if (cpu_data->perf_ctrls.max_perf)
+ desired_perf = min(desired_perf, cpu_data->perf_ctrls.max_perf);
+ if (cpu_data->perf_ctrls.min_perf_valid)
+ desired_perf = max(desired_perf, cpu_data->perf_ctrls.min_perf);
+
+ policy->cur = cppc_perf_to_khz(caps, desired_perf);
+ cpu_data->perf_ctrls.desired_perf = desired_perf;
ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
if (ret) {
pr_debug("Err setting perf value:%d on CPU:%d. ret:%d\n",
- caps->highest_perf, cpu, ret);
+ desired_perf, cpu, ret);
goto out;
}
+ cpu_data->perf_limits_valid = true;
cppc_cpufreq_cpu_fie_init(policy);
return 0;
@@ -757,6 +826,8 @@ static void cppc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
cppc_cpufreq_cpu_fie_exit(policy);
cpu_data->perf_ctrls.desired_perf = caps->lowest_perf;
+ cpu_data->perf_ctrls.min_perf = caps->lowest_perf;
+ cpu_data->perf_ctrls.min_perf_valid = true;
ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
if (ret)
@@ -766,6 +837,16 @@ static void cppc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
cppc_cpufreq_put_cpu_data(policy);
}
+static int cppc_cpufreq_cpu_resume(struct cpufreq_policy *policy)
+{
+ struct cppc_cpudata *cpu_data = policy->driver_data;
+
+ /* The platform may have reset the controls during suspend. */
+ WRITE_ONCE(cpu_data->perf_limits_valid, false);
+
+ return 0;
+}
+
static inline u64 get_delta(u64 t1, u64 t0)
{
if (t1 > t0 || t0 > ~(u32)0)
@@ -910,6 +991,7 @@ static ssize_t store_auto_select(struct cpufreq_policy *policy,
cpu_data->perf_ctrls.auto_sel = val;
if (val) {
+ u32 old_desired_perf = cpu_data->perf_ctrls.desired_perf;
u32 old_min_perf = cpu_data->perf_ctrls.min_perf;
u32 old_max_perf = cpu_data->perf_ctrls.max_perf;
@@ -919,15 +1001,22 @@ static ssize_t store_auto_select(struct cpufreq_policy *policy,
* uses the correct performance bounds immediately.
*/
cppc_cpufreq_update_perf_limits(cpu_data, policy);
+ /* The cached Desired may be outside the new MIN and MAX. */
+ cpu_data->perf_ctrls.desired_perf =
+ clamp(old_desired_perf, cpu_data->perf_ctrls.min_perf,
+ cpu_data->perf_ctrls.max_perf);
ret = cppc_set_perf(policy->cpu, &cpu_data->perf_ctrls);
if (ret) {
+ WRITE_ONCE(cpu_data->perf_limits_valid, false);
+ cpu_data->perf_ctrls.desired_perf = old_desired_perf;
cpu_data->perf_ctrls.min_perf = old_min_perf;
cpu_data->perf_ctrls.max_perf = old_max_perf;
cppc_set_auto_sel(policy->cpu, old_auto_sel);
cpu_data->perf_ctrls.auto_sel = old_auto_sel;
return ret;
}
+ WRITE_ONCE(cpu_data->perf_limits_valid, true);
}
return count;
@@ -1060,6 +1149,7 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
.fast_switch = cppc_cpufreq_fast_switch,
.init = cppc_cpufreq_cpu_init,
.exit = cppc_cpufreq_cpu_exit,
+ .resume = cppc_cpufreq_cpu_resume,
.set_boost = cppc_cpufreq_set_boost,
.attr = cppc_cpufreq_attr,
.name = "cppc_cpufreq",
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 5bcdb0e553b9..4495ee4ab71e 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -165,6 +165,7 @@ struct cppc_cpudata {
struct cppc_perf_fb_ctrs perf_fb_ctrs;
unsigned int shared_type;
cpumask_var_t shared_cpu_map;
+ bool perf_limits_valid;
};
#ifdef CONFIG_ACPI_CPPC_LIB
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v6 2/5] cpufreq: CPPC: Keep the policy across CPU hotplug
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 ` Sumit Gupta
2026-10-20 12:18 ` [PATCH v6 3/5] ACPI: CPPC: Make autonomous selection helpers take a u64 Sumit Gupta
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Sumit Gupta @ 2026-10-20 12:18 UTC (permalink / raw)
To: rafael, viresh.kumar, pierre.gondois, christian.loehle,
ionela.voinescu, zhenglifeng1, zhanjie9, lenb, ray.huang,
mario.limonciello, perry.yuan, kprateek.nayak,
maciej.wieczor-retman, pawel.chmielewski, linux-pm, linux-acpi,
acpica-devel, linux-kernel
Cc: linux-tegra, treding, jonathanh, vsethi, ksitaraman, sanjayc,
mochs, bbasu, sumitg
Without online()/offline() callbacks, the cpufreq core calls exit() when
a policy's last online CPU goes down. That drops the driver's per-policy
data, which init() rebuilds when a CPU comes back.
Add lightweight online()/offline() callbacks so the core instead keeps
the policy live and reuses the driver's cpu_data across CPU hotplug.
init() then runs once instead of on every hotplug, making CPU hotplug
faster.
The driver can now save what the OS set in offline() and put it back in
online(). A later patch in this series uses this to preserve the OSPM-set
registers.
Move what init() and exit() did on hotplug into the new callbacks:
- offline() requests the lowest performance and stops the frequency
invariance updates, as exit() did. It lowers MIN with the request, so
that the parked controls stay valid.
- online() re-enables CPPC and restores the performance controls, as
the platform may have reset them. Failures are logged, not returned,
as the core would free the policy. It also restarts the frequency
invariance updates with a new counter snapshot, as init() did, so
that no sample spans the offline window.
The restore reprograms both limits, so it also refreshes the limits
cached for the fast path.
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
drivers/cpufreq/cppc_cpufreq.c | 106 ++++++++++++++++++++++++++++++++-
1 file changed, 105 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 709363e6d98d..7448ca43b363 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -209,7 +209,7 @@ static void cppc_cpufreq_cpu_fie_exit(struct cpufreq_policy *policy)
if (fie_disabled)
return;
- /* policy->cpus will be empty here, use related_cpus instead */
+ /* policy->cpus excludes the offline CPUs, use related_cpus */
topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_CPPC, policy->related_cpus);
for_each_cpu(cpu, policy->related_cpus) {
@@ -837,6 +837,108 @@ static void cppc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
cppc_cpufreq_put_cpu_data(policy);
}
+/*
+ * Run when the policy's first CPU comes back online, the counterpart of
+ * offline().
+ *
+ * The platform may have disabled CPPC and reset the performance controls
+ * (desired, min and max performance) while the CPU was offline, so re-enable
+ * CPPC and reprogram them.
+ *
+ * Report failures without returning them, or the core would free the policy and
+ * leave the CPU without cpufreq. A failed write to the performance controls is
+ * not fatal, as the governor's next request programs them again. A failed CPPC
+ * enable stops the restore, as the writes that follow may not reach the
+ * platform.
+ */
+static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
+{
+ struct cppc_cpudata *cpu_data = policy->driver_data;
+ u32 desired_perf, min_perf, max_perf;
+ struct cppc_perf_ctrls perf_ctrls = {};
+ unsigned int cpu = policy->cpu;
+ int ret;
+
+ /*
+ * The platform may have reset the controls, so drop the limits cached
+ * for the fast path until the restore below reprograms them.
+ */
+ WRITE_ONCE(cpu_data->perf_limits_valid, false);
+
+ ret = cppc_set_enable(cpu, true);
+ if (ret && ret != -EOPNOTSUPP) {
+ pr_warn("Failed to re-enable CPPC for CPU%u (%d)\n", cpu, ret);
+ goto out_fie;
+ }
+
+ /*
+ * Recompute min/max from the policy, clamp the last request into range,
+ * and reprogram the performance controls. Publish the values only once
+ * the platform has taken them.
+ */
+ cppc_cpufreq_get_perf_limits(cpu_data, policy, &min_perf, &max_perf);
+ desired_perf = clamp_t(u32, cpu_data->perf_ctrls.desired_perf,
+ min_perf, max_perf);
+
+ perf_ctrls.desired_perf = desired_perf;
+ perf_ctrls.min_perf = min_perf;
+ perf_ctrls.max_perf = max_perf;
+ perf_ctrls.min_perf_valid = true;
+
+ ret = cppc_set_perf(cpu, &perf_ctrls);
+ if (ret) {
+ pr_debug("Failed to restore perf controls on CPU%u (%d)\n",
+ cpu, ret);
+ } else {
+ WRITE_ONCE(cpu_data->perf_ctrls.desired_perf, desired_perf);
+ WRITE_ONCE(cpu_data->perf_ctrls.min_perf, min_perf);
+ WRITE_ONCE(cpu_data->perf_ctrls.max_perf, max_perf);
+ WRITE_ONCE(cpu_data->perf_limits_valid, true);
+ }
+
+out_fie:
+ /* Restart what offline() stopped, with a new counter snapshot. */
+ cppc_cpufreq_cpu_fie_init(policy);
+
+ return 0;
+}
+
+/*
+ * Run when the policy's last online CPU goes down, undoing what online() did.
+ * Defining offline() is what makes the core keep the policy alive instead of
+ * tearing it down.
+ */
+static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
+{
+ struct cppc_cpudata *cpu_data = policy->driver_data;
+ struct cppc_perf_ctrls perf_ctrls = cpu_data->perf_ctrls;
+ unsigned int cpu = policy->cpu;
+ int ret;
+
+ /*
+ * Stop the frequency invariance updates and cancel the pending work, so
+ * that no sample spans the offline window. online() restarts them with
+ * a new counter snapshot.
+ */
+ cppc_cpufreq_cpu_fie_exit(policy);
+
+ /*
+ * Request the lowest performance while the policy has no online CPU,
+ * lowering MIN with it so that the request stays a valid tuple. MAX
+ * keeps the value the policy last programmed.
+ */
+ perf_ctrls.desired_perf = cpu_data->perf_caps.lowest_perf;
+ perf_ctrls.min_perf = cpu_data->perf_caps.lowest_perf;
+ perf_ctrls.min_perf_valid = true;
+
+ ret = cppc_set_perf(cpu, &perf_ctrls);
+ if (ret)
+ pr_debug("Err setting perf value:%u on CPU:%u. ret:%d\n",
+ cpu_data->perf_caps.lowest_perf, cpu, ret);
+
+ return 0;
+}
+
static int cppc_cpufreq_cpu_resume(struct cpufreq_policy *policy)
{
struct cppc_cpudata *cpu_data = policy->driver_data;
@@ -1149,6 +1251,8 @@ static struct cpufreq_driver cppc_cpufreq_driver = {
.fast_switch = cppc_cpufreq_fast_switch,
.init = cppc_cpufreq_cpu_init,
.exit = cppc_cpufreq_cpu_exit,
+ .online = cppc_cpufreq_cpu_online,
+ .offline = cppc_cpufreq_cpu_offline,
.resume = cppc_cpufreq_cpu_resume,
.set_boost = cppc_cpufreq_set_boost,
.attr = cppc_cpufreq_attr,
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v6 3/5] ACPI: CPPC: Make autonomous selection helpers take a u64
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 ` Sumit Gupta
2026-10-20 16:10 ` Christian Loehle
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 ` [PATCH v6 5/5] cpufreq: CPPC: Preserve OSPM-set registers across suspend/resume Sumit Gupta
4 siblings, 1 reply; 8+ messages in thread
From: Sumit Gupta @ 2026-10-20 12:18 UTC (permalink / raw)
To: rafael, viresh.kumar, pierre.gondois, christian.loehle,
ionela.voinescu, zhenglifeng1, zhanjie9, lenb, ray.huang,
mario.limonciello, perry.yuan, kprateek.nayak,
maciej.wieczor-retman, pawel.chmielewski, linux-pm, linux-acpi,
acpica-devel, linux-kernel
Cc: linux-tegra, treding, jonathanh, vsethi, ksitaraman, sanjayc,
mochs, bbasu, sumitg
cppc_get_auto_sel()/cppc_set_auto_sel() use a bool, unlike the other
CPPC register get/set helpers which use a u64.
The next patch in this series saves and restores the OSPM-set registers
across CPU hotplug and driver unload through a common table of register
get/set helpers that all take a u64. The bool autonomous selection
helpers cannot be added to that table.
Change cppc_get_auto_sel()/cppc_set_auto_sel() to take a u64 so the
autonomous selection register fits alongside the others, and update
their callers.
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
---
drivers/acpi/cppc_acpi.c | 20 ++++----------------
drivers/cpufreq/amd-pstate.c | 2 +-
drivers/cpufreq/cppc_cpufreq.c | 4 ++--
include/acpi/cppc_acpi.h | 8 ++++----
4 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 8d38924e0750..11a895fa9199 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -3304,23 +3304,11 @@ EXPORT_SYMBOL_GPL(cppc_set_auto_act_window);
/**
* cppc_get_auto_sel() - Read autonomous selection register.
* @cpu: CPU from which to read register.
- * @enable: Return address.
+ * @enable: Return address, set to 0 or 1.
*/
-int cppc_get_auto_sel(int cpu, bool *enable)
+int cppc_get_auto_sel(int cpu, u64 *enable)
{
- u64 auto_sel;
- int ret;
-
- if (enable == NULL)
- return -EINVAL;
-
- ret = cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, &auto_sel);
- if (ret)
- return ret;
-
- *enable = (bool)auto_sel;
-
- return 0;
+ return cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, enable);
}
EXPORT_SYMBOL_GPL(cppc_get_auto_sel);
@@ -3351,7 +3339,7 @@ EXPORT_SYMBOL_GPL(cppc_auto_sel_is_immutable);
* @cpu : CPU to which to write register.
* @enable : the desired value of autonomous selection resiter to be updated.
*/
-int cppc_set_auto_sel(int cpu, bool enable)
+int cppc_set_auto_sel(int cpu, u64 enable)
{
return cppc_set_reg_val(cpu, AUTO_SEL_ENABLE, enable);
}
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 4bedfbfc18b6..bf7d25e89d2a 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -546,7 +546,7 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
struct cppc_perf_caps cppc_perf;
union perf_cached perf = READ_ONCE(cpudata->perf);
u64 numerator;
- bool auto_sel;
+ u64 auto_sel;
int ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
if (ret)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 7448ca43b363..9e1214e1cb0d 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -1059,7 +1059,7 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
{
- bool val;
+ u64 val;
int ret;
ret = cppc_get_auto_sel(policy->cpu, &val);
@@ -1071,7 +1071,7 @@ static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
if (ret)
return ret;
- return sysfs_emit(buf, "%d\n", val);
+ return sysfs_emit(buf, "%llu\n", val);
}
static ssize_t store_auto_select(struct cpufreq_policy *policy,
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 4495ee4ab71e..a5545fd3f8bd 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -197,9 +197,9 @@ extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool e
extern int cppc_set_epp(int cpu, u64 epp_val);
extern int cppc_get_auto_act_window(int cpu, u64 *auto_act_window);
extern int cppc_set_auto_act_window(int cpu, u64 auto_act_window);
-extern int cppc_get_auto_sel(int cpu, bool *enable);
+extern int cppc_get_auto_sel(int cpu, u64 *enable);
bool cppc_auto_sel_is_immutable(int cpu);
-extern int cppc_set_auto_sel(int cpu, bool enable);
+extern int cppc_set_auto_sel(int cpu, u64 enable);
extern int cppc_get_perf_limited(int cpu, u64 *perf_limited);
extern int cppc_set_perf_limited(int cpu, u64 bits_to_clear);
extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf);
@@ -298,7 +298,7 @@ static inline int cppc_set_auto_act_window(int cpu, u64 auto_act_window)
{
return -EOPNOTSUPP;
}
-static inline int cppc_get_auto_sel(int cpu, bool *enable)
+static inline int cppc_get_auto_sel(int cpu, u64 *enable)
{
return -EOPNOTSUPP;
}
@@ -308,7 +308,7 @@ static inline bool cppc_auto_sel_is_immutable(int cpu)
return false;
}
-static inline int cppc_set_auto_sel(int cpu, bool enable)
+static inline int cppc_set_auto_sel(int cpu, u64 enable)
{
return -EOPNOTSUPP;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v6 3/5] ACPI: CPPC: Make autonomous selection helpers take a u64
2026-10-20 12:18 ` [PATCH v6 3/5] ACPI: CPPC: Make autonomous selection helpers take a u64 Sumit Gupta
@ 2026-10-20 16:10 ` Christian Loehle
0 siblings, 0 replies; 8+ messages in thread
From: Christian Loehle @ 2026-10-20 16:10 UTC (permalink / raw)
To: Sumit Gupta, rafael, viresh.kumar, pierre.gondois,
ionela.voinescu, zhenglifeng1, zhanjie9, lenb, ray.huang,
mario.limonciello, perry.yuan, kprateek.nayak,
maciej.wieczor-retman, pawel.chmielewski, linux-pm, linux-acpi,
acpica-devel, linux-kernel
Cc: linux-tegra, treding, jonathanh, vsethi, ksitaraman, sanjayc,
mochs, bbasu
On 10/20/26 13:18, Sumit Gupta wrote:
> cppc_get_auto_sel()/cppc_set_auto_sel() use a bool, unlike the other
> CPPC register get/set helpers which use a u64.
>
> The next patch in this series saves and restores the OSPM-set registers
> across CPU hotplug and driver unload through a common table of register
> get/set helpers that all take a u64. The bool autonomous selection
> helpers cannot be added to that table.
>
> Change cppc_get_auto_sel()/cppc_set_auto_sel() to take a u64 so the
> autonomous selection register fits alongside the others, and update
> their callers.
>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
> ---
> drivers/acpi/cppc_acpi.c | 20 ++++----------------
> drivers/cpufreq/amd-pstate.c | 2 +-
> drivers/cpufreq/cppc_cpufreq.c | 4 ++--
> include/acpi/cppc_acpi.h | 8 ++++----
> 4 files changed, 11 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 8d38924e0750..11a895fa9199 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -3304,23 +3304,11 @@ EXPORT_SYMBOL_GPL(cppc_set_auto_act_window);
> /**
> * cppc_get_auto_sel() - Read autonomous selection register.
> * @cpu: CPU from which to read register.
> - * @enable: Return address.
> + * @enable: Return address, set to 0 or 1.
> */
> -int cppc_get_auto_sel(int cpu, bool *enable)
> +int cppc_get_auto_sel(int cpu, u64 *enable)
> {
> - u64 auto_sel;
> - int ret;
> -
> - if (enable == NULL)
> - return -EINVAL;
> -
> - ret = cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, &auto_sel);
> - if (ret)
> - return ret;
> -
> - *enable = (bool)auto_sel;
> -
> - return 0;
> + return cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, enable);
> }
> EXPORT_SYMBOL_GPL(cppc_get_auto_sel);
>
> @@ -3351,7 +3339,7 @@ EXPORT_SYMBOL_GPL(cppc_auto_sel_is_immutable);
> * @cpu : CPU to which to write register.
> * @enable : the desired value of autonomous selection resiter to be updated.
> */
> -int cppc_set_auto_sel(int cpu, bool enable)
> +int cppc_set_auto_sel(int cpu, u64 enable)
> {
> return cppc_set_reg_val(cpu, AUTO_SEL_ENABLE, enable);
> }
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 4bedfbfc18b6..bf7d25e89d2a 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -546,7 +546,7 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
> struct cppc_perf_caps cppc_perf;
> union perf_cached perf = READ_ONCE(cpudata->perf);
> u64 numerator;
> - bool auto_sel;
> + u64 auto_sel;
>
> int ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
> if (ret)
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 7448ca43b363..9e1214e1cb0d 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -1059,7 +1059,7 @@ static ssize_t show_freqdomain_cpus(struct cpufreq_policy *policy, char *buf)
>
> static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
> {
> - bool val;
> + u64 val;
> int ret;
>
> ret = cppc_get_auto_sel(policy->cpu, &val);
> @@ -1071,7 +1071,7 @@ static ssize_t show_auto_select(struct cpufreq_policy *policy, char *buf)
> if (ret)
> return ret;
>
> - return sysfs_emit(buf, "%d\n", val);
> + return sysfs_emit(buf, "%llu\n", val);
> }
>
> static ssize_t store_auto_select(struct cpufreq_policy *policy,
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 4495ee4ab71e..a5545fd3f8bd 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -197,9 +197,9 @@ extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool e
> extern int cppc_set_epp(int cpu, u64 epp_val);
> extern int cppc_get_auto_act_window(int cpu, u64 *auto_act_window);
> extern int cppc_set_auto_act_window(int cpu, u64 auto_act_window);
> -extern int cppc_get_auto_sel(int cpu, bool *enable);
> +extern int cppc_get_auto_sel(int cpu, u64 *enable);
> bool cppc_auto_sel_is_immutable(int cpu);
> -extern int cppc_set_auto_sel(int cpu, bool enable);
> +extern int cppc_set_auto_sel(int cpu, u64 enable);
> extern int cppc_get_perf_limited(int cpu, u64 *perf_limited);
> extern int cppc_set_perf_limited(int cpu, u64 bits_to_clear);
> extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf);
> @@ -298,7 +298,7 @@ static inline int cppc_set_auto_act_window(int cpu, u64 auto_act_window)
> {
> return -EOPNOTSUPP;
> }
> -static inline int cppc_get_auto_sel(int cpu, bool *enable)
> +static inline int cppc_get_auto_sel(int cpu, u64 *enable)
> {
> return -EOPNOTSUPP;
> }
> @@ -308,7 +308,7 @@ static inline bool cppc_auto_sel_is_immutable(int cpu)
> return false;
> }
>
> -static inline int cppc_set_auto_sel(int cpu, bool enable)
> +static inline int cppc_set_auto_sel(int cpu, u64 enable)
> {
> return -EOPNOTSUPP;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v6 4/5] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload
2026-10-20 12:18 [PATCH v6 0/5] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload Sumit Gupta
` (2 preceding siblings ...)
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 ` Sumit Gupta
2026-10-20 16:11 ` Christian Loehle
2026-10-20 12:18 ` [PATCH v6 5/5] cpufreq: CPPC: Preserve OSPM-set registers across suspend/resume Sumit Gupta
4 siblings, 1 reply; 8+ messages in thread
From: Sumit Gupta @ 2026-10-20 12:18 UTC (permalink / raw)
To: rafael, viresh.kumar, pierre.gondois, christian.loehle,
ionela.voinescu, zhenglifeng1, zhanjie9, lenb, ray.huang,
mario.limonciello, perry.yuan, kprateek.nayak,
maciej.wieczor-retman, pawel.chmielewski, linux-pm, linux-acpi,
acpica-devel, linux-kernel
Cc: linux-tegra, treding, jonathanh, vsethi, ksitaraman, sanjayc,
mochs, bbasu, sumitg
Values written to OSPM-set CPPC registers via sysfs can be lost in two
ways:
- Across CPU hotplug: the platform may reset a CPU's registers while it
is offline.
- On driver unload: the value the driver wrote is left in the register
instead of returning to its pre-driver state.
Add a small table-driven mechanism that handles both:
- On init(), capture each register's firmware value before the
driver programs anything.
- On offline(), read back each register's current value (whatever was
last set via sysfs) so it can be reapplied, then restore the firmware
value.
- On online(), reapply the value captured at offline() after
reprogramming the performance controls. A failed write to the controls
does not skip the reapply, as no other path restores these registers.
- On exit(), nothing is needed, as the core calls offline() first, which
already restored the firmware values.
Cover the Autonomous Selection (auto_sel), Energy Performance Preference
(EPP) and Autonomous Activity Window (auto_act_window) registers. Writes
to EPP and auto_act_window only have meaning while auto_sel is enabled,
so write auto_sel before them when enabling it and after them when
disabling it. While autonomous selection stays disabled, the platform may
ignore those writes.
Suggested-by: Pierre Gondois <pierre.gondois@arm.com>
Link: https://lore.kernel.org/all/86780f97-29ee-4a72-b311-38c89434b707@arm.com/
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
drivers/cpufreq/cppc_cpufreq.c | 176 ++++++++++++++++++++++++++++++++-
1 file changed, 173 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 9e1214e1cb0d..1b733f15d641 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -29,6 +29,166 @@
static struct cpufreq_driver cppc_cpufreq_driver;
+/*
+ * OSPM-set CPPC registers tracked for save/restore. A value the OS wrote is
+ * reapplied from online() across CPU hotplug, and the firmware value is
+ * restored from offline().
+ *
+ * Autonomous Selection (auto_sel) is kept first, as the registers after it
+ * only have meaning while it is enabled.
+ */
+enum cppc_saved_reg_id {
+ CPPC_SAVED_AUTO_SEL,
+ CPPC_SAVED_EPP,
+ CPPC_SAVED_AUTO_ACT_WINDOW,
+ CPPC_NR_SAVED_REGS,
+};
+
+struct cppc_saved_reg {
+ const char *name;
+ int (*get)(int cpu, u64 *val);
+ int (*set)(int cpu, u64 val);
+};
+
+static const struct cppc_saved_reg cppc_saved_regs[CPPC_NR_SAVED_REGS] = {
+ [CPPC_SAVED_AUTO_SEL] = {
+ .name = "auto_sel",
+ .get = cppc_get_auto_sel,
+ .set = cppc_set_auto_sel,
+ },
+ [CPPC_SAVED_EPP] = {
+ .name = "epp",
+ .get = cppc_get_epp_perf,
+ .set = cppc_set_epp,
+ },
+ [CPPC_SAVED_AUTO_ACT_WINDOW] = {
+ .name = "auto_act_window",
+ .get = cppc_get_auto_act_window,
+ .set = cppc_set_auto_act_window,
+ },
+};
+
+enum cppc_saved_type {
+ CPPC_SAVED_FIRMWARE,
+ CPPC_SAVED_REQUESTED,
+ CPPC_NR_SAVED_TYPES,
+};
+
+/*
+ * Per-policy values saved for each register in cppc_saved_regs[], U64_MAX
+ * if there is none:
+ * CPPC_SAVED_FIRMWARE - value before the driver touched it, captured at
+ * init() and written back when the policy goes
+ * offline
+ * CPPC_SAVED_REQUESTED - value in effect when the policy last went offline,
+ * reapplied at online()
+ */
+struct cppc_policy_state {
+ u64 regs[CPPC_NR_SAVED_REGS][CPPC_NR_SAVED_TYPES];
+};
+
+static DEFINE_PER_CPU(struct cppc_policy_state, cppc_policy_state);
+
+/*
+ * Per-policy state is kept in the per-CPU variable of the first CPU the policy
+ * manages. related_cpus (the policy's full set of CPUs) never changes while the
+ * policy exists, so this CPU (unlike policy->cpu) stays the same across CPU
+ * hotplug, and every callback reaches the same copy.
+ */
+static struct cppc_policy_state *
+cppc_cpufreq_policy_state(struct cpufreq_policy *policy)
+{
+ const struct cpumask *policy_cpus = policy->related_cpus;
+
+ /*
+ * related_cpus is empty until the core fills it in after init(), so
+ * fall back to policy->cpus, which has the same first CPU.
+ */
+ if (cpumask_empty(policy_cpus))
+ policy_cpus = policy->cpus;
+
+ return &per_cpu(cppc_policy_state, cpumask_first(policy_cpus));
+}
+
+/*
+ * Save each register's current value, either as the firmware value, captured
+ * before the driver programs anything, or as the requested value, to reapply
+ * at online().
+ */
+static void cppc_cpufreq_save_regs(struct cpufreq_policy *policy,
+ enum cppc_saved_type saved_type)
+{
+ struct cppc_policy_state *st = cppc_cpufreq_policy_state(policy);
+ unsigned int cpu = policy->cpu;
+ u64 val;
+ int i;
+
+ for (i = 0; i < CPPC_NR_SAVED_REGS; i++) {
+ if (cppc_saved_regs[i].get(cpu, &val))
+ val = U64_MAX;
+
+ st->regs[i][saved_type] = val;
+ if (saved_type == CPPC_SAVED_FIRMWARE)
+ st->regs[i][CPPC_SAVED_REQUESTED] = U64_MAX;
+ }
+}
+
+/*
+ * Write one tracked register, skipping it when there is no saved value.
+ * A register the platform does not allow writing is not an error.
+ */
+static void cppc_cpufreq_write_saved_reg(unsigned int cpu,
+ enum cppc_saved_reg_id reg, u64 val,
+ enum cppc_saved_type saved_type)
+{
+ const char *op = (saved_type == CPPC_SAVED_FIRMWARE) ?
+ "restore firmware" : "reapply saved";
+ int ret;
+
+ if (val == U64_MAX)
+ return;
+
+ ret = cppc_saved_regs[reg].set(cpu, val);
+ if (ret == -EOPNOTSUPP)
+ return;
+ if (ret)
+ pr_debug("Failed to %s %s=%llu on CPU%u (%d)\n", op,
+ cppc_saved_regs[reg].name, val, cpu, ret);
+}
+
+/*
+ * Apply the saved firmware or requested value to each tracked register.
+ *
+ * Write auto_sel first when the value being applied enables autonomous
+ * selection and last when it disables it, so the writes to the dependent
+ * registers can still take effect. While autonomous selection stays disabled,
+ * the platform may ignore those writes. Do not enable it temporarily to force
+ * them through.
+ */
+static void cppc_cpufreq_apply_saved_regs(struct cpufreq_policy *policy,
+ enum cppc_saved_type saved_type)
+{
+ const struct cppc_policy_state *st = cppc_cpufreq_policy_state(policy);
+ unsigned int cpu = policy->cpu;
+ u64 auto_sel, val;
+ int i;
+
+ auto_sel = st->regs[CPPC_SAVED_AUTO_SEL][saved_type];
+
+ if (auto_sel)
+ cppc_cpufreq_write_saved_reg(cpu, CPPC_SAVED_AUTO_SEL, auto_sel,
+ saved_type);
+
+ for (i = CPPC_SAVED_AUTO_SEL + 1; i < CPPC_NR_SAVED_REGS; i++) {
+ val = st->regs[i][saved_type];
+ cppc_cpufreq_write_saved_reg(cpu, i, val, saved_type);
+ }
+
+ if (!auto_sel)
+ cppc_cpufreq_write_saved_reg(cpu, CPPC_SAVED_AUTO_SEL, auto_sel,
+ saved_type);
+}
+
#ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
static enum {
FIE_UNSET = -1,
@@ -800,6 +960,8 @@ 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_save_regs(policy, CPPC_SAVED_FIRMWARE);
+
ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
if (ret) {
pr_debug("Err setting perf value:%d on CPU:%d. ret:%d\n",
@@ -843,12 +1005,14 @@ static void cppc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
*
* The platform may have disabled CPPC and reset the performance controls
* (desired, min and max performance) while the CPU was offline, so re-enable
- * CPPC and reprogram them.
+ * CPPC and reprogram them. Also reapply the OSPM-set registers that offline()
+ * reset to firmware values.
*
* Report failures without returning them, or the core would free the policy and
* leave the CPU without cpufreq. A failed write to the performance controls is
- * not fatal, as the governor's next request programs them again. A failed CPPC
- * enable stops the restore, as the writes that follow may not reach the
+ * not fatal, as the governor's next request programs them again. The OSPM-set
+ * registers are reapplied even then, as no other path restores them. A failed
+ * CPPC enable skips both restores, as the writes that follow may not reach the
* platform.
*/
static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
@@ -896,6 +1060,8 @@ static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
WRITE_ONCE(cpu_data->perf_limits_valid, true);
}
+ cppc_cpufreq_apply_saved_regs(policy, CPPC_SAVED_REQUESTED);
+
out_fie:
/* Restart what offline() stopped, with a new counter snapshot. */
cppc_cpufreq_cpu_fie_init(policy);
@@ -915,6 +1081,10 @@ 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);
+
/*
* Stop the frequency invariance updates and cancel the pending work, so
* that no sample spans the offline window. online() restarts them with
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v6 4/5] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload
2026-10-20 12:18 ` [PATCH v6 4/5] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload Sumit Gupta
@ 2026-10-20 16:11 ` Christian Loehle
0 siblings, 0 replies; 8+ messages in thread
From: Christian Loehle @ 2026-10-20 16:11 UTC (permalink / raw)
To: Sumit Gupta, rafael, viresh.kumar, pierre.gondois,
ionela.voinescu, zhenglifeng1, zhanjie9, lenb, ray.huang,
mario.limonciello, perry.yuan, kprateek.nayak,
maciej.wieczor-retman, pawel.chmielewski, linux-pm, linux-acpi,
acpica-devel, linux-kernel
Cc: linux-tegra, treding, jonathanh, vsethi, ksitaraman, sanjayc,
mochs, bbasu
On 10/20/26 13:18, Sumit Gupta wrote:
> Values written to OSPM-set CPPC registers via sysfs can be lost in two
> ways:
>
> - Across CPU hotplug: the platform may reset a CPU's registers while it
> is offline.
> - On driver unload: the value the driver wrote is left in the register
> instead of returning to its pre-driver state.
>
> Add a small table-driven mechanism that handles both:
>
> - On init(), capture each register's firmware value before the
> driver programs anything.
> - On offline(), read back each register's current value (whatever was
> last set via sysfs) so it can be reapplied, then restore the firmware
> value.
Sorry for the late suggestion, but all the ospm-set registers I think we can
save ourselves some headache by just saving these at sysfs_store (and not deal
with potential read-errors during offline/suspend (which actually may happen!).
> - On online(), reapply the value captured at offline() after
> reprogramming the performance controls. A failed write to the controls
> does not skip the reapply, as no other path restores these registers.
> - On exit(), nothing is needed, as the core calls offline() first, which
> already restored the firmware values.
>
> Cover the Autonomous Selection (auto_sel), Energy Performance Preference
> (EPP) and Autonomous Activity Window (auto_act_window) registers. Writes
> to EPP and auto_act_window only have meaning while auto_sel is enabled,
> so write auto_sel before them when enabling it and after them when
> disabling it. While autonomous selection stays disabled, the platform may
> ignore those writes.
>
> Suggested-by: Pierre Gondois <pierre.gondois@arm.com>
> Link: https://lore.kernel.org/all/86780f97-29ee-4a72-b311-38c89434b707@arm.com/
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
> drivers/cpufreq/cppc_cpufreq.c | 176 ++++++++++++++++++++++++++++++++-
> 1 file changed, 173 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 9e1214e1cb0d..1b733f15d641 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -29,6 +29,166 @@
>
> static struct cpufreq_driver cppc_cpufreq_driver;
>
> +/*
> + * OSPM-set CPPC registers tracked for save/restore. A value the OS wrote is
> + * reapplied from online() across CPU hotplug, and the firmware value is
> + * restored from offline().
> + *
> + * Autonomous Selection (auto_sel) is kept first, as the registers after it
> + * only have meaning while it is enabled.
> + */
> +enum cppc_saved_reg_id {
> + CPPC_SAVED_AUTO_SEL,
> + CPPC_SAVED_EPP,
> + CPPC_SAVED_AUTO_ACT_WINDOW,
> + CPPC_NR_SAVED_REGS,
> +};
> +
> +struct cppc_saved_reg {
> + const char *name;
> + int (*get)(int cpu, u64 *val);
> + int (*set)(int cpu, u64 val);
> +};
> +
> +static const struct cppc_saved_reg cppc_saved_regs[CPPC_NR_SAVED_REGS] = {
> + [CPPC_SAVED_AUTO_SEL] = {
> + .name = "auto_sel",
> + .get = cppc_get_auto_sel,
> + .set = cppc_set_auto_sel,
> + },
> + [CPPC_SAVED_EPP] = {
> + .name = "epp",
> + .get = cppc_get_epp_perf,
> + .set = cppc_set_epp,
> + },
> + [CPPC_SAVED_AUTO_ACT_WINDOW] = {
> + .name = "auto_act_window",
> + .get = cppc_get_auto_act_window,
> + .set = cppc_set_auto_act_window,
> + },
> +};
> +
> +enum cppc_saved_type {
> + CPPC_SAVED_FIRMWARE,
> + CPPC_SAVED_REQUESTED,
> + CPPC_NR_SAVED_TYPES,
> +};
> +
> +/*
> + * Per-policy values saved for each register in cppc_saved_regs[], U64_MAX
> + * if there is none:
> + * CPPC_SAVED_FIRMWARE - value before the driver touched it, captured at
> + * init() and written back when the policy goes
> + * offline
> + * CPPC_SAVED_REQUESTED - value in effect when the policy last went offline,
> + * reapplied at online()
> + */
> +struct cppc_policy_state {
> + u64 regs[CPPC_NR_SAVED_REGS][CPPC_NR_SAVED_TYPES];
> +};
> +
> +static DEFINE_PER_CPU(struct cppc_policy_state, cppc_policy_state);
> +
> +/*
> + * Per-policy state is kept in the per-CPU variable of the first CPU the policy
> + * manages. related_cpus (the policy's full set of CPUs) never changes while the
> + * policy exists, so this CPU (unlike policy->cpu) stays the same across CPU
> + * hotplug, and every callback reaches the same copy.
> + */
> +static struct cppc_policy_state *
> +cppc_cpufreq_policy_state(struct cpufreq_policy *policy)
> +{
> + const struct cpumask *policy_cpus = policy->related_cpus;
> +
> + /*
> + * related_cpus is empty until the core fills it in after init(), so
> + * fall back to policy->cpus, which has the same first CPU.
> + */
> + if (cpumask_empty(policy_cpus))
> + policy_cpus = policy->cpus;
> +
> + return &per_cpu(cppc_policy_state, cpumask_first(policy_cpus));
> +}
> +
> +/*
> + * Save each register's current value, either as the firmware value, captured
> + * before the driver programs anything, or as the requested value, to reapply
> + * at online().
> + */
> +static void cppc_cpufreq_save_regs(struct cpufreq_policy *policy,
> + enum cppc_saved_type saved_type)
> +{
> + struct cppc_policy_state *st = cppc_cpufreq_policy_state(policy);
> + unsigned int cpu = policy->cpu;
> + u64 val;
> + int i;
> +
> + for (i = 0; i < CPPC_NR_SAVED_REGS; i++) {
> + if (cppc_saved_regs[i].get(cpu, &val))
> + val = U64_MAX;
> +
> + st->regs[i][saved_type] = val;
> + if (saved_type == CPPC_SAVED_FIRMWARE)
> + st->regs[i][CPPC_SAVED_REQUESTED] = U64_MAX;
> + }
> +}
> +
> +/*
> + * Write one tracked register, skipping it when there is no saved value.
> + * A register the platform does not allow writing is not an error.
> + */
> +static void cppc_cpufreq_write_saved_reg(unsigned int cpu,
> + enum cppc_saved_reg_id reg, u64 val,
> + enum cppc_saved_type saved_type)
> +{
> + const char *op = (saved_type == CPPC_SAVED_FIRMWARE) ?
> + "restore firmware" : "reapply saved";
> + int ret;
> +
> + if (val == U64_MAX)
> + return;
> +
> + ret = cppc_saved_regs[reg].set(cpu, val);
> + if (ret == -EOPNOTSUPP)
> + return;
> + if (ret)
> + pr_debug("Failed to %s %s=%llu on CPU%u (%d)\n", op,
> + cppc_saved_regs[reg].name, val, cpu, ret);
> +}
> +
> +/*
> + * Apply the saved firmware or requested value to each tracked register.
> + *
> + * Write auto_sel first when the value being applied enables autonomous
> + * selection and last when it disables it, so the writes to the dependent
> + * registers can still take effect. While autonomous selection stays disabled,
> + * the platform may ignore those writes. Do not enable it temporarily to force
> + * them through.
> + */
> +static void cppc_cpufreq_apply_saved_regs(struct cpufreq_policy *policy,
> + enum cppc_saved_type saved_type)
> +{
> + const struct cppc_policy_state *st = cppc_cpufreq_policy_state(policy);
> + unsigned int cpu = policy->cpu;
> + u64 auto_sel, val;
> + int i;
> +
> + auto_sel = st->regs[CPPC_SAVED_AUTO_SEL][saved_type];
> +
> + if (auto_sel)
> + cppc_cpufreq_write_saved_reg(cpu, CPPC_SAVED_AUTO_SEL, auto_sel,
> + saved_type);
> +
> + for (i = CPPC_SAVED_AUTO_SEL + 1; i < CPPC_NR_SAVED_REGS; i++) {
> + val = st->regs[i][saved_type];
> + cppc_cpufreq_write_saved_reg(cpu, i, val, saved_type);
> + }
> +
> + if (!auto_sel)
> + cppc_cpufreq_write_saved_reg(cpu, CPPC_SAVED_AUTO_SEL, auto_sel,
> + saved_type);
> +}
> +
> #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
> static enum {
> FIE_UNSET = -1,
> @@ -800,6 +960,8 @@ 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_save_regs(policy, CPPC_SAVED_FIRMWARE);
> +
> ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
> if (ret) {
> pr_debug("Err setting perf value:%d on CPU:%d. ret:%d\n",
> @@ -843,12 +1005,14 @@ static void cppc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
> *
> * The platform may have disabled CPPC and reset the performance controls
> * (desired, min and max performance) while the CPU was offline, so re-enable
> - * CPPC and reprogram them.
> + * CPPC and reprogram them. Also reapply the OSPM-set registers that offline()
> + * reset to firmware values.
> *
> * Report failures without returning them, or the core would free the policy and
> * leave the CPU without cpufreq. A failed write to the performance controls is
> - * not fatal, as the governor's next request programs them again. A failed CPPC
> - * enable stops the restore, as the writes that follow may not reach the
> + * not fatal, as the governor's next request programs them again. The OSPM-set
> + * registers are reapplied even then, as no other path restores them. A failed
> + * CPPC enable skips both restores, as the writes that follow may not reach the
> * platform.
> */
> static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
> @@ -896,6 +1060,8 @@ static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
> WRITE_ONCE(cpu_data->perf_limits_valid, true);
> }
>
> + cppc_cpufreq_apply_saved_regs(policy, CPPC_SAVED_REQUESTED);
> +
> out_fie:
> /* Restart what offline() stopped, with a new counter snapshot. */
> cppc_cpufreq_cpu_fie_init(policy);
> @@ -915,6 +1081,10 @@ 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);
> +
> /*
> * Stop the frequency invariance updates and cancel the pending work, so
> * that no sample spans the offline window. online() restarts them with
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v6 5/5] cpufreq: CPPC: Preserve OSPM-set registers across suspend/resume
2026-10-20 12:18 [PATCH v6 0/5] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload Sumit Gupta
` (3 preceding siblings ...)
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
4 siblings, 0 replies; 8+ messages in thread
From: Sumit Gupta @ 2026-10-20 12:18 UTC (permalink / raw)
To: rafael, viresh.kumar, pierre.gondois, christian.loehle,
ionela.voinescu, zhenglifeng1, zhanjie9, lenb, ray.huang,
mario.limonciello, perry.yuan, kprateek.nayak,
maciej.wieczor-retman, pawel.chmielewski, linux-pm, linux-acpi,
acpica-devel, linux-kernel
Cc: linux-tegra, treding, jonathanh, vsethi, ksitaraman, sanjayc,
mochs, bbasu, sumitg
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
^ permalink raw reply related [flat|nested] 8+ messages in thread