Re: [PATCH v6 5/9] ACPI: CPPC: Extend cppc_set_epp_perf() for FFH/SystemMemory

[Sumit Gupta]

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@xxxxxxxxxx>
> > ---
> >   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;