+Christian (re-sending for the list)
On Tue, Sep 15, 2026 at 1:11 AM Lazar, Lijo <[email protected]> wrote: > > > > On 20-May-26 9:16 PM, Mario Limonciello wrote: > > There are a few cases that code walks up the topology to find the > > link partner of the integrated switch in a dGPU. Split this out > > to a helper and call in all places. > > > > This does have a functional change that amdgpu_device_gpu_bandwidth() > > doesn't cache the internal link but only the parent. > > > > Signed-off-by: Mario Limonciello <[email protected]> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 44 +++++++++++----------- > > 1 file changed, 23 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > index 073f632f295a3..b6a24faaf78dd 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > > @@ -1913,6 +1913,19 @@ static void amdgpu_uid_fini(struct amdgpu_device > > *adev) > > adev->uid_info = NULL; > > } > > > > +static struct pci_dev *amdgpu_device_find_parent(struct amdgpu_device > > *adev) > > +{ > > + struct pci_dev *parent = adev->pdev; > > + > > + /* skip upstream/downstream switches internal to dGPU */ > > + while ((parent = pci_upstream_bridge(parent))) { > > + if (parent->vendor == PCI_VENDOR_ID_ATI) > > + continue; > > + } > > + > > + return parent; > > +} > > + > > /** > > * amdgpu_device_ip_early_init - run early init for hardware IPs > > * > > @@ -5913,8 +5926,6 @@ static void amdgpu_device_partner_bandwidth(struct > > amdgpu_device *adev, > > enum pci_bus_speed *speed, > > enum pcie_link_width *width) > > { > > - struct pci_dev *parent = adev->pdev; > > - > > if (!speed || !width) > > return; > > > > @@ -5922,13 +5933,11 @@ static void amdgpu_device_partner_bandwidth(struct > > amdgpu_device *adev, > > *width = PCIE_LNK_WIDTH_UNKNOWN; > > > > if (amdgpu_device_pcie_dynamic_switching_supported(adev)) { > > - while ((parent = pci_upstream_bridge(parent))) { > > - /* skip upstream/downstream switches internal to > > dGPU*/ > > - if (parent->vendor == PCI_VENDOR_ID_ATI) > > - continue; > > + struct pci_dev *parent = amdgpu_device_find_parent(adev); > > + > > + if (parent) { > > *speed = pcie_get_speed_cap(parent); > > *width = pcie_get_width_cap(parent); > > - break; > > } > > } else { > > /* use the current speeds rather than max if switching is not > > supported */ > > @@ -5955,22 +5964,15 @@ static void amdgpu_device_gpu_bandwidth(struct > > amdgpu_device *adev, > > if (!speed || !width) > > return; > > > > - parent = pci_upstream_bridge(parent); > > - if (parent && parent->vendor == PCI_VENDOR_ID_ATI) { > > - /* use the upstream/downstream switches internal to dGPU */ > > + /* use the device itself */ > > + *speed = pcie_get_speed_cap(adev->pdev); > > + *width = pcie_get_width_cap(adev->pdev); > > + > > + /* use the link outside the device */ > > + parent = amdgpu_device_find_parent(adev); > > This is for GPU device's bandwidth and not the parent device. Suggest to > cache both pdevs separately as link_dev (device which is connected to > actual physical link) and link_partner (the other end of link) one-time > during init. That will avoid confusion. > > Thanks, > Lijo > > > + if (parent) { > > *speed = pcie_get_speed_cap(parent); > > *width = pcie_get_width_cap(parent); > > - while ((parent = pci_upstream_bridge(parent))) { > > - if (parent->vendor == PCI_VENDOR_ID_ATI) { > > - /* use the upstream/downstream switches > > internal to dGPU */ > > - *speed = pcie_get_speed_cap(parent); > > - *width = pcie_get_width_cap(parent); > > - } > > - } > > - } else { > > - /* use the device itself */ > > - *speed = pcie_get_speed_cap(adev->pdev); > > - *width = pcie_get_width_cap(adev->pdev); > > } > > } > > >
