From 79d69285202b55f269aa88a6bcda257257c9dee3 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Thu, 7 Feb 2008 10:40:06 -0800 Subject: 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. --- linux-core/drmP.h | 2 ++ linux-core/drm_irq.c | 29 ++++++++++++++++++++++++----- 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); -- cgit v1.2.3