On Don, 2012-05-31 at 22:16 +0200, Christian König wrote:
So we can skip the looking.
'locking'
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c index 73cd0fd..52f85ba 100644 --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c @@ -87,16 +87,7 @@ void radeon_driver_irq_preinstall_kms(struct drm_device *dev)
int radeon_driver_irq_postinstall_kms(struct drm_device *dev) {
- struct radeon_device *rdev = dev->dev_private;
- unsigned long irqflags;
- unsigned i;
- dev->max_vblank_count = 0x001fffff;
- spin_lock_irqsave(&rdev->irq.lock, irqflags);
- for (i = 0; i < RADEON_NUM_RINGS; i++)
rdev->irq.sw_int[i] = true;- radeon_irq_set(rdev);
- spin_unlock_irqrestore(&rdev->irq.lock, irqflags); return 0;
}
Why does this function no longer need to enable SW interrupts? If it really doesn't, that might be material for a separate patch.
@@ -225,25 +216,28 @@ void radeon_irq_kms_sw_irq_get(struct radeon_device *rdev, int ring) { unsigned long irqflags;
- spin_lock_irqsave(&rdev->irq.lock, irqflags);
- if (rdev->ddev->irq_enabled && (++rdev->irq.sw_refcount[ring] == 1)) {
rdev->irq.sw_int[ring] = true;
- if (!rdev->ddev->irq_enabled)
return;- if (atomic_inc_return(&rdev->irq.ring_int[ring]) == 1) {
spin_lock_irqsave(&rdev->irq.lock, irqflags);spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
- spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
}
void radeon_irq_kms_sw_irq_put(struct radeon_device *rdev, int ring) { unsigned long irqflags;
- spin_lock_irqsave(&rdev->irq.lock, irqflags);
- BUG_ON(rdev->ddev->irq_enabled && rdev->irq.sw_refcount[ring] <= 0);
- if (rdev->ddev->irq_enabled && (--rdev->irq.sw_refcount[ring] == 0)) {
rdev->irq.sw_int[ring] = false;
- if (!rdev->ddev->irq_enabled)
return;- if (atomic_dec_and_test(&rdev->irq.ring_int[ring])) {
spin_lock_irqsave(&rdev->irq.lock, irqflags);spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
- spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
}
void radeon_irq_kms_pflip_irq_get(struct radeon_device *rdev, int crtc)
I think this might introduce a race condition:
Thread 0 Thread 1 -------- -------- atomic_inc_return() returns 1 spin_lock_irqsave() atomic_dec_and_test() radeon_irq_set()
=> the interrupt won't be enabled.
Maybe this explains why you couldn't remove the spinlock in patch 6?