| From 3c076351c4027a56d5005a39a0b518a4ba393ce2 Mon Sep 17 00:00:00 2001 |
| From: Matthew Garrett <mjg@redhat.com> |
| Date: Thu, 10 Nov 2011 16:38:33 -0500 |
| Subject: PCI: Rework ASPM disable code |
| |
| From: Matthew Garrett <mjg@redhat.com> |
| |
| commit 3c076351c4027a56d5005a39a0b518a4ba393ce2 upstream. |
| |
| Right now we forcibly clear ASPM state on all devices if the BIOS indicates |
| that the feature isn't supported. Based on the Microsoft presentation |
| "PCI Express In Depth for Windows Vista and Beyond", I'm starting to think |
| that this may be an error. The implication is that unless the platform |
| grants full control via _OSC, Windows will not touch any PCIe features - |
| including ASPM. In that case clearing ASPM state would be an error unless |
| the platform has granted us that control. |
| |
| This patch reworks the ASPM disabling code such that the actual clearing |
| of state is triggered by a successful handoff of PCIe control to the OS. |
| The general ASPM code undergoes some changes in order to ensure that the |
| ability to clear the bits isn't overridden by ASPM having already been |
| disabled. Further, this theoretically now allows for situations where |
| only a subset of PCIe roots hand over control, leaving the others in the |
| BIOS state. |
| |
| It's difficult to know for sure that this is the right thing to do - |
| there's zero public documentation on the interaction between all of these |
| components. But enough vendors enable ASPM on platforms and then set this |
| bit that it seems likely that they're expecting the OS to leave them alone. |
| |
| Measured to save around 5W on an idle Thinkpad X220. |
| |
| Signed-off-by: Matthew Garrett <mjg@redhat.com> |
| Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/acpi/pci_root.c | 7 +++++ |
| drivers/pci/pci-acpi.c | 1 |
| drivers/pci/pcie/aspm.c | 58 +++++++++++++++++++++++++++++------------------ |
| include/linux/pci-aspm.h | 4 +-- |
| 4 files changed, 46 insertions(+), 24 deletions(-) |
| |
| --- a/drivers/acpi/pci_root.c |
| +++ b/drivers/acpi/pci_root.c |
| @@ -595,6 +595,13 @@ static int __devinit acpi_pci_root_add(s |
| if (ACPI_SUCCESS(status)) { |
| dev_info(root->bus->bridge, |
| "ACPI _OSC control (0x%02x) granted\n", flags); |
| + 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_info(root->bus->bridge, |
| "ACPI _OSC request failed (%s), " |
| --- a/drivers/pci/pci-acpi.c |
| +++ b/drivers/pci/pci-acpi.c |
| @@ -393,7 +393,6 @@ static int __init acpi_pci_init(void) |
| |
| 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(); |
| } |
| |
| --- a/drivers/pci/pcie/aspm.c |
| +++ b/drivers/pci/pcie/aspm.c |
| @@ -68,7 +68,7 @@ struct pcie_link_state { |
| 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 @@ static int pcie_aspm_sanity_check(struct |
| 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 @@ void pcie_aspm_init_link_state(struct pc |
| 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 @@ void pcie_aspm_init_link_state(struct pc |
| * 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 @@ void pcie_aspm_exit_link_state(struct pc |
| struct pci_dev *parent = pdev->bus->self; |
| struct pcie_link_state *link, *root, *parent_link; |
| |
| - if ((aspm_disabled && !aspm_clear_state) || !pci_is_pcie(pdev) || |
| - !parent || !parent->link_state) |
| + if (!pci_is_pcie(pdev) || !parent || !parent->link_state) |
| return; |
| if ((parent->pcie_type != PCI_EXP_TYPE_ROOT_PORT) && |
| (parent->pcie_type != PCI_EXP_TYPE_DOWNSTREAM)) |
| @@ -734,13 +727,18 @@ void pcie_aspm_powersave_config_link(str |
| * 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 || !pci_is_pcie(pdev)) |
| + if (aspm_disabled && !force) |
| + return; |
| + |
| + if (!pci_is_pcie(pdev)) |
| return; |
| + |
| if (pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT || |
| pdev->pcie_type == PCI_EXP_TYPE_DOWNSTREAM) |
| parent = pdev; |
| @@ -768,16 +766,31 @@ static void __pci_disable_link_state(str |
| |
| 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; |
| @@ -935,6 +948,7 @@ void pcie_aspm_remove_sysfs_dev_files(st |
| 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"); |
| @@ -947,16 +961,18 @@ static int __init pcie_aspm_disable(char |
| |
| __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; |
| + } |
| } |
| |
| /** |
| --- a/include/linux/pci-aspm.h |
| +++ b/include/linux/pci-aspm.h |
| @@ -29,7 +29,7 @@ extern void pcie_aspm_pm_state_change(st |
| extern void pcie_aspm_powersave_config_link(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) |
| @@ -47,7 +47,7 @@ static inline void pcie_aspm_powersave_c |
| 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) |