[RFC] drm/radeon: restoring ring commands in case of a lockup

[RFC] drm/radeon: restoring ring commands in case of a lockup

[Christian König]

Hi,

The following patchset tries to save and restore the not yet processed commands
from the rings in case of a lockup and with that should make a userspace
problem with a single application far less problematic.

The first four patches are just stuff this patchset is based upon, followed by
four patches which fix various bugs found while working on this feature.

Followed by patches which change the way how memory is saved/restored on
suspend/resume, basically before we have unpinned most of the buffer objects so
it could be move from vram into system memory. But that is mostly unnecessary
cause the buffer object either are already in system memory or their content
can be easily reinitialized.

The last three patches implement the actual tracking and restoring of commands
in case of a lockup. Please take a look and review.

Cheers,
Christian.

[PATCH 01/16] drm/radeon: add error handling to fence_wait_empty_locked

[Christian König]

Instead of returning the error handle it directly
and while at it fix the comments about the ring lock.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon.h       |    2 +-
 drivers/gpu/drm/radeon/radeon_fence.c |   33 +++++++++++++++++++++------------
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 77b4519b..5861ec8 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -239,7 +239,7 @@ void radeon_fence_process(struct radeon_device *rdev, int ring);
 bool radeon_fence_signaled(struct radeon_fence *fence);
 int radeon_fence_wait(struct radeon_fence *fence, bool interruptible);
 int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring);
-int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
+void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring);
 int radeon_fence_wait_any(struct radeon_device *rdev,
 			  struct radeon_fence **fences,
 			  bool intr);
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 7b55625..be4e4f3 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -440,14 +440,11 @@ int radeon_fence_wait_any(struct radeon_device *rdev,
 	return 0;
 }
 
+/* caller must hold ring lock */
 int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
 {
 	uint64_t seq;
 
-	/* We are not protected by ring lock when reading current seq but
-	 * it's ok as worst case is we return to early while we could have
-	 * wait.
-	 */
 	seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL;
 	if (seq >= rdev->fence_drv[ring].sync_seq[ring]) {
 		/* nothing to wait for, last_seq is
@@ -457,15 +454,27 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring)
 	return radeon_fence_wait_seq(rdev, seq, ring, false, false);
 }
 
-int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
+/* caller must hold ring lock */
+void radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring)
 {
-	/* We are not protected by ring lock when reading current seq
-	 * but it's ok as wait empty is call from place where no more
-	 * activity can be scheduled so there won't be concurrent access
-	 * to seq value.
-	 */
-	return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].sync_seq[ring],
-				     ring, false, false);
+	uint64_t seq = rdev->fence_drv[ring].sync_seq[ring];
+
+	while(1) {
+		int r;
+		r = radeon_fence_wait_seq(rdev, seq, ring, false, false);
+		if (r == -EDEADLK) {
+			mutex_unlock(&rdev->ring_lock);
+			r = radeon_gpu_reset(rdev);
+			mutex_lock(&rdev->ring_lock);
+			if (!r)
+				continue;
+		}
+		if (r) {
+			dev_err(rdev->dev, "error waiting for ring to become"
+				" idle (%d)\n", r);
+		}
+		return;
+	}
 }
 
 struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence)
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/16] drm/radeon: add error handling to radeon_vm_unbind_locked

[Christian König]

Waiting for a fence can fail for different reasons,
the most common is a deadlock.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/radeon/radeon_gart.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 2b34c1a..ee11c50 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -316,10 +316,21 @@ static void radeon_vm_unbind_locked(struct radeon_device *rdev,
 	}
 
 	/* wait for vm use to end */
-	if (vm->fence) {
-		radeon_fence_wait(vm->fence, false);
-		radeon_fence_unref(&vm->fence);
+	while (vm->fence) {
+		int r;
+		r = radeon_fence_wait(vm->fence, false);
+		if (r)
+			DRM_ERROR("error while waiting for fence: %d\n", r);
+		if (r == -EDEADLK) {
+			mutex_unlock(&rdev->vm_manager.lock);
+			r = radeon_gpu_reset(rdev);
+			mutex_lock(&rdev->vm_manager.lock);
+			if (!r)
+				continue;
+		}
+		break;
 	}
+	radeon_fence_unref(&vm->fence);
 
 	/* hw unbind */
 	rdev->vm_manager.funcs->unbind(rdev, vm);
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/16] drm/radeon: fix fence related segfault in CS

[Christian König]

Don't return success if scheduling the IB fails, otherwise
we end up with an oops in ttm_eu_fence_buffer_objects.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/radeon/radeon_cs.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index f1b7527..d5aec09 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -358,7 +358,7 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev,
 	if (r) {
 		DRM_ERROR("Failed to schedule IB !\n");
 	}
-	return 0;
+	return r;
 }
 
 static int radeon_bo_vm_update_pte(struct radeon_cs_parser *parser,
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/16] drm/radeon: add an exclusive lock for GPU reset v2

[Christian König]

From: Jerome Glisse <jglisse@redhat.com>

GPU reset need to be exclusive, one happening at a time. For this
add a rw semaphore so that any path that trigger GPU activities
have to take the semaphore as a reader thus allowing concurency.

The GPU reset path take the semaphore as a writer ensuring that
no concurrent reset take place.

v2: init rw semaphore

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h        |    1 +
 drivers/gpu/drm/radeon/radeon_cs.c     |    5 +++++
 drivers/gpu/drm/radeon/radeon_device.c |    3 +++
 drivers/gpu/drm/radeon/radeon_gem.c    |    8 ++++++++
 4 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 5861ec8..4487873 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1446,6 +1446,7 @@ struct radeon_device {
 	struct device			*dev;
 	struct drm_device		*ddev;
 	struct pci_dev			*pdev;
+	struct rw_semaphore		exclusive_lock;
 	/* ASIC */
 	union radeon_asic_config	config;
 	enum radeon_family		family;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index d5aec09..553da67 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -499,7 +499,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	struct radeon_cs_parser parser;
 	int r;
 
+	down_read(&rdev->exclusive_lock);
 	if (!rdev->accel_working) {
+		up_read(&rdev->exclusive_lock);
 		return -EBUSY;
 	}
 	/* initialize parser */
@@ -512,6 +514,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	if (r) {
 		DRM_ERROR("Failed to initialize parser !\n");
 		radeon_cs_parser_fini(&parser, r);
+		up_read(&rdev->exclusive_lock);
 		r = radeon_cs_handle_lockup(rdev, r);
 		return r;
 	}
@@ -520,6 +523,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		if (r != -ERESTARTSYS)
 			DRM_ERROR("Failed to parse relocation %d!\n", r);
 		radeon_cs_parser_fini(&parser, r);
+		up_read(&rdev->exclusive_lock);
 		r = radeon_cs_handle_lockup(rdev, r);
 		return r;
 	}
@@ -533,6 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	}
 out:
 	radeon_cs_parser_fini(&parser, r);
+	up_read(&rdev->exclusive_lock);
 	r = radeon_cs_handle_lockup(rdev, r);
 	return r;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index f654ba8..254fdb4 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -734,6 +734,7 @@ int radeon_device_init(struct radeon_device *rdev,
 	mutex_init(&rdev->gem.mutex);
 	mutex_init(&rdev->pm.mutex);
 	init_rwsem(&rdev->pm.mclk_lock);
+	init_rwsem(&rdev->exclusive_lock);
 	init_waitqueue_head(&rdev->irq.vblank_queue);
 	init_waitqueue_head(&rdev->irq.idle_queue);
 	r = radeon_gem_init(rdev);
@@ -988,6 +989,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
 	int r;
 	int resched;
 
+	down_write(&rdev->exclusive_lock);
 	radeon_save_bios_scratch_regs(rdev);
 	/* block TTM */
 	resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
@@ -1007,6 +1009,7 @@ int radeon_gpu_reset(struct radeon_device *rdev)
 		dev_info(rdev->dev, "GPU reset failed\n");
 	}
 
+	up_write(&rdev->exclusive_lock);
 	return r;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index d9b0809..b0be9c4 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -215,12 +215,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data,
 	uint32_t handle;
 	int r;
 
+	down_read(&rdev->exclusive_lock);
 	/* create a gem object to contain this object in */
 	args->size = roundup(args->size, PAGE_SIZE);
 	r = radeon_gem_object_create(rdev, args->size, args->alignment,
 					args->initial_domain, false,
 					false, &gobj);
 	if (r) {
+		up_read(&rdev->exclusive_lock);
 		r = radeon_gem_handle_lockup(rdev, r);
 		return r;
 	}
@@ -228,10 +230,12 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data,
 	/* drop reference from allocate - handle holds it now */
 	drm_gem_object_unreference_unlocked(gobj);
 	if (r) {
+		up_read(&rdev->exclusive_lock);
 		r = radeon_gem_handle_lockup(rdev, r);
 		return r;
 	}
 	args->handle = handle;
+	up_read(&rdev->exclusive_lock);
 	return 0;
 }
 
@@ -240,6 +244,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 {
 	/* transition the BO to a domain -
 	 * just validate the BO into a certain domain */
+	struct radeon_device *rdev = dev->dev_private;
 	struct drm_radeon_gem_set_domain *args = data;
 	struct drm_gem_object *gobj;
 	struct radeon_bo *robj;
@@ -247,10 +252,12 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 
 	/* for now if someone requests domain CPU -
 	 * just make sure the buffer is finished with */
+	down_read(&rdev->exclusive_lock);
 
 	/* just do a BO wait for now */
 	gobj = drm_gem_object_lookup(dev, filp, args->handle);
 	if (gobj == NULL) {
+		up_read(&rdev->exclusive_lock);
 		return -ENOENT;
 	}
 	robj = gem_to_radeon_bo(gobj);
@@ -258,6 +265,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	r = radeon_gem_set_domain(gobj, args->read_domains, args->write_domain);
 
 	drm_gem_object_unreference_unlocked(gobj);
+	up_read(&rdev->exclusive_lock);
 	r = radeon_gem_handle_lockup(robj->rdev, r);
 	return r;
 }
-- 
1.7.9.5

[PATCH 05/16] drm/radeon: fix ring commit padding

[Christian König]

We don't need to pad anything if the number of dwords
written to the ring already matches the requirements.

Fixes some "writting more dword to ring than expected"
warnings.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/radeon_ring.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 0826e77..674aaba 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -272,13 +272,8 @@ int radeon_ring_lock(struct radeon_device *rdev, struct radeon_ring *ring, unsig
 
 void radeon_ring_commit(struct radeon_device *rdev, struct radeon_ring *ring)
 {
-	unsigned count_dw_pad;
-	unsigned i;
-
 	/* We pad to match fetch size */
-	count_dw_pad = (ring->align_mask + 1) -
-		       (ring->wptr & ring->align_mask);
-	for (i = 0; i < count_dw_pad; i++) {
+	while (ring->wptr & ring->align_mask) {
 		radeon_ring_write(ring, ring->nop);
 	}
 	DRM_MEMORYBARRIER();
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/16] drm/radeon: fix fence value access

[Christian König]

It is possible that radeon_fence_process is called
after writeback is disabled for suspend, leading
to an invalid read of register 0x0.

This fixes a problem for me where the fence value
is temporary incremented by 0x100000000 on
suspend/resume.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/radeon_fence.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index be4e4f3..a194a14 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -42,21 +42,23 @@
 
 static void radeon_fence_write(struct radeon_device *rdev, u32 seq, int ring)
 {
-	if (rdev->wb.enabled) {
-		*rdev->fence_drv[ring].cpu_addr = cpu_to_le32(seq);
+	struct radeon_fence_driver *drv = &rdev->fence_drv[ring];
+	if (likely(rdev->wb.enabled || !drv->scratch_reg)) {
+		*drv->cpu_addr = cpu_to_le32(seq);
 	} else {
-		WREG32(rdev->fence_drv[ring].scratch_reg, seq);
+		WREG32(drv->scratch_reg, seq);
 	}
 }
 
 static u32 radeon_fence_read(struct radeon_device *rdev, int ring)
 {
+	struct radeon_fence_driver *drv = &rdev->fence_drv[ring];
 	u32 seq = 0;
 
-	if (rdev->wb.enabled) {
-		seq = le32_to_cpu(*rdev->fence_drv[ring].cpu_addr);
+	if (likely(rdev->wb.enabled || !drv->scratch_reg)) {
+		seq = le32_to_cpu(*drv->cpu_addr);
 	} else {
-		seq = RREG32(rdev->fence_drv[ring].scratch_reg);
+		seq = RREG32(drv->scratch_reg);
 	}
 	return seq;
 }
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/16] drm/radeon: fix fence init after resume

[Christian König]

Start with last signaled fence number instead
of last emitted one.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/radeon_fence.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index a194a14..76c5b22 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -578,7 +578,7 @@ int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring)
 	}
 	rdev->fence_drv[ring].cpu_addr = &rdev->wb.wb[index/4];
 	rdev->fence_drv[ring].gpu_addr = rdev->wb.gpu_addr + index;
-	radeon_fence_write(rdev, rdev->fence_drv[ring].sync_seq[ring], ring);
+	radeon_fence_write(rdev, atomic64_read(&rdev->fence_drv[ring].last_seq), ring);
 	rdev->fence_drv[ring].initialized = true;
 	dev_info(rdev->dev, "fence driver on ring %d use gpu addr 0x%016llx and cpu addr 0x%p\n",
 		 ring, rdev->fence_drv[ring].gpu_addr, rdev->fence_drv[ring].cpu_addr);
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/16] drm/radeon: remove FIXME comment from chipset suspend

[Christian König]

For a normal suspend/resume we allready wait for
the rings to be empty, and for a suspend/reasume
in case of a lockup we REALLY don't want to wait
for anything.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/evergreen.c |    1 -
 drivers/gpu/drm/radeon/ni.c        |    1 -
 drivers/gpu/drm/radeon/r600.c      |    1 -
 drivers/gpu/drm/radeon/rv770.c     |    1 -
 drivers/gpu/drm/radeon/si.c        |    1 -
 5 files changed, 5 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index f716e08..eb9a71a 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3137,7 +3137,6 @@ int evergreen_suspend(struct radeon_device *rdev)
 	struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
 
 	r600_audio_fini(rdev);
-	/* FIXME: we should wait for ring to be empty */
 	radeon_ib_pool_suspend(rdev);
 	r600_blit_suspend(rdev);
 	r700_cp_stop(rdev);
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 2366be3..32a6082 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1334,7 +1334,6 @@ int cayman_resume(struct radeon_device *rdev)
 int cayman_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
-	/* FIXME: we should wait for ring to be empty */
 	radeon_ib_pool_suspend(rdev);
 	radeon_vm_manager_suspend(rdev);
 	r600_blit_suspend(rdev);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 43d0c41..de4de2d 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2461,7 +2461,6 @@ int r600_suspend(struct radeon_device *rdev)
 	r600_audio_fini(rdev);
 	radeon_ib_pool_suspend(rdev);
 	r600_blit_suspend(rdev);
-	/* FIXME: we should wait for ring to be empty */
 	r600_cp_stop(rdev);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	r600_irq_suspend(rdev);
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index b4f51c5..7e230f6 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -996,7 +996,6 @@ int rv770_suspend(struct radeon_device *rdev)
 	r600_audio_fini(rdev);
 	radeon_ib_pool_suspend(rdev);
 	r600_blit_suspend(rdev);
-	/* FIXME: we should wait for ring to be empty */
 	r700_cp_stop(rdev);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	r600_irq_suspend(rdev);
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 34603b3c8..78c790f 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -3807,7 +3807,6 @@ int si_resume(struct radeon_device *rdev)
 
 int si_suspend(struct radeon_device *rdev)
 {
-	/* FIXME: we should wait for ring to be empty */
 	radeon_ib_pool_suspend(rdev);
 	radeon_vm_manager_suspend(rdev);
 #if 0
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/16] drm/radeon: make cp init on cayman more robust

[Christian König]

It's not critical, but the current code isn't
100% correct.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/ni.c |  133 ++++++++++++++++++-------------------------
 1 file changed, 56 insertions(+), 77 deletions(-)

diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 32a6082..8b1df33 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -987,10 +987,33 @@ static void cayman_cp_fini(struct radeon_device *rdev)
 
 int cayman_cp_resume(struct radeon_device *rdev)
 {
+	static const int ridx[] = {
+		RADEON_RING_TYPE_GFX_INDEX,
+		CAYMAN_RING_TYPE_CP1_INDEX,
+		CAYMAN_RING_TYPE_CP2_INDEX
+	};
+	static const unsigned cp_rb_cntl[] = {
+		CP_RB0_CNTL,
+		CP_RB1_CNTL,
+		CP_RB2_CNTL,
+	};
+	static const unsigned cp_rb_rptr_addr[] = {
+		CP_RB0_RPTR_ADDR,
+		CP_RB1_RPTR_ADDR,
+		CP_RB2_RPTR_ADDR
+	};
+	static const unsigned cp_rb_rptr_addr_hi[] = {
+		CP_RB0_RPTR_ADDR_HI,
+		CP_RB1_RPTR_ADDR_HI,
+		CP_RB2_RPTR_ADDR_HI
+	};
+	static const unsigned cp_rb_base[] = {
+		CP_RB0_BASE,
+		CP_RB1_BASE,
+		CP_RB2_BASE
+	};
 	struct radeon_ring *ring;
-	u32 tmp;
-	u32 rb_bufsz;
-	int r;
+	int i, r;
 
 	/* Reset cp; if cp is reset, then PA, SH, VGT also need to be reset */
 	WREG32(GRBM_SOFT_RESET, (SOFT_RESET_CP |
@@ -1012,91 +1035,47 @@ int cayman_cp_resume(struct radeon_device *rdev)
 
 	WREG32(CP_DEBUG, (1 << 27));
 
-	/* ring 0 - compute and gfx */
-	/* Set ring buffer size */
-	ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
-	rb_bufsz = drm_order(ring->ring_size / 8);
-	tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
-#ifdef __BIG_ENDIAN
-	tmp |= BUF_SWAP_32BIT;
-#endif
-	WREG32(CP_RB0_CNTL, tmp);
-
-	/* Initialize the ring buffer's read and write pointers */
-	WREG32(CP_RB0_CNTL, tmp | RB_RPTR_WR_ENA);
-	ring->wptr = 0;
-	WREG32(CP_RB0_WPTR, ring->wptr);
-
 	/* set the wb address wether it's enabled or not */
-	WREG32(CP_RB0_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFFFFFFFC);
-	WREG32(CP_RB0_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFF);
 	WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
+	WREG32(SCRATCH_UMSK, 0xff);
 
-	if (rdev->wb.enabled)
-		WREG32(SCRATCH_UMSK, 0xff);
-	else {
-		tmp |= RB_NO_UPDATE;
-		WREG32(SCRATCH_UMSK, 0);
-	}
-
-	mdelay(1);
-	WREG32(CP_RB0_CNTL, tmp);
-
-	WREG32(CP_RB0_BASE, ring->gpu_addr >> 8);
-
-	ring->rptr = RREG32(CP_RB0_RPTR);
+	for (i = 0; i < 3; ++i) {
+		uint32_t rb_cntl;
+		uint64_t addr;
 
-	/* ring1  - compute only */
-	/* Set ring buffer size */
-	ring = &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX];
-	rb_bufsz = drm_order(ring->ring_size / 8);
-	tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
+		/* Set ring buffer size */
+		ring = &rdev->ring[ridx[i]];
+		rb_cntl = drm_order(ring->ring_size / 8);
+		rb_cntl |= drm_order(RADEON_GPU_PAGE_SIZE/8) << 8;
 #ifdef __BIG_ENDIAN
-	tmp |= BUF_SWAP_32BIT;
+		rb_cntl |= BUF_SWAP_32BIT;
 #endif
-	WREG32(CP_RB1_CNTL, tmp);
+		WREG32(cp_rb_cntl[i], rb_cntl);
 
-	/* Initialize the ring buffer's read and write pointers */
-	WREG32(CP_RB1_CNTL, tmp | RB_RPTR_WR_ENA);
-	ring->wptr = 0;
-	WREG32(CP_RB1_WPTR, ring->wptr);
-
-	/* set the wb address wether it's enabled or not */
-	WREG32(CP_RB1_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFFFFFFFC);
-	WREG32(CP_RB1_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFF);
-
-	mdelay(1);
-	WREG32(CP_RB1_CNTL, tmp);
-
-	WREG32(CP_RB1_BASE, ring->gpu_addr >> 8);
-
-	ring->rptr = RREG32(CP_RB1_RPTR);
-
-	/* ring2 - compute only */
-	/* Set ring buffer size */
-	ring = &rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX];
-	rb_bufsz = drm_order(ring->ring_size / 8);
-	tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
-#ifdef __BIG_ENDIAN
-	tmp |= BUF_SWAP_32BIT;
-#endif
-	WREG32(CP_RB2_CNTL, tmp);
-
-	/* Initialize the ring buffer's read and write pointers */
-	WREG32(CP_RB2_CNTL, tmp | RB_RPTR_WR_ENA);
-	ring->wptr = 0;
-	WREG32(CP_RB2_WPTR, ring->wptr);
+		/* set the wb address wether it's enabled or not */
+		addr = rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET;
+		WREG32(cp_rb_rptr_addr[i], addr & 0xFFFFFFFC);
+		WREG32(cp_rb_rptr_addr_hi[i], upper_32_bits(addr) & 0xFF);
+	}
 
-	/* set the wb address wether it's enabled or not */
-	WREG32(CP_RB2_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFFFFFFFC);
-	WREG32(CP_RB2_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFF);
+	/* set the rb base addr, this causes an internal reset of ALL rings */
+	for (i = 0; i < 3; ++i) {
+		ring = &rdev->ring[ridx[i]];
+		WREG32(cp_rb_base[i], ring->gpu_addr >> 8);
+	}
 
-	mdelay(1);
-	WREG32(CP_RB2_CNTL, tmp);
+	for (i = 0; i < 3; ++i) {
+		/* Initialize the ring buffer's read and write pointers */
+		ring = &rdev->ring[ridx[i]];
+		WREG32_P(cp_rb_cntl[i], RB_RPTR_WR_ENA, ~RB_RPTR_WR_ENA);
 
-	WREG32(CP_RB2_BASE, ring->gpu_addr >> 8);
+		ring->rptr = ring->wptr = 0;
+		WREG32(ring->rptr_reg, ring->rptr);
+		WREG32(ring->wptr_reg, ring->wptr);
 
-	ring->rptr = RREG32(CP_RB2_RPTR);
+		mdelay(1);
+		WREG32_P(cp_rb_cntl[i], 0, ~RB_RPTR_WR_ENA);
+	}
 
 	/* start the rings */
 	cayman_cp_start(rdev);
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/16] drm/radeon: remove ip_pool start/suspend

[Christian König]

The IB pool is in gart memory, so it is completely
superfluous to unpin / repin it on suspend / resume.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/evergreen.c   |   17 ++++++-----------
 drivers/gpu/drm/radeon/ni.c          |   16 ++++++----------
 drivers/gpu/drm/radeon/r100.c        |   23 ++++++-----------------
 drivers/gpu/drm/radeon/r300.c        |   17 ++++++-----------
 drivers/gpu/drm/radeon/r420.c        |   17 ++++++-----------
 drivers/gpu/drm/radeon/r520.c        |   14 +++++---------
 drivers/gpu/drm/radeon/r600.c        |   17 ++++++-----------
 drivers/gpu/drm/radeon/radeon.h      |    2 --
 drivers/gpu/drm/radeon/radeon_asic.h |    1 -
 drivers/gpu/drm/radeon/radeon_ring.c |   17 +++++++----------
 drivers/gpu/drm/radeon/rs400.c       |   17 ++++++-----------
 drivers/gpu/drm/radeon/rs600.c       |   17 ++++++-----------
 drivers/gpu/drm/radeon/rs690.c       |   17 ++++++-----------
 drivers/gpu/drm/radeon/rv515.c       |   16 ++++++----------
 drivers/gpu/drm/radeon/rv770.c       |   17 ++++++-----------
 drivers/gpu/drm/radeon/si.c          |   16 ++++++----------
 16 files changed, 84 insertions(+), 157 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index eb9a71a..64e06e6 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3087,9 +3087,11 @@ static int evergreen_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -3137,7 +3139,6 @@ int evergreen_suspend(struct radeon_device *rdev)
 	struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
 
 	r600_audio_fini(rdev);
-	radeon_ib_pool_suspend(rdev);
 	r600_blit_suspend(rdev);
 	r700_cp_stop(rdev);
 	ring->ready = false;
@@ -3224,20 +3225,14 @@ int evergreen_init(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = evergreen_startup(rdev);
 	if (r) {
 		dev_err(rdev->dev, "disabling GPU acceleration\n");
 		r700_cp_fini(rdev);
 		r600_irq_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		evergreen_pcie_gart_fini(rdev);
 		rdev->accel_working = false;
@@ -3264,7 +3259,7 @@ void evergreen_fini(struct radeon_device *rdev)
 	r700_cp_fini(rdev);
 	r600_irq_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_irq_kms_fini(rdev);
 	evergreen_pcie_gart_fini(rdev);
 	r600_vram_scratch_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 8b1df33..fe55310 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1270,9 +1270,11 @@ static int cayman_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -1313,7 +1315,6 @@ int cayman_resume(struct radeon_device *rdev)
 int cayman_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
-	radeon_ib_pool_suspend(rdev);
 	radeon_vm_manager_suspend(rdev);
 	r600_blit_suspend(rdev);
 	cayman_cp_enable(rdev, false);
@@ -1391,12 +1392,7 @@ int cayman_init(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
 	r = radeon_vm_manager_init(rdev);
 	if (r) {
 		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
@@ -1410,7 +1406,7 @@ int cayman_init(struct radeon_device *rdev)
 		if (rdev->flags & RADEON_IS_IGP)
 			si_rlc_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_vm_manager_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		cayman_pcie_gart_fini(rdev);
@@ -1441,7 +1437,7 @@ void cayman_fini(struct radeon_device *rdev)
 		si_rlc_fini(rdev);
 	radeon_wb_fini(rdev);
 	radeon_vm_manager_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_irq_kms_fini(rdev);
 	cayman_pcie_gart_fini(rdev);
 	r600_vram_scratch_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index d06c8dd..9524bd4 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -3722,12 +3722,6 @@ int r100_ib_test(struct radeon_device *rdev, struct radeon_ring *ring)
 	return r;
 }
 
-void r100_ib_fini(struct radeon_device *rdev)
-{
-	radeon_ib_pool_suspend(rdev);
-	radeon_ib_pool_fini(rdev);
-}
-
 void r100_mc_stop(struct radeon_device *rdev, struct r100_mc_save *save)
 {
 	/* Shutdown CP we shouldn't need to do that but better be safe than
@@ -3887,9 +3881,11 @@ static int r100_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -3930,7 +3926,6 @@ int r100_resume(struct radeon_device *rdev)
 
 int r100_suspend(struct radeon_device *rdev)
 {
-	radeon_ib_pool_suspend(rdev);
 	r100_cp_disable(rdev);
 	radeon_wb_disable(rdev);
 	r100_irq_disable(rdev);
@@ -3943,7 +3938,7 @@ void r100_fini(struct radeon_device *rdev)
 {
 	r100_cp_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_gem_fini(rdev);
 	if (rdev->flags & RADEON_IS_PCI)
 		r100_pci_gart_fini(rdev);
@@ -4050,20 +4045,14 @@ int r100_init(struct radeon_device *rdev)
 	}
 	r100_set_safe_registers(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = r100_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		if (rdev->flags & RADEON_IS_PCI)
 			r100_pci_gart_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index 97722a3..b396e34 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -1391,9 +1391,11 @@ static int r300_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -1436,7 +1438,6 @@ int r300_resume(struct radeon_device *rdev)
 
 int r300_suspend(struct radeon_device *rdev)
 {
-	radeon_ib_pool_suspend(rdev);
 	r100_cp_disable(rdev);
 	radeon_wb_disable(rdev);
 	r100_irq_disable(rdev);
@@ -1451,7 +1452,7 @@ void r300_fini(struct radeon_device *rdev)
 {
 	r100_cp_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_gem_fini(rdev);
 	if (rdev->flags & RADEON_IS_PCIE)
 		rv370_pcie_gart_fini(rdev);
@@ -1538,20 +1539,14 @@ int r300_init(struct radeon_device *rdev)
 	}
 	r300_set_reg_safe(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = r300_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		if (rdev->flags & RADEON_IS_PCIE)
 			rv370_pcie_gart_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
index 99137be..0062938 100644
--- a/drivers/gpu/drm/radeon/r420.c
+++ b/drivers/gpu/drm/radeon/r420.c
@@ -275,9 +275,11 @@ static int r420_startup(struct radeon_device *rdev)
 	}
 	r420_cp_errata_init(rdev);
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -324,7 +326,6 @@ int r420_resume(struct radeon_device *rdev)
 
 int r420_suspend(struct radeon_device *rdev)
 {
-	radeon_ib_pool_suspend(rdev);
 	r420_cp_errata_fini(rdev);
 	r100_cp_disable(rdev);
 	radeon_wb_disable(rdev);
@@ -340,7 +341,7 @@ void r420_fini(struct radeon_device *rdev)
 {
 	r100_cp_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_gem_fini(rdev);
 	if (rdev->flags & RADEON_IS_PCIE)
 		rv370_pcie_gart_fini(rdev);
@@ -438,20 +439,14 @@ int r420_init(struct radeon_device *rdev)
 	}
 	r420_set_reg_safe(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = r420_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		if (rdev->flags & RADEON_IS_PCIE)
 			rv370_pcie_gart_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/r520.c b/drivers/gpu/drm/radeon/r520.c
index b5cf837..6df3e51 100644
--- a/drivers/gpu/drm/radeon/r520.c
+++ b/drivers/gpu/drm/radeon/r520.c
@@ -203,9 +203,11 @@ static int r520_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -311,20 +313,14 @@ int r520_init(struct radeon_device *rdev)
 		return r;
 	rv515_set_safe_registers(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = r520_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		rv370_pcie_gart_fini(rdev);
 		radeon_agp_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index de4de2d..9750f53 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2403,9 +2403,11 @@ int r600_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -2459,7 +2461,6 @@ int r600_resume(struct radeon_device *rdev)
 int r600_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
-	radeon_ib_pool_suspend(rdev);
 	r600_blit_suspend(rdev);
 	r600_cp_stop(rdev);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
@@ -2542,20 +2543,14 @@ int r600_init(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = r600_startup(rdev);
 	if (r) {
 		dev_err(rdev->dev, "disabling GPU acceleration\n");
 		r600_cp_fini(rdev);
 		r600_irq_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		r600_pcie_gart_fini(rdev);
 		rdev->accel_working = false;
@@ -2571,7 +2566,7 @@ void r600_fini(struct radeon_device *rdev)
 	r600_cp_fini(rdev);
 	r600_irq_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_irq_kms_fini(rdev);
 	r600_pcie_gart_fini(rdev);
 	r600_vram_scratch_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 4487873..7df76b9 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -755,8 +755,6 @@ void radeon_ib_free(struct radeon_device *rdev, struct radeon_ib *ib);
 int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib);
 int radeon_ib_pool_init(struct radeon_device *rdev);
 void radeon_ib_pool_fini(struct radeon_device *rdev);
-int radeon_ib_pool_start(struct radeon_device *rdev);
-int radeon_ib_pool_suspend(struct radeon_device *rdev);
 int radeon_ib_ring_tests(struct radeon_device *rdev);
 /* Ring access between begin & end cannot sleep */
 int radeon_ring_index(struct radeon_device *rdev, struct radeon_ring *cp);
diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
index 94c427a..f4af243 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.h
+++ b/drivers/gpu/drm/radeon/radeon_asic.h
@@ -103,7 +103,6 @@ int r100_pci_gart_enable(struct radeon_device *rdev);
 void r100_pci_gart_disable(struct radeon_device *rdev);
 int r100_debugfs_mc_info_init(struct radeon_device *rdev);
 int r100_gui_wait_for_idle(struct radeon_device *rdev);
-void r100_ib_fini(struct radeon_device *rdev);
 int r100_ib_test(struct radeon_device *rdev, struct radeon_ring *ring);
 void r100_irq_disable(struct radeon_device *rdev);
 void r100_mc_stop(struct radeon_device *rdev, struct r100_mc_save *save);
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 674aaba..0873834 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -129,6 +129,12 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
 	if (r) {
 		return r;
 	}
+
+	r = radeon_sa_bo_manager_start(rdev, &rdev->ring_tmp_bo);
+	if (r) {
+		return r;
+	}
+
 	rdev->ib_pool_ready = true;
 	if (radeon_debugfs_sa_init(rdev)) {
 		dev_err(rdev->dev, "failed to register debugfs file for SA\n");
@@ -139,21 +145,12 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
 void radeon_ib_pool_fini(struct radeon_device *rdev)
 {
 	if (rdev->ib_pool_ready) {
+		radeon_sa_bo_manager_suspend(rdev, &rdev->ring_tmp_bo);
 		radeon_sa_bo_manager_fini(rdev, &rdev->ring_tmp_bo);
 		rdev->ib_pool_ready = false;
 	}
 }
 
-int radeon_ib_pool_start(struct radeon_device *rdev)
-{
-	return radeon_sa_bo_manager_start(rdev, &rdev->ring_tmp_bo);
-}
-
-int radeon_ib_pool_suspend(struct radeon_device *rdev)
-{
-	return radeon_sa_bo_manager_suspend(rdev, &rdev->ring_tmp_bo);
-}
-
 int radeon_ib_ring_tests(struct radeon_device *rdev)
 {
 	unsigned i;
diff --git a/drivers/gpu/drm/radeon/rs400.c b/drivers/gpu/drm/radeon/rs400.c
index a464eb5..aa26076 100644
--- a/drivers/gpu/drm/radeon/rs400.c
+++ b/drivers/gpu/drm/radeon/rs400.c
@@ -426,9 +426,11 @@ static int rs400_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -470,7 +472,6 @@ int rs400_resume(struct radeon_device *rdev)
 
 int rs400_suspend(struct radeon_device *rdev)
 {
-	radeon_ib_pool_suspend(rdev);
 	r100_cp_disable(rdev);
 	radeon_wb_disable(rdev);
 	r100_irq_disable(rdev);
@@ -482,7 +483,7 @@ void rs400_fini(struct radeon_device *rdev)
 {
 	r100_cp_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_gem_fini(rdev);
 	rs400_gart_fini(rdev);
 	radeon_irq_kms_fini(rdev);
@@ -550,20 +551,14 @@ int rs400_init(struct radeon_device *rdev)
 		return r;
 	r300_set_reg_safe(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = rs400_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		rs400_gart_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		rdev->accel_working = false;
diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
index e11bc46..6dad4e6 100644
--- a/drivers/gpu/drm/radeon/rs600.c
+++ b/drivers/gpu/drm/radeon/rs600.c
@@ -907,9 +907,11 @@ static int rs600_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -955,7 +957,6 @@ int rs600_resume(struct radeon_device *rdev)
 
 int rs600_suspend(struct radeon_device *rdev)
 {
-	radeon_ib_pool_suspend(rdev);
 	r600_audio_fini(rdev);
 	r100_cp_disable(rdev);
 	radeon_wb_disable(rdev);
@@ -969,7 +970,7 @@ void rs600_fini(struct radeon_device *rdev)
 	r600_audio_fini(rdev);
 	r100_cp_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_gem_fini(rdev);
 	rs600_gart_fini(rdev);
 	radeon_irq_kms_fini(rdev);
@@ -1037,20 +1038,14 @@ int rs600_init(struct radeon_device *rdev)
 		return r;
 	rs600_set_safe_registers(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = rs600_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		rs600_gart_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		rdev->accel_working = false;
diff --git a/drivers/gpu/drm/radeon/rs690.c b/drivers/gpu/drm/radeon/rs690.c
index 159b6a4..0c026b0 100644
--- a/drivers/gpu/drm/radeon/rs690.c
+++ b/drivers/gpu/drm/radeon/rs690.c
@@ -637,9 +637,11 @@ static int rs690_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -685,7 +687,6 @@ int rs690_resume(struct radeon_device *rdev)
 
 int rs690_suspend(struct radeon_device *rdev)
 {
-	radeon_ib_pool_suspend(rdev);
 	r600_audio_fini(rdev);
 	r100_cp_disable(rdev);
 	radeon_wb_disable(rdev);
@@ -699,7 +700,7 @@ void rs690_fini(struct radeon_device *rdev)
 	r600_audio_fini(rdev);
 	r100_cp_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_gem_fini(rdev);
 	rs400_gart_fini(rdev);
 	radeon_irq_kms_fini(rdev);
@@ -768,20 +769,14 @@ int rs690_init(struct radeon_device *rdev)
 		return r;
 	rs600_set_safe_registers(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = rs690_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		rs400_gart_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		rdev->accel_working = false;
diff --git a/drivers/gpu/drm/radeon/rv515.c b/drivers/gpu/drm/radeon/rv515.c
index 7f08ced..01e9155 100644
--- a/drivers/gpu/drm/radeon/rv515.c
+++ b/drivers/gpu/drm/radeon/rv515.c
@@ -408,9 +408,11 @@ static int rv515_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -469,7 +471,7 @@ void rv515_fini(struct radeon_device *rdev)
 {
 	r100_cp_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_gem_fini(rdev);
 	rv370_pcie_gart_fini(rdev);
 	radeon_agp_fini(rdev);
@@ -543,20 +545,14 @@ int rv515_init(struct radeon_device *rdev)
 		return r;
 	rv515_set_safe_registers(rdev);
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = rv515_startup(rdev);
 	if (r) {
 		/* Somethings want wront with the accel init stop accel */
 		dev_err(rdev->dev, "Disabling GPU acceleration\n");
 		r100_cp_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		rv370_pcie_gart_fini(rdev);
 		radeon_agp_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index 7e230f6..cc0ffb9 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -951,9 +951,11 @@ static int rv770_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_ring_tests(rdev);
 	if (r)
@@ -994,7 +996,6 @@ int rv770_resume(struct radeon_device *rdev)
 int rv770_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
-	radeon_ib_pool_suspend(rdev);
 	r600_blit_suspend(rdev);
 	r700_cp_stop(rdev);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
@@ -1075,20 +1076,14 @@ int rv770_init(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
-
 	r = rv770_startup(rdev);
 	if (r) {
 		dev_err(rdev->dev, "disabling GPU acceleration\n");
 		r700_cp_fini(rdev);
 		r600_irq_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		rv770_pcie_gart_fini(rdev);
 		rdev->accel_working = false;
@@ -1103,7 +1098,7 @@ void rv770_fini(struct radeon_device *rdev)
 	r700_cp_fini(rdev);
 	r600_irq_fini(rdev);
 	radeon_wb_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_irq_kms_fini(rdev);
 	rv770_pcie_gart_fini(rdev);
 	r600_vram_scratch_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 78c790f..40405d3 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -3750,9 +3750,11 @@ static int si_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_start(rdev);
-	if (r)
+	r = radeon_ib_pool_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
 	if (r) {
@@ -3807,7 +3809,6 @@ int si_resume(struct radeon_device *rdev)
 
 int si_suspend(struct radeon_device *rdev)
 {
-	radeon_ib_pool_suspend(rdev);
 	radeon_vm_manager_suspend(rdev);
 #if 0
 	r600_blit_suspend(rdev);
@@ -3900,12 +3901,7 @@ int si_init(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_pool_init(rdev);
 	rdev->accel_working = true;
-	if (r) {
-		dev_err(rdev->dev, "IB initialization failed (%d).\n", r);
-		rdev->accel_working = false;
-	}
 	r = radeon_vm_manager_init(rdev);
 	if (r) {
 		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
@@ -3918,7 +3914,7 @@ int si_init(struct radeon_device *rdev)
 		si_irq_fini(rdev);
 		si_rlc_fini(rdev);
 		radeon_wb_fini(rdev);
-		r100_ib_fini(rdev);
+		radeon_ib_pool_fini(rdev);
 		radeon_vm_manager_fini(rdev);
 		radeon_irq_kms_fini(rdev);
 		si_pcie_gart_fini(rdev);
@@ -3947,7 +3943,7 @@ void si_fini(struct radeon_device *rdev)
 	si_rlc_fini(rdev);
 	radeon_wb_fini(rdev);
 	radeon_vm_manager_fini(rdev);
-	r100_ib_fini(rdev);
+	radeon_ib_pool_fini(rdev);
 	radeon_irq_kms_fini(rdev);
 	si_pcie_gart_fini(rdev);
 	r600_vram_scratch_fini(rdev);
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/16] drm/radeon: remove r600_blit_suspend

[Christian König]

Just reinitialize the shader content on resume instead.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/evergreen.c          |    1 -
 drivers/gpu/drm/radeon/evergreen_blit_kms.c |   40 +++++++++++++--------------
 drivers/gpu/drm/radeon/ni.c                 |    1 -
 drivers/gpu/drm/radeon/r600.c               |   15 ----------
 drivers/gpu/drm/radeon/r600_blit_kms.c      |   40 +++++++++++++--------------
 drivers/gpu/drm/radeon/radeon.h             |    2 --
 drivers/gpu/drm/radeon/rv770.c              |    1 -
 drivers/gpu/drm/radeon/si.c                 |    3 --
 8 files changed, 40 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 64e06e6..82f7aea 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3139,7 +3139,6 @@ int evergreen_suspend(struct radeon_device *rdev)
 	struct radeon_ring *ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
 
 	r600_audio_fini(rdev);
-	r600_blit_suspend(rdev);
 	r700_cp_stop(rdev);
 	ring->ready = false;
 	evergreen_irq_suspend(rdev);
diff --git a/drivers/gpu/drm/radeon/evergreen_blit_kms.c b/drivers/gpu/drm/radeon/evergreen_blit_kms.c
index e512560..89cb9fe 100644
--- a/drivers/gpu/drm/radeon/evergreen_blit_kms.c
+++ b/drivers/gpu/drm/radeon/evergreen_blit_kms.c
@@ -634,10 +634,6 @@ int evergreen_blit_init(struct radeon_device *rdev)
 
 	rdev->r600_blit.max_dim = 16384;
 
-	/* pin copy shader into vram if already initialized */
-	if (rdev->r600_blit.shader_obj)
-		goto done;
-
 	rdev->r600_blit.state_offset = 0;
 
 	if (rdev->family < CHIP_CAYMAN)
@@ -668,11 +664,26 @@ int evergreen_blit_init(struct radeon_device *rdev)
 		obj_size += cayman_ps_size * 4;
 	obj_size = ALIGN(obj_size, 256);
 
-	r = radeon_bo_create(rdev, obj_size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
-			     NULL, &rdev->r600_blit.shader_obj);
-	if (r) {
-		DRM_ERROR("evergreen failed to allocate shader\n");
-		return r;
+	/* pin copy shader into vram if not already initialized */
+	if (!rdev->r600_blit.shader_obj) {
+		r = radeon_bo_create(rdev, obj_size, PAGE_SIZE, true,
+				     RADEON_GEM_DOMAIN_VRAM,
+				     NULL, &rdev->r600_blit.shader_obj);
+		if (r) {
+			DRM_ERROR("evergreen failed to allocate shader\n");
+			return r;
+		}
+
+		r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
+		if (unlikely(r != 0))
+			return r;
+		r = radeon_bo_pin(rdev->r600_blit.shader_obj, RADEON_GEM_DOMAIN_VRAM,
+				  &rdev->r600_blit.shader_gpu_addr);
+		radeon_bo_unreserve(rdev->r600_blit.shader_obj);
+		if (r) {
+			dev_err(rdev->dev, "(%d) pin blit object failed\n", r);
+			return r;
+		}
 	}
 
 	DRM_DEBUG("evergreen blit allocated bo %08x vs %08x ps %08x\n",
@@ -714,17 +725,6 @@ int evergreen_blit_init(struct radeon_device *rdev)
 	radeon_bo_kunmap(rdev->r600_blit.shader_obj);
 	radeon_bo_unreserve(rdev->r600_blit.shader_obj);
 
-done:
-	r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
-	if (unlikely(r != 0))
-		return r;
-	r = radeon_bo_pin(rdev->r600_blit.shader_obj, RADEON_GEM_DOMAIN_VRAM,
-			  &rdev->r600_blit.shader_gpu_addr);
-	radeon_bo_unreserve(rdev->r600_blit.shader_obj);
-	if (r) {
-		dev_err(rdev->dev, "(%d) pin blit object failed\n", r);
-		return r;
-	}
 	radeon_ttm_set_active_vram_size(rdev, rdev->mc.real_vram_size);
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index fe55310..4004376 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1316,7 +1316,6 @@ int cayman_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
 	radeon_vm_manager_suspend(rdev);
-	r600_blit_suspend(rdev);
 	cayman_cp_enable(rdev, false);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	evergreen_irq_suspend(rdev);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 9750f53..af2f74a 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2307,20 +2307,6 @@ int r600_copy_blit(struct radeon_device *rdev,
 	return 0;
 }
 
-void r600_blit_suspend(struct radeon_device *rdev)
-{
-	int r;
-
-	/* unpin shaders bo */
-	if (rdev->r600_blit.shader_obj) {
-		r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
-		if (!r) {
-			radeon_bo_unpin(rdev->r600_blit.shader_obj);
-			radeon_bo_unreserve(rdev->r600_blit.shader_obj);
-		}
-	}
-}
-
 int r600_set_surface_reg(struct radeon_device *rdev, int reg,
 			 uint32_t tiling_flags, uint32_t pitch,
 			 uint32_t offset, uint32_t obj_size)
@@ -2461,7 +2447,6 @@ int r600_resume(struct radeon_device *rdev)
 int r600_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
-	r600_blit_suspend(rdev);
 	r600_cp_stop(rdev);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	r600_irq_suspend(rdev);
diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c b/drivers/gpu/drm/radeon/r600_blit_kms.c
index 2b8d641..2bef854 100644
--- a/drivers/gpu/drm/radeon/r600_blit_kms.c
+++ b/drivers/gpu/drm/radeon/r600_blit_kms.c
@@ -524,10 +524,6 @@ int r600_blit_init(struct radeon_device *rdev)
 
 	rdev->r600_blit.max_dim = 8192;
 
-	/* pin copy shader into vram if already initialized */
-	if (rdev->r600_blit.shader_obj)
-		goto done;
-
 	rdev->r600_blit.state_offset = 0;
 
 	if (rdev->family >= CHIP_RV770)
@@ -552,11 +548,26 @@ int r600_blit_init(struct radeon_device *rdev)
 	obj_size += r6xx_ps_size * 4;
 	obj_size = ALIGN(obj_size, 256);
 
-	r = radeon_bo_create(rdev, obj_size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
-			     NULL, &rdev->r600_blit.shader_obj);
-	if (r) {
-		DRM_ERROR("r600 failed to allocate shader\n");
-		return r;
+	/* pin copy shader into vram if not already initialized */
+	if (rdev->r600_blit.shader_obj == NULL) {
+		r = radeon_bo_create(rdev, obj_size, PAGE_SIZE, true,
+				     RADEON_GEM_DOMAIN_VRAM,
+				     NULL, &rdev->r600_blit.shader_obj);
+		if (r) {
+			DRM_ERROR("r600 failed to allocate shader\n");
+			return r;
+		}
+
+		r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
+		if (unlikely(r != 0))
+			return r;
+		r = radeon_bo_pin(rdev->r600_blit.shader_obj, RADEON_GEM_DOMAIN_VRAM,
+				  &rdev->r600_blit.shader_gpu_addr);
+		radeon_bo_unreserve(rdev->r600_blit.shader_obj);
+		if (r) {
+			dev_err(rdev->dev, "(%d) pin blit object failed\n", r);
+			return r;
+		}
 	}
 
 	DRM_DEBUG("r6xx blit allocated bo %08x vs %08x ps %08x\n",
@@ -587,17 +598,6 @@ int r600_blit_init(struct radeon_device *rdev)
 	radeon_bo_kunmap(rdev->r600_blit.shader_obj);
 	radeon_bo_unreserve(rdev->r600_blit.shader_obj);
 
-done:
-	r = radeon_bo_reserve(rdev->r600_blit.shader_obj, false);
-	if (unlikely(r != 0))
-		return r;
-	r = radeon_bo_pin(rdev->r600_blit.shader_obj, RADEON_GEM_DOMAIN_VRAM,
-			  &rdev->r600_blit.shader_gpu_addr);
-	radeon_bo_unreserve(rdev->r600_blit.shader_obj);
-	if (r) {
-		dev_err(rdev->dev, "(%d) pin blit object failed\n", r);
-		return r;
-	}
 	radeon_ttm_set_active_vram_size(rdev, rdev->mc.real_vram_size);
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 7df76b9..8a8c3f8 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -735,8 +735,6 @@ struct r600_blit {
 	u32 state_len;
 };
 
-void r600_blit_suspend(struct radeon_device *rdev);
-
 /*
  * SI RLC stuff
  */
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index cc0ffb9..2004f0d 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -996,7 +996,6 @@ int rv770_resume(struct radeon_device *rdev)
 int rv770_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
-	r600_blit_suspend(rdev);
 	r700_cp_stop(rdev);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	r600_irq_suspend(rdev);
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 40405d3..7c2618b 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -3810,9 +3810,6 @@ int si_resume(struct radeon_device *rdev)
 int si_suspend(struct radeon_device *rdev)
 {
 	radeon_vm_manager_suspend(rdev);
-#if 0
-	r600_blit_suspend(rdev);
-#endif
 	si_cp_enable(rdev, false);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX].ready = false;
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/16] drm/radeon: remove vm_manager start/suspend

[Christian König]

Just restore the page table instead. Addressing three
problem with this change:

1. Calling vm_manager_suspend in the suspend path is
   problematic cause it wants to wait for the VM use
   to end, which in case of a lockup never happens.

2. In case of a locked up memory controller
   unbinding the VM seems to make it even more
   unstable, creating an unrecoverable lockup
   in the end.

3. If we want to backup/restore the leftover ring
   content we must not unbind VMs in between.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/ni.c          |   12 ++---
 drivers/gpu/drm/radeon/radeon.h      |    2 -
 drivers/gpu/drm/radeon/radeon_gart.c |   83 +++++++++++++++++++++-------------
 drivers/gpu/drm/radeon/si.c          |   12 ++---
 4 files changed, 59 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 4004376..ec5307c 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1280,9 +1280,11 @@ static int cayman_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_vm_manager_start(rdev);
-	if (r)
+	r = radeon_vm_manager_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
 		return r;
+	}
 
 	r = r600_audio_init(rdev);
 	if (r)
@@ -1315,7 +1317,6 @@ int cayman_resume(struct radeon_device *rdev)
 int cayman_suspend(struct radeon_device *rdev)
 {
 	r600_audio_fini(rdev);
-	radeon_vm_manager_suspend(rdev);
 	cayman_cp_enable(rdev, false);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	evergreen_irq_suspend(rdev);
@@ -1392,11 +1393,6 @@ int cayman_init(struct radeon_device *rdev)
 		return r;
 
 	rdev->accel_working = true;
-	r = radeon_vm_manager_init(rdev);
-	if (r) {
-		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
-	}
-
 	r = cayman_startup(rdev);
 	if (r) {
 		dev_err(rdev->dev, "disabling GPU acceleration\n");
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 8a8c3f8..872270c 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1759,8 +1759,6 @@ extern void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size
  */
 int radeon_vm_manager_init(struct radeon_device *rdev);
 void radeon_vm_manager_fini(struct radeon_device *rdev);
-int radeon_vm_manager_start(struct radeon_device *rdev);
-int radeon_vm_manager_suspend(struct radeon_device *rdev);
 int radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm);
 void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm);
 int radeon_vm_bind(struct radeon_device *rdev, struct radeon_vm *vm);
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index ee11c50..56752da 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -282,27 +282,58 @@ void radeon_gart_fini(struct radeon_device *rdev)
  *
  * TODO bind a default page at vm initialization for default address
  */
+
 int radeon_vm_manager_init(struct radeon_device *rdev)
 {
+	struct radeon_vm *vm;
+	struct radeon_bo_va *bo_va;
 	int r;
 
-	rdev->vm_manager.enabled = false;
+	if (!rdev->vm_manager.enabled) {
+		/* mark first vm as always in use, it's the system one */
+		r = radeon_sa_bo_manager_init(rdev, &rdev->vm_manager.sa_manager,
+					      rdev->vm_manager.max_pfn * 8,
+					      RADEON_GEM_DOMAIN_VRAM);
+		if (r) {
+			dev_err(rdev->dev, "failed to allocate vm bo (%dKB)\n",
+				(rdev->vm_manager.max_pfn * 8) >> 10);
+			return r;
+		}
 
-	/* mark first vm as always in use, it's the system one */
-	r = radeon_sa_bo_manager_init(rdev, &rdev->vm_manager.sa_manager,
-				      rdev->vm_manager.max_pfn * 8,
-				      RADEON_GEM_DOMAIN_VRAM);
-	if (r) {
-		dev_err(rdev->dev, "failed to allocate vm bo (%dKB)\n",
-			(rdev->vm_manager.max_pfn * 8) >> 10);
-		return r;
+		r = rdev->vm_manager.funcs->init(rdev);
+		if (r)
+			return r;
+	
+		rdev->vm_manager.enabled = true;
+
+		r = radeon_sa_bo_manager_start(rdev, &rdev->vm_manager.sa_manager);
+		if (r)
+			return r;
 	}
 
-	r = rdev->vm_manager.funcs->init(rdev);
-	if (r == 0)
-		rdev->vm_manager.enabled = true;
+	/* restore page table */
+	list_for_each_entry(vm, &rdev->vm_manager.lru_vm, list) {
+		if (vm->id == -1)
+			continue;
 
-	return r;
+		list_for_each_entry(bo_va, &vm->va, vm_list) {
+			struct ttm_mem_reg *mem = NULL;
+			if (bo_va->valid)
+				mem = &bo_va->bo->tbo.mem;
+
+			bo_va->valid = false;
+			r = radeon_vm_bo_update_pte(rdev, vm, bo_va->bo, mem);
+			if (r) {
+				DRM_ERROR("Failed to update pte for vm %d!\n", vm->id);
+			}
+		}
+
+		r = rdev->vm_manager.funcs->bind(rdev, vm, vm->id);
+		if (r) {
+			DRM_ERROR("Failed to bind vm %d!\n", vm->id);
+		}
+	}
+	return 0;
 }
 
 /* global mutex must be lock */
@@ -347,26 +378,11 @@ static void radeon_vm_unbind_locked(struct radeon_device *rdev,
 
 void radeon_vm_manager_fini(struct radeon_device *rdev)
 {
-	if (rdev->vm_manager.sa_manager.bo == NULL)
-		return;
-	radeon_vm_manager_suspend(rdev);
-	rdev->vm_manager.funcs->fini(rdev);
-	radeon_sa_bo_manager_fini(rdev, &rdev->vm_manager.sa_manager);
-	rdev->vm_manager.enabled = false;
-}
-
-int radeon_vm_manager_start(struct radeon_device *rdev)
-{
-	if (rdev->vm_manager.sa_manager.bo == NULL) {
-		return -EINVAL;
-	}
-	return radeon_sa_bo_manager_start(rdev, &rdev->vm_manager.sa_manager);
-}
-
-int radeon_vm_manager_suspend(struct radeon_device *rdev)
-{
 	struct radeon_vm *vm, *tmp;
 
+	if (!rdev->vm_manager.enabled)
+		return;
+
 	mutex_lock(&rdev->vm_manager.lock);
 	/* unbind all active vm */
 	list_for_each_entry_safe(vm, tmp, &rdev->vm_manager.lru_vm, list) {
@@ -374,7 +390,10 @@ int radeon_vm_manager_suspend(struct radeon_device *rdev)
 	}
 	rdev->vm_manager.funcs->fini(rdev);
 	mutex_unlock(&rdev->vm_manager.lock);
-	return radeon_sa_bo_manager_suspend(rdev, &rdev->vm_manager.sa_manager);
+
+	radeon_sa_bo_manager_suspend(rdev, &rdev->vm_manager.sa_manager);
+	radeon_sa_bo_manager_fini(rdev, &rdev->vm_manager.sa_manager);
+	rdev->vm_manager.enabled = false;
 }
 
 /* global mutex must be locked */
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 7c2618b..2b691ab 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -3777,9 +3777,11 @@ static int si_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_vm_manager_start(rdev);
-	if (r)
+	r = radeon_vm_manager_init(rdev);
+	if (r) {
+		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
 		return r;
+	}
 
 	return 0;
 }
@@ -3809,7 +3811,6 @@ int si_resume(struct radeon_device *rdev)
 
 int si_suspend(struct radeon_device *rdev)
 {
-	radeon_vm_manager_suspend(rdev);
 	si_cp_enable(rdev, false);
 	rdev->ring[RADEON_RING_TYPE_GFX_INDEX].ready = false;
 	rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX].ready = false;
@@ -3899,11 +3900,6 @@ int si_init(struct radeon_device *rdev)
 		return r;
 
 	rdev->accel_working = true;
-	r = radeon_vm_manager_init(rdev);
-	if (r) {
-		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
-	}
-
 	r = si_startup(rdev);
 	if (r) {
 		dev_err(rdev->dev, "disabling GPU acceleration\n");
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Christian König]

Making it easier to controlwhen it is executed.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/evergreen.c     |    4 ----
 drivers/gpu/drm/radeon/ni.c            |    4 ----
 drivers/gpu/drm/radeon/r100.c          |    4 ----
 drivers/gpu/drm/radeon/r300.c          |    4 ----
 drivers/gpu/drm/radeon/r420.c          |    4 ----
 drivers/gpu/drm/radeon/r520.c          |    4 ----
 drivers/gpu/drm/radeon/r600.c          |    4 ----
 drivers/gpu/drm/radeon/radeon_device.c |   15 +++++++++++++++
 drivers/gpu/drm/radeon/rs400.c         |    4 ----
 drivers/gpu/drm/radeon/rs600.c         |    4 ----
 drivers/gpu/drm/radeon/rs690.c         |    4 ----
 drivers/gpu/drm/radeon/rv515.c         |    4 ----
 drivers/gpu/drm/radeon/rv770.c         |    4 ----
 drivers/gpu/drm/radeon/si.c            |   21 ---------------------
 14 files changed, 15 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 82f7aea..f39b900 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3093,10 +3093,6 @@ static int evergreen_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	r = r600_audio_init(rdev);
 	if (r) {
 		DRM_ERROR("radeon: audio init failed\n");
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index ec5307c..f2afefb 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1276,10 +1276,6 @@ static int cayman_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	r = radeon_vm_manager_init(rdev);
 	if (r) {
 		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 9524bd4..e0f5ae8 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -3887,10 +3887,6 @@ static int r100_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index b396e34..646a192 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -1397,10 +1397,6 @@ static int r300_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
index 0062938..f2f5bf6 100644
--- a/drivers/gpu/drm/radeon/r420.c
+++ b/drivers/gpu/drm/radeon/r420.c
@@ -281,10 +281,6 @@ static int r420_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/r520.c b/drivers/gpu/drm/radeon/r520.c
index 6df3e51..079d3c5 100644
--- a/drivers/gpu/drm/radeon/r520.c
+++ b/drivers/gpu/drm/radeon/r520.c
@@ -209,10 +209,6 @@ static int r520_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index af2f74a..c808fa9 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2395,10 +2395,6 @@ int r600_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	r = r600_audio_init(rdev);
 	if (r) {
 		DRM_ERROR("radeon: audio init failed\n");
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
@@ -946,6 +950,7 @@ int radeon_resume_kms(struct drm_device *dev)
 {
 	struct drm_connector *connector;
 	struct radeon_device *rdev = dev->dev_private;
+	int r;
 
 	if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 		return 0;
@@ -960,6 +965,11 @@ int radeon_resume_kms(struct drm_device *dev)
 	/* resume AGP if in use */
 	radeon_agp_resume(rdev);
 	radeon_resume(rdev);
+
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
+		DRM_ERROR("ib ring test failed (%d).\n", r);
+
 	radeon_pm_resume(rdev);
 	radeon_restore_bios_scratch_regs(rdev);
 
@@ -999,6 +1009,11 @@ int radeon_gpu_reset(struct radeon_device *rdev)
 	if (!r) {
 		dev_info(rdev->dev, "GPU reset succeed\n");
 		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);
 		ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
diff --git a/drivers/gpu/drm/radeon/rs400.c b/drivers/gpu/drm/radeon/rs400.c
index aa26076..2752f7f 100644
--- a/drivers/gpu/drm/radeon/rs400.c
+++ b/drivers/gpu/drm/radeon/rs400.c
@@ -432,10 +432,6 @@ static int rs400_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
index 6dad4e6..5301b3d 100644
--- a/drivers/gpu/drm/radeon/rs600.c
+++ b/drivers/gpu/drm/radeon/rs600.c
@@ -913,10 +913,6 @@ static int rs600_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	r = r600_audio_init(rdev);
 	if (r) {
 		dev_err(rdev->dev, "failed initializing audio\n");
diff --git a/drivers/gpu/drm/radeon/rs690.c b/drivers/gpu/drm/radeon/rs690.c
index 0c026b0..3b663fc 100644
--- a/drivers/gpu/drm/radeon/rs690.c
+++ b/drivers/gpu/drm/radeon/rs690.c
@@ -643,10 +643,6 @@ static int rs690_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	r = r600_audio_init(rdev);
 	if (r) {
 		dev_err(rdev->dev, "failed initializing audio\n");
diff --git a/drivers/gpu/drm/radeon/rv515.c b/drivers/gpu/drm/radeon/rv515.c
index 01e9155..a12fbcc 100644
--- a/drivers/gpu/drm/radeon/rv515.c
+++ b/drivers/gpu/drm/radeon/rv515.c
@@ -414,10 +414,6 @@ static int rv515_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index 2004f0d..b4b1256 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -957,10 +957,6 @@ static int rv770_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_ring_tests(rdev);
-	if (r)
-		return r;
-
 	r = r600_audio_init(rdev);
 	if (r) {
 		DRM_ERROR("radeon: audio init failed\n");
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 2b691ab..f61b550 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -3756,27 +3756,6 @@ static int si_startup(struct radeon_device *rdev)
 		return r;
 	}
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		DRM_ERROR("radeon: failed testing IB (%d) on CP ring 0\n", r);
-		rdev->accel_working = false;
-		return r;
-	}
-
-	r = radeon_ib_test(rdev, CAYMAN_RING_TYPE_CP1_INDEX, &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX]);
-	if (r) {
-		DRM_ERROR("radeon: failed testing IB (%d) on CP ring 1\n", r);
-		rdev->accel_working = false;
-		return r;
-	}
-
-	r = radeon_ib_test(rdev, CAYMAN_RING_TYPE_CP2_INDEX, &rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX]);
-	if (r) {
-		DRM_ERROR("radeon: failed testing IB (%d) on CP ring 2\n", r);
-		rdev->accel_working = false;
-		return r;
-	}
-
 	r = radeon_vm_manager_init(rdev);
 	if (r) {
 		dev_err(rdev->dev, "vm manager initialization failed (%d).\n", r);
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/16] drm/radeon: make align a ring_init parameter

[Christian König]

Instead of setting it directly from the chipset code.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/evergreen.c   |    3 ++-
 drivers/gpu/drm/radeon/ni.c          |    3 ++-
 drivers/gpu/drm/radeon/r100.c        |    3 +--
 drivers/gpu/drm/radeon/r600.c        |    3 +--
 drivers/gpu/drm/radeon/radeon.h      |    3 ++-
 drivers/gpu/drm/radeon/radeon_ring.c |    4 +++-
 drivers/gpu/drm/radeon/rv770.c       |    3 ++-
 drivers/gpu/drm/radeon/si.c          |    9 ++++++---
 8 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index f39b900..f0bbbe1 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3075,7 +3075,8 @@ static int evergreen_startup(struct radeon_device *rdev)
 	}
 	evergreen_irq_set(rdev);
 
-	r = radeon_ring_init(rdev, ring, ring->ring_size, RADEON_WB_CP_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring->ring_size, 16,
+			     RADEON_WB_CP_RPTR_OFFSET,
 			     R600_CP_RB_RPTR, R600_CP_RB_WPTR,
 			     0, 0xfffff, RADEON_CP_PACKET2);
 	if (r)
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index f2afefb..c69cebc 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1258,7 +1258,8 @@ static int cayman_startup(struct radeon_device *rdev)
 	}
 	evergreen_irq_set(rdev);
 
-	r = radeon_ring_init(rdev, ring, ring->ring_size, RADEON_WB_CP_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring->ring_size, 16,
+			     RADEON_WB_CP_RPTR_OFFSET,
 			     CP_RB0_RPTR, CP_RB0_WPTR,
 			     0, 0xfffff, RADEON_CP_PACKET2);
 	if (r)
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index e0f5ae8..116432f 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -977,7 +977,7 @@ int r100_cp_init(struct radeon_device *rdev, unsigned ring_size)
 	rb_bufsz = drm_order(ring_size / 8);
 	ring_size = (1 << (rb_bufsz + 1)) * 4;
 	r100_cp_load_microcode(rdev);
-	r = radeon_ring_init(rdev, ring, ring_size, RADEON_WB_CP_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring_size, 16, RADEON_WB_CP_RPTR_OFFSET,
 			     RADEON_CP_RB_RPTR, RADEON_CP_RB_WPTR,
 			     0, 0x7fffff, RADEON_CP_PACKET2);
 	if (r) {
@@ -988,7 +988,6 @@ int r100_cp_init(struct radeon_device *rdev, unsigned ring_size)
 	rb_blksz = 9;
 	/* cp will read 128bytes at a time (4 dwords) */
 	max_fetch = 1;
-	ring->align_mask = 16 - 1;
 	/* Write to CP_RB_WPTR will be delayed for pre_write_timer clocks */
 	pre_write_timer = 64;
 	/* Force CP_RB_WPTR write if written more than one time before the
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index c808fa9..7d15490 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2160,7 +2160,6 @@ void r600_ring_init(struct radeon_device *rdev, struct radeon_ring *ring, unsign
 	rb_bufsz = drm_order(ring_size / 8);
 	ring_size = (1 << (rb_bufsz + 1)) * 4;
 	ring->ring_size = ring_size;
-	ring->align_mask = 16 - 1;
 }
 
 void r600_cp_fini(struct radeon_device *rdev)
@@ -2376,7 +2375,7 @@ int r600_startup(struct radeon_device *rdev)
 	}
 	r600_irq_set(rdev);
 
-	r = radeon_ring_init(rdev, ring, ring->ring_size, RADEON_WB_CP_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring->ring_size, 16, RADEON_WB_CP_RPTR_OFFSET,
 			     R600_CP_RB_RPTR, R600_CP_RB_WPTR,
 			     0, 0xfffff, RADEON_CP_PACKET2);
 
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 872270c..fef4257 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -767,7 +767,8 @@ int radeon_ring_test(struct radeon_device *rdev, struct radeon_ring *cp);
 void radeon_ring_force_activity(struct radeon_device *rdev, struct radeon_ring *ring);
 void radeon_ring_lockup_update(struct radeon_ring *ring);
 bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring);
-int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *cp, unsigned ring_size,
+int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *cp,
+		     unsigned ring_size, unsigned align,
 		     unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
 		     u32 ptr_reg_shift, u32 ptr_reg_mask, u32 nop);
 void radeon_ring_fini(struct radeon_device *rdev, struct radeon_ring *cp);
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 0873834..d9b2e45 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -362,13 +362,15 @@ bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 	return false;
 }
 
-int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring, unsigned ring_size,
+int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
+		     unsigned ring_size, unsigned align,
 		     unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
 		     u32 ptr_reg_shift, u32 ptr_reg_mask, u32 nop)
 {
 	int r;
 
 	ring->ring_size = ring_size;
+	ring->align_mask = align ? align - 1 : 0;
 	ring->rptr_offs = rptr_offs;
 	ring->rptr_reg = rptr_reg;
 	ring->wptr_reg = wptr_reg;
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index b4b1256..7f316cf 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -939,7 +939,8 @@ static int rv770_startup(struct radeon_device *rdev)
 	}
 	r600_irq_set(rdev);
 
-	r = radeon_ring_init(rdev, ring, ring->ring_size, RADEON_WB_CP_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring->ring_size, 16,
+			     RADEON_WB_CP_RPTR_OFFSET,
 			     R600_CP_RB_RPTR, R600_CP_RB_WPTR,
 			     0, 0xfffff, RADEON_CP_PACKET2);
 	if (r)
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index f61b550..491af42 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -3723,21 +3723,24 @@ static int si_startup(struct radeon_device *rdev)
 	si_irq_set(rdev);
 
 	ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
-	r = radeon_ring_init(rdev, ring, ring->ring_size, RADEON_WB_CP_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring->ring_size, 16,
+			     RADEON_WB_CP_RPTR_OFFSET,
 			     CP_RB0_RPTR, CP_RB0_WPTR,
 			     0, 0xfffff, RADEON_CP_PACKET2);
 	if (r)
 		return r;
 
 	ring = &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX];
-	r = radeon_ring_init(rdev, ring, ring->ring_size, RADEON_WB_CP1_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring->ring_size, 16,
+			     RADEON_WB_CP1_RPTR_OFFSET,
 			     CP_RB1_RPTR, CP_RB1_WPTR,
 			     0, 0xfffff, RADEON_CP_PACKET2);
 	if (r)
 		return r;
 
 	ring = &rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX];
-	r = radeon_ring_init(rdev, ring, ring->ring_size, RADEON_WB_CP2_RPTR_OFFSET,
+	r = radeon_ring_init(rdev, ring, ring->ring_size, 16,
+			     RADEON_WB_CP2_RPTR_OFFSET,
 			     CP_RB2_RPTR, CP_RB2_WPTR,
 			     0, 0xfffff, RADEON_CP_PACKET2);
 	if (r)
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/16] drm/radeon: implement ring commit tracking

[Christian König]

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/radeon.h      |    3 +++
 drivers/gpu/drm/radeon/radeon_ring.c |   39 ++++++++++++++++++++++++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index fef4257..9c11be8 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -637,6 +637,9 @@ struct radeon_ring {
 	u32			ptr_reg_shift;
 	u32			ptr_reg_mask;
 	u32			nop;
+	unsigned		*track_back;
+	unsigned		track_ptr;
+	unsigned		track_mask;
 };
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index d9b2e45..994c98c 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -276,6 +276,8 @@ void radeon_ring_commit(struct radeon_device *rdev, struct radeon_ring *ring)
 	DRM_MEMORYBARRIER();
 	WREG32(ring->wptr_reg, (ring->wptr << ring->ptr_reg_shift) & ring->ptr_reg_mask);
 	(void)RREG32(ring->wptr_reg);
+	ring->track_back[ring->track_ptr++] = ring->wptr_old;
+	ring->track_ptr &= ring->track_mask;
 }
 
 void radeon_ring_unlock_commit(struct radeon_device *rdev, struct radeon_ring *ring)
@@ -362,6 +364,27 @@ bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 	return false;
 }
 
+static unsigned radeon_ring_first_valid_commit(struct radeon_ring *ring)
+{
+	unsigned i, c, result = ring->track_ptr;
+	i = ring->track_ptr - 1;
+	while (i != ring->track_ptr) {
+		i &= ring->track_mask;
+		c = ring->track_back[i];
+
+		if (ring->wptr >= ring->rptr) {
+			if (c < ring->rptr || c >= ring->wptr)
+				break;
+		} else {
+			if (c < ring->rptr && c >= ring->wptr)
+				break;
+		}
+
+		result = i--;
+	}
+	return result;
+}
+
 int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
 		     unsigned ring_size, unsigned align,
 		     unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
@@ -403,6 +426,10 @@ int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
 			dev_err(rdev->dev, "(%d) ring map failed\n", r);
 			return r;
 		}
+		ring->track_back = kmalloc(ring_size / align, GFP_KERNEL);
+		memset(ring->track_back, 0, ring_size / align);
+		ring->track_ptr = 0;
+		ring->track_mask = ((ring->ring_size / 4) / align) - 1;
 	}
 	ring->ptr_mask = (ring->ring_size / 4) - 1;
 	ring->ring_free_dw = ring->ring_size / 4;
@@ -422,6 +449,7 @@ void radeon_ring_fini(struct radeon_device *rdev, struct radeon_ring *ring)
 	ring->ready = false;
 	ring->ring = NULL;
 	ring->ring_obj = NULL;
+	kfree(ring->track_back);
 	mutex_unlock(&rdev->ring_lock);
 
 	if (ring_obj) {
@@ -447,7 +475,7 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
 	struct radeon_device *rdev = dev->dev_private;
 	int ridx = *(int*)node->info_ent->data;
 	struct radeon_ring *ring = &rdev->ring[ridx];
-	unsigned count, i, j;
+	unsigned count, i, j, commit;
 
 	radeon_ring_free_size(rdev, ring);
 	count = (ring->ring_size / 4) - ring->ring_free_dw;
@@ -457,9 +485,16 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
 	seq_printf(m, "driver's copy of the rptr: 0x%08x\n", ring->rptr);
 	seq_printf(m, "%u free dwords in ring\n", ring->ring_free_dw);
 	seq_printf(m, "%u dwords in ring\n", count);
+	commit = radeon_ring_first_valid_commit(ring);
 	i = ring->rptr;
 	for (j = 0; j <= count; j++) {
-		seq_printf(m, "r[%04d]=0x%08x\n", i, ring->ring[i]);
+		seq_printf(m, "r[%04x]=0x%08x", i, ring->ring[i]);
+		if (commit != ring->track_ptr && ring->track_back[commit] == i) {
+			seq_printf(m, " <-");
+			++commit;
+			commit &= ring->track_mask;
+		}
+		seq_printf(m, "\n");
 		i = (i + 1) & ring->ptr_mask;
 	}
 	return 0;
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Christian König]

Try to save whatever is on the rings when
we encounter an lockup.

Signed-off-by: Christian König <deathsimple@vodafone.de>
---
 drivers/gpu/drm/radeon/radeon.h        |    4 ++
 drivers/gpu/drm/radeon/radeon_device.c |   44 ++++++++++++++++----
 drivers/gpu/drm/radeon/radeon_ring.c   |   69 ++++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 9c11be8..1265840 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -770,6 +770,10 @@ int radeon_ring_test(struct radeon_device *rdev, struct radeon_ring *cp);
 void radeon_ring_force_activity(struct radeon_device *rdev, struct radeon_ring *ring);
 void radeon_ring_lockup_update(struct radeon_ring *ring);
 bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring);
+unsigned radeon_ring_backup(struct radeon_device *rdev, struct radeon_ring *ring,
+			    uint32_t **data);
+int radeon_ring_restore(struct radeon_device *rdev, struct radeon_ring *ring,
+			unsigned size, uint32_t *data);
 int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *cp,
 		     unsigned ring_size, unsigned align,
 		     unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index bbd0971..97696e5 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -996,7 +996,12 @@ int radeon_resume_kms(struct drm_device *dev)
 
 int radeon_gpu_reset(struct radeon_device *rdev)
 {
-	int r;
+	unsigned ring_sizes[RADEON_NUM_RINGS];
+	uint32_t *ring_data[RADEON_NUM_RINGS];
+
+	bool saved = false;
+
+	int i, r;
 	int resched;
 
 	down_write(&rdev->exclusive_lock);
@@ -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");
 		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]);
+		}
 
-		radeon_restore_bios_scratch_regs(rdev);
-		drm_helper_resume_force_mode(rdev->ddev);
-		ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
+		r = radeon_ib_ring_tests(rdev);
+		if (r) {
+			dev_err(rdev->dev, "ib ring test failed (%d).\n", r);
+			if (saved) {
+				radeon_suspend(rdev);
+				goto retry;
+			}
+		}
 	}
 
+	ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
 	if (r) {
 		/* bad news, how to tell it to userspace ? */
 		dev_info(rdev->dev, "GPU reset failed\n");
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 994c98c..6ce51d6 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -385,6 +385,75 @@ static unsigned radeon_ring_first_valid_commit(struct radeon_ring *ring)
 	return result;
 }
 
+unsigned radeon_ring_backup(struct radeon_device *rdev, struct radeon_ring *ring,
+			    uint32_t **data)
+{
+	unsigned size, ptr, i;
+	int commit;
+
+	/* just in case lock the ring */
+	mutex_lock(&rdev->ring_lock);
+	*data = NULL;
+
+	if (ring->ring_obj == NULL) {
+		mutex_unlock(&rdev->ring_lock);
+		return 0;
+	}
+
+	/* first of all update the rptr directly from the hw */
+	ring->rptr = (RREG32(ring->rptr_reg) & ring->ptr_reg_mask)
+		 >> ring->ptr_reg_shift;
+
+	/* find the first commit not processed so far */
+	commit = radeon_ring_first_valid_commit(ring);
+	if (commit == ring->track_ptr) {
+		mutex_unlock(&rdev->ring_lock);
+		return 0;
+	}
+
+	/* calculate the number of dw on the ring */
+	ptr = ring->track_back[commit];
+	size = ring->wptr + (ring->ring_size / 4);
+	size -= ptr;
+	size &= ring->ptr_mask;
+	if (size == 0) {
+		mutex_unlock(&rdev->ring_lock);
+		return 0;
+	}
+
+	/* and then save the content of the ring */
+	*data = kmalloc(size * 4, GFP_KERNEL);
+	for (i = 0; i < size; ++i) {
+		(*data)[i] = ring->ring[ptr++];
+		ptr &= ring->ptr_mask;
+	}
+
+	mutex_unlock(&rdev->ring_lock);
+	return size;
+}
+
+int radeon_ring_restore(struct radeon_device *rdev, struct radeon_ring *ring,
+			unsigned size, uint32_t *data)
+{
+	int i, r;
+
+	if (!size || !data)
+		return 0;
+
+	/* restore the saved ring content */
+	r = radeon_ring_lock(rdev, ring, size);
+	if (r)
+		return r;
+
+	for (i = 0; i < size; ++i) {
+		radeon_ring_write(ring, data[i]);
+	}
+
+	radeon_ring_unlock_commit(rdev, ring);
+	kfree(data);
+	return 0;
+}
+
 int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
 		     unsigned ring_size, unsigned align,
 		     unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/16] drm/radeon: make cp init on cayman more robust

[Jerome Glisse]

On Mon, Jul 9, 2012 at 6:41 AM, Christian König <deathsimple@vodafone.de> wrote:
> It's not critical, but the current code isn't
> 100% correct.
>
> Signed-off-by: Christian König <deathsimple@vodafone.de>
> ---
>  drivers/gpu/drm/radeon/ni.c |  133 ++++++++++++++++++-------------------------
>  1 file changed, 56 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
> index 32a6082..8b1df33 100644
> --- a/drivers/gpu/drm/radeon/ni.c
> +++ b/drivers/gpu/drm/radeon/ni.c
> @@ -987,10 +987,33 @@ static void cayman_cp_fini(struct radeon_device *rdev)
>
>  int cayman_cp_resume(struct radeon_device *rdev)
>  {
> +       static const int ridx[] = {
> +               RADEON_RING_TYPE_GFX_INDEX,
> +               CAYMAN_RING_TYPE_CP1_INDEX,
> +               CAYMAN_RING_TYPE_CP2_INDEX
> +       };
> +       static const unsigned cp_rb_cntl[] = {
> +               CP_RB0_CNTL,
> +               CP_RB1_CNTL,
> +               CP_RB2_CNTL,
> +       };
> +       static const unsigned cp_rb_rptr_addr[] = {
> +               CP_RB0_RPTR_ADDR,
> +               CP_RB1_RPTR_ADDR,
> +               CP_RB2_RPTR_ADDR
> +       };
> +       static const unsigned cp_rb_rptr_addr_hi[] = {
> +               CP_RB0_RPTR_ADDR_HI,
> +               CP_RB1_RPTR_ADDR_HI,
> +               CP_RB2_RPTR_ADDR_HI
> +       };
> +       static const unsigned cp_rb_base[] = {
> +               CP_RB0_BASE,
> +               CP_RB1_BASE,
> +               CP_RB2_BASE
> +       };
>         struct radeon_ring *ring;
> -       u32 tmp;
> -       u32 rb_bufsz;
> -       int r;
> +       int i, r;
>
>         /* Reset cp; if cp is reset, then PA, SH, VGT also need to be reset */
>         WREG32(GRBM_SOFT_RESET, (SOFT_RESET_CP |
> @@ -1012,91 +1035,47 @@ int cayman_cp_resume(struct radeon_device *rdev)
>
>         WREG32(CP_DEBUG, (1 << 27));
>
> -       /* ring 0 - compute and gfx */
> -       /* Set ring buffer size */
> -       ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
> -       rb_bufsz = drm_order(ring->ring_size / 8);
> -       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
> -#ifdef __BIG_ENDIAN
> -       tmp |= BUF_SWAP_32BIT;
> -#endif
> -       WREG32(CP_RB0_CNTL, tmp);
> -
> -       /* Initialize the ring buffer's read and write pointers */
> -       WREG32(CP_RB0_CNTL, tmp | RB_RPTR_WR_ENA);
> -       ring->wptr = 0;
> -       WREG32(CP_RB0_WPTR, ring->wptr);
> -
>         /* set the wb address wether it's enabled or not */
> -       WREG32(CP_RB0_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFFFFFFFC);
> -       WREG32(CP_RB0_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFF);
>         WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
> +       WREG32(SCRATCH_UMSK, 0xff);

This looks wrong you set the mask unconditionaly even if writeback is disabled.


> -       if (rdev->wb.enabled)
> -               WREG32(SCRATCH_UMSK, 0xff);
> -       else {
> -               tmp |= RB_NO_UPDATE;
> -               WREG32(SCRATCH_UMSK, 0);
> -       }
> -
> -       mdelay(1);
> -       WREG32(CP_RB0_CNTL, tmp);
> -
> -       WREG32(CP_RB0_BASE, ring->gpu_addr >> 8);
> -
> -       ring->rptr = RREG32(CP_RB0_RPTR);
> +       for (i = 0; i < 3; ++i) {
> +               uint32_t rb_cntl;
> +               uint64_t addr;
>
> -       /* ring1  - compute only */
> -       /* Set ring buffer size */
> -       ring = &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX];
> -       rb_bufsz = drm_order(ring->ring_size / 8);
> -       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
> +               /* Set ring buffer size */
> +               ring = &rdev->ring[ridx[i]];
> +               rb_cntl = drm_order(ring->ring_size / 8);
> +               rb_cntl |= drm_order(RADEON_GPU_PAGE_SIZE/8) << 8;
>  #ifdef __BIG_ENDIAN
> -       tmp |= BUF_SWAP_32BIT;
> +               rb_cntl |= BUF_SWAP_32BIT;
>  #endif
> -       WREG32(CP_RB1_CNTL, tmp);
> +               WREG32(cp_rb_cntl[i], rb_cntl);
>
> -       /* Initialize the ring buffer's read and write pointers */
> -       WREG32(CP_RB1_CNTL, tmp | RB_RPTR_WR_ENA);
> -       ring->wptr = 0;
> -       WREG32(CP_RB1_WPTR, ring->wptr);
> -
> -       /* set the wb address wether it's enabled or not */
> -       WREG32(CP_RB1_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFFFFFFFC);
> -       WREG32(CP_RB1_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFF);
> -
> -       mdelay(1);
> -       WREG32(CP_RB1_CNTL, tmp);
> -
> -       WREG32(CP_RB1_BASE, ring->gpu_addr >> 8);
> -
> -       ring->rptr = RREG32(CP_RB1_RPTR);
> -
> -       /* ring2 - compute only */
> -       /* Set ring buffer size */
> -       ring = &rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX];
> -       rb_bufsz = drm_order(ring->ring_size / 8);
> -       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
> -#ifdef __BIG_ENDIAN
> -       tmp |= BUF_SWAP_32BIT;
> -#endif
> -       WREG32(CP_RB2_CNTL, tmp);
> -
> -       /* Initialize the ring buffer's read and write pointers */
> -       WREG32(CP_RB2_CNTL, tmp | RB_RPTR_WR_ENA);
> -       ring->wptr = 0;
> -       WREG32(CP_RB2_WPTR, ring->wptr);
> +               /* set the wb address wether it's enabled or not */
> +               addr = rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET;
> +               WREG32(cp_rb_rptr_addr[i], addr & 0xFFFFFFFC);
> +               WREG32(cp_rb_rptr_addr_hi[i], upper_32_bits(addr) & 0xFF);
> +       }
>
> -       /* set the wb address wether it's enabled or not */
> -       WREG32(CP_RB2_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFFFFFFFC);
> -       WREG32(CP_RB2_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFF);
> +       /* set the rb base addr, this causes an internal reset of ALL rings */
> +       for (i = 0; i < 3; ++i) {
> +               ring = &rdev->ring[ridx[i]];
> +               WREG32(cp_rb_base[i], ring->gpu_addr >> 8);
> +       }
>
> -       mdelay(1);
> -       WREG32(CP_RB2_CNTL, tmp);
> +       for (i = 0; i < 3; ++i) {
> +               /* Initialize the ring buffer's read and write pointers */
> +               ring = &rdev->ring[ridx[i]];
> +               WREG32_P(cp_rb_cntl[i], RB_RPTR_WR_ENA, ~RB_RPTR_WR_ENA);
>
> -       WREG32(CP_RB2_BASE, ring->gpu_addr >> 8);
> +               ring->rptr = ring->wptr = 0;
> +               WREG32(ring->rptr_reg, ring->rptr);
> +               WREG32(ring->wptr_reg, ring->wptr);
>
> -       ring->rptr = RREG32(CP_RB2_RPTR);
> +               mdelay(1);
> +               WREG32_P(cp_rb_cntl[i], 0, ~RB_RPTR_WR_ENA);
> +       }
>
>         /* start the rings */
>         cayman_cp_start(rdev);
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Michel Dänzer]

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.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

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

[Michel Dänzer]

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.

Not sure about the other places where you're adding
radeon_ib_ring_tests() calls, might need more error handling as well.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [PATCH 09/16] drm/radeon: make cp init on cayman more robust

[Christian König]

On 09.07.2012 16:43, Jerome Glisse wrote:
> On Mon, Jul 9, 2012 at 6:41 AM, Christian König <deathsimple@vodafone.de> wrote:
>> It's not critical, but the current code isn't
>> 100% correct.
>>
>> Signed-off-by: Christian König <deathsimple@vodafone.de>
>> ---
>>   drivers/gpu/drm/radeon/ni.c |  133 ++++++++++++++++++-------------------------
>>   1 file changed, 56 insertions(+), 77 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
>> index 32a6082..8b1df33 100644
>> --- a/drivers/gpu/drm/radeon/ni.c
>> +++ b/drivers/gpu/drm/radeon/ni.c
>> @@ -987,10 +987,33 @@ static void cayman_cp_fini(struct radeon_device *rdev)
>>
>>   int cayman_cp_resume(struct radeon_device *rdev)
>>   {
>> +       static const int ridx[] = {
>> +               RADEON_RING_TYPE_GFX_INDEX,
>> +               CAYMAN_RING_TYPE_CP1_INDEX,
>> +               CAYMAN_RING_TYPE_CP2_INDEX
>> +       };
>> +       static const unsigned cp_rb_cntl[] = {
>> +               CP_RB0_CNTL,
>> +               CP_RB1_CNTL,
>> +               CP_RB2_CNTL,
>> +       };
>> +       static const unsigned cp_rb_rptr_addr[] = {
>> +               CP_RB0_RPTR_ADDR,
>> +               CP_RB1_RPTR_ADDR,
>> +               CP_RB2_RPTR_ADDR
>> +       };
>> +       static const unsigned cp_rb_rptr_addr_hi[] = {
>> +               CP_RB0_RPTR_ADDR_HI,
>> +               CP_RB1_RPTR_ADDR_HI,
>> +               CP_RB2_RPTR_ADDR_HI
>> +       };
>> +       static const unsigned cp_rb_base[] = {
>> +               CP_RB0_BASE,
>> +               CP_RB1_BASE,
>> +               CP_RB2_BASE
>> +       };
>>          struct radeon_ring *ring;
>> -       u32 tmp;
>> -       u32 rb_bufsz;
>> -       int r;
>> +       int i, r;
>>
>>          /* Reset cp; if cp is reset, then PA, SH, VGT also need to be reset */
>>          WREG32(GRBM_SOFT_RESET, (SOFT_RESET_CP |
>> @@ -1012,91 +1035,47 @@ int cayman_cp_resume(struct radeon_device *rdev)
>>
>>          WREG32(CP_DEBUG, (1 << 27));
>>
>> -       /* ring 0 - compute and gfx */
>> -       /* Set ring buffer size */
>> -       ring = &rdev->ring[RADEON_RING_TYPE_GFX_INDEX];
>> -       rb_bufsz = drm_order(ring->ring_size / 8);
>> -       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
>> -#ifdef __BIG_ENDIAN
>> -       tmp |= BUF_SWAP_32BIT;
>> -#endif
>> -       WREG32(CP_RB0_CNTL, tmp);
>> -
>> -       /* Initialize the ring buffer's read and write pointers */
>> -       WREG32(CP_RB0_CNTL, tmp | RB_RPTR_WR_ENA);
>> -       ring->wptr = 0;
>> -       WREG32(CP_RB0_WPTR, ring->wptr);
>> -
>>          /* set the wb address wether it's enabled or not */
>> -       WREG32(CP_RB0_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFFFFFFFC);
>> -       WREG32(CP_RB0_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET) & 0xFF);
>>          WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
>> +       WREG32(SCRATCH_UMSK, 0xff);
> This looks wrong you set the mask unconditionaly even if writeback is disabled.
writeback is always enabled for NI and APUs, but the register docs say 
that this feature isn't validated for NI and shouldn't be used so I'm 
not sure if enabling it is the right thing to do.

Anyway, it doesn't matter at all since we don't use the scratch register 
writeback anymore and use EOP instead.

Christian.


>
>
>> -       if (rdev->wb.enabled)
>> -               WREG32(SCRATCH_UMSK, 0xff);
>> -       else {
>> -               tmp |= RB_NO_UPDATE;
>> -               WREG32(SCRATCH_UMSK, 0);
>> -       }
>> -
>> -       mdelay(1);
>> -       WREG32(CP_RB0_CNTL, tmp);
>> -
>> -       WREG32(CP_RB0_BASE, ring->gpu_addr >> 8);
>> -
>> -       ring->rptr = RREG32(CP_RB0_RPTR);
>> +       for (i = 0; i < 3; ++i) {
>> +               uint32_t rb_cntl;
>> +               uint64_t addr;
>>
>> -       /* ring1  - compute only */
>> -       /* Set ring buffer size */
>> -       ring = &rdev->ring[CAYMAN_RING_TYPE_CP1_INDEX];
>> -       rb_bufsz = drm_order(ring->ring_size / 8);
>> -       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
>> +               /* Set ring buffer size */
>> +               ring = &rdev->ring[ridx[i]];
>> +               rb_cntl = drm_order(ring->ring_size / 8);
>> +               rb_cntl |= drm_order(RADEON_GPU_PAGE_SIZE/8) << 8;
>>   #ifdef __BIG_ENDIAN
>> -       tmp |= BUF_SWAP_32BIT;
>> +               rb_cntl |= BUF_SWAP_32BIT;
>>   #endif
>> -       WREG32(CP_RB1_CNTL, tmp);
>> +               WREG32(cp_rb_cntl[i], rb_cntl);
>>
>> -       /* Initialize the ring buffer's read and write pointers */
>> -       WREG32(CP_RB1_CNTL, tmp | RB_RPTR_WR_ENA);
>> -       ring->wptr = 0;
>> -       WREG32(CP_RB1_WPTR, ring->wptr);
>> -
>> -       /* set the wb address wether it's enabled or not */
>> -       WREG32(CP_RB1_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFFFFFFFC);
>> -       WREG32(CP_RB1_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP1_RPTR_OFFSET) & 0xFF);
>> -
>> -       mdelay(1);
>> -       WREG32(CP_RB1_CNTL, tmp);
>> -
>> -       WREG32(CP_RB1_BASE, ring->gpu_addr >> 8);
>> -
>> -       ring->rptr = RREG32(CP_RB1_RPTR);
>> -
>> -       /* ring2 - compute only */
>> -       /* Set ring buffer size */
>> -       ring = &rdev->ring[CAYMAN_RING_TYPE_CP2_INDEX];
>> -       rb_bufsz = drm_order(ring->ring_size / 8);
>> -       tmp = (drm_order(RADEON_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
>> -#ifdef __BIG_ENDIAN
>> -       tmp |= BUF_SWAP_32BIT;
>> -#endif
>> -       WREG32(CP_RB2_CNTL, tmp);
>> -
>> -       /* Initialize the ring buffer's read and write pointers */
>> -       WREG32(CP_RB2_CNTL, tmp | RB_RPTR_WR_ENA);
>> -       ring->wptr = 0;
>> -       WREG32(CP_RB2_WPTR, ring->wptr);
>> +               /* set the wb address wether it's enabled or not */
>> +               addr = rdev->wb.gpu_addr + RADEON_WB_CP_RPTR_OFFSET;
>> +               WREG32(cp_rb_rptr_addr[i], addr & 0xFFFFFFFC);
>> +               WREG32(cp_rb_rptr_addr_hi[i], upper_32_bits(addr) & 0xFF);
>> +       }
>>
>> -       /* set the wb address wether it's enabled or not */
>> -       WREG32(CP_RB2_RPTR_ADDR, (rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFFFFFFFC);
>> -       WREG32(CP_RB2_RPTR_ADDR_HI, upper_32_bits(rdev->wb.gpu_addr + RADEON_WB_CP2_RPTR_OFFSET) & 0xFF);
>> +       /* set the rb base addr, this causes an internal reset of ALL rings */
>> +       for (i = 0; i < 3; ++i) {
>> +               ring = &rdev->ring[ridx[i]];
>> +               WREG32(cp_rb_base[i], ring->gpu_addr >> 8);
>> +       }
>>
>> -       mdelay(1);
>> -       WREG32(CP_RB2_CNTL, tmp);
>> +       for (i = 0; i < 3; ++i) {
>> +               /* Initialize the ring buffer's read and write pointers */
>> +               ring = &rdev->ring[ridx[i]];
>> +               WREG32_P(cp_rb_cntl[i], RB_RPTR_WR_ENA, ~RB_RPTR_WR_ENA);
>>
>> -       WREG32(CP_RB2_BASE, ring->gpu_addr >> 8);
>> +               ring->rptr = ring->wptr = 0;
>> +               WREG32(ring->rptr_reg, ring->rptr);
>> +               WREG32(ring->wptr_reg, ring->wptr);
>>
>> -       ring->rptr = RREG32(CP_RB2_RPTR);
>> +               mdelay(1);
>> +               WREG32_P(cp_rb_cntl[i], 0, ~RB_RPTR_WR_ENA);
>> +       }
>>
>>          /* start the rings */
>>          cayman_cp_start(rdev);
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Christian König]

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.

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

[Christian König]

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.

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

[Michel Dänzer]

On Mon, 2012-07-09 at 17:22 +0200, Christian König wrote: 
> 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.

Oops, I actually did check this when writing the above, but somehow I
failed to see what I was looking for. :}


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [PATCH 15/16] drm/radeon: implement ring commit tracking

[Jerome Glisse]

[-- Attachment #1: Type: text/plain, Size: 5278 bytes --]

On Mon, Jul 9, 2012 at 6:42 AM, Christian König <deathsimple@vodafone.de> wrote:
> Signed-off-by: Christian König <deathsimple@vodafone.de>

Bit too complex to my taste, what about attached patch, it's lot
simpler. (Haven't tested
the patch but it should work)

Cheers,
Jerome

> ---
>  drivers/gpu/drm/radeon/radeon.h      |    3 +++
>  drivers/gpu/drm/radeon/radeon_ring.c |   39 ++++++++++++++++++++++++++++++++--
>  2 files changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index fef4257..9c11be8 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -637,6 +637,9 @@ struct radeon_ring {
>         u32                     ptr_reg_shift;
>         u32                     ptr_reg_mask;
>         u32                     nop;
> +       unsigned                *track_back;
> +       unsigned                track_ptr;
> +       unsigned                track_mask;
>  };
>
>  /*
> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
> index d9b2e45..994c98c 100644
> --- a/drivers/gpu/drm/radeon/radeon_ring.c
> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> @@ -276,6 +276,8 @@ void radeon_ring_commit(struct radeon_device *rdev, struct radeon_ring *ring)
>         DRM_MEMORYBARRIER();
>         WREG32(ring->wptr_reg, (ring->wptr << ring->ptr_reg_shift) & ring->ptr_reg_mask);
>         (void)RREG32(ring->wptr_reg);
> +       ring->track_back[ring->track_ptr++] = ring->wptr_old;
> +       ring->track_ptr &= ring->track_mask;
>  }
>
>  void radeon_ring_unlock_commit(struct radeon_device *rdev, struct radeon_ring *ring)
> @@ -362,6 +364,27 @@ bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *rin
>         return false;
>  }
>
> +static unsigned radeon_ring_first_valid_commit(struct radeon_ring *ring)
> +{
> +       unsigned i, c, result = ring->track_ptr;
> +       i = ring->track_ptr - 1;
> +       while (i != ring->track_ptr) {
> +               i &= ring->track_mask;
> +               c = ring->track_back[i];
> +
> +               if (ring->wptr >= ring->rptr) {
> +                       if (c < ring->rptr || c >= ring->wptr)
> +                               break;
> +               } else {
> +                       if (c < ring->rptr && c >= ring->wptr)
> +                               break;
> +               }
> +
> +               result = i--;
> +       }
> +       return result;
> +}
> +
>  int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
>                      unsigned ring_size, unsigned align,
>                      unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
> @@ -403,6 +426,10 @@ int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
>                         dev_err(rdev->dev, "(%d) ring map failed\n", r);
>                         return r;
>                 }
> +               ring->track_back = kmalloc(ring_size / align, GFP_KERNEL);
> +               memset(ring->track_back, 0, ring_size / align);
> +               ring->track_ptr = 0;
> +               ring->track_mask = ((ring->ring_size / 4) / align) - 1;
>         }
>         ring->ptr_mask = (ring->ring_size / 4) - 1;
>         ring->ring_free_dw = ring->ring_size / 4;
> @@ -422,6 +449,7 @@ void radeon_ring_fini(struct radeon_device *rdev, struct radeon_ring *ring)
>         ring->ready = false;
>         ring->ring = NULL;
>         ring->ring_obj = NULL;
> +       kfree(ring->track_back);
>         mutex_unlock(&rdev->ring_lock);
>
>         if (ring_obj) {
> @@ -447,7 +475,7 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
>         struct radeon_device *rdev = dev->dev_private;
>         int ridx = *(int*)node->info_ent->data;
>         struct radeon_ring *ring = &rdev->ring[ridx];
> -       unsigned count, i, j;
> +       unsigned count, i, j, commit;
>
>         radeon_ring_free_size(rdev, ring);
>         count = (ring->ring_size / 4) - ring->ring_free_dw;
> @@ -457,9 +485,16 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
>         seq_printf(m, "driver's copy of the rptr: 0x%08x\n", ring->rptr);
>         seq_printf(m, "%u free dwords in ring\n", ring->ring_free_dw);
>         seq_printf(m, "%u dwords in ring\n", count);
> +       commit = radeon_ring_first_valid_commit(ring);
>         i = ring->rptr;
>         for (j = 0; j <= count; j++) {
> -               seq_printf(m, "r[%04d]=0x%08x\n", i, ring->ring[i]);
> +               seq_printf(m, "r[%04x]=0x%08x", i, ring->ring[i]);
> +               if (commit != ring->track_ptr && ring->track_back[commit] == i) {
> +                       seq_printf(m, " <-");
> +                       ++commit;
> +                       commit &= ring->track_mask;
> +               }
> +               seq_printf(m, "\n");
>                 i = (i + 1) & ring->ptr_mask;
>         }
>         return 0;
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #2: 0001-drm-radeon-record-what-is-next-valid-wptr-for-each-r.patch --]
[-- Type: application/octet-stream, Size: 7153 bytes --]

From ed90755e625673c98684191ac0bd9436149e416b Mon Sep 17 00:00:00 2001
From: Jerome Glisse <jglisse@redhat.com>
Date: Mon, 9 Jul 2012 11:30:17 -0400
Subject: [PATCH] drm/radeon: record what is next valid wptr for each ring

Before emitting any indirect buffer, emit the offset of the next
valid ring content if any. This allow code that want to resume
ring to resume ring right after ib that caused GPU lockup.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c     |   17 ++++++++++++++++-
 drivers/gpu/drm/radeon/ni.c            |   17 +++++++++++++++++
 drivers/gpu/drm/radeon/r600.c          |   16 +++++++++++++++-
 drivers/gpu/drm/radeon/radeon.h        |   14 ++++++++------
 drivers/gpu/drm/radeon/radeon_device.c |    3 +++
 drivers/gpu/drm/radeon/si.c            |   16 ++++++++++++++++
 6 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index eef6048..633efe8 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -1376,7 +1376,22 @@ void evergreen_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
 	/* set to DX10/11 mode */
 	radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0));
 	radeon_ring_write(ring, 1);
-	/* FIXME: implement */
+
+	if (rdev->wb.enabled) {
+		u64 next_rptr_gaddr;
+		/* next rptr is wptr + 5 dw for mem_write and 4 for indirect_buffer */
+		u32 next_rptr = ring->wptr + 5 + 4;
+
+		next_rptr_gaddr = rdev->wb.gpu_addr + RADEON_WB_RING0_NEXT_RPTR;
+		next_rptr_gaddr +=  ring->id * 4;
+		WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
+		radeon_ring_write(ring, PACKET3(PACKET3_MEM_WRITE, 3));
+		radeon_ring_write(ring, next_rptr_gaddr & 0xfffffffc);
+		radeon_ring_write(ring, upper_32_bits(next_rptr_gaddr) & 0xff | (1 << 18));
+		radeon_ring_write(ring, next_rptr);
+		radeon_ring_write(ring, 0);
+	}
+
 	radeon_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
 	radeon_ring_write(ring,
 #ifdef __BIG_ENDIAN
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 5397fad..ca2f0d0 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -872,6 +872,23 @@ void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
 	/* set to DX10/11 mode */
 	radeon_ring_write(ring, PACKET3(PACKET3_MODE_CONTROL, 0));
 	radeon_ring_write(ring, 1);
+
+	if (rdev->wb.enabled) {
+		u64 next_rptr_gaddr;
+		/* next rptr is wptr + 5 dw for mem_write and 4 for indirect_buffer
+		 * and 8 dword for coherency and flush */
+		u32 next_rptr = ring->wptr + 5 + 4 + 8;
+
+		next_rptr_gaddr = rdev->wb.gpu_addr + RADEON_WB_RING0_NEXT_RPTR;
+		next_rptr_gaddr +=  ring->id * 4;
+		WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
+		radeon_ring_write(ring, PACKET3(PACKET3_MEM_WRITE, 3));
+		radeon_ring_write(ring, next_rptr_gaddr & 0xfffffffc);
+		radeon_ring_write(ring, upper_32_bits(next_rptr_gaddr) & 0xff | (1 << 18));
+		radeon_ring_write(ring, next_rptr);
+		radeon_ring_write(ring, 0);
+	}
+
 	radeon_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
 	radeon_ring_write(ring,
 #ifdef __BIG_ENDIAN
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index b3c0e0f..7bca9d5 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2617,7 +2617,21 @@ void r600_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
 {
 	struct radeon_ring *ring = &rdev->ring[ib->fence->ring];
 
-	/* FIXME: implement */
+	if (rdev->wb.enabled) {
+		u64 next_rptr_gaddr;
+		/* next rptr is wptr + 5 dw for mem_write and 4 for indirect_buffer */
+		u32 next_rptr = ring->wptr + 5 + 4;
+
+		next_rptr_gaddr = rdev->wb.gpu_addr + RADEON_WB_RING0_NEXT_RPTR;
+		next_rptr_gaddr +=  ring->id * 4;
+		WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
+		radeon_ring_write(ring, PACKET3(PACKET3_MEM_WRITE, 3));
+		radeon_ring_write(ring, next_rptr_gaddr & 0xfffffffc);
+		radeon_ring_write(ring, upper_32_bits(next_rptr_gaddr) & 0xff | (1 << 18));
+		radeon_ring_write(ring, next_rptr);
+		radeon_ring_write(ring, 0);
+	}
+
 	radeon_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
 	radeon_ring_write(ring,
 #ifdef __BIG_ENDIAN
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index fefcca5..f3dd970 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -639,6 +639,7 @@ struct radeon_ib {
 struct radeon_ring {
 	struct radeon_bo	*ring_obj;
 	volatile uint32_t	*ring;
+	unsigned		id;
 	unsigned		rptr;
 	unsigned		rptr_offs;
 	unsigned		rptr_reg;
@@ -890,12 +891,13 @@ struct radeon_wb {
 	bool                    use_event;
 };
 
-#define RADEON_WB_SCRATCH_OFFSET 0
-#define RADEON_WB_CP_RPTR_OFFSET 1024
-#define RADEON_WB_CP1_RPTR_OFFSET 1280
-#define RADEON_WB_CP2_RPTR_OFFSET 1536
-#define R600_WB_IH_WPTR_OFFSET   2048
-#define R600_WB_EVENT_OFFSET     3072
+#define RADEON_WB_SCRATCH_OFFSET	0
+#define RADEON_WB_RING0_NEXT_RPTR	256
+#define RADEON_WB_CP_RPTR_OFFSET	1024
+#define RADEON_WB_CP1_RPTR_OFFSET	1280
+#define RADEON_WB_CP2_RPTR_OFFSET	1536
+#define R600_WB_IH_WPTR_OFFSET		2048
+#define R600_WB_EVENT_OFFSET		3072
 
 /**
  * struct radeon_pm - power management datas
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 653f352..53f4591 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -721,6 +721,9 @@ int radeon_device_init(struct radeon_device *rdev,
 	rdev->usec_timeout = RADEON_MAX_USEC_TIMEOUT;
 	rdev->mc.gtt_size = radeon_gart_size * 1024 * 1024;
 	rdev->accel_working = false;
+	for (i = 0; i < RADEON_NUM_RINGS; i++) {
+		rdev->ring[i].id = i;
+	}
 
 	DRM_INFO("initializing kernel modesetting (%s 0x%04X:0x%04X 0x%04X:0x%04X).\n",
 		radeon_family_name[rdev->family], pdev->vendor, pdev->device,
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index a9eb826..e880aed 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -1765,6 +1765,22 @@ void si_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib)
 	struct radeon_ring *ring = &rdev->ring[ib->fence->ring];
 	u32 header;
 
+	if (rdev->wb.enabled) {
+		u64 next_rptr_gaddr;
+		/* next rptr is wptr + 5 dw for mem_write and 4 for indirect_buffer
+		 * and 8 dword for coherency and flush */
+		u32 next_rptr = ring->wptr + 5 + 4 + 8;
+
+		next_rptr_gaddr = rdev->wb.gpu_addr + RADEON_WB_RING0_NEXT_RPTR;
+		next_rptr_gaddr +=  ring->id * 4;
+		WREG32(SCRATCH_ADDR, ((rdev->wb.gpu_addr + RADEON_WB_SCRATCH_OFFSET) >> 8) & 0xFFFFFFFF);
+		radeon_ring_write(ring, PACKET3(PACKET3_MEM_WRITE, 3));
+		radeon_ring_write(ring, next_rptr_gaddr & 0xfffffffc);
+		radeon_ring_write(ring, upper_32_bits(next_rptr_gaddr) & 0xff | (1 << 18));
+		radeon_ring_write(ring, next_rptr);
+		radeon_ring_write(ring, 0);
+	}
+
 	if (ib->is_const_ib)
 		header = PACKET3(PACKET3_INDIRECT_BUFFER_CONST, 2);
 	else
-- 
1.7.10.4


[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/16] drm/radeon: implement ring commit tracking

[Christian König]

On 09.07.2012 17:36, Jerome Glisse wrote:
> On Mon, Jul 9, 2012 at 6:42 AM, Christian König <deathsimple@vodafone.de> wrote:
>> Signed-off-by: Christian König <deathsimple@vodafone.de>
> Bit too complex to my taste, what about attached patch, it's lot
> simpler. (Haven't tested
> the patch but it should work)
Cool idea! Depending on the writeback mechanism might not be the best 
part of it, but in general speaking all rings should have more than 
enough scratch registers for that!!!

Going to change it.

Thanks,
Christian.

>
> Cheers,
> Jerome
>
>> ---
>>   drivers/gpu/drm/radeon/radeon.h      |    3 +++
>>   drivers/gpu/drm/radeon/radeon_ring.c |   39 ++++++++++++++++++++++++++++++++--
>>   2 files changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
>> index fef4257..9c11be8 100644
>> --- a/drivers/gpu/drm/radeon/radeon.h
>> +++ b/drivers/gpu/drm/radeon/radeon.h
>> @@ -637,6 +637,9 @@ struct radeon_ring {
>>          u32                     ptr_reg_shift;
>>          u32                     ptr_reg_mask;
>>          u32                     nop;
>> +       unsigned                *track_back;
>> +       unsigned                track_ptr;
>> +       unsigned                track_mask;
>>   };
>>
>>   /*
>> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
>> index d9b2e45..994c98c 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ring.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
>> @@ -276,6 +276,8 @@ void radeon_ring_commit(struct radeon_device *rdev, struct radeon_ring *ring)
>>          DRM_MEMORYBARRIER();
>>          WREG32(ring->wptr_reg, (ring->wptr << ring->ptr_reg_shift) & ring->ptr_reg_mask);
>>          (void)RREG32(ring->wptr_reg);
>> +       ring->track_back[ring->track_ptr++] = ring->wptr_old;
>> +       ring->track_ptr &= ring->track_mask;
>>   }
>>
>>   void radeon_ring_unlock_commit(struct radeon_device *rdev, struct radeon_ring *ring)
>> @@ -362,6 +364,27 @@ bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *rin
>>          return false;
>>   }
>>
>> +static unsigned radeon_ring_first_valid_commit(struct radeon_ring *ring)
>> +{
>> +       unsigned i, c, result = ring->track_ptr;
>> +       i = ring->track_ptr - 1;
>> +       while (i != ring->track_ptr) {
>> +               i &= ring->track_mask;
>> +               c = ring->track_back[i];
>> +
>> +               if (ring->wptr >= ring->rptr) {
>> +                       if (c < ring->rptr || c >= ring->wptr)
>> +                               break;
>> +               } else {
>> +                       if (c < ring->rptr && c >= ring->wptr)
>> +                               break;
>> +               }
>> +
>> +               result = i--;
>> +       }
>> +       return result;
>> +}
>> +
>>   int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
>>                       unsigned ring_size, unsigned align,
>>                       unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
>> @@ -403,6 +426,10 @@ int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring,
>>                          dev_err(rdev->dev, "(%d) ring map failed\n", r);
>>                          return r;
>>                  }
>> +               ring->track_back = kmalloc(ring_size / align, GFP_KERNEL);
>> +               memset(ring->track_back, 0, ring_size / align);
>> +               ring->track_ptr = 0;
>> +               ring->track_mask = ((ring->ring_size / 4) / align) - 1;
>>          }
>>          ring->ptr_mask = (ring->ring_size / 4) - 1;
>>          ring->ring_free_dw = ring->ring_size / 4;
>> @@ -422,6 +449,7 @@ void radeon_ring_fini(struct radeon_device *rdev, struct radeon_ring *ring)
>>          ring->ready = false;
>>          ring->ring = NULL;
>>          ring->ring_obj = NULL;
>> +       kfree(ring->track_back);
>>          mutex_unlock(&rdev->ring_lock);
>>
>>          if (ring_obj) {
>> @@ -447,7 +475,7 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
>>          struct radeon_device *rdev = dev->dev_private;
>>          int ridx = *(int*)node->info_ent->data;
>>          struct radeon_ring *ring = &rdev->ring[ridx];
>> -       unsigned count, i, j;
>> +       unsigned count, i, j, commit;
>>
>>          radeon_ring_free_size(rdev, ring);
>>          count = (ring->ring_size / 4) - ring->ring_free_dw;
>> @@ -457,9 +485,16 @@ static int radeon_debugfs_ring_info(struct seq_file *m, void *data)
>>          seq_printf(m, "driver's copy of the rptr: 0x%08x\n", ring->rptr);
>>          seq_printf(m, "%u free dwords in ring\n", ring->ring_free_dw);
>>          seq_printf(m, "%u dwords in ring\n", count);
>> +       commit = radeon_ring_first_valid_commit(ring);
>>          i = ring->rptr;
>>          for (j = 0; j <= count; j++) {
>> -               seq_printf(m, "r[%04d]=0x%08x\n", i, ring->ring[i]);
>> +               seq_printf(m, "r[%04x]=0x%08x", i, ring->ring[i]);
>> +               if (commit != ring->track_ptr && ring->track_back[commit] == i) {
>> +                       seq_printf(m, " <-");
>> +                       ++commit;
>> +                       commit &= ring->track_mask;
>> +               }
>> +               seq_printf(m, "\n");
>>                  i = (i + 1) & ring->ptr_mask;
>>          }
>>          return 0;
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] drm/radeon: restoring ring commands in case of a lockup

[Michel Dänzer]

On Mon, 2012-07-09 at 12:41 +0200, Christian König wrote: 
> Hi,
> 
> The following patchset tries to save and restore the not yet processed commands
> from the rings in case of a lockup and with that should make a userspace
> problem with a single application far less problematic.
> 
> The first four patches are just stuff this patchset is based upon, followed by
> four patches which fix various bugs found while working on this feature.
> 
> Followed by patches which change the way how memory is saved/restored on
> suspend/resume, basically before we have unpinned most of the buffer objects so
> it could be move from vram into system memory. But that is mostly unnecessary
> cause the buffer object either are already in system memory or their content
> can be easily reinitialized.
> 
> The last three patches implement the actual tracking and restoring of commands
> in case of a lockup. Please take a look and review.

Patches 3, 5 and 14 are

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: [RFC] drm/radeon: restoring ring commands in case of a lockup

[Jerome Glisse]

On Mon, Jul 9, 2012 at 11:59 AM, Michel Dänzer <michel@daenzer.net> wrote:
> On Mon, 2012-07-09 at 12:41 +0200, Christian König wrote:
>> Hi,
>>
>> The following patchset tries to save and restore the not yet processed commands
>> from the rings in case of a lockup and with that should make a userspace
>> problem with a single application far less problematic.
>>
>> The first four patches are just stuff this patchset is based upon, followed by
>> four patches which fix various bugs found while working on this feature.
>>
>> Followed by patches which change the way how memory is saved/restored on
>> suspend/resume, basically before we have unpinned most of the buffer objects so
>> it could be move from vram into system memory. But that is mostly unnecessary
>> cause the buffer object either are already in system memory or their content
>> can be easily reinitialized.
>>
>> The last three patches implement the actual tracking and restoring of commands
>> in case of a lockup. Please take a look and review.
>
> Patches 3, 5 and 14 are
>
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>

Patch 1-9 are
Reviewed-by: Jerome Glisse <jglisse@redhat.com>

Other looks good but i want to test them too and spend a bit more time
to double check few things. Will try to do that tomorrow.

Cheers,
Jerome

Re: [PATCH 15/16] drm/radeon: implement ring commit tracking

[Jerome Glisse]

On Mon, Jul 9, 2012 at 11:48 AM, Christian König
<deathsimple@vodafone.de> wrote:
> On 09.07.2012 17:36, Jerome Glisse wrote:
>>
>> On Mon, Jul 9, 2012 at 6:42 AM, Christian König <deathsimple@vodafone.de>
>> wrote:
>>>
>>> Signed-off-by: Christian König <deathsimple@vodafone.de>
>>
>> Bit too complex to my taste, what about attached patch, it's lot
>> simpler. (Haven't tested
>> the patch but it should work)
>
> Cool idea! Depending on the writeback mechanism might not be the best part
> of it, but in general speaking all rings should have more than enough
> scratch registers for that!!!
>
> Going to change it.
>
> Thanks,
> Christian.
>

Note that i haven't included the fence emit intentionally idea being
if you resume ring you fence to emit the last fence as soon as
possible and just pretend that the faulty ib succeeded.

Cheers,
Jerome

Re: [RFC] drm/radeon: restoring ring commands in case of a lockup

[Christian König]

On 09.07.2012 18:10, Jerome Glisse wrote:
> On Mon, Jul 9, 2012 at 11:59 AM, Michel Dänzer <michel@daenzer.net> wrote:
>> On Mon, 2012-07-09 at 12:41 +0200, Christian König wrote:
>>> Hi,
>>>
>>> The following patchset tries to save and restore the not yet processed commands
>>> from the rings in case of a lockup and with that should make a userspace
>>> problem with a single application far less problematic.
>>>
>>> The first four patches are just stuff this patchset is based upon, followed by
>>> four patches which fix various bugs found while working on this feature.
>>>
>>> Followed by patches which change the way how memory is saved/restored on
>>> suspend/resume, basically before we have unpinned most of the buffer objects so
>>> it could be move from vram into system memory. But that is mostly unnecessary
>>> cause the buffer object either are already in system memory or their content
>>> can be easily reinitialized.
>>>
>>> The last three patches implement the actual tracking and restoring of commands
>>> in case of a lockup. Please take a look and review.
>> Patches 3, 5 and 14 are
>>
>> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
>>
> Patch 1-9 are
> Reviewed-by: Jerome Glisse <jglisse@redhat.com>
>
> Other looks good but i want to test them too and spend a bit more time
> to double check few things. Will try to do that tomorrow.
Just send out v2 of the patchset. Mainly it integrates your idea of just 
saving rptr right before we call into the IB, but also contains all the 
other comments and fixes from Michel.

Cheers,
Christian.