| Message ID | 20240530085227.91168-2-kai.heng.feng@canonical.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [1/2] PCI: ASPM: Allow OS to configure ASPM where BIOS is incapable of | expand |
On Thu, May 30, 2024 at 04:52:27PM +0800, Kai-Heng Feng wrote: > Intel SoC cannot reach lower power states when mapped VMD PCIe bridges > and NVMe devices don't have ASPM configured. > > So set aspm_os_control attribute to let OS really enable ASPM for those > devices. > > Fixes: f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and LTR") I assume f492edb40b54 was tested and worked at the time. Is the implication that newer Intel SoCs have added more requirements for getting to low power states, since __pci_enable_link_state() would have warned and done nothing even then? Or maybe this is a new system that sets ACPI_FADT_NO_ASPM, and f492edb40b54 was tested on systems that did *not* set ACPI_FADT_NO_ASPM? > Link: https://lore.kernel.org/linux-pm/218aa81f-9c6-5929-578d-8dc15f83dd48@panix.com/ > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/pci/controller/vmd.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > index 87b7856f375a..1dbc525c473f 100644 > --- a/drivers/pci/controller/vmd.c > +++ b/drivers/pci/controller/vmd.c > @@ -751,6 +751,8 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) > if (!(features & VMD_FEAT_BIOS_PM_QUIRK)) > return 0; > > + pdev->aspm_os_control = 1; > + > pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL); > > pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR); > -- > 2.43.0 >
On Fri, Aug 2, 2024 at 8:04 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Thu, May 30, 2024 at 04:52:27PM +0800, Kai-Heng Feng wrote: > > Intel SoC cannot reach lower power states when mapped VMD PCIe bridges > > and NVMe devices don't have ASPM configured. > > > > So set aspm_os_control attribute to let OS really enable ASPM for those > > devices. > > > > Fixes: f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and LTR") > > I assume f492edb40b54 was tested and worked at the time. Is the > implication that newer Intel SoCs have added more requirements for > getting to low power states, since __pci_enable_link_state() would > have warned and done nothing even then? > > Or maybe this is a new system that sets ACPI_FADT_NO_ASPM, and > f492edb40b54 was tested on systems that did *not* set > ACPI_FADT_NO_ASPM? I believe it's the case here. Vendors may or may not set ACPI_FADT_NO_ASPM, so f492edb40b54 works on some systems but not others. Kai-Heng > > > Link: https://lore.kernel.org/linux-pm/218aa81f-9c6-5929-578d-8dc15f83dd48@panix.com/ > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > --- > > drivers/pci/controller/vmd.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > > index 87b7856f375a..1dbc525c473f 100644 > > --- a/drivers/pci/controller/vmd.c > > +++ b/drivers/pci/controller/vmd.c > > @@ -751,6 +751,8 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) > > if (!(features & VMD_FEAT_BIOS_PM_QUIRK)) > > return 0; > > > > + pdev->aspm_os_control = 1; > > + > > pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL); > > > > pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR); > > -- > > 2.43.0 > >
Hi Bjorn, On Fri, Aug 2, 2024 at 9:04 AM Kai-Heng Feng <kai.heng.feng@canonical.com> wrote: > > On Fri, Aug 2, 2024 at 8:04 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > On Thu, May 30, 2024 at 04:52:27PM +0800, Kai-Heng Feng wrote: > > > Intel SoC cannot reach lower power states when mapped VMD PCIe bridges > > > and NVMe devices don't have ASPM configured. > > > > > > So set aspm_os_control attribute to let OS really enable ASPM for those > > > devices. > > > > > > Fixes: f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and LTR") > > > > I assume f492edb40b54 was tested and worked at the time. Is the > > implication that newer Intel SoCs have added more requirements for > > getting to low power states, since __pci_enable_link_state() would > > have warned and done nothing even then? > > > > Or maybe this is a new system that sets ACPI_FADT_NO_ASPM, and > > f492edb40b54 was tested on systems that did *not* set > > ACPI_FADT_NO_ASPM? > > I believe it's the case here. Vendors may or may not set > ACPI_FADT_NO_ASPM, so f492edb40b54 works on some systems but not > others. Do you think this is code is ready? Or is there any improvement should be made? Kai-Heng > > Kai-Heng > > > > > > Link: https://lore.kernel.org/linux-pm/218aa81f-9c6-5929-578d-8dc15f83dd48@panix.com/ > > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > > --- > > > drivers/pci/controller/vmd.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c > > > index 87b7856f375a..1dbc525c473f 100644 > > > --- a/drivers/pci/controller/vmd.c > > > +++ b/drivers/pci/controller/vmd.c > > > @@ -751,6 +751,8 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) > > > if (!(features & VMD_FEAT_BIOS_PM_QUIRK)) > > > return 0; > > > > > > + pdev->aspm_os_control = 1; > > > + > > > pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL); > > > > > > pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR); > > > -- > > > 2.43.0 > > >
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 87b7856f375a..1dbc525c473f 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -751,6 +751,8 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata) if (!(features & VMD_FEAT_BIOS_PM_QUIRK)) return 0; + pdev->aspm_os_control = 1; + pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL); pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
Intel SoC cannot reach lower power states when mapped VMD PCIe bridges and NVMe devices don't have ASPM configured. So set aspm_os_control attribute to let OS really enable ASPM for those devices. Fixes: f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and LTR") Link: https://lore.kernel.org/linux-pm/218aa81f-9c6-5929-578d-8dc15f83dd48@panix.com/ Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/pci/controller/vmd.c | 2 ++ 1 file changed, 2 insertions(+)