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

[Bjorn Helgaas <helgaas@kernel.org>]

[cc->to: David, Nirmal]

On Fri, Dec 13, 2024 at 02:26:37PM -0800, Kenneth Crudup wrote:
> OK, it looks like the effective change (that's not already contained in the
> LTR SNOOP patches already in Linus' master (et al.)) comes from this line
> from the Ubuntu commit 1a0102a0 ("UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for
> links under VMD domain"):
> 
> ----
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 00143f5fb83a..d2ff44e7fbb1 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -688,7 +688,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state
> *link, int blacklist)
>         aspm_l1ss_init(link);
> 
>         /* Save default state */
> -       link->aspm_default = link->aspm_enabled;
> +       link->aspm_default = parent->dev_flags & PCI_DEV_FLAGS_ENABLE_ASPM ?
> +                            ASPM_STATE_ALL : link->aspm_enabled;

So I thought the "pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL)" in
f492edb40b54 would effectively do the same thing:

> > > > > 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 I guess it doesn't actually work.  I'm hoping David or Nirmal can
figure out why it doesn't because it seems obvious that it's the
intent.

Thanks a lot for all your work to narrow it down to this!

Bjorn