On Tue, Oct 30, 2018 at 12:36:08PM -0400, Mikulas Patocka wrote: > The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask > the register and compare it against them. > > This patch fixes errors "amdgpu: [powerplay] failed to send message 261 > ret is 0" errors when PCIe-v3 card is plugged into PCIe-v1 slot, because > the slot is being incorrectly reported as PCIe-v3 capable. > > Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx> Applied the patch below to for-linus for v4.20 and backport to v4.17+ stable kernels, thanks! I removed the "if (lnkcap)" test because the LNKCAP case is not parallel to the LNKCAP2 case. LNKCAP2 is optional so we have to test for lnkcap2 being non-zero. But LNKCAP is required for all devices. If the config read of it fails, we should get either 0 or ~0, and neither one will satisfy the 5.0 or 2.5 checks. > Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported link speed") > Cc: stable@xxxxxxxxxxxxxxx # v4.17+ > > --- > drivers/pci/pci.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > Index: linux-4.19/drivers/pci/pci.c > =================================================================== > --- linux-4.19.orig/drivers/pci/pci.c 2018-10-30 16:58:58.000000000 +0100 > +++ linux-4.19/drivers/pci/pci.c 2018-10-30 16:58:58.000000000 +0100 > @@ -5492,13 +5492,13 @@ enum pci_bus_speed pcie_get_speed_cap(st > > pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap); > if (lnkcap) { > - if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB) > + if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_16_0GB) > return PCIE_SPEED_16_0GT; > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB) > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_8_0GB) > return PCIE_SPEED_8_0GT; > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB) > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) ==PCI_EXP_LNKCAP_SLS_5_0GB) > return PCIE_SPEED_5_0GT; > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB) > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB) > return PCIE_SPEED_2_5GT; > } > commit 94ea01a6d9a6 Author: Mikulas Patocka <mpatocka@xxxxxxxxxx> Date: Mon Nov 26 10:37:13 2018 -0600 PCI: Fix incorrect value returned from pcie_get_speed_cap() The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask the register and compare it against them. This fixes errors like this: amdgpu: [powerplay] failed to send message 261 ret is 0 when PCIe-v3 card is plugged into a PCIe-v1 slot, because the slot is being incorrectly reported as PCIe-v3 capable. 6cf57be0f78e, which appeared in v4.17, added pcie_get_speed_cap() with the incorrect test of PCI_EXP_LNKCAP_SLS as a bitmask. 5d9a63304032, which appeared in v4.19, changed amdgpu to use pcie_get_speed_cap(), so the amdgpu bug reports below are regressions in v4.19. Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported link speed") Fixes: 5d9a63304032 ("drm/amdgpu: use pcie functions for link width and speed") Link: https://bugs.freedesktop.org/show_bug.cgi?id=108704 Link: https://bugs.freedesktop.org/show_bug.cgi?id=108778 Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx> [bhelgaas: update comment, remove use of PCI_EXP_LNKCAP_SLS_8_0GB and PCI_EXP_LNKCAP_SLS_16_0GB since those should be covered by PCI_EXP_LNKCAP2, remove test of PCI_EXP_LNKCAP for zero, since that register is required] Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> Acked-by: Alex Deucher <alexander.deucher@xxxxxxx> Cc: stable@xxxxxxxxxxxxxxx # v4.17+ diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index d068f11d08a7..c9d8e3c837de 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5556,9 +5556,13 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev) u32 lnkcap2, lnkcap; /* - * PCIe r4.0 sec 7.5.3.18 recommends using the Supported Link - * Speeds Vector in Link Capabilities 2 when supported, falling - * back to Max Link Speed in Link Capabilities otherwise. + * Link Capabilities 2 was added in PCIe r3.0, sec 7.8.18. The + * implementation note there recommends using the Supported Link + * Speeds Vector in Link Capabilities 2 when supported. + * + * Without Link Capabilities 2, i.e., prior to PCIe r3.0, software + * should use the Supported Link Speeds field in Link Capabilities, + * where only 2.5 GT/s and 5.0 GT/s speeds were defined. */ pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2); if (lnkcap2) { /* PCIe r3.0-compliant */ @@ -5574,16 +5578,10 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev) } pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap); - if (lnkcap) { - if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB) - return PCIE_SPEED_16_0GT; - else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB) - return PCIE_SPEED_8_0GT; - else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB) - return PCIE_SPEED_5_0GT; - else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB) - return PCIE_SPEED_2_5GT; - } + if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB) + return PCIE_SPEED_5_0GT; + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB) + return PCIE_SPEED_2_5GT; return PCI_SPEED_UNKNOWN; } _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx