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

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 <[email protected]>
>> Link: https://lore.kernel.org/all/[email protected]/
>> Signed-off-by: Sumit Gupta <[email protected]>
>> ---
>>   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]