Thread (13 messages) 13 messages, 5 authors, 2016-09-19

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

From: Al Stone <hidden>
Date: 2016-08-23 16:14:42
Also in: lkml

On 08/22/2016 10:31 PM, Pandruvada, Srinivas wrote:
On Mon, 2016-08-22 at 12:12 -0600, Al Stone wrote:
quoted
On 08/22/2016 11:45 AM, Ashwin Chaugule wrote:
quoted
Hi Al,

On Mon, Aug 22, 2016 at 10:16 AM, Al Stone [off-list ref] wrote:
quoted
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.
quoted
quoted
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:
quoted
On 08/01/2016 02:31 PM, Viresh Kumar wrote:
quoted
[+ Ashwin's new email id..]

On 20-07-16, 15:10, Al Stone wrote:
quoted
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.
In x86 when CPPC is used, the unit is really unit-less in CPPC tables.
This means that cpu->perf_caps.highest_perf can be just 0xff, instead
of some scaled cppc max performance corresponding to max MHZ the
processor can support. This allows the processor to cap at max which it
can deliver.
Is this case not possible for ARM SoCs?

Thanks,
Srinivas
If I understand the question properly, I don't think it matters, and I don't
think that will work for x86 either.

This patch is meant to allow CPPC to continue operating solely based on the
abstract scale provided by the ACPI tables; this should be true regardless
of architecture.  Any actual processor performance changes are still guided
solely by the CPPC scale provided in the tables, and not the values in the
cpu->perf_caps struct.

Assuming I understand the kernel code, the values in cpu->perf_caps -- in this
case -- are really just for reporting to user space via sysfs, which is the
root of the problem: user space expects frequencies, and we have none when using
CPPC so we have to provide an approximation.  In those circumstances, I think a
value of 0xff would be kind of confusing in sysfs, since it's basically saying
the CPU is operating at a frequency equal to the largest integer value.

To be fair, this is how the ARM processor implements CPPC; I have not examined
in detail the newly submitted x86 patches to use CPPC so I cannot comment on
those.  This patch was written well before those showed up.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help