From: Christian König christian.koenig@amd.com
Use atomics and jiffies_64, so that we don't need to have the ring mutex locked any more and avoid wrap arounds.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon.h | 4 ++-- drivers/gpu/drm/radeon/radeon_ring.c | 21 +++++++++------------ 2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index a415f8e..9269413 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -802,8 +802,8 @@ struct radeon_ring { unsigned ring_size; unsigned ring_free_dw; int count_dw; - unsigned long last_activity; - unsigned last_rptr; + atomic_t last_rptr; + atomic64_t last_activity; uint64_t gpu_addr; uint32_t align_mask; uint32_t ptr_mask; diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c index b14c86d..ae72108 100644 --- a/drivers/gpu/drm/radeon/radeon_ring.c +++ b/drivers/gpu/drm/radeon/radeon_ring.c @@ -485,8 +485,8 @@ void radeon_ring_unlock_undo(struct radeon_device *rdev, struct radeon_ring *rin void radeon_ring_lockup_update(struct radeon_device *rdev, struct radeon_ring *ring) { - ring->last_rptr = radeon_ring_get_rptr(rdev, ring); - ring->last_activity = jiffies; + atomic_set(&ring->last_rptr, radeon_ring_get_rptr(rdev, ring)); + atomic64_set(&ring->last_activity, jiffies_64); }
/** @@ -498,22 +498,19 @@ void radeon_ring_lockup_update(struct radeon_device *rdev, bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring) { uint32_t rptr = radeon_ring_get_rptr(rdev, ring); - unsigned long cjiffies, elapsed; + uint64_t last = atomic64_read(&ring->last_activity); + uint64_t elapsed;
- cjiffies = jiffies; - if (!time_after(cjiffies, ring->last_activity)) { - /* likely a wrap around */ - radeon_ring_lockup_update(rdev, ring); - return false; - } - if (rptr != ring->last_rptr) { + if (rptr != atomic_read(&ring->last_rptr)) { /* CP is still working no lockup */ radeon_ring_lockup_update(rdev, ring); return false; } - elapsed = jiffies_to_msecs(cjiffies - ring->last_activity); + + elapsed = jiffies_to_msecs(jiffies_64 - last); if (radeon_lockup_timeout && elapsed >= radeon_lockup_timeout) { - dev_err(rdev->dev, "GPU lockup CP stall for more than %lumsec\n", elapsed); + dev_err(rdev->dev, "GPU lockup ring %d stall for more " + "than %Lumsec\n", ring->idx, elapsed); return true; } /* give a chance to the GPU ... */
From: Christian König christian.koenig@amd.com
We no longer need to take the ring lock while checking for a gpu lockup, so just cleanup the code.
Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon.h | 5 ++- drivers/gpu/drm/radeon/radeon_device.c | 4 +-- drivers/gpu/drm/radeon/radeon_fence.c | 59 ++++++------------------------- drivers/gpu/drm/radeon/radeon_pm.c | 4 +-- drivers/gpu/drm/radeon/radeon_ring.c | 2 +- drivers/gpu/drm/radeon/radeon_semaphore.c | 12 +++++-- 6 files changed, 25 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 9269413..7337b35 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -360,9 +360,8 @@ int radeon_fence_emit(struct radeon_device *rdev, struct radeon_fence **fence, i 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_locked(struct radeon_fence *fence); -int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring); -int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring); +int radeon_fence_wait_next(struct radeon_device *rdev, int ring); +int radeon_fence_wait_empty(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_device.c b/drivers/gpu/drm/radeon/radeon_device.c index b012cbb..fa7841b 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -1445,10 +1445,9 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, bool fbcon) /* evict vram memory */ radeon_bo_evict_vram(rdev);
- mutex_lock(&rdev->ring_lock); /* wait for gpu to finish processing current batch */ for (i = 0; i < RADEON_NUM_RINGS; i++) { - r = radeon_fence_wait_empty_locked(rdev, i); + r = radeon_fence_wait_empty(rdev, i); if (r) { /* delay GPU reset to resume */ force_completion = true; @@ -1457,7 +1456,6 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, bool fbcon) if (force_completion) { radeon_fence_driver_force_completion(rdev); } - mutex_unlock(&rdev->ring_lock);
radeon_save_bios_scratch_regs(rdev);
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index c37cb79..a77b1c1 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -288,7 +288,6 @@ static bool radeon_fence_any_seq_signaled(struct radeon_device *rdev, u64 *seq) * @rdev: radeon device pointer * @target_seq: sequence number(s) we want to wait for * @intr: use interruptable sleep - * @lock_ring: whether the ring should be locked or not * * Wait for the requested sequence number(s) to be written by any ring * (all asics). Sequnce number array is indexed by ring id. @@ -299,7 +298,7 @@ static bool radeon_fence_any_seq_signaled(struct radeon_device *rdev, u64 *seq) * -EDEADLK is returned when a GPU lockup has been detected. */ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq, - bool intr, bool lock_ring) + bool intr) { uint64_t last_seq[RADEON_NUM_RINGS]; bool signaled; @@ -358,9 +357,6 @@ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq, if (i != RADEON_NUM_RINGS) continue;
- if (lock_ring) - mutex_lock(&rdev->ring_lock); - for (i = 0; i < RADEON_NUM_RINGS; ++i) { if (!target_seq[i]) continue; @@ -378,14 +374,9 @@ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq,
/* remember that we need an reset */ rdev->needs_reset = true; - if (lock_ring) - mutex_unlock(&rdev->ring_lock); wake_up_all(&rdev->fence_queue); return -EDEADLK; } - - if (lock_ring) - mutex_unlock(&rdev->ring_lock); } } return 0; @@ -416,7 +407,7 @@ int radeon_fence_wait(struct radeon_fence *fence, bool intr) if (seq[fence->ring] == RADEON_FENCE_SIGNALED_SEQ) return 0;
- r = radeon_fence_wait_seq(fence->rdev, seq, intr, true); + r = radeon_fence_wait_seq(fence->rdev, seq, intr); if (r) return r;
@@ -464,7 +455,7 @@ int radeon_fence_wait_any(struct radeon_device *rdev, if (num_rings == 0) return -ENOENT;
- r = radeon_fence_wait_seq(rdev, seq, intr, true); + r = radeon_fence_wait_seq(rdev, seq, intr); if (r) { return r; } @@ -472,37 +463,7 @@ int radeon_fence_wait_any(struct radeon_device *rdev, }
/** - * radeon_fence_wait_locked - wait for a fence to signal - * - * @fence: radeon fence object - * - * Wait for the requested fence to signal (all asics). - * Returns 0 if the fence has passed, error for all other cases. - */ -int radeon_fence_wait_locked(struct radeon_fence *fence) -{ - uint64_t seq[RADEON_NUM_RINGS] = {}; - int r; - - if (fence == NULL) { - WARN(1, "Querying an invalid fence : %p !\n", fence); - return -EINVAL; - } - - seq[fence->ring] = fence->seq; - if (seq[fence->ring] == RADEON_FENCE_SIGNALED_SEQ) - return 0; - - r = radeon_fence_wait_seq(fence->rdev, seq, false, false); - if (r) - return r; - - fence->seq = RADEON_FENCE_SIGNALED_SEQ; - return 0; -} - -/** - * radeon_fence_wait_next_locked - wait for the next fence to signal + * radeon_fence_wait_next - wait for the next fence to signal * * @rdev: radeon device pointer * @ring: ring index the fence is associated with @@ -511,7 +472,7 @@ int radeon_fence_wait_locked(struct radeon_fence *fence) * Returns 0 if the next fence has passed, error for all other cases. * Caller must hold ring lock. */ -int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring) +int radeon_fence_wait_next(struct radeon_device *rdev, int ring) { uint64_t seq[RADEON_NUM_RINGS] = {};
@@ -521,11 +482,11 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring) already the last emited fence */ return -ENOENT; } - return radeon_fence_wait_seq(rdev, seq, false, false); + return radeon_fence_wait_seq(rdev, seq, false); }
/** - * radeon_fence_wait_empty_locked - wait for all fences to signal + * radeon_fence_wait_empty - wait for all fences to signal * * @rdev: radeon device pointer * @ring: ring index the fence is associated with @@ -534,7 +495,7 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring) * Returns 0 if the fences have passed, error for all other cases. * Caller must hold ring lock. */ -int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring) +int radeon_fence_wait_empty(struct radeon_device *rdev, int ring) { uint64_t seq[RADEON_NUM_RINGS] = {}; int r; @@ -543,7 +504,7 @@ int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring) if (!seq[ring]) return 0;
- r = radeon_fence_wait_seq(rdev, seq, false, false); + r = radeon_fence_wait_seq(rdev, seq, false); if (r) { if (r == -EDEADLK) return -EDEADLK; @@ -794,7 +755,7 @@ void radeon_fence_driver_fini(struct radeon_device *rdev) for (ring = 0; ring < RADEON_NUM_RINGS; ring++) { if (!rdev->fence_drv[ring].initialized) continue; - r = radeon_fence_wait_empty_locked(rdev, ring); + r = radeon_fence_wait_empty(rdev, ring); if (r) { /* no need to trigger GPU reset as we are unloading */ radeon_fence_driver_force_completion(rdev); diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 4ad9af9..0119af4 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -260,7 +260,7 @@ static void radeon_pm_set_clocks(struct radeon_device *rdev) if (!ring->ready) { continue; } - r = radeon_fence_wait_empty_locked(rdev, i); + r = radeon_fence_wait_empty(rdev, i); if (r) { /* needs a GPU reset dont reset here */ mutex_unlock(&rdev->ring_lock); @@ -896,7 +896,7 @@ force: for (i = 0; i < RADEON_NUM_RINGS; i++) { struct radeon_ring *ring = &rdev->ring[i]; if (ring->ready) - radeon_fence_wait_empty_locked(rdev, i); + radeon_fence_wait_empty(rdev, i); }
/* program the new power state */ diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c index ae72108..767b5e7 100644 --- a/drivers/gpu/drm/radeon/radeon_ring.c +++ b/drivers/gpu/drm/radeon/radeon_ring.c @@ -382,7 +382,7 @@ int radeon_ring_alloc(struct radeon_device *rdev, struct radeon_ring *ring, unsi if (ndw < ring->ring_free_dw) { break; } - r = radeon_fence_wait_next_locked(rdev, ring->idx); + r = radeon_fence_wait_next(rdev, ring->idx); if (r) return r; } diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c index 9006b32..6140af6 100644 --- a/drivers/gpu/drm/radeon/radeon_semaphore.c +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c @@ -147,7 +147,9 @@ int radeon_semaphore_sync_rings(struct radeon_device *rdev,
if (++count > RADEON_NUM_SYNCS) { /* not enough room, wait manually */ - radeon_fence_wait_locked(fence); + r = radeon_fence_wait(fence, false); + if (r) + return r; continue; }
@@ -161,7 +163,9 @@ int radeon_semaphore_sync_rings(struct radeon_device *rdev, if (!radeon_semaphore_emit_signal(rdev, i, semaphore)) { /* signaling wasn't successful wait manually */ radeon_ring_undo(&rdev->ring[i]); - radeon_fence_wait_locked(fence); + r = radeon_fence_wait(fence, false); + if (r) + return r; continue; }
@@ -169,7 +173,9 @@ int radeon_semaphore_sync_rings(struct radeon_device *rdev, if (!radeon_semaphore_emit_wait(rdev, ring, semaphore)) { /* waiting wasn't successful wait manually */ radeon_ring_undo(&rdev->ring[i]); - radeon_fence_wait_locked(fence); + r = radeon_fence_wait(fence, false); + if (r) + return r; continue; }
On Tue, Feb 18, 2014 at 11:52 AM, Christian König deathsimple@vodafone.de wrote:
From: Christian König christian.koenig@amd.com
Use atomics and jiffies_64, so that we don't need to have the ring mutex locked any more and avoid wrap arounds.
Signed-off-by: Christian König christian.koenig@amd.com
Series is:
Reviewed-by: Alex Deucher alexander.deucher@amd.com
drivers/gpu/drm/radeon/radeon.h | 4 ++-- drivers/gpu/drm/radeon/radeon_ring.c | 21 +++++++++------------ 2 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index a415f8e..9269413 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -802,8 +802,8 @@ struct radeon_ring { unsigned ring_size; unsigned ring_free_dw; int count_dw;
unsigned long last_activity;unsigned last_rptr;
atomic_t last_rptr;atomic64_t last_activity; uint64_t gpu_addr; uint32_t align_mask; uint32_t ptr_mask;diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c index b14c86d..ae72108 100644 --- a/drivers/gpu/drm/radeon/radeon_ring.c +++ b/drivers/gpu/drm/radeon/radeon_ring.c @@ -485,8 +485,8 @@ void radeon_ring_unlock_undo(struct radeon_device *rdev, struct radeon_ring *rin void radeon_ring_lockup_update(struct radeon_device *rdev, struct radeon_ring *ring) {
ring->last_rptr = radeon_ring_get_rptr(rdev, ring);ring->last_activity = jiffies;
atomic_set(&ring->last_rptr, radeon_ring_get_rptr(rdev, ring));atomic64_set(&ring->last_activity, jiffies_64);}
/** @@ -498,22 +498,19 @@ void radeon_ring_lockup_update(struct radeon_device *rdev, bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring) { uint32_t rptr = radeon_ring_get_rptr(rdev, ring);
unsigned long cjiffies, elapsed;
uint64_t last = atomic64_read(&ring->last_activity);uint64_t elapsed;
cjiffies = jiffies;if (!time_after(cjiffies, ring->last_activity)) {/* likely a wrap around */radeon_ring_lockup_update(rdev, ring);return false;}if (rptr != ring->last_rptr) {
if (rptr != atomic_read(&ring->last_rptr)) { /* CP is still working no lockup */ radeon_ring_lockup_update(rdev, ring); return false; }
elapsed = jiffies_to_msecs(cjiffies - ring->last_activity);
elapsed = jiffies_to_msecs(jiffies_64 - last); if (radeon_lockup_timeout && elapsed >= radeon_lockup_timeout) {
dev_err(rdev->dev, "GPU lockup CP stall for more than %lumsec\n", elapsed);
dev_err(rdev->dev, "GPU lockup ring %d stall for more ""than %Lumsec\n", ring->idx, elapsed); return true; } /* give a chance to the GPU ... */-- 1.8.3.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christian König