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

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. :)

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?

Also, the added functions aren't documented as mandated by the rules Alex proposed.