Re: Linux ACPI processor driver patch: user-definable power state limit

Re: Linux ACPI processor driver patch: user-definable power state limit

[Len Brown]

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

Jos,
Can you modify this patch to use a kernel boot parameter
rather than a module load parameter?  Some folks don't
load this driver as a module.  Also, it gives you the
chance to add a couple of lines to Documentation/kernel-parameters.txt

thanks,
-Len

On Sat, 2004-08-07 at 13:59, Jos Delbar wrote:
> On Monday 02 August 2004 02:54, you wrote:
> > I think we need a patch like this for a number of reasons.
> > Some systems experience data corruption with C3,
> > so until it is fixed we need an easy way to disable C3.
> >
> > boot parameter names must begin with acpi though.
> > maybe acpi_cstate_limit=3 or something?
> >
> > Please send me the exact model numbers of the Dells
> > that you know have this problem.  I met a guy from
> > Dell who I may be able to get to help us with
> > those models.
> >
> > thanks,
> > -Len
> 
> I browsed through the Dell community forums and I believe it is safe
> to 
> conclude that at least the following notebook models may have this
> problem:
> 
> - Inspiron 300m, 500m, 600m
> - Inspiron 4000, 4100, 4150
> - Inspiron 8100, 8200, 8500, 8600
> - Latitude C840
> - Latitude D500, D600, D800
> 
> As expected, all of these systems use a mobile processor chipset, with
> either 
> an Intel Celeron-M, Pentium III-M, Pentium 4-M or Pentium M processor.
> All of 
> the people who have posted about this buzzing noise are using an
> ACPI-enabled 
> operating system, the majority Windows 2000/XP. Windows 98 does not
> have this 
> problem, for example. On systems that support it, disabling the power 
> management of the USB Root Hub(s) apparently softens--but does not 
> eliminate--the buzzing.
> 
> I updated the patch with the new module parameter name
> "acpi_cstate_limit" and 
> cleaned up the results of /proc/acpi/processor/.../power a little. The
> output 
> using acpi_cstate_limit=2 will now resemble:
> 
> active state: C2
> default state: C1
> user limit: C2
> bus master activity: ffffffff
> states:
> C1: promotion[C2] demotion[--] latency[000] usage[00300510]
> *C2: promotion[--] demotion[C1] latency[050] usage[04964774]
> C3: <disabled>
> 
> If acpi_cstate_limit is passed as a kernel boot parameter, the kernel
> ring 
> buffer will reflect this. For example, using
> "processor.acpi_cstate_limit=1", 
> the relevant output for my system is:
> 
> ACPI: Processor [CPU0] (supports C0 C1, 8 throttling states)
> 
> I chose to always display C0 so this message will still make sense
> when using 
> acpi_cstate_limit=0.
> 
> One of the changes that I made, is unrelated to the C state limit.
> 
> @@ -2417,7 +2446,7 @@
>         pr = (struct acpi_processor *) acpi_driver_data(device);
> 
>         /* Unregister the idle handler when processor #0 is removed.
> */
> -       if (pr->id == 0)
> +       if ((pr->id == 0) && (pr->flags.power))
>                 pm_idle = pm_idle_save;
> 
>         status = acpi_remove_notify_handler(pr->handle,
> ACPI_DEVICE_NOTIFY,
> 
> Without the check for pr->flags.power, it's possible that the old idle
> handler 
> would be "restored" without ever being stored in the function 
> acpi_processor_add(). In practice this shouldn't cause a problem since
> the 
> pm_idle_save global variable is implicitly initialized to NULL, but it
> feels 
> safer to do the check anyway.
> 
> Regards,
> 
> - Jos Delbar
>   jos.delbar-Cru1EgDzd7c@public.gmane.org
> 

[-- Attachment #2: processor.c.diff --]
[-- Type: text/plain, Size: 4384 bytes --]

--- /usr/src/linux/drivers/acpi/processor.c	2004-08-07 18:25:15.064497551 +0200
+++ processor.c	2004-08-07 18:31:24.013809421 +0200
@@ -30,6 +30,14 @@
  *	4. Need C1 timing -- must modify kernel (IRQ handler) to get this.
  */
 
+/*
+ * 07/08/04; Jos Delbar <jos.delbar-Cru1EgDzd7c@public.gmane.org>:
+ * Recognize power state limit as a boot parameter.
+ *
+ * 31/07/04; Jos Delbar <jos.delbar-Cru1EgDzd7c@public.gmane.org>:
+ * Add user-definable processor power state limit.
+ */
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -80,6 +88,16 @@
 MODULE_DESCRIPTION(ACPI_PROCESSOR_DRIVER_NAME);
 MODULE_LICENSE("GPL");
 
+/*
+ * The acpi_cstate_limit module parameter represents the maximum processor power state or
+ * C state to promote to. Values of 1, 2 and 3 are equivalent to the C1, C2 and C3 sleep
+ * states, respectively. A value of 0 disables processor power management altogether.
+ */
+
+static int acpi_cstate_limit = ACPI_C_STATES_MAX;
+
+module_param(acpi_cstate_limit, int, S_IRUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(acpi_cstate_limit, "Limit the processor power state (0 = disable power management)");
 
 static int acpi_processor_add (struct acpi_device *device);
 static int acpi_processor_remove (struct acpi_device *device, int type);
@@ -449,6 +467,7 @@
 		sleep_ticks = ticks_elapsed(t1, t2) - cx->latency_ticks - C3_OVERHEAD;
 		break;
 
+	case ACPI_STATE_C0:
 	default:
 		local_irq_enable();
 		return;
@@ -535,7 +554,7 @@
 	 * C0/C1
 	 * -----
 	 */
-	pr->power.state = ACPI_STATE_C1;
+	pr->power.state = (acpi_cstate_limit != ACPI_STATE_C0 ? ACPI_STATE_C1 : ACPI_STATE_C0);
 	pr->power.default_state = ACPI_STATE_C1;
 
 	/*
@@ -554,7 +573,7 @@
 	 * TBD: Demote to default C-State after long periods of activity.
 	 * TBD: Investigate policy's use of CPU utilization -vs- sleep duration.
 	 */
-	if (pr->power.states[ACPI_STATE_C2].valid) {
+	if (pr->power.states[ACPI_STATE_C2].valid && (acpi_cstate_limit >= ACPI_STATE_C2)) {
 		pr->power.states[ACPI_STATE_C1].promotion.threshold.count = 10;
 		pr->power.states[ACPI_STATE_C1].promotion.threshold.ticks =
 			pr->power.states[ACPI_STATE_C2].latency_ticks;
@@ -575,7 +594,7 @@
 	 * short or whenever bus mastering activity occurs.
 	 */
 	if ((pr->power.states[ACPI_STATE_C2].valid) &&
-		(pr->power.states[ACPI_STATE_C3].valid)) {
+		(pr->power.states[ACPI_STATE_C3].valid) && (acpi_cstate_limit >= ACPI_STATE_C3)) {
 		pr->power.states[ACPI_STATE_C2].promotion.threshold.count = 4;
 		pr->power.states[ACPI_STATE_C2].promotion.threshold.ticks =
 			pr->power.states[ACPI_STATE_C3].latency_ticks;
@@ -1859,10 +1878,15 @@
 		goto end;
 
 	seq_printf(seq, "active state:            C%d\n"
-			"default state:           C%d\n"
-			"bus master activity:     %08x\n",
+			"default state:           C%d\n",
 			pr->power.state,
-			pr->power.default_state,
+			pr->power.default_state);
+
+	if (acpi_cstate_limit < ACPI_C_STATES_MAX) seq_printf(seq,
+			"user limit:              C%d\n",
+			acpi_cstate_limit);
+
+	seq_printf(seq, "bus master activity:     %08x\n",
 			pr->power.bm_activity);
 
 	seq_puts(seq, "states:\n");
@@ -1876,6 +1900,11 @@
 			continue;
 		}
 
+		if (i > acpi_cstate_limit) {
+			seq_puts(seq, "<disabled>\n");
+			continue;
+		}
+
 		if (pr->power.states[i].promotion.state)
 			seq_printf(seq, "promotion[C%d] ",
 				pr->power.states[i].promotion.state);
@@ -2384,7 +2413,7 @@
 	
 	printk(KERN_INFO PREFIX "%s [%s] (supports",
 		acpi_device_name(device), acpi_device_bid(device));
-	for (i=1; i<ACPI_C_STATE_COUNT; i++)
+	for (i=0; i<=acpi_cstate_limit; i++)
 		if (pr->power.states[i].valid)
 			printk(" C%d", i);
 	if (pr->flags.throttling)
@@ -2417,7 +2446,7 @@
 	pr = (struct acpi_processor *) acpi_driver_data(device);
 
 	/* Unregister the idle handler when processor #0 is removed. */
-	if (pr->id == 0)
+	if ((pr->id == 0) && (pr->flags.power))
 		pm_idle = pm_idle_save;
 
 	status = acpi_remove_notify_handler(pr->handle, ACPI_DEVICE_NOTIFY, 
@@ -2447,6 +2476,10 @@
 	memset(&processors, 0, sizeof(processors));
 	memset(&errata, 0, sizeof(errata));
 
+	/* Check for illegal user limits. */
+	if(acpi_cstate_limit < ACPI_STATE_C0 || acpi_cstate_limit > ACPI_C_STATES_MAX)
+		return_VALUE(-EINVAL);
+
 	acpi_processor_dir = proc_mkdir(ACPI_PROCESSOR_CLASS, acpi_root_dir);
 	if (!acpi_processor_dir)
 		return_VALUE(-ENODEV);

Re: Re: Linux ACPI processor driver patch: user-definable power state limit

[Dominik Brodowski]

Len,

module parameters are even better suited for for this task:
processor.acpi_cstate_limit=1 can be passed on the boot line. I'd vote for
the removal of "acpi_", though. As soon as Rusty's and mine "module
parameters" patch make them also available in /sys/module/*/ for
modification during runtime, if so desired.

Thanks,
	Dominik



-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: Linux ACPI processor driver patch: user-definable power state limit

[Jos Delbar]

As Dominik Brodowski pointed out, the module parameter currently exported in 
this patch should be sufficient. I have the ACPI processor module compiled 
into my kernel, yet I can still use "processor.acpi_cstate_limit" on the boot 
line.

Do you require a new version of the patch without the "acpi_" prefix?

On Saturday 09 October 2004 07:52, you wrote:
> Jos,
> Can you modify this patch to use a kernel boot parameter
> rather than a module load parameter?  Some folks don't
> load this driver as a module.  Also, it gives you the
> chance to add a couple of lines to Documentation/kernel-parameters.txt
>
> thanks,
> -Len

-- 

Jos Delbar
jos.delbar-Cru1EgDzd7c@public.gmane.org


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

RE: Linux ACPI processor driver patch: user-definable power state limit

[Brown, Len]

 
>Do you require a new version of the patch without the "acpi_" prefix?

sure, and if you can re-send a version that applies cleanly
to the current tree, that would be great.

thanks,
-Len

ps. FYI, if patches are generated with "diff -Naur from "/usr/src/linux"
then that is easier for me to deal with.  eg.

a/drivers/acpi/processor.c
vs.
b/drivers/acpi/processor.c

also, attachments are ok.


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: Linux ACPI processor driver patch: user-definable power state limit

[Jos Delbar]

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

Here is a new version of the power state limit patch that applies to ACPI 
20040816. I also fixed a few bugs I bumped into along the way...

in acpi_processor_get_power_info_cst()

> @@ -671,7 +696,7 @@
>  
>         /* The first state is always C0, which is already filled. */
>         pr->power.count = 1;
> -       for (i = 1; i <= count; i++) {
> +       for (i = 1; i < count; i++) {
>                 union acpi_object *element;
>                 union acpi_object *obj;
>                 struct acpi_power_register *reg;

The count value extracted from _CST is the number of the highest C state plus 
1 and is not to be confused with the meaning of the 
acpi_processor_power.count member (pr->power.count) which is simply the 
highest C state. An extra iteration was made creating an invalid C4 state for 
my processor, visible in /proc/acpi/processor/CPU0/power.

I think the current usage of acpi_processor_power.count is misleading and 
propose to either rename the member to something like "max_state", or to give 
the member a new meaning as the highest C state plus 1 and rework the code to 
reflect that change.

in acpi_processor_get_power_info_fadt()

> @@ -901,7 +926,7 @@
>          */
>  
>         for (i = 0; i < ACPI_PROCESSOR_MAX_POWER; i++)
> -               memset(pr->power.states, 0, sizeof(struct 
acpi_processor_cx));
> +               memset(&pr->power.states[i], 0, sizeof(struct 
acpi_processor_cx));
>  
>         if (pr->pblk) {
>                 pr->power.states[ACPI_STATE_C2].address = pr->pblk + 4;

The acpi_processor_power.states (pr->power.states) array was not properly 
initialized.

- Jos

On Sunday 10 October 2004 18:56, you wrote:
> >Do you require a new version of the patch without the "acpi_" prefix?
>
> sure, and if you can re-send a version that applies cleanly
> to the current tree, that would be great.
>
> thanks,
> -Len
>
> ps. FYI, if patches are generated with "diff -Naur from "/usr/src/linux"
> then that is easier for me to deal with.  eg.
>
> a/drivers/acpi/processor.c
> vs.
> b/drivers/acpi/processor.c
>
> also, attachments are ok.

-- 

Jos Delbar
jos.delbar-Cru1EgDzd7c@public.gmane.org

[-- Attachment #2: processor.c.diff --]
[-- Type: text/x-diff, Size: 6070 bytes --]

--- a/drivers/acpi/processor.c	2004-10-11 22:09:52.000000000 +0200
+++ b/drivers/acpi/processor.c	2004-10-11 22:56:06.666140905 +0200
@@ -30,6 +30,13 @@
  *	4. Need C1 timing -- must modify kernel (IRQ handler) to get this.
  */
 
+/*
+ * 11/10/2004; Jos Delbar <jos.delbar-Cru1EgDzd7c@public.gmane.org>
+ * Add user-definable processor power state limit (cstate_limit).
+ * Various bug fixes.
+ * Patch applies to acpi-20040816-26-stable.
+ */
+
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -80,6 +87,17 @@
 MODULE_DESCRIPTION(ACPI_PROCESSOR_DRIVER_NAME);
 MODULE_LICENSE("GPL");
 
+/*
+ * The cstate_limit module parameter represents the maximum processor power state or C
+ * state to promote to. Values of 1, 2 and 3 are equivalent to the C1, C2 and C3 sleep
+ * states, respectively. A value of 0 disables processor power management altogether.
+ */
+
+static int cstate_limit = ACPI_C_STATES_MAX;
+
+module_param(cstate_limit, int, S_IRUSR | S_IRGRP | S_IROTH);
+MODULE_PARM_DESC(cstate_limit, "Limit the processor power state (0 = disable power management)");
+
 
 static int acpi_processor_add (struct acpi_device *device);
 static int acpi_processor_remove (struct acpi_device *device, int type);
@@ -454,6 +472,7 @@
 		sleep_ticks = ticks_elapsed(t1, t2) - cx->latency_ticks - C3_OVERHEAD;
 		break;
 
+	case ACPI_STATE_C0:
 	default:
 		local_irq_enable();
 		return;
@@ -544,7 +563,7 @@
 	 * C0/C1
 	 * -----
 	 */
-	pr->power.state = ACPI_STATE_C1;
+	pr->power.state = (cstate_limit != ACPI_STATE_C0 ? ACPI_STATE_C1 : ACPI_STATE_C0);
 	pr->power.default_state = ACPI_STATE_C1;
 
 	for (i = 0; i <= pr->power.count; i++) {
@@ -571,15 +590,18 @@
 		 * TBD: Investigate policy's use of CPU utilization -vs- sleep duration.
 		 * XXX update comment.
 		 */
-		pr->power.states[i-1].promotion.threshold.count = 10;
-		pr->power.states[i-1].promotion.threshold.ticks =
-			pr->power.states[i].latency_ticks;
-		pr->power.states[i-1].promotion.state = i;
-
-		pr->power.states[i].demotion.threshold.count = 1;
-		pr->power.states[i].demotion.threshold.ticks =
-			pr->power.states[i].latency_ticks;
-		pr->power.states[i].demotion.state = i-1;
+		if (cstate_limit >= ACPI_STATE_C2)
+		{
+			pr->power.states[i-1].promotion.threshold.count = 10;
+			pr->power.states[i-1].promotion.threshold.ticks =
+				pr->power.states[i].latency_ticks;
+			pr->power.states[i-1].promotion.state = i;
+	
+			pr->power.states[i].demotion.threshold.count = 1;
+			pr->power.states[i].demotion.threshold.ticks =
+				pr->power.states[i].latency_ticks;
+			pr->power.states[i].demotion.state = i-1;
+		}
 			break;
   
 		case ACPI_STATE_C3:
@@ -593,17 +615,20 @@
 		 *
 		 * XXX update comment.
 		 */
-		pr->power.states[i-1].promotion.threshold.count = 4;
-		pr->power.states[i-1].promotion.threshold.ticks =
-			pr->power.states[i].latency_ticks;
-		pr->power.states[i-1].promotion.threshold.bm = 0x0F;
-		pr->power.states[i-1].promotion.state = i;
-
-		pr->power.states[i].demotion.threshold.count = 1;
-		pr->power.states[i].demotion.threshold.ticks =
-			pr->power.states[i].latency_ticks;
-		pr->power.states[i].demotion.threshold.bm = 0x0F;
-		pr->power.states[i].demotion.state = i-1;
+		if (cstate_limit >= ACPI_STATE_C3)
+		{
+			pr->power.states[i-1].promotion.threshold.count = 4;
+			pr->power.states[i-1].promotion.threshold.ticks =
+				pr->power.states[i].latency_ticks;
+			pr->power.states[i-1].promotion.threshold.bm = 0x0F;
+			pr->power.states[i-1].promotion.state = i;
+	
+			pr->power.states[i].demotion.threshold.count = 1;
+			pr->power.states[i].demotion.threshold.ticks =
+				pr->power.states[i].latency_ticks;
+			pr->power.states[i].demotion.threshold.bm = 0x0F;
+			pr->power.states[i].demotion.state = i-1;
+		}
 			break;
 		default:
 			return_VALUE(-EINVAL);
@@ -671,7 +696,7 @@
 
 	/* The first state is always C0, which is already filled. */
 	pr->power.count = 1;
-	for (i = 1; i <= count; i++) {
+	for (i = 1; i < count; i++) {
 		union acpi_object *element;
 		union acpi_object *obj;
 		struct acpi_power_register *reg;
@@ -901,7 +926,7 @@
 	 */
 
 	for (i = 0; i < ACPI_PROCESSOR_MAX_POWER; i++)
-		memset(pr->power.states, 0, sizeof(struct acpi_processor_cx));
+		memset(&pr->power.states[i], 0, sizeof(struct acpi_processor_cx));
 
 	if (pr->pblk) {
 		pr->power.states[ACPI_STATE_C2].address = pr->pblk + 4;
@@ -2185,10 +2210,15 @@
 		goto end;
 
 	seq_printf(seq, "active state:            C%d\n"
-			"default state:           C%d\n"
-			"bus master activity:     %08x\n",
+			"default state:           C%d\n",
 			pr->power.state,
-			pr->power.default_state,
+			pr->power.default_state);
+
+	if (cstate_limit < ACPI_C_STATES_MAX) seq_printf(seq,
+			"user limit:              C%d\n",
+			cstate_limit);
+
+	seq_printf(seq, "bus master activity:     %08x\n",
 			pr->power.bm_activity);
 
 	seq_puts(seq, "states:\n");
@@ -2204,6 +2234,11 @@
 
 		seq_printf(seq, "type[%d] ", pr->power.states[i].type);
 
+		if (i > cstate_limit) {
+			seq_puts(seq, "<disabled>\n");
+			continue;
+		}
+
 		if (pr->power.states[i].promotion.state)
 			seq_printf(seq, "promotion[C%d] ",
 				pr->power.states[i].promotion.state);
@@ -2727,7 +2762,7 @@
 
 	printk(KERN_INFO PREFIX "%s [%s] (supports",
 		acpi_device_name(device), acpi_device_bid(device));
-	for (i = 1; i <= pr->power.count; i++)
+	for (i = 0; i <= cstate_limit; i++)
 		if (pr->power.states[i].valid)
 			printk(" C%d", i);
 	if (pr->flags.throttling)
@@ -2770,7 +2805,7 @@
 	}
 
 	/* Unregister the idle handler when processor #0 is removed. */
-	if (pr->id == 0)
+	if ((pr->id == 0) && (pr->flags.power))
 		pm_idle = pm_idle_save;
 
 	acpi_processor_remove_fs(device);
@@ -2793,6 +2828,10 @@
 	memset(&processors, 0, sizeof(processors));
 	memset(&errata, 0, sizeof(errata));
 
+	/* Check for illegal user limits. */
+	if(cstate_limit < ACPI_STATE_C0 || cstate_limit > ACPI_C_STATES_MAX)
+		return_VALUE(-EINVAL);
+
 	acpi_processor_dir = proc_mkdir(ACPI_PROCESSOR_CLASS, acpi_root_dir);
 	if (!acpi_processor_dir)
 		return_VALUE(-ENODEV);

Re: Linux ACPI processor driver patch: user-definable power state limit

[Len Brown]

Jos,
I applied a simpler patch from Andi Kleen to disable c2/c3
http://bugme.osdl.org/show_bug.cgi?id=3549

Let me know if it works for you, or if you have any updates
on top of it.

thanks,
-Len




-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl

Re: Linux ACPI processor driver patch: user-definable power state limit

[Len Brown]

Jos,
I agree with you that a single parameter is simpler.

Another thing we need to address -- with either scheme --
is that the parameter must be set in the kernel, not
in the processor modules.

This is because it is necessary for modules, such as ipw2100
to be able to disable c3 automatically when they detect
that it is interfering with their operation.

so we'd export a function from the base kernel for
modules to set the limit, and we'd simply export
the value of acpi_cstate_limit for processor.c
to observe at run-time.

int
acpi_set_cstate_limit(int limit)

int acpi_cstate_limit;

I think it can return the old limit so that
the caller can potentially un-do its call,
or perhaps setting the limit to 0 should
simply mean clear any limit.

cheers,
-Len




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: Linux ACPI processor driver patch: user-definable power state limit

[Len Brown]

Please try out this acpi_cstate_limit patch.

This applies to 2.6.8.1, but in the likely event
that may mailer wraps this patch or you need 2.6.9, go here: 
http://ftp.kernel.org/pub/linux/kernel/people/lenb/acpi/patches/test/cstate_limit/

thanks,
-Len

-----------
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/11/05 17:39:51-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org 
#   [ACPI] Allow limiting idle C-States.
#   
#   Useful to workaround C3 ipw2100 packet loss
#   and resonating noises heard on some laptops.
#   
#   For static processor driver, boot cmdline:
#   processor.acpi_cstate_limit=2
#   
#   For processor module, /etc/modprobe.conf:
#   options processor acpi_cstate_limit=2
#   
#   For manual processor module load:
#   # modprobe processor acpi_cstate_limit=2
#   
#   From kernel or kernel module:
#   #include <linux/acpi.h>
#   acpi_set_cstate_limit(2);
#   
#   Inspired by patches from Jos Delbar and Andi Kleen
#   Signed-off-by: Len Brown <len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
# 
# include/linux/acpi.h
#   2004/11/05 17:39:45-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org +26 -0
#   define API to set and get acpi_cstate_limit
# 
# drivers/acpi/processor.c
#   2004/11/05 17:39:45-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org +10 -1
#   set and obey acpi_cstate_limit
# 
# drivers/acpi/osl.c
#   2004/11/05 17:39:45-05:00 len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org +9 -0
#   define acpi_cstate_limit
# 
diff -Nru a/drivers/acpi/osl.c b/drivers/acpi/osl.c
--- a/drivers/acpi/osl.c	2004-11-05 17:40:07 -05:00
+++ b/drivers/acpi/osl.c	2004-11-05 17:40:07 -05:00
@@ -1078,3 +1078,12 @@
 
 __setup("acpi_leave_gpes_disabled", acpi_leave_gpes_disabled_setup);
 
+/*
+ * acpi_cstate_limit is defined in the base kernel so modules can
+ * change it w/o depending on the state of the processor module.
+ */
+unsigned int acpi_cstate_limit = ACPI_C_STATES_MAX;
+
+
+EXPORT_SYMBOL(acpi_cstate_limit);
+
diff -Nru a/drivers/acpi/processor.c b/drivers/acpi/processor.c
--- a/drivers/acpi/processor.c	2004-11-05 17:40:07 -05:00
+++ b/drivers/acpi/processor.c	2004-11-05 17:40:07 -05:00
@@ -459,8 +459,9 @@
 	 * Track the number of longs (time asleep is greater than threshold)
 	 * and promote when the count threshold is reached.  Note that bus
 	 * mastering activity may prevent promotions.
+	 * Do not promote above acpi_cstate_limit.
 	 */
-	if (cx->promotion.state) {
+	if (cx->promotion.state && (cx->promotion.state <= acpi_cstate_limit))
{
 		if (sleep_ticks > cx->promotion.threshold.ticks) {
 			cx->promotion.count++;
  			cx->demotion.count = 0;
@@ -498,6 +499,13 @@
 
 end:
 	/*
+	 * Demote if current state exceeds acpi_cstate_limit
+	 */
+	if (pr->power.state > acpi_cstate_limit) {
+		next_state = acpi_cstate_limit;
+	}
+
+	/*
 	 * New Cx State?
 	 * -------------
 	 * If we're going to start using a new Cx state we must clean up
@@ -2441,5 +2449,6 @@
 
 module_init(acpi_processor_init);
 module_exit(acpi_processor_exit);
+module_param_named(acpi_cstate_limit, acpi_cstate_limit, uint, 0);
 
 EXPORT_SYMBOL(acpi_processor_set_thermal_limit);
diff -Nru a/include/linux/acpi.h b/include/linux/acpi.h
--- a/include/linux/acpi.h	2004-11-05 17:40:07 -05:00
+++ b/include/linux/acpi.h	2004-11-05 17:40:07 -05:00
@@ -471,4 +471,30 @@
 
 #endif /*!CONFIG_ACPI_INTERPRETER*/
 
+#define	ACPI_CSTATE_LIMIT_DEFINED	/* for driver builds */
+#ifdef	CONFIG_ACPI
+
+/*
+ * Set highest legal C-state
+ * 0: C0 okay, but not C1
+ * 1: C1 okay, but not C2
+ * 2: C2 okay, but not C3 etc.
+ */
+
+extern unsigned int acpi_cstate_limit;
+
+static inline unsigned int acpi_get_cstate_limit(void)
+{
+	return acpi_cstate_limit;
+}
+static inline void acpi_set_cstate_limit(unsigned int new_limit)
+{
+	acpi_cstate_limit = new_limit;
+	return;
+}
+#else
+static inline unsigned int acpi_get_cstate_limit(void) { return 0; }
+static inline void acpi_set_cstate_limit(unsigned int new_limit) {
return; }
+#endif
+
 #endif /*_LINUX_ACPI_H*/





-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: Linux ACPI processor driver patch: user-definable power state limit

[Jos Delbar]

Hey,

To allow modifying the C state limit on the fly, I think we might need to use 
a combination of both schemes.

In Andi Kleen's patch the C state is "permanently" disabled in 
acpi_processor_get_power_info() and this is probably the best way to go for 
the blacklist. However, when another module needs to limit the processor to a 
certain C state, I think it would be best to handle this in 
acpi_processor_set_power_policy(). This seems to make the most sense 
semantically: you can change the power management policy at runtime, not the 
actual capabilities of the processor.

Also, if you have both a blacklist and an option to change the limit at 
runtime, there has to be some logic to prevent a module from lifting a limit 
imposed by the blacklist.

Your patch just came in while I was writing this, I'll be sure to try it later 
today. I'm not sure if the idle handler should be concerned with handling the 
limit though, but it does make the code simpler.

Regards,

- Jos

On Friday 05 November 2004 20:45, Len Brown wrote:
> Jos,
> I agree with you that a single parameter is simpler.
>
> Another thing we need to address -- with either scheme --
> is that the parameter must be set in the kernel, not
> in the processor modules.
>
> This is because it is necessary for modules, such as ipw2100
> to be able to disable c3 automatically when they detect
> that it is interfering with their operation.
>
> so we'd export a function from the base kernel for
> modules to set the limit, and we'd simply export
> the value of acpi_cstate_limit for processor.c
> to observe at run-time.
>
> int
> acpi_set_cstate_limit(int limit)
>
> int acpi_cstate_limit;
>
> I think it can return the old limit so that
> the caller can potentially un-do its call,
> or perhaps setting the limit to 0 should
> simply mean clear any limit.
>
> cheers,
> -Len

-- 

Jos Delbar
jos.delbar-Cru1EgDzd7c@public.gmane.org


-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: Linux ACPI processor driver patch: user-definable power state limit

[Len Brown]

On Fri, 2004-11-05 at 18:21, Jos Delbar wrote:

> 
> Also, if you have both a blacklist and an option to change the limit
> at runtime, there has to be some logic to prevent a module from
> lifting a limit imposed by the blacklist.

the logic could be in the user of the API to simply check
the limit first and be sure that it only lowers and doesn't raise it.
yeah, no guarantees here, but for the users at hand, maybe sufficient.

i'll think about this when i merge on top of andy's patch.  maybe keep
the blacklist limit as a hard limit.  But frankly I'm not sure it is
worth a single byte more of code b/c it is the beginning of the
over-designed workaround slippery slope;-)

> Your patch just came in while I was writing this, I'll be sure to try
> it later today. I'm not sure if the idle handler should be concerned
> with handling the limit though, but it does make the code simpler.

since we had to handle a limit change on the fly i did it that way, and
then i realized that maybe on-the-fly checking is sufficient.

-Len




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click

Re: Linux ACPI processor driver patch: user-definable power state limit

[Len Brown]

Andi's c2/c3 blacklist patch alters the promote/demote
structures and the acpi_cstate_limit patch
lives within those bounds.

So we keep both for now and acpi_state_limit
will never break the blacklist.

Hopefully we'll figure out the problem with the
two c2/c3 blacklisted systems and delete them
and that workaround over time.

thanks,
-Len




-------------------------------------------------------
This SF.Net email is sponsored by:
Sybase ASE Linux Express Edition - download now for FREE
LinuxWorld Reader's Choice Award Winner for best database on Linux.
http://ads.osdn.com/?ad_id=5588&alloc_id=12065&op=click