Hi Rafael,
On 25 September 2015 at 20:05, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
> On Wednesday, September 09, 2015 04:27:04 PM Ashwin Chaugule wrote:
>> CPPC stands for Collaborative Processor Performance Controls
>> and is defined in the ACPI v5.0+ spec. It describes CPU
>> performance controls on an abstract and continuous scale
>> allowing the platform (e.g. remote power processor) to flexibly
>> optimize CPU performance with its knowledge of power budgets
>> and other architecture specific knowledge.
>>
>> This patch adds a shim which exports commonly used functions
>> to get and set CPPC specific controls for each CPU. This enables
>> CPUFreq drivers to gather per CPU performance data and use
>> with exisiting governors or even allows for customized governors
>> which are implemented inside CPUFreq drivers.
>>
>> Signed-off-by: Ashwin Chaugule <ashwin.chaugule@xxxxxxxxxx>
>> Reviewed-by: Al Stone <al.stone@xxxxxxxxxx>
>> ---
>
> [cut]
>
>> +
>> +/**
>> + * acpi_get_psd_map - Map the CPUs in a common freq domain as indicated
>> + * in the respective _PSD entries.
>
> One minor nit: this needs to be one-line in a proper kerneldoc.
>
>> + * @all_cpu_data: This array of pointers contains all the CPU specific
>> + * details to access CPPC registers and freq domain information for each
>> + * CPU.
>
> Ditto.
>
> You can add more description below.
Ok. Will do.
>
> [cut]
>
>> +int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
>> +{
>> + struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpunum);
>> + struct cpc_register_resource *delivered_reg, *reference_reg;
>> + u64 delivered, reference;
>> + int ret = 0;
>> +
>> + if (!cpc_desc) {
>> + pr_debug("No CPC descriptor for CPU:%d\n", cpunum);
>> + return -ENODEV;
>> + }
>> +
>> + delivered_reg = &cpc_desc->cpc_regs[DELIVERED_CTR];
>> + reference_reg = &cpc_desc->cpc_regs[REFERENCE_CTR];
>> +
>> + spin_lock(&pcc_lock);
>
> I still have the question about this lock.
>
> Why does it have to be a spinlock?
The idea is to let CPUs send in their requests as quickly as possible.
The PCC transaction only relays the request. So the CPUs don't really
wait for all the clk + voltage toggling etc. I think any other
primitive, might cause unnecessary delays in sending requests. Also
FWIW, this is another thing I was influenced by from the pcc-cpufreq.c
driver. :)
Regards,
Ashwin.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
