Re: [PATCH v5] Force cppc_cpufreq to report values in KHz to fix user space reporting

[Al Stone]

On 08/22/2016 11:45 AM, Ashwin Chaugule wrote:
> Hi Al,
> 
> On Mon, Aug 22, 2016 at 10:16 AM, Al Stone <ahs3@xxxxxxxxxx> wrote:
>> Maybe a top-post will get attention....
>>
>> Yet another ping; this was first submitted on 20 July, and has received
>> no comments.  It has now been a month and other architectures are starting
>> to use CPPC so they will run into the same errors that this fixes.  Can
>> I get an ACK, NAK, or further instructions, please?
>>
> 
> Apologies for the delay. I thought this patch was merged already.

I've looked in linux-next and linux-pm; I could have missed it, but I didn't
see it...my bad, if I did.

>> Also adding Rafael on the ACPI side, just in case, since he's also reviewing
>> the Intel patches on the linux-acpi mailing list that are adding CPPC usage.
>>
>> On 08/11/2016 12:15 PM, Al Stone wrote:
>>> On 08/01/2016 02:31 PM, Viresh Kumar wrote:
>>>> [+ Ashwin's new email id..]
>>>>
>>>> On 20-07-16, 15:10, Al Stone wrote:
>>>>> When CPPC is being used by ACPI on arm64, user space tools such as
>>>>> cpupower report CPU frequency values from sysfs that are incorrect.
>>>>>
>>>>> What the driver was doing was reporting the values given by ACPI tables
>>>>> in whatever scale was used to provide them.  However, the ACPI spec
>>>>> defines the CPPC values as unitless abstract numbers.  Internal kernel
>>>>> structures such as struct perf_cap, in contrast, expect these values
>>>>> to be in KHz.  When these struct values get reported via sysfs, the
>>>>> user space tools also assume they are in KHz, causing them to report
>>>>> incorrect values (for example, reporting a CPU frequency of 1MHz when
>>>>> it should be 1.8GHz).
>>>>>
>>>>> The downside is that this approach has some assumptions:
>>>>>
>>>>>    (1) It relies on SMBIOS3 being used, *and* that the Max Frequency
>>>>>    value for a processor is set to a non-zero value.
>>>>>
>>>>>    (2) It assumes that all processors run at the same speed, or that
>>>>>    the CPPC values have all been scaled to reflect relative speed.
>>>>>    This patch retrieves the largest CPU Max Frequency from a type 4 DMI
>>>>>    record that it can find.  This may not be an issue, however, as a
>>>>>    sampling of DMI data on x86 and arm64 indicates there is often only
>>>>>    one such record regardless.  Since CPPC is relatively new, it is
>>>>>    unclear if the ACPI ASL will always be written to reflect any sort
>>>>>    of relative performance of processors of differing speeds.
>>>>>
>>>>>    (3) It assumes that performance and frequency both scale linearly.
>>>>>
>>>>> For arm64 servers, this may be sufficient, but it does rely on
>>>>> firmware values being set correctly.  Hence, other approaches will
>>>>> be considered in the future.
>>>>>
>>>>> This has been tested on three arm64 servers, with and without DMI, with
>>>>> and without CPPC support.
>>>>>
>>>>> Changes for v5:
>>>>>     -- Move code to cpufreq/cppc_cpufreq.c from acpi/cppc_acpi.c to keep
>>>>>        frequency-related code together, and keep the CPPC abstract scale
>>>>>        in ACPI (Prashanth Prakash)
>>>>>     -- Fix the scaling to remove the incorrect assumption that frequency
>>>>>        was always a range from zero to max; as a practical matter, it is
>>>>>        not (Prasanth Prakash); this also allowed us to remove an over-
>>>>>        engineered function to do this math.
>>>>>
> 
> This addresses my previous feedback. So FWIW, Acked-by: Ashwin
> Chaugule <ashwinch@xxxxxxxxxx>
> 
> Cheers,
> Ashwin.
> 

Thanks, Ashwin.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@xxxxxxxxxx
-----------------------------------