From 03f8e6abc95f335b1390d203ac769d6589eac61f Mon Sep 17 00:00:00 2001 From: Josh Boyer Date: Fri, 11 Nov 2011 14:36:05 -0500 Subject: [PATCH] Add patch to rework ASPM handling from Matthew Garrett --- kernel.spec | 9 +- pci-Rework-ASPM-disable-code.patch | 287 +++++++++++++++++++++++++++++ 2 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 pci-Rework-ASPM-disable-code.patch diff --git a/kernel.spec b/kernel.spec index baceb5d66..f93eaac55 100644 --- a/kernel.spec +++ b/kernel.spec @@ -54,7 +54,7 @@ Summary: The Linux kernel # For non-released -rc kernels, this will be appended after the rcX and # gitX tags, so a 3 here would become part of release "0.rcX.gitX.3" # -%global baserelease 1 +%global baserelease 2 %global fedora_build %{baserelease} # base_sublevel is the kernel version we're starting with and patching @@ -706,6 +706,8 @@ Patch21080: sysfs-msi-irq-per-device.patch Patch21090: bcma-brcmsmac-compat.patch +Patch21091: pci-Rework-ASPM-disable-code.patch + %endif BuildRoot: %{_tmppath}/kernel-%{KVERREL}-root @@ -1345,6 +1347,8 @@ ApplyPatch sysfs-msi-irq-per-device.patch # Remove overlap between bcma/b43 and brcmsmac and reenable bcm4331 ApplyPatch bcma-brcmsmac-compat.patch +ApplyPatch pci-Rework-ASPM-disable-code.patch + # END OF PATCH APPLICATIONS %endif @@ -2051,6 +2055,9 @@ fi # ||----w | # || || %changelog +* Fri Nov 11 2011 Josh Boyer +- Add reworked pci ASPM patch from Matthew Garrett + * Fri Nov 11 2011 John W. Linville - Remove overlap between bcma/b43 and brcmsmac and reenable bcm4331 diff --git a/pci-Rework-ASPM-disable-code.patch b/pci-Rework-ASPM-disable-code.patch new file mode 100644 index 000000000..d6fb24320 --- /dev/null +++ b/pci-Rework-ASPM-disable-code.patch @@ -0,0 +1,287 @@ +Path: news.gmane.org!not-for-mail +From: Matthew Garrett +Newsgroups: gmane.linux.acpi.devel,gmane.linux.kernel.pci,gmane.linux.kernel +Subject: [PATCH] pci: Rework ASPM disable code +Date: Thu, 10 Nov 2011 16:38:33 -0500 +Lines: 232 +Approved: news@gmane.org +Message-ID: <1320961113-5050-1-git-send-email-mjg@redhat.com> +NNTP-Posting-Host: lo.gmane.org +X-Trace: dough.gmane.org 1320961145 13112 80.91.229.12 (10 Nov 2011 21:39:05 GMT) +X-Complaints-To: usenet@dough.gmane.org +NNTP-Posting-Date: Thu, 10 Nov 2011 21:39:05 +0000 (UTC) +Cc: linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org, + linux-kernel@vger.kernel.org, Matthew Garrett +To: jbarnes@virtuousgeek.org +Original-X-From: linux-acpi-owner@vger.kernel.org Thu Nov 10 22:38:57 2011 +Return-path: +Envelope-to: glad-acpi-devel@lo.gmane.org +Original-Received: from vger.kernel.org ([209.132.180.67]) + by lo.gmane.org with esmtp (Exim 4.69) + (envelope-from ) + id 1ROcKm-0003jN-CL + for glad-acpi-devel@lo.gmane.org; Thu, 10 Nov 2011 22:38:56 +0100 +Original-Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand + id S1751342Ab1KJViu (ORCPT ); + Thu, 10 Nov 2011 16:38:50 -0500 +Original-Received: from mx1.redhat.com ([209.132.183.28]:32030 "EHLO mx1.redhat.com" + rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP + id S1750805Ab1KJVit (ORCPT ); + Thu, 10 Nov 2011 16:38:49 -0500 +Original-Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) + by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pAALcmdw013333 + (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); + Thu, 10 Nov 2011 16:38:49 -0500 +Original-Received: from cavan.codon.org.uk (ovpn-113-157.phx2.redhat.com [10.3.113.157]) + by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id pAALclkW022022 + (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); + Thu, 10 Nov 2011 16:38:48 -0500 +Original-Received: from 209-6-41-104.c3-0.smr-ubr1.sbo-smr.ma.cable.rcn.com ([209.6.41.104] helo=localhost.localdomain) + by cavan.codon.org.uk with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) + (Exim 4.72) + (envelope-from ) + id 1ROcKa-0000F4-E4; Thu, 10 Nov 2011 21:38:44 +0000 +X-SA-Do-Not-Run: Yes +X-SA-Exim-Connect-IP: 209.6.41.104 +X-SA-Exim-Mail-From: mjg@redhat.com +X-SA-Exim-Scanned: No (on cavan.codon.org.uk); SAEximRunCond expanded to false +X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 +Original-Sender: linux-acpi-owner@vger.kernel.org +Precedence: bulk +List-ID: +X-Mailing-List: linux-acpi@vger.kernel.org +Xref: news.gmane.org gmane.linux.acpi.devel:51182 gmane.linux.kernel.pci:12503 gmane.linux.kernel:1214427 +Archived-At: + +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 +--- + 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(-) + +diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c +index 2672c79..7aff631 100644 +--- a/drivers/acpi/pci_root.c ++++ b/drivers/acpi/pci_root.c +@@ -596,6 +596,13 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device) + 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), " +diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c +index 4ecb640..c8e7585 100644 +--- a/drivers/pci/pci-acpi.c ++++ b/drivers/pci/pci-acpi.c +@@ -395,7 +395,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(); + } + +diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c +index cbfbab1..1cfbf22 100644 +--- 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 pci_dev *pdev) + 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 pci_dev *pdev) + 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 pci_dev *pdev) + * 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 pci_dev *pdev) + 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(struct pci_dev *pdev) + * 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(struct pci_dev *pdev, int state, bool sem) + + 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(struct pci_dev *pdev) + 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 *str) + + __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 --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h +index 7cea7b6..c832014 100644 +--- a/include/linux/pci-aspm.h ++++ b/include/linux/pci-aspm.h +@@ -29,7 +29,7 @@ extern void pcie_aspm_pm_state_change(struct pci_dev *pdev); + 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_config_link(struct pci_dev *pdev) + 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) +-- +1.7.7.1 + +-- +To unsubscribe from this list: send the line "unsubscribe linux-acpi" in +the body of a message to majordomo@vger.kernel.org +More majordomo info at http://vger.kernel.org/majordomo-info.html +