* [PATCH 1/2] cpufreq: intel_pstate: Move limits->max_perf to correct position
@ 2016-06-08 0:38 Srinivas Pandruvada
2016-06-08 0:38 ` [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo Srinivas Pandruvada
0 siblings, 1 reply; 10+ messages in thread
From: Srinivas Pandruvada @ 2016-06-08 0:38 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, Srinivas Pandruvada
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(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 0d159b5..724b905 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1460,6 +1460,9 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
intel_pstate_clear_update_util_hook(policy->cpu);
+ pr_debug("set_policy cpuinfo.max %u policy->max %u\n",
+ policy->cpuinfo.max_freq, policy->max);
+
cpu = all_cpu_data[0];
if (cpu->pstate.max_pstate_physical > cpu->pstate.max_pstate &&
policy->max < policy->cpuinfo.max_freq &&
@@ -1495,13 +1498,13 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
limits->max_sysfs_pct);
limits->max_perf_pct = max(limits->min_policy_pct,
limits->max_perf_pct);
- limits->max_perf = round_up(limits->max_perf, FRAC_BITS);
/* Make sure min_perf_pct <= max_perf_pct */
limits->min_perf_pct = min(limits->max_perf_pct, limits->min_perf_pct);
limits->min_perf = div_fp(limits->min_perf_pct, 100);
limits->max_perf = div_fp(limits->max_perf_pct, 100);
+ limits->max_perf = round_up(limits->max_perf, FRAC_BITS);
out:
intel_pstate_set_update_util_hook(policy->cpu);
--
2.5.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 0:38 [PATCH 1/2] cpufreq: intel_pstate: Move limits->max_perf to correct position Srinivas Pandruvada
@ 2016-06-08 0:38 ` Srinivas Pandruvada
2016-06-08 0:42 ` Rafael J. Wysocki
2016-06-08 1:06 ` Rafael J. Wysocki
0 siblings, 2 replies; 10+ messages in thread
From: Srinivas Pandruvada @ 2016-06-08 0:38 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, Srinivas Pandruvada
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(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 724b905..2116666 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -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);
--
2.5.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 0:38 ` [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo Srinivas Pandruvada
@ 2016-06-08 0:42 ` Rafael J. Wysocki
2016-06-08 0:48 ` Srinivas Pandruvada
2016-06-08 1:06 ` Rafael J. Wysocki
1 sibling, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-06-08 0:42 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Rafael J. Wysocki, linux-pm
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> 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?
> ---
> drivers/cpufreq/intel_pstate.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 724b905..2116666 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -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);
> --
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 0:42 ` Rafael J. Wysocki
@ 2016-06-08 0:48 ` Srinivas Pandruvada
2016-06-08 0:50 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Srinivas Pandruvada @ 2016-06-08 0:48 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Rafael J. Wysocki, linux-pm
On Wed, 2016-06-08 at 02:42 +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> 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?
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
>
> >
> > ---
> > drivers/cpufreq/intel_pstate.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/intel_pstate.c
> > b/drivers/cpufreq/intel_pstate.c
> > index 724b905..2116666 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -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);
> > --
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 0:48 ` Srinivas Pandruvada
@ 2016-06-08 0:50 ` Rafael J. Wysocki
2016-06-08 0:55 ` Srinivas Pandruvada
0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-06-08 0:50 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Rafael J. Wysocki, Rafael J. Wysocki, linux-pm
On Wed, Jun 8, 2016 at 2:48 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
> On Wed, 2016-06-08 at 02:42 +0200, Rafael J. Wysocki wrote:
>> On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
>> <srinivas.pandruvada@linux.intel.com> 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?
> 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).
OK
All applicable, then?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 0:50 ` Rafael J. Wysocki
@ 2016-06-08 0:55 ` Srinivas Pandruvada
0 siblings, 0 replies; 10+ messages in thread
From: Srinivas Pandruvada @ 2016-06-08 0:55 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Rafael J. Wysocki, linux-pm
On Wed, 2016-06-08 at 02:50 +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 8, 2016 at 2:48 AM, Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> >
> > On Wed, 2016-06-08 at 02:42 +0200, Rafael J. Wysocki wrote:
> > >
> > > On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
> > > <srinivas.pandruvada@linux.intel.com> 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.i
> > > > ntel
> > > > .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).
> OK
>
> All applicable, then?
Yes. But for some of the trees we need rebase, as the patch may not
apply cleanly.
Thanks,
Srinivas
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 0:38 ` [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo Srinivas Pandruvada
2016-06-08 0:42 ` Rafael J. Wysocki
@ 2016-06-08 1:06 ` Rafael J. Wysocki
2016-06-08 1:10 ` Rafael J. Wysocki
1 sibling, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-06-08 1:06 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Rafael J. Wysocki, linux-pm
On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
<srinivas.pandruvada@linux.intel.com> 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>
> ---
> drivers/cpufreq/intel_pstate.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 724b905..2116666 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -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;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 1:06 ` Rafael J. Wysocki
@ 2016-06-08 1:10 ` Rafael J. Wysocki
2016-06-08 1:24 ` Rafael J. Wysocki
0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-06-08 1:10 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Srinivas Pandruvada, Rafael J. Wysocki, linux-pm
On Wed, Jun 8, 2016 at 3:06 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
> <srinivas.pandruvada@linux.intel.com> 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>
>> ---
>> drivers/cpufreq/intel_pstate.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>> index 724b905..2116666 100644
>> --- a/drivers/cpufreq/intel_pstate.c
>> +++ b/drivers/cpufreq/intel_pstate.c
>> @@ -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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 1:10 ` Rafael J. Wysocki
@ 2016-06-08 1:24 ` Rafael J. Wysocki
2016-06-08 15:39 ` Srinivas Pandruvada
0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2016-06-08 1:24 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: Rafael J. Wysocki, linux-pm
On Wed, Jun 8, 2016 at 3:10 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Wed, Jun 8, 2016 at 3:06 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Wed, Jun 8, 2016 at 2:38 AM, Srinivas Pandruvada
>> <srinivas.pandruvada@linux.intel.com> 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>
>>> ---
>>> drivers/cpufreq/intel_pstate.c | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
>>> index 724b905..2116666 100644
>>> --- a/drivers/cpufreq/intel_pstate.c
>>> +++ b/drivers/cpufreq/intel_pstate.c
>>> @@ -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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo
2016-06-08 1:24 ` Rafael J. Wysocki
@ 2016-06-08 15:39 ` Srinivas Pandruvada
0 siblings, 0 replies; 10+ messages in thread
From: Srinivas Pandruvada @ 2016-06-08 15:39 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Rafael J. Wysocki, linux-pm
On Wed, 2016-06-08 at 03:24 +0200, Rafael J. Wysocki wrote:
> >
[...]
> On Wed, Jun 8, 2016 at 3:10 AM, Rafael J. Wysocki <rafael@kernel.org>
> wrote:
> > > 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.
Checked. Looks good.
Thanks,
Srinivas
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-06-08 15:38 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 0:38 [PATCH 1/2] cpufreq: intel_pstate: Move limits->max_perf to correct position Srinivas Pandruvada
2016-06-08 0:38 ` [PATCH 2/2] cpufreq: intel_pstate: Fix set_policy interface for no_turbo Srinivas Pandruvada
2016-06-08 0:42 ` Rafael J. Wysocki
2016-06-08 0:48 ` Srinivas Pandruvada
2016-06-08 0:50 ` Rafael J. Wysocki
2016-06-08 0:55 ` Srinivas Pandruvada
2016-06-08 1:06 ` Rafael J. Wysocki
2016-06-08 1:10 ` Rafael J. Wysocki
2016-06-08 1:24 ` Rafael J. Wysocki
2016-06-08 15:39 ` Srinivas Pandruvada
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.