[PATCH v3 0/6] Intel P states enhancements

[PATCH v3 0/6] Intel P states enhancements

[Srinivas Pandruvada]

This series enhances Intel P state drivers with the following features:
- When max_perf_pct is reduced in turbo range, we can change the turbo ratios
when platform allows. This is particularly useful in limiting performance with
HWP where whole range is turbo.
- Use Turbo Activation Ratio, when calculating max non turbo P state. This will
show now correct percentage in turbo range
- To calculate busy percent, the estimate is not correct when the max non turbo
is limited by tar, hence using physical max non turbo as before.
- Use ACPI _PSS and _PPC in intel_ptate driver.
- Avoid calculation for P state control value when cpufreq policy requests
frequency limits when matched in _PSS. Sometime calculations causes reduced
control value in boundary conditions.
Although they are independent patches, sending as series to help applying and
testing.
I appreciate review and testing on multiple platforms.

v3:
- Added function : convert_to_native_pstate_format to convert from
perf_ctl value to pstate used ratios.
- Fix bug identified by Rafael

v2:
- When CONFIG_ACPI not defined, then acpi/processor.h can't be included.
Also some variables will be unused when CONFIG_ACPI is not defined, so there
were warnings. Fixed all these compile issues.

v1:
- Change the command line option to "no_acpi"
- changed kernel parameter documentation file and added "no_acpi" parameter
- pr_debug prefixed with "intel_pstate" as suggested by Doug
- Changed the logic to determine turbo freq in _PSS using odd/even convention,
although this is the way it is defined in _PSS. But atleast two reviewers has
questioned the source of this. This is defined usually in non public documents
like BIOS writer guides. Now using the control field value to determine the
turbo and non turbo max.
- Fix the Kconfig dependency on ACPI for ACPI_PROCESSOR
- multi line comment styles

v0:
Base version

Srinivas Pandruvada (6):
  cpufreq: intel_p_state: Fix limiting turbo sub states
  cpufreq: intel_pstate: get P1 from TAR when available
  cpufreq: intel-pstate: Use separate max pstate for scaling
  cpufreq: intel_pstate: Use ACPI perf configuration
  Documentation: kernel_parameters for Intel P state driver
  cpufreq: intel_pstate: Avoid calculation for max/min

 Documentation/kernel-parameters.txt |   3 +
 arch/x86/include/asm/msr-index.h    |   7 +
 drivers/cpufreq/Kconfig.x86         |   1 +
 drivers/cpufreq/intel_pstate.c      | 368 ++++++++++++++++++++++++++++++++++--
 4 files changed, 368 insertions(+), 11 deletions(-)

-- 
1.9.3

[PATCH v3 1/6] cpufreq: intel_p_state: Fix limiting turbo sub states

[Srinivas Pandruvada]

Although the max_perf_pct reflects sub states in turbo range, we can't
really restrict to those states. This gives wrong impression that the
performance is reduced.
This can be achieved by restricting turbo ratio limits (MSR 0x1AD),
when bit 28 of platform info MSR allows (MSR 0xCE) is 1.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 92 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 91 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 3af9dd7..576d9e8 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -80,6 +80,7 @@ struct pstate_data {
 	int	max_pstate;
 	int	scaling;
 	int	turbo_pstate;
+	u64	turbo_ratio_limit;
 };
 
 struct vid_data {
@@ -132,6 +133,8 @@ struct pstate_funcs {
 	int (*get_scaling)(void);
 	void (*set)(struct cpudata*, int pstate);
 	void (*get_vid)(struct cpudata *);
+	u64 (*get_turbo_ratio_limit)(struct cpudata *);
+	int (*set_turbo_ratio_limit)(struct cpudata *, u64, u64);
 };
 
 struct cpu_defaults {
@@ -434,6 +437,23 @@ static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b,
 	limits.max_perf_pct = max(limits.min_perf_pct, limits.max_perf_pct);
 	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
 
+	if (pstate_funcs.set_turbo_ratio_limit) {
+		int max_perf_adj;
+		struct cpudata *cpu = all_cpu_data[0];
+
+		if (limits.max_sysfs_pct == 100)
+			max_perf_adj = cpu->pstate.turbo_ratio_limit;
+		else
+			max_perf_adj = fp_toint(mul_fp(int_tofp(
+					cpu->pstate.turbo_ratio_limit & 0xff),
+					limits.max_perf));
+
+		if (max_perf_adj > cpu->pstate.max_pstate)
+			pstate_funcs.set_turbo_ratio_limit(cpu,
+						cpu->pstate.turbo_ratio_limit,
+						max_perf_adj);
+	}
+
 	if (hwp_active)
 		intel_pstate_hwp_set();
 	return count;
@@ -628,6 +648,55 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
 	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
 }
 
+static u64 core_get_turbo_ratio_limit(struct cpudata *cpudata)
+{
+	u64 value;
+
+	rdmsrl(MSR_NHM_TURBO_RATIO_LIMIT, value);
+
+	return value;
+}
+
+static int core_set_turbo_ratio_limit(struct cpudata *cpudata, u64 def_ratio,
+			       u64 new_ratio)
+{
+	u64 value;
+
+	rdmsrl(MSR_PLATFORM_INFO, value);
+	if (value & BIT(28)) {
+		u64 ratio = 0;
+		u64 out_ratio = 0;
+		u8 max_ratio = new_ratio & 0xff;
+		int i;
+		/*
+		 * If caller provided reduced max ratio (one core active)
+		 * then use this for all other ratios, which are more
+		 * than the default ratio for those many cores active
+		 * for example if default ratio is 0x1a1b1c1d and new ratio
+		 * is 0x1b, then resultant ratio will be 0x1a1b1b1b
+		 */
+		for (i = 0; i < sizeof(def_ratio); ++i) {
+			if (def_ratio & 0xff) {
+				if (new_ratio & 0xff)
+					ratio = new_ratio & 0xff;
+				else {
+					if ((def_ratio & 0xff) > max_ratio)
+						ratio = max_ratio;
+					else
+						ratio = def_ratio & 0xff;
+				}
+				out_ratio |= (ratio << (i * 8));
+			}
+			def_ratio >>= 8;
+			new_ratio >>= 8;
+		}
+		wrmsrl(MSR_NHM_TURBO_RATIO_LIMIT, out_ratio);
+		return 0;
+	}
+
+	return -EPERM;
+}
+
 static int knl_get_turbo_pstate(void)
 {
 	u64 value;
@@ -656,6 +725,8 @@ static struct cpu_defaults core_params = {
 		.get_turbo = core_get_turbo_pstate,
 		.get_scaling = core_get_scaling,
 		.set = core_set_pstate,
+		.get_turbo_ratio_limit = core_get_turbo_ratio_limit,
+		.set_turbo_ratio_limit = core_set_turbo_ratio_limit,
 	},
 };
 
@@ -745,7 +816,10 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
 	cpu->pstate.max_pstate = pstate_funcs.get_max();
 	cpu->pstate.turbo_pstate = pstate_funcs.get_turbo();
 	cpu->pstate.scaling = pstate_funcs.get_scaling();
-
+	if (pstate_funcs.get_turbo_ratio_limit &&
+	    !cpu->pstate.turbo_ratio_limit)
+		cpu->pstate.turbo_ratio_limit =
+			pstate_funcs.get_turbo_ratio_limit(cpu);
 	if (pstate_funcs.get_vid)
 		pstate_funcs.get_vid(cpu);
 	intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate, false);
@@ -949,6 +1023,20 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
 		intel_pstate_hwp_enable(cpu);
 
 	intel_pstate_get_cpu_pstates(cpu);
+	/* readjust turbo limit ratio after resume or hotplug */
+	if (limits.max_sysfs_pct != 100 &&
+	    pstate_funcs.set_turbo_ratio_limit) {
+		int max_perf_adj;
+
+		max_perf_adj = fp_toint(mul_fp(int_tofp(
+					cpu->pstate.turbo_ratio_limit & 0xff),
+					limits.max_perf));
+
+		if (max_perf_adj > cpu->pstate.max_pstate)
+			pstate_funcs.set_turbo_ratio_limit(cpu,
+						cpu->pstate.turbo_ratio_limit,
+						max_perf_adj);
+	}
 
 	init_timer_deferrable(&cpu->timer);
 	cpu->timer.data = (unsigned long)cpu;
@@ -1118,6 +1206,8 @@ static void copy_cpu_funcs(struct pstate_funcs *funcs)
 	pstate_funcs.get_scaling = funcs->get_scaling;
 	pstate_funcs.set       = funcs->set;
 	pstate_funcs.get_vid   = funcs->get_vid;
+	pstate_funcs.set_turbo_ratio_limit = funcs->set_turbo_ratio_limit;
+	pstate_funcs.get_turbo_ratio_limit = funcs->get_turbo_ratio_limit;
 }
 
 #if IS_ENABLED(CONFIG_ACPI)
-- 
1.9.3

[PATCH v3 2/6] cpufreq: intel_pstate: get P1 from TAR when available

[Srinivas Pandruvada]

After Ivybridge, the max non turbo ratio obtained from platform info msr
is not always guaranteed P1 on client platforms. The max non turbo
activation ratio (TAR), determines the max for the current level of TDP.
The ratio in platform info is physical max. The TAR MSR can be locked,
so updating this value is not possible on all platforms.
This change gets this ratio from MSR TURBO_ACTIVATION_RATIO if
available,
but also do some sanity checking to make sure that this value is
correct.
The sanity check involves reading the TDP ratio for the current tdp
control value when platform has configurable TDP present and matching
TAC
with this.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 arch/x86/include/asm/msr-index.h |  7 +++++++
 drivers/cpufreq/intel_pstate.c   | 39 +++++++++++++++++++++++++++++++++++----
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index b8c14bb..9f39056 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -206,6 +206,13 @@
 #define MSR_GFX_PERF_LIMIT_REASONS	0x000006B0
 #define MSR_RING_PERF_LIMIT_REASONS	0x000006B1
 
+/* Config TDP MSRs */
+#define MSR_CONFIG_TDP_NOMINAL		0x00000648
+#define MSR_CONFIG_TDP_LEVEL1		0x00000649
+#define MSR_CONFIG_TDP_LEVEL2		0x0000064A
+#define MSR_CONFIG_TDP_CONTROL		0x0000064B
+#define MSR_TURBO_ACTIVATION_RATIO	0x0000064C
+
 /* Hardware P state interface */
 #define MSR_PPERF			0x0000064e
 #define MSR_PERF_LIMIT_REASONS		0x0000064f
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 576d9e8..b0ae951 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -43,7 +43,6 @@
 #define int_tofp(X) ((int64_t)(X) << FRAC_BITS)
 #define fp_toint(X) ((X) >> FRAC_BITS)
 
-
 static inline int32_t mul_fp(int32_t x, int32_t y)
 {
 	return ((int64_t)x * (int64_t)y) >> FRAC_BITS;
@@ -613,10 +612,42 @@ static int core_get_min_pstate(void)
 
 static int core_get_max_pstate(void)
 {
-	u64 value;
+	u64 tar;
+	u64 plat_info;
+	int max_pstate;
+	int err;
+
+	rdmsrl(MSR_PLATFORM_INFO, plat_info);
+	max_pstate = (plat_info >> 8) & 0xFF;
+
+	err = rdmsrl_safe(MSR_TURBO_ACTIVATION_RATIO, &tar);
+	if (!err) {
+		/* Do some sanity checking for safety */
+		if (plat_info & 0x600000000) {
+			u64 tdp_ctrl;
+			u64 tdp_ratio;
+			int tdp_msr;
+
+			err = rdmsrl_safe(MSR_CONFIG_TDP_CONTROL, &tdp_ctrl);
+			if (err)
+				goto skip_tar;
+
+			tdp_msr = MSR_CONFIG_TDP_NOMINAL + tdp_ctrl;
+			err = rdmsrl_safe(tdp_msr, &tdp_ratio);
+			if (err)
+				goto skip_tar;
+
+			if (tdp_ratio - 1 == tar) {
+				max_pstate = tar;
+				pr_debug("max_pstate=TAC %x\n", max_pstate);
+			} else {
+				goto skip_tar;
+			}
+		}
+	}
 
-	rdmsrl(MSR_PLATFORM_INFO, value);
-	return (value >> 8) & 0xFF;
+skip_tar:
+	return max_pstate;
 }
 
 static int core_get_turbo_pstate(void)
-- 
1.9.3

[PATCH v3 3/6] cpufreq: intel-pstate: Use separate max pstate for scaling

[Srinivas Pandruvada]

Systems with configurable TDP have multiple max non turbo p state. Intel
P state uses max non turbo P state for scaling. But using the real max
non turbo p state causes underestimation of next P state. So using
the physical max non turbo P state as before for scaling.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index b0ae951..0ae9618 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -77,6 +77,7 @@ struct pstate_data {
 	int	current_pstate;
 	int	min_pstate;
 	int	max_pstate;
+	int	max_pstate_physical;
 	int	scaling;
 	int	turbo_pstate;
 	u64	turbo_ratio_limit;
@@ -127,6 +128,7 @@ struct pstate_adjust_policy {
 
 struct pstate_funcs {
 	int (*get_max)(void);
+	int (*get_max_physical)(void);
 	int (*get_min)(void);
 	int (*get_turbo)(void);
 	int (*get_scaling)(void);
@@ -610,6 +612,14 @@ static int core_get_min_pstate(void)
 	return (value >> 40) & 0xFF;
 }
 
+static int core_get_max_pstate_physical(void)
+{
+	u64 value;
+
+	rdmsrl(MSR_PLATFORM_INFO, value);
+	return (value >> 8) & 0xFF;
+}
+
 static int core_get_max_pstate(void)
 {
 	u64 tar;
@@ -752,6 +762,7 @@ static struct cpu_defaults core_params = {
 	},
 	.funcs = {
 		.get_max = core_get_max_pstate,
+		.get_max_physical = core_get_max_pstate_physical,
 		.get_min = core_get_min_pstate,
 		.get_turbo = core_get_turbo_pstate,
 		.get_scaling = core_get_scaling,
@@ -772,6 +783,7 @@ static struct cpu_defaults byt_params = {
 	},
 	.funcs = {
 		.get_max = byt_get_max_pstate,
+		.get_max_physical = byt_get_max_pstate,
 		.get_min = byt_get_min_pstate,
 		.get_turbo = byt_get_turbo_pstate,
 		.set = byt_set_pstate,
@@ -791,6 +803,7 @@ static struct cpu_defaults knl_params = {
 	},
 	.funcs = {
 		.get_max = core_get_max_pstate,
+		.get_max_physical = core_get_max_pstate_physical,
 		.get_min = core_get_min_pstate,
 		.get_turbo = knl_get_turbo_pstate,
 		.get_scaling = core_get_scaling,
@@ -845,6 +858,7 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
 {
 	cpu->pstate.min_pstate = pstate_funcs.get_min();
 	cpu->pstate.max_pstate = pstate_funcs.get_max();
+	cpu->pstate.max_pstate_physical = pstate_funcs.get_max_physical();
 	cpu->pstate.turbo_pstate = pstate_funcs.get_turbo();
 	cpu->pstate.scaling = pstate_funcs.get_scaling();
 	if (pstate_funcs.get_turbo_ratio_limit &&
@@ -866,7 +880,8 @@ static inline void intel_pstate_calc_busy(struct cpudata *cpu)
 
 	sample->freq = fp_toint(
 		mul_fp(int_tofp(
-			cpu->pstate.max_pstate * cpu->pstate.scaling / 100),
+			cpu->pstate.max_pstate_physical *
+			cpu->pstate.scaling / 100),
 			core_pct));
 
 	sample->core_pct_busy = (int32_t)core_pct;
@@ -934,7 +949,7 @@ static inline int32_t intel_pstate_get_scaled_busy(struct cpudata *cpu)
 	 * specified pstate.
 	 */
 	core_busy = cpu->sample.core_pct_busy;
-	max_pstate = int_tofp(cpu->pstate.max_pstate);
+	max_pstate = int_tofp(cpu->pstate.max_pstate_physical);
 	current_pstate = int_tofp(cpu->pstate.current_pstate);
 	core_busy = mul_fp(core_busy, div_fp(max_pstate, current_pstate));
 
@@ -1232,6 +1247,7 @@ static void copy_pid_params(struct pstate_adjust_policy *policy)
 static void copy_cpu_funcs(struct pstate_funcs *funcs)
 {
 	pstate_funcs.get_max   = funcs->get_max;
+	pstate_funcs.get_max_physical = funcs->get_max_physical;
 	pstate_funcs.get_min   = funcs->get_min;
 	pstate_funcs.get_turbo = funcs->get_turbo;
 	pstate_funcs.get_scaling = funcs->get_scaling;
-- 
1.9.3

[PATCH v3 4/6] cpufreq: intel_pstate: Use ACPI perf configuration

[Srinivas Pandruvada]

Use ACPI _PSS to limit the Intel P State turbo, max and min ratios.
This driver uses acpi processor perf lib calls to register performance.
The following logic is used to adjust Intel P state driver limits:
- If there is no turbo entry in _PSS, then disable Intel P state turbo
and limit to non turbo max
- If the non turbo max ratio is more than _PSS max non turbo value, then
set the max non turbo ratio to _PSS non turbo max
- If the min ratio is less than _PSS min then change the min ratio
matching _PSS min
- Scale the _PSS turbo frequency to max turbo frequency based on control
value.
This feature can be disabled by using kernel parameters:
intel_pstate=no_acpi

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/Kconfig.x86    |   1 +
 drivers/cpufreq/intel_pstate.c | 171 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 171 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index c59bdcb..adbd1de 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -5,6 +5,7 @@
 config X86_INTEL_PSTATE
        bool "Intel P state control"
        depends on X86
+       select ACPI_PROCESSOR if ACPI
        help
           This driver provides a P state for Intel core processors.
 	  The driver implements an internal governor and will become
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 0ae9618..869f074 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -34,6 +34,10 @@
 #include <asm/cpu_device_id.h>
 #include <asm/cpufeature.h>
 
+#if IS_ENABLED(CONFIG_ACPI)
+#include <acpi/processor.h>
+#endif
+
 #define BYT_RATIOS		0x66a
 #define BYT_VIDS		0x66b
 #define BYT_TURBO_RATIOS	0x66c
@@ -114,6 +118,9 @@ struct cpudata {
 	u64	prev_mperf;
 	u64	prev_tsc;
 	struct sample sample;
+#if IS_ENABLED(CONFIG_ACPI)
+	struct acpi_processor_performance acpi_perf_data;
+#endif
 };
 
 static struct cpudata **all_cpu_data;
@@ -146,6 +153,7 @@ struct cpu_defaults {
 static struct pstate_adjust_policy pid_params;
 static struct pstate_funcs pstate_funcs;
 static int hwp_active;
+static int no_acpi_perf;
 
 struct perf_limits {
 	int no_turbo;
@@ -173,6 +181,153 @@ static struct perf_limits limits = {
 	.min_sysfs_pct = 0,
 };
 
+#if IS_ENABLED(CONFIG_ACPI)
+/*
+ * The max target pstate ratio is a 8 bit value in both PLATFORM_INFO MSR and
+ * in TURBO_RATIO_LIMIT MSR, which pstate driver stores in max_pstate and
+ * max_turbo_pstate fields. The PERF_CTL MSR contains 16 bit value for P state
+ * ratio, out of it only high 8 bits are used. For example 0x1700 is setting
+ * target ratio 0x17. The _PSS control value stores in a format which can be
+ * directly written to PERF_CTL MSR. But in intel_pstate driver this shift
+ * occurs during write to PERF_CTL (E.g. for cores core_set_pstate()).
+ * This function converts the _PSS control value to intel pstate driver format
+ * for comparison and assignment.
+ */
+static int convert_to_native_pstate_format(struct cpudata *cpu, int index)
+{
+	return cpu->acpi_perf_data.states[index].control >> 8;
+}
+
+static int intel_pstate_init_perf_limits(struct cpufreq_policy *policy)
+{
+	struct cpudata *cpu;
+	int ret;
+	bool turbo_absent = false;
+	int max_pstate_index;
+	int min_pss_ctl, max_pss_ctl, turbo_pss_ctl;
+	int i;
+
+	cpu = all_cpu_data[policy->cpu];
+
+	pr_debug("intel_pstate: default limits 0x%x 0x%x 0x%x\n",
+		 cpu->pstate.min_pstate, cpu->pstate.max_pstate,
+		 cpu->pstate.turbo_pstate);
+
+	if (!cpu->acpi_perf_data.shared_cpu_map &&
+	    zalloc_cpumask_var_node(&cpu->acpi_perf_data.shared_cpu_map,
+				    GFP_KERNEL, cpu_to_node(policy->cpu))) {
+		return -ENOMEM;
+	}
+
+	ret = acpi_processor_register_performance(&cpu->acpi_perf_data,
+						  policy->cpu);
+	if (ret)
+		return ret;
+
+	/*
+	 * Check if the control value in _PSS is for PERF_CTL MSR, which should
+	 * guarantee that the states returned by it map to the states in our
+	 * list directly.
+	 */
+	if (cpu->acpi_perf_data.control_register.space_id !=
+						ACPI_ADR_SPACE_FIXED_HARDWARE)
+		return -EIO;
+
+	pr_debug("intel_pstate: CPU%u - ACPI _PSS perf data\n", policy->cpu);
+	for (i = 0; i < cpu->acpi_perf_data.state_count; i++)
+		pr_debug("     %cP%d: %u MHz, %u mW, 0x%x\n",
+			 (i == cpu->acpi_perf_data.state ? '*' : ' '), i,
+			 (u32) cpu->acpi_perf_data.states[i].core_frequency,
+			 (u32) cpu->acpi_perf_data.states[i].power,
+			 (u32) cpu->acpi_perf_data.states[i].control);
+
+	/*
+	 * If there is only one entry _PSS, simply ignore _PSS and continue as
+	 * usual without taking _PSS into account
+	 */
+	if (cpu->acpi_perf_data.state_count < 2)
+		return 0;
+
+	turbo_pss_ctl = convert_to_native_pstate_format(cpu, 0);
+	min_pss_ctl = convert_to_native_pstate_format(cpu,
+					cpu->acpi_perf_data.state_count - 1);
+	/* Check if there is a turbo freq in _PSS */
+	if (turbo_pss_ctl <= cpu->pstate.max_pstate &&
+	    turbo_pss_ctl > cpu->pstate.min_pstate) {
+		pr_debug("intel_pstate: no turbo range exists in _PSS\n");
+		limits.no_turbo = limits.turbo_disabled = 1;
+		cpu->pstate.turbo_pstate = cpu->pstate.max_pstate;
+		turbo_absent = true;
+	}
+
+	/* Check if the max non turbo p state < Intel P state max */
+	max_pstate_index = turbo_absent ? 0 : 1;
+	max_pss_ctl = convert_to_native_pstate_format(cpu, max_pstate_index);
+	if (max_pss_ctl < cpu->pstate.max_pstate &&
+	    max_pss_ctl > cpu->pstate.min_pstate)
+		cpu->pstate.max_pstate = max_pss_ctl;
+
+	/* check If min perf > Intel P State min */
+	if (min_pss_ctl > cpu->pstate.min_pstate &&
+	    min_pss_ctl < cpu->pstate.max_pstate) {
+		cpu->pstate.min_pstate = min_pss_ctl;
+		policy->cpuinfo.min_freq = min_pss_ctl * cpu->pstate.scaling;
+	}
+
+	if (turbo_absent)
+		policy->cpuinfo.max_freq = cpu->pstate.max_pstate *
+						cpu->pstate.scaling;
+	else {
+		policy->cpuinfo.max_freq = cpu->pstate.turbo_pstate *
+						cpu->pstate.scaling;
+		/*
+		 * The _PSS table doesn't contain whole turbo frequency range.
+		 * This just contains +1 MHZ above the max non turbo frequency,
+		 * with control value corresponding to max turbo ratio. But
+		 * when cpufreq set policy is called, it will call with this
+		 * max frequency, which will cause a reduced performance as
+		 * this driver uses real max turbo frequency as the max
+		 * frequeny. So correct this frequency in _PSS table to
+		 * correct max turbo frequency based on the turbo ratio.
+		 * Also need to convert to MHz as _PSS freq is in MHz.
+		 */
+		cpu->acpi_perf_data.states[0].core_frequency =
+						turbo_pss_ctl * 100;
+	}
+
+	pr_debug("intel_pstate: Updated limits using _PSS 0x%x 0x%x 0x%x\n",
+		 cpu->pstate.min_pstate, cpu->pstate.max_pstate,
+		 cpu->pstate.turbo_pstate);
+	pr_debug("intel_pstate: policy max_freq=%d Khz min_freq = %d KHz\n",
+		 policy->cpuinfo.max_freq, policy->cpuinfo.min_freq);
+
+	return 0;
+}
+
+static int intel_pstate_exit_perf_limits(struct cpufreq_policy *policy)
+{
+	struct cpudata *cpu;
+
+	if (!no_acpi_perf)
+		return 0;
+
+	cpu = all_cpu_data[policy->cpu];
+	acpi_processor_unregister_performance(policy->cpu);
+	return 0;
+}
+
+#else
+static int intel_pstate_init_perf_limits(struct cpufreq_policy *policy)
+{
+	return 0;
+}
+
+static int intel_pstate_exit_perf_limits(struct cpufreq_policy *policy)
+{
+	return 0;
+}
+#endif
+
 static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
 			     int deadband, int integral) {
 	pid->setpoint = setpoint;
@@ -1203,18 +1358,30 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
 	policy->cpuinfo.min_freq = cpu->pstate.min_pstate * cpu->pstate.scaling;
 	policy->cpuinfo.max_freq =
 		cpu->pstate.turbo_pstate * cpu->pstate.scaling;
+	if (!no_acpi_perf)
+		intel_pstate_init_perf_limits(policy);
+	/*
+	 * If there is no acpi perf data or error, we ignore and use Intel P
+	 * state calculated limits, So this is not fatal error.
+	 */
 	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
 	cpumask_set_cpu(policy->cpu, policy->cpus);
 
 	return 0;
 }
 
+static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
+{
+	return intel_pstate_exit_perf_limits(policy);
+}
+
 static struct cpufreq_driver intel_pstate_driver = {
 	.flags		= CPUFREQ_CONST_LOOPS,
 	.verify		= intel_pstate_verify_policy,
 	.setpolicy	= intel_pstate_set_policy,
 	.get		= intel_pstate_get,
 	.init		= intel_pstate_cpu_init,
+	.exit		= intel_pstate_cpu_exit,
 	.stop_cpu	= intel_pstate_stop_cpu,
 	.name		= "intel_pstate",
 };
@@ -1258,7 +1425,6 @@ static void copy_cpu_funcs(struct pstate_funcs *funcs)
 }
 
 #if IS_ENABLED(CONFIG_ACPI)
-#include <acpi/processor.h>
 
 static bool intel_pstate_no_acpi_pss(void)
 {
@@ -1450,6 +1616,9 @@ static int __init intel_pstate_setup(char *str)
 		force_load = 1;
 	if (!strcmp(str, "hwp_only"))
 		hwp_only = 1;
+	if (!strcmp(str, "no_acpi"))
+		no_acpi_perf = 1;
+
 	return 0;
 }
 early_param("intel_pstate", intel_pstate_setup);
-- 
1.9.3

[PATCH v3 5/6] Documentation: kernel_parameters for Intel P state driver

[Srinivas Pandruvada]

Added new option "no_acpi" for not using ACPI processor performance
control objects in Intel P state driver.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 Documentation/kernel-parameters.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 22a4b68..9b75e2a 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1546,6 +1546,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 		hwp_only
 			Only load intel_pstate on systems which support
 			hardware P state control (HWP) if available.
+		no_acpi
+			Don't use ACPI processor performance control objects
+			_PSS and _PPC specified limits.
 
 	intremap=	[X86-64, Intel-IOMMU]
 			on	enable Interrupt Remapping (default)
-- 
1.9.3

[PATCH v3 6/6] cpufreq: intel_pstate: Avoid calculation for max/min

[Srinivas Pandruvada]

When requested from cpufreq to set policy, look into _pss and get
control values, instead of using max/min perf calculations. These
calculation misses next control state in boundary conditions.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/cpufreq/intel_pstate.c | 50 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 869f074..49a16ff 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -166,6 +166,8 @@ struct perf_limits {
 	int max_sysfs_pct;
 	int min_policy_pct;
 	int min_sysfs_pct;
+	int max_perf_ctl;
+	int min_perf_ctl;
 };
 
 static struct perf_limits limits = {
@@ -179,6 +181,8 @@ static struct perf_limits limits = {
 	.max_sysfs_pct = 100,
 	.min_policy_pct = 0,
 	.min_sysfs_pct = 0,
+	.max_perf_ctl = 0,
+	.min_perf_ctl = 0,
 };
 
 #if IS_ENABLED(CONFIG_ACPI)
@@ -980,12 +984,23 @@ static void intel_pstate_get_min_max(struct cpudata *cpu, int *min, int *max)
 	 * policy, or by cpu specific default values determined through
 	 * experimentation.
 	 */
-	max_perf_adj = fp_toint(mul_fp(int_tofp(max_perf), limits.max_perf));
-	*max = clamp_t(int, max_perf_adj,
-			cpu->pstate.min_pstate, cpu->pstate.turbo_pstate);
+	if (limits.max_perf_ctl && limits.max_sysfs_pct >=
+						limits.max_policy_pct) {
+		*max = limits.max_perf_ctl;
+	} else {
+		max_perf_adj = fp_toint(mul_fp(int_tofp(max_perf),
+					limits.max_perf));
+		*max = clamp_t(int, max_perf_adj, cpu->pstate.min_pstate,
+			       cpu->pstate.turbo_pstate);
+	}
 
-	min_perf = fp_toint(mul_fp(int_tofp(max_perf), limits.min_perf));
-	*min = clamp_t(int, min_perf, cpu->pstate.min_pstate, max_perf);
+	if (limits.min_perf_ctl) {
+		*min = limits.min_perf_ctl;
+	} else {
+		min_perf = fp_toint(mul_fp(int_tofp(max_perf),
+				    limits.min_perf));
+		*min = clamp_t(int, min_perf, cpu->pstate.min_pstate, max_perf);
+	}
 }
 
 static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate, bool force)
@@ -1272,6 +1287,12 @@ static unsigned int intel_pstate_get(unsigned int cpu_num)
 
 static int intel_pstate_set_policy(struct cpufreq_policy *policy)
 {
+#if IS_ENABLED(CONFIG_ACPI)
+	struct cpudata *cpu;
+	int i;
+#endif
+	pr_debug("intel_pstate: %s max %u policy->max %u\n", __func__,
+		 policy->cpuinfo.max_freq, policy->max);
 	if (!policy->cpuinfo.max_freq)
 		return -ENODEV;
 
@@ -1284,6 +1305,8 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
 		limits.max_perf_pct = 100;
 		limits.max_perf = int_tofp(1);
 		limits.no_turbo = 0;
+		limits.max_perf_ctl = 0;
+		limits.min_perf_ctl = 0;
 		return 0;
 	}
 
@@ -1304,6 +1327,23 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
 	limits.min_perf = div_fp(int_tofp(limits.min_perf_pct), int_tofp(100));
 	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
 
+#if IS_ENABLED(CONFIG_ACPI)
+	cpu = all_cpu_data[policy->cpu];
+	for (i = 0; i < cpu->acpi_perf_data.state_count; i++) {
+		int control;
+
+		control = convert_to_native_pstate_format(cpu, i);
+		if (control * cpu->pstate.scaling == policy->max)
+			limits.max_perf_ctl = control;
+		if (control * cpu->pstate.scaling == policy->min)
+			limits.min_perf_ctl = control;
+	}
+
+	pr_debug("intel_pstate: max %u policy_max %u perf_ctl [0x%x-0x%x]\n",
+		 policy->cpuinfo.max_freq, policy->max, limits.min_perf_ctl,
+		 limits.max_perf_ctl);
+#endif
+
 	if (hwp_active)
 		intel_pstate_hwp_set();
 
-- 
1.9.3

Re: [PATCH v3 1/6] cpufreq: intel_p_state: Fix limiting turbo sub states

[Rafael J. Wysocki]

On Tuesday, September 29, 2015 03:54:03 PM Srinivas Pandruvada wrote:
> Although the max_perf_pct reflects sub states in turbo range, we can't
> really restrict to those states. This gives wrong impression that the
> performance is reduced.
> This can be achieved by restricting turbo ratio limits (MSR 0x1AD),
> when bit 28 of platform info MSR allows (MSR 0xCE) is 1.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/cpufreq/intel_pstate.c | 92 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 91 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 3af9dd7..576d9e8 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -80,6 +80,7 @@ struct pstate_data {
>  	int	max_pstate;
>  	int	scaling;
>  	int	turbo_pstate;
> +	u64	turbo_ratio_limit;

Why does it have to be u64?

>  };
>  
>  struct vid_data {
> @@ -132,6 +133,8 @@ struct pstate_funcs {
>  	int (*get_scaling)(void);
>  	void (*set)(struct cpudata*, int pstate);
>  	void (*get_vid)(struct cpudata *);
> +	u64 (*get_turbo_ratio_limit)(struct cpudata *);
> +	int (*set_turbo_ratio_limit)(struct cpudata *, u64, u64);

An int is always passed as the last arg to this, so why is the arg u64?

>  };
>  
>  struct cpu_defaults {
> @@ -434,6 +437,23 @@ static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b,
>  	limits.max_perf_pct = max(limits.min_perf_pct, limits.max_perf_pct);
>  	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
>  
> +	if (pstate_funcs.set_turbo_ratio_limit) {
> +		int max_perf_adj;
> +		struct cpudata *cpu = all_cpu_data[0];
> +
> +		if (limits.max_sysfs_pct == 100)
> +			max_perf_adj = cpu->pstate.turbo_ratio_limit;

This will cast the u64 to int anyway.

Also this is going to be the value read from the register at init which is
likely to be greater than 255 ->

> +		else
> +			max_perf_adj = fp_toint(mul_fp(int_tofp(
> +					cpu->pstate.turbo_ratio_limit & 0xff),
> +					limits.max_perf));

-> but this will never be greater than 255 if I'm not mistaken (limits.max_perf
is a representation of a fraction between 0 and 1 and the first value is
bounded by 255).

How are these two values related to each other?

> +
> +		if (max_perf_adj > cpu->pstate.max_pstate)
> +			pstate_funcs.set_turbo_ratio_limit(cpu,
> +						cpu->pstate.turbo_ratio_limit,
> +						max_perf_adj);

I'm not really sure what this is supposed to achieve.  Care to explain a bit?

[BTW, the first arg of core_set/get_turbo_ratio_limit(() is never used, so why
bother with passing it at all?]

> +	}
> +
>  	if (hwp_active)
>  		intel_pstate_hwp_set();
>  	return count;
> @@ -628,6 +648,55 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
>  	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
>  }
>  
> +static u64 core_get_turbo_ratio_limit(struct cpudata *cpudata)
> +{
> +	u64 value;
> +
> +	rdmsrl(MSR_NHM_TURBO_RATIO_LIMIT, value);
> +
> +	return value;
> +}
> +
> +static int core_set_turbo_ratio_limit(struct cpudata *cpudata, u64 def_ratio,
> +			       u64 new_ratio)
> +{
> +	u64 value;
> +

What should happen if def_ratio and new_ratio are the same?

> +	rdmsrl(MSR_PLATFORM_INFO, value);
> +	if (value & BIT(28)) {
> +		u64 ratio = 0;
> +		u64 out_ratio = 0;
> +		u8 max_ratio = new_ratio & 0xff;

Why u8?

> +		int i;
> +		/*
> +		 * If caller provided reduced max ratio (one core active)
> +		 * then use this for all other ratios, which are more
> +		 * than the default ratio for those many cores active
> +		 * for example if default ratio is 0x1a1b1c1d and new ratio
> +		 * is 0x1b, then resultant ratio will be 0x1a1b1b1b
> +		 */

This is a bit messy IMO.  Instead of shifting def_ratio and new_ratio in each
step, I'd shift max_ratio and the mask:

	u64 mask = 0xff;
	u64 max_ratio = new_ratio & mask;

	while (mask) {
		if (def_ratio & mask) {
			u64 ratio;

			if (new_ratio & mask) {
				ratio = new_ratio & mask;
			} else {
				ratio = def_ratio & mask;
				if (ratio > max_ratio)
					ratio = max_ratio;
			}
			out_ratio |= ratio;
		}
		max_ratio <<= 8;
		mask <<= 8;
	}

[I'm not sure why the least significant byte of new_ratio is special, though.]

> +		for (i = 0; i < sizeof(def_ratio); ++i) {
> +			if (def_ratio & 0xff) {
> +				if (new_ratio & 0xff)
> +					ratio = new_ratio & 0xff;
> +				else {
> +					if ((def_ratio & 0xff) > max_ratio)
> +						ratio = max_ratio;
> +					else
> +						ratio = def_ratio & 0xff;
> +				}
> +				out_ratio |= (ratio << (i * 8));
> +			}
> +			def_ratio >>= 8;
> +			new_ratio >>= 8;
> +		}
> +		wrmsrl(MSR_NHM_TURBO_RATIO_LIMIT, out_ratio);
> +		return 0;
> +	}
> +
> +	return -EPERM;

Why -EPERM?

That's not because the user has no permission to carry out the opreration, but
because there is no capability, right?

> +}
> +
>  static int knl_get_turbo_pstate(void)
>  {
>  	u64 value;
> @@ -656,6 +725,8 @@ static struct cpu_defaults core_params = {
>  		.get_turbo = core_get_turbo_pstate,
>  		.get_scaling = core_get_scaling,
>  		.set = core_set_pstate,
> +		.get_turbo_ratio_limit = core_get_turbo_ratio_limit,
> +		.set_turbo_ratio_limit = core_set_turbo_ratio_limit,
>  	},
>  };
>  
> @@ -745,7 +816,10 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
>  	cpu->pstate.max_pstate = pstate_funcs.get_max();
>  	cpu->pstate.turbo_pstate = pstate_funcs.get_turbo();
>  	cpu->pstate.scaling = pstate_funcs.get_scaling();
> -
> +	if (pstate_funcs.get_turbo_ratio_limit &&
> +	    !cpu->pstate.turbo_ratio_limit)
> +		cpu->pstate.turbo_ratio_limit =
> +			pstate_funcs.get_turbo_ratio_limit(cpu);

So we read MSR_NHM_TURBO_RATIO_LIMIT and store it into pstate.turbo_ratio_limit
unless alread populated.  This means we do it only once during initialization.

What if the value we have in there is stale after, say, system suspend-resume?

Don't we need to re-adjust then?

>  	if (pstate_funcs.get_vid)
>  		pstate_funcs.get_vid(cpu);
>  	intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate, false);
> @@ -949,6 +1023,20 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
>  		intel_pstate_hwp_enable(cpu);
>  
>  	intel_pstate_get_cpu_pstates(cpu);
> +	/* readjust turbo limit ratio after resume or hotplug */
> +	if (limits.max_sysfs_pct != 100 &&
> +	    pstate_funcs.set_turbo_ratio_limit) {
> +		int max_perf_adj;
> +
> +		max_perf_adj = fp_toint(mul_fp(int_tofp(
> +					cpu->pstate.turbo_ratio_limit & 0xff),
> +					limits.max_perf));
> +
> +		if (max_perf_adj > cpu->pstate.max_pstate)
> +			pstate_funcs.set_turbo_ratio_limit(cpu,
> +						cpu->pstate.turbo_ratio_limit,
> +						max_perf_adj);
> +	}
>  
>  	init_timer_deferrable(&cpu->timer);
>  	cpu->timer.data = (unsigned long)cpu;
> @@ -1118,6 +1206,8 @@ static void copy_cpu_funcs(struct pstate_funcs *funcs)
>  	pstate_funcs.get_scaling = funcs->get_scaling;
>  	pstate_funcs.set       = funcs->set;
>  	pstate_funcs.get_vid   = funcs->get_vid;
> +	pstate_funcs.set_turbo_ratio_limit = funcs->set_turbo_ratio_limit;
> +	pstate_funcs.get_turbo_ratio_limit = funcs->get_turbo_ratio_limit;

These are defined for Core only, right?  Might be good to write about that in
the changelog.

>  }
>  
>  #if IS_ENABLED(CONFIG_ACPI)
> 

Thanks,
Rafael

Re: [PATCH v3 1/6] cpufreq: intel_p_state: Fix limiting turbo sub states

[Srinivas Pandruvada]

On Tue, 2015-10-06 at 00:56 +0200, Rafael J. Wysocki wrote:
> On Tuesday, September 29, 2015 03:54:03 PM Srinivas Pandruvada wrote:
> > Although the max_perf_pct reflects sub states in turbo range, we can't
> > really restrict to those states. This gives wrong impression that the
> > performance is reduced.
> > This can be achieved by restricting turbo ratio limits (MSR 0x1AD),
> > when bit 28 of platform info MSR allows (MSR 0xCE) is 1.
> > 
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > ---
> >  drivers/cpufreq/intel_pstate.c | 92 +++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 91 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index 3af9dd7..576d9e8 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -80,6 +80,7 @@ struct pstate_data {
> >  	int	max_pstate;
> >  	int	scaling;
> >  	int	turbo_pstate;
> > +	u64	turbo_ratio_limit;
> 
> Why does it have to be u64?
turbo ratio limit is 64 bit value. On some models it will show ratio
when up to 8 cores active like in Xeon E5 v3(SDM Table 35-27).
> 
> >  };
> >  
> >  struct vid_data {
> > @@ -132,6 +133,8 @@ struct pstate_funcs {
> >  	int (*get_scaling)(void);
> >  	void (*set)(struct cpudata*, int pstate);
> >  	void (*get_vid)(struct cpudata *);
> > +	u64 (*get_turbo_ratio_limit)(struct cpudata *);
> > +	int (*set_turbo_ratio_limit)(struct cpudata *, u64, u64);
> 
> An int is always passed as the last arg to this, so why is the arg u64?
> 
> >  };
> >  
> >  struct cpu_defaults {
> > @@ -434,6 +437,23 @@ static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b,
> >  	limits.max_perf_pct = max(limits.min_perf_pct, limits.max_perf_pct);
> >  	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
> >  
> > +	if (pstate_funcs.set_turbo_ratio_limit) {
> > +		int max_perf_adj;
> > +		struct cpudata *cpu = all_cpu_data[0];
> > +
> > +		if (limits.max_sysfs_pct == 100)
> > +			max_perf_adj = cpu->pstate.turbo_ratio_limit;
> 
> This will cast the u64 to int anyway.
I shouldn't have casted to int here, it will take care upto 4 core max
only. But not sure people using Xeons want to reduce turbo range.
> 
> Also this is going to be the value read from the register at init which is
> likely to be greater than 255 ->
> 
yes
> > +		else
> > +			max_perf_adj = fp_toint(mul_fp(int_tofp(
> > +					cpu->pstate.turbo_ratio_limit & 0xff),
> > +					limits.max_perf));
> 
> -> but this will never be greater than 255 if I'm not mistaken (limits.max_perf
> is a representation of a fraction between 0 and 1 and the first value is
> bounded by 255).
limits.max_perf is a value between 0 and 255 (E.g. 100%=255, 90% 230).
max_perf_adj will be scaled ratio based on limits.max_perf (E.g. if 1
core max ratio is 1d, and max_perf is 230 (90%), then max_perf_adj = 1a)
BTW I didn't invent this magic formula, it is copied from  existing
function intel_pstate_get_min_max.
> 
> How are these two values related to each other?
refer to above explanation.
> > +
> > +		if (max_perf_adj > cpu->pstate.max_pstate)
> > +			pstate_funcs.set_turbo_ratio_limit(cpu,
> > +						cpu->pstate.turbo_ratio_limit,
> > +						max_perf_adj);
> 
> I'm not really sure what this is supposed to achieve.  Care to explain a bit?
We only care to set turbo ratio only when user ask to set ratio which is
turbo range. Anything more than cpu->pstate.max_pstate is turbo range
(as this value stores the maximum non turbo ratio)

> 
> [BTW, the first arg of core_set/get_turbo_ratio_limit(() is never used, so why
> bother with passing it at all?]
> 
To be consistent with the current .set pstate functions. We don't use. I
can remove.
> > +	}
> > +
> >  	if (hwp_active)
> >  		intel_pstate_hwp_set();
> >  	return count;
> > @@ -628,6 +648,55 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
> >  	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
> >  }
> >  
> > +static u64 core_get_turbo_ratio_limit(struct cpudata *cpudata)
> > +{
> > +	u64 value;
> > +
> > +	rdmsrl(MSR_NHM_TURBO_RATIO_LIMIT, value);
> > +
> > +	return value;
> > +}
> > +
> > +static int core_set_turbo_ratio_limit(struct cpudata *cpudata, u64 def_ratio,
> > +			       u64 new_ratio)
> > +{
> > +	u64 value;
> > +
> 
> What should happen if def_ratio and new_ratio are the same?
> 
It will not change the resultant, but we could have avoid loop below. I
can add a check here  to return.
> > +	rdmsrl(MSR_PLATFORM_INFO, value);
> > +	if (value & BIT(28)) {
> > +		u64 ratio = 0;
> > +		u64 out_ratio = 0;
> > +		u8 max_ratio = new_ratio & 0xff;
> 
> Why u8?
1C max ratio is the maximum value any ratio can have, which is stored in
first byte.
> > +		int i;
> > +		/*
> > +		 * If caller provided reduced max ratio (one core active)
> > +		 * then use this for all other ratios, which are more
> > +		 * than the default ratio for those many cores active
> > +		 * for example if default ratio is 0x1a1b1c1d and new ratio
> > +		 * is 0x1b, then resultant ratio will be 0x1a1b1b1b
> > +		 */
> 
> This is a bit messy IMO. 
Yes.
>  Instead of shifting def_ratio and new_ratio in each
> step, I'd shift max_ratio and the mask:
> 
> 	u64 mask = 0xff;
> 	u64 max_ratio = new_ratio & mask;
> 
> 	while (mask) {
> 		if (def_ratio & mask) {
> 			u64 ratio;
> 
> 			if (new_ratio & mask) {
> 				ratio = new_ratio & mask;
> 			} else {
> 				ratio = def_ratio & mask;
> 				if (ratio > max_ratio)
> 					ratio = max_ratio;
> 			}
> 			out_ratio |= ratio;
> 		}
> 		max_ratio <<= 8;
> 		mask <<= 8;
> 	}
> 
I will experiment with your algorithm and check.

> [I'm not sure why the least significant byte of new_ratio is special, though.]
> 
LS Byte is the max turbo you can ever achieve as this ratio is for 1
core active turbo.
> > +		for (i = 0; i < sizeof(def_ratio); ++i) {
> > +			if (def_ratio & 0xff) {
> > +				if (new_ratio & 0xff)
> > +					ratio = new_ratio & 0xff;
> > +				else {
> > +					if ((def_ratio & 0xff) > max_ratio)
> > +						ratio = max_ratio;
> > +					else
> > +						ratio = def_ratio & 0xff;
> > +				}
> > +				out_ratio |= (ratio << (i * 8));
> > +			}
> > +			def_ratio >>= 8;
> > +			new_ratio >>= 8;
> > +		}
> > +		wrmsrl(MSR_NHM_TURBO_RATIO_LIMIT, out_ratio);
> > +		return 0;
> > +	}
> > +
> > +	return -EPERM;
> 
> Why -EPERM?
> 
> That's not because the user has no permission to carry out the opreration, but
> because there is no capability, right?
> 
Yes. Is there any better error code?
> > +}
> > +
> >  static int knl_get_turbo_pstate(void)
> >  {
> >  	u64 value;
> > @@ -656,6 +725,8 @@ static struct cpu_defaults core_params = {
> >  		.get_turbo = core_get_turbo_pstate,
> >  		.get_scaling = core_get_scaling,
> >  		.set = core_set_pstate,
> > +		.get_turbo_ratio_limit = core_get_turbo_ratio_limit,
> > +		.set_turbo_ratio_limit = core_set_turbo_ratio_limit,
> >  	},
> >  };
> >  
> > @@ -745,7 +816,10 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
> >  	cpu->pstate.max_pstate = pstate_funcs.get_max();
> >  	cpu->pstate.turbo_pstate = pstate_funcs.get_turbo();
> >  	cpu->pstate.scaling = pstate_funcs.get_scaling();
> > -
> > +	if (pstate_funcs.get_turbo_ratio_limit &&
> > +	    !cpu->pstate.turbo_ratio_limit)
> > +		cpu->pstate.turbo_ratio_limit =
> > +			pstate_funcs.get_turbo_ratio_limit(cpu);
> 
> So we read MSR_NHM_TURBO_RATIO_LIMIT and store it into pstate.turbo_ratio_limit
> unless alread populated.  This means we do it only once during initialization.
> 
> What if the value we have in there is stale after, say, system suspend-resume?
> 
Did I mess up here? You mean data stored in a variable
cpu->pstate.turbo_ratio_limit is corrupted after suspend/resume. This is
a vzalloc memory.. 
all_cpu_data = vzalloc(sizeof(void *) * num_possible_cpus()) one time
during __init intel_pstate_init
We re-adjust the ratio value after suspend/resume from the value stored
during init if user has changed this, because msr will be reset to
default after resume.
> Don't we need to re-adjust then?
> 
> >  	if (pstate_funcs.get_vid)
> >  		pstate_funcs.get_vid(cpu);
> >  	intel_pstate_set_pstate(cpu, cpu->pstate.min_pstate, false);
> > @@ -949,6 +1023,20 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
> >  		intel_pstate_hwp_enable(cpu);
> >  
> >  	intel_pstate_get_cpu_pstates(cpu);
> > +	/* readjust turbo limit ratio after resume or hotplug */
> > +	if (limits.max_sysfs_pct != 100 &&
> > +	    pstate_funcs.set_turbo_ratio_limit) {
> > +		int max_perf_adj;
> > +
> > +		max_perf_adj = fp_toint(mul_fp(int_tofp(
> > +					cpu->pstate.turbo_ratio_limit & 0xff),
> > +					limits.max_perf));
> > +
> > +		if (max_perf_adj > cpu->pstate.max_pstate)
> > +			pstate_funcs.set_turbo_ratio_limit(cpu,
> > +						cpu->pstate.turbo_ratio_limit,
> > +						max_perf_adj);
> > +	}
> >  
> >  	init_timer_deferrable(&cpu->timer);
> >  	cpu->timer.data = (unsigned long)cpu;
> > @@ -1118,6 +1206,8 @@ static void copy_cpu_funcs(struct pstate_funcs *funcs)
> >  	pstate_funcs.get_scaling = funcs->get_scaling;
> >  	pstate_funcs.set       = funcs->set;
> >  	pstate_funcs.get_vid   = funcs->get_vid;
> > +	pstate_funcs.set_turbo_ratio_limit = funcs->set_turbo_ratio_limit;
> > +	pstate_funcs.get_turbo_ratio_limit = funcs->get_turbo_ratio_limit;
> 
> These are defined for Core only, right?  Might be good to write about that in
> the changelog.
Yes. I will.

Thanks,
Srinivas
> 
> >  }
> >  
> >  #if IS_ENABLED(CONFIG_ACPI)
> > 
> 
> Thanks,
> Rafael
> 

Re: [PATCH v3 1/6] cpufreq: intel_p_state: Fix limiting turbo sub states

[Pandruvada, Srinivas]

On Mon, 2015-10-05 at 17:43 -0700, Srinivas Pandruvada wrote:
> On Tue, 2015-10-06 at 00:56 +0200, Rafael J. Wysocki wrote:
[..]
> >  struct vid_data {
> > @@ -132,6 +133,8 @@ struct pstate_funcs {
> >     int (*get_scaling)(void);
> >     void (*set)(struct cpudata*, int pstate);
> >     void (*get_vid)(struct cpudata *);
> > +   u64 (*get_turbo_ratio_limit)(struct cpudata *);
> > +   int (*set_turbo_ratio_limit)(struct cpudata *, u64, u64);
> 
> An int is always passed as the last arg to this, so why is the arg
u64?
> 
It can be int as we care only about low byte which is max turbo ratio.
> >  };
[..]
> >  Instead of shifting def_ratio and new_ratio in each
> > step, I'd shift max_ratio and the mask:
> > 
> > 	u64 mask = 0xff;
> > 	u64 max_ratio = new_ratio & mask;
> > 
> > 	while (mask) {
> > 		if (def_ratio & mask) {
> > 			u64 ratio;
> > 
> > 			if (new_ratio & mask) {
> > 				ratio = new_ratio & mask;
> > 			} else {
> > 				ratio = def_ratio & mask;
> > 				if (ratio > max_ratio)
> > 					ratio = max_ratio;
> > 			}
> > 			out_ratio |= ratio;
> > 		}
> > 		max_ratio <<= 8;
> > 		mask <<= 8;
> > 	}
> > 
> I will experiment with your algorithm and check.
> 
Your algorithm works, so will change to this.

[..]

> > > +	if (pstate_funcs.get_turbo_ratio_limit &&
> > > +	    !cpu->pstate.turbo_ratio_limit)
> > > +		cpu->pstate.turbo_ratio_limit =
> > > +			pstate_funcs.get_turbo_ratio_limit(cpu);
> > 
> > So we read MSR_NHM_TURBO_RATIO_LIMIT and store it into pstate.turbo_ratio_limit
> > unless alread populated.  This means we do it only once during initialization.
> > 
> > What if the value we have in there is stale after, say, system suspend-resume?
> > 
> Did I mess up here? You mean data stored in a variable
> cpu->pstate.turbo_ratio_limit is corrupted after suspend/resume. This is
> a vzalloc memory.. 
> all_cpu_data = vzalloc(sizeof(void *) * num_possible_cpus()) one time
> during __init intel_pstate_init
Also 	all_cpu_data[cpunum] is allocated only once during first cpufreq
callback for init and not freed
	if (!all_cpu_data[cpunum])
		all_cpu_data[cpunum] = kzalloc(sizeof(struct cpudata),
					       GFP_KERNEL);
Did STR tests, didn't see issue. Not sure how else corruption can happen
unless slab memory is corrupted. Can you tell how it will be stale?

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

Re: [PATCH v3 1/6] cpufreq: intel_p_state: Fix limiting turbo sub states

[Rafael J. Wysocki]

On Tuesday, October 06, 2015 05:08:14 PM Pandruvada, Srinivas wrote:
> On Mon, 2015-10-05 at 17:43 -0700, Srinivas Pandruvada wrote:
> > On Tue, 2015-10-06 at 00:56 +0200, Rafael J. Wysocki wrote:
> [..]
> > >  struct vid_data {
> > > @@ -132,6 +133,8 @@ struct pstate_funcs {
> > >     int (*get_scaling)(void);
> > >     void (*set)(struct cpudata*, int pstate);
> > >     void (*get_vid)(struct cpudata *);
> > > +   u64 (*get_turbo_ratio_limit)(struct cpudata *);
> > > +   int (*set_turbo_ratio_limit)(struct cpudata *, u64, u64);

I've started to wonder about the value of having the second new callback at all.

The guys who need it will always have pstate.turbo_ratio_limit different
from zero, so we can use this check to decide whether or not to write to
the register.

> > 
> > An int is always passed as the last arg to this, so why is the arg
> u64?
> > 
> It can be int as we care only about low byte which is max turbo ratio.

Hmm.

> > >  };
> [..]
> > >  Instead of shifting def_ratio and new_ratio in each
> > > step, I'd shift max_ratio and the mask:
> > > 
> > > 	u64 mask = 0xff;
> > > 	u64 max_ratio = new_ratio & mask;
> > > 
> > > 	while (mask) {
> > > 		if (def_ratio & mask) {
> > > 			u64 ratio;
> > > 
> > > 			if (new_ratio & mask) {

So why do we need this?

If we only care about the lowest byte only, we should discard all of the upper
bytes in new_ration here, shouldn't we?

> > > 				ratio = new_ratio & mask;
> > > 			} else {
> > > 				ratio = def_ratio & mask;
> > > 				if (ratio > max_ratio)
> > > 					ratio = max_ratio;
> > > 			}
> > > 			out_ratio |= ratio;
> > > 		}
> > > 		max_ratio <<= 8;
> > > 		mask <<= 8;
> > > 	}
> > > 
> > I will experiment with your algorithm and check.
> > 
> Your algorithm works, so will change to this.
> 
> [..]
> 
> > > > +	if (pstate_funcs.get_turbo_ratio_limit &&
> > > > +	    !cpu->pstate.turbo_ratio_limit)
> > > > +		cpu->pstate.turbo_ratio_limit =
> > > > +			pstate_funcs.get_turbo_ratio_limit(cpu);
> > > 
> > > So we read MSR_NHM_TURBO_RATIO_LIMIT and store it into pstate.turbo_ratio_limit
> > > unless alread populated.  This means we do it only once during initialization.
> > > 
> > > What if the value we have in there is stale after, say, system suspend-resume?
> > > 
> > Did I mess up here? You mean data stored in a variable
> > cpu->pstate.turbo_ratio_limit is corrupted after suspend/resume. This is
> > a vzalloc memory.. 
> > all_cpu_data = vzalloc(sizeof(void *) * num_possible_cpus()) one time
> > during __init intel_pstate_init
> Also 	all_cpu_data[cpunum] is allocated only once during first cpufreq
> callback for init and not freed
> 	if (!all_cpu_data[cpunum])
> 		all_cpu_data[cpunum] = kzalloc(sizeof(struct cpudata),
> 					       GFP_KERNEL);
> Did STR tests, didn't see issue. Not sure how else corruption can happen
> unless slab memory is corrupted. Can you tell how it will be stale?

I was concerned about the BIOS changing the value in the register in which
case we probably should restore what we wrote to it last time or users
may be confused.

Thanks,
Rafael

Re: [PATCH v3 1/6] cpufreq: intel_p_state: Fix limiting turbo sub states

[Rafael J. Wysocki]

On Monday, October 05, 2015 05:43:05 PM Srinivas Pandruvada wrote:
> On Tue, 2015-10-06 at 00:56 +0200, Rafael J. Wysocki wrote:
> > On Tuesday, September 29, 2015 03:54:03 PM Srinivas Pandruvada wrote:
> > > Although the max_perf_pct reflects sub states in turbo range, we can't
> > > really restrict to those states. This gives wrong impression that the
> > > performance is reduced.
> > > This can be achieved by restricting turbo ratio limits (MSR 0x1AD),
> > > when bit 28 of platform info MSR allows (MSR 0xCE) is 1.
> > > 
> > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > ---
> > >  drivers/cpufreq/intel_pstate.c | 92 +++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 91 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > > index 3af9dd7..576d9e8 100644
> > > --- a/drivers/cpufreq/intel_pstate.c
> > > +++ b/drivers/cpufreq/intel_pstate.c
> > > @@ -80,6 +80,7 @@ struct pstate_data {
> > >  	int	max_pstate;
> > >  	int	scaling;
> > >  	int	turbo_pstate;
> > > +	u64	turbo_ratio_limit;
> > 
> > Why does it have to be u64?
> turbo ratio limit is 64 bit value. On some models it will show ratio
> when up to 8 cores active like in Xeon E5 v3(SDM Table 35-27).
> > 
> > >  };
> > >  
> > >  struct vid_data {
> > > @@ -132,6 +133,8 @@ struct pstate_funcs {
> > >  	int (*get_scaling)(void);
> > >  	void (*set)(struct cpudata*, int pstate);
> > >  	void (*get_vid)(struct cpudata *);
> > > +	u64 (*get_turbo_ratio_limit)(struct cpudata *);
> > > +	int (*set_turbo_ratio_limit)(struct cpudata *, u64, u64);
> > 
> > An int is always passed as the last arg to this, so why is the arg u64?
> > 
> > >  };
> > >  
> > >  struct cpu_defaults {
> > > @@ -434,6 +437,23 @@ static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b,
> > >  	limits.max_perf_pct = max(limits.min_perf_pct, limits.max_perf_pct);
> > >  	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
> > >  
> > > +	if (pstate_funcs.set_turbo_ratio_limit) {
> > > +		int max_perf_adj;
> > > +		struct cpudata *cpu = all_cpu_data[0];
> > > +
> > > +		if (limits.max_sysfs_pct == 100)
> > > +			max_perf_adj = cpu->pstate.turbo_ratio_limit;
> > 
> > This will cast the u64 to int anyway.
> I shouldn't have casted to int here, it will take care upto 4 core max
> only. But not sure people using Xeons want to reduce turbo range.
> > 
> > Also this is going to be the value read from the register at init which is
> > likely to be greater than 255 ->
> > 
> yes
> > > +		else
> > > +			max_perf_adj = fp_toint(mul_fp(int_tofp(
> > > +					cpu->pstate.turbo_ratio_limit & 0xff),
> > > +					limits.max_perf));
> > 
> > -> but this will never be greater than 255 if I'm not mistaken (limits.max_perf
> > is a representation of a fraction between 0 and 1 and the first value is
> > bounded by 255).
> limits.max_perf is a value between 0 and 255 (E.g. 100%=255, 90% 230).
> max_perf_adj will be scaled ratio based on limits.max_perf (E.g. if 1
> core max ratio is 1d, and max_perf is 230 (90%), then max_perf_adj = 1a)
> BTW I didn't invent this magic formula, it is copied from  existing
> function intel_pstate_get_min_max.
> > 
> > How are these two values related to each other?
> refer to above explanation.

Well, it looks like you always want to pass a single byte as the second arg of
pstate_funcs.set_turbo_ratio_limit(), because otherwise there are two cases
that are handled differently.

> > > +
> > > +		if (max_perf_adj > cpu->pstate.max_pstate)
> > > +			pstate_funcs.set_turbo_ratio_limit(cpu,
> > > +						cpu->pstate.turbo_ratio_limit,
> > > +						max_perf_adj);
> > 
> > I'm not really sure what this is supposed to achieve.  Care to explain a bit?
> We only care to set turbo ratio only when user ask to set ratio which is
> turbo range. Anything more than cpu->pstate.max_pstate is turbo range
> (as this value stores the maximum non turbo ratio)
> 
> > 
> > [BTW, the first arg of core_set/get_turbo_ratio_limit(() is never used, so why
> > bother with passing it at all?]
> > 
> To be consistent with the current .set pstate functions. We don't use. I
> can remove.

Yes, please.  It is better to avoid passing arguments that aren't used.

> > > +	}
> > > +
> > >  	if (hwp_active)
> > >  		intel_pstate_hwp_set();
> > >  	return count;
> > > @@ -628,6 +648,55 @@ static void core_set_pstate(struct cpudata *cpudata, int pstate)
> > >  	wrmsrl_on_cpu(cpudata->cpu, MSR_IA32_PERF_CTL, val);
> > >  }
> > >  
> > > +static u64 core_get_turbo_ratio_limit(struct cpudata *cpudata)
> > > +{
> > > +	u64 value;
> > > +
> > > +	rdmsrl(MSR_NHM_TURBO_RATIO_LIMIT, value);
> > > +
> > > +	return value;
> > > +}
> > > +
> > > +static int core_set_turbo_ratio_limit(struct cpudata *cpudata, u64 def_ratio,
> > > +			       u64 new_ratio)
> > > +{
> > > +	u64 value;
> > > +
> > 
> > What should happen if def_ratio and new_ratio are the same?
> > 
> It will not change the resultant, but we could have avoid loop below. I
> can add a check here  to return.

That was exactly my thought here. :-)

> > > +	rdmsrl(MSR_PLATFORM_INFO, value);
> > > +	if (value & BIT(28)) {
> > > +		u64 ratio = 0;
> > > +		u64 out_ratio = 0;
> > > +		u8 max_ratio = new_ratio & 0xff;
> > 
> > Why u8?
> 1C max ratio is the maximum value any ratio can have, which is stored in
> first byte.
> > > +		int i;
> > > +		/*
> > > +		 * If caller provided reduced max ratio (one core active)
> > > +		 * then use this for all other ratios, which are more
> > > +		 * than the default ratio for those many cores active
> > > +		 * for example if default ratio is 0x1a1b1c1d and new ratio
> > > +		 * is 0x1b, then resultant ratio will be 0x1a1b1b1b
> > > +		 */
> > 
> > This is a bit messy IMO. 
> Yes.
> >  Instead of shifting def_ratio and new_ratio in each
> > step, I'd shift max_ratio and the mask:
> > 
> > 	u64 mask = 0xff;
> > 	u64 max_ratio = new_ratio & mask;
> > 
> > 	while (mask) {
> > 		if (def_ratio & mask) {
> > 			u64 ratio;
> > 
> > 			if (new_ratio & mask) {
> > 				ratio = new_ratio & mask;
> > 			} else {
> > 				ratio = def_ratio & mask;
> > 				if (ratio > max_ratio)
> > 					ratio = max_ratio;
> > 			}
> > 			out_ratio |= ratio;
> > 		}
> > 		max_ratio <<= 8;
> > 		mask <<= 8;
> > 	}
> > 
> I will experiment with your algorithm and check.
> 
> > [I'm not sure why the least significant byte of new_ratio is special, though.]
> > 
> LS Byte is the max turbo you can ever achieve as this ratio is for 1
> core active turbo.

OK

> > > +		for (i = 0; i < sizeof(def_ratio); ++i) {
> > > +			if (def_ratio & 0xff) {
> > > +				if (new_ratio & 0xff)
> > > +					ratio = new_ratio & 0xff;
> > > +				else {
> > > +					if ((def_ratio & 0xff) > max_ratio)
> > > +						ratio = max_ratio;
> > > +					else
> > > +						ratio = def_ratio & 0xff;
> > > +				}
> > > +				out_ratio |= (ratio << (i * 8));
> > > +			}
> > > +			def_ratio >>= 8;
> > > +			new_ratio >>= 8;
> > > +		}
> > > +		wrmsrl(MSR_NHM_TURBO_RATIO_LIMIT, out_ratio);
> > > +		return 0;
> > > +	}
> > > +
> > > +	return -EPERM;
> > 
> > Why -EPERM?
> > 
> > That's not because the user has no permission to carry out the opreration, but
> > because there is no capability, right?
> > 
> Yes. Is there any better error code?

-ENXIO would be better IMO.

Thanks,
Rafael

Re: [PATCH v3 2/6] cpufreq: intel_pstate: get P1 from TAR when available

[Kristen Carlson Accardi]

On Tue, 29 Sep 2015 15:54:04 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> After Ivybridge, the max non turbo ratio obtained from platform info msr
> is not always guaranteed P1 on client platforms. The max non turbo
> activation ratio (TAR), determines the max for the current level of TDP.
> The ratio in platform info is physical max. The TAR MSR can be locked,
> so updating this value is not possible on all platforms.
> This change gets this ratio from MSR TURBO_ACTIVATION_RATIO if
> available,
> but also do some sanity checking to make sure that this value is
> correct.
> The sanity check involves reading the TDP ratio for the current tdp
> control value when platform has configurable TDP present and matching
> TAC
> with this.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Acked-by: Kristen Carlson Accardi <kristen@linux.intel.com>

> ---
>  arch/x86/include/asm/msr-index.h |  7 +++++++
>  drivers/cpufreq/intel_pstate.c   | 39 +++++++++++++++++++++++++++++++++++----
>  2 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index b8c14bb..9f39056 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -206,6 +206,13 @@
>  #define MSR_GFX_PERF_LIMIT_REASONS	0x000006B0
>  #define MSR_RING_PERF_LIMIT_REASONS	0x000006B1
>  
> +/* Config TDP MSRs */
> +#define MSR_CONFIG_TDP_NOMINAL		0x00000648
> +#define MSR_CONFIG_TDP_LEVEL1		0x00000649
> +#define MSR_CONFIG_TDP_LEVEL2		0x0000064A
> +#define MSR_CONFIG_TDP_CONTROL		0x0000064B
> +#define MSR_TURBO_ACTIVATION_RATIO	0x0000064C
> +
>  /* Hardware P state interface */
>  #define MSR_PPERF			0x0000064e
>  #define MSR_PERF_LIMIT_REASONS		0x0000064f
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 576d9e8..b0ae951 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -43,7 +43,6 @@
>  #define int_tofp(X) ((int64_t)(X) << FRAC_BITS)
>  #define fp_toint(X) ((X) >> FRAC_BITS)
>  
> -
>  static inline int32_t mul_fp(int32_t x, int32_t y)
>  {
>  	return ((int64_t)x * (int64_t)y) >> FRAC_BITS;
> @@ -613,10 +612,42 @@ static int core_get_min_pstate(void)
>  
>  static int core_get_max_pstate(void)
>  {
> -	u64 value;
> +	u64 tar;
> +	u64 plat_info;
> +	int max_pstate;
> +	int err;
> +
> +	rdmsrl(MSR_PLATFORM_INFO, plat_info);
> +	max_pstate = (plat_info >> 8) & 0xFF;
> +
> +	err = rdmsrl_safe(MSR_TURBO_ACTIVATION_RATIO, &tar);
> +	if (!err) {
> +		/* Do some sanity checking for safety */
> +		if (plat_info & 0x600000000) {
> +			u64 tdp_ctrl;
> +			u64 tdp_ratio;
> +			int tdp_msr;
> +
> +			err = rdmsrl_safe(MSR_CONFIG_TDP_CONTROL, &tdp_ctrl);
> +			if (err)
> +				goto skip_tar;
> +
> +			tdp_msr = MSR_CONFIG_TDP_NOMINAL + tdp_ctrl;
> +			err = rdmsrl_safe(tdp_msr, &tdp_ratio);
> +			if (err)
> +				goto skip_tar;
> +
> +			if (tdp_ratio - 1 == tar) {
> +				max_pstate = tar;
> +				pr_debug("max_pstate=TAC %x\n", max_pstate);
> +			} else {
> +				goto skip_tar;
> +			}
> +		}
> +	}
>  
> -	rdmsrl(MSR_PLATFORM_INFO, value);
> -	return (value >> 8) & 0xFF;
> +skip_tar:
> +	return max_pstate;
>  }
>  
>  static int core_get_turbo_pstate(void)

Re: [PATCH v3 3/6] cpufreq: intel-pstate: Use separate max pstate for scaling

[Kristen Carlson Accardi]

On Tue, 29 Sep 2015 15:54:05 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> Systems with configurable TDP have multiple max non turbo p state. Intel
> P state uses max non turbo P state for scaling. But using the real max
> non turbo p state causes underestimation of next P state. So using
> the physical max non turbo P state as before for scaling.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Acked-by: Kristen Carlson Accardi <kristen@linux.intel.com>

> ---
>  drivers/cpufreq/intel_pstate.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index b0ae951..0ae9618 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -77,6 +77,7 @@ struct pstate_data {
>  	int	current_pstate;
>  	int	min_pstate;
>  	int	max_pstate;
> +	int	max_pstate_physical;
>  	int	scaling;
>  	int	turbo_pstate;
>  	u64	turbo_ratio_limit;
> @@ -127,6 +128,7 @@ struct pstate_adjust_policy {
>  
>  struct pstate_funcs {
>  	int (*get_max)(void);
> +	int (*get_max_physical)(void);
>  	int (*get_min)(void);
>  	int (*get_turbo)(void);
>  	int (*get_scaling)(void);
> @@ -610,6 +612,14 @@ static int core_get_min_pstate(void)
>  	return (value >> 40) & 0xFF;
>  }
>  
> +static int core_get_max_pstate_physical(void)
> +{
> +	u64 value;
> +
> +	rdmsrl(MSR_PLATFORM_INFO, value);
> +	return (value >> 8) & 0xFF;
> +}
> +
>  static int core_get_max_pstate(void)
>  {
>  	u64 tar;
> @@ -752,6 +762,7 @@ static struct cpu_defaults core_params = {
>  	},
>  	.funcs = {
>  		.get_max = core_get_max_pstate,
> +		.get_max_physical = core_get_max_pstate_physical,
>  		.get_min = core_get_min_pstate,
>  		.get_turbo = core_get_turbo_pstate,
>  		.get_scaling = core_get_scaling,
> @@ -772,6 +783,7 @@ static struct cpu_defaults byt_params = {
>  	},
>  	.funcs = {
>  		.get_max = byt_get_max_pstate,
> +		.get_max_physical = byt_get_max_pstate,
>  		.get_min = byt_get_min_pstate,
>  		.get_turbo = byt_get_turbo_pstate,
>  		.set = byt_set_pstate,
> @@ -791,6 +803,7 @@ static struct cpu_defaults knl_params = {
>  	},
>  	.funcs = {
>  		.get_max = core_get_max_pstate,
> +		.get_max_physical = core_get_max_pstate_physical,
>  		.get_min = core_get_min_pstate,
>  		.get_turbo = knl_get_turbo_pstate,
>  		.get_scaling = core_get_scaling,
> @@ -845,6 +858,7 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
>  {
>  	cpu->pstate.min_pstate = pstate_funcs.get_min();
>  	cpu->pstate.max_pstate = pstate_funcs.get_max();
> +	cpu->pstate.max_pstate_physical = pstate_funcs.get_max_physical();
>  	cpu->pstate.turbo_pstate = pstate_funcs.get_turbo();
>  	cpu->pstate.scaling = pstate_funcs.get_scaling();
>  	if (pstate_funcs.get_turbo_ratio_limit &&
> @@ -866,7 +880,8 @@ static inline void intel_pstate_calc_busy(struct cpudata *cpu)
>  
>  	sample->freq = fp_toint(
>  		mul_fp(int_tofp(
> -			cpu->pstate.max_pstate * cpu->pstate.scaling / 100),
> +			cpu->pstate.max_pstate_physical *
> +			cpu->pstate.scaling / 100),
>  			core_pct));
>  
>  	sample->core_pct_busy = (int32_t)core_pct;
> @@ -934,7 +949,7 @@ static inline int32_t intel_pstate_get_scaled_busy(struct cpudata *cpu)
>  	 * specified pstate.
>  	 */
>  	core_busy = cpu->sample.core_pct_busy;
> -	max_pstate = int_tofp(cpu->pstate.max_pstate);
> +	max_pstate = int_tofp(cpu->pstate.max_pstate_physical);
>  	current_pstate = int_tofp(cpu->pstate.current_pstate);
>  	core_busy = mul_fp(core_busy, div_fp(max_pstate, current_pstate));
>  
> @@ -1232,6 +1247,7 @@ static void copy_pid_params(struct pstate_adjust_policy *policy)
>  static void copy_cpu_funcs(struct pstate_funcs *funcs)
>  {
>  	pstate_funcs.get_max   = funcs->get_max;
> +	pstate_funcs.get_max_physical = funcs->get_max_physical;
>  	pstate_funcs.get_min   = funcs->get_min;
>  	pstate_funcs.get_turbo = funcs->get_turbo;
>  	pstate_funcs.get_scaling = funcs->get_scaling;

Re: [PATCH v3 4/6] cpufreq: intel_pstate: Use ACPI perf configuration

[Kristen Carlson Accardi]

On Tue, 29 Sep 2015 15:54:06 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> Use ACPI _PSS to limit the Intel P State turbo, max and min ratios.
> This driver uses acpi processor perf lib calls to register performance.
> The following logic is used to adjust Intel P state driver limits:
> - If there is no turbo entry in _PSS, then disable Intel P state turbo
> and limit to non turbo max
> - If the non turbo max ratio is more than _PSS max non turbo value, then
> set the max non turbo ratio to _PSS non turbo max
> - If the min ratio is less than _PSS min then change the min ratio
> matching _PSS min
> - Scale the _PSS turbo frequency to max turbo frequency based on control
> value.
> This feature can be disabled by using kernel parameters:
> intel_pstate=no_acpi
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Acked-by: Kristen Carlson Accardi <kristen@linux.intel.com>

> ---
>  drivers/cpufreq/Kconfig.x86    |   1 +
>  drivers/cpufreq/intel_pstate.c | 171 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 171 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index c59bdcb..adbd1de 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -5,6 +5,7 @@
>  config X86_INTEL_PSTATE
>         bool "Intel P state control"
>         depends on X86
> +       select ACPI_PROCESSOR if ACPI
>         help
>            This driver provides a P state for Intel core processors.
>  	  The driver implements an internal governor and will become
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 0ae9618..869f074 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -34,6 +34,10 @@
>  #include <asm/cpu_device_id.h>
>  #include <asm/cpufeature.h>
>  
> +#if IS_ENABLED(CONFIG_ACPI)
> +#include <acpi/processor.h>
> +#endif
> +
>  #define BYT_RATIOS		0x66a
>  #define BYT_VIDS		0x66b
>  #define BYT_TURBO_RATIOS	0x66c
> @@ -114,6 +118,9 @@ struct cpudata {
>  	u64	prev_mperf;
>  	u64	prev_tsc;
>  	struct sample sample;
> +#if IS_ENABLED(CONFIG_ACPI)
> +	struct acpi_processor_performance acpi_perf_data;
> +#endif
>  };
>  
>  static struct cpudata **all_cpu_data;
> @@ -146,6 +153,7 @@ struct cpu_defaults {
>  static struct pstate_adjust_policy pid_params;
>  static struct pstate_funcs pstate_funcs;
>  static int hwp_active;
> +static int no_acpi_perf;
>  
>  struct perf_limits {
>  	int no_turbo;
> @@ -173,6 +181,153 @@ static struct perf_limits limits = {
>  	.min_sysfs_pct = 0,
>  };
>  
> +#if IS_ENABLED(CONFIG_ACPI)
> +/*
> + * The max target pstate ratio is a 8 bit value in both PLATFORM_INFO MSR and
> + * in TURBO_RATIO_LIMIT MSR, which pstate driver stores in max_pstate and
> + * max_turbo_pstate fields. The PERF_CTL MSR contains 16 bit value for P state
> + * ratio, out of it only high 8 bits are used. For example 0x1700 is setting
> + * target ratio 0x17. The _PSS control value stores in a format which can be
> + * directly written to PERF_CTL MSR. But in intel_pstate driver this shift
> + * occurs during write to PERF_CTL (E.g. for cores core_set_pstate()).
> + * This function converts the _PSS control value to intel pstate driver format
> + * for comparison and assignment.
> + */
> +static int convert_to_native_pstate_format(struct cpudata *cpu, int index)
> +{
> +	return cpu->acpi_perf_data.states[index].control >> 8;
> +}
> +
> +static int intel_pstate_init_perf_limits(struct cpufreq_policy *policy)
> +{
> +	struct cpudata *cpu;
> +	int ret;
> +	bool turbo_absent = false;
> +	int max_pstate_index;
> +	int min_pss_ctl, max_pss_ctl, turbo_pss_ctl;
> +	int i;
> +
> +	cpu = all_cpu_data[policy->cpu];
> +
> +	pr_debug("intel_pstate: default limits 0x%x 0x%x 0x%x\n",
> +		 cpu->pstate.min_pstate, cpu->pstate.max_pstate,
> +		 cpu->pstate.turbo_pstate);
> +
> +	if (!cpu->acpi_perf_data.shared_cpu_map &&
> +	    zalloc_cpumask_var_node(&cpu->acpi_perf_data.shared_cpu_map,
> +				    GFP_KERNEL, cpu_to_node(policy->cpu))) {
> +		return -ENOMEM;
> +	}
> +
> +	ret = acpi_processor_register_performance(&cpu->acpi_perf_data,
> +						  policy->cpu);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Check if the control value in _PSS is for PERF_CTL MSR, which should
> +	 * guarantee that the states returned by it map to the states in our
> +	 * list directly.
> +	 */
> +	if (cpu->acpi_perf_data.control_register.space_id !=
> +						ACPI_ADR_SPACE_FIXED_HARDWARE)
> +		return -EIO;
> +
> +	pr_debug("intel_pstate: CPU%u - ACPI _PSS perf data\n", policy->cpu);
> +	for (i = 0; i < cpu->acpi_perf_data.state_count; i++)
> +		pr_debug("     %cP%d: %u MHz, %u mW, 0x%x\n",
> +			 (i == cpu->acpi_perf_data.state ? '*' : ' '), i,
> +			 (u32) cpu->acpi_perf_data.states[i].core_frequency,
> +			 (u32) cpu->acpi_perf_data.states[i].power,
> +			 (u32) cpu->acpi_perf_data.states[i].control);
> +
> +	/*
> +	 * If there is only one entry _PSS, simply ignore _PSS and continue as
> +	 * usual without taking _PSS into account
> +	 */
> +	if (cpu->acpi_perf_data.state_count < 2)
> +		return 0;
> +
> +	turbo_pss_ctl = convert_to_native_pstate_format(cpu, 0);
> +	min_pss_ctl = convert_to_native_pstate_format(cpu,
> +					cpu->acpi_perf_data.state_count - 1);
> +	/* Check if there is a turbo freq in _PSS */
> +	if (turbo_pss_ctl <= cpu->pstate.max_pstate &&
> +	    turbo_pss_ctl > cpu->pstate.min_pstate) {
> +		pr_debug("intel_pstate: no turbo range exists in _PSS\n");
> +		limits.no_turbo = limits.turbo_disabled = 1;
> +		cpu->pstate.turbo_pstate = cpu->pstate.max_pstate;
> +		turbo_absent = true;
> +	}
> +
> +	/* Check if the max non turbo p state < Intel P state max */
> +	max_pstate_index = turbo_absent ? 0 : 1;
> +	max_pss_ctl = convert_to_native_pstate_format(cpu, max_pstate_index);
> +	if (max_pss_ctl < cpu->pstate.max_pstate &&
> +	    max_pss_ctl > cpu->pstate.min_pstate)
> +		cpu->pstate.max_pstate = max_pss_ctl;
> +
> +	/* check If min perf > Intel P State min */
> +	if (min_pss_ctl > cpu->pstate.min_pstate &&
> +	    min_pss_ctl < cpu->pstate.max_pstate) {
> +		cpu->pstate.min_pstate = min_pss_ctl;
> +		policy->cpuinfo.min_freq = min_pss_ctl * cpu->pstate.scaling;
> +	}
> +
> +	if (turbo_absent)
> +		policy->cpuinfo.max_freq = cpu->pstate.max_pstate *
> +						cpu->pstate.scaling;
> +	else {
> +		policy->cpuinfo.max_freq = cpu->pstate.turbo_pstate *
> +						cpu->pstate.scaling;
> +		/*
> +		 * The _PSS table doesn't contain whole turbo frequency range.
> +		 * This just contains +1 MHZ above the max non turbo frequency,
> +		 * with control value corresponding to max turbo ratio. But
> +		 * when cpufreq set policy is called, it will call with this
> +		 * max frequency, which will cause a reduced performance as
> +		 * this driver uses real max turbo frequency as the max
> +		 * frequeny. So correct this frequency in _PSS table to
> +		 * correct max turbo frequency based on the turbo ratio.
> +		 * Also need to convert to MHz as _PSS freq is in MHz.
> +		 */
> +		cpu->acpi_perf_data.states[0].core_frequency =
> +						turbo_pss_ctl * 100;
> +	}
> +
> +	pr_debug("intel_pstate: Updated limits using _PSS 0x%x 0x%x 0x%x\n",
> +		 cpu->pstate.min_pstate, cpu->pstate.max_pstate,
> +		 cpu->pstate.turbo_pstate);
> +	pr_debug("intel_pstate: policy max_freq=%d Khz min_freq = %d KHz\n",
> +		 policy->cpuinfo.max_freq, policy->cpuinfo.min_freq);
> +
> +	return 0;
> +}
> +
> +static int intel_pstate_exit_perf_limits(struct cpufreq_policy *policy)
> +{
> +	struct cpudata *cpu;
> +
> +	if (!no_acpi_perf)
> +		return 0;
> +
> +	cpu = all_cpu_data[policy->cpu];
> +	acpi_processor_unregister_performance(policy->cpu);
> +	return 0;
> +}
> +
> +#else
> +static int intel_pstate_init_perf_limits(struct cpufreq_policy *policy)
> +{
> +	return 0;
> +}
> +
> +static int intel_pstate_exit_perf_limits(struct cpufreq_policy *policy)
> +{
> +	return 0;
> +}
> +#endif
> +
>  static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
>  			     int deadband, int integral) {
>  	pid->setpoint = setpoint;
> @@ -1203,18 +1358,30 @@ static int intel_pstate_cpu_init(struct cpufreq_policy *policy)
>  	policy->cpuinfo.min_freq = cpu->pstate.min_pstate * cpu->pstate.scaling;
>  	policy->cpuinfo.max_freq =
>  		cpu->pstate.turbo_pstate * cpu->pstate.scaling;
> +	if (!no_acpi_perf)
> +		intel_pstate_init_perf_limits(policy);
> +	/*
> +	 * If there is no acpi perf data or error, we ignore and use Intel P
> +	 * state calculated limits, So this is not fatal error.
> +	 */
>  	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
>  	cpumask_set_cpu(policy->cpu, policy->cpus);
>  
>  	return 0;
>  }
>  
> +static int intel_pstate_cpu_exit(struct cpufreq_policy *policy)
> +{
> +	return intel_pstate_exit_perf_limits(policy);
> +}
> +
>  static struct cpufreq_driver intel_pstate_driver = {
>  	.flags		= CPUFREQ_CONST_LOOPS,
>  	.verify		= intel_pstate_verify_policy,
>  	.setpolicy	= intel_pstate_set_policy,
>  	.get		= intel_pstate_get,
>  	.init		= intel_pstate_cpu_init,
> +	.exit		= intel_pstate_cpu_exit,
>  	.stop_cpu	= intel_pstate_stop_cpu,
>  	.name		= "intel_pstate",
>  };
> @@ -1258,7 +1425,6 @@ static void copy_cpu_funcs(struct pstate_funcs *funcs)
>  }
>  
>  #if IS_ENABLED(CONFIG_ACPI)
> -#include <acpi/processor.h>
>  
>  static bool intel_pstate_no_acpi_pss(void)
>  {
> @@ -1450,6 +1616,9 @@ static int __init intel_pstate_setup(char *str)
>  		force_load = 1;
>  	if (!strcmp(str, "hwp_only"))
>  		hwp_only = 1;
> +	if (!strcmp(str, "no_acpi"))
> +		no_acpi_perf = 1;
> +
>  	return 0;
>  }
>  early_param("intel_pstate", intel_pstate_setup);

Re: [PATCH v3 5/6] Documentation: kernel_parameters for Intel P state driver

[Kristen Carlson Accardi]

On Tue, 29 Sep 2015 15:54:07 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> Added new option "no_acpi" for not using ACPI processor performance
> control objects in Intel P state driver.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Acked-by: Kristen Carlson Accardi <kristen@linux.intel.com>

> ---
>  Documentation/kernel-parameters.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 22a4b68..9b75e2a 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1546,6 +1546,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  		hwp_only
>  			Only load intel_pstate on systems which support
>  			hardware P state control (HWP) if available.
> +		no_acpi
> +			Don't use ACPI processor performance control objects
> +			_PSS and _PPC specified limits.
>  
>  	intremap=	[X86-64, Intel-IOMMU]
>  			on	enable Interrupt Remapping (default)

Re: [PATCH v3 6/6] cpufreq: intel_pstate: Avoid calculation for max/min

[Kristen Carlson Accardi]

On Tue, 29 Sep 2015 15:54:08 -0700
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> When requested from cpufreq to set policy, look into _pss and get
> control values, instead of using max/min perf calculations. These
> calculation misses next control state in boundary conditions.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

Acked-by: Kristen Carlson Accardi <kristen@linux.intel.com>

> ---
>  drivers/cpufreq/intel_pstate.c | 50 +++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index 869f074..49a16ff 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -166,6 +166,8 @@ struct perf_limits {
>  	int max_sysfs_pct;
>  	int min_policy_pct;
>  	int min_sysfs_pct;
> +	int max_perf_ctl;
> +	int min_perf_ctl;
>  };
>  
>  static struct perf_limits limits = {
> @@ -179,6 +181,8 @@ static struct perf_limits limits = {
>  	.max_sysfs_pct = 100,
>  	.min_policy_pct = 0,
>  	.min_sysfs_pct = 0,
> +	.max_perf_ctl = 0,
> +	.min_perf_ctl = 0,
>  };
>  
>  #if IS_ENABLED(CONFIG_ACPI)
> @@ -980,12 +984,23 @@ static void intel_pstate_get_min_max(struct cpudata *cpu, int *min, int *max)
>  	 * policy, or by cpu specific default values determined through
>  	 * experimentation.
>  	 */
> -	max_perf_adj = fp_toint(mul_fp(int_tofp(max_perf), limits.max_perf));
> -	*max = clamp_t(int, max_perf_adj,
> -			cpu->pstate.min_pstate, cpu->pstate.turbo_pstate);
> +	if (limits.max_perf_ctl && limits.max_sysfs_pct >=
> +						limits.max_policy_pct) {
> +		*max = limits.max_perf_ctl;
> +	} else {
> +		max_perf_adj = fp_toint(mul_fp(int_tofp(max_perf),
> +					limits.max_perf));
> +		*max = clamp_t(int, max_perf_adj, cpu->pstate.min_pstate,
> +			       cpu->pstate.turbo_pstate);
> +	}
>  
> -	min_perf = fp_toint(mul_fp(int_tofp(max_perf), limits.min_perf));
> -	*min = clamp_t(int, min_perf, cpu->pstate.min_pstate, max_perf);
> +	if (limits.min_perf_ctl) {
> +		*min = limits.min_perf_ctl;
> +	} else {
> +		min_perf = fp_toint(mul_fp(int_tofp(max_perf),
> +				    limits.min_perf));
> +		*min = clamp_t(int, min_perf, cpu->pstate.min_pstate, max_perf);
> +	}
>  }
>  
>  static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate, bool force)
> @@ -1272,6 +1287,12 @@ static unsigned int intel_pstate_get(unsigned int cpu_num)
>  
>  static int intel_pstate_set_policy(struct cpufreq_policy *policy)
>  {
> +#if IS_ENABLED(CONFIG_ACPI)
> +	struct cpudata *cpu;
> +	int i;
> +#endif
> +	pr_debug("intel_pstate: %s max %u policy->max %u\n", __func__,
> +		 policy->cpuinfo.max_freq, policy->max);
>  	if (!policy->cpuinfo.max_freq)
>  		return -ENODEV;
>  
> @@ -1284,6 +1305,8 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
>  		limits.max_perf_pct = 100;
>  		limits.max_perf = int_tofp(1);
>  		limits.no_turbo = 0;
> +		limits.max_perf_ctl = 0;
> +		limits.min_perf_ctl = 0;
>  		return 0;
>  	}
>  
> @@ -1304,6 +1327,23 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
>  	limits.min_perf = div_fp(int_tofp(limits.min_perf_pct), int_tofp(100));
>  	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
>  
> +#if IS_ENABLED(CONFIG_ACPI)
> +	cpu = all_cpu_data[policy->cpu];
> +	for (i = 0; i < cpu->acpi_perf_data.state_count; i++) {
> +		int control;
> +
> +		control = convert_to_native_pstate_format(cpu, i);
> +		if (control * cpu->pstate.scaling == policy->max)
> +			limits.max_perf_ctl = control;
> +		if (control * cpu->pstate.scaling == policy->min)
> +			limits.min_perf_ctl = control;
> +	}
> +
> +	pr_debug("intel_pstate: max %u policy_max %u perf_ctl [0x%x-0x%x]\n",
> +		 policy->cpuinfo.max_freq, policy->max, limits.min_perf_ctl,
> +		 limits.max_perf_ctl);
> +#endif
> +
>  	if (hwp_active)
>  		intel_pstate_hwp_set();
>  

Re: [PATCH v3 0/6] Intel P states enhancements

[Rafael J. Wysocki]

On Tuesday, September 29, 2015 03:54:02 PM Srinivas Pandruvada wrote:
> This series enhances Intel P state drivers with the following features:
> - When max_perf_pct is reduced in turbo range, we can change the turbo ratios
> when platform allows. This is particularly useful in limiting performance with
> HWP where whole range is turbo.
> - Use Turbo Activation Ratio, when calculating max non turbo P state. This will
> show now correct percentage in turbo range
> - To calculate busy percent, the estimate is not correct when the max non turbo
> is limited by tar, hence using physical max non turbo as before.
> - Use ACPI _PSS and _PPC in intel_ptate driver.
> - Avoid calculation for P state control value when cpufreq policy requests
> frequency limits when matched in _PSS. Sometime calculations causes reduced
> control value in boundary conditions.
> Although they are independent patches, sending as series to help applying and
> testing.
> I appreciate review and testing on multiple platforms.
> 
> v3:
> - Added function : convert_to_native_pstate_format to convert from
> perf_ctl value to pstate used ratios.
> - Fix bug identified by Rafael
> 
> v2:
> - When CONFIG_ACPI not defined, then acpi/processor.h can't be included.
> Also some variables will be unused when CONFIG_ACPI is not defined, so there
> were warnings. Fixed all these compile issues.
> 
> v1:
> - Change the command line option to "no_acpi"
> - changed kernel parameter documentation file and added "no_acpi" parameter
> - pr_debug prefixed with "intel_pstate" as suggested by Doug
> - Changed the logic to determine turbo freq in _PSS using odd/even convention,
> although this is the way it is defined in _PSS. But atleast two reviewers has
> questioned the source of this. This is defined usually in non public documents
> like BIOS writer guides. Now using the control field value to determine the
> turbo and non turbo max.
> - Fix the Kconfig dependency on ACPI for ACPI_PROCESSOR
> - multi line comment styles
> 
> v0:
> Base version
> 
> Srinivas Pandruvada (6):
>   cpufreq: intel_p_state: Fix limiting turbo sub states
>   cpufreq: intel_pstate: get P1 from TAR when available
>   cpufreq: intel-pstate: Use separate max pstate for scaling
>   cpufreq: intel_pstate: Use ACPI perf configuration
>   Documentation: kernel_parameters for Intel P state driver
>   cpufreq: intel_pstate: Avoid calculation for max/min

I'm waiting for an update of [1/6].

If any of [2-6/6] do not depend on it (ie. can be applied separately),
please let me know.

Thanks,
Rafael

Re: [PATCH v3 0/6] Intel P states enhancements

[Srinivas Pandruvada]

On Wed, 2015-10-14 at 02:52 +0200, Rafael J. Wysocki wrote:
> On Tuesday, September 29, 2015 03:54:02 PM Srinivas Pandruvada wrote:
> > This series enhances Intel P state drivers with the following
> > features:
> > - When max_perf_pct is reduced in turbo range, we can change the
> > turbo ratios
> > when platform allows. This is particularly useful in limiting
> > performance with
> > HWP where whole range is turbo.
> > - Use Turbo Activation Ratio, when calculating max non turbo P
> > state. This will
> > show now correct percentage in turbo range
> > - To calculate busy percent, the estimate is not correct when the
> > max non turbo
> > is limited by tar, hence using physical max non turbo as before.
> > - Use ACPI _PSS and _PPC in intel_ptate driver.
> > - Avoid calculation for P state control value when cpufreq policy
> > requests
> > frequency limits when matched in _PSS. Sometime calculations causes
> > reduced
> > control value in boundary conditions.
> > Although they are independent patches, sending as series to help
> > applying and
> > testing.
> > I appreciate review and testing on multiple platforms.
> > 
> > v3:
> > - Added function : convert_to_native_pstate_format to convert from
> > perf_ctl value to pstate used ratios.
> > - Fix bug identified by Rafael
> > 
> > v2:
> > - When CONFIG_ACPI not defined, then acpi/processor.h can't be
> > included.
> > Also some variables will be unused when CONFIG_ACPI is not defined,
> > so there
> > were warnings. Fixed all these compile issues.
> > 
> > v1:
> > - Change the command line option to "no_acpi"
> > - changed kernel parameter documentation file and added "no_acpi"
> > parameter
> > - pr_debug prefixed with "intel_pstate" as suggested by Doug
> > - Changed the logic to determine turbo freq in _PSS using odd/even
> > convention,
> > although this is the way it is defined in _PSS. But atleast two
> > reviewers has
> > questioned the source of this. This is defined usually in non
> > public documents
> > like BIOS writer guides. Now using the control field value to
> > determine the
> > turbo and non turbo max.
> > - Fix the Kconfig dependency on ACPI for ACPI_PROCESSOR
> > - multi line comment styles
> > 
> > v0:
> > Base version
> > 
> > Srinivas Pandruvada (6):
> >   cpufreq: intel_p_state: Fix limiting turbo sub states
> >   cpufreq: intel_pstate: get P1 from TAR when available
> >   cpufreq: intel-pstate: Use separate max pstate for scaling
> >   cpufreq: intel_pstate: Use ACPI perf configuration
> >   Documentation: kernel_parameters for Intel P state driver
> >   cpufreq: intel_pstate: Avoid calculation for max/min
> 
> I'm waiting for an update of [1/6].
> 
> If any of [2-6/6] do not depend on it (ie. can be applied
> separately),
> please let me know.
Other patches don't depend on this, but will not apply because of line
number change. I submitted v4 by removing this patch with Kristen's
ACK. I will submit this patch 1/6 separately.

Thanks,
Srinivas

> 
> Thanks,
> Rafael
> 
> --
> 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

Re: [PATCH v3 0/6] Intel P states enhancements

[Rafael J. Wysocki]

On Wednesday, October 14, 2015 04:15:41 PM Srinivas Pandruvada wrote:
> On Wed, 2015-10-14 at 02:52 +0200, Rafael J. Wysocki wrote:
> > On Tuesday, September 29, 2015 03:54:02 PM Srinivas Pandruvada wrote:
> > > This series enhances Intel P state drivers with the following
> > > features:
> > > - When max_perf_pct is reduced in turbo range, we can change the
> > > turbo ratios
> > > when platform allows. This is particularly useful in limiting
> > > performance with
> > > HWP where whole range is turbo.
> > > - Use Turbo Activation Ratio, when calculating max non turbo P
> > > state. This will
> > > show now correct percentage in turbo range
> > > - To calculate busy percent, the estimate is not correct when the
> > > max non turbo
> > > is limited by tar, hence using physical max non turbo as before.
> > > - Use ACPI _PSS and _PPC in intel_ptate driver.
> > > - Avoid calculation for P state control value when cpufreq policy
> > > requests
> > > frequency limits when matched in _PSS. Sometime calculations causes
> > > reduced
> > > control value in boundary conditions.
> > > Although they are independent patches, sending as series to help
> > > applying and
> > > testing.
> > > I appreciate review and testing on multiple platforms.
> > > 
> > > v3:
> > > - Added function : convert_to_native_pstate_format to convert from
> > > perf_ctl value to pstate used ratios.
> > > - Fix bug identified by Rafael
> > > 
> > > v2:
> > > - When CONFIG_ACPI not defined, then acpi/processor.h can't be
> > > included.
> > > Also some variables will be unused when CONFIG_ACPI is not defined,
> > > so there
> > > were warnings. Fixed all these compile issues.
> > > 
> > > v1:
> > > - Change the command line option to "no_acpi"
> > > - changed kernel parameter documentation file and added "no_acpi"
> > > parameter
> > > - pr_debug prefixed with "intel_pstate" as suggested by Doug
> > > - Changed the logic to determine turbo freq in _PSS using odd/even
> > > convention,
> > > although this is the way it is defined in _PSS. But atleast two
> > > reviewers has
> > > questioned the source of this. This is defined usually in non
> > > public documents
> > > like BIOS writer guides. Now using the control field value to
> > > determine the
> > > turbo and non turbo max.
> > > - Fix the Kconfig dependency on ACPI for ACPI_PROCESSOR
> > > - multi line comment styles
> > > 
> > > v0:
> > > Base version
> > > 
> > > Srinivas Pandruvada (6):
> > >   cpufreq: intel_p_state: Fix limiting turbo sub states
> > >   cpufreq: intel_pstate: get P1 from TAR when available
> > >   cpufreq: intel-pstate: Use separate max pstate for scaling
> > >   cpufreq: intel_pstate: Use ACPI perf configuration
> > >   Documentation: kernel_parameters for Intel P state driver
> > >   cpufreq: intel_pstate: Avoid calculation for max/min
> > 
> > I'm waiting for an update of [1/6].
> > 
> > If any of [2-6/6] do not depend on it (ie. can be applied
> > separately),
> > please let me know.
> Other patches don't depend on this, but will not apply because of line
> number change. I submitted v4 by removing this patch with Kristen's
> ACK. I will submit this patch 1/6 separately.

OK, thanks!

Rafael