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

[Ionela Voinescu <ionela.voinescu@arm.com>]

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");
[..]