On Tue, Jan 24, 2017 at 02:23:01PM -0600, Bjorn Helgaas wrote: > The ACPI FADT has a "PCIe ASPM Controls" bit (ACPI 5.0, sec 5.2.9.3), which > means "If set, indicates to OSPM that it must not enable ASPM control on > this platform." We have historically interpreted that to mean that the OS > should not touch ASPM configuration at all: we leave it as the firmware > configured it. > > However, a driver may know that its device has issues with ASPM and should > not have it enabled. The platform firmware, which cannot be expected to > know this, may have enabled ASPM. The driver should be able to use > pci_disable_link_state() to disable ASPM for its device, even if the > firmware set the ACPI_FADT_NO_ASPM bit. > > Remove the check that prevents pci_disable_link_state() from disabling ASPM > for a device. > > In cases where we previously emitted a message like this and did nothing: > > e1000e 0000:03:00.0: can't disable ASPM; OS doesn't have ASPM control > > we'll instead actually disable ASPM on the device. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=189951 > Reported-by: Mathias Koehrer <mathias.koehrer@xxxxxxxx> > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > CC: Matthew Garrett <mjg59@xxxxxxxxxx> > CC: stable@xxxxxxxxxxxxxxx I think this is the right thing to do, but I haven't applied this yet because I don't think it's quite enough in its current form. Even if I applied this, it would only allow a driver to disable ASPM if CONFIG_PCIEASPM is enabled. Without CONFIG_PCIEASPM, we don't compile aspm.c and pci_disable_link_state() is a stub that does nothing. I think we need to make pci_disable_link_state() work even when CONFIG_PCIEASPM is not enabled. If we did that, we could clean up callers like __e1000e_disable_aspm(), which contains code to disable ASPM manually if pci_disable_link_state() doesn't work. There are other drivers that try to disable ASPM because of device errata, and currently that only works when CONFIG_PCIEASPM is enabled. If we add a pci_disable_link_state() that works when CONFIG_PCIEASPM is not enabled, it should make those drivers work better. > --- > drivers/pci/pcie/aspm.c | 16 ++++------------ > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 17ac1dce3286..9253ae48d777 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -736,18 +736,10 @@ static void __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem) > if (!parent || !parent->link_state) > return; > > - /* > - * A driver requested that ASPM be disabled on this device, but > - * if we don't have permission to manage ASPM (e.g., on ACPI > - * systems we have to observe the FADT ACPI_FADT_NO_ASPM bit and > - * the _OSC method), we can't honor that request. Windows has > - * a similar mechanism using "PciASPMOptOut", which is also > - * ignored in this situation. > - */ > - if (aspm_disabled) { > - dev_warn(&pdev->dev, "can't disable ASPM; OS doesn't have ASPM control\n"); > - return; > - } > + dev_info(&pdev->dev, "disabling %sASPM%s%s\n", > + (state & PCIE_LINK_STATE_CLKPM) ? "Clock PM " : "", > + (state & PCIE_LINK_STATE_L0S) ? " L0s" : "", > + (state & PCIE_LINK_STATE_L1) ? " L1" : ""); > > if (sem) > down_read(&pci_bus_sem); >
