[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <2d36653c-0f5d-21d4-b974-112ccb599d68@linux.intel.com>
Date: Mon, 5 Jan 2026 16:17:03 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Ziyao Li <liziyao@...ontech.com>
cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kwilczynski@...nel.org>,
Manivannan Sadhasivam <mani@...nel.org>, Rob Herring <robh@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, niecheng1@...ontech.com,
zhanjun@...ontech.com, guanwentao@...ontech.com,
Kexy Biscuit <kexybiscuit@...c.io>, linux-pci@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>, Lain Fearyncess Yang <fsf@...e.com>,
Ayden Meng <aydenmeng@...h.net>, Mingcong Bai <jeffbai@...c.io>,
Xi Ruoyao <xry111@...111.site>
Subject: Re: [PATCH v2] PCI: loongson: Override PCIe bridge supported speeds
for older Loongson 3C6000 series steppings
On Sun, 4 Jan 2026, Ziyao Li via B4 Relay wrote:
> From: Ziyao Li <liziyao@...ontech.com>
>
> Older steppings of the Loongson 3C6000 series incorrectly report the
> supported link speeds on their PCIe bridges (device IDs 3c19, 3c29) as
> only 2.5 GT/s, despite the upstream bus supporting speeds from 2.5 GT/s
> up to 16 GT/s.
>
> As a result, certain PCIe devices would be incorrectly probed as a Gen1-
> only, even if higher link speeds are supported, harming performance and
> prevents dynamic link speed functionality from being enabled in drivers
> such as amdgpu.
What do you mean here? Does "dynamic link speed functionality" refer to
bwctrl? (If that's the case, can you write it out explicitly as the
connection is not obvious and I've heard the AMD GPUs do manage link
speed autonomously too so you might be referring to that too.)
I don't see amdgpu using supported_speeds for anything, I guess the
connection comes through pcie_get_speed_cap() which is seemingly used by
amdgpu code (if that's the case, please include that into the
explanation)?
> Manually override the `supported_speeds` field for affected PCIe bridges
> with those found on the upstream bus to correctly reflect the supported
> link speeds.
It's nice to see this field becoming useful for this kind of cases too, I
kind of foresaw it happening one day when I added it. :-)
> This patch was originally found from AOSC OS[1].
>
> Link: https://github.com/AOSC-Tracking/linux/pull/2 #1
> Tested-by: Lain Fearyncess Yang <fsf@...e.com>
> Tested-by: Ayden Meng <aydenmeng@...h.net>
> Signed-off-by: Ayden Meng <aydenmeng@...h.net>
> Signed-off-by: Mingcong Bai <jeffbai@...c.io>
> [Xi Ruoyao: Fix falling through logic and add kernel log output.]
> Signed-off-by: Xi Ruoyao <xry111@...111.site>
> Link: https://github.com/AOSC-Tracking/linux/commit/4392f441363abdf6fa0a0433d73175a17f493454
> [Ziyao Li: move from drivers/pci/quirks.c to drivers/pci/controller/pci-loongson.c]
> Signed-off-by: Ziyao Li <liziyao@...ontech.com>
> Tested-by: Mingcong Bai <jeffbai@...c.io>
> ---
> Changes in v2:
> - Link to v1: https://lore.kernel.org/r/20250822-loongson-pci1-v1-1-39aabbd11fbd@uniontech.com
> - Move from arch/loongarch/pci/pci.c to drivers/pci/controller/pci-loongson.c
> - Fix falling through logic and add kernel log output by Xi Ruoyao
> ---
> drivers/pci/controller/pci-loongson.c | 39 +++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> index bc630ab8a283..75a1b494b527 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -176,6 +176,45 @@ static void loongson_pci_msi_quirk(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT5, loongson_pci_msi_quirk);
>
> +/*
> + * Older steppings of the Loongson 3C6000 series incorrectly report the
> + * supported link speeds on their PCIe bridges (device IDs 3c19, 3c29) as
> + * only 2.5 GT/s, despite the upstream bus supporting speeds from 2.5 GT/s
> + * up to 16 GT/s.
> + */
> +static void quirk_loongson_pci_bridge_supported_speeds(struct pci_dev *pdev)
> +{
> + u8 supported_speeds = pdev->supported_speeds;
> +
> + switch (pdev->bus->max_bus_speed) {
> + case PCIE_SPEED_16_0GT:
> + supported_speeds |= PCI_EXP_LNKCAP2_SLS_16_0GB;
I'd have named the variable old_supported_speeds and adjusted
pdev->supported_speeds here directly (I actually assumed it is what
you're doing here and it took a while for me to even notice you only write
the changes back later, I had to remove a few incorrect review comments
written on basis of that wrong assumption :-)).
> + fallthrough;
> + case PCIE_SPEED_8_0GT:
> + supported_speeds |= PCI_EXP_LNKCAP2_SLS_8_0GB;
> + fallthrough;
> + case PCIE_SPEED_5_0GT:
> + supported_speeds |= PCI_EXP_LNKCAP2_SLS_5_0GB;
> + fallthrough;
> + case PCIE_SPEED_2_5GT:
> + supported_speeds |= PCI_EXP_LNKCAP2_SLS_2_5GB;
> + break;
> + default:
> + pci_warn(pdev, "unexpected max bus speed");
Missing \n
> + return;
> + }
> +
> + if (supported_speeds != pdev->supported_speeds) {
> + pci_info(pdev, "fixing up supported link speeds: 0x%x => 0x%x",
Missing \n
> + pdev->supported_speeds, supported_speeds);
> + pdev->supported_speeds = supported_speeds;
> + }
> +}
--
i.
Powered by blists - more mailing lists