[PATCH v2 0/3] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload

[PATCH v2 0/3] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload

[Sumit Gupta]

CPPC registers that OSPM programs at runtime, via sysfs (auto_sel,
energy_performance_preference, auto_act_window) or via the autonomous
mode boot parameter, can be silently lost across CPU hotplug or left
behind on driver unload:

 - Across CPU hotplug: the cpufreq core tears the policy down and the
   platform may reset the registers while the CPU is offline.
 - On driver unload: the value the driver wrote is left in the register
   instead of returning to its pre-driver state.

This series makes cppc_cpufreq keep these values consistent:

 - Patch 1: adds online()/offline() callbacks so core keeps the policy
   alive across CPU hotplug instead of tearing it down and rebuilding it.
 - Patch 2: adds u64 wrappers for the autonomous selection register.
 - Patch 3: adds a table-driven mechanism that captures each register's
   firmware value, restores it when the policy goes down, and reapplies
   the OSPM-set value when the policy comes back online.

v[1] -> v2:
 - New patch 1: keep policy across hotplug by adding online()/offline().
 - New patch 2: move the auto_sel u64 wrappers into cppc_acpi.c.
 - Restore fw values from offline(), which covers both hotplug and unload
 - Drop cppc_cache_perf_ctrls() and read reg values at offline() instead
   of caching them.
 - Add auto_act_window and defer OSPM Nominal Perf to a later series.

Sumit Gupta (3):
  cpufreq: CPPC: Keep the policy across CPU hotplug
  ACPI: CPPC: Add u64 wrappers for the autonomous selection register
  cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload

 drivers/acpi/cppc_acpi.c       |  40 +++++++++
 drivers/cpufreq/cppc_cpufreq.c | 156 +++++++++++++++++++++++++++++++++
 include/acpi/cppc_acpi.h       |  10 +++
 3 files changed, 206 insertions(+)

[1] v1: https://lore.kernel.org/lkml/20260623095403.3407436-1-sumitg@nvidia.com/

-- 
2.34.1

[PATCH v2 1/3] cpufreq: CPPC: Keep the policy across CPU hotplug

[Sumit Gupta]

Without online()/offline() callbacks, the cpufreq core fully tears
down a policy during exit() when its last online CPU is offlined,
and rebuilds it during init() when it 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. This avoids re-reading the CPPC capabilities on every
offline/online, making CPU hotplug faster.

Re-enable CPPC from online(), as it may have been disabled while
the CPU was offline.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/cpufreq/cppc_cpufreq.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index f6cea0c54dd9..432c6a6288a7 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -722,6 +722,31 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	return ret;
 }
 
+/*
+ * With offline() defined, the cpufreq core keeps the policy alive when
+ * a CPU is hotplugged out.
+ */
+static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
+{
+	return 0;
+}
+
+/*
+ * Re-enable CPPC when the policy's CPU comes back online, since the platform
+ * may have disabled it while the CPU was offline.
+ */
+static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
+{
+	unsigned int cpu = policy->cpu;
+	int ret;
+
+	ret = cppc_set_enable(cpu, true);
+	if (ret && ret != -EOPNOTSUPP)
+		pr_warn("Failed to re-enable CPPC for CPU%d (%d)\n", cpu, ret);
+
+	return 0;
+}
+
 static void cppc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
 {
 	struct cppc_cpudata *cpu_data = policy->driver_data;
@@ -1034,6 +1059,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,
 	.set_boost = cppc_cpufreq_set_boost,
 	.attr = cppc_cpufreq_attr,
 	.name = "cppc_cpufreq",
-- 
2.34.1

Re: [PATCH v2 1/3] cpufreq: CPPC: Keep the policy across CPU hotplug

[Christian Loehle]

On 7/16/26 16:38, Sumit Gupta wrote:
> Without online()/offline() callbacks, the cpufreq core fully tears
> down a policy during exit() when its last online CPU is offlined,
> and rebuilds it during init() when it 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. This avoids re-reading the CPPC capabilities on every
> offline/online, making CPU hotplug faster.
> 
> Re-enable CPPC from online(), as it may have been disabled while
> the CPU was offline.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index f6cea0c54dd9..432c6a6288a7 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -722,6 +722,31 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	return ret;
>  }
>  
> +/*
> + * With offline() defined, the cpufreq core keeps the policy alive when
> + * a CPU is hotplugged out.
> + */
> +static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
> +{
> +	return 0;
> +}
> +
> +/*
> + * Re-enable CPPC when the policy's CPU comes back online, since the platform
> + * may have disabled it while the CPU was offline.
> + */
> +static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
> +{
> +	unsigned int cpu = policy->cpu;
> +	int ret;
> +
> +	ret = cppc_set_enable(cpu, true);
> +	if (ret && ret != -EOPNOTSUPP)
> +		pr_warn("Failed to re-enable CPPC for CPU%d (%d)\n", cpu, ret);

Keeping the policy alive means cpu_data->perf_ctrls survives hotplug, but the callback does not
reapply it. A governor restart does not guarantee an immediate target callback, so the controls
may remain reset indefinitely.

I think the online path needs to restore a complete, current performance-control request before
returning. MIN/MAX should be recomputed from the policy and the last requested DESIRED value
clamped into that range.

Also, what about suspend/resume?

> +
> +	return 0;
> +}
> +
>  static void cppc_cpufreq_cpu_exit(struct cpufreq_policy *policy)
>  {
>  	struct cppc_cpudata *cpu_data = policy->driver_data;
> @@ -1034,6 +1059,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,
>  	.set_boost = cppc_cpufreq_set_boost,
>  	.attr = cppc_cpufreq_attr,
>  	.name = "cppc_cpufreq",

Re: [PATCH v2 1/3] cpufreq: CPPC: Keep the policy across CPU hotplug

[Sumit Gupta]

On 22/07/26 15:48, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> On 7/16/26 16:38, Sumit Gupta wrote:
>> Without online()/offline() callbacks, the cpufreq core fully tears
>> down a policy during exit() when its last online CPU is offlined,
>> and rebuilds it during init() when it 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. This avoids re-reading the CPPC capabilities on every
>> offline/online, making CPU hotplug faster.
>>
>> Re-enable CPPC from online(), as it may have been disabled while
>> the CPU was offline.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>>   drivers/cpufreq/cppc_cpufreq.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index f6cea0c54dd9..432c6a6288a7 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -722,6 +722,31 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>        return ret;
>>   }
>>
>> +/*
>> + * With offline() defined, the cpufreq core keeps the policy alive when
>> + * a CPU is hotplugged out.
>> + */
>> +static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
>> +{
>> +     return 0;
>> +}
>> +
>> +/*
>> + * Re-enable CPPC when the policy's CPU comes back online, since the platform
>> + * may have disabled it while the CPU was offline.
>> + */
>> +static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
>> +{
>> +     unsigned int cpu = policy->cpu;
>> +     int ret;
>> +
>> +     ret = cppc_set_enable(cpu, true);
>> +     if (ret && ret != -EOPNOTSUPP)
>> +             pr_warn("Failed to re-enable CPPC for CPU%d (%d)\n", cpu, ret);
> Keeping the policy alive means cpu_data->perf_ctrls survives hotplug, but the callback does not
> reapply it. A governor restart does not guarantee an immediate target callback, so the controls
> may remain reset indefinitely.
>
> I think the online path needs to restore a complete, current performance-control request before
> returning. MIN/MAX should be recomputed from the policy and the last requested DESIRED value
> clamped into that range.

Agreed.
Will change online() to recompute MIN/MAX from the policy and clamp the
retained DESIRED into that range, as described in my reply on patch 3/3.

>
> Also, what about suspend/resume?
>

Good point. On the platform I test on these registers are retained
across both hotplug and suspend, so I left suspend/resume out initially.
But the same mechanism extends to platforms that do reset them.
So I will add it as its own patch in v3 where suspend() saves the
OSPM set values and resume() restores them, reusing the online() path.

Thanks,
Sumit

....

[PATCH v2 2/3] ACPI: CPPC: Add u64 wrappers for the autonomous selection register

[Sumit Gupta]

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 using a common table of get/set helpers typed as
int (*)(int, u64 *) and int (*)(int, u64), which the bool autonomous
selection helpers do not fit.

Add cppc_get_auto_sel_u64()/cppc_set_auto_sel_u64() wrappers with the u64
signature so the autonomous selection register fits alongside the others.

Suggested-by: Pierre Gondois <pierre.gondois@arm.com>
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/acpi/cppc_acpi.c | 40 ++++++++++++++++++++++++++++++++++++++++
 include/acpi/cppc_acpi.h | 10 ++++++++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 9f572f481241..a7fec6c93178 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1788,6 +1788,46 @@ int cppc_set_auto_sel(int cpu, bool enable)
 }
 EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
 
+/**
+ * cppc_get_auto_sel_u64 - Read the autonomous selection register as a u64.
+ * @cpu: CPU from which to read the register.
+ * @val: Return address, set to 0 or 1.
+ *
+ * u64-typed wrapper around cppc_get_auto_sel() for callers that keep CPPC
+ * register accessors in a common table.
+ *
+ * Return: 0 for success, -ERRNO otherwise.
+ */
+int cppc_get_auto_sel_u64(int cpu, u64 *val)
+{
+	bool enable;
+	int ret;
+
+	ret = cppc_get_auto_sel(cpu, &enable);
+	if (ret)
+		return ret;
+
+	*val = enable;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cppc_get_auto_sel_u64);
+
+/**
+ * cppc_set_auto_sel_u64 - Write the autonomous selection register from a u64.
+ * @cpu: CPU to which to write the register.
+ * @val: Value to write, any non-zero value enables autonomous selection.
+ *
+ * u64-typed wrapper around cppc_set_auto_sel().
+ *
+ * Return: 0 for success, -ERRNO otherwise.
+ */
+int cppc_set_auto_sel_u64(int cpu, u64 val)
+{
+	return cppc_set_auto_sel(cpu, !!val);
+}
+EXPORT_SYMBOL_GPL(cppc_set_auto_sel_u64);
+
 /**
  * cppc_set_enable - Set to enable CPPC on the processor by writing the
  * Continuous Performance Control package EnableRegister field.
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 8693890a7275..cd07e1e92bf4 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -184,6 +184,8 @@ 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_set_auto_sel(int cpu, bool enable);
+extern int cppc_get_auto_sel_u64(int cpu, u64 *val);
+extern int cppc_set_auto_sel_u64(int cpu, u64 val);
 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);
@@ -282,6 +284,14 @@ static inline int cppc_set_auto_sel(int cpu, bool enable)
 {
 	return -EOPNOTSUPP;
 }
+static inline int cppc_get_auto_sel_u64(int cpu, u64 *val)
+{
+	return -EOPNOTSUPP;
+}
+static inline int cppc_set_auto_sel_u64(int cpu, u64 val)
+{
+	return -EOPNOTSUPP;
+}
 static inline int cppc_get_perf_limited(int cpu, u64 *perf_limited)
 {
 	return -EOPNOTSUPP;
-- 
2.34.1

Re: [PATCH v2 2/3] ACPI: CPPC: Add u64 wrappers for the autonomous selection register

[zhenglifeng (A)]

On 7/16/2026 11:38 PM, 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 using a common table of get/set helpers typed as
> int (*)(int, u64 *) and int (*)(int, u64), which the bool autonomous
> selection helpers do not fit.
> 
> Add cppc_get_auto_sel_u64()/cppc_set_auto_sel_u64() wrappers with the u64
> signature so the autonomous selection register fits alongside the others.
> 
> Suggested-by: Pierre Gondois <pierre.gondois@arm.com>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>

If this is necessary, I think it is better to just change the input parameter
of the original cppc_get/set_auto_sel() to u64.

> ---
>  drivers/acpi/cppc_acpi.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/acpi/cppc_acpi.h | 10 ++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 9f572f481241..a7fec6c93178 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1788,6 +1788,46 @@ int cppc_set_auto_sel(int cpu, bool enable)
>  }
>  EXPORT_SYMBOL_GPL(cppc_set_auto_sel);
>  
> +/**
> + * cppc_get_auto_sel_u64 - Read the autonomous selection register as a u64.
> + * @cpu: CPU from which to read the register.
> + * @val: Return address, set to 0 or 1.
> + *
> + * u64-typed wrapper around cppc_get_auto_sel() for callers that keep CPPC
> + * register accessors in a common table.
> + *
> + * Return: 0 for success, -ERRNO otherwise.
> + */
> +int cppc_get_auto_sel_u64(int cpu, u64 *val)
> +{
> +	bool enable;
> +	int ret;
> +
> +	ret = cppc_get_auto_sel(cpu, &enable);
> +	if (ret)
> +		return ret;
> +
> +	*val = enable;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(cppc_get_auto_sel_u64);
> +
> +/**
> + * cppc_set_auto_sel_u64 - Write the autonomous selection register from a u64.
> + * @cpu: CPU to which to write the register.
> + * @val: Value to write, any non-zero value enables autonomous selection.
> + *
> + * u64-typed wrapper around cppc_set_auto_sel().
> + *
> + * Return: 0 for success, -ERRNO otherwise.
> + */
> +int cppc_set_auto_sel_u64(int cpu, u64 val)
> +{
> +	return cppc_set_auto_sel(cpu, !!val);
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_auto_sel_u64);
> +
>  /**
>   * cppc_set_enable - Set to enable CPPC on the processor by writing the
>   * Continuous Performance Control package EnableRegister field.
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 8693890a7275..cd07e1e92bf4 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -184,6 +184,8 @@ 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_set_auto_sel(int cpu, bool enable);
> +extern int cppc_get_auto_sel_u64(int cpu, u64 *val);
> +extern int cppc_set_auto_sel_u64(int cpu, u64 val);
>  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);
> @@ -282,6 +284,14 @@ static inline int cppc_set_auto_sel(int cpu, bool enable)
>  {
>  	return -EOPNOTSUPP;
>  }
> +static inline int cppc_get_auto_sel_u64(int cpu, u64 *val)
> +{
> +	return -EOPNOTSUPP;
> +}
> +static inline int cppc_set_auto_sel_u64(int cpu, u64 val)
> +{
> +	return -EOPNOTSUPP;
> +}
>  static inline int cppc_get_perf_limited(int cpu, u64 *perf_limited)
>  {
>  	return -EOPNOTSUPP;

Re: [PATCH v2 2/3] ACPI: CPPC: Add u64 wrappers for the autonomous selection register

[Rafael J. Wysocki (Intel)]

On Fri, Jul 17, 2026 at 5:47 AM zhenglifeng (A) <zhenglifeng1@huawei.com> wrote:
>
> On 7/16/2026 11:38 PM, 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 using a common table of get/set helpers typed as
> > int (*)(int, u64 *) and int (*)(int, u64), which the bool autonomous
> > selection helpers do not fit.
> >
> > Add cppc_get_auto_sel_u64()/cppc_set_auto_sel_u64() wrappers with the u64
> > signature so the autonomous selection register fits alongside the others.
> >
> > Suggested-by: Pierre Gondois <pierre.gondois@arm.com>
> > Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>
> If this is necessary, I think it is better to just change the input parameter
> of the original cppc_get/set_auto_sel() to u64.

Agreed.

Sumit, why don't you just make cppc_get_auto_sel()/cppc_set_auto_sel()
take a u64 argument?

Re: [PATCH v2 2/3] ACPI: CPPC: Add u64 wrappers for the autonomous selection register

[Sumit Gupta]

On 21/07/26 21:04, Rafael J. Wysocki (Intel) wrote:
> External email: Use caution opening links or attachments
>
>
> On Fri, Jul 17, 2026 at 5:47 AM zhenglifeng (A) <zhenglifeng1@huawei.com> wrote:
>> On 7/16/2026 11:38 PM, 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 using a common table of get/set helpers typed as
>>> int (*)(int, u64 *) and int (*)(int, u64), which the bool autonomous
>>> selection helpers do not fit.
>>>
>>> Add cppc_get_auto_sel_u64()/cppc_set_auto_sel_u64() wrappers with the u64
>>> signature so the autonomous selection register fits alongside the others.
>>>
>>> Suggested-by: Pierre Gondois <pierre.gondois@arm.com>
>>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> If this is necessary, I think it is better to just change the input parameter
>> of the original cppc_get/set_auto_sel() to u64.
> Agreed.
>
> Sumit, why don't you just make cppc_get_auto_sel()/cppc_set_auto_sel()
> take a u64 argument

Yes, that makes sense. I originally added the wrappers to avoid changing
the existing interface, but updating them directly is cleaner.

In v3, I’ll drop the wrappers and change cppc_get/set_auto_sel() to take
a u64 directly, updating all callers accordingly.

Regards,
Sumit Gupta

[PATCH v2 3/3] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload

[Sumit Gupta]

Values written to OSPM-set CPPC registers (via sysfs or the autonomous
boot parameter) 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 or the boot parameter) so it can be reapplied, then
    restore the firmware value.
  - On online(), reapply the value captured at offline().

Cover the Autonomous Selection (auto_sel), Energy Performance Preference
(EPP) and Autonomous Activity Window (auto_act_window) registers.

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 | 130 +++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 432c6a6288a7..9c88512d635c 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -28,6 +28,123 @@
 
 static struct cpufreq_driver cppc_cpufreq_driver;
 
+/*
+ * OSPM-set CPPC registers tracked for save/restore. A value set via sysfs or
+ * the autonomous boot parameter is reapplied from online() across CPU
+ * hotplug, and the firmware value is restored from offline().
+ */
+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 {
+	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] = {
+		cppc_get_auto_sel_u64, cppc_set_auto_sel_u64,
+	},
+	[CPPC_SAVED_EPP] = {
+		cppc_get_epp_perf, cppc_set_epp,
+	},
+	[CPPC_SAVED_AUTO_ACT_WINDOW] = {
+		cppc_get_auto_act_window, cppc_set_auto_act_window,
+	},
+};
+
+/*
+ * Per-policy saved state for each register in cppc_saved_regs[]:
+ *   firmware_val  - value before the driver touched it, captured at init()
+ *                   and restored while the policy is offline. U64_MAX if it
+ *                   could not be read
+ *   requested_val - value in effect when the policy last went offline,
+ *                   reapplied at online(). U64_MAX if none
+ */
+struct cppc_saved_state {
+	u64 firmware_val;
+	u64 requested_val;
+};
+
+static DEFINE_PER_CPU(struct cppc_saved_state[CPPC_NR_SAVED_REGS], cppc_saved_state);
+
+/*
+ * Return this policy's saved state. Each policy keeps a single copy, stored in
+ * the per-CPU variable of the first CPU it manages. related_cpus (the policy's
+ * full set of CPUs) never changes while it exists, so this CPU (unlike
+ * policy->cpu) stays the same across CPU hotplug, and every callback reaches
+ * the same copy.
+ */
+static struct cppc_saved_state *cppc_cpufreq_policy_saved_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(). Until
+	 * then, fall back to policy->cpus, which has the same first CPU.
+	 */
+	if (cpumask_empty(policy_cpus))
+		policy_cpus = policy->cpus;
+
+	return per_cpu(cppc_saved_state, cpumask_first(policy_cpus));
+}
+
+/*
+ * Capture each register's firmware value before the driver programs anything.
+ */
+static void cppc_cpufreq_save_firmware_regs(struct cpufreq_policy *policy)
+{
+	struct cppc_saved_state *st = cppc_cpufreq_policy_saved_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[i].firmware_val = val;
+		st[i].requested_val = U64_MAX;
+	}
+}
+
+/*
+ * Save each register's current value so online() can later reapply it, then
+ * restore the firmware value to leave the platform in its pre-driver state.
+ */
+static void
+cppc_cpufreq_save_req_and_restore_firmware_regs(struct cpufreq_policy *policy)
+{
+	struct cppc_saved_state *st = cppc_cpufreq_policy_saved_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))
+			st[i].requested_val = val;
+		if (st[i].firmware_val != U64_MAX)
+			cppc_saved_regs[i].set(cpu, st[i].firmware_val);
+	}
+}
+
+/*
+ * Reapply each register's requested value that offline() saved.
+ */
+static void cppc_cpufreq_reapply_requested_regs(struct cpufreq_policy *policy)
+{
+	struct cppc_saved_state *st = cppc_cpufreq_policy_saved_state(policy);
+	unsigned int cpu = policy->cpu;
+	int i;
+
+	for (i = 0; i < CPPC_NR_SAVED_REGS; i++)
+		if (st[i].requested_val != U64_MAX)
+			cppc_saved_regs[i].set(cpu, st[i].requested_val);
+}
+
 #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
 static enum {
 	FIE_UNSET = -1,
@@ -707,6 +824,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	policy->cur = cppc_perf_to_khz(caps, caps->highest_perf);
 	cpu_data->perf_ctrls.desired_perf =  caps->highest_perf;
 
+	cppc_cpufreq_save_firmware_regs(policy);
+
 	ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
 	if (ret) {
 		pr_debug("Err setting perf value:%d on CPU:%d. ret:%d\n",
@@ -725,15 +844,24 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 /*
  * With offline() defined, the cpufreq core keeps the policy alive when
  * a CPU is hotplugged out.
+ *
+ * Save each register's current value so online() can reapply it, then restore
+ * the firmware value, leaving the platform in its pre-driver state while the
+ * policy is down (CPU hotplug or driver unload).
  */
 static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
 {
+	cppc_cpufreq_save_req_and_restore_firmware_regs(policy);
+
 	return 0;
 }
 
 /*
  * Re-enable CPPC when the policy's CPU comes back online, since the platform
  * may have disabled it while the CPU was offline.
+ *
+ * offline() reset the registers to their firmware values, so reapply the
+ * OSPM-set values it saved.
  */
 static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
 {
@@ -744,6 +872,8 @@ static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
 	if (ret && ret != -EOPNOTSUPP)
 		pr_warn("Failed to re-enable CPPC for CPU%d (%d)\n", cpu, ret);
 
+	cppc_cpufreq_reapply_requested_regs(policy);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH v2 3/3] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload

[Christian Loehle]

Hi Sumit

On 7/16/26 16:38, Sumit Gupta wrote:
> Values written to OSPM-set CPPC registers (via sysfs or the autonomous
> boot parameter) 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 or the boot parameter) so it can be reapplied, then
>     restore the firmware value.
>   - On online(), reapply the value captured at offline().
> 
> Cover the Autonomous Selection (auto_sel), Energy Performance Preference
> (EPP) and Autonomous Activity Window (auto_act_window) registers.
> 
> 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 | 130 +++++++++++++++++++++++++++++++++
>  1 file changed, 130 insertions(+)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 432c6a6288a7..9c88512d635c 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -28,6 +28,123 @@
>  
>  static struct cpufreq_driver cppc_cpufreq_driver;
>  
> +/*
> + * OSPM-set CPPC registers tracked for save/restore. A value set via sysfs or
> + * the autonomous boot parameter is reapplied from online() across CPU
> + * hotplug, and the firmware value is restored from offline().
> + */
> +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 {
> +	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] = {
> +		cppc_get_auto_sel_u64, cppc_set_auto_sel_u64,
> +	},
> +	[CPPC_SAVED_EPP] = {
> +		cppc_get_epp_perf, cppc_set_epp,
> +	},
> +	[CPPC_SAVED_AUTO_ACT_WINDOW] = {
> +		cppc_get_auto_act_window, cppc_set_auto_act_window,
> +	},
> +};
> +
> +/*
> + * Per-policy saved state for each register in cppc_saved_regs[]:
> + *   firmware_val  - value before the driver touched it, captured at init()
> + *                   and restored while the policy is offline. U64_MAX if it
> + *                   could not be read
> + *   requested_val - value in effect when the policy last went offline,
> + *                   reapplied at online(). U64_MAX if none
> + */
> +struct cppc_saved_state {
> +	u64 firmware_val;
> +	u64 requested_val;
> +};
> +
> +static DEFINE_PER_CPU(struct cppc_saved_state[CPPC_NR_SAVED_REGS], cppc_saved_state);
> +
> +/*
> + * Return this policy's saved state. Each policy keeps a single copy, stored in
> + * the per-CPU variable of the first CPU it manages. related_cpus (the policy's
> + * full set of CPUs) never changes while it exists, so this CPU (unlike
> + * policy->cpu) stays the same across CPU hotplug, and every callback reaches
> + * the same copy.
> + */
> +static struct cppc_saved_state *cppc_cpufreq_policy_saved_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(). Until
> +	 * then, fall back to policy->cpus, which has the same first CPU.
> +	 */
> +	if (cpumask_empty(policy_cpus))
> +		policy_cpus = policy->cpus;
> +
> +	return per_cpu(cppc_saved_state, cpumask_first(policy_cpus));
> +}
> +
> +/*
> + * Capture each register's firmware value before the driver programs anything.
> + */
> +static void cppc_cpufreq_save_firmware_regs(struct cpufreq_policy *policy)
> +{
> +	struct cppc_saved_state *st = cppc_cpufreq_policy_saved_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[i].firmware_val = val;
> +		st[i].requested_val = U64_MAX;
> +	}
> +}
> +
> +/*
> + * Save each register's current value so online() can later reapply it, then
> + * restore the firmware value to leave the platform in its pre-driver state.
> + */
> +static void
> +cppc_cpufreq_save_req_and_restore_firmware_regs(struct cpufreq_policy *policy)
> +{
> +	struct cppc_saved_state *st = cppc_cpufreq_policy_saved_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))
> +			st[i].requested_val = val;
> +		if (st[i].firmware_val != U64_MAX)
> +			cppc_saved_regs[i].set(cpu, st[i].firmware_val);
> +	}
> +}
> +
> +/*
> + * Reapply each register's requested value that offline() saved.
> + */
> +static void cppc_cpufreq_reapply_requested_regs(struct cpufreq_policy *policy)
> +{
> +	struct cppc_saved_state *st = cppc_cpufreq_policy_saved_state(policy);
> +	unsigned int cpu = policy->cpu;
> +	int i;
> +
> +	for (i = 0; i < CPPC_NR_SAVED_REGS; i++)
> +		if (st[i].requested_val != U64_MAX)
> +			cppc_saved_regs[i].set(cpu, st[i].requested_val);
> +}
> +
>  #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
>  static enum {
>  	FIE_UNSET = -1,
> @@ -707,6 +824,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	policy->cur = cppc_perf_to_khz(caps, caps->highest_perf);
>  	cpu_data->perf_ctrls.desired_perf =  caps->highest_perf;
>  
> +	cppc_cpufreq_save_firmware_regs(policy);
> +
>  	ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
>  	if (ret) {
>  		pr_debug("Err setting perf value:%d on CPU:%d. ret:%d\n",
> @@ -725,15 +844,24 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  /*
>   * With offline() defined, the cpufreq core keeps the policy alive when
>   * a CPU is hotplugged out.
> + *
> + * Save each register's current value so online() can reapply it, then restore
> + * the firmware value, leaving the platform in its pre-driver state while the
> + * policy is down (CPU hotplug or driver unload).
>   */
>  static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
>  {
> +	cppc_cpufreq_save_req_and_restore_firmware_regs(policy);
> +
>  	return 0;
>  }
>  
>  /*
>   * Re-enable CPPC when the policy's CPU comes back online, since the platform
>   * may have disabled it while the CPU was offline.
> + *
> + * offline() reset the registers to their firmware values, so reapply the
> + * OSPM-set values it saved.
>   */
>  static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
>  {
> @@ -744,6 +872,8 @@ static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
>  	if (ret && ret != -EOPNOTSUPP)
>  		pr_warn("Failed to re-enable CPPC for CPU%d (%d)\n", cpu, ret);
>  
> +	cppc_cpufreq_reapply_requested_regs(policy);
> +
>  	return 0;
>  }
>  

I had a look as well and I think the saved registers aren't enough for the stated hotplug problem.

CPPC also programs DESIRED_PERF, MIN_PERF and MAX_PERF. If the platform resets those controls
while a policy is offline, online() your series reapplies AUTO_SEL_ENABLE, EPP and the activity
window but leaves the main performance request unrestored.
So autonomous selection may be re-enabled while MIN_PERF and MAX_PERF still contain
reset values.

I do not think these controls should simply be added to cppc_saved_regs[]:
- MIN_PERF and MAX_PERF should be derived from the current policy, which may have changed.
- After state loss, direct controls require safe ordering: establish [0, all-ones], restore
a clamped DESIRED_PERF, then install the final bounds.
PCC controls make this more complex, they should remain grouped in a coordinated transaction.

Could the hotplug state instead retain the last requested DESIRED value, recompute the bounds
during online(), restore the complete tuple, and only then reapply AUTO_SEL_ENABLE?

Re: [PATCH v2 3/3] cpufreq: CPPC: Preserve OSPM-set registers across hotplug and unload

[Sumit Gupta]

Hi Christian,

Thanks for the review.


On 22/07/26 15:47, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Sumit
>
> On 7/16/26 16:38, Sumit Gupta wrote:
>> Values written to OSPM-set CPPC registers (via sysfs or the autonomous
>> boot parameter) 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 or the boot parameter) so it can be reapplied, then
>>      restore the firmware value.
>>    - On online(), reapply the value captured at offline().
>>
>> Cover the Autonomous Selection (auto_sel), Energy Performance Preference
>> (EPP) and Autonomous Activity Window (auto_act_window) registers.
>>
>> 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 | 130 +++++++++++++++++++++++++++++++++
>>   1 file changed, 130 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 432c6a6288a7..9c88512d635c 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -28,6 +28,123 @@
>>
>>   static struct cpufreq_driver cppc_cpufreq_driver;
>>
>> +/*
>> + * OSPM-set CPPC registers tracked for save/restore. A value set via sysfs or
>> + * the autonomous boot parameter is reapplied from online() across CPU
>> + * hotplug, and the firmware value is restored from offline().
>> + */
>> +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 {
>> +     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] = {
>> +             cppc_get_auto_sel_u64, cppc_set_auto_sel_u64,
>> +     },
>> +     [CPPC_SAVED_EPP] = {
>> +             cppc_get_epp_perf, cppc_set_epp,
>> +     },
>> +     [CPPC_SAVED_AUTO_ACT_WINDOW] = {
>> +             cppc_get_auto_act_window, cppc_set_auto_act_window,
>> +     },
>> +};
>> +
>> +/*
>> + * Per-policy saved state for each register in cppc_saved_regs[]:
>> + *   firmware_val  - value before the driver touched it, captured at init()
>> + *                   and restored while the policy is offline. U64_MAX if it
>> + *                   could not be read
>> + *   requested_val - value in effect when the policy last went offline,
>> + *                   reapplied at online(). U64_MAX if none
>> + */
>> +struct cppc_saved_state {
>> +     u64 firmware_val;
>> +     u64 requested_val;
>> +};
>> +
>> +static DEFINE_PER_CPU(struct cppc_saved_state[CPPC_NR_SAVED_REGS], cppc_saved_state);
>> +
>> +/*
>> + * Return this policy's saved state. Each policy keeps a single copy, stored in
>> + * the per-CPU variable of the first CPU it manages. related_cpus (the policy's
>> + * full set of CPUs) never changes while it exists, so this CPU (unlike
>> + * policy->cpu) stays the same across CPU hotplug, and every callback reaches
>> + * the same copy.
>> + */
>> +static struct cppc_saved_state *cppc_cpufreq_policy_saved_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(). Until
>> +      * then, fall back to policy->cpus, which has the same first CPU.
>> +      */
>> +     if (cpumask_empty(policy_cpus))
>> +             policy_cpus = policy->cpus;
>> +
>> +     return per_cpu(cppc_saved_state, cpumask_first(policy_cpus));
>> +}
>> +
>> +/*
>> + * Capture each register's firmware value before the driver programs anything.
>> + */
>> +static void cppc_cpufreq_save_firmware_regs(struct cpufreq_policy *policy)
>> +{
>> +     struct cppc_saved_state *st = cppc_cpufreq_policy_saved_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[i].firmware_val = val;
>> +             st[i].requested_val = U64_MAX;
>> +     }
>> +}
>> +
>> +/*
>> + * Save each register's current value so online() can later reapply it, then
>> + * restore the firmware value to leave the platform in its pre-driver state.
>> + */
>> +static void
>> +cppc_cpufreq_save_req_and_restore_firmware_regs(struct cpufreq_policy *policy)
>> +{
>> +     struct cppc_saved_state *st = cppc_cpufreq_policy_saved_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))
>> +                     st[i].requested_val = val;
>> +             if (st[i].firmware_val != U64_MAX)
>> +                     cppc_saved_regs[i].set(cpu, st[i].firmware_val);
>> +     }
>> +}
>> +
>> +/*
>> + * Reapply each register's requested value that offline() saved.
>> + */
>> +static void cppc_cpufreq_reapply_requested_regs(struct cpufreq_policy *policy)
>> +{
>> +     struct cppc_saved_state *st = cppc_cpufreq_policy_saved_state(policy);
>> +     unsigned int cpu = policy->cpu;
>> +     int i;
>> +
>> +     for (i = 0; i < CPPC_NR_SAVED_REGS; i++)
>> +             if (st[i].requested_val != U64_MAX)
>> +                     cppc_saved_regs[i].set(cpu, st[i].requested_val);
>> +}
>> +
>>   #ifdef CONFIG_ACPI_CPPC_CPUFREQ_FIE
>>   static enum {
>>        FIE_UNSET = -1,
>> @@ -707,6 +824,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>        policy->cur = cppc_perf_to_khz(caps, caps->highest_perf);
>>        cpu_data->perf_ctrls.desired_perf =  caps->highest_perf;
>>
>> +     cppc_cpufreq_save_firmware_regs(policy);
>> +
>>        ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls);
>>        if (ret) {
>>                pr_debug("Err setting perf value:%d on CPU:%d. ret:%d\n",
>> @@ -725,15 +844,24 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>   /*
>>    * With offline() defined, the cpufreq core keeps the policy alive when
>>    * a CPU is hotplugged out.
>> + *
>> + * Save each register's current value so online() can reapply it, then restore
>> + * the firmware value, leaving the platform in its pre-driver state while the
>> + * policy is down (CPU hotplug or driver unload).
>>    */
>>   static int cppc_cpufreq_cpu_offline(struct cpufreq_policy *policy)
>>   {
>> +     cppc_cpufreq_save_req_and_restore_firmware_regs(policy);
>> +
>>        return 0;
>>   }
>>
>>   /*
>>    * Re-enable CPPC when the policy's CPU comes back online, since the platform
>>    * may have disabled it while the CPU was offline.
>> + *
>> + * offline() reset the registers to their firmware values, so reapply the
>> + * OSPM-set values it saved.
>>    */
>>   static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
>>   {
>> @@ -744,6 +872,8 @@ static int cppc_cpufreq_cpu_online(struct cpufreq_policy *policy)
>>        if (ret && ret != -EOPNOTSUPP)
>>                pr_warn("Failed to re-enable CPPC for CPU%d (%d)\n", cpu, ret);
>>
>> +     cppc_cpufreq_reapply_requested_regs(policy);
>> +
>>        return 0;
>>   }
>>
> I had a look as well and I think the saved registers aren't enough for the stated hotplug problem.
>
> CPPC also programs DESIRED_PERF, MIN_PERF and MAX_PERF. If the platform resets those controls
> while a policy is offline, online() your series reapplies AUTO_SEL_ENABLE, EPP and the activity
> window but leaves the main performance request unrestored.
> So autonomous selection may be re-enabled while MIN_PERF and MAX_PERF still contain
> reset values.
>
> I do not think these controls should simply be added to cppc_saved_regs[]:
> - MIN_PERF and MAX_PERF should be derived from the current policy, which may have changed.
> - After state loss, direct controls require safe ordering: establish [0, all-ones], restore
> a clamped DESIRED_PERF, then install the final bounds.
> PCC controls make this more complex, they should remain grouped in a coordinated transaction.
>
> Could the hotplug state instead retain the last requested DESIRED value, recompute the bounds
> during online(), restore the complete tuple, and only then reapply AUTO_SEL_ENABLE?
>

Agreed.
Will change online() to derive MIN/MAX from the current policy,
clamp the retained cpu_data->perf_ctrls.desired_perf into that range and
reprogram the tuple with cppc_set_perf() (one coordinated transaction
for PCC) before AUTO_SEL is reapplied.
With desired_perf clamped to the final [MIN, MAX] range, the temporary
[0, all-ones] widening is not needed.

Regards,
Sumit