Bug 64251

Summary: Intel Pstate driver minimum percent does not change with turbo off
Product: Power Management Reporter: Doug Smythies (dsmythies)
Component: cpufreqAssignee: Dirk Brandewie (dirk.brandewie)
Status: CLOSED CODE_FIX    
Severity: normal CC: dirk.brandewie, lenb, rui.zhang, tianyu.lan
Priority: P1    
Hardware: Intel   
OS: Linux   
Kernel Version: 3.12rc7 Subsystem:
Regression: No Bisected commit-id:
Attachments: Frequency Vs. Requested Frequency
BIOS disabled Turbo mode CPU freq Vs. Requested.

Description Doug Smythies 2013-11-02 21:42:47 UTC
Created attachment 113141 [details]
Frequency Vs. Requested Frequency

When turbo is turned off the maximum frequency percent does not change. O.K.
So the minimum frequency percent should change, but it doesn't.

Using my computer as an example: The turbo on maximum frequency is 3.8 GHz; The turbo off maximum frequency is 3.4 GHz; The minimum frequency is 1.6 GHz.

Therefore we would expect: For turbo on, max percent = 100, min percent = 42; For turbo off, max percent = 100, min percent = 47.

starting from a fresh boot:

doug@s15:~$ uname -a
Linux s15 3.12.0-rc7+ #48 SMP Fri Nov 1 21:09:51 PDT 2013 x86_64 x86_64 x86_64 GNU/Linux
doug@s15:~$ cat /sys/devices/system/cpu/intel_pstate/*
100
42
0
doug@s15:~$ cd temp
doug@s15:~/temp$ sudo ./set_cpu_turbo_off
[sudo] password for doug:
doug@s15:~/temp$ cat /sys/devices/system/cpu/intel_pstate/*
100
42
1
doug@s15:~/temp$ cat set_cpu_turbo_off
#! /bin/bash
echo "1" > /sys/devices/system/cpu/intel_pstate/no_turbo

The attached png file demonstrates. (the rounding part will be a separate bug report)

Note: This bug is extracted from bug 59481 which mentioned several issues, the main issue was solved and the bug closed. Now I am following up on the other issues.
Comment 1 Dirk Brandewie 2013-11-04 16:35:24 UTC
min_pct_perf is a user configurable floor value (not to go below) so it is still valid in both cases.
Comment 2 Doug Smythies 2013-11-05 07:31:37 UTC
But the system overrides any user value that had been previously set upon mode change. It doesn't make any sense (to me at least) in turbo off that it be set to 42, which is basically a meaningless number under these conditions.

Example:
doug@s15:~/temp$ echo "53" | sudo tee /sys/devices/system/cpu/intel_pstate/min_perf_pct
53
doug@s15:~/temp$ cat /sys/devices/system/cpu/intel_pstate/*
53
53
1
doug@s15:~/temp$ sudo ./set_cpu_performance
doug@s15:~/temp$ cat /sys/devices/system/cpu/intel_pstate/*
100
100
0
doug@s15:~/temp$ sudo ./set_cpu_powersave
doug@s15:~/temp$ cat /sys/devices/system/cpu/intel_pstate/*
53
42
0
Comment 3 Dirk Brandewie 2013-11-05 07:56:55 UTC
I was trying to provide some useful information by setting min_perf_pct to a value.  I guess I could leave it at 0 since any value between 0 and the percent of the cpu represented by the lowest P state are the same. 

The reason for having the ability to disable turbo is for people that avoid the possible jitter introduced into the system caused by thermal throttling.

If you want to limit the P state selection to something to something greater than the minimum to further reduce jitter caused by P state transitions then you would set the min/floor value to something significantly higher than the minimums.

Regardless 42% is a valid floor value.
Comment 4 Doug Smythies 2013-11-07 05:28:49 UTC
(In reply to Dirk Brandewie from comment #3)
> I was trying to provide some useful information by setting min_perf_pct to a
> value.
Which I totally agree with, and do find the information very useful.
I am just suggesting that it would be equally useful to provide that same information for the other cases.

I see 4 scenarios, maybe there are more (using my computer as the example):

1.) BIOS allows Turbo mode and Turbo on enabled in driver:
min = 42 percent, which represents 1.6 GHz and is accurate
max = 100 percent, which represents 3.8 GHz and is accurate

2.) BIOS allows Turbo mode and Turbo is disabled in driver:
min = 42 percent, which doesn't represent anything.
max = 100 percent, which represents 3.4 GHz and is accurate.

3.) BIOS disabled Turbo mode and Turbo on enabled in driver:
min = 42 percent, which represents 1.6 GHz and is accurate.
max = 100 percent, which represents 3.8 GHz, but can not be achieved.
(a supporting graph will be attached in a minute)

4.) BIOS disabled Turbo mode and Turbo is disabled in driver:
This does not work at all on my computer. The CPU frequency just locks at whatever it was when the driver Turbo disable transition command is issued.
(a supporting graph will be attached in a minute)
Comment 5 Doug Smythies 2013-11-07 05:36:45 UTC
Created attachment 113721 [details]
BIOS disabled Turbo mode CPU freq Vs. Requested.

This graph is in support of the previous comments.

The "rnd" and "nokick" in the legend are related to other bug reports, and can be ignored in the context of this bug report. They are both with driver turbo on, but BIOS turbo disabled.

"Toff" means driver turbo disabled, and multiple tests were done, each one with a different frequency on CPU 7 at the time of driver transition from turbo enabled to disabled, to demonstrate that the CPU frequency will lock and be thereafter unchangeable.
Comment 6 Len Brown 2014-06-16 17:35:33 UTC
> 2.) BIOS allows Turbo mode and Turbo is disabled in driver:
> min = 42 percent, which doesn't represent anything.
> max = 100 percent, which represents 3.4 GHz and is accurate.

Documentation/ABI/testing/sysfs-devices-system-cpu
says this:

min_perf_pct: limits the minimum P state that will be requested by
the driver stated as a percentage of the available performance.

Documentation/cpu-freq/intel-pstate.txt
does not clarify.  Indeed, it contains identical text.

This is a bug because the code doesn't do what the Documentation
says it should do.  One or the other should be changed so
that they agree.

> 4.) BIOS disabled Turbo mode and Turbo is disabled in driver:
> This does not work at all on my computer. The CPU frequency
> just locks at > whatever it was when the driver Turbo disable
> transition command is issued.

this attributed is documented like so:

no_turbo: limits the driver to selecting P states below the turbo
frequency range.

As locking at the last frequency is different from 'selecting P-states
below the trubo frequency range', this is also a bug.  Presumably the
driver can check if the BIOS disabled turbo to avoid this failure.
Comment 7 Dirk Brandewie 2014-06-16 18:14:46 UTC
Does the the patch below make things sufficiently clear for min,max?
 

commit 049084f77fa3c58214972fd63050cbab9ac26352
Author: Dirk Brandewie <dirk.j.brandewie@intel.com>
Date:   Mon Jun 16 11:06:53 2014 -0700

    intel_pstate: Update documentation of {max,min}_perf_pct sysfs files
    
    
    Update documentation to make the interpretation of the values clearer
    
    Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 Documentation/cpu-freq/intel-pstate.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/cpu-freq/intel-pstate.txt b/Documentation/cpu-freq/intel-pstate.txt
index e742d21..a69ffe1 100644
--- a/Documentation/cpu-freq/intel-pstate.txt
+++ b/Documentation/cpu-freq/intel-pstate.txt
@@ -15,10 +15,13 @@ New sysfs files for controlling P state selection have been added to
 /sys/devices/system/cpu/intel_pstate/
 
       max_perf_pct: limits the maximum P state that will be requested by
-      the driver stated as a percentage of the available performance.
+      the driver stated as a percentage of the available performance. The
+      available (P states) performance may be reduced by the no_turbo
+      setting described below.
 
       min_perf_pct: limits the minimum P state that will be  requested by
-      the driver stated as a percentage of the available performance.
+      the driver stated as a percentage of the max (non-turbo)
+      performance level.
 
       no_turbo: limits the driver to selecting P states below the turbo
       frequency range.
Comment 8 Dirk Brandewie 2014-06-16 18:16:12 UTC
patch to fix no_turbo when turbo has been disabled in BIOS or the SKU does not support turbo.
commit 2765e48f8a1716d361c2b1a520841099a2b3942f
Author: Dirk Brandewie <dirk.j.brandewie@intel.com>
Date:   Mon Jun 16 07:46:53 2014 -0700

    intel_pstate: don't touch turbo bit if turbo disabled or unavailable.
    
    If turbo is disabled in the BIOS bit 38 should be set in
    MSR_IA32_MISC_ENABLE register per section 14.3.2.1 of the SDM Vol 3
    document 325384-050US Feb 2014.  If this bit is set do *not* attempt
    to disable trubo via the MSR_IA32_PERF_CTL register.  On some systems
    trying to disable turbo via MSR_IA32_PERF_CTL will cause subsequent
    writes to MSR_IA32_PERF_CTL not take affect, in fact reading
    MSR_IA32_PERF_CTL will not show the IDA/Turbo DISENGAGE bit(32) as
    set. A write of bit 32 to zero returns to normal operation.
    
    Also deal with the case where the processor does not support
    turbo and the BIOS does not report the fact in MSR_IA32_MISC_ENABLE
    but does report the max and turbo P states as the same value.
    
    Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/intel_pstate.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 4e7f492..0229953 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -128,6 +128,7 @@ static struct pstate_funcs pstate_funcs;
 
 struct perf_limits {
 	int no_turbo;
+	int turbo_disabled;
 	int max_perf_pct;
 	int min_perf_pct;
 	int32_t max_perf;
@@ -290,7 +291,10 @@ static ssize_t store_no_turbo(struct kobject *a, struct attribute *b,
 	if (ret != 1)
 		return -EINVAL;
 	limits.no_turbo = clamp_t(int, input, 0 , 1);
-
+	if (limits.turbo_disabled) {
+		pr_warn("Turbo disabled by BIOS or unavailable on processor\n");
+		limits.no_turbo = limits.turbo_disabled;
+	}
 	return count;
 }
 
@@ -384,7 +388,7 @@ static void byt_set_pstate(struct cpudata *cpudata, int pstate)
 	u32 vid;
 
 	val = pstate << 8;
-	if (limits.no_turbo)
+	if (limits.no_turbo && !limits.turbo_disabled)
 		val |= (u64)1 << 32;
 
 	vid_fp = cpudata->vid.min + mul_fp(
@@ -451,7 +455,7 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
 	u64 val;
 
 	val = pstate << 8;
-	if (limits.no_turbo)
+	if (limits.no_turbo && !limits.turbo_disabled)
 		val |= (u64)1 << 32;
 
 	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
@@ -744,7 +748,7 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
 		limits.min_perf = int_tofp(1);
 		limits.max_perf_pct = 100;
 		limits.max_perf = int_tofp(1);
-		limits.no_turbo = 0;
+		limits.no_turbo = limits.turbo_disabled;
 		return 0;
 	}
 	limits.min_perf_pct = (policy->min * 100) / policy->cpuinfo.max_freq;
@@ -787,6 +791,7 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 {
 	struct cpudata *cpu;
 	int rc;
+	u64 misc_en;
 
 	rc = intel_pstate_init_cpu(policy->cpu);
 	if (rc)
@@ -794,8 +799,13 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	cpu = all_cpu_data[policy->cpu];
 
-	if (!limits.no_turbo &&
-		limits.min_perf_pct == 100 && limits.max_perf_pct == 100)
+	rdmsrl(MSR_IA32_MISC_ENABLE, misc_en);
+	if (misc_en & MSR_IA32_MISC_ENABLE_TURBO_DISABLE ||
+		cpu->pstate.max_pstate == cpu->pstate.turbo_pstate) {
+		limits.turbo_disabled = 1;
+		limits.no_turbo = 1;
+	}
+	if (limits.min_perf_pct == 100 && limits.max_perf_pct == 100)
 		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
 	else
 		policy->policy = CPUFREQ_POLICY_POWERSAVE;
Comment 9 Doug Smythies 2014-06-17 03:45:32 UTC
I tried a version of the above listed patch back on this one:
https://bugzilla.kernel.org/show_bug.cgi?id=62851
It solved the "BIOS disabled Turbo mode and Turbo is disabled in driver" issue on my computer.
Comment 10 Len Brown 2014-11-03 21:34:52 UTC
dd5fbf70f96dbfd7ee432096a1f979b2b3267856
Author: Dirk Brandewie <dirk.j.brandewie@intel.com>  2014-06-20 10:27:59
Committer: Rafael J. Wysocki <rafael.j.wysocki@intel.com>  2014-07-06 19:22:19
Follows: v3.16-rc1
Precedes: v3.16-rc5

    intel_pstate: don't touch turbo bit if turbo disabled or unavailable.


closed.