2012-05-24 20:40:44

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH] PCI / ACPI: Do not request control of ASPM if the BIOS has disabled it

From: Rafael J. Wysocki <[email protected]>

We want to report that the kernel supports ASPM to the BIOS even if
the BIOS signals us that it doesn't. So, we need the flags to include
(OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
before calling acpi_pci_osc_support(root, flags) in
acpi_pci_root_add() (unless there's a command line telling us that
ASPM is not to be supported).

However, knowing that the BIOS doesn't support ASPM, we shouldn't
request the control of it, so we should remove
(OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
from the flags before calling acpi_pci_osc_control_set() in those
cases. Failing to do so causes the evaluation of _OSC for the PCI
root bridge to return error codes on some systems where it should
work correctly.

Reported-and-tested-by: Wei Kong <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---

-stable material, I think.

Thanks,
Rafael

---
drivers/acpi/pci_root.c | 4 ++++
1 file changed, 4 insertions(+)

Index: linux/drivers/acpi/pci_root.c
===================================================================
--- linux.orig/drivers/acpi/pci_root.c
+++ linux/drivers/acpi/pci_root.c
@@ -574,6 +574,10 @@ static int __devinit acpi_pci_root_add(s
if (flags != base_flags)
acpi_pci_osc_support(root, flags);

+ if (!pcie_aspm_enabled())
+ flags &= ~(OSC_ACTIVE_STATE_PWR_SUPPORT |
+ OSC_CLOCK_PWR_CAPABILITY_SUPPORT);
+
if (!pcie_ports_disabled
&& (flags & ACPI_PCIE_REQ_SUPPORT) == ACPI_PCIE_REQ_SUPPORT) {
flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL


2012-05-24 20:45:45

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] PCI / ACPI: Do not request control of ASPM if the BIOS has disabled it

On Thu, May 24, 2012 at 10:45:45PM +0200, Rafael J. Wysocki wrote:

> However, knowing that the BIOS doesn't support ASPM, we shouldn't
> request the control of it, so we should remove
> (OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
> from the flags before calling acpi_pci_osc_control_set() in those
> cases. Failing to do so causes the evaluation of _OSC for the PCI
> root bridge to return error codes on some systems where it should
> work correctly.

Is there an example of an affected machine's DSDT?

--
Matthew Garrett | [email protected]

2012-05-25 12:15:05

by Jean DELVARE

[permalink] [raw]
Subject: Re: [PATCH] PCI / ACPI: Do not request control of ASPM if the BIOS has disabled it

Hi Matthew,

Le jeudi 24 mai 2012 à 21:45 +0100, Matthew Garrett a écrit :
> On Thu, May 24, 2012 at 10:45:45PM +0200, Rafael J. Wysocki wrote:
>
> > However, knowing that the BIOS doesn't support ASPM, we shouldn't
> > request the control of it, so we should remove
> > (OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
> > from the flags before calling acpi_pci_osc_control_set() in those
> > cases. Failing to do so causes the evaluation of _OSC for the PCI
> > root bridge to return error codes on some systems where it should
> > work correctly.
>
> Is there an example of an affected machine's DSDT?

Attached.

Jean


Attachments:
DSDT.dat (43.36 kB)

2012-08-15 19:47:47

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [PATCH] PCI / ACPI: Do not request control of ASPM if the BIOS has disabled it

On Thu, May 24, 2012 at 2:45 PM, Rafael J. Wysocki <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> We want to report that the kernel supports ASPM to the BIOS even if
> the BIOS signals us that it doesn't. So, we need the flags to include
> (OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
> before calling acpi_pci_osc_support(root, flags) in
> acpi_pci_root_add() (unless there's a command line telling us that
> ASPM is not to be supported).
>
> However, knowing that the BIOS doesn't support ASPM, we shouldn't
> request the control of it, so we should remove
> (OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
> from the flags before calling acpi_pci_osc_control_set() in those
> cases. Failing to do so causes the evaluation of _OSC for the PCI
> root bridge to return error codes on some systems where it should
> work correctly.
>
> Reported-and-tested-by: Wei Kong <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> -stable material, I think.

Len, since you've applied recent patches in this area, can you take
care of this, too?

> ---
> drivers/acpi/pci_root.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> Index: linux/drivers/acpi/pci_root.c
> ===================================================================
> --- linux.orig/drivers/acpi/pci_root.c
> +++ linux/drivers/acpi/pci_root.c
> @@ -574,6 +574,10 @@ static int __devinit acpi_pci_root_add(s
> if (flags != base_flags)
> acpi_pci_osc_support(root, flags);
>
> + if (!pcie_aspm_enabled())
> + flags &= ~(OSC_ACTIVE_STATE_PWR_SUPPORT |
> + OSC_CLOCK_PWR_CAPABILITY_SUPPORT);
> +
> if (!pcie_ports_disabled
> && (flags & ACPI_PCIE_REQ_SUPPORT) == ACPI_PCIE_REQ_SUPPORT) {
> flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL

2012-08-15 19:57:56

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] PCI / ACPI: Do not request control of ASPM if the BIOS has disabled it

On Wednesday, August 15, 2012, Bjorn Helgaas wrote:
> On Thu, May 24, 2012 at 2:45 PM, Rafael J. Wysocki <[email protected]> wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > We want to report that the kernel supports ASPM to the BIOS even if
> > the BIOS signals us that it doesn't. So, we need the flags to include
> > (OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
> > before calling acpi_pci_osc_support(root, flags) in
> > acpi_pci_root_add() (unless there's a command line telling us that
> > ASPM is not to be supported).
> >
> > However, knowing that the BIOS doesn't support ASPM, we shouldn't
> > request the control of it, so we should remove
> > (OSC_ACTIVE_STATE_PWR_SUPPORT | OSC_CLOCK_PWR_CAPABILITY_SUPPORT)
> > from the flags before calling acpi_pci_osc_control_set() in those
> > cases. Failing to do so causes the evaluation of _OSC for the PCI
> > root bridge to return error codes on some systems where it should
> > work correctly.
> >
> > Reported-and-tested-by: Wei Kong <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> >
> > -stable material, I think.
>
> Len, since you've applied recent patches in this area, can you take
> care of this, too?

No, please don't.

I'd discussed that with Matthew and we concluded that it would break things
sometimes. Please disregard it (and sorry for not saying that earlier).

Thanks,
Rafael


> > ---
> > drivers/acpi/pci_root.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > Index: linux/drivers/acpi/pci_root.c
> > ===================================================================
> > --- linux.orig/drivers/acpi/pci_root.c
> > +++ linux/drivers/acpi/pci_root.c
> > @@ -574,6 +574,10 @@ static int __devinit acpi_pci_root_add(s
> > if (flags != base_flags)
> > acpi_pci_osc_support(root, flags);
> >
> > + if (!pcie_aspm_enabled())
> > + flags &= ~(OSC_ACTIVE_STATE_PWR_SUPPORT |
> > + OSC_CLOCK_PWR_CAPABILITY_SUPPORT);
> > +
> > if (!pcie_ports_disabled
> > && (flags & ACPI_PCIE_REQ_SUPPORT) == ACPI_PCIE_REQ_SUPPORT) {
> > flags = OSC_PCI_EXPRESS_CAP_STRUCTURE_CONTROL
>
>