Bug 1958
| Summary: | Feature request. Implement _CST support. | ||
|---|---|---|---|
| Product: | ACPI | Reporter: | Bruno Ducrot (ducrot) |
| Component: | Power-Processor | Assignee: | Robert Moore (Robert.Moore) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | acpi-bugzilla, herman, linux, luca, luming.yu, yi.zhu, ying |
| Priority: | P2 | ||
| Hardware: | i386 | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.1 | Subsystem: | |
| Regression: | --- | Bisected commit-id: | |
| Attachments: |
Add support for _CST method in processor object.
Rediffed against current bk tree. C1 should now be displayed correctly. _CST support. The handler work now. (1/2): make C-States dynamic [linked list] (2/2): _CST support 'acpi_ksysms.c', make Bruno's patch working rediff of the original (non-dynamic) patch against 2.6.7 rediff of the static patch against 2.6.9-rc2 processor.c splitup and _CST handling on top of 2.6.10-rc2-mm3 |
||
|
Description
Bruno Ducrot
2004-01-27 02:38:39 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.
Way too much duplication (i.e. the errata checks). In FreeBSD, we have share a lot of this code. 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. Created attachment 2276 [details]
Rediffed against current bk tree.
Rediffed against current bk tree, also included the missing processor.h.
Created attachment 2290 [details]
C1 should now be displayed correctly.
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.
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.
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.
(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>
=====
(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.
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
(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>
=====
(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
=====
the acpi_ksyms patch is still needed? Well, I was thinking there were something like that already in vanilla kernel? accept to get this out of "NEW" state. Starting to see systems with bogus C-state info in the FADT, which makes a _CST-based implementation more important. 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# ===== *** Bug 2974 has been marked as a duplicate of this bug. *** 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.
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 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... 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:~#
=====
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. 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 shipped in 2.6.11-rc1 -- thanks for the good work! |