Re: [PATCH] ACPI: CPPC: Scale EPP values to the register width

[Sumit Gupta <sumitg@nvidia.com>]

On 13/11/26 18:02, Christian Loehle wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 11/13/26 12:28, Rafael J. Wysocki (Intel) wrote:
>> On Fri, Nov 13, 2026 at 1:22 PM Christian Loehle
>> <christian.loehle@arm.com> wrote:
>>>
>>> On 11/12/26 17:19, Christian Loehle wrote:
>>>> On 11/12/26 17:11, Mario Limonciello wrote:
>>>>> On 11/12/26 08:05, Christian Loehle wrote:
>>>>>> On 11/12/26 14:01, Rafael J. Wysocki (Intel) wrote:
>>>>>>> On Thu, Nov 12, 2026 at 2:58 PM Christian Loehle
>>>>>>> <christian.loehle@arm.com> wrote:
>>>>>>>>
>>>>>>>> On 11/12/26 13:13, Christian Loehle wrote:
>>>>>>>>> The EPP sysfs interface uses 0-255, but ACPI permits 4-8 bit fields.
>>>>>>>>> Writing 128 to a 4- or 6-bit SystemMemory field currently succeeds but
>>>>>>>>> stores zero, selecting maximum performance.
>>>>>>>>>
>>>>>>>>> Scale writes to supported narrow EPP registers and normalize reads to
>>>>>>>>> 0-255. Cover both EPP setters and cppc_get_perf() so saved firmware values
>>>>>>>>> round-trip correctly.
>>>>>>>>>
>>>>>>>>> Fixes: 922607a2b462 ("cpufreq: CPPC: Add support for autonomous selection")
>>>>>>>>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>>>>>>>>> ---
>>>>>>>>>    .../ABI/testing/sysfs-devices-system-cpu      |  5 ++
>>>>>>>>>    drivers/acpi/cppc_acpi.c                      | 46 +++++++++++++++++--
>>>>>>>>>    2 files changed, 47 insertions(+), 4 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/ABI/testing/sysfs-devices-system-cpu b/Documentation/ABI/testing/sysfs-devices-system-cpu
>>>>>>>>> index d219fc27a0c9..c63b5cfd993c 100644
>>>>>>>>> --- a/Documentation/ABI/testing/sysfs-devices-system-cpu
>>>>>>>>> +++ b/Documentation/ABI/testing/sysfs-devices-system-cpu
>>>>>>>>> @@ -322,6 +322,11 @@ Description:     Energy performance preference
>>>>>>>>>                 performance increase/decrease and the result of the hardware's
>>>>>>>>>                 energy efficiency and performance optimization policies.
>>>>>>>>>
>>>>>>>>> +             For supported EPP registers narrower than eight bits, values are
>>>>>>>>> +             scaled and rounded to the nearest hardware preference.
>>>>>>>>> +             Reads use the same 0-255 scale and may differ from the requested
>>>>>>>>> +             value due to the reduced precision.
>>>>>>>>> +
>>>>>>>>>                 Writing to this file only has meaning when Autonomous Selection is
>>>>>>>>>                 enabled.
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>>>>>>>> index 11a9f9eb6b4a..10b52163eb56 100644
>>>>>>>>> --- a/drivers/acpi/cppc_acpi.c
>>>>>>>>> +++ b/drivers/acpi/cppc_acpi.c
>>>>>>>>> @@ -38,6 +38,7 @@
>>>>>>>>>    #include <linux/iopoll.h>
>>>>>>>>>    #include <linux/ktime.h>
>>>>>>>>>    #include <linux/list.h>
>>>>>>>>> +#include <linux/math64.h>
>>>>>>>>>    #include <linux/mutex.h>
>>>>>>>>>    #include <linux/rbtree.h>
>>>>>>>>>    #include <linux/rwsem.h>
>>>>>>>>> @@ -2841,6 +2842,25 @@ int cppc_get_highest_perf(int cpunum, u64 *highest_perf)
>>>>>>>>>    }
>>>>>>>>>    EXPORT_SYMBOL_GPL(cppc_get_highest_perf);
>>>>>>>>>
>>>>>>>>> +/* Keep the EPP interface on an 8-bit scale, including firmware readback. */
>>>>>>>>> +static u64 cppc_scale_epp(const struct cpc_register_resource *reg,
>>>>>>>>> +                       u64 epp, bool to_reg)
>>>>>>>>> +{
>>>>>>>>> +     unsigned int width = reg->cpc_entry.reg.bit_width;
>>>>>>>>> +     u32 max;
>>>>>>>>> +
>>>>>>>>> +     if (reg->type != ACPI_TYPE_BUFFER || width < 4 || width >= 8)
>>>>>>>>> +             return epp;
>>>>>>>>> +
>>>>>>>>> +     max = BIT(width) - 1;
>>>>>>>>> +     if (to_reg)
>>>>>>>>> +             return DIV_ROUND_CLOSEST_ULL(epp * max,
>>>>>>>>> +                                          CPPC_EPP_ENERGY_EFFICIENCY_PREF);
>>>>>>>>> +
>>>>>>>>> +     return DIV_ROUND_CLOSEST_ULL(epp * CPPC_EPP_ENERGY_EFFICIENCY_PREF,
>>>>>>>>> +                                  max);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>>    /**
>>>>>>>>>     * cppc_get_epp_perf - Get the epp register value.
>>>>>>>>>     * @cpunum: CPU from which to get epp preference value.
>>>>>>>>> @@ -2850,7 +2870,15 @@ EXPORT_SYMBOL_GPL(cppc_get_highest_perf);
>>>>>>>>>     */
>>>>>>>>>    int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
>>>>>>>>>    {
>>>>>>>>> -     return cppc_get_reg_val(cpunum, ENERGY_PERF, epp_perf);
>>>>>>>>> +     struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
>>>>>>>>> +     int ret;
>>>>>>>>> +
>>>>>>>>> +     ret = cppc_get_reg_val(cpunum, ENERGY_PERF, epp_perf);
>>>>>>>>> +     if (!ret)
>>>>>>>>> +             *epp_perf = cppc_scale_epp(&cpc_desc->cpc_regs[ENERGY_PERF],
>>>>>>>>> +                                        *epp_perf, false);
>>>>>>>>> +
>>>>>>>>> +     return ret;
>>>>>>>>>    }
>>>>>>>>>    EXPORT_SYMBOL_GPL(cppc_get_epp_perf);
>>>>>>>>>
>>>>>>>>> @@ -3150,6 +3178,7 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>>>>>>>>         bool auto_sel_non_pcc;
>>>>>>>>>         bool epp_pcc;
>>>>>>>>>         bool epp_non_pcc;
>>>>>>>>> +     u64 epp;
>>>>>>>>>         int ret;
>>>>>>>>>
>>>>>>>>>         if (!cpc_desc) {
>>>>>>>>> @@ -3159,6 +3188,7 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>>>>>>>>
>>>>>>>>>         auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
>>>>>>>>>         epp_set_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
>>>>>>>>> +     epp = cppc_scale_epp(epp_set_reg, perf_ctrls->energy_perf, true);
>>>>>>>>>         if (!enable && auto_sel_reg->type == ACPI_TYPE_INTEGER &&
>>>>>>>>>             auto_sel_reg->cpc_entry.int_value == 1)
>>>>>>>>>                 return -EOPNOTSUPP;
>>>>>>>>> @@ -3177,7 +3207,7 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>>>>>>>>                         return ret;
>>>>>>>>>         }
>>>>>>>>>         if (epp_non_pcc) {
>>>>>>>>> -             ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
>>>>>>>>> +             ret = cpc_write(cpu, epp_set_reg, epp);
>>>>>>>>>                 if (ret)
>>>>>>>>>                         return ret;
>>>>>>>>>         }
>>>>>>>>> @@ -3205,7 +3235,7 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>>>>>>>>                 }
>>>>>>>>>
>>>>>>>>>                 if (epp_pcc) {
>>>>>>>>> -                     ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
>>>>>>>>> +                     ret = cpc_write(cpu, epp_set_reg, epp);
>>>>>>>>>                         if (ret)
>>>>>>>>>                                 goto out_unlock;
>>>>>>>>>                 }
>>>>>>>>> @@ -3235,9 +3265,17 @@ EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
>>>>>>>>>     */
>>>>>>>>>    int cppc_set_epp(int cpu, u64 epp_val)
>>>>>>>>>    {
>>>>>>>>> +     struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>>>>>>>>> +
>>>>>>>>>         if (epp_val > CPPC_EPP_ENERGY_EFFICIENCY_PREF)
>>>>>>>>>                 return -EINVAL;
>>>>>>>>>
>>>>>>>>> +     if (!cpc_desc)
>>>>>>>>> +             return -ENODEV;
>>>>>>>>> +
>>>>>>>>> +     epp_val = cppc_scale_epp(&cpc_desc->cpc_regs[ENERGY_PERF],
>>>>>>>>> +                              epp_val, true);
>>>>>>>>> +
>>>>>>>>>         return cppc_set_reg_val(cpu, ENERGY_PERF, epp_val);
>>>>>>>>>    }
>>>>>>>>>    EXPORT_SYMBOL_GPL(cppc_set_epp);
>>>>>>>>> @@ -3496,7 +3534,7 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>>>>>>>>>                 if (ret)
>>>>>>>>>                         goto out_err;
>>>>>>>>>         }
>>>>>>>>> -     perf_ctrls->energy_perf = energy_perf;
>>>>>>>>> +     perf_ctrls->energy_perf = cppc_scale_epp(energy_perf_reg, energy_perf, false);
>>>>>>>>>
>>>>>>>>>         if (CPC_SUPPORTED(auto_sel_reg)) {
>>>>>>>>>                 ret = cpc_read(cpu, auto_sel_reg, &auto_sel);
>>>>>>>>
>>>>>>>> +CC Sumit's actual email, sorry about that.
>>>>>>>
>>>>>>> So I'm assuming that this one is for Sumit to pick up and send again
>>>>>>> along with a new version of
>>>>>>>
>>>>>>> https://lore.kernel.org/linux-acpi/20261112080442.4007723-1-sumitg@nvidia.com/
>>>>>>>
>>>>>>> Or please let me know if that is not the case.
>>>>>>
>>>>>> Yup, while it fixes an issue by itself technically, I don't think there
>>>>>> are any affected platforms.
>>>>>> I'm hoping that Sumit this (and the limit-write-at-init) patch as a series
>>>>>> for all our convenience (and hopefully to get Sashiko to review them too).
>>>>>> (I guess I should've mentioned that in the notes.)
>>>>>>
>>>>>> And sorry about ACPI/CPPC having such high traffic lately, I'm expecting once
>>>>>> we've got the issues ironed out and the new features supported it'll be a
>>>>>> lot more boring :)
>>>>>
>>>>> So the basic idea with this patch that the userspace interface is always 8 bit so higher level software will not need to know the lower level implementation detail at the expense of some granularity.
>>>>>
>>>>> Makes sense to me, but I'd like to know how realistic this hardware is going to be.  If it's purely hypothetical hardware; how about proposing to ASWG that this field must always be 8 bit?
>>>>
>>>> Good point, IMO it's hypothetical hardware that hopefully doesn't and won't exist.
>>>> I'll try to raise this through that channel as well.
>>>
>>>
>>> Quick update on this, after proposing it, Souvik came back and made me aware that
>>> we've already proposed this:
>>> https://github.com/tianocore/edk2/issues/10838
>>>
>>> I guess forget about $SUBJECT after all and I'll probe-fail <8bit and get the amd-pstate
>>> handling correct.
>>
>> So should I wait for an update of
>>
>> https://lore.kernel.org/linux-acpi/20261112080442.4007723-1-sumitg@nvidia.com/
>>
>> or is it applicable as-is?  In which case, care to give it a tag?
> 
> I guess the issue is arguably preexisting and the fix doesn't touch the same code,
> so feel free to apply,


Yes, [1] can be applied as is. The issue Sashiko reported on it exists
without [1] too, and [2] fixes it.

[2] replaces the firmware MIN/MAX at init(). v5 of [1] kept them in
autonomous mode. I'm not aware of any platform that needs the OS to
keep them, but if one does, I will look at that separately.

I can also resend [1] and [2] as a series if you prefer.

[1] 
https://lore.kernel.org/linux-acpi/20261112080442.4007723-1-sumitg@nvidia.com/
[2] 
https://lore.kernel.org/all/20261112113204.778904-1-christian.loehle@arm.com/

Thanks,
Sumit