Re: [PATCH 13/16] drm/radeon: move radeon_ib_ring_tests out of chipset code

["Christian König" <deathsimple@vodafone.de>]

On 09.07.2012 17:06, Michel Dänzer wrote:
> On Mon, 2012-07-09 at 12:42 +0200, Christian König wrote:
>> Making it easier to controlwhen it is executed.
>>
>> Signed-off-by: Christian König <deathsimple@vodafone.de>
> [...]
>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
>> index 254fdb4..bbd0971 100644
>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>> @@ -822,6 +822,10 @@ int radeon_device_init(struct radeon_device *rdev,
>>   	if (r)
>>   		return r;
>>   
>> +	r = radeon_ib_ring_tests(rdev);
>> +	if (r)
>> +		DRM_ERROR("ib ring test failed (%d).\n", r);
>> +
>>   	if (rdev->flags & RADEON_IS_AGP && !rdev->accel_working) {
>>   		/* Acceleration not working on AGP card try again
>>   		 * with fallback to PCI or PCIE GART
> I think this needs to set rdev->accel_working = false on failure, so the
> AGP -> PCI(e) fallback can kick in.

See the implementation of radeon_ib_ring_tests, it is already handling 
that internally.

> Not sure about the other places where you're adding
> radeon_ib_ring_tests() calls, might need more error handling as well.
radeon_ib_ring_tests is already handling most errors internally, e. g. 
it sets the accel_working and the ring->ready flags to false if anything 
goes wrong. The return value is mostly for the case where we want to try 
the reset a second time if restoring the ring commands leads to another 
lockup. I just doesn't want to ignore the result completely, so the 
additional error message.

Christian.