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

[Christian Loehle]

On 7/24/26 22:59, 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() after the
>     performance request is re-established.
> 
> Keep Autonomous Selection (auto_sel) last in the table so that, on
> online(), its saved value is reapplied after the other registers that
> shape its behaviour.
> 
> Cover the Autonomous Selection (auto_sel), Energy Performance Preference
> (EPP) and Autonomous Activity Window (auto_act_window) registers.
> 
> Suggested-by: Pierre Gondois <pierre.gondois@xxxxxxx>
> Link: https://lore.kernel.org/all/86780f97-29ee-4a72-b311-38c89434b707@xxxxxxx/
> Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 157 +++++++++++++++++++++++++++++++++
>  1 file changed, 157 insertions(+)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 34cdba00e61a..8a13ec49eb9d 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -28,6 +28,150 @@
>  
>  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().
> + *
> + * Autonomous Selection (auto_sel) is kept last so its saved value is
> + * reapplied after the other registers that shape its behaviour.

While that would make sense, unfortunately:
"8.4.6.1.6 Autonomous Activity Window Register
Writes to this register only have meaning when Autonomous Selection is enabled."
I think restoring an inappropriate EPP/AUTO_ACT_WINDOW temporarily is okay though.
AFAICS it has to be:
1. enable CPPC
2. restore a valid DESIRED/MIN/MAX
3. restore AUTO_SEL_ENABLE
4. if autonomous mode is enabled, restore EPP and AUTO_ACT_WINDOW

> + */
> +enum cppc_saved_reg_id {
> +	CPPC_SAVED_EPP,
> +	CPPC_SAVED_AUTO_ACT_WINDOW,
> +	CPPC_SAVED_AUTO_SEL,
> +	CPPC_NR_SAVED_REGS,
> +};
> [snip]