Fix vblank enable/disable callbacks

There were two problems with the existing callback code:  the vblank
enable callback happened multiple times per disable, making drivers more
complex than they had to be, and there was a race between the final
decrement of the vblank usage counter and the next enable call, which
could have resulted in a put->schedule disable->get->enable->disable
sequence, which would be bad.

So add a new vblank_enabled array to track vblank enable on per-pipe
basis, and add a lock to protect it along with the refcount +
enable/disable calls to fix the race.

2 files changed, 26 insertions, 5 deletions

diff --git a/linux-core/drmP.h b/linux-core/drmP.h
index 4e8b087b..33f3649e 100644
--- a/linux-core/drmP.h
+++ b/linux-core/drmP.h
@@ -836,6 +836,8 @@ struct drm_device {
 	u32 *last_vblank;		/* protected by dev->vbl_lock, used */
 					/* for wraparound handling */
 	u32 *vblank_offset;		/* used to track how many vblanks */
+	int *vblank_enabled;		/* so we don't call enable more than
+					   once per disable */
 	u32 *vblank_premodeset;		/*  were lost during modeset */
 	struct timer_list vblank_disable_timer;
 
diff --git a/linux-core/drm_irq.c b/linux-core/drm_irq.c
index 367d2dd8..e4940bb7 100644
--- a/linux-core/drm_irq.c
+++ b/linux-core/drm_irq.c
@@ -74,11 +74,18 @@ int drm_irq_by_busid(struct drm_device *dev, void *data,
 static void vblank_disable_fn(unsigned long arg)
 {
 	struct drm_device *dev = (struct drm_device *)arg;
+	unsigned long irqflags;
 	int i;
 
-	for (i = 0; i < dev->num_crtcs; i++)
-		if (atomic_read(&dev->vblank_refcount[i]) == 0)
+	for (i = 0; i < dev->num_crtcs; i++) {
+		spin_lock_irqsave(&dev->vbl_lock, irqflags);
+		if (atomic_read(&dev->vblank_refcount[i]) == 0 &&
+		    dev->vblank_enabled[i]) {
 			dev->driver->disable_vblank(dev, i);
+			dev->vblank_enabled[i] = 0;
+		}
+		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
+	}
 }
 
 int drm_vblank_init(struct drm_device *dev, int num_crtcs)
@@ -111,6 +118,11 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
 	if (!dev->vblank_refcount)
 		goto err;
 
+	dev->vblank_enabled = drm_calloc(num_crtcs, sizeof(int),
+					 DRM_MEM_DRIVER);
+	if (!dev->vblank_enabled)
+		goto err;
+
 	dev->last_vblank = drm_calloc(num_crtcs, sizeof(u32), DRM_MEM_DRIVER);
 	if (!dev->last_vblank)
 		goto err;
@@ -143,6 +155,8 @@ err:
 		 DRM_MEM_DRIVER);
 	drm_free(dev->vblank_refcount, sizeof(*dev->vblank_refcount) *
 		 num_crtcs, DRM_MEM_DRIVER);
+	drm_free(dev->vblank_enabled, sizeof(*dev->vblank_enabled) * num_crtcs,
+		 DRM_MEM_DRIVER);
 	drm_free(dev->last_vblank, sizeof(*dev->last_vblank) * num_crtcs,
 		 DRM_MEM_DRIVER);
 	drm_free(dev->vblank_premodeset, sizeof(*dev->vblank_premodeset) *
@@ -357,14 +371,20 @@ EXPORT_SYMBOL(drm_update_vblank_count);
  */
 int drm_vblank_get(struct drm_device *dev, int crtc)
 {
+	unsigned long irqflags;
 	int ret = 0;
 
+	spin_lock_irqsave(&dev->vbl_lock, irqflags);	
 	/* Going from 0->1 means we have to enable interrupts again */
-	if (atomic_add_return(1, &dev->vblank_refcount[crtc]) == 1) {
+	if (atomic_add_return(1, &dev->vblank_refcount[crtc]) == 1 &&
+	    !dev->vblank_enabled[crtc]) {
 		ret = dev->driver->enable_vblank(dev, crtc);
 		if (ret)
 			atomic_dec(&dev->vblank_refcount[crtc]);
+		else
+			dev->vblank_enabled[crtc] = 1;
 	}
+	spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
 
 	return ret;
 }
@@ -382,8 +402,7 @@ void drm_vblank_put(struct drm_device *dev, int crtc)
 {
 	/* Last user schedules interrupt disable */
 	if (atomic_dec_and_test(&dev->vblank_refcount[crtc]))
-		mod_timer(&dev->vblank_disable_timer,
-			  round_jiffies_relative(DRM_HZ));
+	    mod_timer(&dev->vblank_disable_timer, jiffies + 5*DRM_HZ);
 }
 EXPORT_SYMBOL(drm_vblank_put);