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

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

[Al Stone]

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.

Changes for v4:
    -- Replaced magic constants with #defines (Rafael Wysocki)
    -- Renamed cppc_unitless_to_khz() to cppc_to_khz() (Rafael Wysocki)
    -- Replaced hidden initialization with a clearer form (Rafael Wysocki)
    -- Instead of picking up the first Max Speed value from DMI, we will
       now get the largest Max Speed; still an approximation, but slightly
       less subject to error (Rafael Wysocki)
    -- Kconfig for cppc_cpufreq now depends on DMI, instead of selecting
       it, in order to make sure DMI is set up properly (Rafael Wysocki)

Changes for v3:
    -- Added clarifying commentary re short-term vs long-term fix (Alexey
       Klimov)
    -- Added range checking code to ensure proper arithmetic occurs,
       especially no division by zero (Alexey Klimov)

Changes for v2:
    -- Corrected thinko: needed to have DEPENDS on DMI in Kconfig.arm,
       not SELECT DMI (found by build daemon)

Signed-off-by: Al Stone <ahs3@redhat.com>
Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
---
 drivers/cpufreq/cppc_cpufreq.c | 53 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
index 8882b8e..6debc18 100644
--- a/drivers/cpufreq/cppc_cpufreq.c
+++ b/drivers/cpufreq/cppc_cpufreq.c
@@ -19,10 +19,19 @@
 #include <linux/delay.h>
 #include <linux/cpu.h>
 #include <linux/cpufreq.h>
+#include <linux/dmi.h>
 #include <linux/vmalloc.h>
 
+#include <asm/unaligned.h>
+
 #include <acpi/cppc_acpi.h>
 
+/* Minimum struct length needed for the DMI processor entry we want */
+#define DMI_ENTRY_PROCESSOR_MIN_LENGTH	48
+
+/* Offest in the DMI processor structure for the max frequency */
+#define DMI_PROCESSOR_MAX_SPEED  0x14
+
 /*
  * These structs contain information parsed from per CPU
  * ACPI _CPC structures.
@@ -32,6 +41,39 @@
  */
 static struct cpudata **all_cpu_data;
 
+/* Capture the max KHz from DMI */
+static u64 cppc_dmi_max_khz;
+
+/* Callback function used to retrieve the max frequency from DMI */
+static void cppc_find_dmi_mhz(const struct dmi_header *dm, void *private)
+{
+	const u8 *dmi_data = (const u8 *)dm;
+	u16 *mhz = (u16 *)private;
+
+	if (dm->type == DMI_ENTRY_PROCESSOR &&
+	    dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) {
+		u16 val = (u16)get_unaligned((const u16 *)
+				(dmi_data + DMI_PROCESSOR_MAX_SPEED));
+		*mhz = val > *mhz ? val : *mhz;
+	}
+}
+
+/* Look up the max frequency in DMI */
+static u64 cppc_get_dmi_max_khz(void)
+{
+	u16 mhz = 0;
+
+	dmi_walk(cppc_find_dmi_mhz, &mhz);
+
+	/*
+	 * Real stupid fallback value, just in case there is no
+	 * actual value set.
+	 */
+	mhz = mhz ? mhz : 1;
+
+	return (1000 * mhz);
+}
+
 static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
 		unsigned int target_freq,
 		unsigned int relation)
@@ -42,7 +84,7 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
 
 	cpu = all_cpu_data[policy->cpu];
 
-	cpu->perf_ctrls.desired_perf = target_freq;
+	cpu->perf_ctrls.desired_perf = target_freq * policy->max / cppc_dmi_max_khz;
 	freqs.old = policy->cur;
 	freqs.new = target_freq;
 
@@ -94,8 +136,10 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 		return ret;
 	}
 
-	policy->min = cpu->perf_caps.lowest_perf;
-	policy->max = cpu->perf_caps.highest_perf;
+	cppc_dmi_max_khz = cppc_get_dmi_max_khz();
+
+	policy->min = cpu->perf_caps.lowest_perf * cppc_dmi_max_khz / cpu->perf_caps.highest_perf;
+	policy->max = cppc_dmi_max_khz;
 	policy->cpuinfo.min_freq = policy->min;
 	policy->cpuinfo.max_freq = policy->max;
 	policy->shared_type = cpu->shared_type;
@@ -112,7 +156,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
 	cpu->cur_policy = policy;
 
 	/* Set policy->cur to max now. The governors will adjust later. */
-	policy->cur = cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
+	policy->cur = cppc_dmi_max_khz;
+	cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
 
 	ret = cppc_set_perf(cpu_num, &cpu->perf_ctrls);
 	if (ret)
-- 
2.7.4

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

[Al Stone]

On 07/20/2016 03:10 PM, 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).
> [snip...]

Ping?  If anyone has sent comments, I have not received them.

Thanks in advance for the time and effort.  I know merge window can be
a very busy time.

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

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

[Viresh Kumar]

[+ 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.
> 
> Changes for v4:
>     -- Replaced magic constants with #defines (Rafael Wysocki)
>     -- Renamed cppc_unitless_to_khz() to cppc_to_khz() (Rafael Wysocki)
>     -- Replaced hidden initialization with a clearer form (Rafael Wysocki)
>     -- Instead of picking up the first Max Speed value from DMI, we will
>        now get the largest Max Speed; still an approximation, but slightly
>        less subject to error (Rafael Wysocki)
>     -- Kconfig for cppc_cpufreq now depends on DMI, instead of selecting
>        it, in order to make sure DMI is set up properly (Rafael Wysocki)
> 
> Changes for v3:
>     -- Added clarifying commentary re short-term vs long-term fix (Alexey
>        Klimov)
>     -- Added range checking code to ensure proper arithmetic occurs,
>        especially no division by zero (Alexey Klimov)
> 
> Changes for v2:
>     -- Corrected thinko: needed to have DEPENDS on DMI in Kconfig.arm,
>        not SELECT DMI (found by build daemon)
> 
> Signed-off-by: Al Stone <ahs3@redhat.com>
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> ---
>  drivers/cpufreq/cppc_cpufreq.c | 53 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 8882b8e..6debc18 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -19,10 +19,19 @@
>  #include <linux/delay.h>
>  #include <linux/cpu.h>
>  #include <linux/cpufreq.h>
> +#include <linux/dmi.h>
>  #include <linux/vmalloc.h>
>  
> +#include <asm/unaligned.h>
> +
>  #include <acpi/cppc_acpi.h>
>  
> +/* Minimum struct length needed for the DMI processor entry we want */
> +#define DMI_ENTRY_PROCESSOR_MIN_LENGTH	48
> +
> +/* Offest in the DMI processor structure for the max frequency */
> +#define DMI_PROCESSOR_MAX_SPEED  0x14
> +
>  /*
>   * These structs contain information parsed from per CPU
>   * ACPI _CPC structures.
> @@ -32,6 +41,39 @@
>   */
>  static struct cpudata **all_cpu_data;
>  
> +/* Capture the max KHz from DMI */
> +static u64 cppc_dmi_max_khz;
> +
> +/* Callback function used to retrieve the max frequency from DMI */
> +static void cppc_find_dmi_mhz(const struct dmi_header *dm, void *private)
> +{
> +	const u8 *dmi_data = (const u8 *)dm;
> +	u16 *mhz = (u16 *)private;
> +
> +	if (dm->type == DMI_ENTRY_PROCESSOR &&
> +	    dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) {
> +		u16 val = (u16)get_unaligned((const u16 *)
> +				(dmi_data + DMI_PROCESSOR_MAX_SPEED));
> +		*mhz = val > *mhz ? val : *mhz;
> +	}
> +}
> +
> +/* Look up the max frequency in DMI */
> +static u64 cppc_get_dmi_max_khz(void)
> +{
> +	u16 mhz = 0;
> +
> +	dmi_walk(cppc_find_dmi_mhz, &mhz);
> +
> +	/*
> +	 * Real stupid fallback value, just in case there is no
> +	 * actual value set.
> +	 */
> +	mhz = mhz ? mhz : 1;
> +
> +	return (1000 * mhz);
> +}
> +
>  static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>  		unsigned int target_freq,
>  		unsigned int relation)
> @@ -42,7 +84,7 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>  
>  	cpu = all_cpu_data[policy->cpu];
>  
> -	cpu->perf_ctrls.desired_perf = target_freq;
> +	cpu->perf_ctrls.desired_perf = target_freq * policy->max / cppc_dmi_max_khz;
>  	freqs.old = policy->cur;
>  	freqs.new = target_freq;
>  
> @@ -94,8 +136,10 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  		return ret;
>  	}
>  
> -	policy->min = cpu->perf_caps.lowest_perf;
> -	policy->max = cpu->perf_caps.highest_perf;
> +	cppc_dmi_max_khz = cppc_get_dmi_max_khz();
> +
> +	policy->min = cpu->perf_caps.lowest_perf * cppc_dmi_max_khz / cpu->perf_caps.highest_perf;
> +	policy->max = cppc_dmi_max_khz;
>  	policy->cpuinfo.min_freq = policy->min;
>  	policy->cpuinfo.max_freq = policy->max;
>  	policy->shared_type = cpu->shared_type;
> @@ -112,7 +156,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>  	cpu->cur_policy = policy;
>  
>  	/* Set policy->cur to max now. The governors will adjust later. */
> -	policy->cur = cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
> +	policy->cur = cppc_dmi_max_khz;
> +	cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
>  
>  	ret = cppc_set_perf(cpu_num, &cpu->perf_ctrls);
>  	if (ret)
> -- 
> 2.7.4

-- 
viresh

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

[Al Stone]

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.
>>
>> Changes for v4:
>>     -- Replaced magic constants with #defines (Rafael Wysocki)
>>     -- Renamed cppc_unitless_to_khz() to cppc_to_khz() (Rafael Wysocki)
>>     -- Replaced hidden initialization with a clearer form (Rafael Wysocki)
>>     -- Instead of picking up the first Max Speed value from DMI, we will
>>        now get the largest Max Speed; still an approximation, but slightly
>>        less subject to error (Rafael Wysocki)
>>     -- Kconfig for cppc_cpufreq now depends on DMI, instead of selecting
>>        it, in order to make sure DMI is set up properly (Rafael Wysocki)
>>
>> Changes for v3:
>>     -- Added clarifying commentary re short-term vs long-term fix (Alexey
>>        Klimov)
>>     -- Added range checking code to ensure proper arithmetic occurs,
>>        especially no division by zero (Alexey Klimov)
>>
>> Changes for v2:
>>     -- Corrected thinko: needed to have DEPENDS on DMI in Kconfig.arm,
>>        not SELECT DMI (found by build daemon)
>>
>> Signed-off-by: Al Stone <ahs3@redhat.com>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>> ---
>>  drivers/cpufreq/cppc_cpufreq.c | 53 ++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>> index 8882b8e..6debc18 100644
>> --- a/drivers/cpufreq/cppc_cpufreq.c
>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>> @@ -19,10 +19,19 @@
>>  #include <linux/delay.h>
>>  #include <linux/cpu.h>
>>  #include <linux/cpufreq.h>
>> +#include <linux/dmi.h>
>>  #include <linux/vmalloc.h>
>>  
>> +#include <asm/unaligned.h>
>> +
>>  #include <acpi/cppc_acpi.h>
>>  
>> +/* Minimum struct length needed for the DMI processor entry we want */
>> +#define DMI_ENTRY_PROCESSOR_MIN_LENGTH	48
>> +
>> +/* Offest in the DMI processor structure for the max frequency */
>> +#define DMI_PROCESSOR_MAX_SPEED  0x14
>> +
>>  /*
>>   * These structs contain information parsed from per CPU
>>   * ACPI _CPC structures.
>> @@ -32,6 +41,39 @@
>>   */
>>  static struct cpudata **all_cpu_data;
>>  
>> +/* Capture the max KHz from DMI */
>> +static u64 cppc_dmi_max_khz;
>> +
>> +/* Callback function used to retrieve the max frequency from DMI */
>> +static void cppc_find_dmi_mhz(const struct dmi_header *dm, void *private)
>> +{
>> +	const u8 *dmi_data = (const u8 *)dm;
>> +	u16 *mhz = (u16 *)private;
>> +
>> +	if (dm->type == DMI_ENTRY_PROCESSOR &&
>> +	    dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) {
>> +		u16 val = (u16)get_unaligned((const u16 *)
>> +				(dmi_data + DMI_PROCESSOR_MAX_SPEED));
>> +		*mhz = val > *mhz ? val : *mhz;
>> +	}
>> +}
>> +
>> +/* Look up the max frequency in DMI */
>> +static u64 cppc_get_dmi_max_khz(void)
>> +{
>> +	u16 mhz = 0;
>> +
>> +	dmi_walk(cppc_find_dmi_mhz, &mhz);
>> +
>> +	/*
>> +	 * Real stupid fallback value, just in case there is no
>> +	 * actual value set.
>> +	 */
>> +	mhz = mhz ? mhz : 1;
>> +
>> +	return (1000 * mhz);
>> +}
>> +
>>  static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>>  		unsigned int target_freq,
>>  		unsigned int relation)
>> @@ -42,7 +84,7 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>>  
>>  	cpu = all_cpu_data[policy->cpu];
>>  
>> -	cpu->perf_ctrls.desired_perf = target_freq;
>> +	cpu->perf_ctrls.desired_perf = target_freq * policy->max / cppc_dmi_max_khz;
>>  	freqs.old = policy->cur;
>>  	freqs.new = target_freq;
>>  
>> @@ -94,8 +136,10 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>  		return ret;
>>  	}
>>  
>> -	policy->min = cpu->perf_caps.lowest_perf;
>> -	policy->max = cpu->perf_caps.highest_perf;
>> +	cppc_dmi_max_khz = cppc_get_dmi_max_khz();
>> +
>> +	policy->min = cpu->perf_caps.lowest_perf * cppc_dmi_max_khz / cpu->perf_caps.highest_perf;
>> +	policy->max = cppc_dmi_max_khz;
>>  	policy->cpuinfo.min_freq = policy->min;
>>  	policy->cpuinfo.max_freq = policy->max;
>>  	policy->shared_type = cpu->shared_type;
>> @@ -112,7 +156,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>  	cpu->cur_policy = policy;
>>  
>>  	/* Set policy->cur to max now. The governors will adjust later. */
>> -	policy->cur = cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
>> +	policy->cur = cppc_dmi_max_khz;
>> +	cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
>>  
>>  	ret = cppc_set_perf(cpu_num, &cpu->perf_ctrls);
>>  	if (ret)
>> -- 
>> 2.7.4
> 

Another gentle ping -- any comments?  Can this get pulled in now?

Thanks.

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

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

[Al Stone]

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?

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.
>>>
>>> Changes for v4:
>>>     -- Replaced magic constants with #defines (Rafael Wysocki)
>>>     -- Renamed cppc_unitless_to_khz() to cppc_to_khz() (Rafael Wysocki)
>>>     -- Replaced hidden initialization with a clearer form (Rafael Wysocki)
>>>     -- Instead of picking up the first Max Speed value from DMI, we will
>>>        now get the largest Max Speed; still an approximation, but slightly
>>>        less subject to error (Rafael Wysocki)
>>>     -- Kconfig for cppc_cpufreq now depends on DMI, instead of selecting
>>>        it, in order to make sure DMI is set up properly (Rafael Wysocki)
>>>
>>> Changes for v3:
>>>     -- Added clarifying commentary re short-term vs long-term fix (Alexey
>>>        Klimov)
>>>     -- Added range checking code to ensure proper arithmetic occurs,
>>>        especially no division by zero (Alexey Klimov)
>>>
>>> Changes for v2:
>>>     -- Corrected thinko: needed to have DEPENDS on DMI in Kconfig.arm,
>>>        not SELECT DMI (found by build daemon)
>>>
>>> Signed-off-by: Al Stone <ahs3@redhat.com>
>>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>>> ---
>>>  drivers/cpufreq/cppc_cpufreq.c | 53 ++++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 49 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>> index 8882b8e..6debc18 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -19,10 +19,19 @@
>>>  #include <linux/delay.h>
>>>  #include <linux/cpu.h>
>>>  #include <linux/cpufreq.h>
>>> +#include <linux/dmi.h>
>>>  #include <linux/vmalloc.h>
>>>  
>>> +#include <asm/unaligned.h>
>>> +
>>>  #include <acpi/cppc_acpi.h>
>>>  
>>> +/* Minimum struct length needed for the DMI processor entry we want */
>>> +#define DMI_ENTRY_PROCESSOR_MIN_LENGTH	48
>>> +
>>> +/* Offest in the DMI processor structure for the max frequency */
>>> +#define DMI_PROCESSOR_MAX_SPEED  0x14
>>> +
>>>  /*
>>>   * These structs contain information parsed from per CPU
>>>   * ACPI _CPC structures.
>>> @@ -32,6 +41,39 @@
>>>   */
>>>  static struct cpudata **all_cpu_data;
>>>  
>>> +/* Capture the max KHz from DMI */
>>> +static u64 cppc_dmi_max_khz;
>>> +
>>> +/* Callback function used to retrieve the max frequency from DMI */
>>> +static void cppc_find_dmi_mhz(const struct dmi_header *dm, void *private)
>>> +{
>>> +	const u8 *dmi_data = (const u8 *)dm;
>>> +	u16 *mhz = (u16 *)private;
>>> +
>>> +	if (dm->type == DMI_ENTRY_PROCESSOR &&
>>> +	    dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) {
>>> +		u16 val = (u16)get_unaligned((const u16 *)
>>> +				(dmi_data + DMI_PROCESSOR_MAX_SPEED));
>>> +		*mhz = val > *mhz ? val : *mhz;
>>> +	}
>>> +}
>>> +
>>> +/* Look up the max frequency in DMI */
>>> +static u64 cppc_get_dmi_max_khz(void)
>>> +{
>>> +	u16 mhz = 0;
>>> +
>>> +	dmi_walk(cppc_find_dmi_mhz, &mhz);
>>> +
>>> +	/*
>>> +	 * Real stupid fallback value, just in case there is no
>>> +	 * actual value set.
>>> +	 */
>>> +	mhz = mhz ? mhz : 1;
>>> +
>>> +	return (1000 * mhz);
>>> +}
>>> +
>>>  static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>>>  		unsigned int target_freq,
>>>  		unsigned int relation)
>>> @@ -42,7 +84,7 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>>>  
>>>  	cpu = all_cpu_data[policy->cpu];
>>>  
>>> -	cpu->perf_ctrls.desired_perf = target_freq;
>>> +	cpu->perf_ctrls.desired_perf = target_freq * policy->max / cppc_dmi_max_khz;
>>>  	freqs.old = policy->cur;
>>>  	freqs.new = target_freq;
>>>  
>>> @@ -94,8 +136,10 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>  		return ret;
>>>  	}
>>>  
>>> -	policy->min = cpu->perf_caps.lowest_perf;
>>> -	policy->max = cpu->perf_caps.highest_perf;
>>> +	cppc_dmi_max_khz = cppc_get_dmi_max_khz();
>>> +
>>> +	policy->min = cpu->perf_caps.lowest_perf * cppc_dmi_max_khz / cpu->perf_caps.highest_perf;
>>> +	policy->max = cppc_dmi_max_khz;
>>>  	policy->cpuinfo.min_freq = policy->min;
>>>  	policy->cpuinfo.max_freq = policy->max;
>>>  	policy->shared_type = cpu->shared_type;
>>> @@ -112,7 +156,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>  	cpu->cur_policy = policy;
>>>  
>>>  	/* Set policy->cur to max now. The governors will adjust later. */
>>> -	policy->cur = cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
>>> +	policy->cur = cppc_dmi_max_khz;
>>> +	cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
>>>  
>>>  	ret = cppc_set_perf(cpu_num, &cpu->perf_ctrls);
>>>  	if (ret)
>>> -- 
>>> 2.7.4
>>
> 
> Another gentle ping -- any comments?  Can this get pulled in now?
> 
> Thanks.
> 


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

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

[Al Stone]

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?

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.
>>>
>>> Changes for v4:
>>>     -- Replaced magic constants with #defines (Rafael Wysocki)
>>>     -- Renamed cppc_unitless_to_khz() to cppc_to_khz() (Rafael Wysocki)
>>>     -- Replaced hidden initialization with a clearer form (Rafael Wysocki)
>>>     -- Instead of picking up the first Max Speed value from DMI, we will
>>>        now get the largest Max Speed; still an approximation, but slightly
>>>        less subject to error (Rafael Wysocki)
>>>     -- Kconfig for cppc_cpufreq now depends on DMI, instead of selecting
>>>        it, in order to make sure DMI is set up properly (Rafael Wysocki)
>>>
>>> Changes for v3:
>>>     -- Added clarifying commentary re short-term vs long-term fix (Alexey
>>>        Klimov)
>>>     -- Added range checking code to ensure proper arithmetic occurs,
>>>        especially no division by zero (Alexey Klimov)
>>>
>>> Changes for v2:
>>>     -- Corrected thinko: needed to have DEPENDS on DMI in Kconfig.arm,
>>>        not SELECT DMI (found by build daemon)
>>>
>>> Signed-off-by: Al Stone <ahs3@redhat.com>
>>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
>>> ---
>>>  drivers/cpufreq/cppc_cpufreq.c | 53 ++++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 49 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
>>> index 8882b8e..6debc18 100644
>>> --- a/drivers/cpufreq/cppc_cpufreq.c
>>> +++ b/drivers/cpufreq/cppc_cpufreq.c
>>> @@ -19,10 +19,19 @@
>>>  #include <linux/delay.h>
>>>  #include <linux/cpu.h>
>>>  #include <linux/cpufreq.h>
>>> +#include <linux/dmi.h>
>>>  #include <linux/vmalloc.h>
>>>  
>>> +#include <asm/unaligned.h>
>>> +
>>>  #include <acpi/cppc_acpi.h>
>>>  
>>> +/* Minimum struct length needed for the DMI processor entry we want */
>>> +#define DMI_ENTRY_PROCESSOR_MIN_LENGTH	48
>>> +
>>> +/* Offest in the DMI processor structure for the max frequency */
>>> +#define DMI_PROCESSOR_MAX_SPEED  0x14
>>> +
>>>  /*
>>>   * These structs contain information parsed from per CPU
>>>   * ACPI _CPC structures.
>>> @@ -32,6 +41,39 @@
>>>   */
>>>  static struct cpudata **all_cpu_data;
>>>  
>>> +/* Capture the max KHz from DMI */
>>> +static u64 cppc_dmi_max_khz;
>>> +
>>> +/* Callback function used to retrieve the max frequency from DMI */
>>> +static void cppc_find_dmi_mhz(const struct dmi_header *dm, void *private)
>>> +{
>>> +	const u8 *dmi_data = (const u8 *)dm;
>>> +	u16 *mhz = (u16 *)private;
>>> +
>>> +	if (dm->type == DMI_ENTRY_PROCESSOR &&
>>> +	    dm->length >= DMI_ENTRY_PROCESSOR_MIN_LENGTH) {
>>> +		u16 val = (u16)get_unaligned((const u16 *)
>>> +				(dmi_data + DMI_PROCESSOR_MAX_SPEED));
>>> +		*mhz = val > *mhz ? val : *mhz;
>>> +	}
>>> +}
>>> +
>>> +/* Look up the max frequency in DMI */
>>> +static u64 cppc_get_dmi_max_khz(void)
>>> +{
>>> +	u16 mhz = 0;
>>> +
>>> +	dmi_walk(cppc_find_dmi_mhz, &mhz);
>>> +
>>> +	/*
>>> +	 * Real stupid fallback value, just in case there is no
>>> +	 * actual value set.
>>> +	 */
>>> +	mhz = mhz ? mhz : 1;
>>> +
>>> +	return (1000 * mhz);
>>> +}
>>> +
>>>  static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>>>  		unsigned int target_freq,
>>>  		unsigned int relation)
>>> @@ -42,7 +84,7 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy,
>>>  
>>>  	cpu = all_cpu_data[policy->cpu];
>>>  
>>> -	cpu->perf_ctrls.desired_perf = target_freq;
>>> +	cpu->perf_ctrls.desired_perf = target_freq * policy->max / cppc_dmi_max_khz;
>>>  	freqs.old = policy->cur;
>>>  	freqs.new = target_freq;
>>>  
>>> @@ -94,8 +136,10 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>  		return ret;
>>>  	}
>>>  
>>> -	policy->min = cpu->perf_caps.lowest_perf;
>>> -	policy->max = cpu->perf_caps.highest_perf;
>>> +	cppc_dmi_max_khz = cppc_get_dmi_max_khz();
>>> +
>>> +	policy->min = cpu->perf_caps.lowest_perf * cppc_dmi_max_khz / cpu->perf_caps.highest_perf;
>>> +	policy->max = cppc_dmi_max_khz;
>>>  	policy->cpuinfo.min_freq = policy->min;
>>>  	policy->cpuinfo.max_freq = policy->max;
>>>  	policy->shared_type = cpu->shared_type;
>>> @@ -112,7 +156,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
>>>  	cpu->cur_policy = policy;
>>>  
>>>  	/* Set policy->cur to max now. The governors will adjust later. */
>>> -	policy->cur = cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
>>> +	policy->cur = cppc_dmi_max_khz;
>>> +	cpu->perf_ctrls.desired_perf = cpu->perf_caps.highest_perf;
>>>  
>>>  	ret = cppc_set_perf(cpu_num, &cpu->perf_ctrls);
>>>  	if (ret)
>>> -- 
>>> 2.7.4
>>
> 
> Another gentle ping -- any comments?  Can this get pulled in now?
> 
> Thanks.
> 


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

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

[Ashwin Chaugule]

Hi Al,

On Mon, Aug 22, 2016 at 10:16 AM, Al Stone <ahs3@redhat.com> 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.

> 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@google.com>

Cheers,
Ashwin.

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@redhat.com> 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@google.com>
> 
> Cheers,
> Ashwin.
> 

Thanks, Ashwin.

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

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

[Pandruvada, Srinivas]

On Mon, 2016-08-22 at 12:12 -0600, Al Stone wrote:
> On 08/22/2016 11:45 AM, Ashwin Chaugule wrote:
> > 
> > Hi Al,
> > 
> > On Mon, Aug 22, 2016 at 10:16 AM, Al Stone <ahs3@redhat.com> 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.
> > > > > > 

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

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

[Al Stone]

On 08/22/2016 10:31 PM, Pandruvada, Srinivas wrote:
> On Mon, 2016-08-22 at 12:12 -0600, Al Stone wrote:
>> On 08/22/2016 11:45 AM, Ashwin Chaugule wrote:
>>>
>>> Hi Al,
>>>
>>> On Mon, Aug 22, 2016 at 10:16 AM, Al Stone <ahs3@redhat.com> 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.
>>>>>>>
> 
> 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
-----------------------------------

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

[Pandruvada, Srinivas]

On Tue, 2016-08-23 at 10:14 -0600, Al Stone wrote:
> > > 

[...]

> > 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?
> > 
[...]
> 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.
Currently we are not using cppc-cpufreq driver, so not will not
directly impact (you are  not changing acpi-cpufreq source, which we
are using).
x86 has other way to get max/min cpufreq policy frequencies using MSRs.
So you may choose to ignore my comments here, as long as your changes
are limited to drivers/cpufreq/cppc_cpufreq.c

When you are doing:
policy->min = cpu->perf_caps.lowest_perf * cppc_dmi_max_khz / cpu-
>perf_caps.highest_perf;

Aren't you assuming that scale from max to min performance is only
related to frequency? It is possible that many points in between can be
same frequency with multiple voltages.
As per spec " The platform may choose to use a single metric such as
processor frequency, or it may choose to blend multiple hardware
metrics to create a synthetic measure of performance".

Thanks,
Srinivas 


> 

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

[Al Stone]

On 08/25/2016 04:00 PM, Pandruvada, Srinivas wrote:
> On Tue, 2016-08-23 at 10:14 -0600, Al Stone wrote:
>>>>
> 
> [...]
> 
>>> 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?
>>>
> [...]
>> 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.
> Currently we are not using cppc-cpufreq driver, so not will not
> directly impact (you are  not changing acpi-cpufreq source, which we
> are using).
> x86 has other way to get max/min cpufreq policy frequencies using MSRs.
> So you may choose to ignore my comments here, as long as your changes
> are limited to drivers/cpufreq/cppc_cpufreq.c

Ah, okay.  Yes, the changes are intentionaly limited to cppc_cpufreq.c.

> When you are doing:
> policy->min = cpu->perf_caps.lowest_perf * cppc_dmi_max_khz / cpu-
>> perf_caps.highest_perf;
> 
> Aren't you assuming that scale from max to min performance is only
> related to frequency? It is possible that many points in between can be
> same frequency with multiple voltages.
> As per spec " The platform may choose to use a single metric such as
> processor frequency, or it may choose to blend multiple hardware
> metrics to create a synthetic measure of performance".

There are a couple of assumptions: (1) that the CPPC scale is a linear
scale, and that (2) there is a direct correlation to the frequency.  And
this is why longer term, we have to separate performance reporting from
the frequency -- any correlation between them is suspect on most modern
processors.  We know a priori that these assumptions are only approximations,
at best.

However, this is the only information we currently have, so we have to make
a best guess...um, I mean, "apply heuristics".  I'm treating this as two
problems, really: the first is the immediate term where we need to make sure
user space tools don't report complete garbage, which this patch tries to
address.  The second is the much larger problem of changing the way the kernel
reports performance in general, and fixing the user space tools that rely on
the info being reported; I'm still thinking through those patches (suggestions
are always welcome).

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

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

[Rafael J. Wysocki]

On Wednesday, July 20, 2016 03:10:04 PM 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.
> 
> Changes for v4:
>     -- Replaced magic constants with #defines (Rafael Wysocki)
>     -- Renamed cppc_unitless_to_khz() to cppc_to_khz() (Rafael Wysocki)
>     -- Replaced hidden initialization with a clearer form (Rafael Wysocki)
>     -- Instead of picking up the first Max Speed value from DMI, we will
>        now get the largest Max Speed; still an approximation, but slightly
>        less subject to error (Rafael Wysocki)
>     -- Kconfig for cppc_cpufreq now depends on DMI, instead of selecting
>        it, in order to make sure DMI is set up properly (Rafael Wysocki)
> 
> Changes for v3:
>     -- Added clarifying commentary re short-term vs long-term fix (Alexey
>        Klimov)
>     -- Added range checking code to ensure proper arithmetic occurs,
>        especially no division by zero (Alexey Klimov)
> 
> Changes for v2:
>     -- Corrected thinko: needed to have DEPENDS on DMI in Kconfig.arm,
>        not SELECT DMI (found by build daemon)
> 
> Signed-off-by: Al Stone <ahs3@redhat.com>
> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>

Applied.

Thanks,
Rafael

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

[Al Stone]

On 09/13/2016 07:09 PM, Rafael J. Wysocki wrote:
> On Wednesday, July 20, 2016 03:10:04 PM 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.
>>
>> Changes for v4:
>>     -- Replaced magic constants with #defines (Rafael Wysocki)
>>     -- Renamed cppc_unitless_to_khz() to cppc_to_khz() (Rafael Wysocki)
>>     -- Replaced hidden initialization with a clearer form (Rafael Wysocki)
>>     -- Instead of picking up the first Max Speed value from DMI, we will
>>        now get the largest Max Speed; still an approximation, but slightly
>>        less subject to error (Rafael Wysocki)
>>     -- Kconfig for cppc_cpufreq now depends on DMI, instead of selecting
>>        it, in order to make sure DMI is set up properly (Rafael Wysocki)
>>
>> Changes for v3:
>>     -- Added clarifying commentary re short-term vs long-term fix (Alexey
>>        Klimov)
>>     -- Added range checking code to ensure proper arithmetic occurs,
>>        especially no division by zero (Alexey Klimov)
>>
>> Changes for v2:
>>     -- Corrected thinko: needed to have DEPENDS on DMI in Kconfig.arm,
>>        not SELECT DMI (found by build daemon)
>>
>> Signed-off-by: Al Stone <ahs3@redhat.com>
>> Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
> 
> Applied.
> 
> Thanks,
> Rafael
> 

I've been on vacation so just now am seeing this.  Thanks, Rafael!

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