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

["David E. Box" <david.e.box@linux.intel.com>]

+Rafael

On Thu, 2024-12-19 at 10:17 -0800, Kenneth Crudup wrote:
> I do see that:
> 
> ----
> [E0] 781 /usr/src/ubuntu-kernel> dmesg | fgrep -i aspm
> [    0.164233] ACPI FADT declares the system doesn't support PCIe ASPM, 
> so disable it

So, PCIe ASPM refers to OS control of ASPM. Disabling it means the BIOS alone
controls it, leaving the OS to stick with the defaults programmed into the
controllers by the BIOS. This might happen due to critical bugs in certain ASPM
states or simply because the OEM decided to configure it that way. We don't know
the exact reason.

The issue with VMD is that its controllers are hidden from the BIOS, so ASPM
defaults are never programmed into them. When PCIe ASPM is disabled, it's
unclear whether this should apply to controllers in VMD mode. To be cautious, we
avoid modifying ASPM settings in this scenario.

If you want to override this behavior, you can try setting pcie_aspm=force on
the kernel command line.

David

> [    0.579946] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM 
> ClockPM Segments MSI EDR HPX-Type3]
> [    0.587377] acpi PNP0A08:00: FADT indicates ASPM is unsupported, 
> using BIOS configuration
> [    1.309826] pci 10000:e0:06.0: enable ASPM for pci bridge behind vmd
> [    1.622705] pci 10000:e1:00.0: can't override BIOS ASPM; OS doesn't 
> have ASPM control
> [110757.878494] pcieport 0000:00:07.0: ASPM: current common clock 
> configuration is inconsistent, reconfiguring
> [171953.284616] pcieport 0000:00:07.0: ASPM: current common clock 
> configuration is inconsistent, reconfiguring
> ----
> 
> On 12/19/24 08:25, David E. Box wrote:
> > Hi Kenneth,
> > 
> > On Fri, 2024-12-13 at 17:02 -0600, Bjorn Helgaas wrote:
> > > [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.
> > 
> > Is PCIe ASPM disabled? In the kernel log do you see:
> > 
> >      "can't override BIOS ASPM; OS doesn't have ASPM control"
> > 
> > David
> > 
>