Followup on 2.6.13-rc3 ACPI processor C-state regression

Followup on 2.6.13-rc3 ACPI processor C-state regression

[Kevin Radloff]

Previously, I had said that in 2.6.13-rc3, C2/C3 capabilities were not
detected on my Fujitsu Lifebook P7010D. I found that in the merge at:

http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdiff;h=893b074e3d1a48a4390cf84b4c1a10ef6be2460c;hp=c9d671cf7857dbc7101e99d469fa24eed711ac60;hb=5028770a42e7bc4d15791a44c28f0ad539323807;f=drivers/acpi/processor_idle.c

.. in the section at (please forgive my destruction of the formatting) ...

@@ -787,10 +843,7 @@ static int acpi_processor_get_power_info
         if ((result) || (acpi_processor_power_verify(pr) < 2)) {
             result = acpi_processor_get_power_info_fadt(pr);
             if (result)
-                return_VALUE(result);
-
-            if (acpi_processor_power_verify(pr) < 2)
-                return_VALUE(-ENODEV);
+               result = acpi_processor_get_power_info_default_c1(pr);
         }

.. a call to acpi_processor_power_verify() is removed, which breaks
detection of C2/C3 capabilities if the above
acpi_processor_get_power_info_cst() failed. It it had succeeded (and
returned 0), then acpi_processor_power_verify() is called in the
conditional statement, which will set the valid flags for C2/C3. But
if it fails, like on my laptop, then the valid flags will never be
set, despite the fact that the acpi_processor_get_power_info_fadt()
function finds the necessary info for a subsequent
acpi_processor_power_verify() call to succeed.

I don't know what exactly the proper fix here is (with the
introduction of the acpi_processor_get_power_info_default_c1()
function, that is), but simply reversing this part of the patch fixes
detection of C2/C3 on my laptop.

Please CC me with any followups, as I'm not on the list.

-- 
Kevin 'radsaq' Radloff
radsaq@gmail.com
http://saqataq.us/

Re: Followup on 2.6.13-rc3 ACPI processor C-state regression

[Andrew Morton]

Kevin Radloff <radsaq-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Previously, I had said that in 2.6.13-rc3, C2/C3 capabilities were not
> detected on my Fujitsu Lifebook P7010D. I found that in the merge at:
> 
> http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdiff;h=893b074e3d1a48a4390cf84b4c1a10ef6be2460c;hp=c9d671cf7857dbc7101e99d469fa24eed711ac60;hb=5028770a42e7bc4d15791a44c28f0ad539323807;f=drivers/acpi/processor_idle.c
> 
> .. in the section at (please forgive my destruction of the formatting) ...
> 
> @@ -787,10 +843,7 @@ static int acpi_processor_get_power_info
>          if ((result) || (acpi_processor_power_verify(pr) < 2)) {
>              result = acpi_processor_get_power_info_fadt(pr);
>              if (result)
> -                return_VALUE(result);
> -
> -            if (acpi_processor_power_verify(pr) < 2)
> -                return_VALUE(-ENODEV);
> +               result = acpi_processor_get_power_info_default_c1(pr);
>          }
> 
> .. a call to acpi_processor_power_verify() is removed, which breaks
> detection of C2/C3 capabilities if the above
> acpi_processor_get_power_info_cst() failed. It it had succeeded (and
> returned 0), then acpi_processor_power_verify() is called in the
> conditional statement, which will set the valid flags for C2/C3. But
> if it fails, like on my laptop, then the valid flags will never be
> set, despite the fact that the acpi_processor_get_power_info_fadt()
> function finds the necessary info for a subsequent
> acpi_processor_power_verify() call to succeed.
> 
> I don't know what exactly the proper fix here is (with the
> introduction of the acpi_processor_get_power_info_default_c1()
> function, that is), but simply reversing this part of the patch fixes
> detection of C2/C3 on my laptop.
> 

Len, Kevin confirms that the below patch fixes the above regression for
him.  Should we merge it now?


From: Jindrich Makovicka <makovick-/k6j/d2J2EitDQUhxP5w7+MCGPLT8p0O@public.gmane.org>

Re-enable C2/C3 states for systems without CST.  Fixes a regression after
the patch for C2/C3 support for multiprocessors
(http://bugme.osdl.org/show_bug.cgi?id=4401), which accidentally removed
the acpi_processor_power_verify() call.

Signed-off-by: Jindrich Makovicka <makovick-/k6j/d2J2EitDQUhxP5w7+MCGPLT8p0O@public.gmane.org>
Cc: "Brown, Len" <len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Andrew Morton <akpm-3NddpPZAyC0@public.gmane.org>
---

 drivers/acpi/processor_idle.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/acpi/processor_idle.c~acpi-re-enable-c2-c3-cpu-states-for-systems-without drivers/acpi/processor_idle.c
--- devel/drivers/acpi/processor_idle.c~acpi-re-enable-c2-c3-cpu-states-for-systems-without	2005-07-14 15:53:47.000000000 -0700
+++ devel-akpm/drivers/acpi/processor_idle.c	2005-07-14 15:53:47.000000000 -0700
@@ -881,7 +881,7 @@ static int acpi_processor_get_power_info
 	result = acpi_processor_get_power_info_cst(pr);
 	if ((result) || (acpi_processor_power_verify(pr) < 2)) {
 		result = acpi_processor_get_power_info_fadt(pr);
-		if (result)
+		if ((result) || acpi_processor_power_verify(pr) < 2)
 			result = acpi_processor_get_power_info_default_c1(pr);
 	}
 
_



-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

Re: Followup on 2.6.13-rc3 ACPI processor C-state regression

[Andrew Morton]

Kevin Radloff <radsaq@gmail.com> wrote:
>
> Previously, I had said that in 2.6.13-rc3, C2/C3 capabilities were not
> detected on my Fujitsu Lifebook P7010D. I found that in the merge at:
> 
> http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blobdiff;h=893b074e3d1a48a4390cf84b4c1a10ef6be2460c;hp=c9d671cf7857dbc7101e99d469fa24eed711ac60;hb=5028770a42e7bc4d15791a44c28f0ad539323807;f=drivers/acpi/processor_idle.c
> 
> .. in the section at (please forgive my destruction of the formatting) ...
> 
> @@ -787,10 +843,7 @@ static int acpi_processor_get_power_info
>          if ((result) || (acpi_processor_power_verify(pr) < 2)) {
>              result = acpi_processor_get_power_info_fadt(pr);
>              if (result)
> -                return_VALUE(result);
> -
> -            if (acpi_processor_power_verify(pr) < 2)
> -                return_VALUE(-ENODEV);
> +               result = acpi_processor_get_power_info_default_c1(pr);
>          }
> 
> .. a call to acpi_processor_power_verify() is removed, which breaks
> detection of C2/C3 capabilities if the above
> acpi_processor_get_power_info_cst() failed. It it had succeeded (and
> returned 0), then acpi_processor_power_verify() is called in the
> conditional statement, which will set the valid flags for C2/C3. But
> if it fails, like on my laptop, then the valid flags will never be
> set, despite the fact that the acpi_processor_get_power_info_fadt()
> function finds the necessary info for a subsequent
> acpi_processor_power_verify() call to succeed.
> 
> I don't know what exactly the proper fix here is (with the
> introduction of the acpi_processor_get_power_info_default_c1()
> function, that is), but simply reversing this part of the patch fixes
> detection of C2/C3 on my laptop.
> 

Len, Kevin confirms that the below patch fixes the above regression for
him.  Should we merge it now?


From: Jindrich Makovicka <makovick@kmlinux.fjfi.cvut.cz>

Re-enable C2/C3 states for systems without CST.  Fixes a regression after
the patch for C2/C3 support for multiprocessors
(http://bugme.osdl.org/show_bug.cgi?id=4401), which accidentally removed
the acpi_processor_power_verify() call.

Signed-off-by: Jindrich Makovicka <makovick@kmlinux.fjfi.cvut.cz>
Cc: "Brown, Len" <len.brown@intel.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 drivers/acpi/processor_idle.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/acpi/processor_idle.c~acpi-re-enable-c2-c3-cpu-states-for-systems-without drivers/acpi/processor_idle.c
--- devel/drivers/acpi/processor_idle.c~acpi-re-enable-c2-c3-cpu-states-for-systems-without	2005-07-14 15:53:47.000000000 -0700
+++ devel-akpm/drivers/acpi/processor_idle.c	2005-07-14 15:53:47.000000000 -0700
@@ -881,7 +881,7 @@ static int acpi_processor_get_power_info
 	result = acpi_processor_get_power_info_cst(pr);
 	if ((result) || (acpi_processor_power_verify(pr) < 2)) {
 		result = acpi_processor_get_power_info_fadt(pr);
-		if (result)
+		if ((result) || acpi_processor_power_verify(pr) < 2)
 			result = acpi_processor_get_power_info_default_c1(pr);
 	}
 
_

Re: Followup on 2.6.13-rc3 ACPI processor C-state regression

[Jose Luis Domingo Lopez]

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

On Wednesday, 13 July 2005, at 18:05:30 -0400,
Kevin Radloff wrote:

> Previously, I had said that in 2.6.13-rc3, C2/C3 capabilities were not
> detected on my Fujitsu Lifebook P7010D. I found that in the merge at:
> 
I was just about to gather data to report the exact same problem in my
box, a no-brand non-mobile PC (AMD Athlon XP 1700+). Kernel version
2.6.13-rc2 and before correctly detected both C1 and C2 processor states,
so when the system is idle some energy and heat is preserved. Now, with
2.6.13-rc3-git8 only C1 gets detected, so no power save.

I am going to try the patch Andrew suggest in this same thread right now,
and will report back is this fixes the issue.

I inline a (trimmed) diff of two boots, the first with the (working)
2.6.13-rc2 kernel version, and the other with the (non-working)
2.6.13-rc3-git8 one. No changes in the .config file except for the new
options appeared in between (some options shuffling apart, the only change
relevant to the problem should be the new CONFIG_ACPI_HOTKEY=m):

--- boot_stripped-2.6.13-rc2.txt	2005-07-29 19:46:44.000000000 +0200
+++ boot_stripped-2.6.13-rc3-git8.txt	2005-07-29 19:46:59.000000000 +0200
@@ -1,4 +1,4 @@
-dardhal kernel: Linux version 2.6.13-rc2 (jdomingo@dardhal) (gcc versión 3.3.6 (Debian 1:3.3.6-7)) #2 Fri Jul 8 22:06:45 CEST 2005
+dardhal kernel: Linux version 2.6.13-rc3-git8 (jdomingo@dardhal) (gcc versión 3.3.6 (Debian 1:3.3.6-7)) #3 Wed Jul 27 20:53:55 CEST 2005
 dardhal kernel: BIOS-provided physical RAM map:
 dardhal kernel: BIOS-e820: 0000000000000000 - 000000000009fc00 (usable)
 dardhal kernel: BIOS-e820: 000000000009fc00 - 00000000000a0000 (reserved)
@@ -11,7 +11,7 @@
 dardhal kernel: DMI 2.3 present.
 dardhal kernel: Allocating PCI resources starting at 20000000 (gap: 20000000:dfff0000)
 dardhal kernel: Built 1 zonelists
-dardhal kernel: Kernel command line: auto BOOT_IMAGE=2.6.13-rc2 ro root=302 rootfstype=ext3
+dardhal kernel: Kernel command line: BOOT_IMAGE=2.6.13rc3git8 ro root=302 rootfstype=ext3
 dardhal kernel: Local APIC disabled by BIOS -- you can enable it with "lapic"
 dardhal kernel: Initializing CPU#0
 dardhal kernel: PID hash table entries: 2048 (order: 11, 32768 bytes)
@@ -20,31 +20,32 @@
 dardhal kernel: Console: colour VGA+ 80x25
 dardhal kernel: Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
 dardhal kernel: Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
-dardhal kernel: Memory: 515756k/524224k available (1805k kernel code, 7920k reserved, 835k data, 160k init, 0k highmem)
+dardhal kernel: Memory: 515740k/524224k available (1817k kernel code, 7936k reserved, 843k data, 160k init, 0k highmem)
 dardhal kernel: Checking if this processor honours the WP bit even in supervisor mode... Ok.
-dardhal kernel: Calibrating delay using timer specific routine.. 2959.79 BogoMIPS (lpj=5919586)
+dardhal kernel: Calibrating delay using timer specific routine.. 2959.75 BogoMIPS (lpj=5919514)
 dardhal kernel: Mount-cache hash table entries: 512
 dardhal kernel: CPU: L1 I Cache: 64K (64 bytes/line), D cache 64K (64 bytes/line)
 dardhal kernel: CPU: L2 Cache: 256K (64 bytes/line)
 dardhal kernel: Intel machine check architecture supported.
 dardhal kernel: Intel machine check reporting enabled on CPU#0.
+dardhal kernel: mtrr: v2.0 (20020519)
 dardhal kernel: CPU: AMD Athlon(tm) XP 1700+ stepping 02
 dardhal kernel: Enabling fast FPU save and restore... done.
 dardhal kernel: Enabling unmasked SIMD FPU exception support... done.
 dardhal kernel: Checking 'hlt' instruction... OK.
-dardhal kernel: tbxface-0118 [02] acpi_load_tables      : ACPI Tables successfully acquired
+dardhal kernel: tbxface-0120 [02] acpi_load_tables      : ACPI Tables successfully acquired
 dardhal kernel: Parsing all Control Methods:.......................................................................................................
 dardhal kernel: Table [DSDT](id F004) - 398 Objects with 37 Devices 103 Methods 23 Regions
-dardhal kernel: ACPI Namespace successfully loaded at root c03d7ba0
+dardhal kernel: ACPI Namespace successfully loaded at root c03dc5a0
 dardhal kernel: ACPI: setting ELCR to 0200 (from 0a20)
-dardhal kernel: evxfevnt-0094 [03] acpi_enable           : Transition to ACPI mode successful
+dardhal kernel: evxfevnt-0096 [03] acpi_enable           : Transition to ACPI mode successful
 dardhal kernel: NET: Registered protocol family 16
+dardhal kernel: ACPI: bus type pci registered
 dardhal kernel: PCI: PCI BIOS revision 2.10 entry at 0xfb250, last bus=1
 dardhal kernel: PCI: Using configuration type 1
-dardhal kernel: mtrr: v2.0 (20020519)
-dardhal kernel: ACPI: Subsystem revision 20050309
-dardhal kernel: evgpeblk-0979 [06] ev_create_gpe_block   : GPE 00 to 0F [_GPE] 2 regs on int 0x9
-dardhal kernel: evgpeblk-0987 [06] ev_create_gpe_block   : Found 4 Wake, Enabled 1 Runtime GPEs in this block
+dardhal kernel: ACPI: Subsystem revision 20050408
+dardhal kernel: evgpeblk-1016 [06] ev_create_gpe_block   : GPE 00 to 0F [_GPE] 2 regs on int 0x9
+dardhal kernel: evgpeblk-1024 [06] ev_create_gpe_block   : Found 4 Wake, Enabled 1 Runtime GPEs in this block
 dardhal kernel: Completing Region/Field/Buffer/Package initialization:....................................................
 dardhal kernel: Initialized 23/23 Regions 5/5 Fields 18/18 Buffers 6/10 Packages (407 nodes)
 dardhal kernel: Executing all Device _STA and_INI methods:........................................
@@ -53,6 +54,8 @@
 dardhal kernel: ACPI: Using PIC for interrupt routing
 dardhal kernel: ACPI: PCI Root Bridge [PCI0] (0000:00)
 dardhal kernel: PCI: Probing PCI hardware (bus 00)
+dardhal kernel: ACPI: Assume root bridge [\_SB_.PCI0] segment is 0
+dardhal kernel: ACPI: Assume root bridge [\_SB_.PCI0] bus is 0
 dardhal kernel: ACPI: PCI Interrupt Link [LNKA] (IRQs 1 3 4 5 6 7 10 *11 12 14 15)
 dardhal kernel: ACPI: PCI Interrupt Link [LNKB] (IRQs 1 3 4 *5 6 7 10 11 12 14 15)
 dardhal kernel: ACPI: PCI Interrupt Link [LNKC] (IRQs 1 3 4 *5 6 7 10 11 12 14 15)
@@ -68,8 +71,9 @@
 dardhal kernel: Dquot-cache hash table entries: 1024 (order 0, 4096 bytes)
 dardhal kernel: Initializing Cryptographic API
 dardhal kernel: ACPI: Power Button (FF) [PWRF]
+dardhal kernel: ACPI: Power Button (CM) [PWRB]
 dardhal kernel: ACPI: Sleep Button (CM) [SLPB]
-dardhal kernel: ACPI: CPU0 (power states: C1[C1] C2[C2])
+dardhal kernel: ACPI: CPU0 (power states: C1[C1])
 dardhal kernel: ACPI: Processor [CPU0] (supports 2 throttling states)
 dardhal kernel: serio: i8042 AUX port at 0x60,0x64 irq 12
 dardhal kernel: serio: i8042 KBD port at 0x60,0x64 irq 1
@@ -85,6 +89,7 @@
 dardhal kernel: Uniform Multi-Platform E-IDE driver Revision: 7.00alpha2
 dardhal kernel: ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx
 dardhal kernel: VP_IDE: IDE controller at PCI slot 0000:00:11.1
+dardhal kernel: acpi_bus-0212 [01] acpi_bus_set_power    : Device is not power manageable
 dardhal kernel: ACPI: PCI Interrupt Link [LNKA] enabled at IRQ 11
 dardhal kernel: PCI: setting IRQ 11 as level-triggered
 dardhal kernel: ACPI: PCI Interrupt 0000:00:11.1[A] -> Link [LNKA] -> GSI 11 (level, low) -> IRQ 11
@@ -193,6 +191,7 @@
 dardhal kernel: usbcore: registered new driver usbfs
 dardhal kernel: usbcore: registered new driver hub
 dardhal kernel: USB Universal Host Controller Interface driver v2.3
+dardhal kernel: acpi_bus-0212 [01] acpi_bus_set_power    : Device is not power manageable
 dardhal kernel: ACPI: PCI Interrupt 0000:00:11.2[D] -> Link [LNKD] -> GSI 11 (level, low) -> IRQ 11
 dardhal kernel: PCI: Via IRQ fixup for 0000:00:11.2, from 0 to 11
 dardhal kernel: uhci_hcd 0000:00:11.2: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller
@@ -200,6 +199,7 @@
 dardhal kernel: uhci_hcd 0000:00:11.2: irq 11, io base 0x0000dc00
 dardhal kernel: hub 1-0:1.0: USB hub found
 dardhal kernel: hub 1-0:1.0: 2 ports detected
+dardhal kernel: acpi_bus-0212 [01] acpi_bus_set_power    : Device is not power manageable
 dardhal kernel: ACPI: PCI Interrupt 0000:00:11.3[D] -> Link [LNKD] -> GSI 11 (level, low) -> IRQ 11
 dardhal kernel: PCI: Via IRQ fixup for 0000:00:11.3, from 255 to 11
 dardhal kernel: uhci_hcd 0000:00:11.3: VIA Technologies, Inc. VT82xxxxx UHCI USB 1.1 Controller (#2)

Hope it helps.

Greetings,

-- 
Jose Luis Domingo Lopez
Linux Registered User #189436     Debian Linux Sid (Linux 2.6.13-rc3-git8)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Followup on 2.6.13-rc3 ACPI processor C-state regression

[Jose Luis Domingo Lopez]

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

On Friday, 29 July 2005, at 20:33:18 +0200,
Jose Luis Domingo Lopez wrote:

> I was just about to gather data to report the exact same problem in my
> box, a no-brand non-mobile PC (AMD Athlon XP 1700+). Kernel version
> 2.6.13-rc2 and before correctly detected both C1 and C2 processor states,
> so when the system is idle some energy and heat is preserved. Now, with
> 2.6.13-rc3-git8 only C1 gets detected, so no power save.
> 
> I am going to try the patch Andrew suggest in this same thread right now,
> and will report back is this fixes the issue.
> 
Just rebooted with a 2.6.13-rc3-git8 kernel patched with the patch Andrew
showed in his last email in this thread, and now processor C2 power state
is recognized and operational again, giving in my setup a 12ºC decrease in
processor temperature while idle.

Greetings,

-- 
Jose Luis Domingo Lopez
Linux Registered User #189436     Debian Linux Sid (Linux 2.6.13-rc3-git8)


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

RE: Followup on 2.6.13-rc3 ACPI processor C-state regression

[Brown, Len]

>Len, Kevin confirms that the below patch fixes the above regression for
>him.  Should we merge it now?

Yes.
It is in my to-linus git tree.

thanks,
-Len


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_idt77&alloc_id\x16492&op=click

RE: Followup on 2.6.13-rc3 ACPI processor C-state regression

[Brown, Len]

>Len, Kevin confirms that the below patch fixes the above regression for
>him.  Should we merge it now?

Yes.
It is in my to-linus git tree.

thanks,
-Len