RE: [PATCH] drm/amdgpu: check ASPM on the dGPU host link

AMD General

> -----Original Message-----
> From: Feng, Kenneth <[email protected]>
> Sent: Thursday, August 6, 2026 7:17 PM
> To: Wang, Yang(Kevin) <[email protected]>; amd-
> [email protected]
> Cc: Deucher, Alexander <[email protected]>; Zhang, Hawking
> <[email protected]>
> Subject: RE: [PATCH] drm/amdgpu: check ASPM on the dGPU host link
>
> AMD General
>
> dev_info() could flood dmesg, better to use dev_dbg().
> Reviewed-by: Kenneth Feng <[email protected]>

Thanks, will refine it before submitting. (and Asad raised the same suggestion during our offline talk)

Best Regards,
Kevin
>
> -----Original Message-----
> From: Wang, Yang(Kevin) <[email protected]>
> Sent: Thursday, August 6, 2026 1:47 PM
> To: [email protected]
> Cc: Deucher, Alexander <[email protected]>; Zhang, Hawking
> <[email protected]>; Feng, Kenneth <[email protected]>
> Subject: [PATCH] drm/amdgpu: check ASPM on the dGPU host link
>
> dGPUs with an internal PCIe switch expose graphics functions below the
> switch downstream port. The automatic ASPM check uses the display
> endpoint and evaluates the internal link instead of the host link.
>
> Use the switch upstream port for the check and report the selected link.
>
> Fixes: 0ab5d711ec74 ("drm/amd: Refactor `amdgpu_aspm` to be evaluated
> per device")
> Signed-off-by: Yang Wang <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 50
> +++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 2b9799033174..e68a5ddcae69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1344,6 +1344,31 @@ static bool
> amdgpu_device_aspm_support_quirk(struct amdgpu_device *adev)  #endif  }
>
> +/*
> + * Some dGPUs expose their display endpoint below an internal PCIe switch.
> + * Use the switch upstream port to query the host-facing link.
> + */
> +static struct pci_dev *amdgpu_device_get_aspm_pdev(struct
> amdgpu_device
> +*adev) {
> +       struct pci_dev *swds, *swus;
> +
> +       swds = pci_upstream_bridge(adev->pdev);
> +       if (!swds ||
> +           (swds->vendor != PCI_VENDOR_ID_ATI &&
> +            swds->vendor != PCI_VENDOR_ID_AMD) ||
> +           pci_pcie_type(swds) != PCI_EXP_TYPE_DOWNSTREAM)
> +               return adev->pdev;
> +
> +       swus = pci_upstream_bridge(swds);
> +       if (!swus ||
> +           (swus->vendor != PCI_VENDOR_ID_ATI &&
> +            swus->vendor != PCI_VENDOR_ID_AMD) ||
> +           pci_pcie_type(swus) != PCI_EXP_TYPE_UPSTREAM)
> +               return adev->pdev;
> +
> +       return swus;
> +}
> +
>  /**
>   * amdgpu_device_should_use_aspm - check if the device should program
> ASPM
>   *
> @@ -1356,6 +1381,9 @@ static bool
> amdgpu_device_aspm_support_quirk(struct amdgpu_device *adev)
>   */
>  bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev)  {
> +       struct pci_dev *aspm_pdev, *parent;
> +       bool enabled;
> +
>         switch (amdgpu_aspm) {
>         case -1:
>                 break;
> @@ -1370,7 +1398,27 @@ bool amdgpu_device_should_use_aspm(struct
> amdgpu_device *adev)
>                 return false;
>         if (amdgpu_device_aspm_support_quirk(adev))
>                 return false;
> -       return pcie_aspm_enabled(adev->pdev);
> +
> +       /*
> +        * pcie_aspm_enabled() checks the link between its argument and
> +        * the immediate upstream bridge. Use SWUS for dGPUs with an
> +        * internal switch so that this is the host-facing link.
> +        */
> +       aspm_pdev = amdgpu_device_get_aspm_pdev(adev);
> +       parent = pci_upstream_bridge(aspm_pdev);
> +       if (!parent) {
> +               dev_info(adev->dev, "ASPM: no upstream PCIe link for %s\n",
> +                        pci_name(aspm_pdev));
> +               return false;
> +       }
> +
> +       enabled = pcie_aspm_enabled(aspm_pdev);
> +       /* Report the exact link used for the automatic ASPM decision. */
> +       dev_info(adev->dev, "ASPM: link %s <-> %s is %s\n",
> +                pci_name(parent), pci_name(aspm_pdev),
> +                enabled ? "enabled" : "disabled");
> +
> +       return enabled;
>  }
>
>  /* if we get transitioned to only one device, take VGA back */
> --
> 2.54.0
>