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 <mpato...@redhat.com>

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: sta...@vger.kernel.org    # 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 <mpato...@redhat.com>
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 <mpato...@redhat.com>
    [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 <bhelg...@google.com>
    Acked-by: Alex Deucher <alexander.deuc...@amd.com>
    Cc: sta...@vger.kernel.org      # 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to