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.
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
drivers/acpi/cppc_acpi.c | 24 +++++++++++++++++++++---
1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index de35aeb07833..45c6bd6ec24b 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1562,6 +1562,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) {
@@ -1572,6 +1574,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);
@@ -1597,11 +1604,22 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
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);
+ (epp_ffh_sysmem || autosel_ffh_sysmem)) {
+ 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");
+ pr_debug("_CPC in PCC/FFH/SystemMemory are not supported\n");
}
return ret;
--
2.34.1On 2026/1/20 22:56, 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.
>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
> drivers/acpi/cppc_acpi.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index de35aeb07833..45c6bd6ec24b 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1562,6 +1562,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) {
> @@ -1572,6 +1574,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);
> @@ -1597,11 +1604,22 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> 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);
> + (epp_ffh_sysmem || autosel_ffh_sysmem)) {
> + 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;
> + }
Don't know if such a scenario exists, but if one of them is in PCC and the
other is in FFH or system memory, only the one in PCC will be updated
based on your modifications.
> } else {
> ret = -ENOTSUPP;
> - pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
> + pr_debug("_CPC in PCC/FFH/SystemMemory are not supported\n");
> }
>
> return ret;
On 22/01/26 14:48, zhenglifeng (A) wrote:
> External email: Use caution opening links or attachments
>
>
> On 2026/1/20 22:56, 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.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>> drivers/acpi/cppc_acpi.c | 24 +++++++++++++++++++++---
>> 1 file changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index de35aeb07833..45c6bd6ec24b 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -1562,6 +1562,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) {
>> @@ -1572,6 +1574,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);
>> @@ -1597,11 +1604,22 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>> 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);
>> + (epp_ffh_sysmem || autosel_ffh_sysmem)) {
>> + 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;
>> + }
> Don't know if such a scenario exists, but if one of them is in PCC and the
> other is in FFH or system memory, only the one in PCC will be updated
> based on your modifications.
The current code handles mixed cases correctly.
When either register is in PCC, the first if block executes and calls
cpc_write() for both registers. The cpc_write() internally handles
each register's type (PCC, FFH, or SystemMemory)
Thank you,
Sumit Gupta
>> } else {
>> ret = -ENOTSUPP;
>> - pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
>> + pr_debug("_CPC in PCC/FFH/SystemMemory are not supported\n");
>> }
>>
>> return ret;
On 2026/1/25 4:08, Sumit Gupta wrote:
>
> On 22/01/26 14:48, zhenglifeng (A) wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2026/1/20 22:56, 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.
>>>
>>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>>> ---
>>> drivers/acpi/cppc_acpi.c | 24 +++++++++++++++++++++---
>>> 1 file changed, 21 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>> index de35aeb07833..45c6bd6ec24b 100644
>>> --- a/drivers/acpi/cppc_acpi.c
>>> +++ b/drivers/acpi/cppc_acpi.c
>>> @@ -1562,6 +1562,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) {
>>> @@ -1572,6 +1574,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);
>>> @@ -1597,11 +1604,22 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>> 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);
>>> + (epp_ffh_sysmem || autosel_ffh_sysmem)) {
>>> + 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;
>>> + }
>> Don't know if such a scenario exists, but if one of them is in PCC and the
>> other is in FFH or system memory, only the one in PCC will be updated
>> based on your modifications.
> The current code handles mixed cases correctly.
> When either register is in PCC, the first if block executes and calls
> cpc_write() for both registers. The cpc_write() internally handles
> each register's type (PCC, FFH, or SystemMemory)
Yes, I was wrong.
According to the first if block, cpc_wite() is OK to be called for a
register not in PCC. So it looks like this 'else if' is unnecessary. Only
CPC_SUPPORTED is needed to be checked before calling cpc_write(), isn't it?
>
>
> Thank you,
> Sumit Gupta
>
>
>
>>> } else {
>>> ret = -ENOTSUPP;
>>> - pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
>>> + pr_debug("_CPC in PCC/FFH/SystemMemory are not supported\n");
>>> }
>>>
>>> return ret;
>>> On 2026/1/20 22:56, 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.
>>>>
>>>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>>>> ---
>>>> drivers/acpi/cppc_acpi.c | 24 +++++++++++++++++++++---
>>>> 1 file changed, 21 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>>> index de35aeb07833..45c6bd6ec24b 100644
>>>> --- a/drivers/acpi/cppc_acpi.c
>>>> +++ b/drivers/acpi/cppc_acpi.c
>>>> @@ -1562,6 +1562,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) {
>>>> @@ -1572,6 +1574,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);
>>>> @@ -1597,11 +1604,22 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>>>> 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);
>>>> + (epp_ffh_sysmem || autosel_ffh_sysmem)) {
>>>> + 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;
>>>> + }
>>> Don't know if such a scenario exists, but if one of them is in PCC and the
>>> other is in FFH or system memory, only the one in PCC will be updated
>>> based on your modifications.
>> The current code handles mixed cases correctly.
>> When either register is in PCC, the first if block executes and calls
>> cpc_write() for both registers. The cpc_write() internally handles
>> each register's type (PCC, FFH, or SystemMemory)
> Yes, I was wrong.
>
> According to the first if block, cpc_wite() is OK to be called for a
> register not in PCC. So it looks like this 'else if' is unnecessary. Only
> CPC_SUPPORTED is needed to be checked before calling cpc_write(), isn't it?
Yes, Once 'osc_cpc_flexible_adr_space_confirmed' is removed,
cppc_set_epp_perf() can be simplified to just call cpc_write() for
supported registers and only do PCC handling when needed.
As Pierre suggested [1], I will send a separate patch set for this
cleanup after the current patch set.
[1]
https://lore.kernel.org/all/c3fd7249-3cba-43e9-85c6-eadd711c0527@nvidia.com/
Thank you,
Sumit Gupta
....
© 2016 - 2026 Red Hat, Inc.