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

[Sumit Gupta]

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@xxxxxxxxxxxxxxx # v6.16+
> > Signed-off-by: Sumit Gupta <sumitg@xxxxxxxxxx>
>
> 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@xxxxxxxxxx/
> >
> >   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@xxxxxxxxxx/

Thanks,
Sumit