All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] thermal: powerclamp: Add support for ARM machine to powerclamp driver.
@ 2013-07-25 10:22 Jonghwa Lee
  2013-07-25 13:16 ` Eduardo Valentin
  2013-07-29 18:11 ` Jacob Pan
  0 siblings, 2 replies; 3+ messages in thread
From: Jonghwa Lee @ 2013-07-25 10:22 UTC (permalink / raw)
  To: linux-pm
  Cc: jacob.jun.pan, rui.zhang, eduardo.valentin, amit.daniel,
	Jonghwa Lee, Myungjoo Ham

This patch modifies intel_powerclamp driver can be used in not only x86
but also ARM. Any ARM system can use intel_powerclamp driver for managing
thermal problem through injecting intentional idle cycle. The required
modification is only that it replaces x86's instruction, mwait, with arm
specific one, wfi and changes the way of calculation of idle rate of last
window. Other algorithm and codes can be shared without any amendment.

Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
---
 drivers/thermal/Kconfig            |    3 +-
 drivers/thermal/intel_powerclamp.c |   94 ++++++++++++++++++++++++++----------
 2 files changed, 70 insertions(+), 27 deletions(-)

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index e988c81..912a788 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -162,8 +162,7 @@ config DB8500_CPUFREQ_COOLING
 config INTEL_POWERCLAMP
 	tristate "Intel PowerClamp idle injection driver"
 	depends on THERMAL
-	depends on X86
-	depends on CPU_SUP_INTEL
+	depends on (X86 && CPU_SUP_INTEL) || ARM
 	help
 	  Enable this to enable Intel PowerClamp idle injection driver. This
 	  enforce idle time which results in more package C-state residency. The
diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
index b40b37c..3ff350b 100644
--- a/drivers/thermal/intel_powerclamp.c
+++ b/drivers/thermal/intel_powerclamp.c
@@ -52,12 +52,18 @@
 #include <linux/seq_file.h>
 #include <linux/sched/rt.h>
 
+#include <asm/hardirq.h>
+#ifdef CONFIG_X86
 #include <asm/nmi.h>
 #include <asm/msr.h>
 #include <asm/mwait.h>
 #include <asm/cpu_device_id.h>
 #include <asm/idle.h>
-#include <asm/hardirq.h>
+
+static unsigned int target_mwait;
+#elif CONFIG_ARM
+#include <asm/proc-fns.h>
+#endif
 
 #define MAX_TARGET_RATIO (50U)
 /* For each undisturbed clamping period (no extra wake ups during idle time),
@@ -71,7 +77,6 @@
  */
 #define DEFAULT_DURATION_JIFFIES (6)
 
-static unsigned int target_mwait;
 static struct dentry *debug_dir;
 
 /* user selected target */
@@ -178,6 +183,7 @@ MODULE_PARM_DESC(window_size, "sliding window in number of clamping cycles\n"
 	"\twindow size results in slower response time but more smooth\n"
 	"\tclamping results. default to 2.");
 
+#ifdef CONFIG_X86
 static void find_target_mwait(void)
 {
 	unsigned int eax, ebx, ecx, edx;
@@ -246,6 +252,7 @@ static u64 pkg_state_counter(void)
 
 	return count;
 }
+#endif
 
 static void noop_timer(unsigned long foo)
 {
@@ -316,6 +323,25 @@ static void adjust_compensation(int target_ratio, unsigned int win)
 	}
 }
 
+#ifdef CONFIG_X86
+static void inline powerclamp_get_cstate_inform(u64 *msr_now, u64 *tsc_now)
+{
+	*msr_now = pkg_state_counter();
+	rdtscll(*tsc_now);
+}
+#elif CONFIG_ARM
+static void inline powerclamp_get_cstate_inform(u64 *idle, u64 *wall)
+{
+	u64 _wall;
+	int cpu;
+
+	for_each_online_cpu(cpu) {
+		*idle += get_cpu_idle_time_us(cpu, &_wall);
+		*wall += _wall;
+	}
+}
+#endif
+
 static bool powerclamp_adjust_controls(unsigned int target_ratio,
 				unsigned int guard, unsigned int win)
 {
@@ -324,8 +350,7 @@ static bool powerclamp_adjust_controls(unsigned int target_ratio,
 	u64 val64;
 
 	/* check result for the last window */
-	msr_now = pkg_state_counter();
-	rdtscll(tsc_now);
+	powerclamp_get_cstate_inform(&msr_now, &tsc_now);
 
 	/* calculate pkg cstate vs tsc ratio */
 	if (!msr_last || !tsc_last)
@@ -353,6 +378,34 @@ static bool powerclamp_adjust_controls(unsigned int target_ratio,
 	return set_target_ratio + guard <= current_ratio;
 }
 
+#ifdef CONFIG_X86
+static void powerclamp_enter_idle(void)
+{
+	unsigned long ecx = 1;
+	unsigned long eax = target_mwait;
+
+	/*
+	 * REVISIT: may call enter_idle() to notify drivers who
+	 * can save power during cpu idle. same for exit_idle()
+	 */
+	local_touch_nmi();
+	stop_critical_timings();
+	__monitor((void *)&current_thread_info()->flags, 0, 0);
+	cpu_relax(); /* allow HT sibling to run */
+	__mwait(eax, ecx);
+	start_critical_timings();
+	atomic_inc(&idle_wakeup_counter);
+}
+#elif CONFIG_ARM
+static void  powerclamp_enter_idle(void)
+{
+	stop_critical_timings();
+	cpu_do_idle();
+	start_critical_timings();
+	atomic_inc(&idle_wakeup_counter);
+}
+#endif
+
 static int clamp_thread(void *arg)
 {
 	int cpunr = (unsigned long)arg;
@@ -428,22 +481,9 @@ static int clamp_thread(void *arg)
 		preempt_disable();
 		tick_nohz_idle_enter();
 		/* mwait until target jiffies is reached */
-		while (time_before(jiffies, target_jiffies)) {
-			unsigned long ecx = 1;
-			unsigned long eax = target_mwait;
-
-			/*
-			 * REVISIT: may call enter_idle() to notify drivers who
-			 * can save power during cpu idle. same for exit_idle()
-			 */
-			local_touch_nmi();
-			stop_critical_timings();
-			__monitor((void *)&current_thread_info()->flags, 0, 0);
-			cpu_relax(); /* allow HT sibling to run */
-			__mwait(eax, ecx);
-			start_critical_timings();
-			atomic_inc(&idle_wakeup_counter);
-		}
+		while (time_before(jiffies, target_jiffies))
+			powerclamp_enter_idle();
+
 		tick_nohz_idle_exit();
 		preempt_enable_no_resched();
 	}
@@ -465,13 +505,12 @@ static void poll_pkg_cstate(struct work_struct *dummy)
 	static u64 tsc_last;
 	static unsigned long jiffies_last;
 
-	u64 msr_now;
+	u64 msr_now = 0;
 	unsigned long jiffies_now;
-	u64 tsc_now;
+	u64 tsc_now = 0;
 	u64 val64;
 
-	msr_now = pkg_state_counter();
-	rdtscll(tsc_now);
+	powerclamp_get_cstate_inform(&msr_now, &tsc_now);
 	jiffies_now = jiffies;
 
 	/* calculate pkg cstate vs tsc ratio */
@@ -499,11 +538,13 @@ static int start_power_clamp(void)
 	unsigned long cpu;
 	struct task_struct *thread;
 
+#ifdef CONFIG_X86
 	/* check if pkg cstate counter is completely 0, abort in this case */
 	if (!pkg_state_counter()) {
 		pr_err("pkg cstate counter not functional, abort\n");
 		return -EINVAL;
 	}
+#endif
 
 	set_target_ratio = clamp(set_target_ratio, 0U, MAX_TARGET_RATIO - 1);
 	/* prevent cpu hotplug */
@@ -661,6 +702,7 @@ static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
 	.set_cur_state = powerclamp_set_cur_state,
 };
 
+#ifdef CONFIG_X86
 /* runs on Nehalem and later */
 static const struct x86_cpu_id intel_powerclamp_ids[] = {
 	{ X86_VENDOR_INTEL, 6, 0x1a},
@@ -678,9 +720,11 @@ static const struct x86_cpu_id intel_powerclamp_ids[] = {
 	{}
 };
 MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
+#endif
 
 static int powerclamp_probe(void)
 {
+#ifdef CONFIG_X86
 	if (!x86_match_cpu(intel_powerclamp_ids)) {
 		pr_err("Intel powerclamp does not run on family %d model %d\n",
 				boot_cpu_data.x86, boot_cpu_data.x86_model);
@@ -694,7 +738,7 @@ static int powerclamp_probe(void)
 
 	/* find the deepest mwait value */
 	find_target_mwait();
-
+#endif
 	return 0;
 }
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] thermal: powerclamp: Add support for ARM machine to powerclamp driver.
  2013-07-25 10:22 [RFC PATCH] thermal: powerclamp: Add support for ARM machine to powerclamp driver Jonghwa Lee
@ 2013-07-25 13:16 ` Eduardo Valentin
  2013-07-29 18:11 ` Jacob Pan
  1 sibling, 0 replies; 3+ messages in thread
From: Eduardo Valentin @ 2013-07-25 13:16 UTC (permalink / raw)
  To: Jonghwa Lee
  Cc: linux-pm, jacob.jun.pan, rui.zhang, eduardo.valentin,
	amit.daniel, Myungjoo Ham

[-- Attachment #1: Type: text/plain, Size: 8279 bytes --]

Hello Jonghwa,

On 25-07-2013 06:22, Jonghwa Lee wrote:
> This patch modifies intel_powerclamp driver can be used in not only x86
> but also ARM. Any ARM system can use intel_powerclamp driver for managing
> thermal problem through injecting intentional idle cycle. The required
> modification is only that it replaces x86's instruction, mwait, with arm
> specific one, wfi and changes the way of calculation of idle rate of last
> window. Other algorithm and codes can be shared without any amendment.
> 

First of all, thanks for taking this up. It was on my TODO list, but I
still could not manage to start doing it.

I have had a quick look on your patch. I promise to review it properly
as soon as possible. Two major concerns are (1) I think the ARM part of
it is not really specific to ARM (more like non-x86), and could be
reused to other archs (ok, there are couple of points that may need to
be checked); and (2) I would rather split the code into two layers, one
that talks to thermal fw and another that is specific to arch, this way
we could reduce ifdefery.



> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> ---
>  drivers/thermal/Kconfig            |    3 +-
>  drivers/thermal/intel_powerclamp.c |   94 ++++++++++++++++++++++++++----------
>  2 files changed, 70 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e988c81..912a788 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -162,8 +162,7 @@ config DB8500_CPUFREQ_COOLING
>  config INTEL_POWERCLAMP
>  	tristate "Intel PowerClamp idle injection driver"
>  	depends on THERMAL
> -	depends on X86
> -	depends on CPU_SUP_INTEL
> +	depends on (X86 && CPU_SUP_INTEL) || ARM
>  	help
>  	  Enable this to enable Intel PowerClamp idle injection driver. This
>  	  enforce idle time which results in more package C-state residency. The
> diff --git a/drivers/thermal/intel_powerclamp.c b/drivers/thermal/intel_powerclamp.c
> index b40b37c..3ff350b 100644
> --- a/drivers/thermal/intel_powerclamp.c
> +++ b/drivers/thermal/intel_powerclamp.c
> @@ -52,12 +52,18 @@
>  #include <linux/seq_file.h>
>  #include <linux/sched/rt.h>
>  
> +#include <asm/hardirq.h>
> +#ifdef CONFIG_X86
>  #include <asm/nmi.h>
>  #include <asm/msr.h>
>  #include <asm/mwait.h>
>  #include <asm/cpu_device_id.h>
>  #include <asm/idle.h>
> -#include <asm/hardirq.h>
> +
> +static unsigned int target_mwait;
> +#elif CONFIG_ARM
> +#include <asm/proc-fns.h>

I don't think this is specific to ARM.

> +#endif
>  
>  #define MAX_TARGET_RATIO (50U)
>  /* For each undisturbed clamping period (no extra wake ups during idle time),
> @@ -71,7 +77,6 @@
>   */
>  #define DEFAULT_DURATION_JIFFIES (6)
>  
> -static unsigned int target_mwait;
>  static struct dentry *debug_dir;
>  
>  /* user selected target */
> @@ -178,6 +183,7 @@ MODULE_PARM_DESC(window_size, "sliding window in number of clamping cycles\n"
>  	"\twindow size results in slower response time but more smooth\n"
>  	"\tclamping results. default to 2.");
>  
> +#ifdef CONFIG_X86
>  static void find_target_mwait(void)
>  {
>  	unsigned int eax, ebx, ecx, edx;
> @@ -246,6 +252,7 @@ static u64 pkg_state_counter(void)
>  
>  	return count;
>  }
> +#endif
>  
>  static void noop_timer(unsigned long foo)
>  {
> @@ -316,6 +323,25 @@ static void adjust_compensation(int target_ratio, unsigned int win)
>  	}
>  }
>  
> +#ifdef CONFIG_X86
> +static void inline powerclamp_get_cstate_inform(u64 *msr_now, u64 *tsc_now)
> +{
> +	*msr_now = pkg_state_counter();
> +	rdtscll(*tsc_now);
> +}
> +#elif CONFIG_ARM
> +static void inline powerclamp_get_cstate_inform(u64 *idle, u64 *wall)
> +{
> +	u64 _wall;
> +	int cpu;
> +
> +	for_each_online_cpu(cpu) {
> +		*idle += get_cpu_idle_time_us(cpu, &_wall);
> +		*wall += _wall;
> +	}
> +}
> +#endif
> +
>  static bool powerclamp_adjust_controls(unsigned int target_ratio,
>  				unsigned int guard, unsigned int win)
>  {
> @@ -324,8 +350,7 @@ static bool powerclamp_adjust_controls(unsigned int target_ratio,
>  	u64 val64;
>  
>  	/* check result for the last window */
> -	msr_now = pkg_state_counter();
> -	rdtscll(tsc_now);
> +	powerclamp_get_cstate_inform(&msr_now, &tsc_now);
>  
>  	/* calculate pkg cstate vs tsc ratio */
>  	if (!msr_last || !tsc_last)
> @@ -353,6 +378,34 @@ static bool powerclamp_adjust_controls(unsigned int target_ratio,
>  	return set_target_ratio + guard <= current_ratio;
>  }
>  
> +#ifdef CONFIG_X86
> +static void powerclamp_enter_idle(void)
> +{
> +	unsigned long ecx = 1;
> +	unsigned long eax = target_mwait;
> +
> +	/*
> +	 * REVISIT: may call enter_idle() to notify drivers who
> +	 * can save power during cpu idle. same for exit_idle()
> +	 */
> +	local_touch_nmi();
> +	stop_critical_timings();
> +	__monitor((void *)&current_thread_info()->flags, 0, 0);
> +	cpu_relax(); /* allow HT sibling to run */
> +	__mwait(eax, ecx);
> +	start_critical_timings();
> +	atomic_inc(&idle_wakeup_counter);
> +}
> +#elif CONFIG_ARM
> +static void  powerclamp_enter_idle(void)
> +{
> +	stop_critical_timings();
> +	cpu_do_idle();
> +	start_critical_timings();
> +	atomic_inc(&idle_wakeup_counter);
> +}
> +#endif
> +
>  static int clamp_thread(void *arg)
>  {
>  	int cpunr = (unsigned long)arg;
> @@ -428,22 +481,9 @@ static int clamp_thread(void *arg)
>  		preempt_disable();
>  		tick_nohz_idle_enter();
>  		/* mwait until target jiffies is reached */
> -		while (time_before(jiffies, target_jiffies)) {
> -			unsigned long ecx = 1;
> -			unsigned long eax = target_mwait;
> -
> -			/*
> -			 * REVISIT: may call enter_idle() to notify drivers who
> -			 * can save power during cpu idle. same for exit_idle()
> -			 */
> -			local_touch_nmi();
> -			stop_critical_timings();
> -			__monitor((void *)&current_thread_info()->flags, 0, 0);
> -			cpu_relax(); /* allow HT sibling to run */
> -			__mwait(eax, ecx);
> -			start_critical_timings();
> -			atomic_inc(&idle_wakeup_counter);
> -		}
> +		while (time_before(jiffies, target_jiffies))
> +			powerclamp_enter_idle();
> +
>  		tick_nohz_idle_exit();
>  		preempt_enable_no_resched();
>  	}
> @@ -465,13 +505,12 @@ static void poll_pkg_cstate(struct work_struct *dummy)
>  	static u64 tsc_last;
>  	static unsigned long jiffies_last;
>  
> -	u64 msr_now;
> +	u64 msr_now = 0;
>  	unsigned long jiffies_now;
> -	u64 tsc_now;
> +	u64 tsc_now = 0;
>  	u64 val64;
>  
> -	msr_now = pkg_state_counter();
> -	rdtscll(tsc_now);
> +	powerclamp_get_cstate_inform(&msr_now, &tsc_now);
>  	jiffies_now = jiffies;
>  
>  	/* calculate pkg cstate vs tsc ratio */
> @@ -499,11 +538,13 @@ static int start_power_clamp(void)
>  	unsigned long cpu;
>  	struct task_struct *thread;
>  
> +#ifdef CONFIG_X86
>  	/* check if pkg cstate counter is completely 0, abort in this case */
>  	if (!pkg_state_counter()) {
>  		pr_err("pkg cstate counter not functional, abort\n");
>  		return -EINVAL;
>  	}
> +#endif
>  
>  	set_target_ratio = clamp(set_target_ratio, 0U, MAX_TARGET_RATIO - 1);
>  	/* prevent cpu hotplug */
> @@ -661,6 +702,7 @@ static struct thermal_cooling_device_ops powerclamp_cooling_ops = {
>  	.set_cur_state = powerclamp_set_cur_state,
>  };
>  
> +#ifdef CONFIG_X86
>  /* runs on Nehalem and later */
>  static const struct x86_cpu_id intel_powerclamp_ids[] = {
>  	{ X86_VENDOR_INTEL, 6, 0x1a},
> @@ -678,9 +720,11 @@ static const struct x86_cpu_id intel_powerclamp_ids[] = {
>  	{}
>  };
>  MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
> +#endif
>  
>  static int powerclamp_probe(void)
>  {
> +#ifdef CONFIG_X86
>  	if (!x86_match_cpu(intel_powerclamp_ids)) {
>  		pr_err("Intel powerclamp does not run on family %d model %d\n",
>  				boot_cpu_data.x86, boot_cpu_data.x86_model);
> @@ -694,7 +738,7 @@ static int powerclamp_probe(void)
>  
>  	/* find the deepest mwait value */
>  	find_target_mwait();
> -
> +#endif
>  	return 0;
>  }
>  
> 


-- 
You have got to be excited about what you are doing. (L. Lamport)

Eduardo Valentin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 295 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH] thermal: powerclamp: Add support for ARM machine to powerclamp driver.
  2013-07-25 10:22 [RFC PATCH] thermal: powerclamp: Add support for ARM machine to powerclamp driver Jonghwa Lee
  2013-07-25 13:16 ` Eduardo Valentin
@ 2013-07-29 18:11 ` Jacob Pan
  1 sibling, 0 replies; 3+ messages in thread
From: Jacob Pan @ 2013-07-29 18:11 UTC (permalink / raw)
  To: Jonghwa Lee
  Cc: linux-pm, rui.zhang, eduardo.valentin, amit.daniel, Myungjoo Ham,
	Van De Ven, Arjan

On Thu, 25 Jul 2013 19:22:07 +0900
Jonghwa Lee <jonghwa3.lee@samsung.com> wrote:

> This patch modifies intel_powerclamp driver can be used in not only
> x86 but also ARM. Any ARM system can use intel_powerclamp driver for
> managing thermal problem through injecting intentional idle cycle.
> The required modification is only that it replaces x86's instruction,
> mwait, with arm specific one, wfi and changes the way of calculation
> of idle rate of last window. Other algorithm and codes can be shared
> without any amendment.
> 
+arjan

In general, I agree with you to decouple platform specific part
such as timing and idle data collection from the algorithm. The
abstraction can be cleaner and more complete. Please see my comments
below.

> Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
> Signed-off-by: Myungjoo Ham <myungjoo.ham@samsung.com>
> ---
>  drivers/thermal/Kconfig            |    3 +-
>  drivers/thermal/intel_powerclamp.c |   94
> ++++++++++++++++++++++++++---------- 2 files changed, 70
> insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index e988c81..912a788 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -162,8 +162,7 @@ config DB8500_CPUFREQ_COOLING
>  config INTEL_POWERCLAMP
>  	tristate "Intel PowerClamp idle injection driver"
>  	depends on THERMAL
> -	depends on X86
> -	depends on CPU_SUP_INTEL
> +	depends on (X86 && CPU_SUP_INTEL) || ARM
>  	help
>  	  Enable this to enable Intel PowerClamp idle injection
> driver. This enforce idle time which results in more package C-state
> residency. The diff --git a/drivers/thermal/intel_powerclamp.c
> b/drivers/thermal/intel_powerclamp.c index b40b37c..3ff350b 100644
> --- a/drivers/thermal/intel_powerclamp.c
> +++ b/drivers/thermal/intel_powerclamp.c
> @@ -52,12 +52,18 @@
>  #include <linux/seq_file.h>
>  #include <linux/sched/rt.h>
>  
> +#include <asm/hardirq.h>
> +#ifdef CONFIG_X86
>  #include <asm/nmi.h>
>  #include <asm/msr.h>
>  #include <asm/mwait.h>
>  #include <asm/cpu_device_id.h>
>  #include <asm/idle.h>
> -#include <asm/hardirq.h>
> +
> +static unsigned int target_mwait;
> +#elif CONFIG_ARM
> +#include <asm/proc-fns.h>
> +#endif
>  
>  #define MAX_TARGET_RATIO (50U)
>  /* For each undisturbed clamping period (no extra wake ups during
> idle time), @@ -71,7 +77,6 @@
>   */
>  #define DEFAULT_DURATION_JIFFIES (6)
>  
> -static unsigned int target_mwait;
>  static struct dentry *debug_dir;
>  
>  /* user selected target */
> @@ -178,6 +183,7 @@ MODULE_PARM_DESC(window_size, "sliding window in
> number of clamping cycles\n" "\twindow size results in slower
> response time but more smooth\n" "\tclamping results. default to 2.");
>  
> +#ifdef CONFIG_X86
>  static void find_target_mwait(void)
>  {
>  	unsigned int eax, ebx, ecx, edx;
> @@ -246,6 +252,7 @@ static u64 pkg_state_counter(void)
>  
>  	return count;
>  }
> +#endif
>  
>  static void noop_timer(unsigned long foo)
>  {
> @@ -316,6 +323,25 @@ static void adjust_compensation(int
> target_ratio, unsigned int win) }
>  }
>  
> +#ifdef CONFIG_X86
> +static void inline powerclamp_get_cstate_inform(u64 *msr_now, u64
> *tsc_now) +{
> +	*msr_now = pkg_state_counter();
> +	rdtscll(*tsc_now);
> +}
> +#elif CONFIG_ARM
> +static void inline powerclamp_get_cstate_inform(u64 *idle, u64 *wall)
> +{
> +	u64 _wall;
> +	int cpu;
> +
> +	for_each_online_cpu(cpu) {
> +		*idle += get_cpu_idle_time_us(cpu, &_wall);
> +		*wall += _wall;
> +	}
> +}
> +#endif
> +
>  static bool powerclamp_adjust_controls(unsigned int target_ratio,
>  				unsigned int guard, unsigned int win)
>  {
> @@ -324,8 +350,7 @@ static bool powerclamp_adjust_controls(unsigned
> int target_ratio, u64 val64;
>  
>  	/* check result for the last window */
> -	msr_now = pkg_state_counter();
> -	rdtscll(tsc_now);
> +	powerclamp_get_cstate_inform(&msr_now, &tsc_now);
>  
This function name is misleading, it is better to separate clock source
data and idle state counter.
>  	/* calculate pkg cstate vs tsc ratio */
>  	if (!msr_last || !tsc_last)
> @@ -353,6 +378,34 @@ static bool powerclamp_adjust_controls(unsigned
> int target_ratio, return set_target_ratio + guard <= current_ratio;
>  }
>  
> +#ifdef CONFIG_X86
> +static void powerclamp_enter_idle(void)
> +{
> +	unsigned long ecx = 1;
> +	unsigned long eax = target_mwait;
> +
> +	/*
> +	 * REVISIT: may call enter_idle() to notify drivers who
> +	 * can save power during cpu idle. same for exit_idle()
> +	 */
> +	local_touch_nmi();
> +	stop_critical_timings();
> +	__monitor((void *)&current_thread_info()->flags, 0, 0);
> +	cpu_relax(); /* allow HT sibling to run */
> +	__mwait(eax, ecx);
> +	start_critical_timings();
> +	atomic_inc(&idle_wakeup_counter);
> +}
> +#elif CONFIG_ARM

#elif defined(CONFIG_ARM)
otherwise, get compiler warning on x86

> +static void  powerclamp_enter_idle(void)
> +{
> +	stop_critical_timings();
> +	cpu_do_idle();
> +	start_critical_timings();
> +	atomic_inc(&idle_wakeup_counter);
> +}
> +#endif
> +
>  static int clamp_thread(void *arg)
>  {
>  	int cpunr = (unsigned long)arg;
> @@ -428,22 +481,9 @@ static int clamp_thread(void *arg)
>  		preempt_disable();
>  		tick_nohz_idle_enter();
>  		/* mwait until target jiffies is reached */
> -		while (time_before(jiffies, target_jiffies)) {
> -			unsigned long ecx = 1;
> -			unsigned long eax = target_mwait;
> -
> -			/*
> -			 * REVISIT: may call enter_idle() to notify
> drivers who
> -			 * can save power during cpu idle. same for
> exit_idle()
> -			 */
> -			local_touch_nmi();
> -			stop_critical_timings();
> -			__monitor((void
> *)&current_thread_info()->flags, 0, 0);
> -			cpu_relax(); /* allow HT sibling to run */
> -			__mwait(eax, ecx);
> -			start_critical_timings();
> -			atomic_inc(&idle_wakeup_counter);
> -		}
> +		while (time_before(jiffies, target_jiffies))
> +			powerclamp_enter_idle();
> +
>  		tick_nohz_idle_exit();
>  		preempt_enable_no_resched();
>  	}
> @@ -465,13 +505,12 @@ static void poll_pkg_cstate(struct work_struct
> *dummy) static u64 tsc_last;
>  	static unsigned long jiffies_last;
>  
> -	u64 msr_now;
> +	u64 msr_now = 0;
>  	unsigned long jiffies_now;
> -	u64 tsc_now;
> +	u64 tsc_now = 0;
>  	u64 val64;
>  
> -	msr_now = pkg_state_counter();
> -	rdtscll(tsc_now);
> +	powerclamp_get_cstate_inform(&msr_now, &tsc_now);
>  	jiffies_now = jiffies;
>  
>  	/* calculate pkg cstate vs tsc ratio */
> @@ -499,11 +538,13 @@ static int start_power_clamp(void)
>  	unsigned long cpu;
>  	struct task_struct *thread;
>  
> +#ifdef CONFIG_X86
>  	/* check if pkg cstate counter is completely 0, abort in
> this case */ if (!pkg_state_counter()) {
>  		pr_err("pkg cstate counter not functional, abort\n");
>  		return -EINVAL;
>  	}
> +#endif
>  
>  	set_target_ratio = clamp(set_target_ratio, 0U,
> MAX_TARGET_RATIO - 1); /* prevent cpu hotplug */
> @@ -661,6 +702,7 @@ static struct thermal_cooling_device_ops
> powerclamp_cooling_ops = { .set_cur_state = powerclamp_set_cur_state,
>  };
>  
> +#ifdef CONFIG_X86
>  /* runs on Nehalem and later */
>  static const struct x86_cpu_id intel_powerclamp_ids[] = {
>  	{ X86_VENDOR_INTEL, 6, 0x1a},
> @@ -678,9 +720,11 @@ static const struct x86_cpu_id
> intel_powerclamp_ids[] = { {}
>  };
>  MODULE_DEVICE_TABLE(x86cpu, intel_powerclamp_ids);
> +#endif
>  
>  static int powerclamp_probe(void)
>  {
> +#ifdef CONFIG_X86
>  	if (!x86_match_cpu(intel_powerclamp_ids)) {
>  		pr_err("Intel powerclamp does not run on family %d
> model %d\n", boot_cpu_data.x86, boot_cpu_data.x86_model);
> @@ -694,7 +738,7 @@ static int powerclamp_probe(void)
>  
>  	/* find the deepest mwait value */
>  	find_target_mwait();
> -
> +#endif
>  	return 0;
>  }
>  

[Jacob Pan]

-- 
Thanks,

Jacob

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-07-29 18:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-25 10:22 [RFC PATCH] thermal: powerclamp: Add support for ARM machine to powerclamp driver Jonghwa Lee
2013-07-25 13:16 ` Eduardo Valentin
2013-07-29 18:11 ` Jacob Pan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.