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

[Christian Loehle <christian.loehle@arm.com>]

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 :)