We currently refuse to touch the ASPM registers if the BIOS tells us that
ASPM isn't supported. This can cause problems if the BIOS has (for any
reason) enabled ASPM on some devices anyway. Change the code such that we
explicitly clear ASPM if the FADT indicates that ASPM isn't supported,
and make sure we tidy up appropriately on device removal in order to deal
with the hotplug case. If ASPM is disabled because the BIOS doesn't hand
over control then we won't touch the registers.
Signed-off-by: Matthew Garrett <[email protected]>
---
Implement Rafael's suggestion to use two separate functions, and also
ensure that we clear the clkpm bit as well as the ASPM bits.
drivers/pci/pci-acpi.c | 1 +
drivers/pci/pcie/aspm.c | 21 +++++++++++++++++----
include/linux/pci-aspm.h | 5 ++++-
3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 24e19c5..d7ea699 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -399,6 +399,7 @@ 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 7122281..8112415 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;
+static int aspm_disabled, aspm_force, aspm_clear_state;
static DEFINE_MUTEX(aspm_lock);
static LIST_HEAD(link_list);
@@ -139,7 +139,7 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
{
/* Don't enable Clock PM if the link is not Clock PM capable */
if (!link->clkpm_capable && enable)
- return;
+ enable = 0;
/* Need nothing if the specified equals to current state */
if (link->clkpm_enabled == enable)
return;
@@ -498,6 +498,10 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
struct pci_dev *child;
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
@@ -563,12 +567,15 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
struct pcie_link_state *link;
int blacklist = !!pcie_aspm_sanity_check(pdev);
- if (aspm_disabled || !pci_is_pcie(pdev) || pdev->link_state)
+ if (!pci_is_pcie(pdev) || pdev->link_state)
return;
if (pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT &&
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)
@@ -641,7 +648,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 || !pci_is_pcie(pdev) ||
+ if ((aspm_disabled && !aspm_clear_state) || !pci_is_pcie(pdev) ||
!parent || !parent->link_state)
return;
if ((parent->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
@@ -899,6 +906,12 @@ static int __init pcie_aspm_disable(char *str)
__setup("pcie_aspm=", pcie_aspm_disable);
+void pci_clear_aspm(void)
+{
+ if (!aspm_force)
+ aspm_clear_state = 1;
+}
+
void pcie_no_aspm(void)
{
if (!aspm_force)
diff --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h
index 91ba0b3..ce68105 100644
--- a/include/linux/pci-aspm.h
+++ b/include/linux/pci-aspm.h
@@ -27,6 +27,7 @@ extern void pcie_aspm_init_link_state(struct pci_dev *pdev);
extern void pcie_aspm_exit_link_state(struct pci_dev *pdev);
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 pcie_clear_aspm(void);
extern void pcie_no_aspm(void);
#else
static inline void pcie_aspm_init_link_state(struct pci_dev *pdev)
@@ -41,7 +42,9 @@ static inline void pcie_aspm_pm_state_change(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_no_aspm(void)
{
}
--
1.7.3.2
On Monday, December 06, 2010, Matthew Garrett wrote:
> We currently refuse to touch the ASPM registers if the BIOS tells us that
> ASPM isn't supported. This can cause problems if the BIOS has (for any
> reason) enabled ASPM on some devices anyway. Change the code such that we
> explicitly clear ASPM if the FADT indicates that ASPM isn't supported,
> and make sure we tidy up appropriately on device removal in order to deal
> with the hotplug case. If ASPM is disabled because the BIOS doesn't hand
> over control then we won't touch the registers.
>
> Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
>
> Implement Rafael's suggestion to use two separate functions, and also
> ensure that we clear the clkpm bit as well as the ASPM bits.
>
> drivers/pci/pci-acpi.c | 1 +
> drivers/pci/pcie/aspm.c | 21 +++++++++++++++++----
> include/linux/pci-aspm.h | 5 ++++-
> 3 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index 24e19c5..d7ea699 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -399,6 +399,7 @@ 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 7122281..8112415 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;
> +static int aspm_disabled, aspm_force, aspm_clear_state;
> static DEFINE_MUTEX(aspm_lock);
> static LIST_HEAD(link_list);
>
> @@ -139,7 +139,7 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
> {
> /* Don't enable Clock PM if the link is not Clock PM capable */
> if (!link->clkpm_capable && enable)
> - return;
> + enable = 0;
> /* Need nothing if the specified equals to current state */
> if (link->clkpm_enabled == enable)
> return;
> @@ -498,6 +498,10 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
> struct pci_dev *child;
> 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
> @@ -563,12 +567,15 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
> struct pcie_link_state *link;
> int blacklist = !!pcie_aspm_sanity_check(pdev);
>
> - if (aspm_disabled || !pci_is_pcie(pdev) || pdev->link_state)
> + if (!pci_is_pcie(pdev) || pdev->link_state)
> return;
> if (pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT &&
> 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)
> @@ -641,7 +648,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 || !pci_is_pcie(pdev) ||
> + if ((aspm_disabled && !aspm_clear_state) || !pci_is_pcie(pdev) ||
> !parent || !parent->link_state)
> return;
> if ((parent->pcie_type != PCI_EXP_TYPE_ROOT_PORT) &&
> @@ -899,6 +906,12 @@ static int __init pcie_aspm_disable(char *str)
>
> __setup("pcie_aspm=", pcie_aspm_disable);
>
> +void pci_clear_aspm(void)
> +{
> + if (!aspm_force)
> + aspm_clear_state = 1;
> +}
> +
> void pcie_no_aspm(void)
> {
> if (!aspm_force)
> diff --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h
> index 91ba0b3..ce68105 100644
> --- a/include/linux/pci-aspm.h
> +++ b/include/linux/pci-aspm.h
> @@ -27,6 +27,7 @@ extern void pcie_aspm_init_link_state(struct pci_dev *pdev);
> extern void pcie_aspm_exit_link_state(struct pci_dev *pdev);
> 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 pcie_clear_aspm(void);
> extern void pcie_no_aspm(void);
> #else
> static inline void pcie_aspm_init_link_state(struct pci_dev *pdev)
> @@ -41,7 +42,9 @@ static inline void pcie_aspm_pm_state_change(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_no_aspm(void)
> {
> }
>
On Mon, 6 Dec 2010 14:00:56 -0500
Matthew Garrett <[email protected]> wrote:
> We currently refuse to touch the ASPM registers if the BIOS tells us that
> ASPM isn't supported. This can cause problems if the BIOS has (for any
> reason) enabled ASPM on some devices anyway. Change the code such that we
> explicitly clear ASPM if the FADT indicates that ASPM isn't supported,
> and make sure we tidy up appropriately on device removal in order to deal
> with the hotplug case. If ASPM is disabled because the BIOS doesn't hand
> over control then we won't touch the registers.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> ---
>
Applied to linux-next, thanks.
--
Jesse Barnes, Intel Open Source Technology Center
On Fri, Dec 10, 2010 at 01:07:19PM -0800, Jesse Barnes wrote:
> On Mon, 6 Dec 2010 14:00:56 -0500
> Matthew Garrett <[email protected]> wrote:
>
> > We currently refuse to touch the ASPM registers if the BIOS tells us that
> > ASPM isn't supported. This can cause problems if the BIOS has (for any
> > reason) enabled ASPM on some devices anyway. Change the code such that we
> > explicitly clear ASPM if the FADT indicates that ASPM isn't supported,
> > and make sure we tidy up appropriately on device removal in order to deal
> > with the hotplug case. If ASPM is disabled because the BIOS doesn't hand
> > over control then we won't touch the registers.
> >
> > Signed-off-by: Matthew Garrett <[email protected]>
> > ---
> >
>
> Applied to linux-next, thanks.
Agh. It seems I sent the wrong version - there's a missing e in
pci_clear_aspm. Can you fix that up, or should I resend?
--
Matthew Garrett | [email protected]
On Fri, 10 Dec 2010 21:18:49 +0000
Matthew Garrett <[email protected]> wrote:
> On Fri, Dec 10, 2010 at 01:07:19PM -0800, Jesse Barnes wrote:
> > On Mon, 6 Dec 2010 14:00:56 -0500
> > Matthew Garrett <[email protected]> wrote:
> >
> > > We currently refuse to touch the ASPM registers if the BIOS tells us that
> > > ASPM isn't supported. This can cause problems if the BIOS has (for any
> > > reason) enabled ASPM on some devices anyway. Change the code such that we
> > > explicitly clear ASPM if the FADT indicates that ASPM isn't supported,
> > > and make sure we tidy up appropriately on device removal in order to deal
> > > with the hotplug case. If ASPM is disabled because the BIOS doesn't hand
> > > over control then we won't touch the registers.
> > >
> > > Signed-off-by: Matthew Garrett <[email protected]>
> > > ---
> > >
> >
> > Applied to linux-next, thanks.
>
> Agh. It seems I sent the wrong version - there's a missing e in
> pci_clear_aspm. Can you fix that up, or should I resend?
I'll fix it up (was just doing the build test now), thanks.
--
Jesse Barnes, Intel Open Source Technology Center