The limits->max_perf is rounded_up but immediately overwritten by
another assignment to limits->max_perf. Move it to correct
position.
While here also added a pr_debug call in set_policy to aid in
debugging.
Fixes: 785ee2788141 "cpufreq: intel_pstate: Fix limits->max_perf rounding error"
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for calculating
limits. But this will not be correct. By definition the intel_pstate
sysfs limits, shows percentage of available performance. So when
BIOS has disabled turbo, the available performance is max non turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2016-06-08 00:42:12
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
[off-list ref] wrote:
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for calculating
limits. But this will not be correct. By definition the intel_pstate
sysfs limits, shows percentage of available performance. So when
BIOS has disabled turbo, the available performance is max non turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
I guess we need this in -stable?
If so, all of them, or is there a specific starting point?
On Wed, 2016-06-08 at 02:42 +0200, Rafael J. Wysocki wrote:
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for calculating
limits. But this will not be correct. By definition the
intel_pstate
sysfs limits, shows percentage of available performance. So when
BIOS has disabled turbo, the available performance is max non
turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
.com>
I guess we need this in -stable?
Yes.
If so, all of them, or is there a specific starting point?
I think this is from (even in 3.10, it should have the same behavior
looking at the code).
Thanks,
Srinivas
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2016-06-08 00:50:16
On Wed, Jun 8, 2016 at 2:48 AM, Srinivas Pandruvada
[off-list ref] wrote:
On Wed, 2016-06-08 at 02:42 +0200, Rafael J. Wysocki wrote:
quoted
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for calculating
limits. But this will not be correct. By definition the
intel_pstate
sysfs limits, shows percentage of available performance. So when
BIOS has disabled turbo, the available performance is max non
turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel
.com>
I guess we need this in -stable?
Yes.
quoted
If so, all of them, or is there a specific starting point?
I think this is from (even in 3.10, it should have the same behavior
looking at the code).
On Wed, 2016-06-08 at 02:50 +0200, Rafael J. Wysocki wrote:
On Wed, Jun 8, 2016 at 2:48 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
On Wed, 2016-06-08 at 02:42 +0200, Rafael J. Wysocki wrote:
quoted
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using
max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for
calculating
limits. But this will not be correct. By definition the
intel_pstate
sysfs limits, shows percentage of available performance. So
when
BIOS has disabled turbo, the available performance is max non
turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.i
ntel
.com>
I guess we need this in -stable?
Yes.
quoted
If so, all of them, or is there a specific starting point?
I think this is from (even in 3.10, it should have the same
behavior
looking at the code).
OK
All applicable, then?
Yes. But for some of the trees we need rebase, as the patch may not
apply cleanly.
Thanks,
Srinivas
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2016-06-08 01:06:04
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted hunk
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for calculating
limits. But this will not be correct. By definition the intel_pstate
sysfs limits, shows percentage of available performance. So when
BIOS has disabled turbo, the available performance is max non turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
@@ -1561,8 +1561,14 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)/* cpuinfo and default policy values */policy->cpuinfo.min_freq=cpu->pstate.min_pstate*cpu->pstate.scaling;-policy->cpuinfo.max_freq=-cpu->pstate.turbo_pstate*cpu->pstate.scaling;+update_turbo_state();+if(limits->turbo_disabled)+policy->cpuinfo.max_freq=+cpu->pstate.max_pstate*cpu->pstate.scaling;+else+policy->cpuinfo.max_freq=+cpu->pstate.turbo_pstate*cpu->pstate.scaling;+intel_pstate_init_acpi_perf_limits(policy);policy->cpuinfo.transition_latency=CPUFREQ_ETERNAL;cpumask_set_cpu(policy->cpu,policy->cpus);--
BTW, I would write this slightly differently. What about:
policy->cpuinfo.max_freq = limits->turbo_disabled ?
cpu->pstate.max_pstate : cpu->pstate.turbo_pstate;
policy->cpuinfo.max_freq *= cpu->pstate.scaling;
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2016-06-08 01:10:59
On Wed, Jun 8, 2016 at 3:06 AM, Rafael J. Wysocki [off-list ref] wrote:
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for calculating
limits. But this will not be correct. By definition the intel_pstate
sysfs limits, shows percentage of available performance. So when
BIOS has disabled turbo, the available performance is max non turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
@@ -1561,8 +1561,14 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)/* cpuinfo and default policy values */policy->cpuinfo.min_freq=cpu->pstate.min_pstate*cpu->pstate.scaling;-policy->cpuinfo.max_freq=-cpu->pstate.turbo_pstate*cpu->pstate.scaling;+update_turbo_state();+if(limits->turbo_disabled)+policy->cpuinfo.max_freq=+cpu->pstate.max_pstate*cpu->pstate.scaling;+else+policy->cpuinfo.max_freq=+cpu->pstate.turbo_pstate*cpu->pstate.scaling;+intel_pstate_init_acpi_perf_limits(policy);policy->cpuinfo.transition_latency=CPUFREQ_ETERNAL;cpumask_set_cpu(policy->cpu,policy->cpus);--
BTW, I would write this slightly differently. What about:
policy->cpuinfo.max_freq = limits->turbo_disabled ?
cpu->pstate.max_pstate : cpu->pstate.turbo_pstate;
policy->cpuinfo.max_freq *= cpu->pstate.scaling;
But of course without GMail-induced whitespace breakage.
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2016-06-08 01:24:35
On Wed, Jun 8, 2016 at 3:10 AM, Rafael J. Wysocki [off-list ref] wrote:
On Wed, Jun 8, 2016 at 3:06 AM, Rafael J. Wysocki [off-list ref] wrote:
quoted
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
[off-list ref] wrote:
quoted
When turbo is disabled, the set_policy interface is broken.
For example, when turbo is disabled and
cpuinfo.max = 2900000 (full max turbo frequency)
Setting the limits results in frequency less than settings:
Set 1000000 KHz results in 0700000 KHz
Set 1500000 KHz results in 1100000 KHz
Set 2000000 KHz results in 1500000 KHz
This is because limits->max_perf fraction is calculated using max
turbo frequency as the reference, but when the max P-State is
capped in the function intel_pstate_get_min_max, the reference
is not the max turbo P-State. This results in reducing max
P-State.
One option is to always use max turbo as reference for calculating
limits. But this will not be correct. By definition the intel_pstate
sysfs limits, shows percentage of available performance. So when
BIOS has disabled turbo, the available performance is max non turbo.
So the max_perf_pct should still show 100%.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
@@ -1561,8 +1561,14 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)/* cpuinfo and default policy values */policy->cpuinfo.min_freq=cpu->pstate.min_pstate*cpu->pstate.scaling;-policy->cpuinfo.max_freq=-cpu->pstate.turbo_pstate*cpu->pstate.scaling;+update_turbo_state();+if(limits->turbo_disabled)+policy->cpuinfo.max_freq=+cpu->pstate.max_pstate*cpu->pstate.scaling;+else+policy->cpuinfo.max_freq=+cpu->pstate.turbo_pstate*cpu->pstate.scaling;+intel_pstate_init_acpi_perf_limits(policy);policy->cpuinfo.transition_latency=CPUFREQ_ETERNAL;cpumask_set_cpu(policy->cpu,policy->cpus);--
BTW, I would write this slightly differently. What about:
policy->cpuinfo.max_freq = limits->turbo_disabled ?
cpu->pstate.max_pstate : cpu->pstate.turbo_pstate;
policy->cpuinfo.max_freq *= cpu->pstate.scaling;
But of course without GMail-induced whitespace breakage.
So I went on and changed it this way before applying. Please check
the result in bleeding-edge.