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

[Pierre Gondois <pierre.gondois@arm.com>]

Hello Rafael, Sumit,

On 6/24/26 15:20, Rafael J. Wysocki wrote:
> On Wed, Jun 24, 2026 at 2:56 PM Sumit Gupta<sumitg@nvidia.com> wrote:
>> Hi Rafael,
>>
>>
>> On 23/06/26 16:30, Rafael J. Wysocki wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On Tue, Jun 23, 2026 at 11:54 AM Sumit Gupta<sumitg@nvidia.com> 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 when it
>>>>       is offlined.
>>>>     - 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:
>>>>
>>>>     - Capture each register's firmware value when a CPU is first seen and
>>>>       restore it on driver unload.
>>>>     - Record the last value the driver set and reapply it from ->init()
>>>>       when the policy is reactivated after CPU hotplug.
>>> I'm not sure if this is a good idea TBH.
>>>
>>> The overall system state when the CPU goes online may be completely
>>> different from the system state when the CPU was online last time, so
>>> there is no reason to restore its settings from before offline, at
>>> least in principle.
>> These are values userspace deliberately set, more like a frequency QoS
>> request that persists until userspace changes it than transient state.
>> It's platform dependent though, as the platform I test on preserves them
>> across hotplug, but where the platform resets the register on offline
>> the value is silently lost. Should the kernel re-apply it on online,
>> or is that better left to userspace? If so, I will drop the hotplug reapply
>> and keep only the restore on driver unload.
>>
>> intel_pstate and amd-pstate already re-sync these on CPU online, as they
>> keep the policy across hotplug via ->online()/->offline(). So for them
>> it's re-syncing the hardware, not restoring state from before offline.
>>
>> cppc_cpufreq has no ->online()/->offline() today, so it fully tears the
>> policy down and rebuilds it, which is why this reads as "restoring
>> settings from before offline".
>> Would adding ->online()/->offline() be acceptable, with policy preserved
>> and ->online() just re-syncing the registers the platform reset?
> That would be a better approach IMV.
Just an example to be sure I understand correctly.

1. on a platform, auto_sel=0 by default
2. cppc_cpufreq driver is loaded + we set auto_sel=1
3. all the CPUs of a policy are unplugged
4. cppc_cpufreq driver is unloaded

To be sure we let the platform in the default state,
we should set auto_sel=0 in step A.3, i.e. in the
.offline() callback.
Doing it at A.4 (i.e. in the .exit()) would not be possible:
the CPU's registers might not be available anymore once offline.

IoW, we should only need:
.init()
\-cppc_cpufreq_save_firmware_regs()
firmware_captured=0, save the firmware regs.

.online()
\-cppc_cpufreq_save_firmware_regs()
firmware_captured=1, restore the user (or previous)
values that were saved in the previous .offline() call
(as .online() is not called for new policies).

.offline()
\-cppc_cpufreq_restore_firmware_regs()
Save the user/current regs values
+ Restore the firmware registers.

And it should be ok to remove:
- cppc_cpufreq_reapply_requested_regs()
- the firmware_captured field.
If during .init(), we init requested_val to the firmware
values aswell, firmware_captured should be useless