[PATCH v3 3/8] ACPI: CPPC: extend APIs to support auto_sel and epp

Sumit Gupta posted 8 patches 4 months, 1 week ago
There is a newer version of this series
[PATCH v3 3/8] ACPI: CPPC: extend APIs to support auto_sel and epp
Posted by Sumit Gupta 4 months, 1 week ago
- Add auto_sel read support in cppc_get_perf_caps().
- Add write of both auto_sel and energy_perf in cppc_set_epp_perf().
- Remove redundant energy_perf field from 'struct cppc_perf_caps' as
  the same is available in 'struct cppc_perf_ctrls' which is used.

Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
 drivers/acpi/cppc_acpi.c | 30 ++++++++++++++++++++++++++----
 include/acpi/cppc_acpi.h |  1 -
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index ab8dd5cdb13b..12b2516b971c 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1344,8 +1344,8 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
 	struct cpc_register_resource *highest_reg, *lowest_reg,
 		*lowest_non_linear_reg, *nominal_reg, *guaranteed_reg,
-		*low_freq_reg = NULL, *nom_freq_reg = NULL;
-	u64 high, low, guaranteed, nom, min_nonlinear, low_f = 0, nom_f = 0;
+		*low_freq_reg = NULL, *nom_freq_reg = NULL, *auto_sel_reg = NULL;
+	u64 high, low, guaranteed, nom, min_nonlinear, low_f = 0, nom_f = 0, auto_sel = 0;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
 	struct cppc_pcc_data *pcc_ss_data = NULL;
 	int ret = 0, regs_in_pcc = 0;
@@ -1362,11 +1362,12 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 	low_freq_reg = &cpc_desc->cpc_regs[LOWEST_FREQ];
 	nom_freq_reg = &cpc_desc->cpc_regs[NOMINAL_FREQ];
 	guaranteed_reg = &cpc_desc->cpc_regs[GUARANTEED_PERF];
+	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
 
 	/* Are any of the regs PCC ?*/
 	if (CPC_IN_PCC(highest_reg) || CPC_IN_PCC(lowest_reg) ||
 		CPC_IN_PCC(lowest_non_linear_reg) || CPC_IN_PCC(nominal_reg) ||
-		CPC_IN_PCC(low_freq_reg) || CPC_IN_PCC(nom_freq_reg)) {
+		CPC_IN_PCC(low_freq_reg) || CPC_IN_PCC(nom_freq_reg) || CPC_IN_PCC(auto_sel_reg)) {
 		if (pcc_ss_id < 0) {
 			pr_debug("Invalid pcc_ss_id\n");
 			return -ENODEV;
@@ -1414,6 +1415,9 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 	perf_caps->lowest_freq = low_f;
 	perf_caps->nominal_freq = nom_f;
 
+	if (CPC_SUPPORTED(auto_sel_reg))
+		cpc_read(cpunum, auto_sel_reg, &auto_sel);
+	perf_caps->auto_sel = (bool)auto_sel;
 
 out_err:
 	if (regs_in_pcc)
@@ -1555,6 +1559,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_support_in_ffh_or_sysmem;
+	bool epp_support_in_ffh_or_sysmem;
 	int ret;
 
 	if (!cpc_desc) {
@@ -1565,6 +1571,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_support_in_ffh_or_sysmem = CPC_SUPPORTED(epp_set_reg) &&
+				(CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
+	autosel_support_in_ffh_or_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,8 +1601,19 @@ 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)) {
+		   epp_support_in_ffh_or_sysmem && autosel_support_in_ffh_or_sysmem) {
+		ret = cpc_write(cpu, auto_sel_reg, enable);
+		if (ret) {
+			pr_debug("Failed to write auto_sel=%d for CPU:%d\n", enable, cpu);
+			return ret;
+		}
+
 		ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
+		if (ret) {
+			pr_debug("Failed to write energy_perf=%u for CPU:%d\n",
+				 perf_ctrls->energy_perf, cpu);
+			return ret;
+		}
 	} else {
 		ret = -ENOTSUPP;
 		pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 213bd389ec57..3babc6d6e70a 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -119,7 +119,6 @@ struct cppc_perf_caps {
 	u32 lowest_nonlinear_perf;
 	u32 lowest_freq;
 	u32 nominal_freq;
-	u32 energy_perf;
 	bool auto_sel;
 };
 
-- 
2.34.1
Re: [PATCH v3 3/8] ACPI: CPPC: extend APIs to support auto_sel and epp
Posted by Ionela Voinescu 3 months, 2 weeks ago
Hi Sumit,

On Wednesday 01 Oct 2025 at 20:30:59 (+0530), Sumit Gupta wrote:
> - Add auto_sel read support in cppc_get_perf_caps().
> - Add write of both auto_sel and energy_perf in cppc_set_epp_perf().
> - Remove redundant energy_perf field from 'struct cppc_perf_caps' as
>   the same is available in 'struct cppc_perf_ctrls' which is used.
> 
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
[..]
> @@ -1555,6 +1559,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_support_in_ffh_or_sysmem;
> +	bool epp_support_in_ffh_or_sysmem;
>  	int ret;
>  
>  	if (!cpc_desc) {
> @@ -1565,6 +1571,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_support_in_ffh_or_sysmem = CPC_SUPPORTED(epp_set_reg) &&
> +				(CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
> +	autosel_support_in_ffh_or_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,8 +1601,19 @@ 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)) {
> +		   epp_support_in_ffh_or_sysmem && autosel_support_in_ffh_or_sysmem) {

Isn't this problematic for when auto-select is an integer set to 1 or it's
not present at all? In those cases the EPP register won't be written and
-ENOTSUPP will be returned.

I suppose for the case when auto-select is not present at all in _CPC
(it's an optional attribute) it's not very clear what one should do
regarding writing the EPP register. The specification mentions that
"Writes to this register only have meaning when Autonomous Selection
is enabled.". From my perspective the OS should not block writes to the
EPP register for this case, as autonomous selection might still be
enabled but not exposed to the OS. 

Thanks,
Ionela.

> +		ret = cpc_write(cpu, auto_sel_reg, enable);
> +		if (ret) {
> +			pr_debug("Failed to write auto_sel=%d for CPU:%d\n", enable, cpu);
> +			return ret;
> +		}
> +
>  		ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
> +		if (ret) {
> +			pr_debug("Failed to write energy_perf=%u for CPU:%d\n",
> +				 perf_ctrls->energy_perf, cpu);
> +			return ret;
> +		}
>  	} else {
>  		ret = -ENOTSUPP;
>  		pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
[..]
Re: [PATCH v3 3/8] ACPI: CPPC: extend APIs to support auto_sel and epp
Posted by Sumit Gupta 3 months, 2 weeks ago
Hi Ionela,

Thank you for the comments.


On 22/10/25 14:42, Ionela Voinescu wrote:
> External email: Use caution opening links or attachments
>
>
> Hi Sumit,
>
> On Wednesday 01 Oct 2025 at 20:30:59 (+0530), Sumit Gupta wrote:
>> - Add auto_sel read support in cppc_get_perf_caps().
>> - Add write of both auto_sel and energy_perf in cppc_set_epp_perf().
>> - Remove redundant energy_perf field from 'struct cppc_perf_caps' as
>>    the same is available in 'struct cppc_perf_ctrls' which is used.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
> [..]
>> @@ -1555,6 +1559,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_support_in_ffh_or_sysmem;
>> +     bool epp_support_in_ffh_or_sysmem;
>>        int ret;
>>
>>        if (!cpc_desc) {
>> @@ -1565,6 +1571,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_support_in_ffh_or_sysmem = CPC_SUPPORTED(epp_set_reg) &&
>> +                             (CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
>> +     autosel_support_in_ffh_or_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,8 +1601,19 @@ 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)) {
>> +                epp_support_in_ffh_or_sysmem && autosel_support_in_ffh_or_sysmem) {
> Isn't this problematic for when auto-select is an integer set to 1 or it's
> not present at all? In those cases the EPP register won't be written and
> -ENOTSUPP will be returned.
>
> I suppose for the case when auto-select is not present at all in _CPC
> (it's an optional attribute) it's not very clear what one should do
> regarding writing the EPP register. The specification mentions that
> "Writes to this register only have meaning when Autonomous Selection
> is enabled.". From my perspective the OS should not block writes to the
> EPP register for this case, as autonomous selection might still be
> enabled but not exposed to the OS.
>
> Thanks,
> Ionela.

Will change in v4 to write them independently as below.

-------------
         } else if (osc_cpc_flexible_adr_space_confirmed) {
                 if (!epp_support_in_ffh_or_sysmem && 
!autosel_support_in_ffh_or_sysmem) {
                         ret = -ENOTSUPP;
                 } else {
                         if (autosel_support_in_ffh_or_sysmem) {
                                 ret = cpc_write(cpu, auto_sel_reg, enable);
                                 if (ret)
                                         return ret;
                         }

                         if (epp_support_in_ffh_or_sysmem) {
                                 ret = cpc_write(cpu, epp_set_reg, 
perf_ctrls->energy_perf);
                                 if (ret)
                                         return ret;
                         }
                 }
         } else {
                 ret = -ENOTSUPP;
         }

         if (ret == -ENOTSUPP)
                 pr_debug("_CPC in PCC and _CPC in FFH are not 
supported\n");

-------------

Thank you,
Sumit Gupta


>> +             ret = cpc_write(cpu, auto_sel_reg, enable);
>> +             if (ret) {
>> +                     pr_debug("Failed to write auto_sel=%d for CPU:%d\n", enable, cpu);
>> +                     return ret;
>> +             }
>> +
>>                ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
>> +             if (ret) {
>> +                     pr_debug("Failed to write energy_perf=%u for CPU:%d\n",
>> +                              perf_ctrls->energy_perf, cpu);
>> +                     return ret;
>> +             }
>>        } else {
>>                ret = -ENOTSUPP;
>>                pr_debug("_CPC in PCC and _CPC in FFH are not supported\n");
> [..]