These are minor cleanups for drm_pcie_get_speed_cap_mask() to use standard #defines and PCIe capability accessors. They depend on a pci_regs.h change (130f1b8f35) that appeared in v3.8-rc2.
They don't address the issue of DRM devices directly below a host bridge that doesn't appear as a PCI device (the issue Lucas has been working on).
I'm a little skeptical about the premise of drm_pcie_get_speed_cap_mask() to begin with. Link speed seems like something fairly generic that should be handled in the core, not in individual drivers. Sec 6.11, "Link Speed Management", in the PCIe 3.0 spec seems relevant and suggests that the hardware should automatically use the highest speed supported by both ends of the link unless software sets a lower maximum via Target Link Speed.
But I can't match up the code, e.g., evergreen_pcie_gen2_enable(), to anything in the generic PCIe specs, so maybe this driver code is essentially quirks for misbehaving hardware?
---
Bjorn Helgaas (3): drm/pci: Use the standard #defines for PCIe Link Capability bits drm/pci: Set all supported speeds in speed cap mask for pre-3.0 devices drm/pci: Use PCI Express Capability accessors
drivers/gpu/drm/drm_pci.c | 27 ++++++++------------------- 1 files changed, 8 insertions(+), 19 deletions(-)
Use the standard #defines rather than bare numbers for the PCIe Link Capabilities speed bits.
Signed-off-by: Bjorn Helgaas bhelgaas@google.com --- drivers/gpu/drm/drm_pci.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 754bc96..11c8add 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -504,9 +504,9 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB) *mask |= DRM_PCIE_SPEED_80; } else { - if (lnkcap & 1) + if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB) *mask |= DRM_PCIE_SPEED_25; - if (lnkcap & 2) + if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB) *mask |= DRM_PCIE_SPEED_50; }
For devices that conform to PCIe r3.0 and have a Link Capabilities 2 register, we test and report every bit in the Supported Link Speeds Vector field. For a device that supports both 2.5GT/s and 5.0GT/s, we set both DRM_PCIE_SPEED_25 and DRM_PCIE_SPEED_50 in the returned mask.
For pre-r3.0 devices, the Link Capabilities 0010b encoding (PCI_EXP_LNKCAP_SLS_5_0GB) means that both 5.0GT/s and 2.5GT/s are supported, so set both DRM_PCIE_SPEED_25 and DRM_PCIE_SPEED_50 in this case as well.
Signed-off-by: Bjorn Helgaas bhelgaas@google.com --- drivers/gpu/drm/drm_pci.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 11c8add..50e26f2 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -507,7 +507,7 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB) *mask |= DRM_PCIE_SPEED_25; if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB) - *mask |= DRM_PCIE_SPEED_50; + *mask |= (DRM_PCIE_SPEED_25 | DRM_PCIE_SPEED_50); }
DRM_INFO("probing gen 2 caps for device %x:%x = %x/%x\n", root->vendor, root->device, lnkcap, lnkcap2);
Use PCI Express Capability access functions to simplify this code a bit. For non-PCIe devices or pre-PCIe 3.0 devices that don't implement the Link Capabilities 2 register, pcie_capability_read_dword() reads a zero.
Since we're only testing whether the bits we care about are set, there's no need to mask out the other bits we *don't* care about.
Signed-off-by: Bjorn Helgaas bhelgaas@google.com --- drivers/gpu/drm/drm_pci.c | 21 +++++---------------- 1 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 50e26f2..86102a0 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -469,41 +469,30 @@ EXPORT_SYMBOL(drm_pci_exit); int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) { struct pci_dev *root; - int pos; - u32 lnkcap = 0, lnkcap2 = 0; + u32 lnkcap, lnkcap2;
*mask = 0; if (!dev->pdev) return -EINVAL;
- if (!pci_is_pcie(dev->pdev)) - return -EINVAL; - root = dev->pdev->bus->self;
- pos = pci_pcie_cap(root); - if (!pos) - return -EINVAL; - /* we've been informed via and serverworks don't make the cut */ if (root->vendor == PCI_VENDOR_ID_VIA || root->vendor == PCI_VENDOR_ID_SERVERWORKS) return -EINVAL;
- pci_read_config_dword(root, pos + PCI_EXP_LNKCAP, &lnkcap); - pci_read_config_dword(root, pos + PCI_EXP_LNKCAP2, &lnkcap2); - - lnkcap &= PCI_EXP_LNKCAP_SLS; - lnkcap2 &= 0xfe; + pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap); + pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2);
- if (lnkcap2) { /* PCIE GEN 3.0 */ + if (lnkcap2) { /* PCIe r3.0-compliant */ if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB) *mask |= DRM_PCIE_SPEED_25; if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB) *mask |= DRM_PCIE_SPEED_50; if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB) *mask |= DRM_PCIE_SPEED_80; - } else { + } else { /* pre-r3.0 */ if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB) *mask |= DRM_PCIE_SPEED_25; if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
On Fri, Jan 4, 2013 at 2:10 PM, Bjorn Helgaas bhelgaas@google.com wrote:
These are minor cleanups for drm_pcie_get_speed_cap_mask() to use standard #defines and PCIe capability accessors. They depend on a pci_regs.h change (130f1b8f35) that appeared in v3.8-rc2.
They don't address the issue of DRM devices directly below a host bridge that doesn't appear as a PCI device (the issue Lucas has been working on).
I'm a little skeptical about the premise of drm_pcie_get_speed_cap_mask() to begin with. Link speed seems like something fairly generic that should be handled in the core, not in individual drivers. Sec 6.11, "Link Speed Management", in the PCIe 3.0 spec seems relevant and suggests that the hardware should automatically use the highest speed supported by both ends of the link unless software sets a lower maximum via Target Link Speed.
But I can't match up the code, e.g., evergreen_pcie_gen2_enable(), to anything in the generic PCIe specs, so maybe this driver code is essentially quirks for misbehaving hardware?
At least for radeon, there is an asic specific sequence required to change the PCIE gen link speed at runtime. Depending on the bios, the board may come up in the highest mode supported by either side or something lower. If it comes up at a lower speed than the hardware is capable of, we can increase it in the driver to improve performance. Additionally, you can select a lower link speed to save power. I don't know if there is a generic non-asic specific way to change the link speed of a device at runtime, but I'm not really a PCI expert.
Alex
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Bjorn Helgaas