288 lines
11 KiB
Diff
288 lines
11 KiB
Diff
Path: news.gmane.org!not-for-mail
|
|
From: Matthew Garrett <mjg@redhat.com>
|
|
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 <mjg@redhat.com>
|
|
To: jbarnes@virtuousgeek.org
|
|
Original-X-From: linux-acpi-owner@vger.kernel.org Thu Nov 10 22:38:57 2011
|
|
Return-path: <linux-acpi-owner@vger.kernel.org>
|
|
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 <linux-acpi-owner@vger.kernel.org>)
|
|
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 <rfc822;glad-acpi-devel@m.gmane.org>);
|
|
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 <rfc822;linux-acpi@vger.kernel.org>);
|
|
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 <mjg@redhat.com>)
|
|
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: <linux-acpi.vger.kernel.org>
|
|
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: <http://permalink.gmane.org/gmane.linux.acpi.devel/51182>
|
|
|
|
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>
|
|
---
|
|
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
|
|
|