Re: [PATCH v5 04/11] ACPI: CPPC: Extend cppc_set_epp_perf() to support auto_sel and epp

Pierre Gondois
2026-01-16T16:00:47Z

On 12/23/25 13:13, Sumit Gupta wrote:
> Extend cppc_set_epp_perf() to write both auto_sel and energy_perf
> registers when they are in FFH or SystemMemory address space.
>
> This keeps the behavior consistent with PCC case where both registers
> are already updated together, but was missing for FFH/SystemMemory.
>
> Also update EPP constants for better clarity:
> - Add CPPC_EPP_PERFORMANCE_PREF (0x00) for performance preference
> - Add CPPC_EPP_ENERGY_EFFICIENCY_PREF (0xFF) for energy efficiency
>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
> drivers/acpi/cppc_acpi.c | 35 +++++++++++++++++++++++++++++------
> include/acpi/cppc_acpi.h | 3 ++-
> 2 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index a4e89fe6aab5..403ee988a8c6 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1556,6 +1556,8 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> struct cpc_register_resource *auto_sel_reg;
> struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
> struct cppc_pcc_data *pcc_ss_data = NULL;
> + bool autosel_ffh_sysmem;
> + bool epp_ffh_sysmem;
> int ret;
>
> if (!cpc_desc) {
> @@ -1566,6 +1568,11 @@ 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_ffh_sysmem = CPC_SUPPORTED(epp_set_reg) &&
> + (CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
> + autosel_ffh_sysmem = CPC_SUPPORTED(auto_sel_reg) &&
> + (CPC_IN_FFH(auto_sel_reg) || CPC_IN_SYSTEM_MEMORY(auto_sel_reg));
> +
> if (CPC_IN_PCC(epp_set_reg) || CPC_IN_PCC(auto_sel_reg)) {
> if (pcc_ss_id < 0) {
> pr_debug("Invalid pcc_ss_id for CPU:%d\n", cpu);
> @@ -1590,14 +1597,30 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> /* after writing CPC, transfer the ownership of PCC to platform */
> ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
> up_write(&pcc_ss_data->pcc_lock);
> - } else if (osc_cpc_flexible_adr_space_confirmed &&
> - CPC_SUPPORTED(epp_set_reg) && CPC_IN_FFH(epp_set_reg)) {
> - ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
> + } else if (osc_cpc_flexible_adr_space_confirmed) {

I had a comment in last version about the
osc_cpc_flexible_adr_space_confirmed
handling. I don't know if it's still relevant:

https://lore.kernel.org/all/8fb77549-ce33-4c89-959b-57113eb716b6@arm.com/

> + if (!epp_ffh_sysmem && !autosel_ffh_sysmem) {
> + ret = -EOPNOTSUPP;
> + } else {
> + if (autosel_ffh_sysmem) {
> + ret = cpc_write(cpu, auto_sel_reg, enable);
> + if (ret)
> + return ret;
> + }
> +
> + if (epp_ffh_sysmem) {
> + ret = cpc_write(cpu, epp_set_reg,
> + perf_ctrls->energy_perf);
> + if (ret)
> + return ret;
> + }
> + }
> } else {
> - ret = -ENOTSUPP;
> - pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
> + ret = -EOPNOTSUPP;
> }
>
> + if (ret == -EOPNOTSUPP)
> + pr_debug("CPU%d: _CPC not in PCC/FFH/SystemMemory\n", cpu);
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
> @@ -1609,7 +1632,7 @@ EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
> */
> int cppc_set_epp(int cpu, u64 epp_val)
> {
> - if (epp_val > CPPC_ENERGY_PERF_MAX)
> + if (epp_val > CPPC_EPP_ENERGY_EFFICIENCY_PREF)
> return -EINVAL;
>
> return cppc_set_reg_val(cpu, ENERGY_PERF, epp_val);
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 12a1dc31bf2a..2860a0252313 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -39,7 +39,8 @@
> /* CPPC_AUTO_ACT_WINDOW_MAX_SIG is 127, so 128 and 129 will decay to 127 when writing */
> #define CPPC_AUTO_ACT_WINDOW_SIG_CARRY_THRESH 129
>
> -#define CPPC_ENERGY_PERF_MAX (0xFF)
> +#define CPPC_EPP_PERFORMANCE_PREF 0x00
> +#define CPPC_EPP_ENERGY_EFFICIENCY_PREF 0xFF
>
> /* Each register has the folowing format. */
> struct cpc_reg {