2022-11-03 02:50:12

by David E. Box

[permalink] [raw]
Subject: [PATCH V8 0/4] PCI: vmd: Enable PCIe ASPM and LTR on select hardware

This series adds a work around for enabling PCIe ASPM and for setting PCIe
LTR values on VMD reserved root ports on select platforms. While
configuration of these capabilities is usually done by BIOS, on these
platforms these capabilities will not be configured because the ports are
not visible to BIOS. This was part of an initial design that expected the
driver to completely handle the ports, including power management. However
on Linux those ports are still managed by the PCIe core, which has the
expectation that they adhere to device standards including BIOS
configuration, leading to this problem.

The target platforms are Tiger Lake, Alder Lake, and Raptor Lake though the
latter has already implemented support for configuring the LTR values.
Meteor Lake is expected add BIOS ASPM support, eliminating the future need
for this work around.

Note, the driver programs the LTRs because BIOS would also normally do this
for devices that do not set them by default. Without this, SoC power
management would be blocked on those platform. This SoC specific value is
the maximum latency required to allow the SoC to enter the deepest power
state.

This patch addresses the following open bugzillas on VMD enabled laptops
that cannot enter low power states.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=212355
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215063
Link: https://bugzilla.kernel.org/show_bug.cgi?id=213717

David E. Box (3):
PCI: vmd: Use PCI_VDEVICE in device list
PCI: vmd: Create feature grouping for client products
PCI: vmd: Add quirk to configure PCIe ASPM and LTR

Michael Bottini (1):
PCI/ASPM: Add pci_enable_link_state()

drivers/pci/controller/vmd.c | 96 ++++++++++++++++++++++++++----------
drivers/pci/pcie/aspm.c | 54 ++++++++++++++++++++
include/linux/pci.h | 7 +++
3 files changed, 131 insertions(+), 26 deletions(-)


base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740
--
2.25.1



2022-11-03 02:51:19

by David E. Box

[permalink] [raw]
Subject: [PATCH V8 1/4] PCI/ASPM: Add pci_enable_link_state()

From: Michael Bottini <[email protected]>

Add pci_enable_link_state() to allow devices to change the default BIOS
configured states. Clears the BIOS default settings then sets the new
states and reconfigures the link under the semaphore. Also add
PCIE_LINK_STATE_ALL macro for convenience for callers that want to enable
all link states.

Signed-off-by: Michael Bottini <[email protected]>
Signed-off-by: David E. Box <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
---
V8
- No change

V7
- Fix description as suggested by Bjorn
- Rename function to pci_enable_link_state

V6
- No change
V5
- Rename to pci_enable_default_link_state and model after
pci_disable_link_state() as suggested by Bjorn.
- Add helper PCIE_LINK_STATE_ALL which sets bits for all links states and
clock pm.
- Clarify commit language to indicate the function changes the default
link states (not ASPM policy).
V4
- Refactor vmd_enable_apsm() to exit early, making the lines shorter
and more readable. Suggested by Christoph.
V3
- No changes
V2
- Use return status to print pci_info message if ASPM cannot be enabled.
- Add missing static declaration, caught by [email protected]

drivers/pci/pcie/aspm.c | 54 +++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 7 ++++++
2 files changed, 61 insertions(+)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 53a1fa306e1e..339c686a5094 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1181,6 +1181,60 @@ int pci_disable_link_state(struct pci_dev *pdev, int state)
}
EXPORT_SYMBOL(pci_disable_link_state);

+/**
+ * pci_enable_link_state - Clear and set the default device link state so that
+ * the link may be allowed to enter the specified states. Note that if the
+ * BIOS didn't grant ASPM control to the OS, this does nothing because we can't
+ * touch the LNKCTL register. Also note that this does not enable states
+ * disabled by pci_disable_link_state(). Return 0 or a negative errno.
+ *
+ * @pdev: PCI device
+ * @state: Mask of ASPM link states to enable
+ */
+int pci_enable_link_state(struct pci_dev *pdev, int state)
+{
+ struct pcie_link_state *link = pcie_aspm_get_link(pdev);
+
+ if (!link)
+ return -EINVAL;
+ /*
+ * A driver requested that ASPM be enabled 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.
+ */
+ if (aspm_disabled) {
+ pci_warn(pdev, "can't override BIOS ASPM; OS doesn't have ASPM control\n");
+ return -EPERM;
+ }
+
+ down_read(&pci_bus_sem);
+ mutex_lock(&aspm_lock);
+ link->aspm_default = 0;
+ if (state & PCIE_LINK_STATE_L0S)
+ link->aspm_default |= ASPM_STATE_L0S;
+ if (state & PCIE_LINK_STATE_L1)
+ /* L1 PM substates require L1 */
+ link->aspm_default |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
+ if (state & PCIE_LINK_STATE_L1_1)
+ link->aspm_default |= ASPM_STATE_L1_1;
+ if (state & PCIE_LINK_STATE_L1_2)
+ link->aspm_default |= ASPM_STATE_L1_2;
+ if (state & PCIE_LINK_STATE_L1_1_PCIPM)
+ link->aspm_default |= ASPM_STATE_L1_1_PCIPM;
+ if (state & PCIE_LINK_STATE_L1_2_PCIPM)
+ link->aspm_default |= ASPM_STATE_L1_2_PCIPM;
+ pcie_config_aspm_link(link, policy_to_aspm_state(link));
+
+ link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;
+ pcie_set_clkpm(link, policy_to_clkpm_state(link));
+ mutex_unlock(&aspm_lock);
+ up_read(&pci_bus_sem);
+
+ return 0;
+}
+EXPORT_SYMBOL(pci_enable_link_state);
+
static int pcie_aspm_set_policy(const char *val,
const struct kernel_param *kp)
{
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2bda4a4e47e8..8c35f15e6012 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1651,10 +1651,15 @@ extern bool pcie_ports_native;
#define PCIE_LINK_STATE_L1_2 BIT(4)
#define PCIE_LINK_STATE_L1_1_PCIPM BIT(5)
#define PCIE_LINK_STATE_L1_2_PCIPM BIT(6)
+#define PCIE_LINK_STATE_ALL (PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1 |\
+ PCIE_LINK_STATE_CLKPM | PCIE_LINK_STATE_L1_1 |\
+ PCIE_LINK_STATE_L1_2 | PCIE_LINK_STATE_L1_1_PCIPM |\
+ PCIE_LINK_STATE_L1_2_PCIPM)

#ifdef CONFIG_PCIEASPM
int pci_disable_link_state(struct pci_dev *pdev, int state);
int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
+int pci_enable_link_state(struct pci_dev *pdev, int state);
void pcie_no_aspm(void);
bool pcie_aspm_support_enabled(void);
bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1663,6 +1668,8 @@ static inline int pci_disable_link_state(struct pci_dev *pdev, int state)
{ return 0; }
static inline int pci_disable_link_state_locked(struct pci_dev *pdev, int state)
{ return 0; }
+static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
+{ return 0; }
static inline void pcie_no_aspm(void) { }
static inline bool pcie_aspm_support_enabled(void) { return false; }
static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }
--
2.25.1


2022-11-03 02:51:56

by David E. Box

[permalink] [raw]
Subject: [PATCH V8 2/4] PCI: vmd: Use PCI_VDEVICE in device list

Refactor the PCI ID list to use PCI_VDEVICE.

Signed-off-by: David E. Box <[email protected]>
Reviewed-by: Jon Derrick <[email protected]>
Reviewed-by: Nirmal Patel <[email protected]>
---
V8 - No change

V7 - New Patch. Separate patch suggested by Lorenzo

drivers/pci/controller/vmd.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index e06e9f4fc50f..9dedca714c18 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -994,33 +994,33 @@ static int vmd_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(vmd_dev_pm_ops, vmd_suspend, vmd_resume);

static const struct pci_device_id vmd_ids[] = {
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_201D),
+ {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_VMD_201D),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP,},
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
+ {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_VMD_28C0),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW |
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_CAN_BYPASS_MSI_REMAP,},
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
+ {PCI_VDEVICE(INTEL, 0x467f),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_OFFSET_FIRST_VECTOR,},
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d),
+ {PCI_VDEVICE(INTEL, 0x4c3d),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_OFFSET_FIRST_VECTOR,},
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xa77f),
+ {PCI_VDEVICE(INTEL, 0xa77f),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_OFFSET_FIRST_VECTOR,},
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x7d0b),
+ {PCI_VDEVICE(INTEL, 0x7d0b),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_OFFSET_FIRST_VECTOR,},
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0xad0b),
+ {PCI_VDEVICE(INTEL, 0xad0b),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_OFFSET_FIRST_VECTOR,},
- {PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
+ {PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_OFFSET_FIRST_VECTOR,},
--
2.25.1


2022-11-03 03:23:53

by David E. Box

[permalink] [raw]
Subject: [PATCH V8 3/4] PCI: vmd: Create feature grouping for client products

Simplify the device ID list by creating a grouping of features shared by
client products.

Suggested-by: Jon Derrick <[email protected]>
Signed-off-by: David E. Box <[email protected]>
---

V8 - New patch.

drivers/pci/controller/vmd.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 9dedca714c18..86f3085db014 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -68,6 +68,10 @@ enum vmd_features {
VMD_FEAT_CAN_BYPASS_MSI_REMAP = (1 << 4),
};

+#define VMD_FEATS_CLIENT (VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP | \
+ VMD_FEAT_HAS_BUS_RESTRICTIONS | \
+ VMD_FEAT_OFFSET_FIRST_VECTOR)
+
static DEFINE_IDA(vmd_instance_ida);

/*
@@ -1001,29 +1005,17 @@ static const struct pci_device_id vmd_ids[] = {
VMD_FEAT_HAS_BUS_RESTRICTIONS |
VMD_FEAT_CAN_BYPASS_MSI_REMAP,},
{PCI_VDEVICE(INTEL, 0x467f),
- .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
- VMD_FEAT_HAS_BUS_RESTRICTIONS |
- VMD_FEAT_OFFSET_FIRST_VECTOR,},
+ .driver_data = VMD_FEATS_CLIENT,},
{PCI_VDEVICE(INTEL, 0x4c3d),
- .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
- VMD_FEAT_HAS_BUS_RESTRICTIONS |
- VMD_FEAT_OFFSET_FIRST_VECTOR,},
+ .driver_data = VMD_FEATS_CLIENT,},
{PCI_VDEVICE(INTEL, 0xa77f),
- .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
- VMD_FEAT_HAS_BUS_RESTRICTIONS |
- VMD_FEAT_OFFSET_FIRST_VECTOR,},
+ .driver_data = VMD_FEATS_CLIENT,},
{PCI_VDEVICE(INTEL, 0x7d0b),
- .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
- VMD_FEAT_HAS_BUS_RESTRICTIONS |
- VMD_FEAT_OFFSET_FIRST_VECTOR,},
+ .driver_data = VMD_FEATS_CLIENT,},
{PCI_VDEVICE(INTEL, 0xad0b),
- .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
- VMD_FEAT_HAS_BUS_RESTRICTIONS |
- VMD_FEAT_OFFSET_FIRST_VECTOR,},
+ .driver_data = VMD_FEATS_CLIENT,},
{PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
- .driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
- VMD_FEAT_HAS_BUS_RESTRICTIONS |
- VMD_FEAT_OFFSET_FIRST_VECTOR,},
+ .driver_data = VMD_FEATS_CLIENT,},
{0,}
};
MODULE_DEVICE_TABLE(pci, vmd_ids);
--
2.25.1


2022-11-23 10:58:47

by You-Sheng Yang

[permalink] [raw]
Subject: Re: [PATCH V8 0/4] PCI: vmd: Enable PCIe ASPM and LTR on select hardware

Hi David,

On 11/3/22 10:18, David E. Box wrote:
> This series adds a work around for enabling PCIe ASPM and for setting PCIe
> LTR values on VMD reserved root ports on select platforms. While
> configuration of these capabilities is usually done by BIOS, on these
> platforms these capabilities will not be configured because the ports are
> not visible to BIOS. This was part of an initial design that expected the
> driver to completely handle the ports, including power management. However
> on Linux those ports are still managed by the PCIe core, which has the
> expectation that they adhere to device standards including BIOS
> configuration, leading to this problem.
>
> The target platforms are Tiger Lake, Alder Lake, and Raptor Lake though the
> latter has already implemented support for configuring the LTR values.
> Meteor Lake is expected add BIOS ASPM support, eliminating the future need
> for this work around.


It appears to me that this patch series works only on Tiger Lake. We
have tried to revert our current work-arounds in Ubuntu kernels
generic-5.15/oem-5.17/oem-6.0/unstable-6.1 and apply this series, the
prebuilt kernels can be found in:

  https://launchpad.net/~vicamo/+archive/ubuntu/ppa-1996620

However, only TGL can still enter PC10 as before.


ADL-M, RPL platforms will stay in PC3 with vmd LTR set, but ASPM
disabled. i915 RC6 blocked, too:

$ sudo cat /sys/kernel/debug/dri/

0/i915_dmc_info
...
DC3CO count: 0
DC3 -> DC5 count: 100
DC5 -> DC6 count: 0


> Note, the driver programs the LTRs because BIOS would also normally do this
> for devices that do not set them by default. Without this, SoC power
> management would be blocked on those platform. This SoC specific value is
> the maximum latency required to allow the SoC to enter the deepest power
> state.
>
> This patch addresses the following open bugzillas on VMD enabled laptops
> that cannot enter low power states.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=212355
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215063
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=213717
>
> David E. Box (3):
> PCI: vmd: Use PCI_VDEVICE in device list
> PCI: vmd: Create feature grouping for client products
> PCI: vmd: Add quirk to configure PCIe ASPM and LTR
>
> Michael Bottini (1):
> PCI/ASPM: Add pci_enable_link_state()
>
> drivers/pci/controller/vmd.c | 96 ++++++++++++++++++++++++++----------
> drivers/pci/pcie/aspm.c | 54 ++++++++++++++++++++
> include/linux/pci.h | 7 +++
> 3 files changed, 131 insertions(+), 26 deletions(-)
>
>
> base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740


Regards,
You-Sheng Yang

2022-11-23 16:30:15

by David E. Box

[permalink] [raw]
Subject: Re: [PATCH V8 0/4] PCI: vmd: Enable PCIe ASPM and LTR on select hardware

Hi You-Sheng,

On Wed, 2022-11-23 at 18:27 +0800, You-Sheng Yang wrote:
> Hi David,
>
> On 11/3/22 10:18, David E. Box wrote:
> > This series adds a work around for enabling PCIe ASPM and for setting PCIe
> > LTR values on VMD reserved root ports on select platforms. While
> > configuration of these capabilities is usually done by BIOS, on these
> > platforms these capabilities will not be configured because the ports are
> > not visible to BIOS. This was part of an initial design that expected the
> > driver to completely handle the ports, including power management. However
> > on Linux those ports are still managed by the PCIe core, which has the
> > expectation that they adhere to device standards including BIOS
> > configuration, leading to this problem.
> >
> > The target platforms are Tiger Lake, Alder Lake, and Raptor Lake though the
> > latter has already implemented support for configuring the LTR values.
> > Meteor Lake is expected add BIOS ASPM support, eliminating the future need
> > for this work around.
>
>
> It appears to me that this patch series works only on Tiger Lake. We
> have tried to revert our current work-arounds in Ubuntu kernels
> generic-5.15/oem-5.17/oem-6.0/unstable-6.1 and apply this series, the
> prebuilt kernels can be found in:
>
>    https://launchpad.net/~vicamo/+archive/ubuntu/ppa-1996620
>
> However, only TGL can still enter PC10 as before.
>
>
> ADL-M, RPL platforms will stay in PC3 with vmd LTR set, but ASPM
> disabled.

For the patch to work BIOS must allow the OS to control ASPM. If this is not the
case then you will see the message "ACPI FADT declares the system doesn't
support PCIe ASPM, so disable it". Please check for this on the systems that
don't work. If so the only option is a BIOS change to enable it.

David

> i915 RC6 blocked, too:
>
> $ sudo cat /sys/kernel/debug/dri/
>
> 0/i915_dmc_info
> ...
> DC3CO count: 0
> DC3 -> DC5 count: 100
> DC5 -> DC6 count: 0
>
>
> > Note, the driver programs the LTRs because BIOS would also normally do this
> > for devices that do not set them by default. Without this, SoC power
> > management would be blocked on those platform. This SoC specific value is
> > the maximum latency required to allow the SoC to enter the deepest power
> > state.
> >
> > This patch addresses the following open bugzillas on VMD enabled laptops
> > that cannot enter low power states.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=212355
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215063
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=213717
> >
> > David E. Box (3):
> >    PCI: vmd: Use PCI_VDEVICE in device list
> >    PCI: vmd: Create feature grouping for client products
> >    PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> >
> > Michael Bottini (1):
> >    PCI/ASPM: Add pci_enable_link_state()
> >
> >   drivers/pci/controller/vmd.c | 96 ++++++++++++++++++++++++++----------
> >   drivers/pci/pcie/aspm.c      | 54 ++++++++++++++++++++
> >   include/linux/pci.h          |  7 +++
> >   3 files changed, 131 insertions(+), 26 deletions(-)
> >
> >
> > base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740
>
>
> Regards,
> You-Sheng Yang
>

2022-11-24 16:46:45

by You-Sheng Yang

[permalink] [raw]
Subject: Re: [PATCH V8 0/4] PCI: vmd: Enable PCIe ASPM and LTR on select hardware

On Thu, Nov 24, 2022 at 12:09 AM David E. Box
<[email protected]> wrote:
>
> Hi You-Sheng,
>
> On Wed, 2022-11-23 at 18:27 +0800, You-Sheng Yang wrote:
> > Hi David,
> >
> > On 11/3/22 10:18, David E. Box wrote:
> > > This series adds a work around for enabling PCIe ASPM and for setting PCIe
> > > LTR values on VMD reserved root ports on select platforms. While
> > > configuration of these capabilities is usually done by BIOS, on these
> > > platforms these capabilities will not be configured because the ports are
> > > not visible to BIOS. This was part of an initial design that expected the
> > > driver to completely handle the ports, including power management. However
> > > on Linux those ports are still managed by the PCIe core, which has the
> > > expectation that they adhere to device standards including BIOS
> > > configuration, leading to this problem.
> > >
> > > The target platforms are Tiger Lake, Alder Lake, and Raptor Lake though the
> > > latter has already implemented support for configuring the LTR values.
> > > Meteor Lake is expected add BIOS ASPM support, eliminating the future need
> > > for this work around.
> >
> >
> > It appears to me that this patch series works only on Tiger Lake. We
> > have tried to revert our current work-arounds in Ubuntu kernels
> > generic-5.15/oem-5.17/oem-6.0/unstable-6.1 and apply this series, the
> > prebuilt kernels can be found in:
> >
> > https://launchpad.net/~vicamo/+archive/ubuntu/ppa-1996620
> >
> > However, only TGL can still enter PC10 as before.
> >
> >
> > ADL-M, RPL platforms will stay in PC3 with vmd LTR set, but ASPM
> > disabled.
>
> For the patch to work BIOS must allow the OS to control ASPM. If this is not the
> case then you will see the message "ACPI FADT declares the system doesn't
> support PCIe ASPM, so disable it". Please check for this on the systems that
> don't work. If so the only option is a BIOS change to enable it.

Thank you. It's exactly what you said. The ADL-M/RPL platforms I have
do not support OS PCIe ASPM.

> David
>
> > i915 RC6 blocked, too:
> >
> > $ sudo cat /sys/kernel/debug/dri/
> >
> > 0/i915_dmc_info
> > ...
> > DC3CO count: 0
> > DC3 -> DC5 count: 100
> > DC5 -> DC6 count: 0
> >
> >
> > > Note, the driver programs the LTRs because BIOS would also normally do this
> > > for devices that do not set them by default. Without this, SoC power
> > > management would be blocked on those platform. This SoC specific value is
> > > the maximum latency required to allow the SoC to enter the deepest power
> > > state.
> > >
> > > This patch addresses the following open bugzillas on VMD enabled laptops
> > > that cannot enter low power states.
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=212355
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215063
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=213717
> > >
> > > David E. Box (3):
> > > PCI: vmd: Use PCI_VDEVICE in device list
> > > PCI: vmd: Create feature grouping for client products
> > > PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> > >
> > > Michael Bottini (1):
> > > PCI/ASPM: Add pci_enable_link_state()
> > >
> > > drivers/pci/controller/vmd.c | 96 ++++++++++++++++++++++++++----------
> > > drivers/pci/pcie/aspm.c | 54 ++++++++++++++++++++
> > > include/linux/pci.h | 7 +++
> > > 3 files changed, 131 insertions(+), 26 deletions(-)
> > >
> > >
> > > base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740
> >
> >
> > Regards,
> > You-Sheng Yang
> >
>


--
Regards,
You-Sheng Yang

2022-12-07 09:12:09

by Luke D. Jones

[permalink] [raw]
Subject: Re: [PATCH V8 0/4] PCI: vmd: Enable PCIe ASPM and LTR on select hardware

On Wed, 2022-11-02 at 19:18 -0700, David E. Box wrote:
> This series adds a work around for enabling PCIe ASPM and for setting
> PCIe
> LTR values on VMD reserved root ports on select platforms. While
> configuration of these capabilities is usually done by BIOS, on these
> platforms these capabilities will not be configured because the ports
> are
> not visible to BIOS. This was part of an initial design that expected
> the
> driver to completely handle the ports, including power management.
> However
> on Linux those ports are still managed by the PCIe core, which has
> the
> expectation that they adhere to device standards including BIOS
> configuration, leading to this problem.
>
> The target platforms are Tiger Lake, Alder Lake, and Raptor Lake
> though the
> latter has already implemented support for configuring the LTR
> values.
> Meteor Lake is expected add BIOS ASPM support, eliminating the future
> need
> for this work around.
>
> Note, the driver programs the LTRs because BIOS would also normally
> do this
> for devices that do not set them by default. Without this, SoC power
> management would be blocked on those platform. This SoC specific
> value is
> the maximum latency required to allow the SoC to enter the deepest
> power
> state.
>
> This patch addresses the following open bugzillas on VMD enabled
> laptops
> that cannot enter low power states.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=212355
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215063
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=213717
>
> David E. Box (3):
>   PCI: vmd: Use PCI_VDEVICE in device list
>   PCI: vmd: Create feature grouping for client products
>   PCI: vmd: Add quirk to configure PCIe ASPM and LTR
>
> Michael Bottini (1):
>   PCI/ASPM: Add pci_enable_link_state()
>
>  drivers/pci/controller/vmd.c | 96 ++++++++++++++++++++++++++--------
> --
>  drivers/pci/pcie/aspm.c      | 54 ++++++++++++++++++++
>  include/linux/pci.h          |  7 +++
>  3 files changed, 131 insertions(+), 26 deletions(-)
>
>
> base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740

I'd like to confirm that this patch series solves the VMD power issues
present on the ASUS ROG M16 (GU603) laptop range (PCI dev ID = 467f).

The difference is quite drastic.

2022-12-08 02:54:00

by David E. Box

[permalink] [raw]
Subject: Re: [PATCH V8 0/4] PCI: vmd: Enable PCIe ASPM and LTR on select hardware

On Wed, 2022-12-07 at 22:05 +1300, Luke Jones wrote:
> On Wed, 2022-11-02 at 19:18 -0700, David E. Box wrote:
> > This series adds a work around for enabling PCIe ASPM and for setting
> > PCIe
> > LTR values on VMD reserved root ports on select platforms. While
> > configuration of these capabilities is usually done by BIOS, on these
> > platforms these capabilities will not be configured because the ports
> > are
> > not visible to BIOS. This was part of an initial design that expected
> > the
> > driver to completely handle the ports, including power management.
> > However
> > on Linux those ports are still managed by the PCIe core, which has
> > the
> > expectation that they adhere to device standards including BIOS
> > configuration, leading to this problem.
> >
> > The target platforms are Tiger Lake, Alder Lake, and Raptor Lake
> > though the
> > latter has already implemented support for configuring the LTR
> > values.
> > Meteor Lake is expected add BIOS ASPM support, eliminating the future
> > need
> > for this work around.
> >
> > Note, the driver programs the LTRs because BIOS would also normally
> > do this
> > for devices that do not set them by default. Without this, SoC power
> > management would be blocked on those platform. This SoC specific
> > value is
> > the maximum latency required to allow the SoC to enter the deepest
> > power
> > state.
> >
> > This patch addresses the following open bugzillas on VMD enabled
> > laptops
> > that cannot enter low power states.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=212355
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215063
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=213717
> >
> > David E. Box (3):
> >   PCI: vmd: Use PCI_VDEVICE in device list
> >   PCI: vmd: Create feature grouping for client products
> >   PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> >
> > Michael Bottini (1):
> >   PCI/ASPM: Add pci_enable_link_state()
> >
> >  drivers/pci/controller/vmd.c | 96 ++++++++++++++++++++++++++--------
> > --
> >  drivers/pci/pcie/aspm.c      | 54 ++++++++++++++++++++
> >  include/linux/pci.h          |  7 +++
> >  3 files changed, 131 insertions(+), 26 deletions(-)
> >
> >
> > base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740
>
> I'd like to confirm that this patch series solves the VMD power issues
> present on the ASUS ROG M16 (GU603) laptop range (PCI dev ID = 467f).
>
> The difference is quite drastic.
>

Thanks for testing this Luke.

David