On Fri, May 23, 2025 at 9:21 PM Kenneth Crudup <kenny@xxxxxxxxx> wrote:
>
> Raphael, any input?
Well, Bjorn's concerns are valid AFAICS.
> On 5/14/25 18:23, Russell Haley wrote:
> >
> >
> > On 5/12/25 4:09 PM, Bjorn Helgaas wrote:
> >
> >>> static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
> >>> {
> >>> struct pci_dev *child = link->downstream, *parent = link->pdev;
> >>> @@ -866,7 +891,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
> >>> }
> >>>
> >>> /* Save default state */
> >>> - link->aspm_default = link->aspm_enabled;
> >>> + link->aspm_default = pci_fixup_vmd_bridge_enable_aspm(parent) ?
> >>> + PCIE_LINK_STATE_ASPM_ALL : link->aspm_enabled;
> >>
> >> PCIE_LINK_STATE_ASPM_ALL includes PCIE_LINK_STATE_L1_2, so I think
> >> this potentially enables L1.2. The L1.2 configuration depends on
> >> T_POWER_ON and Common_Mode_Restore_Time, which depend on electrical
> >> design and are not discoverable by the kernel. See PCIe r6.0, sec
> >> 5.5.4:
> >>
> >> The TPOWER_ON and Common_Mode_Restore_Time fields must be programmed
> >> to the appropriate values based on the components and AC coupling
> >> capacitors used in the connection linking the two components. The
> >> determination of these values is design implementation specific.
> >
> > Does that apply to VMD? As far as I know it's not an actual physical
> > PCIe device.
The devices on the VMD bus are physical PCIe devices, but they need to
be accessed in a special way and the BIOS doesn't have access to them.
I would try to replace the PCIE_LINK_STATE_ASPM_ALL in the patch with
PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1 | PCIE_LINK_STATE_L1_1 |
PCIE_LINK_STATE_L1_1_PCIPM
and see how far it gets you. Please let me know how it goes.
In the meantime, I'll try to find somebody who can pick this up.
Thanks!
