Fix vblank enable/disable callbacks
authorJesse Barnes <jbarnes@nietzche.virtuousgeek.org>
Thu, 7 Feb 2008 18:40:06 +0000 (10:40 -0800)
committerJesse Barnes <jbarnes@nietzche.virtuousgeek.org>
Thu, 7 Feb 2008 18:40:06 +0000 (10:40 -0800)
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.

linux-core/drmP.h
linux-core/drm_irq.c

index 4e8b087..33f3649 100644 (file)
@@ -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;
 
index 367d2dd..e4940bb 100644 (file)
@@ -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);