Re: My AlderLake Dell (XPS-9320) needs these patches to get full standby/low-power modes

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc David, Nirmal, linux-pci]

On Thu, Dec 12, 2024 at 03:04:53PM -0800, Kenneth Crudup wrote:
> On 12/12/24 12:56, Bjorn Helgaas wrote:
> > On Wed, Dec 11, 2024 at 03:26:37PM -0800, Kenneth Crudup wrote:
> > > So we're on 6.13-rc2 and the patches are getting closer and
> > > closer, but they still need to be manually added.
> > > 
> > > The good news is now only (variants of) "PCI/ASPM: Enable LTR
> > > for endpoints behind VMD" and "PCI/ASPM: Enable ASPM for links
> > > under VMD domain" are needed.
> ...

> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/lunar/commit/?id=47c7bfd31514e7b54a1f830f7707297aebbb8679

This is "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind
VMD", which writes PCI_LTR_MAX_SNOOP_LAT and PCI_LTR_MAX_NOSNOOP_LAT
for PCI_CLASS_STORAGE_EXPRESS devices directly below VMDs with Device
IDs 0x9a09, 0xa0b0, or 0xa0bc.

This looks equivalent in spirit to upstream
https://git.kernel.org/linus/f492edb40b54 ("PCI: vmd: Add quirk to
configure PCIe ASPM and LTR"), which writes PCI_LTR_MAX_SNOOP_LAT and
PCI_LTR_MAX_NOSNOOP_LAT for any kind of device below VMDs tagged with
VMD_FEATS_CLIENT, which includes 0x467f, 0x4c3d, 0x7d0b, 0x9a0b,
0xa77f, 0xad0b, 0xb06f, 0xb60b.

However, IDs 0x9a09, 0xa0b0, and 0xa0bc are NOT tagged with
VMD_FEATS_CLIENT.  In fact, they're not included in vmd_ids[] at all,
so I'd be surprised if VMD worked for those devices unless BIOS set
up the VMD itself.

Maybe David or Nirmal can comment on this?

> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/lunar/commit/?id=1a0102a08f206149d9abd56c2b28877c878b5526

This is "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
domain", which adds "link->aspm_default = ASPM_STATE_ALL" for device
IDs 0x9a09 and 0xa0b0.

This looks like it should also be handled by upstream f492edb40b54
("PCI: vmd: Add quirk to configure PCIe ASPM and LTR") [1], which adds
"pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL)".

But again, the Device IDs mentioned in the Ubuntu commit are NOT
included in the upstream VMD_FEATS_CLIENT list.

> https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/lunar/commit/?id=154d48da2c57514e4b5dadc7b8c70a4edb550981

This is "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name
instead", which applies the quirk that writes PCI_LTR_MAX_SNOOP_LAT
and PCI_LTR_MAX_NOSNOOP_LAT for PCI_CLASS_STORAGE_EXPRESS devices
below any VMD claimed by the "vmd" driver, not just VMDs with Device
IDs 0x9a09, 0xa0b0, or 0xa0bc.

I think the only thing that's missing is that the upstream vmd_ids[]
needs to be updated with some new VMD Device IDs that are tagged with
VMD_FEATS_CLIENT.

I don't know what the vmd_ids[] strategy is, but Kenneth, you might
try an upstream patch like the one below.  If that resolves the
standby/low-power issues, maybe David or Nirmal can figure out the
"right" way to do this.

Bjorn

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 9d9596947350..4de7ff3bbf23 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -1109,6 +1109,12 @@ static const struct pci_device_id vmd_ids[] = {
                 .driver_data = VMD_FEATS_CLIENT,},
 	{PCI_VDEVICE(INTEL, 0xb06f),
                 .driver_data = VMD_FEATS_CLIENT,},
+	{PCI_VDEVICE(INTEL, 0x9a09),
+                .driver_data = VMD_FEATS_CLIENT,},
+	{PCI_VDEVICE(INTEL, 0xa0b0),
+                .driver_data = VMD_FEATS_CLIENT,},
+	{PCI_VDEVICE(INTEL, 0xa0bc),
+                .driver_data = VMD_FEATS_CLIENT,},
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, vmd_ids);