CPUFREQ: Ondemand and powernow-k8 enhancements/fixes/cleanups

CPUFREQ: Ondemand and powernow-k8 enhancements/fixes/cleanups

[Thomas Renninger]

These are the same I sent out some days ago, but messed up the
cpufreq list address.

The patches are based against Dave's cpufreq git tree's fixes branch.
There is also the one from Mark Langsdorf (adjusted to latest tree)
included which got posted on the cpufreq list recently.
guilt should add the
From: Mark Langsdorf ...
tag right, so that he shows up as the author of this one.

Dave, can you add these to your tree, please.

Thanks,

    Thomas

[PATCH 1/7] CPUFREQ: Introduce /sys/devices/system/cpu/cpu*/cpufreq/cpuinfo_transition_latency

[Thomas Renninger]

It's not only useful for the ondemand and conservative governors, but
also for userspace daemons to know about the HW transition latency of
the CPU.
It is especially useful for userspace to know about this value when
the ondemand or conservative governors are run. The sampling rate
control value depends on it and for userspace being able to set sane
tuning values there it has to know about the transition latency.

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 Documentation/cpu-freq/user-guide.txt |   12 ++++++++++++
 drivers/cpufreq/cpufreq.c             |    3 +++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt
index e3443dd..57f23cb 100644
--- a/Documentation/cpu-freq/user-guide.txt
+++ b/Documentation/cpu-freq/user-guide.txt
@@ -152,6 +152,18 @@ cpuinfo_min_freq :		this file shows the minimum operating
 				frequency the processor can run at(in kHz) 
 cpuinfo_max_freq :		this file shows the maximum operating
 				frequency the processor can run at(in kHz) 
+cpuinfo_transition_latency	The time it takes on this CPU to
+				switch between two frequencies in nano
+				seconds. If unknown or known to be
+				that high that the driver does not
+				work with the ondemand governor, -1
+				(CPUFREQ_ETERNAL) will be returned.
+				Using this information can be useful
+				to choose an appropriate polling
+				frequency for a kernel governor or
+				userspace daemon. Make sure to not
+				switch the frequency too often
+				resulting in performance loss.
 scaling_driver :		this file shows what cpufreq driver is
 				used to set the frequency on this CPU
 
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b55cb67..ae92986 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -455,6 +455,7 @@ static ssize_t show_##file_name				\
 
 show_one(cpuinfo_min_freq, cpuinfo.min_freq);
 show_one(cpuinfo_max_freq, cpuinfo.max_freq);
+show_one(cpuinfo_transition_latency, cpuinfo.transition_latency);
 show_one(scaling_min_freq, min);
 show_one(scaling_max_freq, max);
 show_one(scaling_cur_freq, cur);
@@ -660,6 +661,7 @@ __ATTR(_name, 0644, show_##_name, store_##_name)
 define_one_ro0400(cpuinfo_cur_freq);
 define_one_ro(cpuinfo_min_freq);
 define_one_ro(cpuinfo_max_freq);
+define_one_ro(cpuinfo_transition_latency);
 define_one_ro(scaling_available_governors);
 define_one_ro(scaling_driver);
 define_one_ro(scaling_cur_freq);
@@ -673,6 +675,7 @@ define_one_rw(scaling_setspeed);
 static struct attribute *default_attrs[] = {
 	&cpuinfo_min_freq.attr,
 	&cpuinfo_max_freq.attr,
+	&cpuinfo_transition_latency.attr,
 	&scaling_min_freq.attr,
 	&scaling_max_freq.attr,
 	&affected_cpus.attr,
-- 
1.6.0.2

[PATCH 2/7] CPUFREQ: ondemand/conservative: deprecate sampling_rate{min,max}

[Thomas Renninger]

The same info can be obtained via the transition_latency sysfs file

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 Documentation/cpu-freq/governors.txt   |   10 ++++++++--
 drivers/cpufreq/cpufreq_conservative.c |    4 ++++
 drivers/cpufreq/cpufreq_ondemand.c     |    5 +++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/cpu-freq/governors.txt b/Documentation/cpu-freq/governors.txt
index 5b0cfa6..9b18512 100644
--- a/Documentation/cpu-freq/governors.txt
+++ b/Documentation/cpu-freq/governors.txt
@@ -119,8 +119,14 @@ want the kernel to look at the CPU usage and to make decisions on
 what to do about the frequency.  Typically this is set to values of
 around '10000' or more.
 
-show_sampling_rate_(min|max): the minimum and maximum sampling rates
-available that you may set 'sampling_rate' to.
+show_sampling_rate_(min|max): THIS INTERFACE IS DEPRECATED, DON'T USE IT.
+You can use wider ranges now and the general
+cpuinfo_transition_latency variable (cmp. with user-guide.txt) can be
+used to obtain exactly the same info:
+show_sampling_rate_min = transtition_latency * 500    / 1000
+show_sampling_rate_max = transtition_latency * 500000 / 1000
+(divided by 1000 is to illustrate that sampling rate is in us and
+transition latency is exported ns).
 
 up_threshold: defines what the average CPU usage between the samplings
 of 'sampling_rate' needs to be for the kernel to make a decision on
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 0320962..5ba0a3f 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -140,11 +140,15 @@ static struct notifier_block dbs_cpufreq_notifier_block = {
 /************************** sysfs interface ************************/
 static ssize_t show_sampling_rate_max(struct cpufreq_policy *policy, char *buf)
 {
+	printk(KERN_INFO "CPUFREQ: conservative sampling_rate_max "
+	       "sysfs file is deprecated - used by: %s\n", current->comm);
 	return sprintf (buf, "%u\n", MAX_SAMPLING_RATE);
 }
 
 static ssize_t show_sampling_rate_min(struct cpufreq_policy *policy, char *buf)
 {
+	printk(KERN_INFO "CPUFREQ: conservative sampling_rate_max "
+	       "sysfs file is deprecated - used by: %s\n", current->comm);
 	return sprintf (buf, "%u\n", MIN_SAMPLING_RATE);
 }
 
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 6f45b16..4b42e36 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -21,6 +21,7 @@
 #include <linux/hrtimer.h>
 #include <linux/tick.h>
 #include <linux/ktime.h>
+#include <linux/sched.h>
 
 /*
  * dbs is used in this file as a shortform for demandbased switching
@@ -203,11 +204,15 @@ static void ondemand_powersave_bias_init(void)
 /************************** sysfs interface ************************/
 static ssize_t show_sampling_rate_max(struct cpufreq_policy *policy, char *buf)
 {
+	printk(KERN_INFO "CPUFREQ: ondemand sampling_rate_max "
+	       "sysfs file is deprecated - used by: %s\n", current->comm);
 	return sprintf (buf, "%u\n", MAX_SAMPLING_RATE);
 }
 
 static ssize_t show_sampling_rate_min(struct cpufreq_policy *policy, char *buf)
 {
+	printk(KERN_INFO "CPUFREQ: ondemand sampling_rate_min "
+	       "sysfs file is deprecated - used by: %s\n", current->comm);
 	return sprintf (buf, "%u\n", MIN_SAMPLING_RATE);
 }
 
-- 
1.6.0.2

Re: [PATCH 2/7] CPUFREQ: ondemand/conservative: deprecate sampling_rate{min,max}

[Andrew Morton]

On Tue,  3 Feb 2009 17:46:41 +0100 Thomas Renninger <trenn@suse.de> wrote:

>  static ssize_t show_sampling_rate_max(struct cpufreq_policy *policy, char *buf)
>  {
> +	printk(KERN_INFO "CPUFREQ: conservative sampling_rate_max "
> +	       "sysfs file is deprecated - used by: %s\n", current->comm);
>  	return sprintf (buf, "%u\n", MAX_SAMPLING_RATE);
>  }

I'd have thought that the user-irritation risk here is pretty high.

Would it make sense to throttle these or to make them once-per-boot
or something?

[PATCH 3/7] CPUFREQ: ondemand/conservative: sanitize sampling_rate restrictions

[Thomas Renninger]

Limit sampling rate to transition_latency * 100 or kernel limits.
If sampling_rate is tried to be set too low, set the lowest allowed value.

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 Documentation/cpu-freq/governors.txt   |   14 +++++++++++++-
 drivers/cpufreq/cpufreq_conservative.c |   19 ++++++++++++++++---
 drivers/cpufreq/cpufreq_ondemand.c     |   19 +++++++++++++++----
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/Documentation/cpu-freq/governors.txt b/Documentation/cpu-freq/governors.txt
index 9b18512..ce73f3e 100644
--- a/Documentation/cpu-freq/governors.txt
+++ b/Documentation/cpu-freq/governors.txt
@@ -117,7 +117,19 @@ accessible parameters:
 sampling_rate: measured in uS (10^-6 seconds), this is how often you
 want the kernel to look at the CPU usage and to make decisions on
 what to do about the frequency.  Typically this is set to values of
-around '10000' or more.
+around '10000' or more. It's default value is (cmp. with users-guide.txt):
+transition_latency * 1000
+The lowest value you can set is:
+transition_latency * 100 or it may get restricted to a value where it
+makes not sense for the kernel anymore to poll that often which depends
+on your HZ config variable (HZ=1000: max=20000us, HZ=250: max=5000).
+Be aware that transition latency is in ns and sampling_rate is in us, so you
+get the same sysfs value by default.
+Sampling rate should always get adjusted considering the transition latency
+To set the sampling rate 750 times as high as the transition latency
+in the bash (as said, 1000 is default), do:
+echo `$(($(cat cpuinfo_transition_latency) * 750 / 1000)) \
+    >ondemand/sampling_rate
 
 show_sampling_rate_(min|max): THIS INTERFACE IS DEPRECATED, DON'T USE IT.
 You can use wider ranges now and the general
diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 5ba0a3f..b8bbd4a 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -54,7 +54,18 @@ static unsigned int def_sampling_rate;
 			(MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
 #define MIN_SAMPLING_RATE			\
 			(def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
+/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
+ * Define the minimal settable sampling rate to the greater of:
+ *   - "HW transition latency" * 100 (same as default sampling / 10)
+ *   - MIN_STAT_SAMPLING_RATE
+ * To avoid that userspace shoots itself.
+*/
+#define MINIMUM_SAMPLING_RATE			\
+			((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
+			 ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))
+/* This will also vanish soon with removing sampling_rate_max */
 #define MAX_SAMPLING_RATE			(500 * def_sampling_rate)
+
 #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER	(1000)
 #define DEF_SAMPLING_DOWN_FACTOR		(1)
 #define MAX_SAMPLING_DOWN_FACTOR		(10)
@@ -197,12 +208,14 @@ static ssize_t store_sampling_rate(struct cpufreq_policy *unused,
 	ret = sscanf (buf, "%u", &input);
 
 	mutex_lock(&dbs_mutex);
-	if (ret != 1 || input > MAX_SAMPLING_RATE || input < MIN_SAMPLING_RATE) {
+	if (ret != 1) {
 		mutex_unlock(&dbs_mutex);
 		return -EINVAL;
 	}
-
-	dbs_tuners_ins.sampling_rate = input;
+	if  (input < MINIMUM_SAMPLING_RATE)
+		dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
+	else
+		dbs_tuners_ins.sampling_rate = input;
 	mutex_unlock(&dbs_mutex);
 
 	return count;
diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
index 4b42e36..70e0841 100644
--- a/drivers/cpufreq/cpufreq_ondemand.c
+++ b/drivers/cpufreq/cpufreq_ondemand.c
@@ -52,6 +52,16 @@ static unsigned int def_sampling_rate;
 			(MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
 #define MIN_SAMPLING_RATE			\
 			(def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
+/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
+ * Define the minimal settable sampling rate to the greater of:
+ *   - "HW transition latency" * 100 (same as default sampling / 10)
+ *   - MIN_STAT_SAMPLING_RATE
+ * To avoid that userspace shoots itself.
+*/
+#define MINIMUM_SAMPLING_RATE			\
+			((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
+			 ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))
+/* This will also vanish soon with removing sampling_rate_max */
 #define MAX_SAMPLING_RATE			(500 * def_sampling_rate)
 #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER	(1000)
 #define TRANSITION_LATENCY_LIMIT		(10 * 1000 * 1000)
@@ -243,13 +253,14 @@ static ssize_t store_sampling_rate(struct cpufreq_policy *unused,
 	ret = sscanf(buf, "%u", &input);
 
 	mutex_lock(&dbs_mutex);
-	if (ret != 1 || input > MAX_SAMPLING_RATE
-		     || input < MIN_SAMPLING_RATE) {
+	if (ret != 1) {
 		mutex_unlock(&dbs_mutex);
 		return -EINVAL;
 	}
-
-	dbs_tuners_ins.sampling_rate = input;
+	if  (input < MINIMUM_SAMPLING_RATE)
+		dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
+	else
+		dbs_tuners_ins.sampling_rate = input;
 	mutex_unlock(&dbs_mutex);
 
 	return count;
-- 
1.6.0.2

Re: [PATCH 3/7] CPUFREQ: ondemand/conservative: sanitize sampling_rate restrictions

[Andrew Morton]

On Tue,  3 Feb 2009 17:46:42 +0100 Thomas Renninger <trenn@suse.de> wrote:

> Limit sampling rate to transition_latency * 100 or kernel limits.
> If sampling_rate is tried to be set too low, set the lowest allowed value.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> ---
>  Documentation/cpu-freq/governors.txt   |   14 +++++++++++++-
>  drivers/cpufreq/cpufreq_conservative.c |   19 ++++++++++++++++---
>  drivers/cpufreq/cpufreq_ondemand.c     |   19 +++++++++++++++----
>  3 files changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/governors.txt b/Documentation/cpu-freq/governors.txt
> index 9b18512..ce73f3e 100644
> --- a/Documentation/cpu-freq/governors.txt
> +++ b/Documentation/cpu-freq/governors.txt
> @@ -117,7 +117,19 @@ accessible parameters:
>  sampling_rate: measured in uS (10^-6 seconds), this is how often you
>  want the kernel to look at the CPU usage and to make decisions on
>  what to do about the frequency.  Typically this is set to values of
> -around '10000' or more.
> +around '10000' or more. It's default value is (cmp. with users-guide.txt):
> +transition_latency * 1000
> +The lowest value you can set is:
> +transition_latency * 100 or it may get restricted to a value where it
> +makes not sense for the kernel anymore to poll that often which depends
> +on your HZ config variable (HZ=1000: max=20000us, HZ=250: max=5000).
> +Be aware that transition latency is in ns and sampling_rate is in us, so you
> +get the same sysfs value by default.
> +Sampling rate should always get adjusted considering the transition latency
> +To set the sampling rate 750 times as high as the transition latency
> +in the bash (as said, 1000 is default), do:
> +echo `$(($(cat cpuinfo_transition_latency) * 750 / 1000)) \
> +    >ondemand/sampling_rate
>  
>  show_sampling_rate_(min|max): THIS INTERFACE IS DEPRECATED, DON'T USE IT.
>  You can use wider ranges now and the general
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index 5ba0a3f..b8bbd4a 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -54,7 +54,18 @@ static unsigned int def_sampling_rate;
>  			(MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
>  #define MIN_SAMPLING_RATE			\
>  			(def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
> +/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
> + * Define the minimal settable sampling rate to the greater of:
> + *   - "HW transition latency" * 100 (same as default sampling / 10)
> + *   - MIN_STAT_SAMPLING_RATE
> + * To avoid that userspace shoots itself.
> +*/
> +#define MINIMUM_SAMPLING_RATE			\
> +			((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
> +			 ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))

Use
	max(def_sampling_rate / 10, MIN_STAT_SAMPLING_RATE)
?

But really, this should be a plain old C function.  Hiding a bunch of C
code inside a macro which purports to be a compile-time constant is rude.

> +/* This will also vanish soon with removing sampling_rate_max */
>  #define MAX_SAMPLING_RATE			(500 * def_sampling_rate)

Ditto, really.

> +
>  #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER	(1000)
>  #define DEF_SAMPLING_DOWN_FACTOR		(1)
>  #define MAX_SAMPLING_DOWN_FACTOR		(10)
> @@ -197,12 +208,14 @@ static ssize_t store_sampling_rate(struct cpufreq_policy *unused,
>  	ret = sscanf (buf, "%u", &input);
>  
>  	mutex_lock(&dbs_mutex);
> -	if (ret != 1 || input > MAX_SAMPLING_RATE || input < MIN_SAMPLING_RATE) {
> +	if (ret != 1) {
>  		mutex_unlock(&dbs_mutex);
>  		return -EINVAL;
>  	}
> -
> -	dbs_tuners_ins.sampling_rate = input;
> +	if  (input < MINIMUM_SAMPLING_RATE)
> +		dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
> +	else
> +		dbs_tuners_ins.sampling_rate = input;

	dbs_tuners_ins.sampling_rate = max(MINIMUM_SAMPLING_RATE, input);

>  	mutex_unlock(&dbs_mutex);
>  
>  	return count;
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index 4b42e36..70e0841 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -52,6 +52,16 @@ static unsigned int def_sampling_rate;
>  			(MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
>  #define MIN_SAMPLING_RATE			\
>  			(def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
> +/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
> + * Define the minimal settable sampling rate to the greater of:
> + *   - "HW transition latency" * 100 (same as default sampling / 10)
> + *   - MIN_STAT_SAMPLING_RATE
> + * To avoid that userspace shoots itself.
> +*/
> +#define MINIMUM_SAMPLING_RATE			\
> +			((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
> +			 ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))
> +/* This will also vanish soon with removing sampling_rate_max */
>  #define MAX_SAMPLING_RATE			(500 * def_sampling_rate)
>  #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER	(1000)
>  #define TRANSITION_LATENCY_LIMIT		(10 * 1000 * 1000)
> @@ -243,13 +253,14 @@ static ssize_t store_sampling_rate(struct cpufreq_policy *unused,
>  	ret = sscanf(buf, "%u", &input);
>  
>  	mutex_lock(&dbs_mutex);
> -	if (ret != 1 || input > MAX_SAMPLING_RATE
> -		     || input < MIN_SAMPLING_RATE) {
> +	if (ret != 1) {
>  		mutex_unlock(&dbs_mutex);
>  		return -EINVAL;
>  	}
> -
> -	dbs_tuners_ins.sampling_rate = input;
> +	if  (input < MINIMUM_SAMPLING_RATE)
> +		dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
> +	else
> +		dbs_tuners_ins.sampling_rate = input;
>  	mutex_unlock(&dbs_mutex);

etceterata.

Re: [PATCH 3/7] CPUFREQ: ondemand/conservative: sanitize sampling_rate restrictions

[Thomas Renninger]

On Wednesday 04 February 2009 06:07:25 Andrew Morton wrote:
> On Tue,  3 Feb 2009 17:46:42 +0100 Thomas Renninger <trenn@suse.de> wrote:
> > Limit sampling rate to transition_latency * 100 or kernel limits.
> > If sampling_rate is tried to be set too low, set the lowest allowed
> > value.
> >
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > ---
> >  Documentation/cpu-freq/governors.txt   |   14 +++++++++++++-
> >  drivers/cpufreq/cpufreq_conservative.c |   19 ++++++++++++++++---
> >  drivers/cpufreq/cpufreq_ondemand.c     |   19 +++++++++++++++----
> >  3 files changed, 44 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/cpu-freq/governors.txt
> > b/Documentation/cpu-freq/governors.txt index 9b18512..ce73f3e 100644
> > --- a/Documentation/cpu-freq/governors.txt
> > +++ b/Documentation/cpu-freq/governors.txt
> > @@ -117,7 +117,19 @@ accessible parameters:
> >  sampling_rate: measured in uS (10^-6 seconds), this is how often you
> >  want the kernel to look at the CPU usage and to make decisions on
> >  what to do about the frequency.  Typically this is set to values of
> > -around '10000' or more.
> > +around '10000' or more. It's default value is (cmp. with
> > users-guide.txt): +transition_latency * 1000
> > +The lowest value you can set is:
> > +transition_latency * 100 or it may get restricted to a value where it
> > +makes not sense for the kernel anymore to poll that often which depends
> > +on your HZ config variable (HZ=1000: max=20000us, HZ=250: max=5000).
> > +Be aware that transition latency is in ns and sampling_rate is in us, so
> > you +get the same sysfs value by default.
> > +Sampling rate should always get adjusted considering the transition
> > latency +To set the sampling rate 750 times as high as the transition
> > latency +in the bash (as said, 1000 is default), do:
> > +echo `$(($(cat cpuinfo_transition_latency) * 750 / 1000)) \
> > +    >ondemand/sampling_rate
> >
> >  show_sampling_rate_(min|max): THIS INTERFACE IS DEPRECATED, DON'T USE
> > IT. You can use wider ranges now and the general
> > diff --git a/drivers/cpufreq/cpufreq_conservative.c
> > b/drivers/cpufreq/cpufreq_conservative.c index 5ba0a3f..b8bbd4a 100644
> > --- a/drivers/cpufreq/cpufreq_conservative.c
> > +++ b/drivers/cpufreq/cpufreq_conservative.c
> > @@ -54,7 +54,18 @@ static unsigned int def_sampling_rate;
> >  			(MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
> >  #define MIN_SAMPLING_RATE			\
> >  			(def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
This one will vanish after the deprecation time expired.

> > +/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
> > + * Define the minimal settable sampling rate to the greater of:
> > + *   - "HW transition latency" * 100 (same as default sampling / 10)
> > + *   - MIN_STAT_SAMPLING_RATE
> > + * To avoid that userspace shoots itself.
> > +*/
> > +#define MINIMUM_SAMPLING_RATE			\
> > +			((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
> > +			 ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))
>
> Use
> 	max(def_sampling_rate / 10, MIN_STAT_SAMPLING_RATE)
> ?
Ok.

> But really, this should be a plain old C function.  Hiding a bunch of C
> code inside a macro which purports to be a compile-time constant is rude.
>
> > +/* This will also vanish soon with removing sampling_rate_max */
> >  #define MAX_SAMPLING_RATE			(500 * def_sampling_rate)
>
> Ditto, really.
This one will also be removed after the deprecation time expired.
Therefore I'd prefer to not touch it now and leave it as it is,
but I will remove these with finally removing sampling_rate_{min,max}.

I will also use max(..) where suggested and repost the three, modified
patches.
Thanks for your detailed, review. It's appreciated!

     Thomas

> > +
> >  #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER	(1000)
> >  #define DEF_SAMPLING_DOWN_FACTOR		(1)
> >  #define MAX_SAMPLING_DOWN_FACTOR		(10)
> > @@ -197,12 +208,14 @@ static ssize_t store_sampling_rate(struct
> > cpufreq_policy *unused, ret = sscanf (buf, "%u", &input);
> >
> >  	mutex_lock(&dbs_mutex);
> > -	if (ret != 1 || input > MAX_SAMPLING_RATE || input < MIN_SAMPLING_RATE)
> > { +	if (ret != 1) {
> >  		mutex_unlock(&dbs_mutex);
> >  		return -EINVAL;
> >  	}
> > -
> > -	dbs_tuners_ins.sampling_rate = input;
> > +	if  (input < MINIMUM_SAMPLING_RATE)
> > +		dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
> > +	else
> > +		dbs_tuners_ins.sampling_rate = input;
>
> 	dbs_tuners_ins.sampling_rate = max(MINIMUM_SAMPLING_RATE, input);
>
> >  	mutex_unlock(&dbs_mutex);
> >
> >  	return count;
> > diff --git a/drivers/cpufreq/cpufreq_ondemand.c
> > b/drivers/cpufreq/cpufreq_ondemand.c index 4b42e36..70e0841 100644
> > --- a/drivers/cpufreq/cpufreq_ondemand.c
> > +++ b/drivers/cpufreq/cpufreq_ondemand.c
> > @@ -52,6 +52,16 @@ static unsigned int def_sampling_rate;
> >  			(MIN_SAMPLING_RATE_RATIO * jiffies_to_usecs(10))
> >  #define MIN_SAMPLING_RATE			\
> >  			(def_sampling_rate / MIN_SAMPLING_RATE_RATIO)
> > +/* Above MIN_SAMPLING_RATE will vanish with its sysfs file soon
> > + * Define the minimal settable sampling rate to the greater of:
> > + *   - "HW transition latency" * 100 (same as default sampling / 10)
> > + *   - MIN_STAT_SAMPLING_RATE
> > + * To avoid that userspace shoots itself.
> > +*/
> > +#define MINIMUM_SAMPLING_RATE			\
> > +			((def_sampling_rate / 10) < MIN_STAT_SAMPLING_RATE \
> > +			 ? MIN_STAT_SAMPLING_RATE : (def_sampling_rate / 10))
> > +/* This will also vanish soon with removing sampling_rate_max */
> >  #define MAX_SAMPLING_RATE			(500 * def_sampling_rate)
> >  #define DEF_SAMPLING_RATE_LATENCY_MULTIPLIER	(1000)
> >  #define TRANSITION_LATENCY_LIMIT		(10 * 1000 * 1000)
> > @@ -243,13 +253,14 @@ static ssize_t store_sampling_rate(struct
> > cpufreq_policy *unused, ret = sscanf(buf, "%u", &input);
> >
> >  	mutex_lock(&dbs_mutex);
> > -	if (ret != 1 || input > MAX_SAMPLING_RATE
> > -		     || input < MIN_SAMPLING_RATE) {
> > +	if (ret != 1) {
> >  		mutex_unlock(&dbs_mutex);
> >  		return -EINVAL;
> >  	}
> > -
> > -	dbs_tuners_ins.sampling_rate = input;
> > +	if  (input < MINIMUM_SAMPLING_RATE)
> > +		dbs_tuners_ins.sampling_rate = MINIMUM_SAMPLING_RATE;
> > +	else
> > +		dbs_tuners_ins.sampling_rate = input;
> >  	mutex_unlock(&dbs_mutex);
>
> etceterata.
> --
> To unsubscribe from this list: send the line "unsubscribe cpufreq" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] CPUFREQ: powernow-k8: Get transition latency from ACPI _PSS table

[Thomas Renninger]

From: Mark Langsdorf <mark.langsdorf@amd.com>

At this time, the PowerNow! driver for K8 uses an experimentally
derived formula to calculate transition latency.  The value it
provides is orders of magnitude too large on modern systems.
This patch replaces the formula with ACPI _PSS latency values
for more accuracy and better performance.

I've tested it on two 2nd generation Opteron systems, a 3rd
generation Operton system, and a Turion X2 without seeing any
stability problems.

Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index 5c28b37..fb039cd 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -939,10 +939,25 @@ static void powernow_k8_cpu_exit_acpi(struct powernow_k8_data *data)
 	free_cpumask_var(data->acpi_data.shared_cpu_map);
 }
 
+static int get_transition_latency(struct powernow_k8_data *data)
+{
+	int max_latency = 0;
+	int i;
+	for (i = 0; i < data->acpi_data.state_count; i++) {
+		int cur_latency = data->acpi_data.states[i].transition_latency
+			+ data->acpi_data.states[i].bus_master_latency;
+		if (cur_latency > max_latency)
+			max_latency = cur_latency;
+	}
+	/* value in usecs, needs to be in nanoseconds */
+	return 1000 * max_latency;
+}
+
 #else
 static int powernow_k8_cpu_init_acpi(struct powernow_k8_data *data) { return -ENODEV; }
 static void powernow_k8_cpu_exit_acpi(struct powernow_k8_data *data) { return; }
 static void powernow_k8_acpi_pst_values(struct powernow_k8_data *data, unsigned int index) { return; }
+static int get_transition_latency(struct powernow_k8_data *data) { return 0; }
 #endif /* CONFIG_X86_POWERNOW_K8_ACPI */
 
 /* Take a frequency, and issue the fid/vid transition command */
@@ -1173,7 +1188,13 @@ static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
 		if (rc) {
 			goto err_out;
 		}
-	}
+		/* Take a crude guess here.
+		 * That guess was in microseconds, so multiply with 1000 */
+		pol->cpuinfo.transition_latency = (
+			 ((data->rvo + 8) * data->vstable * VST_UNITS_20US) +
+			 ((1 << data->irt) * 30)) * 1000;
+	} else /* ACPI _PSS objects available */
+		pol->cpuinfo.transition_latency = get_transition_latency(data);
 
 	/* only run on specific CPU from here on */
 	oldmask = current->cpus_allowed;
@@ -1204,11 +1225,6 @@ static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
 		cpumask_copy(pol->cpus, &per_cpu(cpu_core_map, pol->cpu));
 	data->available_cores = pol->cpus;
 
-	/* Take a crude guess here.
-	 * That guess was in microseconds, so multiply with 1000 */
-	pol->cpuinfo.transition_latency = (((data->rvo + 8) * data->vstable * VST_UNITS_20US)
-	    + (3 * (1 << data->irt) * 10)) * 1000;
-
 	if (cpu_family == CPU_HW_PSTATE)
 		pol->cur = find_khz_freq_from_pstate(data->powernow_table, data->currpstate);
 	else
-- 
1.6.0.2

Re: [PATCH 4/7] CPUFREQ: powernow-k8: Get transition latency from ACPI _PSS table

[Robert Hancock]

Thomas Renninger wrote:
> From: Mark Langsdorf <mark.langsdorf@amd.com>
> 
> At this time, the PowerNow! driver for K8 uses an experimentally
> derived formula to calculate transition latency.  The value it
> provides is orders of magnitude too large on modern systems.
> This patch replaces the formula with ACPI _PSS latency values
> for more accuracy and better performance.
> 
> I've tested it on two 2nd generation Opteron systems, a 3rd
> generation Operton system, and a Turion X2 without seeing any
> stability problems.
> 
> Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> ---
>  arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   28 ++++++++++++++++++++++------
>  1 files changed, 22 insertions(+), 6 deletions(-)

Hopefully somebody is pushing this for .29? This seems to make a big 
difference in system responsiveness on my Athlon X2 system.

[PATCH 5/7] CPUFREQ: powernow-k8: Always compile powernow-k8 driver with ACPI support

[Thomas Renninger]

powernow-k8 driver should always try to get cpufreq info from ACPI.
Otherwise it will not be able to detect the transition latency correctly
which results in ondemand governor taking a wrong sampling rate which will
then result in sever performance loss.

Let the user not shoot himself in the foot and always compile in ACPI
support for powernow-k8.

This also fixes a wrong message if ACPI_PROCESSOR is compiled as a module and
#ifndef CONFIG_ACPI_PROCESSOR
path is chosen.

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 arch/x86/kernel/cpu/cpufreq/Kconfig       |   19 ++-----------------
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   17 -----------------
 arch/x86/kernel/cpu/cpufreq/powernow-k8.h |    5 +----
 3 files changed, 3 insertions(+), 38 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
index efae3b2..b63b0db 100644
--- a/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -87,30 +87,15 @@ config X86_POWERNOW_K7_ACPI
 config X86_POWERNOW_K8
 	tristate "AMD Opteron/Athlon64 PowerNow!"
 	select CPU_FREQ_TABLE
+	depends on ACPI && ACPI_PROCESSOR
 	help
-	  This adds the CPUFreq driver for mobile AMD Opteron/Athlon64 processors.
+	  This adds the CPUFreq driver for K8/K10 Opteron/Athlon64 processors.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called powernow-k8.
 
 	  For details, take a look at <file:Documentation/cpu-freq/>.
 
-	  If in doubt, say N.
-
-config X86_POWERNOW_K8_ACPI
-	bool
-	prompt "ACPI Support" if X86_32
-	depends on ACPI && X86_POWERNOW_K8 && ACPI_PROCESSOR
-	depends on !(X86_POWERNOW_K8 = y && ACPI_PROCESSOR = m)
-	default y
-	help
-	  This provides access to the K8s Processor Performance States via ACPI.
-	  This driver is probably required for CPUFreq to work with multi-socket and
-	  SMP systems.  It is not required on at least some single-socket yet
-	  multi-core systems, even if SMP is enabled.
-
-	  It is safe to say Y here.
-
 config X86_GX_SUSPMOD
 	tristate "Cyrix MediaGX/NatSemi Geode Suspend Modulation"
 	depends on X86_32 && PCI
diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index fb039cd..9accffb 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -38,11 +38,9 @@
 #include <asm/io.h>
 #include <asm/delay.h>
 
-#ifdef CONFIG_X86_POWERNOW_K8_ACPI
 #include <linux/acpi.h>
 #include <linux/mutex.h>
 #include <acpi/processor.h>
-#endif
 
 #define PFX "powernow-k8: "
 #define VERSION "version 2.20.00"
@@ -749,7 +747,6 @@ static int find_psb_table(struct powernow_k8_data *data)
 	return -ENODEV;
 }
 
-#ifdef CONFIG_X86_POWERNOW_K8_ACPI
 static void powernow_k8_acpi_pst_values(struct powernow_k8_data *data, unsigned int index)
 {
 	if (!data->acpi_data.state_count || (cpu_family == CPU_HW_PSTATE))
@@ -953,13 +950,6 @@ static int get_transition_latency(struct powernow_k8_data *data)
 	return 1000 * max_latency;
 }
 
-#else
-static int powernow_k8_cpu_init_acpi(struct powernow_k8_data *data) { return -ENODEV; }
-static void powernow_k8_cpu_exit_acpi(struct powernow_k8_data *data) { return; }
-static void powernow_k8_acpi_pst_values(struct powernow_k8_data *data, unsigned int index) { return; }
-static int get_transition_latency(struct powernow_k8_data *data) { return 0; }
-#endif /* CONFIG_X86_POWERNOW_K8_ACPI */
-
 /* Take a frequency, and issue the fid/vid transition command */
 static int transition_frequency_fidvid(struct powernow_k8_data *data, unsigned int index)
 {
@@ -1164,18 +1154,11 @@ static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
 		 * an UP version, and is deprecated by AMD.
 		 */
 		if (num_online_cpus() != 1) {
-#ifndef CONFIG_ACPI_PROCESSOR
-			printk(KERN_ERR PFX "ACPI Processor support is required "
-			       "for SMP systems but is absent. Please load the "
-			       "ACPI Processor module before starting this "
-			       "driver.\n");
-#else
 			printk(KERN_ERR FW_BUG PFX "Your BIOS does not provide"
 			       " ACPI _PSS objects in a way that Linux "
 			       "understands. Please report this to the Linux "
 			       "ACPI maintainers and complain to your BIOS "
 			       "vendor.\n");
-#endif
 			goto err_out;
 		}
 		if (pol->cpu != 0) {
diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.h b/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
index 8ecc75b..6c6698f 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.h
@@ -45,11 +45,10 @@ struct powernow_k8_data {
 	 * frequency is in kHz */
 	struct cpufreq_frequency_table  *powernow_table;
 
-#ifdef CONFIG_X86_POWERNOW_K8_ACPI
 	/* the acpi table needs to be kept. it's only available if ACPI was
 	 * used to determine valid frequency/vid/fid states */
 	struct acpi_processor_performance acpi_data;
-#endif
+
 	/* we need to keep track of associated cores, but let cpufreq
 	 * handle hotplug events - so just point at cpufreq pol->cpus
 	 * structure */
@@ -222,10 +221,8 @@ static int core_frequency_transition(struct powernow_k8_data *data, u32 reqfid);
 
 static void powernow_k8_acpi_pst_values(struct powernow_k8_data *data, unsigned int index);
 
-#ifdef CONFIG_X86_POWERNOW_K8_ACPI
 static int fill_powernow_table_pstate(struct powernow_k8_data *data, struct cpufreq_frequency_table *powernow_table);
 static int fill_powernow_table_fidvid(struct powernow_k8_data *data, struct cpufreq_frequency_table *powernow_table);
-#endif
 
 #ifdef CONFIG_SMP
 static inline void define_siblings(int cpu, cpumask_t cpu_sharedcore_mask[])
-- 
1.6.0.2

[PATCH 6/7] CPUFREQ: powernow-k8: Only print error message once, not per core.

[Thomas Renninger]

This is the typical message you get if you plug in a CPU
which is newer than your BIOS. It's annoying seeing this
message for each core.

Signed-off-by: Thomas Renninger <trenn@suse.de>
---
 arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
index 9accffb..9e312c5 100644
--- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
+++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
@@ -743,7 +743,7 @@ static int find_psb_table(struct powernow_k8_data *data)
 	 * BIOS and Kernel Developer's Guide, which is available on
 	 * www.amd.com
 	 */
-	printk(KERN_ERR PFX "BIOS error - no PSB or ACPI _PSS objects\n");
+	printk(KERN_ERR FW_BUG PFX "No PSB or ACPI _PSS objects\n");
 	return -ENODEV;
 }
 
@@ -1154,11 +1154,11 @@ static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
 		 * an UP version, and is deprecated by AMD.
 		 */
 		if (num_online_cpus() != 1) {
-			printk(KERN_ERR FW_BUG PFX "Your BIOS does not provide"
-			       " ACPI _PSS objects in a way that Linux "
-			       "understands. Please report this to the Linux "
-			       "ACPI maintainers and complain to your BIOS "
-			       "vendor.\n");
+			WARN_ONCE(1, KERN_ERR FW_BUG PFX "Your BIOS does not "
+				  "provide ACPI _PSS objects in a way that "
+				  "Linux understands. Please report this to "
+				  "the Linux ACPI maintainers and complain to "
+				  "your BIOS vendor.\n");
 			goto err_out;
 		}
 		if (pol->cpu != 0) {
-- 
1.6.0.2

Re: [PATCH 6/7] CPUFREQ: powernow-k8: Only print error message once, not per core.

[Andrew Morton]

On Tue,  3 Feb 2009 17:46:45 +0100 Thomas Renninger <trenn@suse.de> wrote:

> This is the typical message you get if you plug in a CPU
> which is newer than your BIOS. It's annoying seeing this
> message for each core.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> ---
>  arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   12 ++++++------
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> index 9accffb..9e312c5 100644
> --- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> +++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> @@ -743,7 +743,7 @@ static int find_psb_table(struct powernow_k8_data *data)
>  	 * BIOS and Kernel Developer's Guide, which is available on
>  	 * www.amd.com
>  	 */
> -	printk(KERN_ERR PFX "BIOS error - no PSB or ACPI _PSS objects\n");
> +	printk(KERN_ERR FW_BUG PFX "No PSB or ACPI _PSS objects\n");
>  	return -ENODEV;
>  }
>  
> @@ -1154,11 +1154,11 @@ static int __cpuinit powernowk8_cpu_init(struct cpufreq_policy *pol)
>  		 * an UP version, and is deprecated by AMD.
>  		 */
>  		if (num_online_cpus() != 1) {
> -			printk(KERN_ERR FW_BUG PFX "Your BIOS does not provide"
> -			       " ACPI _PSS objects in a way that Linux "
> -			       "understands. Please report this to the Linux "
> -			       "ACPI maintainers and complain to your BIOS "
> -			       "vendor.\n");
> +			WARN_ONCE(1, KERN_ERR FW_BUG PFX "Your BIOS does not "
> +				  "provide ACPI _PSS objects in a way that "
> +				  "Linux understands. Please report this to "
> +				  "the Linux ACPI maintainers and complain to "
> +				  "your BIOS vendor.\n");
>  			goto err_out;
>  		}
>  		if (pol->cpu != 0) {

WARN_ONCE will also spew a stack backtrace, which seems inappropriate here.

There was talk of writing a simple ONCE() macro for this sort of thing:

	if (ONCE())
		printk(...)

but it never happened.

Re: [PATCH 6/7] CPUFREQ: powernow-k8: Only print error message once, not per core.

[Thomas Renninger]

On Wednesday 04 February 2009 06:09:47 Andrew Morton wrote:
> On Tue,  3 Feb 2009 17:46:45 +0100 Thomas Renninger <trenn@suse.de> wrote:
> > This is the typical message you get if you plug in a CPU
> > which is newer than your BIOS. It's annoying seeing this
> > message for each core.
> >
> > Signed-off-by: Thomas Renninger <trenn@suse.de>
> > ---
> >  arch/x86/kernel/cpu/cpufreq/powernow-k8.c |   12 ++++++------
> >  1 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> > b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c index 9accffb..9e312c5 100644
> > --- a/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> > +++ b/arch/x86/kernel/cpu/cpufreq/powernow-k8.c
> > @@ -743,7 +743,7 @@ static int find_psb_table(struct powernow_k8_data
> > *data) * BIOS and Kernel Developer's Guide, which is available on
> >  	 * www.amd.com
> >  	 */
> > -	printk(KERN_ERR PFX "BIOS error - no PSB or ACPI _PSS objects\n");
> > +	printk(KERN_ERR FW_BUG PFX "No PSB or ACPI _PSS objects\n");
> >  	return -ENODEV;
> >  }
> >
> > @@ -1154,11 +1154,11 @@ static int __cpuinit powernowk8_cpu_init(struct
> > cpufreq_policy *pol) * an UP version, and is deprecated by AMD.
> >  		 */
> >  		if (num_online_cpus() != 1) {
> > -			printk(KERN_ERR FW_BUG PFX "Your BIOS does not provide"
> > -			       " ACPI _PSS objects in a way that Linux "
> > -			       "understands. Please report this to the Linux "
> > -			       "ACPI maintainers and complain to your BIOS "
> > -			       "vendor.\n");
> > +			WARN_ONCE(1, KERN_ERR FW_BUG PFX "Your BIOS does not "
> > +				  "provide ACPI _PSS objects in a way that "
> > +				  "Linux understands. Please report this to "
> > +				  "the Linux ACPI maintainers and complain to "
> > +				  "your BIOS vendor.\n");
> >  			goto err_out;
> >  		}
> >  		if (pol->cpu != 0) {
>
> WARN_ONCE will also spew a stack backtrace, which seems inappropriate here.
Yes. I saw WARN_ONCE(1, KERN_INFO..), which should be a KERN_ERR then
(if you get a backtrace you do not want to have the message suppressed?)
and thus thought it's a simple printk.
>
> There was talk of writing a simple ONCE() macro for this sort of thing:
>
> 	if (ONCE())
> 		printk(...)
Yep, that would be nice here.

> but it never happened.
I added a static int print_once; hack now.

Thanks,

   Thomas

[PATCH 7/7] ACPI: cpufreq: Remove deprecated /proc/acpi/processor/../performance proc entries

[Thomas Renninger]

They were long enough set deprecated...

Update Documentation/cpu-freq/users-guide.txt:
The deprecated files listed there seen not to exist for some time anymore
already.

Signed-off-by: Thomas Renninger <trenn@suse.de>
CC: Len Brown <lenb@kernel.org>
CC: Linux ACPI list <linux-acpi@vger.kernel.org>
---
 Documentation/cpu-freq/user-guide.txt |   16 -----
 arch/x86/kernel/cpu/cpufreq/Kconfig   |   11 ----
 drivers/acpi/processor_perflib.c      |  105 ---------------------------------
 3 files changed, 0 insertions(+), 132 deletions(-)

diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt
index 57f23cb..75f4119 100644
--- a/Documentation/cpu-freq/user-guide.txt
+++ b/Documentation/cpu-freq/user-guide.txt
@@ -207,19 +207,3 @@ scaling_setspeed.		By "echoing" a new frequency into this
 				you can change the speed of the CPU,
 				but only within the limits of
 				scaling_min_freq and scaling_max_freq.
-				
-
-3.2 Deprecated Interfaces
--------------------------
-
-Depending on your kernel configuration, you might find the following 
-cpufreq-related files:
-/proc/cpufreq
-/proc/sys/cpu/*/speed
-/proc/sys/cpu/*/speed-min
-/proc/sys/cpu/*/speed-max
-
-These are files for deprecated interfaces to cpufreq, which offer far
-less functionality. Because of this, these interfaces aren't described
-here.
-
diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
index b63b0db..52c8398 100644
--- a/arch/x86/kernel/cpu/cpufreq/Kconfig
+++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
@@ -230,17 +230,6 @@ config X86_E_POWERSAVER
 
 comment "shared options"
 
-config X86_ACPI_CPUFREQ_PROC_INTF
-	bool "/proc/acpi/processor/../performance interface (deprecated)"
-	depends on PROC_FS
-	depends on X86_ACPI_CPUFREQ || X86_POWERNOW_K7_ACPI || X86_POWERNOW_K8_ACPI
-	help
-	  This enables the deprecated /proc/acpi/processor/../performance
-	  interface. While it is helpful for debugging, the generic,
-	  cross-architecture cpufreq interfaces should be used.
-
-	  If in doubt, say N.
-
 config X86_SPEEDSTEP_LIB
 	tristate
 	default (X86_SPEEDSTEP_ICH || X86_SPEEDSTEP_SMI || X86_P4_CLOCKMOD)
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index 846e227..9cc769b 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -31,14 +31,6 @@
 #include <linux/init.h>
 #include <linux/cpufreq.h>
 
-#ifdef CONFIG_X86_ACPI_CPUFREQ_PROC_INTF
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include <linux/mutex.h>
-
-#include <asm/uaccess.h>
-#endif
-
 #ifdef CONFIG_X86
 #include <asm/cpufeature.h>
 #endif
@@ -434,96 +426,6 @@ int acpi_processor_notify_smm(struct module *calling_module)
 
 EXPORT_SYMBOL(acpi_processor_notify_smm);
 
-#ifdef CONFIG_X86_ACPI_CPUFREQ_PROC_INTF
-/* /proc/acpi/processor/../performance interface (DEPRECATED) */
-
-static int acpi_processor_perf_open_fs(struct inode *inode, struct file *file);
-static struct file_operations acpi_processor_perf_fops = {
-	.owner = THIS_MODULE,
-	.open = acpi_processor_perf_open_fs,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = single_release,
-};
-
-static int acpi_processor_perf_seq_show(struct seq_file *seq, void *offset)
-{
-	struct acpi_processor *pr = seq->private;
-	int i;
-
-
-	if (!pr)
-		goto end;
-
-	if (!pr->performance) {
-		seq_puts(seq, "<not supported>\n");
-		goto end;
-	}
-
-	seq_printf(seq, "state count:             %d\n"
-		   "active state:            P%d\n",
-		   pr->performance->state_count, pr->performance->state);
-
-	seq_puts(seq, "states:\n");
-	for (i = 0; i < pr->performance->state_count; i++)
-		seq_printf(seq,
-			   "   %cP%d:                  %d MHz, %d mW, %d uS\n",
-			   (i == pr->performance->state ? '*' : ' '), i,
-			   (u32) pr->performance->states[i].core_frequency,
-			   (u32) pr->performance->states[i].power,
-			   (u32) pr->performance->states[i].transition_latency);
-
-      end:
-	return 0;
-}
-
-static int acpi_processor_perf_open_fs(struct inode *inode, struct file *file)
-{
-	return single_open(file, acpi_processor_perf_seq_show,
-			   PDE(inode)->data);
-}
-
-static void acpi_cpufreq_add_file(struct acpi_processor *pr)
-{
-	struct acpi_device *device = NULL;
-
-
-	if (acpi_bus_get_device(pr->handle, &device))
-		return;
-
-	/* add file 'performance' [R/W] */
-	proc_create_data(ACPI_PROCESSOR_FILE_PERFORMANCE, S_IFREG | S_IRUGO,
-			 acpi_device_dir(device),
-			 &acpi_processor_perf_fops, acpi_driver_data(device));
-	return;
-}
-
-static void acpi_cpufreq_remove_file(struct acpi_processor *pr)
-{
-	struct acpi_device *device = NULL;
-
-
-	if (acpi_bus_get_device(pr->handle, &device))
-		return;
-
-	/* remove file 'performance' */
-	remove_proc_entry(ACPI_PROCESSOR_FILE_PERFORMANCE,
-			  acpi_device_dir(device));
-
-	return;
-}
-
-#else
-static void acpi_cpufreq_add_file(struct acpi_processor *pr)
-{
-	return;
-}
-static void acpi_cpufreq_remove_file(struct acpi_processor *pr)
-{
-	return;
-}
-#endif				/* CONFIG_X86_ACPI_CPUFREQ_PROC_INTF */
-
 static int acpi_processor_get_psd(struct acpi_processor	*pr)
 {
 	int result = 0;
@@ -747,14 +649,12 @@ err_ret:
 }
 EXPORT_SYMBOL(acpi_processor_preregister_performance);
 
-
 int
 acpi_processor_register_performance(struct acpi_processor_performance
 				    *performance, unsigned int cpu)
 {
 	struct acpi_processor *pr;
 
-
 	if (!(acpi_processor_ppc_status & PPC_REGISTERED))
 		return -EINVAL;
 
@@ -781,8 +681,6 @@ acpi_processor_register_performance(struct acpi_processor_performance
 		return -EIO;
 	}
 
-	acpi_cpufreq_add_file(pr);
-
 	mutex_unlock(&performance_mutex);
 	return 0;
 }
@@ -795,7 +693,6 @@ acpi_processor_unregister_performance(struct acpi_processor_performance
 {
 	struct acpi_processor *pr;
 
-
 	mutex_lock(&performance_mutex);
 
 	pr = per_cpu(processors, cpu);
@@ -808,8 +705,6 @@ acpi_processor_unregister_performance(struct acpi_processor_performance
 		kfree(pr->performance->states);
 	pr->performance = NULL;
 
-	acpi_cpufreq_remove_file(pr);
-
 	mutex_unlock(&performance_mutex);
 
 	return;
-- 
1.6.0.2

Re: [PATCH 7/7] ACPI: cpufreq: Remove deprecated /proc/acpi/processor/../performance proc entries

[Len Brown]

applied

thanks,
--
Len Brown, Intel Open Source Technology Center

On Tue, 3 Feb 2009, Thomas Renninger wrote:

> They were long enough set deprecated...
> 
> Update Documentation/cpu-freq/users-guide.txt:
> The deprecated files listed there seen not to exist for some time anymore
> already.
> 
> Signed-off-by: Thomas Renninger <trenn@suse.de>
> CC: Len Brown <lenb@kernel.org>
> CC: Linux ACPI list <linux-acpi@vger.kernel.org>
> ---
>  Documentation/cpu-freq/user-guide.txt |   16 -----
>  arch/x86/kernel/cpu/cpufreq/Kconfig   |   11 ----
>  drivers/acpi/processor_perflib.c      |  105 ---------------------------------
>  3 files changed, 0 insertions(+), 132 deletions(-)
> 
> diff --git a/Documentation/cpu-freq/user-guide.txt b/Documentation/cpu-freq/user-guide.txt
> index 57f23cb..75f4119 100644
> --- a/Documentation/cpu-freq/user-guide.txt
> +++ b/Documentation/cpu-freq/user-guide.txt
> @@ -207,19 +207,3 @@ scaling_setspeed.		By "echoing" a new frequency into this
>  				you can change the speed of the CPU,
>  				but only within the limits of
>  				scaling_min_freq and scaling_max_freq.
> -				
> -
> -3.2 Deprecated Interfaces
> --------------------------
> -
> -Depending on your kernel configuration, you might find the following 
> -cpufreq-related files:
> -/proc/cpufreq
> -/proc/sys/cpu/*/speed
> -/proc/sys/cpu/*/speed-min
> -/proc/sys/cpu/*/speed-max
> -
> -These are files for deprecated interfaces to cpufreq, which offer far
> -less functionality. Because of this, these interfaces aren't described
> -here.
> -
> diff --git a/arch/x86/kernel/cpu/cpufreq/Kconfig b/arch/x86/kernel/cpu/cpufreq/Kconfig
> index b63b0db..52c8398 100644
> --- a/arch/x86/kernel/cpu/cpufreq/Kconfig
> +++ b/arch/x86/kernel/cpu/cpufreq/Kconfig
> @@ -230,17 +230,6 @@ config X86_E_POWERSAVER
>  
>  comment "shared options"
>  
> -config X86_ACPI_CPUFREQ_PROC_INTF
> -	bool "/proc/acpi/processor/../performance interface (deprecated)"
> -	depends on PROC_FS
> -	depends on X86_ACPI_CPUFREQ || X86_POWERNOW_K7_ACPI || X86_POWERNOW_K8_ACPI
> -	help
> -	  This enables the deprecated /proc/acpi/processor/../performance
> -	  interface. While it is helpful for debugging, the generic,
> -	  cross-architecture cpufreq interfaces should be used.
> -
> -	  If in doubt, say N.
> -
>  config X86_SPEEDSTEP_LIB
>  	tristate
>  	default (X86_SPEEDSTEP_ICH || X86_SPEEDSTEP_SMI || X86_P4_CLOCKMOD)
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index 846e227..9cc769b 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -31,14 +31,6 @@
>  #include <linux/init.h>
>  #include <linux/cpufreq.h>
>  
> -#ifdef CONFIG_X86_ACPI_CPUFREQ_PROC_INTF
> -#include <linux/proc_fs.h>
> -#include <linux/seq_file.h>
> -#include <linux/mutex.h>
> -
> -#include <asm/uaccess.h>
> -#endif
> -
>  #ifdef CONFIG_X86
>  #include <asm/cpufeature.h>
>  #endif
> @@ -434,96 +426,6 @@ int acpi_processor_notify_smm(struct module *calling_module)
>  
>  EXPORT_SYMBOL(acpi_processor_notify_smm);
>  
> -#ifdef CONFIG_X86_ACPI_CPUFREQ_PROC_INTF
> -/* /proc/acpi/processor/../performance interface (DEPRECATED) */
> -
> -static int acpi_processor_perf_open_fs(struct inode *inode, struct file *file);
> -static struct file_operations acpi_processor_perf_fops = {
> -	.owner = THIS_MODULE,
> -	.open = acpi_processor_perf_open_fs,
> -	.read = seq_read,
> -	.llseek = seq_lseek,
> -	.release = single_release,
> -};
> -
> -static int acpi_processor_perf_seq_show(struct seq_file *seq, void *offset)
> -{
> -	struct acpi_processor *pr = seq->private;
> -	int i;
> -
> -
> -	if (!pr)
> -		goto end;
> -
> -	if (!pr->performance) {
> -		seq_puts(seq, "<not supported>\n");
> -		goto end;
> -	}
> -
> -	seq_printf(seq, "state count:             %d\n"
> -		   "active state:            P%d\n",
> -		   pr->performance->state_count, pr->performance->state);
> -
> -	seq_puts(seq, "states:\n");
> -	for (i = 0; i < pr->performance->state_count; i++)
> -		seq_printf(seq,
> -			   "   %cP%d:                  %d MHz, %d mW, %d uS\n",
> -			   (i == pr->performance->state ? '*' : ' '), i,
> -			   (u32) pr->performance->states[i].core_frequency,
> -			   (u32) pr->performance->states[i].power,
> -			   (u32) pr->performance->states[i].transition_latency);
> -
> -      end:
> -	return 0;
> -}
> -
> -static int acpi_processor_perf_open_fs(struct inode *inode, struct file *file)
> -{
> -	return single_open(file, acpi_processor_perf_seq_show,
> -			   PDE(inode)->data);
> -}
> -
> -static void acpi_cpufreq_add_file(struct acpi_processor *pr)
> -{
> -	struct acpi_device *device = NULL;
> -
> -
> -	if (acpi_bus_get_device(pr->handle, &device))
> -		return;
> -
> -	/* add file 'performance' [R/W] */
> -	proc_create_data(ACPI_PROCESSOR_FILE_PERFORMANCE, S_IFREG | S_IRUGO,
> -			 acpi_device_dir(device),
> -			 &acpi_processor_perf_fops, acpi_driver_data(device));
> -	return;
> -}
> -
> -static void acpi_cpufreq_remove_file(struct acpi_processor *pr)
> -{
> -	struct acpi_device *device = NULL;
> -
> -
> -	if (acpi_bus_get_device(pr->handle, &device))
> -		return;
> -
> -	/* remove file 'performance' */
> -	remove_proc_entry(ACPI_PROCESSOR_FILE_PERFORMANCE,
> -			  acpi_device_dir(device));
> -
> -	return;
> -}
> -
> -#else
> -static void acpi_cpufreq_add_file(struct acpi_processor *pr)
> -{
> -	return;
> -}
> -static void acpi_cpufreq_remove_file(struct acpi_processor *pr)
> -{
> -	return;
> -}
> -#endif				/* CONFIG_X86_ACPI_CPUFREQ_PROC_INTF */
> -
>  static int acpi_processor_get_psd(struct acpi_processor	*pr)
>  {
>  	int result = 0;
> @@ -747,14 +649,12 @@ err_ret:
>  }
>  EXPORT_SYMBOL(acpi_processor_preregister_performance);
>  
> -
>  int
>  acpi_processor_register_performance(struct acpi_processor_performance
>  				    *performance, unsigned int cpu)
>  {
>  	struct acpi_processor *pr;
>  
> -
>  	if (!(acpi_processor_ppc_status & PPC_REGISTERED))
>  		return -EINVAL;
>  
> @@ -781,8 +681,6 @@ acpi_processor_register_performance(struct acpi_processor_performance
>  		return -EIO;
>  	}
>  
> -	acpi_cpufreq_add_file(pr);
> -
>  	mutex_unlock(&performance_mutex);
>  	return 0;
>  }
> @@ -795,7 +693,6 @@ acpi_processor_unregister_performance(struct acpi_processor_performance
>  {
>  	struct acpi_processor *pr;
>  
> -
>  	mutex_lock(&performance_mutex);
>  
>  	pr = per_cpu(processors, cpu);
> @@ -808,8 +705,6 @@ acpi_processor_unregister_performance(struct acpi_processor_performance
>  		kfree(pr->performance->states);
>  	pr->performance = NULL;
>  
> -	acpi_cpufreq_remove_file(pr);
> -
>  	mutex_unlock(&performance_mutex);
>  
>  	return;
> -- 
> 1.6.0.2
> 

Re: CPUFREQ: Ondemand and powernow-k8 enhancements/fixes/cleanups

[Dave Jones]

On Tue, Feb 03, 2009 at 05:46:39PM +0100, Thomas Renninger wrote:
 > These are the same I sent out some days ago, but messed up the
 > cpufreq list address.
 > 
 > The patches are based against Dave's cpufreq git tree's fixes branch.
 > There is also the one from Mark Langsdorf (adjusted to latest tree)
 > included which got posted on the cpufreq list recently.
 > guilt should add the
 > From: Mark Langsdorf ...
 > tag right, so that he shows up as the author of this one.
 > 
 > Dave, can you add these to your tree, please.

Thanks Thomas,
 I'll get to these today and queue them up.

	Dave

-- 
http://www.codemonkey.org.uk

Re: CPUFREQ: Ondemand and powernow-k8 enhancements/fixes/cleanups

[Dave Jones]

On Tue, Feb 03, 2009 at 05:46:39PM +0100, Thomas Renninger wrote:
 > These are the same I sent out some days ago, but messed up the
 > cpufreq list address.
 > 
 > The patches are based against Dave's cpufreq git tree's fixes branch.

'fixes' is for stuff pending for .29, which this patchset seems out
of scope for.  Unfortunatly, there's a ton of checkpatch cleanups up
the 'next' branch which makes this impossible to apply.

Can you rediff against that branch please?
 
thanks,

	Dave

-- 
http://www.codemonkey.org.uk

Re: CPUFREQ: Ondemand and powernow-k8 enhancements/fixes/cleanups

[Thomas Renninger]

On Tuesday 03 February 2009 10:21:06 pm Dave Jones wrote:
> On Tue, Feb 03, 2009 at 05:46:39PM +0100, Thomas Renninger wrote:
>  > These are the same I sent out some days ago, but messed up the
>  > cpufreq list address.
>  >
>  > The patches are based against Dave's cpufreq git tree's fixes branch.
>
> 'fixes' is for stuff pending for .29, which this patchset seems out
> of scope for.  Unfortunatly, there's a ton of checkpatch cleanups up
> the 'next' branch which makes this impossible to apply.
Mark's latency fix for powernow-k8 should go to .29.
It's a sever fix which makes powernow-k8 work fine with ondemand
without tweaking sampling rate. Otherwise you could hit sever
performance loss, e.g. there are machines out there which currently
only check for frequency changes every 1.2 seconds (see comment #24):
https://bugzilla.novell.com/show_bug.cgi?id=436717

According to Mark, Windows PowerNow! driver also uses these ACPI
values and it got tested on various different K8 CPUs.
It should be the first patch touching powernow-k8, so this
one can be picked out of the rest.

> Can you rediff against that branch please?
The rest is not critical, I will rediff everything. I expect you still
want to have the whole series rebased, even if you decide to add
above one to .29?

Thanks,

     Thomas

Re: CPUFREQ: Ondemand and powernow-k8 enhancements/fixes/cleanups

[Dave Jones]

On Wed, Feb 04, 2009 at 12:29:51AM +0100, Thomas Renninger wrote:
 
 > Mark's latency fix for powernow-k8 should go to .29.

Ok, I'll get that moved along to Linus

 > > Can you rediff against that branch please?
 > The rest is not critical, I will rediff everything. I expect you still
 > want to have the whole series rebased, even if you decide to add
 > above one to .29?

Yes please.  Thanks.

	Dave

-- 
http://www.codemonkey.org.uk