[PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
On 04/24/2013 08:48 PM, Tony Breeds wrote: > On Wed, Apr 24, 2013 at 07:54:49PM -0300, lucaskt at linux.vnet.ibm.com wrote: >> From: Lucas Kannebley Tavares >> >> 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 >> --- >> 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, -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
Re: [PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
On 04/24/2013 08:48 PM, Tony Breeds wrote: On Wed, Apr 24, 2013 at 07:54:49PM -0300, luca...@linux.vnet.ibm.com wrote: From: Lucas Kannebley Tavares 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 --- 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, -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 08:42 AM, Michael Ellerman wrote: > On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: >> 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 fails to build for me on ppc64_defconfig, with: > > arch/powerpc/include/asm/machdep.h:111:5: error: 'struct pci_host_bridge' > declared inside parameter list [-Werror] > > > Presumably you tested it using some other defconfig? > > cheers > Yes, I tested with another config, I did get warnings, though, so I should've fixed that earlier. Adding a forward declaration to prevent this from even throwing out warnings. -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 02:00 AM, Michael Ellerman wrote: > On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: >> 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. > > The crucial detail you didn't mention is that pcibios_root_bridge_prepare() > already exists as a weak function in the PCI code and is called from > pci_create_root_bus(). Adding this comment. > >> diff --git a/arch/powerpc/platforms/pseries/setup.c >> b/arch/powerpc/platforms/pseries/setup.c >> index 8bcc9ca..15796b5 100644 >> --- a/arch/powerpc/platforms/pseries/setup.c >> +++ b/arch/powerpc/platforms/pseries/setup.c >> @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image) >> } >> #endif >> >> +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); >> + > > Don't do that, put it in a header where it belongs. > And done as well. > cheers > -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: > On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares > wrote: >> radeon currently uses a drm function to get the speed capabilities for >> the bus. However, this is a non-standard method of performing this >> detection and this patch changes it to use the max_bus_speed attribute. >> --- >> drivers/gpu/drm/radeon/evergreen.c |9 ++--- >> drivers/gpu/drm/radeon/r600.c |8 +--- >> drivers/gpu/drm/radeon/rv770.c |8 +--- >> 3 files changed, 4 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/evergreen.c >> b/drivers/gpu/drm/radeon/evergreen.c >> index 305a657..3291f62 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,7 @@ 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) > > For devices on a root bus, we previously dereferenced a NULL pointer > in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a > root bus. (I think this is the original problem you tripped over, > Lucas.) > > These patches fix that problem. On pseries, where the device *is* on > a root bus, your patches set max_bus_speed so this will work as > expected. On most other systems, max_bus_speed for root buses will be > PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because > most arches don't have code like the pseries code you're adding). > > PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on > the root bus, we'll attempt to enable Gen2 on the device even though > we have no idea what the bus will support. > > That's why I originally suggested skipping the Gen2 stuff if > "max_bus_speed == PCI_SPEED_UNKNOWN". I was just being conservative, > thinking that it's better to have a functional but slow GPU rather > than the unknown (to me) effects of enabling Gen2 on a link that might > not support it. But I'm fine with this being either way. You're right here, of course. I'll test for it being different from 5_0GT and 8_0GT instead. Though at some point I suppose someone will want to tackle Gen3 speeds. > > It would be nice if we could get rid of drm_pcie_get_speed_cap_mask() > altogether. It is exported, but I have no idea of anybody else uses > it. Maybe it could at least be marked __deprecated now? > I'll mark it. > I don't know who should take these patches. They don't touch > drivers/pci, but I'd be happy to push them, given the appropriate ACKs > from DRM and powerpc folks. > > Bjorn > >> 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..64b90c0 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,7 @@ 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) >> 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 d63fe1
Re: [PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 08:42 AM, Michael Ellerman wrote: On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: 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 fails to build for me on ppc64_defconfig, with: arch/powerpc/include/asm/machdep.h:111:5: error: 'struct pci_host_bridge' declared inside parameter list [-Werror] Presumably you tested it using some other defconfig? cheers Yes, I tested with another config, I did get warnings, though, so I should've fixed that earlier. Adding a forward declaration to prevent this from even throwing out warnings. -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
On 04/15/2013 02:00 AM, Michael Ellerman wrote: On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote: 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. The crucial detail you didn't mention is that pcibios_root_bridge_prepare() already exists as a weak function in the PCI code and is called from pci_create_root_bus(). Adding this comment. diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c index 8bcc9ca..15796b5 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image) } #endif +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); + Don't do that, put it in a header where it belongs. And done as well. cheers -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
On 04/12/2013 01:38 PM, Bjorn Helgaas wrote: On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares wrote: radeon currently uses a drm function to get the speed capabilities for the bus. However, this is a non-standard method of performing this detection and this patch changes it to use the max_bus_speed attribute. --- drivers/gpu/drm/radeon/evergreen.c |9 ++--- drivers/gpu/drm/radeon/r600.c |8 +--- drivers/gpu/drm/radeon/rv770.c |8 +--- 3 files changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..3291f62 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,7 @@ 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) For devices on a root bus, we previously dereferenced a NULL pointer in drm_pcie_get_speed_cap_mask() because pdev->bus->self is NULL on a root bus. (I think this is the original problem you tripped over, Lucas.) These patches fix that problem. On pseries, where the device *is* on a root bus, your patches set max_bus_speed so this will work as expected. On most other systems, max_bus_speed for root buses will be PCI_SPEED_UNKNOWN (set in pci_alloc_bus() and never updated because most arches don't have code like the pseries code you're adding). PCI_SPEED_UNKNOWN = 0xff, so if we see another machine with a GPU on the root bus, we'll attempt to enable Gen2 on the device even though we have no idea what the bus will support. That's why I originally suggested skipping the Gen2 stuff if "max_bus_speed == PCI_SPEED_UNKNOWN". I was just being conservative, thinking that it's better to have a functional but slow GPU rather than the unknown (to me) effects of enabling Gen2 on a link that might not support it. But I'm fine with this being either way. You're right here, of course. I'll test for it being different from 5_0GT and 8_0GT instead. Though at some point I suppose someone will want to tackle Gen3 speeds. It would be nice if we could get rid of drm_pcie_get_speed_cap_mask() altogether. It is exported, but I have no idea of anybody else uses it. Maybe it could at least be marked __deprecated now? I'll mark it. I don't know who should take these patches. They don't touch drivers/pci, but I'd be happy to push them, given the appropriate ACKs from DRM and powerpc folks. Bjorn 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..64b90c0 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,7 @@ 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) 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..c683c36 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,7 @@ 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) return;
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
radeon currently uses a drm function to get the speed capabilities for the bus. However, this is a non-standard method of performing this detection and this patch changes it to use the max_bus_speed attribute. --- drivers/gpu/drm/radeon/evergreen.c |9 ++--- drivers/gpu/drm/radeon/r600.c |8 +--- drivers/gpu/drm/radeon/rv770.c |8 +--- 3 files changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..3291f62 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,7 @@ 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) 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..64b90c0 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,7 @@ 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) 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..c683c36 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,7 @@ 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) return; DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); -- 1.7.4.4
[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
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. Signed-off-by: Lucas Kannebley Tavares --- 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/setup.c |4 ++ 4 files changed, 65 insertions(+), 0 deletions(-) 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/setup.c b/arch/powerpc/platforms/pseries/setup.c index 8bcc9ca..15796b5 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image) } #endif +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); + static void __init pSeries_setup_arch(void) { panic_timeout = 10; @@ -466,6 +468,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) { -- 1.7.4.4
[PATCHv3 0/2] Speed Cap fixes for ppc64
After all the comments in the last patch series, I did a refactoring of what I was proposing and came up with this. Basically, now: 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 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/setup.c |4 ++ drivers/gpu/drm/radeon/evergreen.c |9 + drivers/gpu/drm/radeon/r600.c |8 + drivers/gpu/drm/radeon/rv770.c |8 + 7 files changed, 69 insertions(+), 21 deletions(-) -- 1.7.4.4
[PATCHv3 2/2] radeon: use max_bus_speed to activate gen2 speeds
radeon currently uses a drm function to get the speed capabilities for the bus. However, this is a non-standard method of performing this detection and this patch changes it to use the max_bus_speed attribute. --- drivers/gpu/drm/radeon/evergreen.c |9 ++--- drivers/gpu/drm/radeon/r600.c |8 +--- drivers/gpu/drm/radeon/rv770.c |8 +--- 3 files changed, 4 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..3291f62 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,7 @@ 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) 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..64b90c0 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,7 @@ 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) 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..c683c36 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,7 @@ 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) return; DRM_INFO("enabling PCIE gen 2 link speeds, disable with radeon.pcie_gen2=0\n"); -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3 0/2] Speed Cap fixes for ppc64
After all the comments in the last patch series, I did a refactoring of what I was proposing and came up with this. Basically, now: 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 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/setup.c |4 ++ drivers/gpu/drm/radeon/evergreen.c |9 + drivers/gpu/drm/radeon/r600.c |8 + drivers/gpu/drm/radeon/rv770.c |8 + 7 files changed, 69 insertions(+), 21 deletions(-) -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
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. Signed-off-by: Lucas Kannebley Tavares --- 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/setup.c |4 ++ 4 files changed, 65 insertions(+), 0 deletions(-) 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/setup.c b/arch/powerpc/platforms/pseries/setup.c index 8bcc9ca..15796b5 100644 --- a/arch/powerpc/platforms/pseries/setup.c +++ b/arch/powerpc/platforms/pseries/setup.c @@ -430,6 +430,8 @@ static void pSeries_machine_kexec(struct kimage *image) } #endif +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge); + static void __init pSeries_setup_arch(void) { panic_timeout = 10; @@ -466,6 +468,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) { -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 3/3] ppc64: implemented pcibios_get_speed_cap_mask
Implementation of a architecture-specific pcibios_get_speed_cap_mask. This implementation detects bus capabilities based on OF ibm,pcie-link-speed-stats property. Signed-off-by: Lucas Kannebley Tavares --- arch/powerpc/platforms/pseries/pci.c | 35 ++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..4da8deb 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -108,3 +109,37 @@ 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 pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask) +{ + struct device_node *dn, *pdn; + const uint32_t *pcie_link_speed_stats = NULL; + + *mask = 0; + dn = pci_bus_to_OF_node(dev->bus); + + /* Find nearest ibm,pcie-link-speed-stats, walking up the device tree */ + for (pdn = dn; pdn != NULL; pdn = pdn->parent) { + pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, + "ibm,pcie-link-speed-stats", NULL); + if (pcie_link_speed_stats != NULL) + break; + } + + if (pcie_link_speed_stats == NULL) { + dev_info(&dev->dev, "no ibm,pcie-link-speed-stats property\n"); + return -EINVAL; + } + + switch (pcie_link_speed_stats[0]) { + case 0x02: + *mask |= PCIE_SPEED_50; + case 0x01: + *mask |= PCIE_SPEED_25; + case 0x00: + default: + return -EINVAL; + } + + return 0; +} -- 1.7.4.4
[PATCHv2 2/3] drm: removed drm_pcie_get_speed_cap_mask function
This function was moved to the pci subsystem where it fits better, as this is much more of a generic pci task, than a drm specific one. All references to the function (all in the radeon driver) are updated. This is the second step in moving function drm_pcie_get_speed_cap_mask from the drm subsystem to the pci one. Signed-off-by: Lucas Kannebley Tavares --- drivers/gpu/drm/drm_pci.c | 38 drivers/gpu/drm/radeon/evergreen.c |5 ++- drivers/gpu/drm/radeon/r600.c |5 ++- drivers/gpu/drm/radeon/rv770.c |5 ++- include/drm/drmP.h |6 - 5 files changed, 9 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index bd719e9..ba70844 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -439,44 +439,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return 0; } -int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) -{ - struct pci_dev *root; - u32 lnkcap, lnkcap2; - - *mask = 0; - if (!dev->pdev) - return -EINVAL; - - root = dev->pdev->bus->self; - - /* 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; - - pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap); - pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2); - - 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 {/* pre-r3.0 */ - 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_25 | DRM_PCIE_SPEED_50); - } - - DRM_INFO("probing gen 2 caps for device %x:%x = %x/%x\n", root->vendor, root->device, lnkcap, lnkcap2); - return 0; -} -EXPORT_SYMBOL(drm_pcie_get_speed_cap_mask); - #else int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..6ba204d 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "radeon.h" #include "radeon_asic.h" @@ -3871,11 +3872,11 @@ 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); + ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask); if (ret != 0) return; - if (!(mask & DRM_PCIE_SPEED_50)) + if (!(mask & PCIE_SPEED_50)) 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..89a7387 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include "radeon.h" @@ -4371,11 +4372,11 @@ 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); + ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask); if (ret != 0) return; - if (!(mask & DRM_PCIE_SPEED_50)) + if (!(mask & PCIE_SPEED_50)) 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..81c7f1c 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include "radeon.h" #include "radeon_asic.h" @@ -1254,11 +1255,11 @@ 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); + ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask); if (ret != 0) return; - if (!(mask & DRM_PCIE_SPEED_50)) + if (!(mask & PCIE_SPEED_50)) return; DRM_INFO("enablin
[PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function
Added function to gather the speed cap for a device and return a mask to supported speeds. The function is divided into an interface and a weak implementation so that architecture-specific functions can be called. This is the first step in moving function drm_pcie_get_speed_cap_mask from the drm subsystem to the pci one. Signed-off-by: Lucas Kannebley Tavares --- drivers/pci/pci.c | 44 include/linux/pci.h |6 ++ 2 files changed, 50 insertions(+), 0 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b099e00..d94ab79 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3931,6 +3931,50 @@ static int __init pci_setup(char *str) } early_param("pci", pci_setup); +int __weak pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask) +{ + struct pci_dev *root; + u32 lnkcap, lnkcap2; + + *mask = 0; + if (!dev) + return -EINVAL; + + root = dev->bus->self; + + /* 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; + + pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap); + pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2); + + if (lnkcap2) { /* PCIe r3.0-compliant */ + if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB) + *mask |= PCIE_SPEED_25; + if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB) + *mask |= PCIE_SPEED_50; + if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB) + *mask |= PCIE_SPEED_80; + } else {/* pre-r3.0 */ + if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB) + *mask |= PCIE_SPEED_25; + if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB) + *mask |= (PCIE_SPEED_25 | PCIE_SPEED_50); + } + + dev_info(&dev->dev, "probing gen 2 caps for device %x:%x = %x/%x\n", + root->vendor, root->device, lnkcap, lnkcap2); + return 0; +} + +int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *mask) +{ + return pcibios_get_speed_cap_mask(dev, mask); +} +EXPORT_SYMBOL(pcie_get_speed_cap_mask); + EXPORT_SYMBOL(pci_reenable_device); EXPORT_SYMBOL(pci_enable_device_io); EXPORT_SYMBOL(pci_enable_device_mem); diff --git a/include/linux/pci.h b/include/linux/pci.h index 2461033a..24a2f63 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1861,4 +1861,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev) */ struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev); +#define PCIE_SPEED_25 1 +#define PCIE_SPEED_50 2 +#define PCIE_SPEED_80 4 + +extern int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *speed_mask); + #endif /* LINUX_PCI_H */ -- 1.7.4.4
[PATCHv2 0/3] PCI Speed Cap Fixes for ppc64
This patch series first implements a function called pcie_get_speed_cap_mask in the PCI subsystem based off from drm_pcie_get_speed_cap_mask in drm. Then it removes the latter and fixes all references to it. And ultimately, it implements an architecture-specific version of the same function for ppc64. The refactor is done because the function that was used by drm is more architecture goo than module-specific. Whilst the function also needed a platform-specific implementation to get PCIE Gen2 speeds on ppc64. Lucas Kannebley Tavares (3): pci: added pcie_get_speed_cap_mask function drm: removed drm_pcie_get_speed_cap_mask function ppc64: implemented pcibios_get_speed_cap_mask arch/powerpc/platforms/pseries/pci.c | 35 +++ drivers/gpu/drm/drm_pci.c| 38 - drivers/gpu/drm/radeon/evergreen.c |5 ++- drivers/gpu/drm/radeon/r600.c|5 ++- drivers/gpu/drm/radeon/rv770.c |5 ++- drivers/pci/pci.c| 44 ++ include/drm/drmP.h |6 include/linux/pci.h |6 8 files changed, 94 insertions(+), 50 deletions(-) -- 1.7.4.4
[PATCHv2 1/3] pci: added pcie_get_speed_cap_mask function
Added function to gather the speed cap for a device and return a mask to supported speeds. The function is divided into an interface and a weak implementation so that architecture-specific functions can be called. This is the first step in moving function drm_pcie_get_speed_cap_mask from the drm subsystem to the pci one. Signed-off-by: Lucas Kannebley Tavares --- drivers/pci/pci.c | 44 include/linux/pci.h |6 ++ 2 files changed, 50 insertions(+), 0 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index b099e00..d94ab79 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3931,6 +3931,50 @@ static int __init pci_setup(char *str) } early_param("pci", pci_setup); +int __weak pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask) +{ + struct pci_dev *root; + u32 lnkcap, lnkcap2; + + *mask = 0; + if (!dev) + return -EINVAL; + + root = dev->bus->self; + + /* 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; + + pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap); + pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2); + + if (lnkcap2) { /* PCIe r3.0-compliant */ + if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB) + *mask |= PCIE_SPEED_25; + if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB) + *mask |= PCIE_SPEED_50; + if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB) + *mask |= PCIE_SPEED_80; + } else {/* pre-r3.0 */ + if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB) + *mask |= PCIE_SPEED_25; + if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB) + *mask |= (PCIE_SPEED_25 | PCIE_SPEED_50); + } + + dev_info(&dev->dev, "probing gen 2 caps for device %x:%x = %x/%x\n", + root->vendor, root->device, lnkcap, lnkcap2); + return 0; +} + +int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *mask) +{ + return pcibios_get_speed_cap_mask(dev, mask); +} +EXPORT_SYMBOL(pcie_get_speed_cap_mask); + EXPORT_SYMBOL(pci_reenable_device); EXPORT_SYMBOL(pci_enable_device_io); EXPORT_SYMBOL(pci_enable_device_mem); diff --git a/include/linux/pci.h b/include/linux/pci.h index 2461033a..24a2f63 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1861,4 +1861,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev) */ struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev); +#define PCIE_SPEED_25 1 +#define PCIE_SPEED_50 2 +#define PCIE_SPEED_80 4 + +extern int pcie_get_speed_cap_mask(struct pci_dev *dev, u32 *speed_mask); + #endif /* LINUX_PCI_H */ -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 3/3] ppc64: implemented pcibios_get_speed_cap_mask
Implementation of a architecture-specific pcibios_get_speed_cap_mask. This implementation detects bus capabilities based on OF ibm,pcie-link-speed-stats property. Signed-off-by: Lucas Kannebley Tavares --- arch/powerpc/platforms/pseries/pci.c | 35 ++ 1 files changed, 35 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c index 0b580f4..4da8deb 100644 --- a/arch/powerpc/platforms/pseries/pci.c +++ b/arch/powerpc/platforms/pseries/pci.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -108,3 +109,37 @@ 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 pcibios_get_speed_cap_mask(struct pci_dev *dev, u32 *mask) +{ + struct device_node *dn, *pdn; + const uint32_t *pcie_link_speed_stats = NULL; + + *mask = 0; + dn = pci_bus_to_OF_node(dev->bus); + + /* Find nearest ibm,pcie-link-speed-stats, walking up the device tree */ + for (pdn = dn; pdn != NULL; pdn = pdn->parent) { + pcie_link_speed_stats = (const uint32_t *) of_get_property(pdn, + "ibm,pcie-link-speed-stats", NULL); + if (pcie_link_speed_stats != NULL) + break; + } + + if (pcie_link_speed_stats == NULL) { + dev_info(&dev->dev, "no ibm,pcie-link-speed-stats property\n"); + return -EINVAL; + } + + switch (pcie_link_speed_stats[0]) { + case 0x02: + *mask |= PCIE_SPEED_50; + case 0x01: + *mask |= PCIE_SPEED_25; + case 0x00: + default: + return -EINVAL; + } + + return 0; +} -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 0/3] PCI Speed Cap Fixes for ppc64
This patch series first implements a function called pcie_get_speed_cap_mask in the PCI subsystem based off from drm_pcie_get_speed_cap_mask in drm. Then it removes the latter and fixes all references to it. And ultimately, it implements an architecture-specific version of the same function for ppc64. The refactor is done because the function that was used by drm is more architecture goo than module-specific. Whilst the function also needed a platform-specific implementation to get PCIE Gen2 speeds on ppc64. Lucas Kannebley Tavares (3): pci: added pcie_get_speed_cap_mask function drm: removed drm_pcie_get_speed_cap_mask function ppc64: implemented pcibios_get_speed_cap_mask arch/powerpc/platforms/pseries/pci.c | 35 +++ drivers/gpu/drm/drm_pci.c| 38 - drivers/gpu/drm/radeon/evergreen.c |5 ++- drivers/gpu/drm/radeon/r600.c|5 ++- drivers/gpu/drm/radeon/rv770.c |5 ++- drivers/pci/pci.c| 44 ++ include/drm/drmP.h |6 include/linux/pci.h |6 8 files changed, 94 insertions(+), 50 deletions(-) -- 1.7.4.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCHv2 2/3] drm: removed drm_pcie_get_speed_cap_mask function
This function was moved to the pci subsystem where it fits better, as this is much more of a generic pci task, than a drm specific one. All references to the function (all in the radeon driver) are updated. This is the second step in moving function drm_pcie_get_speed_cap_mask from the drm subsystem to the pci one. Signed-off-by: Lucas Kannebley Tavares --- drivers/gpu/drm/drm_pci.c | 38 drivers/gpu/drm/radeon/evergreen.c |5 ++- drivers/gpu/drm/radeon/r600.c |5 ++- drivers/gpu/drm/radeon/rv770.c |5 ++- include/drm/drmP.h |6 - 5 files changed, 9 insertions(+), 50 deletions(-) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index bd719e9..ba70844 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -439,44 +439,6 @@ int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) return 0; } -int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) -{ - struct pci_dev *root; - u32 lnkcap, lnkcap2; - - *mask = 0; - if (!dev->pdev) - return -EINVAL; - - root = dev->pdev->bus->self; - - /* 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; - - pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap); - pcie_capability_read_dword(root, PCI_EXP_LNKCAP2, &lnkcap2); - - 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 {/* pre-r3.0 */ - 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_25 | DRM_PCIE_SPEED_50); - } - - DRM_INFO("probing gen 2 caps for device %x:%x = %x/%x\n", root->vendor, root->device, lnkcap, lnkcap2); - return 0; -} -EXPORT_SYMBOL(drm_pcie_get_speed_cap_mask); - #else int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 305a657..6ba204d 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include "radeon.h" #include "radeon_asic.h" @@ -3871,11 +3872,11 @@ 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); + ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask); if (ret != 0) return; - if (!(mask & DRM_PCIE_SPEED_50)) + if (!(mask & PCIE_SPEED_50)) 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..89a7387 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include "radeon.h" @@ -4371,11 +4372,11 @@ 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); + ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask); if (ret != 0) return; - if (!(mask & DRM_PCIE_SPEED_50)) + if (!(mask & PCIE_SPEED_50)) 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..81c7f1c 100644 --- a/drivers/gpu/drm/radeon/rv770.c +++ b/drivers/gpu/drm/radeon/rv770.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include "radeon.h" #include "radeon_asic.h" @@ -1254,11 +1255,11 @@ 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); + ret = pcie_get_speed_cap_mask(rdev->ddev->pdev, &mask); if (ret != 0) return; - if (!(mask & DRM_PCIE_SPEED_50)) + if (!(mask & PCIE_SPEED_50)) return; DRM_INFO(&
[PATCH] drm: change pci_read_config_dword calls to pcie_capability_read_dword ones
Replacing these calls avoids compatibility problems with PCIe v1/v2 Capability structures. Signed-off-by: Lucas Kannebley Tavares --- drivers/gpu/drm/drm_pci.c |8 ++-- 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index ea41234..b824d4c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -486,17 +486,13 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) if (root == NULL) root = dev->pdev; - 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); + pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap); + pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap2); lnkcap &= PCI_EXP_LNKCAP_SLS; lnkcap2 &= 0xfe; -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
[PATCH] drm: fixed access to PCI host bridges
During the process of obtaining the speed cap for the device, it attempts go get the PCI Host bus. However on architectures such as PPC or IA64, those do not appear as devices. Signed-off-by: Lucas Kannebley Tavares --- drivers/gpu/drm/drm_pci.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 754bc96..ea41234 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -479,8 +479,13 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) if (!pci_is_pcie(dev->pdev)) return -EINVAL; + // find PCI device for capabilities root = dev->pdev->bus->self; + // some architectures might not have host bridges as PCI devices + if (root == NULL) + root = dev->pdev; + pos = pci_pcie_cap(root); if (!pos) return -EINVAL; -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
drm: Added ppc64 root device getter
On 12/13/2012 09:31 PM, Bjorn Helgaas wrote: > [+cc Betty] > > On Thu, Dec 13, 2012 at 4:04 PM, Lucas Kannebley Tavares > wrote: >> On architectures such as ppc64, there is no root bus device (it belongs >> to the hypervisor). DRM attempted to get one, causing a null-pointer >> dereference. > > In addition to ppc64, at least ia64 and parisc have the same situation > of the PCI host bridge not appearing as a PCI device itself. > >> Signed-off-by: Lucas Kannebley Tavares >> >> -- >> diff --git a/arch/powerpc/platforms/pseries/Makefile >> b/arch/powerpc/platforms/pseries/Makefile >> index 890622b..ddfdda8 100644 >> --- a/arch/powerpc/platforms/pseries/Makefile >> +++ b/arch/powerpc/platforms/pseries/Makefile >> @@ -1,6 +1,8 @@ >> ccflags-$(CONFIG_PPC64):= -mno-minimal-toc >> ccflags-$(CONFIG_PPC_PSERIES_DEBUG)+= -DDEBUG >> >> +drm-y += drm_pci.o >> + >> obj-y := lpar.o hvCall.o nvram.o reconfig.o \ >> setup.o iommu.o event_sources.o ras.o \ >> firmware.o power.o dlpar.o mobility.o >> diff --git a/arch/powerpc/platforms/pseries/drm_pci.c >> b/arch/powerpc/platforms/pseries/drm_pci.c >> new file mode 100644 >> index 000..da6675e >> --- /dev/null >> +++ b/arch/powerpc/platforms/pseries/drm_pci.c >> @@ -0,0 +1,24 @@ >> +/* >> + * Copyright (C) 2012 Lucas Kannebley Tavares, IBM Corporation >> + * >> + * pSeries specific routines for DRM. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write to the Free Software >> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA >> + */ >> + >> +inline struct pci_device *drm_get_parent_device(struct drm_device *dev) { >> + return (dev->pdev->bus->self == NULL) ? dev->pdev : >> dev->pdev->bus->self; > > So for DRM devices on a root bus, the parent is the DRM device itself, > while for DRM devices deeper in the hierarchy, the parent is the > upstream P2P bridge? That doesn't really make sense to me. If the > caller operates on the DRM device in some cases and on the bridge in > other cases, it's going to need to know the difference, so hiding the > difference in this wrapper seems counterproductive. > >> +} >> + >> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c >> index eb37466..5a8a4f5 100644 >> --- a/drivers/gpu/drm/drm_pci.c >> +++ b/drivers/gpu/drm/drm_pci.c >> @@ -466,6 +466,10 @@ void drm_pci_exit(struct drm_driver *driver, struct >> pci_driver *pdriver) >> } >> EXPORT_SYMBOL(drm_pci_exit); >> >> +inline __weak struct pci_device *drm_get_parent_device(struct drm_device >> *dev) { >> + return dev->pdev->bus->self; >> +} >> + >> int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) >> { >> struct pci_dev *root; >> @@ -479,7 +483,7 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, >> u32 *mask) >> return -EINVAL; >> >> // find PCI device for capabilities >> - root = dev->pdev->bus->self; >> + root = drm_get_parent_device(dev); >> >> // some architectures might not have host bridges as PCI devices >> if (root == NULL) > > What tree does this apply to? Upstream doesn't have the "if (root == > NULL)" check yet. That check looks like the sort of thing you'd need > to avoid the null pointer dereference. So maybe adding that check and > the associated code is enough to fix the problem, even without adding > drm_get_parent_device(). > > With the code in the tree, it looks like you'd dereference a null > pointer in pci_pcie_cap(root), so I assume that's what you tripped > over. > > I'm not really sure that code outside the PCI core should be looking > at capabilities of upstream devices like this. It seems like the sort > of thing where the core might need to provide better interfaces. > > Bjorn > Ok Bjorn, thanks for the comments, indeed I had a dirty tree here and didn't realize it, sorry. Either way I'm then sending the "if (root == NULL)" patch as a reply to this. I'm sending it along with another independent patch (they are NOT a series) that changes pci_read_config_dword calls to pci_capability_read_dword ones on the drm driver. There were only a couple of those to start with. -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
[PATCH] drm: change pci_read_config_dword calls to pcie_capability_read_dword ones
Replacing these calls avoids compatibility problems with PCIe v1/v2 Capability structures. Signed-off-by: Lucas Kannebley Tavares --- drivers/gpu/drm/drm_pci.c |8 ++-- 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index ea41234..b824d4c 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -486,17 +486,13 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) if (root == NULL) root = dev->pdev; - 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); + pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap); + pcie_capability_read_dword(root, PCI_EXP_LNKCAP, &lnkcap2); lnkcap &= PCI_EXP_LNKCAP_SLS; lnkcap2 &= 0xfe; -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: fixed access to PCI host bridges
During the process of obtaining the speed cap for the device, it attempts go get the PCI Host bus. However on architectures such as PPC or IA64, those do not appear as devices. Signed-off-by: Lucas Kannebley Tavares --- drivers/gpu/drm/drm_pci.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index 754bc96..ea41234 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -479,8 +479,13 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) if (!pci_is_pcie(dev->pdev)) return -EINVAL; + // find PCI device for capabilities root = dev->pdev->bus->self; + // some architectures might not have host bridges as PCI devices + if (root == NULL) + root = dev->pdev; + pos = pci_pcie_cap(root); if (!pos) return -EINVAL; -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: drm: Added ppc64 root device getter
On 12/13/2012 09:31 PM, Bjorn Helgaas wrote: [+cc Betty] On Thu, Dec 13, 2012 at 4:04 PM, Lucas Kannebley Tavares wrote: On architectures such as ppc64, there is no root bus device (it belongs to the hypervisor). DRM attempted to get one, causing a null-pointer dereference. In addition to ppc64, at least ia64 and parisc have the same situation of the PCI host bridge not appearing as a PCI device itself. Signed-off-by: Lucas Kannebley Tavares -- diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 890622b..ddfdda8 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -1,6 +1,8 @@ ccflags-$(CONFIG_PPC64):= -mno-minimal-toc ccflags-$(CONFIG_PPC_PSERIES_DEBUG)+= -DDEBUG +drm-y += drm_pci.o + obj-y := lpar.o hvCall.o nvram.o reconfig.o \ setup.o iommu.o event_sources.o ras.o \ firmware.o power.o dlpar.o mobility.o diff --git a/arch/powerpc/platforms/pseries/drm_pci.c b/arch/powerpc/platforms/pseries/drm_pci.c new file mode 100644 index 000..da6675e --- /dev/null +++ b/arch/powerpc/platforms/pseries/drm_pci.c @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2012 Lucas Kannebley Tavares, IBM Corporation + * + * pSeries specific routines for DRM. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +inline struct pci_device *drm_get_parent_device(struct drm_device *dev) { + return (dev->pdev->bus->self == NULL) ? dev->pdev : dev->pdev->bus->self; So for DRM devices on a root bus, the parent is the DRM device itself, while for DRM devices deeper in the hierarchy, the parent is the upstream P2P bridge? That doesn't really make sense to me. If the caller operates on the DRM device in some cases and on the bridge in other cases, it's going to need to know the difference, so hiding the difference in this wrapper seems counterproductive. +} + diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index eb37466..5a8a4f5 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -466,6 +466,10 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) } EXPORT_SYMBOL(drm_pci_exit); +inline __weak struct pci_device *drm_get_parent_device(struct drm_device *dev) { + return dev->pdev->bus->self; +} + int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) { struct pci_dev *root; @@ -479,7 +483,7 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) return -EINVAL; // find PCI device for capabilities - root = dev->pdev->bus->self; + root = drm_get_parent_device(dev); // some architectures might not have host bridges as PCI devices if (root == NULL) What tree does this apply to? Upstream doesn't have the "if (root == NULL)" check yet. That check looks like the sort of thing you'd need to avoid the null pointer dereference. So maybe adding that check and the associated code is enough to fix the problem, even without adding drm_get_parent_device(). With the code in the tree, it looks like you'd dereference a null pointer in pci_pcie_cap(root), so I assume that's what you tripped over. I'm not really sure that code outside the PCI core should be looking at capabilities of upstream devices like this. It seems like the sort of thing where the core might need to provide better interfaces. Bjorn Ok Bjorn, thanks for the comments, indeed I had a dirty tree here and didn't realize it, sorry. Either way I'm then sending the "if (root == NULL)" patch as a reply to this. I'm sending it along with another independent patch (they are NOT a series) that changes pci_read_config_dword calls to pci_capability_read_dword ones on the drm driver. There were only a couple of those to start with. -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
drm: Added ppc64 root device getter
On architectures such as ppc64, there is no root bus device (it belongs to the hypervisor). DRM attempted to get one, causing a null-pointer dereference. Signed-off-by: Lucas Kannebley Tavares -- diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 890622b..ddfdda8 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -1,6 +1,8 @@ ccflags-$(CONFIG_PPC64):= -mno-minimal-toc ccflags-$(CONFIG_PPC_PSERIES_DEBUG)+= -DDEBUG +drm-y += drm_pci.o + obj-y := lpar.o hvCall.o nvram.o reconfig.o \ setup.o iommu.o event_sources.o ras.o \ firmware.o power.o dlpar.o mobility.o diff --git a/arch/powerpc/platforms/pseries/drm_pci.c b/arch/powerpc/platforms/pseries/drm_pci.c new file mode 100644 index 000..da6675e --- /dev/null +++ b/arch/powerpc/platforms/pseries/drm_pci.c @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2012 Lucas Kannebley Tavares, IBM Corporation + * + * pSeries specific routines for DRM. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +inline struct pci_device *drm_get_parent_device(struct drm_device *dev) { + return (dev->pdev->bus->self == NULL) ? dev->pdev : dev->pdev->bus->self; +} + diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index eb37466..5a8a4f5 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -466,6 +466,10 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) } EXPORT_SYMBOL(drm_pci_exit); +inline __weak struct pci_device *drm_get_parent_device(struct drm_device *dev) { + return dev->pdev->bus->self; +} + int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) { struct pci_dev *root; @@ -479,7 +483,7 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) return -EINVAL; // find PCI device for capabilities - root = dev->pdev->bus->self; + root = drm_get_parent_device(dev); // some architectures might not have host bridges as PCI devices if (root == NULL) -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
drm: Added ppc64 root device getter
On architectures such as ppc64, there is no root bus device (it belongs to the hypervisor). DRM attempted to get one, causing a null-pointer dereference. Signed-off-by: Lucas Kannebley Tavares -- diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile index 890622b..ddfdda8 100644 --- a/arch/powerpc/platforms/pseries/Makefile +++ b/arch/powerpc/platforms/pseries/Makefile @@ -1,6 +1,8 @@ ccflags-$(CONFIG_PPC64) := -mno-minimal-toc ccflags-$(CONFIG_PPC_PSERIES_DEBUG) += -DDEBUG +drm-y += drm_pci.o + obj-y := lpar.o hvCall.o nvram.o reconfig.o \ setup.o iommu.o event_sources.o ras.o \ firmware.o power.o dlpar.o mobility.o diff --git a/arch/powerpc/platforms/pseries/drm_pci.c b/arch/powerpc/platforms/pseries/drm_pci.c new file mode 100644 index 000..da6675e --- /dev/null +++ b/arch/powerpc/platforms/pseries/drm_pci.c @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2012 Lucas Kannebley Tavares, IBM Corporation + * + * pSeries specific routines for DRM. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +inline struct pci_device *drm_get_parent_device(struct drm_device *dev) { + return (dev->pdev->bus->self == NULL) ? dev->pdev : dev->pdev->bus->self; +} + diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index eb37466..5a8a4f5 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -466,6 +466,10 @@ void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver) } EXPORT_SYMBOL(drm_pci_exit); +inline __weak struct pci_device *drm_get_parent_device(struct drm_device *dev) { + return dev->pdev->bus->self; +} + int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) { struct pci_dev *root; @@ -479,7 +483,7 @@ int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) return -EINVAL; // find PCI device for capabilities - root = dev->pdev->bus->self; + root = drm_get_parent_device(dev); // some architectures might not have host bridges as PCI devices if (root == NULL) -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center
radeon: RFC speed cap detection on ppc64
The radeon driver does speed cap detection on the root PCI device for the maximum speed with which the adapter can communicate. On ppc64 systems, however, the root device belongs to the Hypervisor, so the current code would case a null pointer dereference. I propose to look for the outmost bus with a parent node and get speed caps from it, though I suppose the cleaner way would be to inspect all devices along the way and choose the smallest speed cap. Does anyone have suggestions for this? Thanks -- --- /home/lucaskt/work/devdrivers/kernel/linux/drivers/gpu/drm/drm_pci.c 2012-09-26 10:06:00.280549928 -0300 +++ drm_pci.c 2012-09-26 15:38:51.121786353 -0300 @@ -466,6 +466,19 @@ } EXPORT_SYMBOL(drm_pci_exit); +static struct pci_dev *drm_get_pcie_root_dev(struct pci_dev *dev) +{ + // Go up through all possible busses to get the info for the outmost bus + while (!pci_is_root_bus(dev->bus)) + dev = dev->bus->parent->self; + + // In POWER architectures there's no PCI root device, so it should just read + // the caps from the device itself + if (dev->bus->self != NULL) + return dev->bus->self; + else + return dev; +} + int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) { struct pci_dev *root; @@ -479,7 +492,7 @@ if (!pci_is_pcie(dev->pdev)) return -EINVAL; - root = dev->pdev->bus->self; + root = drm_get_pcie_root_dev(dev->pdev); pos = pci_pcie_cap(root); if (!pos) -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
radeon: RFC speed cap detection on ppc64
The radeon driver does speed cap detection on the root PCI device for the maximum speed with which the adapter can communicate. On ppc64 systems, however, the root device belongs to the Hypervisor, so the current code would case a null pointer dereference. I propose to look for the outmost bus with a parent node and get speed caps from it, though I suppose the cleaner way would be to inspect all devices along the way and choose the smallest speed cap. Does anyone have suggestions for this? Thanks -- --- /home/lucaskt/work/devdrivers/kernel/linux/drivers/gpu/drm/drm_pci.c 2012-09-26 10:06:00.280549928 -0300 +++ drm_pci.c 2012-09-26 15:38:51.121786353 -0300 @@ -466,6 +466,19 @@ } EXPORT_SYMBOL(drm_pci_exit); +static struct pci_dev *drm_get_pcie_root_dev(struct pci_dev *dev) +{ + // Go up through all possible busses to get the info for the outmost bus + while (!pci_is_root_bus(dev->bus)) + dev = dev->bus->parent->self; + + // In POWER architectures there's no PCI root device, so it should just read + // the caps from the device itself + if (dev->bus->self != NULL) + return dev->bus->self; + else + return dev; +} + int drm_pcie_get_speed_cap_mask(struct drm_device *dev, u32 *mask) { struct pci_dev *root; @@ -479,7 +492,7 @@ if (!pci_is_pcie(dev->pdev)) return -EINVAL; - root = dev->pdev->bus->self; + root = drm_get_pcie_root_dev(dev->pdev); pos = pci_pcie_cap(root); if (!pos) -- Lucas Kannebley Tavares Software Engineer IBM Linux Technology Center