Re: [PATCH] cpufreq: CPPC: Set CPPC Enable register in cpu_init

[Sumit Gupta <sumitg@nvidia.com>]

On 05/11/26 18:52, Christian Loehle wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 11/5/26 12:32, Sumit Gupta wrote:
>> As per ACPI 6.x s8.4.6.1.4 (CPPC Enable register):
>>    "If supported by the platform, OSPM writes a one to this register
>>     to enable CPPC on this processor. If not implemented, OSPM assumes
>>     the platform always has CPPC enabled."
>>
>> The driver never writes this register. Call cppc_set_enable() at the
>> start of cppc_cpufreq_cpu_init(). Errors are logged but not fatal, as
>> the register is optional.
>>
>> Fixes: 5477fb3bd1e8 ("ACPI / CPPC: Add a CPUFreq driver for use with CPPC")
>> Cc: stable@vger.kernel.org # v6.16+
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> 
> So apparently Nvidia GB10 has and needs this
> (I guess it doesn't hurt putting this into the
> commit message)
> amd-pstate sets it so all good.
> 

Will add it in v2.

>> ---
>> Sending this separately from the autonomous mode boot parameter series
>> where it was patch 1/2, as it is an independent bug fix.
>> v5 of the series:
>>   https://lore.kernel.org/lkml/20260623080652.3353386-1-sumitg@nvidia.com/
>>
>>   drivers/cpufreq/cppc_cpufreq.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 957fa7775260..b8f4399b6e0b 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -679,6 +679,14 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>        caps = &cpu_data->perf_caps;
>>        policy->driver_data = cpu_data;
>>
>> +     /*
>> +      * Enable CPPC. The Enable register is optional - some platforms
>> +      * may not support it.
>> +      */
>> +     ret = cppc_set_enable(cpu, true);
>> +     if (ret && ret != -EOPNOTSUPP)
>> +             pr_warn("Failed to enable CPPC for CPU%u (%d)\n", cpu, ret);
>> +
> 
> Any reason to not fail here?
> 

No strong reason. If this write fails, cppc_set_perf() below would
likely fail too, and no platform reports this failure today.
Will fail init in v2.

>>        /*
>>         * Set min to lowest nonlinear perf to avoid any efficiency penalty (see
>>         * Section 8.4.7.1.1.5 of ACPI 6.1 spec)
> 
> 
> ... also I guess we need to set_enable(false) on
> other failures?
> 

Yes. Will move the call to just before cppc_set_perf(), so that is the
only failure after it, and disable it again there.

> It should be okay to do this only for policy->cpu AFAICS
> but I guess we do need to also reenable on resume.

The preserve series [1] handles resume, as resume() there calls
online() which re-enables CPPC. So I would leave it out of this patch.

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

Thanks,
Sumit