Re: [PATCH 16/16] drm/radeon: implement ring saving on reset

From: "Christian König" <deathsimple@vodafone.de>
To: "Michel Dänzer" <michel@daenzer.net>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 16/16] drm/radeon: implement ring saving on reset
Date: Mon, 09 Jul 2012 17:24:25 +0200	[thread overview]
Message-ID: <4FFAF7A9.1030706@vodafone.de> (raw)
In-Reply-To: <1341842820.12576.272.camel@thor.local>

On 09.07.2012 17:06, Michel Dänzer wrote:
> On Mon, 2012-07-09 at 12:42 +0200, Christian König wrote:
>> Try to save whatever is on the rings when
>> we encounter an lockup.
>>
>> Signed-off-by: Christian König <deathsimple@vodafone.de>
> [...]
>> @@ -1005,20 +1010,43 @@ int radeon_gpu_reset(struct radeon_device *rdev)
>>   	resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
>>   	radeon_suspend(rdev);
>>   
>> +	for (i = 0; i < RADEON_NUM_RINGS; ++i) {
>> +		ring_sizes[i] = radeon_ring_backup(rdev, &rdev->ring[i],
>> +						   &ring_data[i]);
>> +		if (ring_sizes[i]) {
>> +			saved = true;
>> +			dev_info(rdev->dev, "Saved %d dwords of commands "
>> +				 "on ring %d.\n", ring_sizes[i], i);
>> +		}
>> +	}
>> +
>> +retry:
>>   	r = radeon_asic_reset(rdev);
>>   	if (!r) {
>> -		dev_info(rdev->dev, "GPU reset succeed\n");
>> +		dev_info(rdev->dev, "GPU reset succeed trying to resume\n");
> Could fix the spelling of 'succeeded' while you're at it. :)
Akk, fixed it.
>
>
>> 		radeon_resume(rdev);
>> +	}
>>   
>> -		r = radeon_ib_ring_tests(rdev);
>> -		if (r)
>> -			DRM_ERROR("ib ring test failed (%d).\n", r);
>> +	radeon_restore_bios_scratch_regs(rdev);
>> +	drm_helper_resume_force_mode(rdev->ddev);
>> +
>> +	if (!r) {
>> +		for (i = 0; i < RADEON_NUM_RINGS; ++i) {
>> +			radeon_ring_restore(rdev, &rdev->ring[i],
>> +					    ring_sizes[i], ring_data[i]);
>> +		}
> If radeon_asic_reset fails, this leaks the memory referenced by
> ring_data, doesn't it?
Oh yes indeed, going to fix that.

> Also, the added functions aren't documented as mandated by the rules
> Alex proposed.
>
True, also going to fix that.

Christian.