From: Lucas Kannebley Tavares lucaskt@vnet.linux.ibm.com
This patch series does: 1. max_bus_speed is used to set the device to gen2 speeds 2. on power there's no longer a conflict between the pseries call and other architectures, because the overwrite is done via a ppc_md hook 3. radeon is using bus->max_bus_speed instead of drm_pcie_get_speed_cap_mask for gen2 capability detection
And I've also added the changes proposed by Michael Ellerman: 1. Corrected Patch 1's comments 2. Moved forward function declarations to pseries.h header 3. Added forward references to struct pci_host_bridge, preventing compilation fails.
The first patch consists of some architecture changes, such as adding a hook on powerpc for pci_root_bridge_prepare, so that pseries will initialize it to a function, while all other architectures get a NULL pointer. So that whenever whenever pci_create_root_bus is called, we'll get max_bus_speed properly setup from OpenFirmware.
The second patch consists of simple radeon changes not to call drm_get_pcie_speed_cap_mask anymore. I assume that on x86 machines, the max_bus_speed property will be properly set already.
Lucas Kannebley Tavares (2): ppc64: perform proper max_bus_speed detection radeon: use max_bus_speed to activate gen2 speeds
arch/powerpc/include/asm/machdep.h | 2 ++ arch/powerpc/kernel/pci-common.c | 8 +++++ arch/powerpc/platforms/pseries/pci.c | 51 ++++++++++++++++++++++++++++++++ arch/powerpc/platforms/pseries/pseries.h | 4 +++ arch/powerpc/platforms/pseries/setup.c | 2 ++ drivers/gpu/drm/radeon/evergreen.c | 10 ++----- drivers/gpu/drm/radeon/r600.c | 9 ++---- drivers/gpu/drm/radeon/rv770.c | 9 ++---- 8 files changed, 74 insertions(+), 21 deletions(-)
From: Lucas Kannebley Tavares lucaskt@linux.vnet.ibm.com
On pseries machines the detection for max_bus_speed should be done through an OpenFirmware property. This patch adds a function to perform this detection and a hook to perform dynamic adding of the function only for pseries. This is done by overwriting the weak pcibios_root_bridge_prepare function which is called by pci_create_root_bus().
Signed-off-by: Lucas Kannebley Tavares lucaskt@linux.vnet.ibm.com --- arch/powerpc/include/asm/machdep.h | 2 ++ arch/powerpc/kernel/pci-common.c | 8 +++++ arch/powerpc/platforms/pseries/pci.c | 51 ++++++++++++++++++++++++++++++++ arch/powerpc/platforms/pseries/pseries.h | 4 +++ arch/powerpc/platforms/pseries/setup.c | 2 ++ 5 files changed, 67 insertions(+)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 3d6b410..8f558bf 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -107,6 +107,8 @@ struct machdep_calls { void (*pcibios_fixup)(void); int (*pci_probe_mode)(struct pci_bus *); void (*pci_irq_fixup)(struct pci_dev *dev); + int (*pcibios_root_bridge_prepare)(struct pci_host_bridge + *bridge);
/* To setup PHBs when using automatic OF platform driver for PCI */ int (*pci_setup_phb)(struct pci_controller *host); diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index fa12ae4..80986cf 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -844,6 +844,14 @@ int pci_proc_domain(struct pci_bus *bus) return 1; }
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{ + if (ppc_md.pcibios_root_bridge_prepare) + return ppc_md.pcibios_root_bridge_prepare(bridge); + + return 0; +} + /* This header fixup will do the resource fixup for all devices as they are * probed, but not for bridge ranges */ diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..7f9c956 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105, fixup_winbond_82c105); + +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) +{ + struct device_node *dn, *pdn; + struct pci_bus *bus; + const uint32_t *pcie_link_speed_stats; + + bus = bridge->bus; + + dn = pcibios_get_phb_of_node(bus); + if (!dn) + return 0; + + for (pdn = dn; pdn != NULL; pdn = pdn->parent) { + pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, + "ibm,pcie-link-speed-stats", NULL); + if (pcie_link_speed_stats) + break; + } + + if (!pcie_link_speed_stats) { + pr_err("no ibm,pcie-link-speed-stats property\n"); + return 0; + } + + switch (pcie_link_speed_stats[0]) { + case 0x01: + bus->max_bus_speed = PCIE_SPEED_2_5GT; + break; + case 0x02: + bus->max_bus_speed = PCIE_SPEED_5_0GT; + break; + default: + bus->max_bus_speed = PCI_SPEED_UNKNOWN; + break; + } + + switch (pcie_link_speed_stats[1]) { + case 0x01: + bus->cur_bus_speed = PCIE_SPEED_2_5GT; + break; + case 0x02: + bus->cur_bus_speed = PCIE_SPEED_5_0GT; + break; + default: + bus->cur_bus_speed = PCI_SPEED_UNKNOWN; + break; + } + + return 0; +} diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h index 9a3dda0..b79393d 100644 --- a/arch/powerpc/platforms/pseries/pseries.h +++ b/arch/powerpc/platforms/pseries/pseries.h @@ -60,4 +60,8 @@ extern int dlpar_detach_node(struct device_node *); /* Snooze Delay, pseries_idle */ DECLARE_PER_CPU(long, smt_snooze_delay);
+/* PCI root bridge prepare function override for pseries */ +struct pci_host_bridge; +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); + #endif /* _PSERIES_PSERIES_H */ diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 8bcc9ca..bf34cc9 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -466,6 +466,8 @@ static void __init pSeries_setup_arch(void) else ppc_md.enable_pmcs = power4_enable_pmcs;
+ ppc_md.pcibios_root_bridge_prepare = pseries_root_bridge_prepare; + if (firmware_has_feature(FW_FEATURE_SET_MODE)) { long rc; if ((rc = pSeries_enable_reloc_on_exc()) != H_SUCCESS) {
On Wed, Apr 24, 2013 at 07:54:49PM -0300, lucaskt@linux.vnet.ibm.com wrote:
From: Lucas Kannebley Tavares lucaskt@linux.vnet.ibm.com
On pseries machines the detection for max_bus_speed should be done through an OpenFirmware property. This patch adds a function to perform this detection and a hook to perform dynamic adding of the function only for pseries. This is done by overwriting the weak pcibios_root_bridge_prepare function which is called by pci_create_root_bus().
Signed-off-by: Lucas Kannebley Tavares lucaskt@linux.vnet.ibm.com
arch/powerpc/include/asm/machdep.h | 2 ++ arch/powerpc/kernel/pci-common.c | 8 +++++ arch/powerpc/platforms/pseries/pci.c | 51 ++++++++++++++++++++++++++++++++ arch/powerpc/platforms/pseries/pseries.h | 4 +++ arch/powerpc/platforms/pseries/setup.c | 2 ++ 5 files changed, 67 insertions(+)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 3d6b410..8f558bf 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -107,6 +107,8 @@ struct machdep_calls { void (*pcibios_fixup)(void); int (*pci_probe_mode)(struct pci_bus *); void (*pci_irq_fixup)(struct pci_dev *dev);
int (*pcibios_root_bridge_prepare)(struct pci_host_bridge
*bridge);/* To setup PHBs when using automatic OF platform driver for PCI */ int (*pci_setup_phb)(struct pci_controller *host);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index fa12ae4..80986cf 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -844,6 +844,14 @@ int pci_proc_domain(struct pci_bus *bus) return 1; }
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- if (ppc_md.pcibios_root_bridge_prepare)
return ppc_md.pcibios_root_bridge_prepare(bridge);- return 0;
+}
/* This header fixup will do the resource fixup for all devices as they are
- probed, but not for bridge ranges
*/ diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..7f9c956 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105, fixup_winbond_82c105);
+int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct device_node *dn, *pdn;
- struct pci_bus *bus;
- const uint32_t *pcie_link_speed_stats;
- bus = bridge->bus;
- dn = pcibios_get_phb_of_node(bus);
- if (!dn)
return 0;- for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
pcie_link_speed_stats = (const uint32_t *) of_get_property(dn,"ibm,pcie-link-speed-stats", NULL);if (pcie_link_speed_stats)break;- }
Please use the helpers in include/linux/of.h rather than open coding this.
Yours Tony
On 04/24/2013 08:48 PM, Tony Breeds wrote:
On Wed, Apr 24, 2013 at 07:54:49PM -0300, lucaskt@linux.vnet.ibm.com wrote:
From: Lucas Kannebley Tavareslucaskt@linux.vnet.ibm.com
On pseries machines the detection for max_bus_speed should be done through an OpenFirmware property. This patch adds a function to perform this detection and a hook to perform dynamic adding of the function only for pseries. This is done by overwriting the weak pcibios_root_bridge_prepare function which is called by pci_create_root_bus().
Signed-off-by: Lucas Kannebley Tavareslucaskt@linux.vnet.ibm.com
arch/powerpc/include/asm/machdep.h | 2 ++ arch/powerpc/kernel/pci-common.c | 8 +++++ arch/powerpc/platforms/pseries/pci.c | 51 ++++++++++++++++++++++++++++++++ arch/powerpc/platforms/pseries/pseries.h | 4 +++ arch/powerpc/platforms/pseries/setup.c | 2 ++ 5 files changed, 67 insertions(+)
diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h index 3d6b410..8f558bf 100644 --- a/arch/powerpc/include/asm/machdep.h +++ b/arch/powerpc/include/asm/machdep.h @@ -107,6 +107,8 @@ struct machdep_calls { void (*pcibios_fixup)(void); int (*pci_probe_mode)(struct pci_bus *); void (*pci_irq_fixup)(struct pci_dev *dev);
int (*pcibios_root_bridge_prepare)(struct pci_host_bridge
*bridge);/* To setup PHBs when using automatic OF platform driver for PCI */ int (*pci_setup_phb)(struct pci_controller *host);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index fa12ae4..80986cf 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -844,6 +844,14 @@ int pci_proc_domain(struct pci_bus *bus) return 1; }
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- if (ppc_md.pcibios_root_bridge_prepare)
return ppc_md.pcibios_root_bridge_prepare(bridge);- return 0;
+}
- /* This header fixup will do the resource fixup for all devices as they are
*/
- probed, but not for bridge ranges
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..7f9c956 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105, fixup_winbond_82c105);
+int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct device_node *dn, *pdn;
- struct pci_bus *bus;
- const uint32_t *pcie_link_speed_stats;
- bus = bridge->bus;
- dn = pcibios_get_phb_of_node(bus);
- if (!dn)
return 0;- for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
pcie_link_speed_stats = (const uint32_t *) of_get_property(dn,"ibm,pcie-link-speed-stats", NULL);if (pcie_link_speed_stats)break;- }
Please use the helpers in include/linux/of.h rather than open coding this.
Yours Tony
Hi Tony,
This is what I can find as an equivalent code:
for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, "ibm,pcie-link-speed-stats", NULL); if (pcie_link_speed_stats) break; }
is this your suggestion, or was it another approach that will have the same result?
Thanks,
On 04/25/2013 02:34 PM, Lucas Kannebley Tavares wrote:
On 04/24/2013 08:48 PM, Tony Breeds wrote:
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..7f9c956 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev) } DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105, fixup_winbond_82c105);
+int pseries_root_bridge_prepare(struct pci_host_bridge *bridge) +{
- struct device_node *dn, *pdn;
- struct pci_bus *bus;
- const uint32_t *pcie_link_speed_stats;
- bus = bridge->bus;
- dn = pcibios_get_phb_of_node(bus);
- if (!dn)
return 0;- for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
pcie_link_speed_stats = (const uint32_t *) of_get_property(dn,"ibm,pcie-link-speed-stats", NULL);if (pcie_link_speed_stats)break;- }
Please use the helpers in include/linux/of.h rather than open coding this.
Yours Tony
Hi Tony,
This is what I can find as an equivalent code:
for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { pcie_link_speed_stats = (const uint32_t *) of_get_property(dn, "ibm,pcie-link-speed-stats", NULL); if (pcie_link_speed_stats) break; }is this your suggestion, or was it another approach that will have the same result?
Thanks,
Hi Tony,
It seems Lucas' change is a bit incomplete and is not handling the reference counter to the device_node correctly. Is the following change what you had in mind?
dn = pcibios_get_phb_of_node(bus); if (!dn) return 0;
for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, "ibm,pcie-link-speed-stats", NULL); if (pcie_link_speed_stats) break; }
of_node_put(pdn);
Thanks,
On Thu, May 02, 2013 at 12:21:37PM -0300, Kleber Sacilotto de Souza wrote:
Hi Tony,
It seems Lucas' change is a bit incomplete and is not handling the reference counter to the device_node correctly. Is the following change what you had in mind?
Ahh Sorry I expected there would be a for_each_parent_of_node macro. I did a quick grep and it seems that's not very common, so open coding it should be fine.
dn = pcibios_get_phb_of_node(bus); if (!dn) return 0;
for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, "ibm,pcie-link-speed-stats", NULL); if (pcie_link_speed_stats) break; }
of_node_put(pdn);
I think you need the of_node_put() in the body of the loop, otherwise aren't you leaking refcounts?
Yours Tony
On 05/03/2013 03:40 AM, Tony Breeds wrote:
On Thu, May 02, 2013 at 12:21:37PM -0300, Kleber Sacilotto de Souza wrote:
Hi Tony,
It seems Lucas' change is a bit incomplete and is not handling the reference counter to the device_node correctly. Is the following change what you had in mind?
Ahh Sorry I expected there would be a for_each_parent_of_node macro. I did a quick grep and it seems that's not very common, so open coding it should be fine.
dn = pcibios_get_phb_of_node(bus); if (!dn) return 0;
for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) { pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, "ibm,pcie-link-speed-stats", NULL); if (pcie_link_speed_stats) break; }
of_node_put(pdn);
I think you need the of_node_put() in the body of the loop, otherwise aren't you leaking refcounts?
of_get_next_parent() takes care of that. It does of_node_put() on the current node after doing of_node_get() on the parent.
Thanks,
From: Lucas Kannebley Tavares lucaskt@linux.vnet.ibm.com
radeon currently uses a drm function to get the speed capabilities for the bus, drm_pcie_get_speed_cap_mask. However, this is a non-standard method of performing this detection and this patch changes it to use the max_bus_speed attribute.
Signed-off-by: Lucas Kannebley Tavares lucaskt@linux.vnet.ibm.com --- drivers/gpu/drm/radeon/evergreen.c | 10 +++------- drivers/gpu/drm/radeon/r600.c | 9 ++------- drivers/gpu/drm/radeon/rv770.c | 9 ++------- 3 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..ee45026 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -3855,8 +3855,7 @@ void evergreen_fini(struct radeon_device *rdev)
void evergreen_pcie_gen2_enable(struct radeon_device *rdev) { - u32 link_width_cntl, speed_cntl, mask; - int ret; + u32 link_width_cntl, speed_cntl;
if (radeon_pcie_gen2 == 0) return; @@ -3871,11 +3870,8 @@ void evergreen_pcie_gen2_enable(struct radeon_device *rdev) if (ASIC_IS_X2(rdev)) return;
- ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask); - if (ret != 0) - return; - - if (!(mask & DRM_PCIE_SPEED_50)) + if ((rdev->pdev->bus->max_bus_speed != PCIE_SPEED_5_0GT) && + (rdev->pdev->bus->max_bus_speed != PCIE_SPEED_8_0GT)) return;
speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 0740db3..4d5ba32 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -4351,8 +4351,6 @@ static void r600_pcie_gen2_enable(struct radeon_device *rdev) { u32 link_width_cntl, lanes, speed_cntl, training_cntl, tmp; u16 link_cntl2; - u32 mask; - int ret;
if (radeon_pcie_gen2 == 0) return; @@ -4371,11 +4369,8 @@ static void r600_pcie_gen2_enable(struct radeon_device *rdev) if (rdev->family <= CHIP_R600) return;
- ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask); - if (ret != 0) - return; - - if (!(mask & DRM_PCIE_SPEED_50)) + if ((rdev->pdev->bus->max_bus_speed != PCIE_SPEED_5_0GT) && + (rdev->pdev->bus->max_bus_speed != PCIE_SPEED_8_0GT)) return;
speed_cntl = RREG32_PCIE_P(PCIE_LC_SPEED_CNTL); diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c index d63fe1d..f4860f6 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -1238,8 +1238,6 @@ static void rv770_pcie_gen2_enable(struct radeon_device *rdev) { u32 link_width_cntl, lanes, speed_cntl, tmp; u16 link_cntl2; - u32 mask; - int ret;
if (radeon_pcie_gen2 == 0) return; @@ -1254,11 +1252,8 @@ static void rv770_pcie_gen2_enable(struct radeon_device *rdev) if (ASIC_IS_X2(rdev)) return;
- ret = drm_pcie_get_speed_cap_mask(rdev->ddev, &mask); - if (ret != 0) - return; - - if (!(mask & DRM_PCIE_SPEED_50)) + if ((rdev->pdev->bus->max_bus_speed != PCIE_SPEED_5_0GT) && + (rdev->pdev->bus->max_bus_speed != PCIE_SPEED_8_0GT)) return;
DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n");
dri-devel@lists.freedesktop.org
-
Kleber Sacilotto de Souza -
Lucas Kannebley Tavares -
lucaskt＠linux.vnet.ibm.com
-
Tony Breeds