Bug 15886 - acpi_idle: Very idle Core i7 machine never enters C6
Summary: acpi_idle: Very idle Core i7 machine never enters C6
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Processor (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: acpi_power-processor
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-30 18:30 UTC by Philip Langdale
Modified: 2010-07-26 23:55 UTC (History)
4 users (show)

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


Attachments
ACPI dump from GA-P55M-UD4 (172.84 KB, application/octet-stream)
2010-04-30 18:30 UTC, Philip Langdale
Details
lspci from HP DL360G6 (70.82 KB, application/octet-stream)
2010-05-25 18:17 UTC, Matthew Garrett
Details
acpidump from HP dl360g6 (144.38 KB, application/octet-stream)
2010-05-25 18:20 UTC, Matthew Garrett
Details
patch vs 2.6.35-rc5 (2.90 KB, patch)
2010-07-21 18:13 UTC, Len Brown
Details | Diff
acpidump from HP DL380G6 (141.64 KB, application/octet-stream)
2010-07-22 20:42 UTC, Iain Paton
Details
patch vs 2.6.35-rc6 to disable BM_STS checking via ACPI _CST.FFH flag (2.78 KB, patch)
2010-07-22 21:19 UTC, Len Brown
Details | Diff
patch vs 2.6.35-rc6 to add "processor.bm_check_disable1=1" (1.97 KB, patch)
2010-07-22 21:34 UTC, Len Brown
Details | Diff
DL360G6 acpidump, powertop & turbostat logs (89.98 KB, application/gzip)
2010-07-23 12:37 UTC, Iain Paton
Details

Description Philip Langdale 2010-04-30 18:30:40 UTC
Created attachment 26188 [details]
ACPI dump from GA-P55M-UD4

As discussed on the mailing list. Jeff and I both have systems where an idle Core i7 spends almost no time in C6 (C3 as reported by powertop).

In my case, it's a gigabyte GA-P55M-UD4 and Jeff's is some Dell motherboard.

Matthew Garrett pointed out the following, seemingly ignored, patch as a logical solution to the problem and it does indeed appear to work.

http://lkml.org/lkml/2009/11/10/23

(Patchwork seems to have mangled the patch in question so I didn't link to it)
Comment 1 Zhang Rui 2010-05-10 07:08:55 UTC
Patch available.
ping Len...
Comment 2 Matthew Garrett 2010-05-25 18:17:11 UTC
Created attachment 26539 [details]
lspci from HP DL360G6
Comment 3 Matthew Garrett 2010-05-25 18:20:13 UTC
Created attachment 26540 [details]
acpidump from HP dl360g6
Comment 4 Matthew Garrett 2010-05-25 18:22:41 UTC
This machine sets the BM_STS flag several times a second. I've unloaded all drivers except for storage and see the same behaviour.
Comment 5 Len Brown 2010-07-21 18:13:25 UTC
Created attachment 27187 [details]
patch vs 2.6.35-rc5

Please give this patch a try.
You should notice that in /proc/acpi/processor/*/power
that the C3 type C-states are now C2-type.
As such, BM_STS will not be checked on entry
and deep c-states will be used even if the chipset reports BM_STS.
Comment 6 Len Brown 2010-07-22 05:15:30 UTC
According to "Intel Processor Vendor-Specific ACPI Interface Specification":
http://download.intel.com/technology/IAPC/acpi/downloads/30222305.pdf

table 4 "_CST FFH GAS Field Encoding"

Bit 1: Set to 1 if OSPM should use Bus Master avoidance for this C-state

This means that Linux should not be checking BM_STS on these states
unless this bit is set.  However, Linux is always checking BM_STS
for C3-type states.

The Good news is that according to Philip's acpidump for the
failing gigabyte motherboard in the original description above,
Gigabyte does not set this bit in _CST.GAS -- only bit 0
is set in access size, which indicates HW coordiation:


            If (CondRefOf (\_OSI, Local0))
            {
                If (LAnd (\_OSI ("Windows 2001.1"), LAnd (LNot (\_OSI ("Windows 2006")), LNot (\_OSI (
                    "Windows 2001.1 SP1")))))
                {
                    Store (0x00, CSEN)
                }
            }

            If (CSEN)
            {
                If (LAnd (MWOS, And (TYPE, 0x0200)))
                {
                    Return (Package (0x04)
                    {
                        0x03,
                        Package (0x04)
                        {
                            ResourceTemplate ()
                            {
                                Register (FFixedHW,
                                    0x01,               // Bit Width
                                    0x02,               // Bit Offset
                                    0x0000000000000000, // Address
                                    0x01,               // Access Size
                                    )
                            },

                            0x01,
                            0x0001,
                            0x000003E8
                        },

                        Package (0x04)
                        {
                            ResourceTemplate ()
                            {
                                Register (FFixedHW,
                                    0x01,               // Bit Width
                                    0x02,               // Bit Offset
                                    0x0000000000000010, // Address
                                    0x01,               // Access Size
                                    )
                            },

                            0x02,
                            0x0040,
                            0x000001F4
                        },

                        Package (0x04)
                        {
                            ResourceTemplate ()
                            {
                                Register (FFixedHW,
                                    0x01,               // Bit Width
                                    0x02,               // Bit Offset
                                    0x0000000000000020, // Address
                                    0x01,               // Access Size
                                    )
                            },

                            0x03,
                            0x0060,
                            0x0000015E
                        }
                    })

The Bad news is that according to the acpidump in comment #3,
HP is asking for BM_STS checking on the DL360G6
via access size 3 for C2 and C3:


       Method (_CST, 0, NotSerialized)
        {
            If (CSEN)
            {
                If (LAnd (MWOS, And (TYPE, 0x0200)))
                {
                    Return (Package (0x04)
                    {
                        0x03,
                        Package (0x04)
                        {
                            ResourceTemplate ()
                            {
                                Register (FFixedHW,
                                    0x01,               // Bit Width
                                    0x02,               // Bit Offset
                                    0x0000000000000000, // Address
                                    0x01,               // Access Size
                                    )
                            },

                            0x01,
                            0x0001,
                            0x000003E8
                        },

                        Package (0x04)
                        {
                            ResourceTemplate ()
                            {
                                Register (FFixedHW,
                                    0x01,               // Bit Width
                                    0x02,               // Bit Offset
                                    0x0000000000000010, // Address
                                    0x03,               // Access Size
                                    )
                            },

                            0x03,
                            0x0040,
                            0x000001F4
                        },

                        Package (0x04)
                        {
                            ResourceTemplate ()
                            {
                                Register (FFixedHW,
                                    0x01,               // Bit Width
                                    0x02,               // Bit Offset
                                    0x0000000000000020, // Address
                                    0x03,               // Access Size
                                    )
                            },

                            0x03,
                            0x0060,
                            0x0000015E
                        }
                    })
                }

So if we fix Linux to check this bit, the Gigabyte will be fixed,
but the HP appears to be explicitly asking us to check BM_STS.
Unclear if that is HP's intention or if it is a BIOS bug.
Comment 7 Philip Langdale 2010-07-22 05:32:51 UTC
Seems to be working fine on the Gigabyte board.
Comment 8 Venkatesh Pallipadi 2010-07-22 16:18:59 UTC
I never understood the meaning of this bit.

table 4 "_CST FFH GAS Field Encoding"
Bit 1: Set to 1 if OSPM should use Bus Master avoidance for this C-state

Does it mean OS should check BM_STS or does it mean OS OS should do ARB_DIS to avoid BM activity before entering this C-state?

I always felt that its the second one. But, never saw any BIOS having this bit set..
Comment 9 Iain Paton 2010-07-22 20:42:30 UTC
Created attachment 27211 [details]
acpidump from HP DL380G6
Comment 10 Len Brown 2010-07-22 21:08:29 UTC
The BWG says:

if a device can't tolerate the latency then...
"report this C-state with the BM_STS avoidance bit set".

I thought they were referring to enabling BM_STS in the chipset.
I didn't realize that they were referring to FFH.GAS.access_size.

So I think our policy should be for FADT and _CST IOspace C3,
to continue checking BM_STS.

For _CST FFH Intel C3, check BM_STS only if this bit is set.
Comment 11 Len Brown 2010-07-22 21:19:37 UTC
Created attachment 27212 [details]
patch vs 2.6.35-rc6 to disable BM_STS checking via ACPI _CST.FFH flag

Philip,
Please test this one.
It should solve the problem on your gigabyte.

The HP DL360G6 will be tougher, as the BIOS is asking us to check BM_STS...
Comment 12 Len Brown 2010-07-22 21:34:55 UTC
Created attachment 27213 [details]
patch vs 2.6.35-rc6 to add "processor.bm_check_disable1=1"

Matthew, Iain,

Please boot your HP DL3690 G6 with "processor.bm_check_disable1=1"
and verify with turbostat that you can get into deep c-states.
Comment 13 Philip Langdale 2010-07-23 05:29:40 UTC
Len,

Latest patch works great.

Thanks!
Comment 14 Iain Paton 2010-07-23 12:37:47 UTC
Created attachment 27216 [details]
DL360G6 acpidump, powertop & turbostat logs

bios date and power regulator settings on the end of the filenames
Comment 15 Len Brown 2010-07-25 03:17:18 UTC
Iain,
Turbostat output shows that BIOS 07-24-2010 does not enter c3, pc3, pc6
and BIOS 05-15-2010 enters all of them.

This is independent of the "balanced" vs "oscontrol" BIOS setting.

So it appears that while the older BIOS didn't work well,
the newer BIOS is working fine.

Looking at the AML, the newer BIOS has Access Size 1 on _CST
no matter if balanced or oscontrol.
The older BIOS has Access Size 3 on C3-type _CST's.
Again, independent of "balanced" vs "oscontrol".
Of course you are running a kernel that doesn't look at _CST.GAS.Access Width,
so apparently the new BIOS is programming the chip-set to not assert BM_STS...

In summary, you have no problem with the new BIOS
and the lack of package c-states appears to be a problem associated
only w/ the old BIOS, yes?
Comment 16 Len Brown 2010-07-26 23:55:10 UTC
patch in comment #11 shipped upstream after linux-2.6.35-rc6-git1

and has been sent to stable@kernel.org for 2.6.32.y, 2.6.33.y, 2.6.34.y

commit 718be4aaf3613cf7c2d097f925abc3d3553c0605
Author: Len Brown <len.brown@intel.com>
Date:   Thu Jul 22 16:54:27 2010 -0400

    ACPI: skip checking BM_STS if the BIOS doesn't ask for it

It appears that the problem with the HP DL380G6
occurs only with an old BIOS, and so that doesn't seem
to justify any modification to Linux.

closed.

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