Re: [PATCH] PCI/ASPM: Enable L0s/L1 for removable devices when BIOS didn't configure ASPM

[Mario Limonciello <mario.limonciello@amd.com>]

On 5/5/26 23:53, Mika Westerberg wrote:
> Hi,
> 
> On Tue, May 05, 2026 at 08:09:22PM +0200, Rafael J. Wysocki wrote:
>> +Mika Westerberg
>>
>> On Tue, May 5, 2026 at 12:53 AM Mario Limonciello
>> <mario.limonciello@amd.com> wrote:
>>>
>>> When comparing lspci output between Windows and Linux for hotplugged
>>> Thunderbolt 5 eGPU devices, Windows enables ASPM L1 but Linux doesn't:
>>>
>>>    Windows: LnkCtl: ASPM L1 Enabled
>>>    Linux:   LnkCtl: ASPM Disabled
>>>
>>> This difference in ASPM configuration can cause behavioral differences
>>> between the two operating systems for the same hardware.
>>>
>>> The root cause is that Linux's default ASPM policy (POLICY_DEFAULT) relies
>>> on firmware/BIOS configuration. For hotplugged devices like Thunderbolt/USB4
>>> eGPUs, the BIOS may not have configured ASPM since the device wasn't present
>>> at boot time. As a result, link->aspm_enabled is 0, link->aspm_default is
>>> set to 0, and Linux never enables ASPM for these devices.
>>>
>>> Devicetree platforms already have special handling to enable L0s/L1 by
>>> default regardless of firmware configuration. Extend this same logic to
>>> removable devices when firmware hasn't configured any ASPM states.
>>>
>>> This makes Linux behavior more consistent with Windows for hotplugged
>>> Thunderbolt/USB4 devices.
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=221319
>>> Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>>   drivers/pci/pcie/aspm.c | 11 +++++++++--
>>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>>> index 925373b98dff0..77497d90be0b7 100644
>>> --- a/drivers/pci/pcie/aspm.c
>>> +++ b/drivers/pci/pcie/aspm.c
>>> @@ -804,8 +804,15 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>>>          struct pci_dev *pdev = link->downstream;
>>>          u32 override;
>>>
>>> -       /* For devicetree platforms, enable L0s and L1 by default */
>>> -       if (of_have_populated_dt()) {
>>> +       /*
>>> +        * For devicetree platforms, enable L0s and L1 by default.
>>> +        *
>>> +        * For removable devices (e.g., Thunderbolt/USB4), enable L0s and L1
>>> +        * by default if BIOS didn't configure any ASPM states. This handles
>>> +        * hotplugged devices where firmware may not have configured ASPM.
>>> +        */
> 
> Only L1 is supported over TB/USB4 tunnel (no L0s, no L1 substates). The
> PCIe endpoint and the downstream port it connects to of course can support
> the full range as that's a real PCIe link.
> 

OK - the comment should be updated but I do expect that below code 
(link->aspm_support) should remain OK.

>>> +       if (of_have_populated_dt() ||
>>> +           (dev_is_removable(&pdev->dev) && !link->aspm_enabled)) {
>>>                  if (link->aspm_support & PCIE_LINK_STATE_L0S)
>>>                          link->aspm_default |= PCIE_LINK_STATE_L0S;
>>>                  if (link->aspm_support & PCIE_LINK_STATE_L1)
>>> --
>>> 2.43.0
>>>