[PATCHES] ACPI Processor update [idle,throttling,thermal,cpufreq]

[PATCHES] ACPI Processor update [idle,throttling,thermal,cpufreq]

[Dominik Brodowski]

Over the last few hours, I've rewritten large portions of ACPI processor
handling. Due to lack of appropriate hardware, I can hardly test these
changes, so expect a few rough edges.

These patches 
- a) replace the broken ACPI CPUfreq driver with a better, flexible variant,
- b) modularize the processor.c code: instead of one large file there are
     many small files.
- c) improve passive cooling support
- d) allow for easy adding of proper locking and ref-counting in processor.c

http://www.brodo.de/cpufreq_tmp/cpufreq-2.6.0-test4-ALL-series1
	Large CPUfreq update -- needed for other changes
	[in CPUfreq BK tree, merge with Linus expected soon]

http://www.brodo.de/cpufreq_tmp/cpufreq-2.6.0-test4-update_policy
http://www.brodo.de/cpufreq_tmp/cpufreq-2.6.0-test4-Kconfig-speedstep
	Additional CPUfreq updates -- needed for other changes
	[Already sent to CPUfreq list]

http://www.brodo.de/cpufreq_tmp/drivermodel-2.6.0-test4-exports
	Missing EXPORTs in sysdevice -- needed for other changes
	[Already sent to Patrick]


http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-remove_previous_pstates_implementation
	The previous arch/i386/kernel/cpu/cpufreq/acpi.c CPUfreq
	driver was broken, of bad design, and needs to replaced 
	by something better. But, as a first step, remove all 
	parts related to P-States from ACPI code.

http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_submodules
	Add a "submodule interface" to drivers/acpi/processor.c

	It allows to create other "modules" which access the acpi_handle 
	for the processor, and which get notified if the event value 
	maches the value passed in acpi_processor_register_notify.

http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_perflib
	This patch adds a new "P-States library" to drivers/acpi/

	CPUfreq drivers can now easly access the contents of the 
	_PCT and the _PSS. For example, the speedstep-centrino 
	driver could be appended so that it passes the appropriate 
	value to the P-States library which then evaluates _PDC, 
	and then returns the updated _PCT and _PSS.

	Also, the platform limit is now handled as a cpufreq notifier and
	as a call to cpufreq_policy_update.

http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-new_acpi_io_driver
	This re-adds the acpi P-States I/O driver. It is much smaller,
	leaner and cleaner.

http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_idle_submodule
	This moves the idle handler out of drivers/acpi/processor and into
	an own module.

	Even if only C1 is available, it is now used. If the user prefers the 
	default pm_idle, he can unload processor_idle, and still have the other
	functionality available.

http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_submodule
	This adds a new mechanism to manage passive cooling. Instead of
	hardcoded -and partly wrong- access to CPUfreq and ACPI 
	throttling-, add a generic mechanism which other modules can 
	register with.

http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_cpufreq
	Use _any_ CPUfreq driver for passive cooling. Implemented by an
	cpufreq policy notifier and cpufreq_update_policy.
 
http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_throttling
	Move throttling into its own submodule, and register it with the new
	passive cooling module. Also, the now-useless "limit" interface is
	removed.


Please review, test (carefully), and -if appropriate- merge,

	Dominik

Re: [ACPI] [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]

[Dmitry Torokhov]

Dominik,

I have couple of concerns regarding P-states IO library, esp. 
acpi_processor_get_frequency. It seems that ACPI does not allow
to read current state without setting it first, at least with
the following code on my notebook I was always getting garbage 
in the status register:

+static int
+acpi_processor_get_current_state(
+       struct acpi_processor_performance       *perf)
+{
+       int     result = 0;
+       int     i;
+       u8      value;
+
+       ACPI_FUNCTION_TRACE("acpi_processor_get_current_state");
+
+       if (!perf->pr->flags.performance)
+               goto out;
+
+       value = inb(perf->status_register);
+
+       ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+               "Got 0x%02x from port 0x%04x\n", value, perf->status_register));
+
+       for (i = 0; i < perf->state_count; i++) {
+               if (value == (u8) perf->states[i].status) {
+                       perf->state = i;
+                       goto out;
+               }
+       }
+
+       /* couldn't match our state table - garbage in status register */
+       ACPI_DEBUG_PRINT((ACPI_DB_ERROR,
+               "Bad data 0x%02x in performance status register 0x%04x\n",
+               value, perf->status_register));
+       result = -EFAULT;
+out:
+       return_VALUE(result);
+}

I am wondering is it just me, my code or general deficiency.

Also, do you really need to do notify_transition twice 
(acpi_processor_set_performance)?

Dmitry

Re: [ACPI] [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]

[Dominik Brodowski]

Dmitry,

Thanks for your input.

On Fri, Sep 05, 2003 at 12:23:06AM -0500, Dmitry Torokhov wrote:
> I have couple of concerns regarding P-states IO library, esp. 
> acpi_processor_get_frequency. It seems that ACPI does not allow
> to read current state without setting it first.

Indeed. The specification allows this behaviour. That's really annoying, but
well...

> Also, do you really need to do notify_transition twice 
> (acpi_processor_set_performance)?

No, it's a bug.

The attached patch should fix both issues.

diff -ruN linux-original/arch/i386/kernel/cpu/cpufreq/acpi-io-cpufreq.c linux/arch/i386/kernel/cpu/cpufreq/acpi-io-cpufreq.c
--- linux-original/arch/i386/kernel/cpu/cpufreq/acpi-io-cpufreq.c	2003-09-04 21:36:58.000000000 +0200
+++ linux/arch/i386/kernel/cpu/cpufreq/acpi-io-cpufreq.c	2003-09-05 08:47:54.302664624 +0200
@@ -33,6 +33,7 @@
 #include <asm/io.h>
 #include <asm/delay.h>
 #include <asm/uaccess.h>
+#include <asm/timex.h>
 
 #include <linux/acpi.h>
 #include <acpi/processor.h>
@@ -54,32 +55,16 @@
 	u16 				control_register;
 	u16 				status_register;
 
+	unsigned long			previous_freq;
 	struct cpufreq_frequency_table	*table;
 };
 
 static struct cpufreq_acpi_io_data acpi_io_data[NR_CPUS];
 
-static unsigned long acpi_processor_get_frequency (
-	struct cpufreq_acpi_io_data 	*data)
-{
-	unsigned int		i;
-	u16			port;
-	u8			value;
-
-	port = data->status_register;
-	value = inb(port);
-
-	for (i=0; i<data->data->state_count; i++) {
-		if (value == (u8) data->data->states[i].status)
-			return (1000 * data->data->states[i].core_frequency);
-	}
-
-	return 0;
-}
-
 static int acpi_processor_set_performance (
 	struct cpufreq_acpi_io_data 	*data,
-	int				state)
+	int				state,
+	unsigned int			notify)
 {
 	u16			port;
 	u8			value;
@@ -88,14 +73,12 @@
 
 	/* cpufreq frequency struct */
 	cpufreq_freqs.cpu = data->cpu;
-	cpufreq_freqs.old = acpi_processor_get_frequency(data);
+	cpufreq_freqs.old = data->previous_freq;
 	cpufreq_freqs.new = data->data->states[state].core_frequency;
 
 	/* notify cpufreq */
-	cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_PRECHANGE);
-
-	/* notify cpufreq */
-	cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_PRECHANGE);
+	if (notify)
+		cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_PRECHANGE);
 
 	/*
 	 * First we write the target state's 'control' value to the
@@ -129,18 +112,21 @@
 	}
 
 	/* notify cpufreq */
+	if (notify)
 	cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_POSTCHANGE);
 
 	if (value != data->data->states[state].status) {
 		unsigned int tmp = cpufreq_freqs.new;
 		cpufreq_freqs.new = cpufreq_freqs.old;
 		cpufreq_freqs.old = tmp;
-		cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_PRECHANGE);
-		cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_POSTCHANGE);
+		if (notify) {
+			cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_PRECHANGE);
+			cpufreq_notify_transition(&cpufreq_freqs, CPUFREQ_POSTCHANGE);
+		}
 		printk(KERN_ERR "Transition failed\n");
 		return -EINVAL;
 	}
-
+	data->previous_freq = data->data->states[state].core_frequency * 1000;
 	dprintk(KERN_INFO "Transition successful after %d microseconds\n", i * 10);
 	return 0;
 }
@@ -162,7 +148,7 @@
 	if (result)
 		return (result);
 
-	return acpi_processor_set_performance (data, next_state);
+	return acpi_processor_set_performance (data, next_state, 1);
 }
 
 static int acpi_cpufreq_verify (struct cpufreq_policy   *policy)
@@ -183,6 +169,7 @@
 	struct acpi_perflib_data *data;
 	struct cpufreq_frequency_table *table;
 	unsigned int i;
+	unsigned long tmp_speed;
 
 	data = acpi_processor_perflib_register (&acpi_cpufreq_perflib_driver,
 						 policy->cpu);
@@ -221,7 +208,29 @@
 	acpi_io_data[policy->cpu].status_register = (u16) data->pct_status.address;
 	acpi_io_data[policy->cpu].control_register = (u16) data->pct_control.address;
 
-	return 0;
+	/* 
+	 * The ACPI specificiation is broken in the regard that
+	 * it is impossible to detect the _current_ P-State.
+	 * The control_register is only valid _after_ a frequency
+	 * transition.
+	 * So, we try to get the current setting from cpu_khz,
+	 * but to make sure, this state we think the CPU is in
+	 * is also set.
+	 */
+	if (cpu_khz) {
+		tmp_speed = cpu_khz / 100;
+		tmp_speed *= 105;
+	} else {
+		tmp_speed = policy->cpuinfo.max_freq;
+	}
+	result = cpufreq_frequency_table_target(policy, data->table,
+						tmp_speed,
+						CPUFREQ_RELATION_L,
+						&i);
+	if (result)
+		return (result);
+
+	return acpi_processor_set_performance (data, i, 0);
 }

Re: [ACPI] [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]

[Pavel Machek]

Hi!

>http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_cpufreq
> 	Use _any_ CPUfreq driver for passive cooling. Implemented by an
> 	cpufreq policy notifier and cpufreq_update_policy.
>  

Good - it is going to help a lot on k8 notebooks.

http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_throttling
> 	Move throttling into its own submodule, and register it with the new
> 	passive cooling module. Also, the now-useless "limit" interface is
> 	removed.
> 

Actually I liked to be able to echo 0:7 > limit and conserve battery/
make sure machine does not overheat  with it...


-- 
				Pavel
Written on sharp zaurus, because my Velo1 broke. If you have Velo you don't need...

Re: [ACPI] [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]

[linux-JhLEnvuH02M]

Hi Pavel,

Glad you liked [at least parts] of my patches.

On Mon, Sep 08, 2003 at 03:29:40PM +0200, Pavel Machek wrote:
> http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_throttling
> > 	Move throttling into its own submodule, and register it with the new
> > 	passive cooling module. Also, the now-useless "limit" interface is
> > 	removed.
> > 
> 
> Actually I liked to be able to echo 0:7 > limit and conserve battery/
> make sure machine does not overheat  with it...

echo 7 > throttling should do the same thing. However, don't expect any
battey conservation if ACPI C2 throttling works, and to set custom levels
against "overheating" there's a write access to
/proc/acpi/thermal/*/trip_points too...

	Dominik

Re: [ACPI] [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]

[Pavel Machek]

Hi!

> Glad you liked [at least parts] of my patches.

:-). [BTW what is status of powernow-k8? is it pending or does it need
to be fixed some more?]

> On Mon, Sep 08, 2003 at 03:29:40PM +0200, Pavel Machek wrote:
> > http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_throttling
> > > 	Move throttling into its own submodule, and register it with the new
> > > 	passive cooling module. Also, the now-useless "limit" interface is
> > > 	removed.
> > > 
> > 
> > Actually I liked to be able to echo 0:7 > limit and conserve battery/
> > make sure machine does not overheat  with it...
> 
> echo 7 > throttling should do the same thing. 

Will not first change in thermal system unthrottle it for me, behind
my back?

> However, don't expect any
> battey conservation if ACPI C2 throttling works, and to set custom levels
> against "overheating" there's a write access to
> /proc/acpi/thermal/*/trip_points too...

I'd should not expect battery conservation *when cpu is idle*. If I'm
moving windows in X, 0:7 limit will actually help. And it should allow
battery to be drained further by limiting maximum amps eaten by
CPU. [Ok, that's little evil, but being able to run for 30 minutes+ on
0% battery is nice ;-)].
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [ACPI] [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]

[linux-JhLEnvuH02M]

Hi,

[powernow-k8 see separate e-mail]

On Wed, Sep 10, 2003 at 01:13:03AM +0200, Pavel Machek wrote:
> > On Mon, Sep 08, 2003 at 03:29:40PM +0200, Pavel Machek wrote:
> > > http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_throttling
> > > > 	Move throttling into its own submodule, and register it with the new
> > > > 	passive cooling module. Also, the now-useless "limit" interface is
> > > > 	removed.
> > > > 
> > > 
> > > Actually I liked to be able to echo 0:7 > limit and conserve battery/
> > > make sure machine does not overheat  with it...
> > 
> > echo 7 > throttling should do the same thing. 
> 
> Will not first change in thermal system unthrottle it for me, behind
> my back?

Well yes, it might, if the temperature had been higher than the passive
cooling trip point. If you want me to, I'll change the behaviour so that the
/proc/acpi/.../throttling input is consistent (minimum limit) until the user 
choses a different throttling level.

> > However, don't expect any
> > battey conservation if ACPI C2 throttling works, and to set custom levels
> > against "overheating" there's a write access to
> > /proc/acpi/thermal/*/trip_points too...
> 
> I'd should not expect battery conservation *when cpu is idle*. If I'm
> moving windows in X, 0:7 limit will actually help.

depends if you see it relative to the time or relative to the energy...

> And it should allow
> battery to be drained further by limiting maximum amps eaten by
> CPU. [Ok, that's little evil, but being able to run for 30 minutes+ on
> 0% battery is nice ;-)].

Well yes, for this case, setting throttling to 0:7 might indeed make
sense...

	Dominik

Re: [ACPI] [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]

[Pavel Machek]

Hi!

> > > > http://www.brodo.de/cpufreq_tmp/acpi-2.6.0-test4-processor_thermal_throttling
> > > > > 	Move throttling into its own submodule, and register it with the new
> > > > > 	passive cooling module. Also, the now-useless "limit" interface is
> > > > > 	removed.
> > > > > 
> > > > 
> > > > Actually I liked to be able to echo 0:7 > limit and conserve battery/
> > > > make sure machine does not overheat  with it...
> > > 
> > > echo 7 > throttling should do the same thing. 
> > 
> > Will not first change in thermal system unthrottle it for me, behind
> > my back?
> 
> Well yes, it might, if the temperature had been higher than the passive
> cooling trip point. If you want me to, I'll change the behaviour so that the
> /proc/acpi/.../throttling input is consistent (minimum limit) until the user 
> choses a different throttling level.

Yes I guess that would make sense.

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

[PATCH] acpi 2.6: consistent user-forced throttling (9/8) [Was: Re: [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]]

[linux-JhLEnvuH02M]

As Pavel Machek has noted, it may be desireable for an user to force a
ceratin throttling level which is kept even if a thermal "decrease cooling"
commands are issued. So, save the user input, and make sure throttling is at
least set to this user_state

diff -ruN linux-original/drivers/acpi/processor_throttling.c linux/drivers/acpi/processor_throttling.c
--- linux-original/drivers/acpi/processor_throttling.c	2003-09-09 23:07:10.000000000 +0200
+++ linux/drivers/acpi/processor_throttling.c	2003-09-10 02:13:05.491536944 +0200
@@ -77,6 +77,7 @@
 struct acpi_processor_throttling {
 	acpi_handle		handle;
 	int			state;
+	int			user_state;
 	u32			address;
 	u8			duty_offset;
 	u8			duty_width;
@@ -211,8 +212,9 @@
 
 	seq_puts(seq, "states:\n");
 	for (i = 0; i < throttle_data[cpu].state_count; i++)
-		seq_printf(seq, "   %cT%d:                  %02d%%\n",
+		seq_printf(seq, " %c%cT%d:                  %02d%%\n",
 			(i == throttle_data[cpu].state?'*':' '), i,
+			(i >= throttle_data[cpu].user_state?'u':' '), i,
 			(throttle_data[cpu].states[i].performance?throttle_data[cpu].states[i].performance/10:0));
 
 end:
@@ -252,6 +254,8 @@
 	if (result)
 		return_VALUE(result);
 
+	throttle_data[cpu].user_state = simple_strtoul(state_string, NULL, 0);
+
 	return_VALUE(count);
 }
 
@@ -536,6 +540,9 @@
 	if (throttle_data[cpu].state == 0)
 		return -ERANGE;
 
+	if (throttle_data[cpu].state == throttle_data[cpu].user_state)
+		return -ERANGE;
+
 	ret = acpi_processor_set_throttling(cpu, throttle_data[cpu].state - 1);
 
 	return ret;




-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: [PATCH] acpi 2.6: consistent user-forced throttling (9/8) [Was: Re: [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]]

[Nils Faerber]

Am Mi, 2003-09-10 um 02.17 schrieb linux-JhLEnvuH02M@public.gmane.org:
> As Pavel Machek has noted, it may be desireable for an user to force a
> ceratin throttling level which is kept even if a thermal "decrease cooling"
> commands are issued. So, save the user input, and make sure throttling is at
> least set to this user_state
[...]

This makes sense! Thanks!
Additionally also the performance setting should be kept like this. I
always have the problem when having used passive cooling that on its way
down from max-temp the CPU is forced into P0 whereas P1 would conserve
power and cause less heat.
Manually resetting to P1 just causes it to be reset to P0 after some
seconds. Tools like autospeedstepd (very nice BTW!) are afterwards also
unable to switch the CPU to P1 again. If this would work it, passive
cooling would make more sense ;)

CU
  nils faerber

-- 
kernel concepts          Tel: +49-271-771091-12
Dreisbachstr. 24         Fax: +49-271-771091-19
D-57250 Netphen          D1 : +49-170-2729106
--



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: [PATCH] acpi 2.6: consistent user-forced throttling (9/8) [Was: Re: [PATCHES] ACPI Processor update [idle, throttling, thermal, cpufreq]]

[linux-JhLEnvuH02M]

On Wed, Sep 10, 2003 at 12:54:24PM +0200, Nils Faerber wrote:
> Am Mi, 2003-09-10 um 02.17 schrieb linux-JhLEnvuH02M@public.gmane.org:
> > As Pavel Machek has noted, it may be desireable for an user to force a
> > ceratin throttling level which is kept even if a thermal "decrease cooling"
> > commands are issued. So, save the user input, and make sure throttling is at
> > least set to this user_state
> [...]
> 
> This makes sense! Thanks!
> Additionally also the performance setting should be kept like this. I
> always have the problem when having used passive cooling that on its way
> down from max-temp the CPU is forced into P0 whereas P1 would conserve
> power and cause less heat.
> Manually resetting to P1 just causes it to be reset to P0 after some
> seconds.

This is one of the "features" in my patchset :-)

	Dominik


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf