[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9fa21599-004a-4af8-acc2-190fd0404e35@nvidia.com>
Date: Tue, 27 Jan 2026 16:38:14 +0530
From: Sumit Gupta <sumitg@...dia.com>
To: Pierre Gondois <pierre.gondois@....com>,
"zhenglifeng (A)" <zhenglifeng1@...wei.com>
Cc: rafael@...nel.org, viresh.kumar@...aro.org, ionela.voinescu@....com,
lenb@...nel.org, robert.moore@...el.com, corbet@....net,
rdunlap@...radead.org, ray.huang@....com, gautham.shenoy@....com,
mario.limonciello@....com, perry.yuan@....com, zhanjie9@...ilicon.com,
linux-pm@...r.kernel.org, linux-acpi@...r.kernel.org,
linux-doc@...r.kernel.org, acpica-devel@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org,
treding@...dia.com, jonathanh@...dia.com, vsethi@...dia.com,
ksitaraman@...dia.com, sanjayc@...dia.com, nhartman@...dia.com,
bbasu@...dia.com, sumitg@...dia.com
Subject: Re: [PATCH v6 4/9] ACPI: CPPC: Add cppc_get_perf() API to read
performance controls
>>>> On 2026/1/20 22:56, Sumit Gupta wrote:
>>>>> Add cppc_get_perf() function to read values of performance control
>>>>> registers including desired_perf, min_perf, max_perf, energy_perf,
>>>>> and auto_sel.
>>>>>
>>>>> This provides a read interface to complement the existing
>>>>> cppc_set_perf() write interface for performance control registers.
>>>>>
>>>>> Note that auto_sel is read by cppc_get_perf() but not written by
>>>>> cppc_set_perf() to avoid unintended mode changes during performance
>>>>> updates. It can be updated with existing dedicated
>>>>> cppc_set_auto_sel()
>>>>> API.
>>>>>
>>>>> Use cppc_get_perf() in cppc_cpufreq_get_cpu_data() to initialize
>>>>> perf_ctrls with current hardware register values during cpufreq
>>>>> policy initialization.
>>>>>
>>>>> Signed-off-by: Sumit Gupta <sumitg@...dia.com>
>>>>> ---
>>>>> drivers/acpi/cppc_acpi.c | 80
>>>>> ++++++++++++++++++++++++++++++++++
>>>>> drivers/cpufreq/cppc_cpufreq.c | 6 +++
>>>>> include/acpi/cppc_acpi.h | 5 +++
>>>>> 3 files changed, 91 insertions(+)
>>>>>
>>>>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>>>>> index a09bdabaa804..de35aeb07833 100644
>>>>> --- a/drivers/acpi/cppc_acpi.c
>>>>> +++ b/drivers/acpi/cppc_acpi.c
>>>>> @@ -1739,6 +1739,86 @@ int cppc_set_enable(int cpu, bool enable)
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(cppc_set_enable);
>>>>>
>>>>> +/**
>>>>> + * cppc_get_perf - Get a CPU's performance controls.
>>>>> + * @cpu: CPU for which to get performance controls.
>>>>> + * @perf_ctrls: ptr to cppc_perf_ctrls. See cppc_acpi.h
>>>>> + *
>>>>> + * Return: 0 for success with perf_ctrls, -ERRNO otherwise.
>>>>> + */
>>>>> +int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
>>>>> +{
>>>>> + struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
>>>>> + struct cpc_register_resource *desired_perf_reg,
>>>>> + *min_perf_reg, *max_perf_reg,
>>>>> + *energy_perf_reg, *auto_sel_reg;
>>>>> + u64 desired_perf = 0, min = 0, max = 0, energy_perf = 0,
>>>>> auto_sel = 0;
>>>>> + int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
>>>>> + struct cppc_pcc_data *pcc_ss_data = NULL;
>>>>> + int ret = 0, regs_in_pcc = 0;
>>>>> +
>>>>> + if (!cpc_desc) {
>>>>> + pr_debug("No CPC descriptor for CPU:%d\n", cpu);
>>>>> + return -ENODEV;
>>>>> + }
>>>>> +
>>>>> + if (!perf_ctrls) {
>>>>> + pr_debug("Invalid perf_ctrls pointer\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + desired_perf_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
>>>>> + min_perf_reg = &cpc_desc->cpc_regs[MIN_PERF];
>>>>> + max_perf_reg = &cpc_desc->cpc_regs[MAX_PERF];
>>>>> + energy_perf_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
>>>>> + auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
>>>>> +
>>>>> + /* Are any of the regs PCC ?*/
>>>>> + if (CPC_IN_PCC(desired_perf_reg) || CPC_IN_PCC(min_perf_reg) ||
>>>>> + CPC_IN_PCC(max_perf_reg) || CPC_IN_PCC(energy_perf_reg) ||
>>>>> + CPC_IN_PCC(auto_sel_reg)) {
>>>>> + if (pcc_ss_id < 0) {
>>>>> + pr_debug("Invalid pcc_ss_id for CPU:%d\n",
>>>>> cpu);
>>>>> + return -ENODEV;
>>>>> + }
>>>>> + pcc_ss_data = pcc_data[pcc_ss_id];
>>>>> + regs_in_pcc = 1;
>>>>> + down_write(&pcc_ss_data->pcc_lock);
>>>>> + /* Ring doorbell once to update PCC subspace */
>>>>> + if (send_pcc_cmd(pcc_ss_id, CMD_READ) < 0) {
>>>>> + ret = -EIO;
>>>>> + goto out_err;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + /* Read optional elements if present */
>>>>> + if (CPC_SUPPORTED(max_perf_reg))
>>>>> + cpc_read(cpu, max_perf_reg, &max);
>>>>> + perf_ctrls->max_perf = max;
>>>>> +
>>>>> + if (CPC_SUPPORTED(min_perf_reg))
>>>>> + cpc_read(cpu, min_perf_reg, &min);
>>>>> + perf_ctrls->min_perf = min;
>>>>> +
>>>>> + if (CPC_SUPPORTED(desired_perf_reg))
>>>>> + cpc_read(cpu, desired_perf_reg, &desired_perf);
>>>>> + perf_ctrls->desired_perf = desired_perf;
>>>> desired_perf_reg is not an optional one, so it has to be supported.
>>
>> Please ignore my previous reply on this. Had some problem with my email
>> client and both mails got mixed.
>>
>> The register is optional when Autonomous mode is enabled.
>> Seems the comment here is already resolved with Pierre's reply.
>> We discussed this during v4 [1] also.
>> [1]
>> https://lore.kernel.org/lkml/ccd45c1b-2f69-4725-918f-18063f00a864@nvidia.com/
>>
>>
>> Thank you,
>> Sumit Gupta
>>
>>
> As suggested at:
>
> https://lore.kernel.org/all/5afea521-7d80-4e72-8809-77af60b0d957@arm.com/
>
> Maybe it would be useful to add a patch checking this:
>
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>
> index e3796b520e473..7db74e19425e6 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -854,6 +854,12 @@ int acpi_cppc_processor_probe(struct acpi_processor
> *pr)
> }
> per_cpu(cpu_pcc_subspace_idx, pr->id) = pcc_subspace_id;
>
> + if (!CPC_SUPPORTED(&cpc_ptr->cpc_regs[DESIRED_PERF]) &&
> + (!osc_sb_cppc2_support_acked ||
> + !CPC_SUPPORTED(&cpc_ptr->cpc_regs[AUTO_SEL_ENABLE])))
> + pr_warn("Desired perf. register is mandatory if CPPCV2
> is not supported "
> + "or autonomous selection is disabled.\n");
> +
> /*
> * Initialize the remaining cpc_regs as unsupported.
> * Example: In case FW exposes CPPC v2, the below loop will
> initialize
>
Thanks for the suggestion. I will add this as a separate patch in v7.
Thank you,
Sumit Gupta
Powered by blists - more mailing lists