[PATCH 0/5] Add P state driver for Intel Core Processors

[PATCH 0/5] Add P state driver for Intel Core Processors

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

This driver implements a scaling driver with an internal governor for
Intel Core processors.  The driver follows the same model as the
Transmeta scaling driver (longrun.c) and implements the setpolicy()
instead of target().  Scaling drivers that implement setpolicy() are
assmuned to implement internal governors by the cpufreq core. All the
logic for selecting the current P state is contained within the driver
no external governor is used by the cpufreq core.

At the moment only Intel SandyBridge processors are supported. As
testing on SandyBridge+ processors is completed support will be added
to the driver.

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 avail performance.
    
      min_perf_pct: limits the minimum P state that will be  requested by
      the driver stated as a percentage of the avail performance.
    
      no_turbo: limits the driver to selecting P states below the turbo
      frequency range.

The units for these for these files are purposely abstract and stated
in terms of available performance and not frequency.  In idea that
frequency can be set to a single frequency is a fiction for Intel Core
processors. Even if the scaling driver selects a single P state the
actual frequency the processor will run at is selected by the
processor

Changes since last version:
Dropped unneeded patches.

Rebased on bleeding edge 1071b3b

Patch 1:
    updated per Viresh's comments, added his ack.

Patch 2:
    updated description to be more informative and move the check as
    to whether the scaling driver implements target() to only effect
    whether cpufreq_out_of_sync() is called

Patch 3:
    Added Viresh's Ack

Patch 4:
    Upadted to not use unneeded local variable


Dirk Brandewie (5):
  cpufreq: Retrieve current frequency from scaling drivers with
    internal governors
  cpufreq: Only call cpufreq_out_of_sync() with drivers that implement
    cpufreq_driver.target()
  cpufreq: Do not track governor name for scaling drivers with internal
    governors.
  cpufreq_stats: do not remove sysfs files if frequency table is not
    present
  cpufreq/x86: Add P-state driver for sandy bridge.

 drivers/cpufreq/Kconfig.x86     |   18 +
 drivers/cpufreq/Makefile        |    1 +
 drivers/cpufreq/cpufreq.c       |   13 +-
 drivers/cpufreq/cpufreq_stats.c |    4 +
 drivers/cpufreq/intel_pstate.c  |  829 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 862 insertions(+), 3 deletions(-)
 create mode 100644 drivers/cpufreq/intel_pstate.c

-- 
1.7.7.6

[PATCH 1/5] cpufreq: Retrieve current frequency from scaling drivers with internal governors

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

Scaling drivers that implement the cpufreq_driver.setpolicy() versus
the cpufreq_driver.target() interface do not set policy->cur.

Normally policy->cur is set during the call to cpufreq_driver.target()
when the frequnecy request is made by the governor.

If the scaling driver implements cpufreq_driver.setpolicy() and
cpufreq_driver.get() interfaces use cpufreq_driver.get() to retrieve
the current frequency.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index ccc598a..bb45f93 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1177,9 +1177,14 @@ static void cpufreq_out_of_sync(unsigned int cpu, unsigned int old_freq,
  */
 unsigned int cpufreq_quick_get(unsigned int cpu)
 {
-	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+	struct cpufreq_policy *policy;
 	unsigned int ret_freq = 0;
 
+	if (cpufreq_driver && cpufreq_driver->setpolicy &&
+			cpufreq_driver->get)
+			return cpufreq_driver->get(cpu);
+
+	policy = cpufreq_cpu_get(cpu);
 	if (policy) {
 		ret_freq = policy->cur;
 		cpufreq_cpu_put(policy);
-- 
1.7.7.6

[PATCH 2/5] cpufreq: Only call cpufreq_out_of_sync() for driver that implement cpufreq_driver.target()

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

Scaling drivers that implement cpufreq_driver.setpolicy() have
internal governors that do not signal changes via
cpufreq_notify_transition() so the frequncy in the policy will almost
certainly be different than the current frequncy.  Only call
cpufreq_out_of_sync() when the underlying driver implements
cpufreq_driver.target()

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/cpufreq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index bb45f93..0ba0344 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1798,7 +1798,7 @@ int cpufreq_update_policy(unsigned int cpu)
 			pr_debug("Driver did not initialize current freq");
 			data->cur = policy.cur;
 		} else {
-			if (data->cur != policy.cur)
+			if (data->cur != policy.cur && driver->target)
 				cpufreq_out_of_sync(cpu, data->cur,
 								policy.cur);
 		}
-- 
1.7.7.6

[PATCH 2/5] cpufreq: Only call cpufreq_out_of_sync() with drivers that implement cpufreq_driver.target()

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

Scaling drivers that implement cpufreq_driver.setpolicy() have
internal governors that do not signal changes via
cpufreq_notify_transition() so the frequncy in the policy will almost
certainly be different than the current frequncy.  Only call
cpufreq_out_of_sync() when the underlying driver implements
cpufreq_driver.target()

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/cpufreq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index bb45f93..0ba0344 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1798,7 +1798,7 @@ int cpufreq_update_policy(unsigned int cpu)
 			pr_debug("Driver did not initialize current freq");
 			data->cur = policy.cur;
 		} else {
-			if (data->cur != policy.cur)
+			if (data->cur != policy.cur && driver->target)
 				cpufreq_out_of_sync(cpu, data->cur,
 								policy.cur);
 		}
-- 
1.7.7.6

[PATCH 3/5] cpufreq: Do not track governor name for scaling drivers with internal governors.

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

Scaling drivers that implement internal governors do not have governor
structures assocaited with them.  Only track the name of the governor
associated with the CPU if the driver does not implement
cpufreq_driver.setpolicy()

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/cpufreq/cpufreq.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 0ba0344..e772053 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1049,7 +1049,9 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif
 		__cpufreq_governor(data, CPUFREQ_GOV_STOP);
 
 #ifdef CONFIG_HOTPLUG_CPU
-	strncpy(per_cpu(cpufreq_cpu_governor, cpu), data->governor->name,
+	if (!cpufreq_driver->setpolicy)
+		strncpy(per_cpu(cpufreq_cpu_governor, cpu),
+			data->governor->name,
 			CPUFREQ_NAME_LEN);
 #endif
 
-- 
1.7.7.6

[PATCH 4/5] cpufreq_stats: do not remove sysfs files if frequency table is not present

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

The sysfs files for cpufreq_stats are created in cpufreq_stats_create_table()
called from cpufreq_stat_notifier_policy() when a policy is added to
the cpu. cpufreq_stats_create_table() will not be called if the
scaling driver does not export a frequency table to cpufreq.  Use the
same fence on tear down.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/cpufreq_stats.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index a2dee4c..2fd779e 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -179,6 +179,10 @@ static void cpufreq_stats_free_table(unsigned int cpu)
 static void cpufreq_stats_free_sysfs(unsigned int cpu)
 {
 	struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+
+	if (!cpufreq_frequency_get_table(cpu))
+		return;
+
 	if (policy && !policy_is_shared(policy)) {
 		pr_debug("%s: Free sysfs stat\n", __func__);
 		sysfs_remove_group(&policy->kobj, &stats_attr_group);
-- 
1.7.7.6

[PATCH 5/5] cpufreq/x86: Add P-state driver for sandy bridge.

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

Add a P-state driver for the Intel Sandy bridge processor. In cpufreq
terminology this driver implements a  scaling driver with an internal
governor.

When built into the the kernel this driver will be the preferred
scaling driver for Sandy bridge processors.

In addition to the interfaces provided by the cpufreq subsystem for
controlling scaling drivers. The user may control the behavior of the
driver via three sysfs files located in
"/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 avail performance.

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

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

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 drivers/cpufreq/Kconfig.x86    |   18 +
 drivers/cpufreq/Makefile       |    1 +
 drivers/cpufreq/intel_pstate.c |  829 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 848 insertions(+), 0 deletions(-)
 create mode 100644 drivers/cpufreq/intel_pstate.c

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 7227cd7..6aa7053 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -2,6 +2,24 @@
 # x86 CPU Frequency scaling drivers
 #
 
+config X86_INTEL_PSTATE
+       tristate "Intel P state control"
+       depends on X86
+       help
+          This driver provides a P state for Intel core processors.
+	  The driver implements an internal governor and will become
+          the scaling driver and governor for Sandy bridge processors.
+
+	  When this driver is enabled it will become the perferred
+          scaling driver for Sandy bridge processors.
+
+	  Note: This driver should be built with the same settings as
+	  the other scaling drivers configured into the system
+	  (module/built-in) in order for the driver to register itself
+	  as the scaling driver on the system.
+
+	  If in doubt, say N.
+
 config X86_PCC_CPUFREQ
 	tristate "Processor Clocking Control interface driver"
 	depends on ACPI && ACPI_PROCESSOR
diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
index 93610b2..cd3d4ff 100644
--- a/drivers/cpufreq/Makefile
+++ b/drivers/cpufreq/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_X86_SPEEDSTEP_SMI)		+= speedstep-smi.o
 obj-$(CONFIG_X86_SPEEDSTEP_CENTRINO)	+= speedstep-centrino.o
 obj-$(CONFIG_X86_P4_CLOCKMOD)		+= p4-clockmod.o
 obj-$(CONFIG_X86_CPUFREQ_NFORCE2)	+= cpufreq-nforce2.o
+obj-$(CONFIG_X86_INTEL_PSTATE)		+= intel_pstate.o
 
 ##################################################################################
 # ARM SoC drivers
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
new file mode 100644
index 0000000..21cee6a
--- /dev/null
+++ b/drivers/cpufreq/intel_pstate.c
@@ -0,0 +1,829 @@
+/*
+ * cpufreq_snb.c: Native P state management for Intel processors
+ *
+ * (C) Copyright 2012 Intel Corporation
+ * Author: Dirk Brandewie <dirk.j.brandewie@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/kernel.h>
+#include <linux/kernel_stat.h>
+#include <linux/module.h>
+#include <linux/ktime.h>
+#include <linux/hrtimer.h>
+#include <linux/tick.h>
+#include <linux/slab.h>
+#include <linux/sched.h>
+#include <linux/list.h>
+#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <trace/events/power.h>
+
+#include <asm/div64.h>
+#include <asm/msr.h>
+#include <asm/cpu_device_id.h>
+
+#define SAMPLE_COUNT		3
+
+#define FRAC_BITS 8
+#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;
+}
+
+static inline int32_t div_fp(int32_t x, int32_t y)
+{
+	return div_s64((int64_t)x << FRAC_BITS, (int64_t)y);
+}
+
+struct sample {
+	ktime_t start_time;
+	ktime_t end_time;
+	int core_pct_busy;
+	int pstate_pct_busy;
+	u64 duration_us;
+	u64 idletime_us;
+	u64 aperf;
+	u64 mperf;
+	int freq;
+};
+
+struct pstate_data {
+	int	current_pstate;
+	int	min_pstate;
+	int	max_pstate;
+	int	turbo_pstate;
+};
+
+struct _pid {
+	int setpoint;
+	int32_t integral;
+	int32_t p_gain;
+	int32_t i_gain;
+	int32_t d_gain;
+	int deadband;
+	int last_err;
+};
+
+struct cpudata {
+	int cpu;
+
+	char name[64];
+
+	struct timer_list timer;
+
+	struct pstate_adjust_policy *pstate_policy;
+	struct pstate_data pstate;
+	struct _pid pid;
+	struct _pid idle_pid;
+
+	int min_pstate_count;
+	int idle_mode;
+
+	ktime_t prev_sample;
+	u64	prev_idle_time_us;
+	u64	prev_aperf;
+	u64	prev_mperf;
+	int	sample_ptr;
+	struct sample samples[SAMPLE_COUNT];
+};
+
+struct cpudata **all_cpu_data;
+struct pstate_adjust_policy {
+	int sample_rate_ms;
+	int deadband;
+	int setpoint;
+	int p_gain_pct;
+	int d_gain_pct;
+	int i_gain_pct;
+};
+
+static struct pstate_adjust_policy default_policy = {
+	.sample_rate_ms = 10,
+	.deadband = 0,
+	.setpoint = 109,
+	.p_gain_pct = 17,
+	.d_gain_pct = 0,
+	.i_gain_pct = 4,
+};
+
+struct perf_limits {
+	int no_turbo;
+	int max_perf_pct;
+	int min_perf_pct;
+	int32_t max_perf;
+	int32_t min_perf;
+};
+
+static struct perf_limits limits = {
+	.no_turbo = 0,
+	.max_perf_pct = 100,
+	.max_perf = int_tofp(1),
+	.min_perf_pct = 0,
+	.min_perf = 0,
+};
+
+static inline void pid_reset(struct _pid *pid, int setpoint, int busy,
+			int deadband, int integral) {
+	pid->setpoint = setpoint;
+	pid->deadband  = deadband;
+	pid->integral  = int_tofp(integral);
+	pid->last_err  = setpoint - busy;
+}
+
+static inline void pid_p_gain_set(struct _pid *pid, int percent)
+{
+	pid->p_gain = div_fp(int_tofp(percent), int_tofp(100));
+}
+
+static inline void pid_i_gain_set(struct _pid *pid, int percent)
+{
+	pid->i_gain = div_fp(int_tofp(percent), int_tofp(100));
+}
+
+static inline void pid_d_gain_set(struct _pid *pid, int percent)
+{
+
+	pid->d_gain = div_fp(int_tofp(percent), int_tofp(100));
+}
+
+static  signed int pid_calc(struct _pid *pid, int busy)
+{
+	signed int err, result;
+	int32_t pterm, dterm, fp_error;
+	int32_t integral_limit;
+
+
+	err = pid->setpoint - busy;
+	fp_error = int_tofp(err);
+
+	if (abs(err) <= pid->deadband)
+		return 0;
+
+	pterm = mul_fp(pid->p_gain, fp_error);
+
+	pid->integral += fp_error;
+
+	/* limit the integral term */
+	integral_limit = int_tofp(30);
+	if (pid->integral > integral_limit)
+		pid->integral = integral_limit;
+	if (pid->integral < -integral_limit)
+		pid->integral = -integral_limit;
+
+	dterm = mul_fp(pid->d_gain, (err - pid->last_err));
+	pid->last_err = err;
+
+	result = pterm + mul_fp(pid->integral, pid->i_gain) + dterm;
+
+	return (signed int)fp_toint(result);
+}
+
+
+static inline void intel_pstate_busy_pid_reset(struct cpudata *cpu)
+{
+	pid_p_gain_set(&cpu->pid, cpu->pstate_policy->p_gain_pct);
+	pid_d_gain_set(&cpu->pid, cpu->pstate_policy->d_gain_pct);
+	pid_i_gain_set(&cpu->pid, cpu->pstate_policy->i_gain_pct);
+
+	pid_reset(&cpu->pid,
+		cpu->pstate_policy->setpoint,
+		100,
+		cpu->pstate_policy->deadband,
+		0);
+
+}
+
+static inline void intel_pstate_idle_pid_reset(struct cpudata *cpu)
+{
+	pid_p_gain_set(&cpu->idle_pid, cpu->pstate_policy->p_gain_pct);
+	pid_d_gain_set(&cpu->idle_pid, cpu->pstate_policy->d_gain_pct);
+	pid_i_gain_set(&cpu->idle_pid, cpu->pstate_policy->i_gain_pct);
+
+	pid_reset(&cpu->idle_pid,
+		75,
+		50,
+		cpu->pstate_policy->deadband,
+		0);
+}
+
+static inline void intel_pstate_reset_all_pid(void)
+{
+	unsigned int cpu;
+	for_each_online_cpu(cpu) {
+		if (all_cpu_data[cpu])
+			intel_pstate_busy_pid_reset(all_cpu_data[cpu]);
+	}
+}
+
+/************************** debugfs begin ************************/
+static int pid_param_set(void *data, u64 val)
+{
+	*(u32 *)data = val;
+	intel_pstate_reset_all_pid();
+	return 0;
+}
+static int pid_param_get(void *data, u64 *val)
+{
+	*val = *(u32 *)data;
+	return 0;
+}
+DEFINE_SIMPLE_ATTRIBUTE(fops_pid_param, pid_param_get,
+			pid_param_set, "%llu\n");
+
+struct pid_param {
+	char *name;
+	void *value;
+};
+
+static struct pid_param pid_files[] = {
+	{"sample_rate_ms", &default_policy.sample_rate_ms},
+	{"d_gain_pct", &default_policy.d_gain_pct},
+	{"i_gain_pct", &default_policy.i_gain_pct},
+	{"deadband", &default_policy.deadband},
+	{"setpoint", &default_policy.setpoint},
+	{"p_gain_pct", &default_policy.p_gain_pct},
+	{NULL, NULL}
+};
+
+struct dentry *debugfs_parent;
+void intel_pstate_debug_expose_params(void)
+{
+	int i = 0;
+
+	debugfs_parent = debugfs_create_dir("pstate_snb", NULL);
+	if (IS_ERR_OR_NULL(debugfs_parent))
+		return;
+	while (pid_files[i].name) {
+		debugfs_create_file(pid_files[i].name, 0660,
+				debugfs_parent, pid_files[i].value,
+				&fops_pid_param);
+		i++;
+	}
+
+}
+
+/************************** debugfs end ************************/
+
+/************************** sysfs begin ************************/
+#define show_one(file_name, object)					\
+	static ssize_t show_##file_name					\
+	(struct kobject *kobj, struct attribute *attr, char *buf)	\
+	{								\
+		return sprintf(buf, "%u\n", limits.object);	\
+	}
+
+static ssize_t store_no_turbo(struct kobject *a, struct attribute *b,
+				const char *buf, size_t count)
+{
+	unsigned int input;
+	int ret;
+	ret = sscanf(buf, "%u", &input);
+	if (ret != 1)
+		return -EINVAL;
+	limits.no_turbo =  clamp_t(int, input, 0 , 1);
+
+	return count;
+}
+
+static ssize_t store_max_perf_pct(struct kobject *a, struct attribute *b,
+				const char *buf, size_t count)
+{
+	unsigned int input;
+	int ret;
+	ret = sscanf(buf, "%u", &input);
+	if (ret != 1)
+		return -EINVAL;
+
+	limits.max_perf_pct = clamp_t(int, input, 0 , 100);
+	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
+	return count;
+}
+
+static ssize_t store_min_perf_pct(struct kobject *a, struct attribute *b,
+				const char *buf, size_t count)
+{
+	unsigned int input;
+	int ret;
+	ret = sscanf(buf, "%u", &input);
+	if (ret != 1)
+		return -EINVAL;
+	limits.min_perf_pct = clamp_t(int, input, 0 , 100);
+	limits.min_perf = div_fp(int_tofp(limits.min_perf_pct), int_tofp(100));
+
+	return count;
+}
+
+show_one(no_turbo, no_turbo);
+show_one(max_perf_pct, max_perf_pct);
+show_one(min_perf_pct, min_perf_pct);
+
+
+define_one_global_rw(no_turbo);
+define_one_global_rw(max_perf_pct);
+define_one_global_rw(min_perf_pct);
+
+
+static struct attribute *intel_pstate_attributes[] = {
+	&no_turbo.attr,
+	&max_perf_pct.attr,
+	&min_perf_pct.attr,
+	NULL
+};
+
+static struct attribute_group intel_pstate_attr_group = {
+	.attrs = intel_pstate_attributes,
+};
+static struct kobject *intel_pstate_kobject;
+
+void intel_pstate_sysfs_expose_params(void)
+{
+	int rc;
+
+	intel_pstate_kobject = kobject_create_and_add("intel_pstate",
+						&cpu_subsys.dev_root->kobj);
+	BUG_ON(!intel_pstate_kobject);
+	rc = sysfs_create_group(intel_pstate_kobject,
+				&intel_pstate_attr_group);
+	BUG_ON(rc);
+}
+
+
+/************************** sysfs end ************************/
+
+static int intel_pstate_min_pstate(void)
+{
+	u64 value;
+	rdmsrl(0xCE, value);
+	return (value >> 40) & 0xFF;
+}
+
+static int intel_pstate_max_pstate(void)
+{
+	u64 value;
+	rdmsrl(0xCE, value);
+	return (value >> 8) & 0xFF;
+}
+
+static int intel_pstate_turbo_pstate(void)
+{
+	u64 value;
+	int nont, ret;
+	rdmsrl(0x1AD, value);
+	nont = intel_pstate_max_pstate();
+	ret = ((value) & 255);
+	if (ret <= nont)
+		ret = nont;
+	return ret;
+}
+
+static void intel_pstate_get_min_max(struct cpudata *cpu, int *min, int *max)
+{
+	int max_perf = cpu->pstate.turbo_pstate;
+	int min_perf;
+	if (limits.no_turbo)
+		max_perf = cpu->pstate.max_pstate;
+
+	max_perf = fp_toint(mul_fp(int_tofp(max_perf), limits.max_perf));
+	*max = clamp_t(int, max_perf,
+			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);
+}
+
+
+static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
+{
+	int max_perf, min_perf;
+
+	intel_pstate_get_min_max(cpu, &min_perf, &max_perf);
+
+	pstate = clamp_t(int, pstate, min_perf, max_perf);
+
+	if (pstate == cpu->pstate.current_pstate)
+		return;
+
+
+#ifndef MODULE
+	trace_cpu_frequency(pstate * 100000, cpu->cpu);
+#endif
+	cpu->pstate.current_pstate = pstate;
+	wrmsrl(MSR_IA32_PERF_CTL, pstate << 8);
+
+}
+
+static inline void intel_pstate_pstate_increase(struct cpudata *cpu, int steps)
+{
+	int target;
+	target = cpu->pstate.current_pstate + steps;
+
+	intel_pstate_set_pstate(cpu, target);
+}
+
+static inline void intel_pstate_pstate_decrease(struct cpudata *cpu, int steps)
+{
+	int target;
+	target = cpu->pstate.current_pstate - steps;
+	intel_pstate_set_pstate(cpu, target);
+}
+
+static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
+{
+	sprintf(cpu->name, "Intel 2nd generation core");
+
+	cpu->pstate.min_pstate = intel_pstate_min_pstate();
+	cpu->pstate.max_pstate = intel_pstate_max_pstate();
+	cpu->pstate.turbo_pstate = intel_pstate_turbo_pstate();
+
+	/* goto max pstate so we don't slow up boot if we are built-in
+	   if we are a module we will take care of it during normal
+	   operation
+	*/
+	intel_pstate_set_pstate(cpu, cpu->pstate.max_pstate);
+}
+
+
+static inline void intel_pstate_calc_busy(struct cpudata *cpu,
+					struct sample *sample)
+{
+	u64 core_pct;
+	sample->pstate_pct_busy = 100 - div64_u64(
+					sample->idletime_us * 100,
+					sample->duration_us);
+	core_pct = div64_u64(sample->aperf * 100, sample->mperf);
+	sample->freq = cpu->pstate.turbo_pstate * core_pct * 1000;
+
+	sample->core_pct_busy = sample->pstate_pct_busy * core_pct / 100;
+}
+
+static inline int intel_pstate_sample(struct cpudata *cpu)
+{
+	ktime_t now;
+	u64 idle_time_us;
+	u64 aperf, mperf;
+
+	now = ktime_get();
+	idle_time_us = get_cpu_idle_time_us(cpu->cpu, NULL);
+
+	rdmsrl(MSR_IA32_APERF, aperf);
+	rdmsrl(MSR_IA32_MPERF, mperf);
+	/* for the first sample, don't actually record a sample, just
+	 * set the baseline */
+	if (cpu->prev_idle_time_us > 0) {
+		cpu->sample_ptr = (cpu->sample_ptr + 1) % SAMPLE_COUNT;
+		cpu->samples[cpu->sample_ptr].start_time = cpu->prev_sample;
+		cpu->samples[cpu->sample_ptr].end_time = now;
+		cpu->samples[cpu->sample_ptr].duration_us =
+			ktime_us_delta(now, cpu->prev_sample);
+		cpu->samples[cpu->sample_ptr].idletime_us =
+			idle_time_us - cpu->prev_idle_time_us;
+
+		cpu->samples[cpu->sample_ptr].aperf = aperf;
+		cpu->samples[cpu->sample_ptr].mperf = mperf;
+		cpu->samples[cpu->sample_ptr].aperf -= cpu->prev_aperf;
+		cpu->samples[cpu->sample_ptr].mperf -= cpu->prev_mperf;
+
+		intel_pstate_calc_busy(cpu, &cpu->samples[cpu->sample_ptr]);
+	}
+
+	cpu->prev_sample = now;
+	cpu->prev_idle_time_us = idle_time_us;
+	cpu->prev_aperf = aperf;
+	cpu->prev_mperf = mperf;
+	return cpu->sample_ptr;
+}
+
+static inline void intel_pstate_set_sample_time(struct cpudata *cpu)
+{
+	int sample_time;
+	int delay;
+
+	sample_time = cpu->pstate_policy->sample_rate_ms;
+	delay = msecs_to_jiffies(sample_time);
+	delay -= jiffies % delay;
+	mod_timer_pinned(&cpu->timer, jiffies + delay);
+}
+
+static inline void intel_pstate_idle_mode(struct cpudata *cpu)
+{
+	cpu->idle_mode = 1;
+}
+
+static inline void intel_pstate_normal_mode(struct cpudata *cpu)
+{
+	cpu->idle_mode = 0;
+}
+
+static inline int intel_pstate_get_scaled_busy(struct cpudata *cpu)
+{
+	int32_t busy_scaled;
+	int32_t core_busy, turbo_pstate, current_pstate;
+
+	core_busy    = int_tofp(cpu->samples[cpu->sample_ptr].core_pct_busy);
+	turbo_pstate   = int_tofp(cpu->pstate.turbo_pstate);
+	current_pstate = int_tofp(cpu->pstate.current_pstate);
+	busy_scaled = mul_fp(core_busy, div_fp(turbo_pstate, current_pstate));
+
+
+	return fp_toint(busy_scaled);
+}
+
+static inline void intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
+{
+	int busy_scaled;
+	struct _pid *pid;
+	signed int ctl = 0;
+	int steps;
+
+	pid = &cpu->pid;
+	busy_scaled =  intel_pstate_get_scaled_busy(cpu);
+
+	ctl = pid_calc(pid, busy_scaled);
+
+	steps = abs(ctl);
+	if (ctl < 0)
+		intel_pstate_pstate_increase(cpu, steps);
+	else
+		intel_pstate_pstate_decrease(cpu, steps);
+}
+
+static inline void intel_pstate_adjust_idle_pstate(struct cpudata *cpu)
+{
+	int busy_scaled;
+	struct _pid *pid;
+	int ctl = 0;
+	int steps;
+
+	pid = &cpu->idle_pid;
+
+	busy_scaled =  intel_pstate_get_scaled_busy(cpu);
+
+	ctl = pid_calc(pid, 100 - busy_scaled);
+
+	steps = abs(ctl);
+	if (ctl < 0)
+		intel_pstate_pstate_decrease(cpu, steps);
+	else
+		intel_pstate_pstate_increase(cpu, steps);
+
+	if (cpu->pstate.current_pstate == cpu->pstate.min_pstate)
+		intel_pstate_normal_mode(cpu);
+}
+
+static void intel_pstate_timer_func(unsigned long __data)
+{
+	struct cpudata *cpu = (struct cpudata *) __data;
+	int idx;
+
+
+	idx = intel_pstate_sample(cpu);
+
+
+	if (!cpu->idle_mode)
+		intel_pstate_adjust_busy_pstate(cpu);
+	else
+		intel_pstate_adjust_idle_pstate(cpu);
+
+#if defined(XPERF_FIX)
+	if (cpu->pstate.current_pstate == cpu->pstate.min_pstate) {
+		cpu->min_pstate_count++;
+		if (!(cpu->min_pstate_count % 5)) {
+			intel_pstate_set_pstate(cpu, cpu->pstate.max_pstate);
+			intel_pstate_idle_mode(cpu);
+		}
+	} else
+		cpu->min_pstate_count = 0;
+#endif
+	intel_pstate_set_sample_time(cpu);
+}
+
+#define ICPU(model, policy) \
+	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, (unsigned long)&policy }
+
+static const struct x86_cpu_id intel_pstate_cpu_ids[] = {
+	ICPU(0x2a, default_policy),
+	ICPU(0x2d, default_policy),
+	{}
+};
+MODULE_DEVICE_TABLE(x86cpu, intel_pstate_cpu_ids);
+
+static int intel_pstate_init_cpu(unsigned int cpunum)
+{
+
+	const struct x86_cpu_id *id;
+	struct cpudata *cpu;
+
+	id = x86_match_cpu(intel_pstate_cpu_ids);
+	if (!id)
+		return -ENODEV;
+
+	all_cpu_data[cpunum] = kzalloc(sizeof(struct cpudata), GFP_KERNEL);
+	if (!all_cpu_data[cpunum])
+		return -ENOMEM;
+
+	cpu = all_cpu_data[cpunum];
+
+	intel_pstate_get_cpu_pstates(cpu);
+
+	cpu->cpu = cpunum;
+	cpu->pstate_policy =
+		(struct pstate_adjust_policy *)id->driver_data;
+	init_timer_deferrable(&cpu->timer);
+	cpu->timer.function = intel_pstate_timer_func;
+	cpu->timer.data =
+		(unsigned long)cpu;
+	cpu->timer.expires = jiffies + HZ/100;
+	intel_pstate_busy_pid_reset(cpu);
+	intel_pstate_idle_pid_reset(cpu);
+	intel_pstate_sample(cpu);
+	intel_pstate_set_pstate(cpu, cpu->pstate.max_pstate);
+
+	add_timer_on(&cpu->timer, cpunum);
+
+	pr_info("Intel pstate controlling:  cpu %d\n", cpunum);
+
+	return 0;
+}
+
+unsigned int intel_pstate_get(unsigned int cpu_num)
+{
+	struct sample *sample;
+	struct cpudata *cpu;
+
+	cpu = all_cpu_data[cpu_num];
+	if (!cpu)
+		return 0;
+	sample = &cpu->samples[cpu->sample_ptr];
+	return sample->freq;
+}
+
+static int intel_pstate_set_policy(struct cpufreq_policy *policy)
+{
+	struct cpudata *cpu;
+	int min, max;
+
+	cpu = all_cpu_data[policy->cpu];
+
+	intel_pstate_get_min_max(cpu, &min, &max);
+
+	limits.min_perf_pct = (policy->min * 100) / policy->cpuinfo.max_freq;
+	limits.min_perf_pct = clamp_t(int, limits.min_perf_pct, 0 , 100);
+	limits.min_perf = div_fp(int_tofp(limits.min_perf_pct), int_tofp(100));
+
+
+	limits.max_perf_pct = policy->max * 100 / policy->cpuinfo.max_freq;
+	limits.max_perf_pct = clamp_t(int, limits.max_perf_pct, 0 , 100);
+	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));
+
+
+
+	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
+		limits.min_perf_pct = 100;
+		limits.min_perf = int_tofp(1);
+		limits.max_perf_pct = 100;
+		limits.max_perf = int_tofp(1);
+		limits.no_turbo = 0;
+	}
+
+	return 0;
+}
+
+static int intel_pstate_verify_policy(struct cpufreq_policy *policy)
+{
+	cpufreq_verify_within_limits(policy,
+				policy->cpuinfo.min_freq,
+				policy->cpuinfo.max_freq);
+
+	if ((policy->policy != CPUFREQ_POLICY_POWERSAVE) &&
+		(policy->policy != CPUFREQ_POLICY_PERFORMANCE))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int __cpuinit intel_pstate_cpu_exit(struct cpufreq_policy *policy)
+{
+	int cpu = policy->cpu;
+
+	del_timer(&all_cpu_data[cpu]->timer);
+	kfree(all_cpu_data[cpu]);
+	all_cpu_data[cpu] = NULL;
+	return 0;
+}
+
+static int __cpuinit intel_pstate_cpu_init(struct cpufreq_policy *policy)
+{
+	int rc, min_pstate, max_pstate;
+	struct cpudata *cpu;
+
+	rc = intel_pstate_init_cpu(policy->cpu);
+	if (rc)
+		return rc;
+
+	cpu = all_cpu_data[policy->cpu];
+
+	if (!limits.no_turbo &&
+		limits.min_perf_pct == 100 && limits.max_perf_pct == 100)
+		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
+	else
+		policy->policy = CPUFREQ_POLICY_POWERSAVE;
+
+	intel_pstate_get_min_max(cpu, &min_pstate, &max_pstate);
+	policy->min = min_pstate * 100000;
+	policy->max = max_pstate * 100000;
+
+	/* cpuinfo and default policy values */
+	policy->cpuinfo.min_freq = cpu->pstate.min_pstate * 100000;
+	policy->cpuinfo.max_freq = cpu->pstate.turbo_pstate * 100000;
+	policy->cpuinfo.transition_latency = CPUFREQ_ETERNAL;
+	cpumask_set_cpu(policy->cpu, policy->cpus);
+
+	return 0;
+}
+
+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,
+	.name		= "intel_pstate",
+	.owner		= THIS_MODULE,
+};
+
+
+static void intel_pstate_exit(void)
+{
+	int cpu;
+
+
+	sysfs_remove_group(intel_pstate_kobject,
+				&intel_pstate_attr_group);
+	debugfs_remove_recursive(debugfs_parent);
+
+	cpufreq_unregister_driver(&intel_pstate_driver);
+
+	if (!all_cpu_data)
+		return;
+
+	get_online_cpus();
+	for_each_online_cpu(cpu) {
+		if (all_cpu_data[cpu]) {
+			del_timer_sync(&all_cpu_data[cpu]->timer);
+			kfree(all_cpu_data[cpu]);
+		}
+	}
+
+	put_online_cpus();
+	vfree(all_cpu_data);
+}
+
+static int __init intel_pstate_init(void)
+{
+	int rc = 0;
+
+	const struct x86_cpu_id *id;
+
+	id = x86_match_cpu(intel_pstate_cpu_ids);
+	if (!id)
+		return -ENODEV;
+
+	pr_info("Intel P-state driver initializing.\n");
+
+	all_cpu_data = vmalloc(sizeof(void *) * num_possible_cpus());
+	if (!all_cpu_data)
+		return -ENOMEM;
+	memset(all_cpu_data, 0, sizeof(void *) * num_possible_cpus());
+
+	rc = cpufreq_register_driver(&intel_pstate_driver);
+	if (rc)
+		goto out;
+
+	intel_pstate_debug_expose_params();
+	intel_pstate_sysfs_expose_params();
+	return rc;
+out:
+	intel_pstate_exit();
+	return -ENODEV;
+}
+
+MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
+MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
+MODULE_LICENSE("GPL");
+
+device_initcall(intel_pstate_init);
+module_exit(intel_pstate_exit);
-- 
1.7.7.6

Re: [PATCH 2/5] cpufreq: Only call cpufreq_out_of_sync() with drivers that implement cpufreq_driver.target()

[Viresh Kumar]

On Wed, Feb 6, 2013 at 10:32 PM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> Scaling drivers that implement cpufreq_driver.setpolicy() have
> internal governors that do not signal changes via
> cpufreq_notify_transition() so the frequncy in the policy will almost
> certainly be different than the current frequncy.  Only call
> cpufreq_out_of_sync() when the underlying driver implements
> cpufreq_driver.target()
>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  drivers/cpufreq/cpufreq.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index bb45f93..0ba0344 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1798,7 +1798,7 @@ int cpufreq_update_policy(unsigned int cpu)
>                         pr_debug("Driver did not initialize current freq");
>                         data->cur = policy.cur;
>                 } else {
> -                       if (data->cur != policy.cur)
> +                       if (data->cur != policy.cur && driver->target)
>                                 cpufreq_out_of_sync(cpu, data->cur,
>                                                                 policy.cur);

Looks much better now :)

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Viresh Kumar]

On Wed, Feb 6, 2013 at 10:32 PM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> This driver implements a scaling driver with an internal governor for
> Intel Core processors.  The driver follows the same model as the
> Transmeta scaling driver (longrun.c) and implements the setpolicy()
> instead of target().  Scaling drivers that implement setpolicy() are
> assmuned to implement internal governors by the cpufreq core. All the
> logic for selecting the current P state is contained within the driver
> no external governor is used by the cpufreq core.

Hi Dirk,

All look good now :)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

@Rafael: I wasn't sure if these would apply over my patches without any
conflicts and so i applied them in my repo. Added my Acked-by's and fixed
a minor thing in one of the patches (as mentioned in the other mail).

You can pick these from: dirk-patches

http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/dirk-patches

Re: [PATCH 3/5] cpufreq: Do not track governor name for scaling drivers with internal governors.

[Viresh Kumar]

On Wed, Feb 6, 2013 at 10:32 PM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> Scaling drivers that implement internal governors do not have governor
> structures assocaited with them.  Only track the name of the governor
> associated with the CPU if the driver does not implement
> cpufreq_driver.setpolicy()
>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/cpufreq/cpufreq.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 0ba0344..e772053 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1049,7 +1049,9 @@ static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif
>                 __cpufreq_governor(data, CPUFREQ_GOV_STOP);
>
>  #ifdef CONFIG_HOTPLUG_CPU
> -       strncpy(per_cpu(cpufreq_cpu_governor, cpu), data->governor->name,
> +       if (!cpufreq_driver->setpolicy)
> +               strncpy(per_cpu(cpufreq_cpu_governor, cpu),
> +                       data->governor->name,
>                         CPUFREQ_NAME_LEN);

While applying this patch in my repo, i have merged above two lines into a
single line.

Re: [PATCH 3/5] cpufreq: Do not track governor name for scaling drivers with internal governors.

[Viresh Kumar]

On Wed, Feb 6, 2013 at 10:32 PM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> Scaling drivers that implement internal governors do not have governor
> structures assocaited with them.  Only track the name of the governor
> associated with the CPU if the driver does not implement
> cpufreq_driver.setpolicy()
>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

To fix sparse warning updated this patch as:

commit ea241d95152c535c3d261f3a2240c45480b040bc
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Thu Feb 7 19:13:43 2013 +0530

    fixup! cpufreq: Do not track governor name for scaling drivers
with internal governors.
---
 drivers/cpufreq/cpufreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index e140b81..819aa33 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1039,7 +1039,7 @@ static int __cpufreq_remove_dev(struct device
*dev, struct subsys_interface *sif
                __cpufreq_governor(data, CPUFREQ_GOV_STOP);

 #ifdef CONFIG_HOTPLUG_CPU
-       if (!cpufreq_driver->setpolicy)
+       if (!driver->setpolicy)
                strncpy(per_cpu(cpufreq_cpu_governor, cpu),
                        data->governor->name, CPUFREQ_NAME_LEN);
 #endif

Re: [PATCH 1/5] cpufreq: Retrieve current frequency from scaling drivers with internal governors

[Viresh Kumar]

On Wed, Feb 6, 2013 at 10:32 PM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> Scaling drivers that implement the cpufreq_driver.setpolicy() versus
> the cpufreq_driver.target() interface do not set policy->cur.
>
> Normally policy->cur is set during the call to cpufreq_driver.target()
> when the frequnecy request is made by the governor.
>
> If the scaling driver implements cpufreq_driver.setpolicy() and
> cpufreq_driver.get() interfaces use cpufreq_driver.get() to retrieve
> the current frequency.
>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

To fix sparse warning updated this patch as:

commit f6060fa13ace2da4e691233706a788f91ad453da
Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Thu Feb 7 19:12:16 2013 +0530

    fixup! cpufreq: Retrieve current frequency from scaling drivers
with internal governors
---
 drivers/cpufreq/cpufreq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 447b313..e140b81 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1173,12 +1173,12 @@ static void cpufreq_out_of_sync(unsigned int
cpu, unsigned int old_freq,
  */
 unsigned int cpufreq_quick_get(unsigned int cpu)
 {
+       struct cpufreq_driver *driver = rcu_dereference(cpufreq_driver);
        struct cpufreq_policy *policy;
        unsigned int ret_freq = 0;

-       if (cpufreq_driver && cpufreq_driver->setpolicy &&
-                       cpufreq_driver->get)
-                       return cpufreq_driver->get(cpu);
+       if (driver && driver->setpolicy && driver->get)
+                       return driver->get(cpu);

        policy = cpufreq_cpu_get(cpu);
        if (policy) {

Re: [PATCH 5/5] cpufreq/x86: Add P-state driver for sandy bridge.

[Viresh Kumar]

On Wed, Feb 6, 2013 at 10:32 PM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> Add a P-state driver for the Intel Sandy bridge processor. In cpufreq
> terminology this driver implements a  scaling driver with an internal
> governor.
>
> When built into the the kernel this driver will be the preferred
> scaling driver for Sandy bridge processors.

I have got some sparse warning fixes + generic cleanup (formatting related).

From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Thu, 7 Feb 2013 19:27:50 +0530
Subject: [PATCH] fixup! cpufreq/x86: Add P-state driver for sandy bridge.

---
 drivers/cpufreq/intel_pstate.c | 65 ++++++++++++++----------------------------
 1 file changed, 21 insertions(+), 44 deletions(-)

diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 21cee6a..6c225d7 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -99,7 +99,7 @@ struct cpudata {
 	struct sample samples[SAMPLE_COUNT];
 };

-struct cpudata **all_cpu_data;
+static struct cpudata **all_cpu_data;
 struct pstate_adjust_policy {
 	int sample_rate_ms;
 	int deadband;
@@ -158,13 +158,12 @@ static inline void pid_d_gain_set(struct _pid
*pid, int percent)
 	pid->d_gain = div_fp(int_tofp(percent), int_tofp(100));
 }

-static  signed int pid_calc(struct _pid *pid, int busy)
+static signed int pid_calc(struct _pid *pid, int busy)
 {
 	signed int err, result;
 	int32_t pterm, dterm, fp_error;
 	int32_t integral_limit;

-
 	err = pid->setpoint - busy;
 	fp_error = int_tofp(err);

@@ -190,7 +189,6 @@ static  signed int pid_calc(struct _pid *pid, int busy)
 	return (signed int)fp_toint(result);
 }

-
 static inline void intel_pstate_busy_pid_reset(struct cpudata *cpu)
 {
 	pid_p_gain_set(&cpu->pid, cpu->pstate_policy->p_gain_pct);
@@ -202,7 +200,6 @@ static inline void
intel_pstate_busy_pid_reset(struct cpudata *cpu)
 		100,
 		cpu->pstate_policy->deadband,
 		0);
-
 }

 static inline void intel_pstate_idle_pid_reset(struct cpudata *cpu)
@@ -257,8 +254,8 @@ static struct pid_param pid_files[] = {
 	{NULL, NULL}
 };

-struct dentry *debugfs_parent;
-void intel_pstate_debug_expose_params(void)
+static struct dentry *debugfs_parent;
+static void intel_pstate_debug_expose_params(void)
 {
 	int i = 0;

@@ -271,7 +268,6 @@ void intel_pstate_debug_expose_params(void)
 				&fops_pid_param);
 		i++;
 	}
-
 }

 /************************** debugfs end ************************/
@@ -281,7 +277,7 @@ void intel_pstate_debug_expose_params(void)
 	static ssize_t show_##file_name					\
 	(struct kobject *kobj, struct attribute *attr, char *buf)	\
 	{								\
-		return sprintf(buf, "%u\n", limits.object);	\
+		return sprintf(buf, "%u\n", limits.object);		\
 	}

 static ssize_t store_no_turbo(struct kobject *a, struct attribute *b,
@@ -292,7 +288,7 @@ static ssize_t store_no_turbo(struct kobject *a,
struct attribute *b,
 	ret = sscanf(buf, "%u", &input);
 	if (ret != 1)
 		return -EINVAL;
-	limits.no_turbo =  clamp_t(int, input, 0 , 1);
+	limits.no_turbo = clamp_t(int, input, 0 , 1);

 	return count;
 }
@@ -329,12 +325,10 @@ show_one(no_turbo, no_turbo);
 show_one(max_perf_pct, max_perf_pct);
 show_one(min_perf_pct, min_perf_pct);

-
 define_one_global_rw(no_turbo);
 define_one_global_rw(max_perf_pct);
 define_one_global_rw(min_perf_pct);

-
 static struct attribute *intel_pstate_attributes[] = {
 	&no_turbo.attr,
 	&max_perf_pct.attr,
@@ -347,7 +341,7 @@ static struct attribute_group intel_pstate_attr_group = {
 };
 static struct kobject *intel_pstate_kobject;

-void intel_pstate_sysfs_expose_params(void)
+static void intel_pstate_sysfs_expose_params(void)
 {
 	int rc;

@@ -359,7 +353,6 @@ void intel_pstate_sysfs_expose_params(void)
 	BUG_ON(rc);
 }

-
 /************************** sysfs end ************************/

 static int intel_pstate_min_pstate(void)
@@ -404,7 +397,6 @@ static void intel_pstate_get_min_max(struct
cpudata *cpu, int *min, int *max)
 			cpu->pstate.min_pstate, max_perf);
 }

-
 static void intel_pstate_set_pstate(struct cpudata *cpu, int pstate)
 {
 	int max_perf, min_perf;
@@ -416,7 +408,6 @@ static void intel_pstate_set_pstate(struct cpudata
*cpu, int pstate)
 	if (pstate == cpu->pstate.current_pstate)
 		return;

-
 #ifndef MODULE
 	trace_cpu_frequency(pstate * 100000, cpu->cpu);
 #endif
@@ -448,14 +439,13 @@ static void intel_pstate_get_cpu_pstates(struct
cpudata *cpu)
 	cpu->pstate.max_pstate = intel_pstate_max_pstate();
 	cpu->pstate.turbo_pstate = intel_pstate_turbo_pstate();

-	/* goto max pstate so we don't slow up boot if we are built-in
-	   if we are a module we will take care of it during normal
-	   operation
-	*/
+	/*
+	 * goto max pstate so we don't slow up boot if we are built-in if we are
+	 * a module we will take care of it during normal operation
+	 */
 	intel_pstate_set_pstate(cpu, cpu->pstate.max_pstate);
 }

-
 static inline void intel_pstate_calc_busy(struct cpudata *cpu,
 					struct sample *sample)
 {
@@ -508,8 +498,7 @@ static inline int intel_pstate_sample(struct cpudata *cpu)

 static inline void intel_pstate_set_sample_time(struct cpudata *cpu)
 {
-	int sample_time;
-	int delay;
+	int sample_time, delay;

 	sample_time = cpu->pstate_policy->sample_rate_ms;
 	delay = msecs_to_jiffies(sample_time);
@@ -532,12 +521,11 @@ static inline int
intel_pstate_get_scaled_busy(struct cpudata *cpu)
 	int32_t busy_scaled;
 	int32_t core_busy, turbo_pstate, current_pstate;

-	core_busy    = int_tofp(cpu->samples[cpu->sample_ptr].core_pct_busy);
-	turbo_pstate   = int_tofp(cpu->pstate.turbo_pstate);
+	core_busy = int_tofp(cpu->samples[cpu->sample_ptr].core_pct_busy);
+	turbo_pstate = int_tofp(cpu->pstate.turbo_pstate);
 	current_pstate = int_tofp(cpu->pstate.current_pstate);
 	busy_scaled = mul_fp(core_busy, div_fp(turbo_pstate, current_pstate));

-
 	return fp_toint(busy_scaled);
 }

@@ -549,7 +537,7 @@ static inline void
intel_pstate_adjust_busy_pstate(struct cpudata *cpu)
 	int steps;

 	pid = &cpu->pid;
-	busy_scaled =  intel_pstate_get_scaled_busy(cpu);
+	busy_scaled = intel_pstate_get_scaled_busy(cpu);

 	ctl = pid_calc(pid, busy_scaled);

@@ -569,7 +557,7 @@ static inline void
intel_pstate_adjust_idle_pstate(struct cpudata *cpu)

 	pid = &cpu->idle_pid;

-	busy_scaled =  intel_pstate_get_scaled_busy(cpu);
+	busy_scaled = intel_pstate_get_scaled_busy(cpu);

 	ctl = pid_calc(pid, 100 - busy_scaled);

@@ -586,11 +574,7 @@ static inline void
intel_pstate_adjust_idle_pstate(struct cpudata *cpu)
 static void intel_pstate_timer_func(unsigned long __data)
 {
 	struct cpudata *cpu = (struct cpudata *) __data;
-	int idx;
-
-
-	idx = intel_pstate_sample(cpu);
-
+	int idx = intel_pstate_sample(cpu);

 	if (!cpu->idle_mode)
 		intel_pstate_adjust_busy_pstate(cpu);
@@ -653,12 +637,12 @@ static int intel_pstate_init_cpu(unsigned int cpunum)

 	add_timer_on(&cpu->timer, cpunum);

-	pr_info("Intel pstate controlling:  cpu %d\n", cpunum);
+	pr_info("Intel pstate controlling: cpu %d\n", cpunum);

 	return 0;
 }

-unsigned int intel_pstate_get(unsigned int cpu_num)
+static unsigned int intel_pstate_get(unsigned int cpu_num)
 {
 	struct sample *sample;
 	struct cpudata *cpu;
@@ -683,13 +667,10 @@ static int intel_pstate_set_policy(struct
cpufreq_policy *policy)
 	limits.min_perf_pct = clamp_t(int, limits.min_perf_pct, 0 , 100);
 	limits.min_perf = div_fp(int_tofp(limits.min_perf_pct), int_tofp(100));

-
 	limits.max_perf_pct = policy->max * 100 / policy->cpuinfo.max_freq;
 	limits.max_perf_pct = clamp_t(int, limits.max_perf_pct, 0 , 100);
 	limits.max_perf = div_fp(int_tofp(limits.max_perf_pct), int_tofp(100));

-
-
 	if (policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
 		limits.min_perf_pct = 100;
 		limits.min_perf = int_tofp(1);
@@ -765,12 +746,10 @@ static struct cpufreq_driver intel_pstate_driver = {
 	.owner		= THIS_MODULE,
 };

-
 static void intel_pstate_exit(void)
 {
 	int cpu;

-
 	sysfs_remove_group(intel_pstate_kobject,
 				&intel_pstate_attr_group);
 	debugfs_remove_recursive(debugfs_parent);
@@ -791,11 +770,11 @@ static void intel_pstate_exit(void)
 	put_online_cpus();
 	vfree(all_cpu_data);
 }
+module_exit(intel_pstate_exit);

 static int __init intel_pstate_init(void)
 {
 	int rc = 0;
-
 	const struct x86_cpu_id *id;

 	id = x86_match_cpu(intel_pstate_cpu_ids);
@@ -820,10 +799,8 @@ out:
 	intel_pstate_exit();
 	return -ENODEV;
 }
+device_initcall(intel_pstate_init);

 MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
 MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
 MODULE_LICENSE("GPL");
-
-device_initcall(intel_pstate_init);
-module_exit(intel_pstate_exit);

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Viresh Kumar]

On Thu, Feb 7, 2013 at 4:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> @Rafael: I wasn't sure if these would apply over my patches without any
> conflicts and so i applied them in my repo. Added my Acked-by's and fixed
> a minor thing in one of the patches (as mentioned in the other mail).
>
> You can pick these from: dirk-patches
>
> http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/dirk-patches

Hi Rafael,

I have got few sparse fixes for these patches. Updated the branch,
please pick it up again.

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Rafael J. Wysocki]

On Thursday, February 07, 2013 07:32:26 PM Viresh Kumar wrote:
> On Thu, Feb 7, 2013 at 4:12 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > @Rafael: I wasn't sure if these would apply over my patches without any
> > conflicts and so i applied them in my repo. Added my Acked-by's and fixed
> > a minor thing in one of the patches (as mentioned in the other mail).
> >
> > You can pick these from: dirk-patches
> >
> > http://git.linaro.org/gitweb?p=people/vireshk/linux.git;a=shortlog;h=refs/heads/dirk-patches
> 
> Hi Rafael,
> 
> I have got few sparse fixes for these patches. Updated the branch,
> please pick it up again.

I haven't picked it up yet, so thanks a lot for that work!

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Dave Jones]

On Wed, Feb 06, 2013 at 09:02:07AM -0800, dirk.brandewie@gmail.com wrote:
 > From: Dirk Brandewie <dirk.brandewie@gmail.com>
 > 
 > This driver implements a scaling driver with an internal governor for
 > Intel Core processors.  The driver follows the same model as the
 > Transmeta scaling driver (longrun.c) and implements the setpolicy()
 > instead of target().  Scaling drivers that implement setpolicy() are
 > assmuned to implement internal governors by the cpufreq core. All the
 > logic for selecting the current P state is contained within the driver
 > no external governor is used by the cpufreq core.
 > 
 > At the moment only Intel SandyBridge processors are supported. As
 > testing on SandyBridge+ processors is completed support will be added
 > to the driver.

Won't you also need to patch drivers/cpufreq/acpi-cpufreq.c to not load
on the processors that you want this driver to run on ?

	Dave

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Dirk Brandewie]

Hi Dave,

On 02/12/2013 01:49 PM, Dave Jones wrote:
> On Wed, Feb 06, 2013 at 09:02:07AM -0800, dirk.brandewie@gmail.com wrote:
>
> Won't you also need to patch drivers/cpufreq/acpi-cpufreq.c to not load
> on the processors that you want this driver to run on ?
>
> 	Dave
>

For the case where both are built-in the load order works my driver uses
device_initcall() and acpi_cpufreq uses late_initcall().

For the case where both are a module (which I was sure I tested) you are right
I will have to do something.

For now I propose to make my driver built-in only while I sort out the right
solution for the module build.  Does this seem reasonable to everyone?

--Dirk

[PATCH] cpufreq/intel_pstate: Change to disallow module build

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

Load order is important in order for intel_pstate to take over as the
default scaling driver from acpi-cpufreq.

If both are built-in acpi-cpufreq uses late_initcall() and
intel_pstate uses device_initcall() so it will be able to register as
the scaling before acpi-cpufreq for the processors supported by
intel_pstate.

If acpi-cpufreq is built as a module then intel_pstate still gets
first option to become the scaling driver.

Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
---

based on linux-pm/linux-next fe9e0a66

 drivers/cpufreq/Kconfig.x86 |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 6aa7053..98e5abb 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -3,7 +3,7 @@
 #
 
 config X86_INTEL_PSTATE
-       tristate "Intel P state control"
+       bool "Intel P state control"
        depends on X86
        help
           This driver provides a P state for Intel core processors.
@@ -13,11 +13,6 @@ config X86_INTEL_PSTATE
 	  When this driver is enabled it will become the perferred
           scaling driver for Sandy bridge processors.
 
-	  Note: This driver should be built with the same settings as
-	  the other scaling drivers configured into the system
-	  (module/built-in) in order for the driver to register itself
-	  as the scaling driver on the system.
-
 	  If in doubt, say N.
 
 config X86_PCC_CPUFREQ
-- 
1.7.7.6

[PATCH] cpufreq/intel_pstate: Change to disallow module build

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

Load order is important in order for intel_pstate to take over as the
default scaling driver from acpi-cpufreq.

If both are built-in acpi-cpufreq uses late_initcall() and
intel_pstate uses device_initcall() so it will be able to register as
the scaling before acpi-cpufreq for the processors supported by
intel_pstate.

If acpi-cpufreq is built as a module then intel_pstate still gets
first option to become the scaling driver.

Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>
---
 drivers/cpufreq/Kconfig.x86 |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
index 6aa7053..98e5abb 100644
--- a/drivers/cpufreq/Kconfig.x86
+++ b/drivers/cpufreq/Kconfig.x86
@@ -3,7 +3,7 @@
 #
 
 config X86_INTEL_PSTATE
-       tristate "Intel P state control"
+       bool "Intel P state control"
        depends on X86
        help
           This driver provides a P state for Intel core processors.
@@ -13,11 +13,6 @@ config X86_INTEL_PSTATE
 	  When this driver is enabled it will become the perferred
           scaling driver for Sandy bridge processors.
 
-	  Note: This driver should be built with the same settings as
-	  the other scaling drivers configured into the system
-	  (module/built-in) in order for the driver to register itself
-	  as the scaling driver on the system.
-
 	  If in doubt, say N.
 
 config X86_PCC_CPUFREQ
-- 
1.7.7.6

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Rafael J. Wysocki]

On Wednesday, February 13, 2013 08:38:04 AM Dirk Brandewie wrote:
> Hi Dave,
> 
> On 02/12/2013 01:49 PM, Dave Jones wrote:
> > On Wed, Feb 06, 2013 at 09:02:07AM -0800, dirk.brandewie@gmail.com wrote:
> >
> > Won't you also need to patch drivers/cpufreq/acpi-cpufreq.c to not load
> > on the processors that you want this driver to run on ?
> >
> > 	Dave
> >
> 
> For the case where both are built-in the load order works my driver uses
> device_initcall() and acpi_cpufreq uses late_initcall().
> 
> For the case where both are a module (which I was sure I tested) you are right
> I will have to do something.
> 
> For now I propose to make my driver built-in only while I sort out the right
> solution for the module build.  Does this seem reasonable to everyone?

Well, I've been saying I think your driver should be non-modular from the
start. :-)

May I ask for a kernel command line switch to prevent it from registering if
the user doesn't actually want it, though, if it's going to be non-modular?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Viresh Kumar]

On Wed, Feb 13, 2013 at 10:08 PM, Dirk Brandewie
<dirk.brandewie@gmail.com> wrote:
> For the case where both are built-in the load order works my driver uses
> device_initcall() and acpi_cpufreq uses late_initcall().
>
> For the case where both are a module (which I was sure I tested) you are
> right
> I will have to do something.
>
> For now I propose to make my driver built-in only while I sort out the right
> solution for the module build.  Does this seem reasonable to everyone?

Of-course i am missing something here. Why would anybody want to insert
acpi-cpufreq module when the system supports the pstate driver.

In case they are mutually exclusive, then we can have something like
depends on !ACPI-DRIVER in the kconfig option of pstate driver.

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Rafael J. Wysocki]

On Thursday, February 14, 2013 09:38:21 AM Viresh Kumar wrote:
> On Wed, Feb 13, 2013 at 10:08 PM, Dirk Brandewie
> <dirk.brandewie@gmail.com> wrote:
> > For the case where both are built-in the load order works my driver uses
> > device_initcall() and acpi_cpufreq uses late_initcall().
> >
> > For the case where both are a module (which I was sure I tested) you are
> > right
> > I will have to do something.
> >
> > For now I propose to make my driver built-in only while I sort out the right
> > solution for the module build.  Does this seem reasonable to everyone?
> 
> Of-course i am missing something here. Why would anybody want to insert
> acpi-cpufreq module when the system supports the pstate driver.
> 
> In case they are mutually exclusive, then we can have something like
> depends on !ACPI-DRIVER in the kconfig option of pstate driver.

Yes.  Or the other way around (i.e. make acpi_cpufreq depend on
!X86_INTEL_PSTATE).

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Dirk Brandewie]

On 02/14/2013 04:21 AM, Rafael J. Wysocki wrote:
> On Thursday, February 14, 2013 09:38:21 AM Viresh Kumar wrote:
>> On Wed, Feb 13, 2013 at 10:08 PM, Dirk Brandewie
>> <dirk.brandewie@gmail.com> wrote:
>>> For the case where both are built-in the load order works my driver uses
>>> device_initcall() and acpi_cpufreq uses late_initcall().
>>>
>>> For the case where both are a module (which I was sure I tested) you are
>>> right
>>> I will have to do something.
>>>
>>> For now I propose to make my driver built-in only while I sort out the right
>>> solution for the module build.  Does this seem reasonable to everyone?
>>
>> Of-course i am missing something here. Why would anybody want to insert
>> acpi-cpufreq module when the system supports the pstate driver.
>>
>> In case they are mutually exclusive, then we can have something like
>> depends on !ACPI-DRIVER in the kconfig option of pstate driver.
>
> Yes.  Or the other way around (i.e. make acpi_cpufreq depend on
> !X86_INTEL_PSTATE).
>

The issue is that acpi-cpufreq still needs to be available for Intel processors 
before SandyBridge and for other x86 compatible processors we can't make
intel_pstate and acpi-cpufreq mutually exclusive.

Having intel_pstate built-in solves the issue without the need to patch
acpi-cpufreq.  I believe that most distros build the scaling drivers in
so the distro/user will make the explicit decision to use intel_pstate.


> Thanks,
> Rafael
>
>

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Dirk Brandewie]

On 02/14/2013 04:21 AM, Rafael J. Wysocki wrote:
> On Thursday, February 14, 2013 09:38:21 AM Viresh Kumar wrote:
>> On Wed, Feb 13, 2013 at 10:08 PM, Dirk Brandewie
>> <dirk.brandewie@gmail.com> wrote:
>>> For the case where both are built-in the load order works my driver uses
>>> device_initcall() and acpi_cpufreq uses late_initcall().
>>>
>>> For the case where both are a module (which I was sure I tested) you are
>>> right
>>> I will have to do something.
>>>
>>> For now I propose to make my driver built-in only while I sort out the right
>>> solution for the module build.  Does this seem reasonable to everyone?
>>
>> Of-course i am missing something here. Why would anybody want to insert
>> acpi-cpufreq module when the system supports the pstate driver.
>>
>> In case they are mutually exclusive, then we can have something like
>> depends on !ACPI-DRIVER in the kconfig option of pstate driver.
>
> Yes.  Or the other way around (i.e. make acpi_cpufreq depend on
> !X86_INTEL_PSTATE).
>

The issue is that acpi-cpufreq still needs to be available for Intel processors 
before SandyBridge and for other x86 compatible processors we can't make
intel_pstate and acpi-cpufreq mutually exclusive.

Having intel_pstate built-in solves the issue without the need to patch
acpi-cpufreq.  I believe that most distros build the scaling drivers in
so the distro/user will make the explicit decision to use intel_pstate.


> Thanks,
> Rafael
>
>

Re: [PATCH] cpufreq/intel_pstate: Change to disallow module build

[Viresh Kumar]

On Thu, Feb 14, 2013 at 1:44 AM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> Load order is important in order for intel_pstate to take over as the
> default scaling driver from acpi-cpufreq.
>
> If both are built-in acpi-cpufreq uses late_initcall() and
> intel_pstate uses device_initcall() so it will be able to register as
> the scaling before acpi-cpufreq for the processors supported by
> intel_pstate.
>
> If acpi-cpufreq is built as a module then intel_pstate still gets
> first option to become the scaling driver.
>
> Signed-off-by: Dirk Brandewie <dirk.brandewie@gmail.com>

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Re: [PATCH 0/5] Add P state driver for Intel Core Processors

[Rafael J. Wysocki]

On Thursday, February 14, 2013 07:34:56 AM Dirk Brandewie wrote:
> On 02/14/2013 04:21 AM, Rafael J. Wysocki wrote:
> > On Thursday, February 14, 2013 09:38:21 AM Viresh Kumar wrote:
> >> On Wed, Feb 13, 2013 at 10:08 PM, Dirk Brandewie
> >> <dirk.brandewie@gmail.com> wrote:
> >>> For the case where both are built-in the load order works my driver uses
> >>> device_initcall() and acpi_cpufreq uses late_initcall().
> >>>
> >>> For the case where both are a module (which I was sure I tested) you are
> >>> right
> >>> I will have to do something.
> >>>
> >>> For now I propose to make my driver built-in only while I sort out the right
> >>> solution for the module build.  Does this seem reasonable to everyone?
> >>
> >> Of-course i am missing something here. Why would anybody want to insert
> >> acpi-cpufreq module when the system supports the pstate driver.
> >>
> >> In case they are mutually exclusive, then we can have something like
> >> depends on !ACPI-DRIVER in the kconfig option of pstate driver.
> >
> > Yes.  Or the other way around (i.e. make acpi_cpufreq depend on
> > !X86_INTEL_PSTATE).
> >
> 
> The issue is that acpi-cpufreq still needs to be available for Intel processors 
> before SandyBridge and for other x86 compatible processors we can't make
> intel_pstate and acpi-cpufreq mutually exclusive.

Right, I forgot about that and recalled it just a minute after sending that
message.
 
> Having intel_pstate built-in solves the issue without the need to patch
> acpi-cpufreq.  I believe that most distros build the scaling drivers in
> so the distro/user will make the explicit decision to use intel_pstate.

But then we need to allow the user to choose acpi_cpufreq anyway if it's
preferred whatever the reason.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCH] cpufreq/intel_pstate: Add kernel command line option disable intel_pstate.

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

When intel_pstate is configured into the kernel it will become the
perferred scaling driver for processors that is supports.  Allow the
user to override this by adding:
   intel_pstate=disable
on the kernel command line.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 Documentation/kernel-parameters.txt |    5 +++++
 drivers/cpufreq/intel_pstate.c      |   16 ++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 6c72381..41c5d9e 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1131,6 +1131,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			0	disables intel_idle and fall back on acpi_idle.
 			1 to 6	specify maximum depth of C-state.
 
+	intel_pstate=  [X86]
+		       disable
+		         Do not enable intel_pstate as the default
+		         scaling driver for the supported processors
+
 	intremap=	[X86-64, Intel-IOMMU]
 			on	enable Interrupt Remapping (default)
 			off	disable Interrupt Remapping
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index e879963..9cbb733 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -773,11 +773,16 @@ static void intel_pstate_exit(void)
 }
 module_exit(intel_pstate_exit);
 
+static int no_load;
+
 static int __init intel_pstate_init(void)
 {
 	int rc = 0;
 	const struct x86_cpu_id *id;
 
+	if (no_load)
+		return -ENODEV;
+
 	id = x86_match_cpu(intel_pstate_cpu_ids);
 	if (!id)
 		return -ENODEV;
@@ -802,6 +807,17 @@ out:
 }
 device_initcall(intel_pstate_init);
 
+static int __init intel_pstate_setup(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "disable"))
+		no_load = 1;
+	return 0;
+}
+early_param("intel_pstate", intel_pstate_setup);
+
 MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
 MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
 MODULE_LICENSE("GPL");
-- 
1.7.7.6

Re: [PATCH] cpufreq/intel_pstate: Add kernel command line option disable intel_pstate.

[Yinghai Lu]

On Thu, Feb 14, 2013 at 10:38 AM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> When intel_pstate is configured into the kernel it will become the
> perferred scaling driver for processors that is supports.  Allow the
> user to override this by adding:
>    intel_pstate=disable
> on the kernel command line.
>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  Documentation/kernel-parameters.txt |    5 +++++
>  drivers/cpufreq/intel_pstate.c      |   16 ++++++++++++++++
>  2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 6c72381..41c5d9e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1131,6 +1131,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>                         0       disables intel_idle and fall back on acpi_idle.
>                         1 to 6  specify maximum depth of C-state.
>
> +       intel_pstate=  [X86]
> +                      disable
> +                        Do not enable intel_pstate as the default
> +                        scaling driver for the supported processors
> +
>         intremap=       [X86-64, Intel-IOMMU]
>                         on      enable Interrupt Remapping (default)
>                         off     disable Interrupt Remapping
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index e879963..9cbb733 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -773,11 +773,16 @@ static void intel_pstate_exit(void)
>  }
>  module_exit(intel_pstate_exit);
>
> +static int no_load;

Looks like you can use __initdata with it.

Thanks

Yinghai


> +
>  static int __init intel_pstate_init(void)
>  {
>         int rc = 0;
>         const struct x86_cpu_id *id;
>
> +       if (no_load)
> +               return -ENODEV;
> +
>         id = x86_match_cpu(intel_pstate_cpu_ids);
>         if (!id)
>                 return -ENODEV;
> @@ -802,6 +807,17 @@ out:
>  }
>  device_initcall(intel_pstate_init);
>
> +static int __init intel_pstate_setup(char *str)
> +{
> +       if (!str)
> +               return -EINVAL;
> +
> +       if (!strcmp(str, "disable"))
> +               no_load = 1;
> +       return 0;
> +}
> +early_param("intel_pstate", intel_pstate_setup);
> +
>  MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
>  MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
>  MODULE_LICENSE("GPL");
> --
> 1.7.7.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] cpufreq/intel_pstate: Add kernel command line option disable intel_pstate.

[Viresh Kumar]

On Fri, Feb 15, 2013 at 12:08 AM,  <dirk.brandewie@gmail.com> wrote:
> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>
> When intel_pstate is configured into the kernel it will become the
> perferred scaling driver for processors that is supports.  Allow the

s/perferred/preferred

s/is/it

> user to override this by adding:
>    intel_pstate=disable
> on the kernel command line.
>
> Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> ---
>  Documentation/kernel-parameters.txt |    5 +++++
>  drivers/cpufreq/intel_pstate.c      |   16 ++++++++++++++++
>  2 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 6c72381..41c5d9e 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1131,6 +1131,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>                         0       disables intel_idle and fall back on acpi_idle.
>                         1 to 6  specify maximum depth of C-state.
>
> +       intel_pstate=  [X86]
> +                      disable
> +                        Do not enable intel_pstate as the default
> +                        scaling driver for the supported processors
> +
>         intremap=       [X86-64, Intel-IOMMU]
>                         on      enable Interrupt Remapping (default)
>                         off     disable Interrupt Remapping
> diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> index e879963..9cbb733 100644
> --- a/drivers/cpufreq/intel_pstate.c
> +++ b/drivers/cpufreq/intel_pstate.c
> @@ -773,11 +773,16 @@ static void intel_pstate_exit(void)
>  }
>  module_exit(intel_pstate_exit);
>
> +static int no_load;
> +
>  static int __init intel_pstate_init(void)
>  {
>         int rc = 0;
>         const struct x86_cpu_id *id;
>
> +       if (no_load)
> +               return -ENODEV;
> +
>         id = x86_match_cpu(intel_pstate_cpu_ids);
>         if (!id)
>                 return -ENODEV;
> @@ -802,6 +807,17 @@ out:
>  }
>  device_initcall(intel_pstate_init);
>
> +static int __init intel_pstate_setup(char *str)

Parameter to this is really not required. By default intel_pstate
driver is enabled, so just past disable_intel_pstate from command line
without any args.

> +{
> +       if (!str)
> +               return -EINVAL;
> +
> +       if (!strcmp(str, "disable"))
> +               no_load = 1;
> +       return 0;
> +}
> +early_param("intel_pstate", intel_pstate_setup);

Re: [PATCH] cpufreq/intel_pstate: Add kernel command line option disable intel_pstate.

[Rafael J. Wysocki]

On Friday, February 15, 2013 09:53:24 AM Viresh Kumar wrote:
> On Fri, Feb 15, 2013 at 12:08 AM,  <dirk.brandewie@gmail.com> wrote:
> > From: Dirk Brandewie <dirk.brandewie@gmail.com>
> >
> > When intel_pstate is configured into the kernel it will become the
> > perferred scaling driver for processors that is supports.  Allow the
> 
> s/perferred/preferred
> 
> s/is/it
> 
> > user to override this by adding:
> >    intel_pstate=disable
> > on the kernel command line.
> >
> > Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
> > ---
> >  Documentation/kernel-parameters.txt |    5 +++++
> >  drivers/cpufreq/intel_pstate.c      |   16 ++++++++++++++++
> >  2 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > index 6c72381..41c5d9e 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -1131,6 +1131,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
> >                         0       disables intel_idle and fall back on acpi_idle.
> >                         1 to 6  specify maximum depth of C-state.
> >
> > +       intel_pstate=  [X86]
> > +                      disable
> > +                        Do not enable intel_pstate as the default
> > +                        scaling driver for the supported processors
> > +
> >         intremap=       [X86-64, Intel-IOMMU]
> >                         on      enable Interrupt Remapping (default)
> >                         off     disable Interrupt Remapping
> > diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
> > index e879963..9cbb733 100644
> > --- a/drivers/cpufreq/intel_pstate.c
> > +++ b/drivers/cpufreq/intel_pstate.c
> > @@ -773,11 +773,16 @@ static void intel_pstate_exit(void)
> >  }
> >  module_exit(intel_pstate_exit);
> >
> > +static int no_load;
> > +
> >  static int __init intel_pstate_init(void)
> >  {
> >         int rc = 0;
> >         const struct x86_cpu_id *id;
> >
> > +       if (no_load)
> > +               return -ENODEV;
> > +
> >         id = x86_match_cpu(intel_pstate_cpu_ids);
> >         if (!id)
> >                 return -ENODEV;
> > @@ -802,6 +807,17 @@ out:
> >  }
> >  device_initcall(intel_pstate_init);
> >
> > +static int __init intel_pstate_setup(char *str)
> 
> Parameter to this is really not required. By default intel_pstate
> driver is enabled, so just past disable_intel_pstate from command line
> without any args.

Well, quite frankly, I'm fine with both.  It all boils down to passing a
specific string on the kernel command line.

Thanks,
Rafael


> > +{
> > +       if (!str)
> > +               return -EINVAL;
> > +
> > +       if (!strcmp(str, "disable"))
> > +               no_load = 1;
> > +       return 0;
> > +}
> > +early_param("intel_pstate", intel_pstate_setup);
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCH] cpufreq/intel_pstate: Add kernel command line option disable intel_pstate.

[Viresh Kumar]

On 15 February 2013 18:00, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday, February 15, 2013 09:53:24 AM Viresh Kumar wrote:
>> Parameter to this is really not required. By default intel_pstate
>> driver is enabled, so just past disable_intel_pstate from command line
>> without any args.
>
> Well, quite frankly, I'm fine with both.  It all boils down to passing a
> specific string on the kernel command line.

Well, we are all looking to save worlds power here and my suggestion
would definitely use less power as it has less instructions to execute.
Just for fun!!.

Re: [PATCH] cpufreq/intel_pstate: Add kernel command line option disable intel_pstate.

[Dirk Brandewie]

On 02/15/2013 05:28 AM, Viresh Kumar wrote:
> On 15 February 2013 18:00, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> On Friday, February 15, 2013 09:53:24 AM Viresh Kumar wrote:
>>> Parameter to this is really not required. By default intel_pstate
>>> driver is enabled, so just past disable_intel_pstate from command line
>>> without any args.
>>
>> Well, quite frankly, I'm fine with both.  It all boils down to passing a
>> specific string on the kernel command line.
>
> Well, we are all looking to save worlds power here and my suggestion
> would definitely use less power as it has less instructions to execute.
> Just for fun!!.
>

I would like to leave it the way it is now. If for what ever reason we need
another switch passed to the driver we have a place for it without adding
another single purpose command line option

The updated with __initdata and the spelling fixes will follow shortly

--Dirk

[PATCH] cpufreq/intel_pstate: Add kernel command line option disable intel_pstate.

[dirk.brandewie]

From: Dirk Brandewie <dirk.brandewie@gmail.com>

When intel_pstate is configured into the kernel it will become the
preferred scaling driver for processors that it supports.  Allow the
user to override this by adding:
   intel_pstate=disable
on the kernel command line.

Signed-off-by: Dirk Brandewie <dirk.j.brandewie@intel.com>
---
 Documentation/kernel-parameters.txt |    5 +++++
 drivers/cpufreq/intel_pstate.c      |   16 ++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 6c72381..41c5d9e 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1131,6 +1131,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			0	disables intel_idle and fall back on acpi_idle.
 			1 to 6	specify maximum depth of C-state.
 
+	intel_pstate=  [X86]
+		       disable
+		         Do not enable intel_pstate as the default
+		         scaling driver for the supported processors
+
 	intremap=	[X86-64, Intel-IOMMU]
 			on	enable Interrupt Remapping (default)
 			off	disable Interrupt Remapping
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index e879963..096fde0 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -773,11 +773,16 @@ static void intel_pstate_exit(void)
 }
 module_exit(intel_pstate_exit);
 
+static int __initdata no_load;
+
 static int __init intel_pstate_init(void)
 {
 	int rc = 0;
 	const struct x86_cpu_id *id;
 
+	if (no_load)
+		return -ENODEV;
+
 	id = x86_match_cpu(intel_pstate_cpu_ids);
 	if (!id)
 		return -ENODEV;
@@ -802,6 +807,17 @@ out:
 }
 device_initcall(intel_pstate_init);
 
+static int __init intel_pstate_setup(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "disable"))
+		no_load = 1;
+	return 0;
+}
+early_param("intel_pstate", intel_pstate_setup);
+
 MODULE_AUTHOR("Dirk Brandewie <dirk.j.brandewie@intel.com>");
 MODULE_DESCRIPTION("'intel_pstate' - P state driver Intel Core processors");
 MODULE_LICENSE("GPL");
-- 
1.7.7.6