After going from CentOS 5.7 to 6.2, a lot of things turned out to be much better, but there are also quite some regressions. The most obvious one is power consumption on my notebook. It was notably lower before.

The ASPM issue introduced in 2.6.38 was widely reported and discussed, and the 6.2 kernel has exacatly this code as a backport.

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=2f671e2dbff6eb5ef4e2600adbec550c13b8fe72

So I started to experiment with the upstream patch:

http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=commitdiff;h=3c076351c4027a56d5005a39a0b518a4ba393ce2;hp=69166fbf02c7a21745013f2de037bf7af26e4279

To make it apply, one needs to change 'pci_is_pcie(pdev)' into 'pdev->is_pcie'. One also needs to fiddle a little with the first chunk.

I came up with the patch attached, but unfortunately the new kernel showed no improvement. Most probably I got something wrong.

Anyone else here who tried this or is interested in sorting this out?

Thanks,
  Michael


diff -ur a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
--- a/drivers/acpi/pci_root.c   2012-01-19 19:45:17.000000000 +0100
+++ b/drivers/acpi/pci_root.c   2012-02-12 03:20:19.104634583 +0100
@@ -33,6 +33,7 @@
 #include <linux/pci.h>
 #include <linux/pci-acpi.h>
 #include <linux/acpi.h>
+#include <linux/pci-aspm.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/apei.h>
@@ -583,12 +584,21 @@
 
                status = acpi_pci_osc_control_set(device->handle, &flags,
                                        OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL);
-               if (ACPI_SUCCESS(status))
+               if (ACPI_SUCCESS(status)) {
                        dev_info(root->bus->bridge,
                                "ACPI _OSC control (0x%02x) granted\n", flags);
-               else
+                       if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
+                               /*
+                                * We have ASPM control, but the FADT indicates
+                                * that it's unsupported. Clear it.
+                                */
+                               pcie_clear_aspm(root->bus);
+                       }
+               } else {
                        dev_dbg(root->bus->bridge,
                                "ACPI _OSC request failed (code %d)\n", status);
+                       pcie_no_aspm();
+               }
        }
 
        return 0;
diff -ur a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
--- a/drivers/pci/pci-acpi.c    2012-01-19 19:44:15.000000000 +0100
+++ b/drivers/pci/pci-acpi.c    2012-02-12 01:39:31.441635301 +0100
@@ -195,7 +195,6 @@
 
        if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
                printk(KERN_INFO"ACPI FADT declares the system doesn't support 
PCIe ASPM, so disable it\n");
-               pcie_clear_aspm();
                pcie_no_aspm();
        }
 
diff -ur a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
--- a/drivers/pci/pcie/aspm.c   2012-01-19 19:45:17.000000000 +0100
+++ b/drivers/pci/pcie/aspm.c   2012-02-12 01:39:31.449635162 +0100
@@ -68,7 +68,7 @@
        struct aspm_latency acceptable[8];
 };
 
-static int aspm_disabled, aspm_force, aspm_clear_state;
+static int aspm_disabled, aspm_force;
 static bool aspm_support_enabled = true;
 static DEFINE_MUTEX(aspm_lock);
 static LIST_HEAD(link_list);
@@ -500,9 +500,6 @@
        int pos;
        u32 reg32;
 
-       if (aspm_clear_state)
-               return -EINVAL;
-
        /*
         * Some functions in a slot might not all be PCIe functions,
         * very strange. Disable ASPM for the whole slot
@@ -574,9 +571,6 @@
            pdev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM)
                return;
 
-       if (aspm_disabled && !aspm_clear_state)
-               return;
-
        /* VIA has a strange chipset, root port is under a bridge */
        if (pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT &&
            pdev->bus->self)
@@ -608,7 +602,7 @@
         * the BIOS's expectation, we'll do so once pci_enable_device() is
         * called.
         */
-       if (aspm_policy != POLICY_POWERSAVE || aspm_clear_state) {
+       if (aspm_policy != POLICY_POWERSAVE) {
                pcie_config_aspm_path(link);
                pcie_set_clkpm(link, policy_to_clkpm_state(link));
        }
@@ -649,8 +643,7 @@
        struct pci_dev *parent = pdev->bus->self;
        struct pcie_link_state *link, *root, *parent_link;
 
-       if ((aspm_disabled && !aspm_clear_state) || !pdev->is_pcie ||
-           !parent || !parent->link_state)
+       if (!pdev->is_pcie || !parent || !parent->link_state)
                return;
        if ((parent->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
            (parent->pcie_type != PCI_EXP_TYPE_DOWNSTREAM))
@@ -712,13 +705,18 @@
  * pci_disable_link_state - disable pci device's link state, so the link will
  * never enter specific states
  */
-static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
+static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem,
+                                    bool force)
 {
        struct pci_dev *parent = pdev->bus->self;
        struct pcie_link_state *link;
 
-       if (aspm_disabled || !pdev->is_pcie)
+       if (aspm_disabled && !force)
+               return;
+
+       if (!pdev->is_pcie)
                return;
+
        if (pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT ||
            pdev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM)
                parent = pdev;
@@ -746,16 +744,31 @@
 
 void pci_disable_link_state_locked(struct pci_dev *pdev, int state)
 {
-       __pci_disable_link_state(pdev, state, false);
+       __pci_disable_link_state(pdev, state, false, false);
 }
 EXPORT_SYMBOL(pci_disable_link_state_locked);
 
 void pci_disable_link_state(struct pci_dev *pdev, int state)
 {
-       __pci_disable_link_state(pdev, state, true);
+       __pci_disable_link_state(pdev, state, true, false);
 }
 EXPORT_SYMBOL(pci_disable_link_state);
 
+void pcie_clear_aspm(struct pci_bus *bus)
+{
+       struct pci_dev *child;
+
+       /*
+        * Clear any ASPM setup that the firmware has carried out on this bus
+        */
+       list_for_each_entry(child, &bus->devices, bus_list) {
+               __pci_disable_link_state(child, PCIE_LINK_STATE_L0S |
+                                        PCIE_LINK_STATE_L1 |
+                                        PCIE_LINK_STATE_CLKPM,
+                                        false, true);
+       }
+}
+
 static int pcie_aspm_set_policy(const char *val, struct kernel_param *kp)
 {
        int i;
@@ -907,6 +920,7 @@
 static int __init pcie_aspm_disable(char *str)
 {
        if (!strcmp(str, "off")) {
+               aspm_policy = POLICY_DEFAULT;
                aspm_disabled = 1;
                aspm_support_enabled = false;
                printk(KERN_INFO "PCIe ASPM is disabled\n");
@@ -919,16 +933,18 @@
 
 __setup("pcie_aspm=", pcie_aspm_disable);
 
-void pcie_clear_aspm(void)
-{
-       if (!aspm_force)
-               aspm_clear_state = 1;
-}
-
 void pcie_no_aspm(void)
 {
-       if (!aspm_force)
+       /*
+        * Disabling ASPM is intended to prevent the kernel from modifying
+        * existing hardware state, not to clear existing state. To that end:
+        * (a) set policy to POLICY_DEFAULT in order to avoid changing state
+        * (b) prevent userspace from changing policy
+        */
+       if (!aspm_force) {
+               aspm_policy = POLICY_DEFAULT;
                aspm_disabled = 1;
+       }
 }
 
 /**
diff -ur a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h
--- a/include/linux/pci-aspm.h  2012-01-19 19:44:42.000000000 +0100
+++ b/include/linux/pci-aspm.h  2012-02-12 01:39:31.452664277 +0100
@@ -28,7 +28,7 @@
 extern void pcie_aspm_pm_state_change(struct pci_dev *pdev);
 extern void pci_disable_link_state(struct pci_dev *pdev, int state);
 extern void pci_disable_link_state_locked(struct pci_dev *pdev, int state);
-extern void pcie_clear_aspm(void);
+extern void pcie_clear_aspm(struct pci_bus *bus);
 extern void pcie_no_aspm(void);
 #else
 static inline void pcie_aspm_init_link_state(struct pci_dev *pdev)
@@ -43,7 +43,7 @@
 static inline void pci_disable_link_state(struct pci_dev *pdev, int state)
 {
 }
-static inline void pcie_clear_aspm(void)
+static inline void pcie_clear_aspm(struct pci_bus *bus)
 {
 }
 static inline void pcie_no_aspm(void)
_______________________________________________
CentOS mailing list
CentOS@centos.org
http://lists.centos.org/mailman/listinfo/centos

Reply via email to