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) {
+ 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 {
--
2.34.1
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 {
On 2025/12/23 20: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) {
> + if (!epp_ffh_sysmem && !autosel_ffh_sysmem) {
> + ret = -EOPNOTSUPP;
> + } else {
I think this can do the same thing:
} else if (osc_cpc_flexible_adr_space_confirmed &&
(epp_ffh_sysmem || autosel_ffh_sysmem)) {
and reduce the levels of indentation.
> + 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);
The code below is unrelated to the code above and should ideally be
separated into a separate patch I think.
> @@ -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 {
On 25/12/25 09:26, zhenglifeng (A) wrote:
> External email: Use caution opening links or attachments
>
>
> On 2025/12/23 20: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) {
>> + if (!epp_ffh_sysmem && !autosel_ffh_sysmem) {
>> + ret = -EOPNOTSUPP;
>> + } else {
> I think this can do the same thing:
>
> } else if (osc_cpc_flexible_adr_space_confirmed &&
> (epp_ffh_sysmem || autosel_ffh_sysmem)) {
>
> and reduce the levels of indentation.
Thanks for suggesting. Will change in v6.
>> + 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);
> The code below is unrelated to the code above and should ideally be
> separated into a separate patch I think.
Will move it to a separate patch in v6.
Thank you,
Sumit Gupta
>
>> @@ -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 {
© 2016 - 2026 Red Hat, Inc.