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

[Sumit Gupta]

On 27/07/26 18:45, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> 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

Right, the ordering depends on the auto_sel value being restored, so a
single fixed table order is insufficient.
When restoring 1, auto_sel is written before EPP/window.
When restoring 0, EPP/window are written before disabling auto_sel,
allowing them to take effect if autonomous selection is still enabled.
online() already enables CPPC and restores valid desired/min/max values
before this sequence.

If autonomous selection is already disabled, the EPP/window writes may
have no effect. Temporarily enabling it to force them through would
introduce an observable mode change and would not work where auto_sel
is not writable.

Thanks,
Sumit


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