On Wed, Apr 17, 2013 at 8:38 AM, Lucas Kannebley Tavares
<lucaskt@xxxxxxxxxxxxxxxxxx> wrote:
> On 04/12/2013 01:38 PM, Bjorn Helgaas wrote:
>>
>> On Thu, Apr 11, 2013 at 7:13 AM, Lucas Kannebley Tavares
>> <lucaskt@xxxxxxxxxxxxxxxxxx> 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.
drm_pcie_get_speed_cap_mask() actually checked the pci bridge to see
what speeds it supported. If the new code doesn't actually check the
bridge caps then the new code does not seem like a valid replacement
unless I'm missing something.
Alex
>
>
>>
>> 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;
>>>
>>> DRM_INFO("enabling PCIE gen 2 link speeds, disable with
>>> radeon.pcie_gen2=0\n");
>>> --
>>> 1.7.4.4
>>>
>>
>
>
> --
> Lucas Kannebley Tavares
> Software Engineer
> IBM Linux Technology Center
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
