Bug 13139 - _PPC used for thermals - Thinkpad T60
Summary: _PPC used for thermals - Thinkpad T60
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Processor (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Len Brown
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-20 04:45 UTC by Len Brown
Modified: 2011-07-31 20:58 UTC (History)
1 user (show)

See Also:
Kernel Version: 2.6.29
Subsystem:
Regression: No
Bisected commit-id:


Attachments
acpidump (276.81 KB, application/octet-stream)
2009-04-20 04:45 UTC, Len Brown
Details
dynamic table CPU0IST.dat (576 bytes, application/octet-stream)
2009-04-20 05:03 UTC, Len Brown
Details
dynamic table CPU1IST.dat (200 bytes, application/octet-stream)
2009-04-20 05:04 UTC, Len Brown
Details
dynamic table CPU0CST.dat (1.59 KB, application/octet-stream)
2009-04-20 05:05 UTC, Len Brown
Details
dynamic table CPU1CST.dat (133 bytes, application/octet-stream)
2009-04-20 05:06 UTC, Len Brown
Details

Description Len Brown 2009-04-20 04:45:19 UTC
Created attachment 21049 [details]
acpidump

Filing this bug to document the trail of the T60's use of P-states
for thermal management.
Comment 1 Len Brown 2009-04-20 05:03:02 UTC
Created attachment 21050 [details]
dynamic table CPU0IST.dat

SSDT4.dsl:

Or (And (PDC0, 0x7FFFFFFF), CAP0, PDC0)
If (And (CFGD, 0x01)) {
        If (LAnd (LAnd (And (CFGD, 0x01000000), LEqual (And (PDC0, 0x09), 0x09)), LNot (And (SDTL, 0x01)))) {
                Or (SDTL, 0x01, SDTL)
                OperationRegion (IST0, SystemMemory, DerefOf (Index (SSDT, 0x01)), DerefOf (Index (SSDT, 0x02)))
                Load (IST0, HI0)
        }
}

---
        Name (SSDT, Package (0x0C)
        {
            "CPU0IST ", 
            0x1F6F1D36, 
            0x00000240, 
---
/lab/bin/acpidump -a 0x1F6F1D36 -l 0x00000240 > CPU0IST.dat

attached.




$ grep Load *.dsl
SSDT4.dsl:                    Load (IST0, HI0)
SSDT4.dsl:                    Load (CST0, HC0)
SSDT4.dsl:                    Load (IST1, HI1)
SSDT4.dsl:                    Load (CST1, HC1)
            Or (And (PDC0, 0x7FFFFFFF), CAP0, PDC0)
            If (And (CFGD, 0x01))
            {
                If (LAnd (LAnd (And (CFGD, 0x01000000), LEqual (And (PDC0,
                    0x09), 0x09)), LNot (And (SDTL, 0x01))))
                {
                    Or (SDTL, 0x01, SDTL)
                    OperationRegion (IST0, SystemMemory, DerefOf (Index (SSDT, 0x01)), DerefOf (Index (SSDT, 0x02
                        )))
                    Load (IST0, HI0)
                }
            }


SSDT4.dsl:
       Name (SSDT, Package (0x0C)
        {
            "CPU0IST ",
            0x1F6F1D36,
            0x00000240,
            "CPU1IST ",
            0x1F6F1C6E,
            0x000000C8,
            "CPU0CST ",
            0x1F6F1FFB,
            0x0000065A,
            "CPU1CST ",
            0x1F6F1F76,
            0x00000085

# /lab/bin/acpidump -a 0x1F6F1D36 -l 0x00000240 > CPU0IST.dat
# /lab/bin/acpidump -a 0x1F6F1C6E -l 0x000000C8 > CPU1IST.dat
# /lab/bin/acpidump -a 0x1F6F1FFB -l 0x0000065A > CPU0CST.dat
# /lab/bin/acpidump -a 0x1F6F1F76 -l 0x00000085 > CPU1CST.dat
Comment 2 Len Brown 2009-04-20 05:04:55 UTC
Created attachment 21051 [details]
dynamic table CPU1IST.dat

(sorry about the extra console log in the previous entry)
Comment 3 Len Brown 2009-04-20 05:05:44 UTC
Created attachment 21052 [details]
dynamic table CPU0CST.dat
Comment 4 Len Brown 2009-04-20 05:06:38 UTC
Created attachment 21053 [details]
dynamic table CPU1CST.dat
Comment 5 Len Brown 2009-04-20 16:45:55 UTC
the acpidump above is from BIOS Version 2.21 05 Feb 2008

The latest is v2.23 from 16 Sep 2008, but the changes don't
seem important:


<2.23>
 - (Fix) The computer may hang if Rescue and Recovery is installed on
         Windows Vista 64-bit SP1.
 - (Fix) PXE time-out is too long was fixed by IBA IBA v1.3.24.

<2.22>
 - (Fix) The boot sequence is different from the expected one in case of Wake
         on LAN(WOL).
 - (Fix) System hangs while formatting Cardbus ATA drive under Windows Vista 64bit SP1.
Comment 6 Len Brown 2009-04-20 16:58:06 UTC
The patch that started this conversation:

From e4233dec749a3519069d9390561b5636a75c7579
From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 26 Jan 2007 00:56:55 -0800
Subject: [PATCH] [PATCH] ACPI: fix cpufreq regression
Follows: v2.6.20-rc6
Precedes: v2.6.20-rc7

Recently cpufreq support on my laptop (Lenovo T60) broke completely: when
it's plugged into AC it would never go higher than 1 GHz - neither 1.3 GHz
nor 1.83 GHz is possible - no matter which governor (userspace, speed or
ondemand) is used.

After some cpufreq debugging i tracked the regression back to the following
(totally correct) bug-fix commit:

   commit 0916bd3ebb7cefdd0f432e8491abe24f4b5a101e
   Author: Dave Jones <davej@redhat.com>
   Date:   Wed Nov 22 20:42:01 2006 -0500

    [PATCH] Correct bound checking from the value returned from _PPC method.

This bugfix, which makes other laptops work, made a previously hidden
(BIOS) bug visible on my laptop.

The bug is the following: if the _PPC (Performance Present Capabilities)
optional ACPI object is queried /after/ bootup then the BIOS reports an
incorrect value of '2'.

My laptop (Lenovo T60) has the following performance states supported:

   0: 1833000
   1: 1333000
   2: 1000000

Per ACPI specification, a _PPC value of '0' means that all 3 performance
states are usable.  A _PPC value of '1' means states 1 ..  2 are usable, a
value of '2' means only state '2' (slowest) is usable.

now, the _PPC object is optional, and it also comes with notification.
Furthermore, when a CPU object is initialized, the _PPC object is
initialized as well.  So the following evaluation of the _PPC object is
superfluous:

 [<c028ba5f>] acpi_processor_get_platform_limit+0xa1/0xaf
 [<c028c040>] acpi_processor_register_performance+0x3b9/0x3ef
 [<c0111a85>] acpi_cpufreq_cpu_init+0xb7/0x596
 [<c03dab74>] cpufreq_add_dev+0x160/0x4a8
 [<c02bed90>] sysdev_driver_register+0x5a/0xa0
 [<c03d9c4c>] cpufreq_register_driver+0xb4/0x176
 [<c068ac08>] acpi_cpufreq_init+0xe5/0xeb
 [<c010056e>] init+0x14f/0x3dd

And this is the point where my laptop's BIOS returns the incorrect value of
'2'.  Note that it has not sent any notification event, so the value is
probably not really intentional (possibly spurious), and Windows likely
doesnt query it after bootup either.  Maybe the value is kept at '2'
normally, and is only set to the real value when a true asynchronous event
(such as AC plug event, battery switch, etc.) occurs.

So i /think/ this is a grey area of the ACPI spec: per the letter of the
spec the _PPC value only changes when notified, so there's no reason to
query it after the system has booted up.  So in my opinion the best (and
most compatible) strategy would be to do the change below, and to not
evaluate the _PPC object in the acpi_processor_get_performance_info() call,
but only evaluate it if _PPC is present during CPU object init, or if it's
notified during an asynchronous event.  This change is more permissive than
the previous logic, so it definitely shouldnt break any existing system.

This also happens to fix my laptop, which is merrily chugging along at
1.83 GHz now. Yay!

Signed-off-by: Ingo Molnar <mingo@elte.hu>
Cc: Dave Jones <davej@redhat.com>
Acked-by: Len Brown <lenb@kernel.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 drivers/acpi/processor_perflib.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index 5207f9e..cbb6f08 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -322,10 +322,6 @@ static int acpi_processor_get_performance_info(struct acpi_processor *pr)
        if (result)
                return result;

-       result = acpi_processor_get_platform_limit(pr);
-       if (result)
-               return result;
-
        return 0;
 }
Comment 7 Zhang Rui 2009-05-21 08:11:28 UTC
len,
what's the status of this bug?
do we have a patch/conclusion for this problem?
Comment 8 Zhang Rui 2009-06-18 06:16:42 UTC
ping len~~
Comment 9 Zhang Rui 2009-08-12 06:19:57 UTC
the problem is probably fixed by Thomas' patch, no?
Comment 10 Len Brown 2011-07-31 20:58:02 UTC
I forget why I filed this bug report.

the _PPC on the T60 does query the EC to decide whether
to limit P-states via PPC -- but what logic resides
inside the EC is anybody's guess:

        Method (_PPC, 0, NotSerialized)
        {
            Store (0x00, Local0)
            Store (\_SB.PCI0.LPC.EC.LPMD, Local0)
            If (LNot (Local0))
            {
                If (LOr (\DT00, \_SB.PCI0.LPC.EC.HT10))
                {
                    Store (\LWST, Local0)
                }
            }

            Return (Local0)
        }

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