drm/radeon: fix & improve ih ring handling v3

The spinlock was actually there to protect the
rptr, but rptr was read outside of the locked area.

Also we don't really need a spinlock here, an
atomic should to quite fine since we only need to
prevent it from being reentrant.

v2: Keep the spinlock....
v3: Back to an atomic again after finding & fixing the real bug.

Signed-off-by: Christian Koenig <christian.koenig@amd.com>

1 files changed, 13 insertions, 16 deletions

diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index aced97bb79e0..b487c69a486d 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2858,7 +2858,6 @@ void r600_disable_interrupts(struct radeon_device *rdev)
         WREG32(IH_RB_RPTR, 0);
         WREG32(IH_RB_WPTR, 0);
         rdev->ih.enabled = false;
-        rdev->ih.wptr = 0;
         rdev->ih.rptr = 0;
 }
 
@@ -3310,7 +3309,6 @@ int r600_irq_process(struct radeon_device *rdev)
         u32 rptr;
         u32 src_id, src_data;
         u32 ring_index;
-        unsigned long flags;
         bool queue_hotplug = false;
         bool queue_hdmi = false;
 
@@ -3322,24 +3320,21 @@ int r600_irq_process(struct radeon_device *rdev)
                 RREG32(IH_RB_WPTR);
 
         wptr = r600_get_ih_wptr(rdev);
-        rptr = rdev->ih.rptr;
-        DRM_DEBUG("r600_irq_process start: rptr %d, wptr %d\n", rptr, wptr);
-
-        spin_lock_irqsave(&rdev->ih.lock, flags);
 
-        if (rptr == wptr) {
-                spin_unlock_irqrestore(&rdev->ih.lock, flags);
+restart_ih:
+        /* is somebody else already processing irqs? */
+        if (atomic_xchg(&rdev->ih.lock, 1))
                 return IRQ_NONE;
-        }
 
-restart_ih:
+        rptr = rdev->ih.rptr;
+        DRM_DEBUG("r600_irq_process start: rptr %d, wptr %d\n", rptr, wptr);
+
         /* Order reading of wptr vs. reading of IH ring data */
         rmb();
 
         /* display interrupts */
         r600_irq_ack(rdev);
 
-        rdev->ih.wptr = wptr;
         while (rptr != wptr) {
                 /* wptr/rptr are in bytes! */
                 ring_index = rptr / 4;
@@ -3493,17 +3488,19 @@ restart_ih:
                 rptr += 16;
                 rptr &= rdev->ih.ptr_mask;
         }
-        /* make sure wptr hasn't changed while processing */
-        wptr = r600_get_ih_wptr(rdev);
-        if (wptr != rdev->ih.wptr)
-                goto restart_ih;
         if (queue_hotplug)
                 schedule_work(&rdev->hotplug_work);
         if (queue_hdmi)
                 schedule_work(&rdev->audio_work);
         rdev->ih.rptr = rptr;
         WREG32(IH_RB_RPTR, rdev->ih.rptr);
-        spin_unlock_irqrestore(&rdev->ih.lock, flags);
+        atomic_set(&rdev->ih.lock, 0);
+
+        /* make sure wptr hasn't changed while processing */
+        wptr = r600_get_ih_wptr(rdev);
+        if (wptr != rptr)
+                goto restart_ih;
+
         return IRQ_HANDLED;
 }