Bug 1958 - Feature request. Implement _CST support.
Summary: Feature request. Implement _CST support.
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Processor (show other bugs)
Hardware: i386 Linux
: P2 normal
Assignee: Robert Moore
URL:
Keywords:
: 2974 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-01-27 02:38 UTC by Bruno Ducrot
Modified: 2005-01-21 21:39 UTC (History)
7 users (show)

See Also:
Kernel Version: 2.6.1
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
Add support for _CST method in processor object. (16.95 KB, patch)
2004-01-27 02:42 UTC, Bruno Ducrot
Details | Diff
Rediffed against current bk tree. (18.18 KB, patch)
2004-03-03 02:50 UTC, Bruno Ducrot
Details | Diff
C1 should now be displayed correctly. (17.80 KB, patch)
2004-03-05 08:33 UTC, Bruno Ducrot
Details | Diff
_CST support. The handler work now. (23.13 KB, patch)
2004-03-22 01:16 UTC, Bruno Ducrot
Details | Diff
(1/2): make C-States dynamic [linked list] (23.48 KB, patch)
2004-03-22 06:29 UTC, Dominik Brodowski
Details | Diff
(2/2): _CST support (3.99 KB, patch)
2004-03-22 06:32 UTC, Dominik Brodowski
Details | Diff
'acpi_ksysms.c', make Bruno's patch working (448 bytes, patch)
2004-03-23 07:17 UTC, Luca Capello
Details | Diff
rediff of the original (non-dynamic) patch against 2.6.7 (23.48 KB, patch)
2004-07-06 13:57 UTC, Herman Sheremetyev
Details | Diff
rediff of the static patch against 2.6.9-rc2 (23.11 KB, patch)
2004-09-27 04:51 UTC, Luca Capello
Details | Diff
processor.c splitup and _CST handling on top of 2.6.10-rc2-mm3 (192 bytes, patch)
2004-11-27 14:43 UTC, Dominik Brodowski
Details | Diff

Description Bruno Ducrot 2004-01-27 02:38:39 UTC
It may be nice to add _CST support.  This will allow to get
more power saving in the idle loop for processors that are able to
do deeper sleep state, like P3m (tualatin), P4m, and Banias, to name a few.
Comment 1 Bruno Ducrot 2004-01-27 02:42:47 UTC
Created attachment 1955 [details]
Add support for _CST method in processor object.

This patch add the requested feature.  Please rewiew, and consider for
inclusion.
Comment 2 Nate Lawson 2004-02-12 10:07:12 UTC
Way too much duplication (i.e. the errata checks).  In FreeBSD, we have share a
lot of this code.
Comment 3 Bruno Ducrot 2004-02-12 20:30:41 UTC
Yes, there is a need to factorize that.  ALso the event handler really need to
be fixed or else there may be bug report that C4 is not present if booted on AC
for example.
Comment 4 Bruno Ducrot 2004-03-03 02:50:13 UTC
Created attachment 2276 [details]
Rediffed against current bk tree.

Rediffed against current bk tree, also included the missing processor.h.
Comment 5 Bruno Ducrot 2004-03-05 08:33:20 UTC
Created attachment 2290 [details]
C1 should now be displayed correctly.
Comment 6 Bruno Ducrot 2004-03-22 01:16:19 UTC
Created attachment 2385 [details]
_CST support.  The handler work now.

New version.  Support notify now, and correct a bug for which the configuration
from the FADT was broken.
Comment 7 Dominik Brodowski 2004-03-22 06:29:36 UTC
Created attachment 2386 [details]
(1/2): make C-States dynamic [linked list]

This first of two patches makes the number of C-States dynamic. It means less
memory usage compared to Bruno's patch [on which these patches are based, BTW],
especially if only C1/C2 is available.
Comment 8 Dominik Brodowski 2004-03-22 06:32:06 UTC
Created attachment 2387 [details]
(2/2): _CST support

This second of two patches implements the actual _CST support. Notification of
_CST changes is supported. Also, less code is duplicated.
Comment 9 Luca Capello 2004-03-23 07:12:26 UTC
(1/4): first post to show the situation :-)

On my ASUS M6842NWH, I've
- 2.6.5-rc1 (with no other ACPI patches)
- ACPI DSDT in initrd patch v0.4-2.6.4
     http://gaugusch.at/kernel.shtml
- DSDT hacked for battery/AC from
     http://www.isis.de/members/~messersch/asus-m6800n.html
- ECIadsl patch (for USB GlobeSpan ADSL modem)
     http://eciadsl.flashtux.org/

This is the "normality":
===== dmesg =====
Asus Laptop ACPI Extras version 0.27
  M6N model detected, unsupported, trying default values, supply the developers
with your DSDT
ACPI: AC Adapter [AC] (on-line)
acpi_processor-2444 [32] acpi_processor_get_inf: Invalid PBLK length [7]
ACPI: Processor [CPU1] (supports C1)
ACPI: Battery Slot [BAT0] (battery present)
ACPI: Battery Slot [BAT1] (battery absent)
ACPI: Power Button (FF) [PWRF]
ACPI: Lid Switch [LID]
ACPI: Sleep Button (CM) [SLPB]
ACPI: Thermal Zone [THRM] (52 C)
=====
===== cat /proc/acpi/processor/CPU1/* =====
processor id:            0
acpi id:                 1
bus mastering control:   yes
power management:        no
throttling control:      no
limit interface:         no
<not supported>
active state:            C1
default state:           C1
bus master activity:     00000000
states:
   *C1:                  promotion[--] demotion[--] latency[000] usage[00000000]
    C2:                  <not supported>
    C3:                  <not supported>
<not supported>
=====
Comment 10 Luca Capello 2004-03-23 07:14:44 UTC
(2/4): Bruno's patch

> _CST support.  The handler work now.
> 
> New version.  Support notify now, and correct a bug for which the 
> configuration from the FADT was broken.
Bruno, it seems I still need the 'acpi_ksyms.c' patched you privately send me at
20040311 (I created an attachment for it), because with the new version I've:
===== dmesg =====
Asus Laptop ACPI Extras version 0.27
  M6N model detected, unsupported, trying default values, supply the developers
with your DSDT
ACPI: AC Adapter [AC] (on-line)
processor: Unknown symbol acpi_os_write_port
ACPI: Battery Slot [BAT0] (battery present)
ACPI: Battery Slot [BAT1] (battery absent)
ACPI: Power Button (FF) [PWRF]
ACPI: Lid Switch [LID]
ACPI: Sleep Button (CM) [SLPB]
processor: Unknown symbol acpi_os_write_port
thermal: Unknown symbol acpi_processor_set_thermal_limit
======

This is normal, I checked that your new patch is the same you privately sent me.
Comment 11 Luca Capello 2004-03-23 07:17:10 UTC
Created attachment 2389 [details]
'acpi_ksysms.c', make Bruno's patch working

With Bruno's patch and without this patch, I got an error:
     processor: Unknown symbol acpi_os_write_port
Comment 12 Luca Capello 2004-03-23 07:19:20 UTC
(3/4): Bruno's patch and 'acpi_ksyms.c' patch

All is ok, even if I've just C1 & C2, not C3 in AC neither in battery:
===== dmesg =====
Asus Laptop ACPI Extras version 0.27
  M6N model detected, unsupported, trying default values, supply the developers
with your DSDT
ACPI: AC Adapter [AC] (off-line)
acpi_processor-2539 [34] acpi_processor_get_inf: Invalid PBLK length [7]
ACPI: Processor [CPU1] (supports C1 C2)
ACPI: Battery Slot [BAT0] (battery present)
ACPI: Battery Slot [BAT1] (battery absent)
ACPI: Power Button (FF) [PWRF]
ACPI: Lid Switch [LID]
ACPI: Sleep Button (CM) [SLPB]
ACPI: Thermal Zone [THRM] (53 C)
=====
===== cat /proc/acpi/processor/CPU1/* =====
processor id:            0
acpi id:                 1
bus mastering control:   yes
power management:        yes
throttling control:      no
limit interface:         no
<not supported>
active state:            C2
default state:           C1
bus master activity:     00000000
states:
    C1:         type[1] promotion[C2] demotion[--] latency[000] usage[00000010]
   *C2:         type[2] promotion[--] demotion[C1] latency[001] usage[00043123]
<not supported>
=====
Comment 13 Luca Capello 2004-03-23 07:20:50 UTC
(4/4): Dominik's patches

> (1/2): make C-States dynamic [linked list]
> (2/2): _CST support
only C1 and oops, here the results:
===== dmesg =====
Asus Laptop ACPI Extras version 0.27
  M6N model detected, unsupported, trying default values, supply the developers
with your DSDT
ACPI: AC Adapter [AC] (on-line)
acpi_processor-2444 [32] acpi_processor_get_inf: Invalid PBLK length [7]
acpi_processor-0878 [34] acpi_processor_get_pow: Invalid _CST data -- too few
C-States
ACPI: Processor [CPU1] (supports C1)
ACPI: Battery Slot [BAT0] (battery present)
ACPI: Battery Slot [BAT1] (battery absent)
ACPI: Power Button (FF) [PWRF]
ACPI: Lid Switch [LID]
ACPI: Sleep Button (CM) [SLPB]
ACPI: Thermal Zone [THRM] (52 C)
=====
===== 'cat /proc/acpi/processor/CPU1/power' =====
Unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
e11bb212
*pde = 00000000
Oops: 0000 [#4]
PREEMPT
CPU:    0
EIP:    0060:[<e11bb212>]    Tainted: GF
EFLAGS: 00010002   (2.6.5-rc1)
EIP is at acpi_processor_power_seq_show+0x72/0x20a [processor]
eax: 00000000   ebx: 00000001   ecx: dff3f800   edx: de681380
esi: e11bc6db   edi: e11bc6db   ebp: d8d1df24   esp: d8d1def4
ds: 007b   es: 007b   ss: 0068
Process cat (pid: 1971, threadinfo=d8d1c000 task=de2722c0)
Stack: 00000001 00000000 00000003 00000286 de681380 01000000 e11bc6bd e11bc190
       00000296 00000001 de0c0180 00000001 d8d1df6c c0170b6f de0c0180 00000001
       00000000 00000000 406035d2 0a2ea968 406035d2 de0c0198 00000000 0a2ea968
Call Trace:
 [<c0170b6f>] seq_read+0x10f/0x2f0
 [<c01531ca>] vfs_read+0xaa/0x130
 [<c015347f>] sys_read+0x3f/0x60
 [<c010935b>] syscall_call+0x7/0xb

Code: 0f b6 00 50 68 e0 d1 1b e1 ff 75 08 e8 3d 5f fb de 83 c4 14
 <6>note: cat[1971] exited with preempt_count 1
=====
Comment 14 Bruno Ducrot 2004-03-23 08:10:04 UTC
the acpi_ksyms patch is still needed?  Well, I was thinking there were
something like that already in vanilla kernel?
Comment 15 Len Brown 2004-03-24 18:01:14 UTC
accept to get this out of "NEW" state.
Comment 16 Len Brown 2004-05-20 20:38:31 UTC
Starting to see systems with bogus C-state info in the FADT, 
which makes a _CST-based implementation more important. 
Comment 17 Luca Capello 2004-06-06 23:05:50 UTC
I still have only C1 on my ASUS M6N (even with the latest BIOS upgrade to 0208A)
and I was trying to apply Bruno's patch on a vanilla 2.6.6 to have at least C1 +
C2, but his patch fails:
=====
gismo:/usr/src# tar jxvf kernel/v2.6/linux-2.6.6.tar.bz2
gismo:/usr/src# cd linux-2.6.6
gismo:/usr/src/linux-2.6.6# patch -p1 < ../kernel/v2.6/bruno_20040322.diff
patching file drivers/acpi/processor.c
Hunk #1 succeeded at 296 (offset -2 lines).
Hunk #2 succeeded at 404 (offset -2 lines).
Hunk #3 succeeded at 421 (offset -2 lines).
Hunk #4 succeeded at 438 (offset -2 lines).
Hunk #5 succeeded at 514 (offset -2 lines).
Hunk #6 succeeded at 543 (offset -2 lines).
Hunk #7 succeeded at 837 (offset -2 lines).
Hunk #8 succeeded at 847 (offset -2 lines).
Hunk #9 succeeded at 935 (offset -2 lines).
Hunk #10 succeeded at 989 (offset -2 lines).
Hunk #11 succeeded at 1058 (offset -2 lines).
Hunk #12 succeeded at 2192 (offset -1 lines).
Hunk #13 succeeded at 2201 (offset -1 lines).
Hunk #14 FAILED at 2537.
Hunk #15 succeeded at 2610 (offset 19 lines).
Hunk #16 succeeded at 2656 (offset 19 lines).
Hunk #17 succeeded at 2677 (offset 19 lines).
Hunk #18 succeeded at 2711 (offset 19 lines).
Hunk #19 succeeded at 2718 (offset 19 lines).
1 out of 19 hunks FAILED -- saving rejects to file drivers/acpi/processor.c.rej
patching file include/acpi/processor.h
gismo:/usr/src/linux-2.6.6#
=====

And the 'acpi_ksysms.c' patch is still required:
=====
gismo:/usr/src# tar jxvf kernel/v2.6/linux-2.6.6.tar.bz2
gismo:/usr/src# cd linux-2.6.6
gismo:/usr/src/linux-2.6.6# patch -p1 < ../kernel/v2.6/acpi_ksyms.c_20040311.diff
patching file drivers/acpi/acpi_ksyms.c
gismo:/usr/src/linux-2.6.6#
=====
Comment 18 Herman Sheremetyev 2004-06-28 21:23:42 UTC
*** Bug 2974 has been marked as a duplicate of this bug. ***
Comment 19 Herman Sheremetyev 2004-07-06 13:57:21 UTC
Created attachment 3311 [details]
rediff of the original (non-dynamic) patch against 2.6.7

I fixed the original patch (not the dynamic linked-list one) to apply to 2.6.7.
 As Luca pointed out above the dynamic one doesn't work on the Asus M6N.  I'll
try to figure out why that is and fix that one up as well, in the meantime this
patch provides working C2 support for me.
Comment 20 Robert Moore 2004-09-14 14:08:18 UTC
I prefer the static number of C-states vs. dynamic.  Realistically, there are 
only a few actual states (1-2 to 5-6), and the OSPM/policy software will only 
take advantage of a small number of states, no matter how many are actually 
implemented.

Therefore, I feel that generalizing the number of C-states with a dynamic 
linked list is most probably overkill.

Bob
Comment 21 Dominik Brodowski 2004-09-14 14:59:58 UTC
Agreed. We need to find a good way of naming these states, though: if there is
one of _type_ C1 [there can only be one, AFAICS], two of _type_ C2, and one or
more of _type_ C3, we can't call them C1,C2,C3,C4,... then C3 would be of _type_
C2, which is confusing... Threrefore, I'd suggest dropping the 'C' in front of
the number... 
Comment 22 Luca Capello 2004-09-27 04:51:21 UTC
Created attachment 3727 [details]
rediff of the static patch against 2.6.9-rc2

I used Herman's rediff against 2.6.7 to make this patch against 2.6.9-rc2 and
the latest ACPI patch (20040816). The patch isn't completely clean in the sense
that sometimes there're empty lines, so if someone could check it this will be
a very good idea. BTW, the 'acpi_ksysms.c' patch is still neede.

Anyway, it works in my case and I've this output:
=====
root@gismo:~# uname -a
Linux gismo 2.6.9-rc2 #1 Mon Sep 20 00:37:15 CEST 2004 i686 GNU/Linux
root@gismo:~# cat /proc/acpi/info
version:		 20040816
root@gismo:~# cat /proc/acpi/processor/CPU1/
info	    limit	power	    throttling
root@gismo:~# cat /proc/acpi/processor/CPU1/*
processor id:		 0
acpi id:		 1
bus mastering control:	 yes
power management:	 yes
throttling control:	 no
limit interface:	 no
<not supported>
active state:		 C2
default state:		 C1
bus master activity:	 00000000
states:
    C1: 	type[1] promotion[C2] demotion[--] latency[000] usage[00000010]

   *C2: 	type[2] promotion[--] demotion[C1] latency[001] usage[31435084]

<not supported>
root@gismo:~#
=====
Comment 23 Dominik Brodowski 2004-11-27 14:43:56 UTC
Created attachment 4156 [details]
processor.c splitup and _CST handling on top of 2.6.10-rc2-mm3

Splitup: http://marc.theaimsgroup.com/?l=acpi4linux&m=110157873826289&w=2
_CST message should hit the mailing list archives soon.
Comment 24 Dominik Brodowski 2004-11-27 14:45:09 UTC
_CST thread:
http://sourceforge.net/mailarchive/forum.php?thread_id=6044594&forum_id=6102
Comment 25 Len Brown 2004-12-22 20:11:47 UTC
Dominik's updated patches are now included in the acpi patch
http://ftp.kernel.org/pub/linux/kernel/people/lenb/acpi/patches/release/2.6.10/
and here:
http://linux-acpi.bkbits.net:8080/26-latest-mm
Comment 26 Len Brown 2005-01-21 21:39:14 UTC
shipped in 2.6.11-rc1 -- thanks for the good work! 

Note You need to log in before you can comment on or make changes to this bug.