| Message ID | 1465346333-3104-2-git-send-email-srinivas.pandruvada@linux.intel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
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); > -- -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); > > -- -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(-)