From e6ae8687a87b1fe5c25e824c8ad300f5587eb622 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Wed, 6 Aug 2014 13:16:59 -0400 Subject: drm: idiot-proof vblank After spending slightly more time than I'd care to admit debugging the various and presumably spectacular way things fail when you pass too low a value to drm_vblank_init() (thanks console-lock for not letting me see the carnage!), I decided it might be a good idea to add some sanity checking. Signed-off-by: Rob Clark Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 0de123afdb34..6f16a104d6d0 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -730,6 +730,8 @@ EXPORT_SYMBOL(drm_get_last_vbltimestamp); */ u32 drm_vblank_count(struct drm_device *dev, int crtc) { + if (WARN_ON(crtc >= dev->num_crtcs)) + return 0; return atomic_read(&dev->vblank[crtc].count); } EXPORT_SYMBOL(drm_vblank_count); @@ -752,6 +754,9 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc, { u32 cur_vblank; + if (WARN_ON(crtc >= dev->num_crtcs)) + return 0; + /* Read timestamp from slot of _vblank_time ringbuffer * that corresponds to current vblank count. Retry if * count has incremented during readout. This works like @@ -927,6 +932,9 @@ int drm_vblank_get(struct drm_device *dev, int crtc) unsigned long irqflags; int ret = 0; + if (WARN_ON(crtc >= dev->num_crtcs)) + return -EINVAL; + 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[crtc].refcount) == 1) { @@ -975,6 +983,9 @@ void drm_vblank_put(struct drm_device *dev, int crtc) { BUG_ON(atomic_read(&dev->vblank[crtc].refcount) == 0); + if (WARN_ON(crtc >= dev->num_crtcs)) + return; + /* Last user schedules interrupt disable */ if (atomic_dec_and_test(&dev->vblank[crtc].refcount) && (drm_vblank_offdelay > 0)) @@ -1019,6 +1030,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc) unsigned long irqflags; unsigned int seq; + if (WARN_ON(crtc >= dev->num_crtcs)) + return; + spin_lock_irqsave(&dev->vbl_lock, irqflags); vblank_disable_and_save(dev, crtc); wake_up(&dev->vblank[crtc].queue); @@ -1078,6 +1092,9 @@ void drm_vblank_on(struct drm_device *dev, int crtc) { unsigned long irqflags; + if (WARN_ON(crtc >= dev->num_crtcs)) + return; + spin_lock_irqsave(&dev->vbl_lock, irqflags); /* re-enable interrupts if there's are users left */ if (atomic_read(&dev->vblank[crtc].refcount) != 0) @@ -1131,6 +1148,10 @@ void drm_vblank_pre_modeset(struct drm_device *dev, int crtc) /* vblank is not initialized (IRQ not installed ?), or has been freed */ if (!dev->num_crtcs) return; + + if (WARN_ON(crtc >= dev->num_crtcs)) + return; + /* * To avoid all the problems that might happen if interrupts * were enabled/disabled around or between these calls, we just @@ -1439,6 +1460,9 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) if (!dev->num_crtcs) return false; + if (WARN_ON(crtc >= dev->num_crtcs)) + return false; + /* Need timestamp lock to prevent concurrent execution with * vblank enable/disable, as this would cause inconsistent * or corrupted timestamps and vblank counts. -- cgit v1.2.2 From 7ffd7a68511c710b84db3548a1997fd2625f580a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:44 +0300 Subject: drm: Always reject drm_vblank_get() after drm_vblank_off() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make sure drm_vblank_get() never succeeds when called between drm_vblank_off() and drm_vblank_on(). Borrow a trick from the old drm_vblank_{pre,post}_modeset() functions and just bump the refcount in drm_vblank_off() and drop it in drm_vblank_on(). When drm_vblank_get() encounters a >0 refcount and the vblank interrupt is already disabled it will simply return -EINVAL. Hopefully the use of inmodeset won't conflict badly with drm_vblank_{pre,post}_modeset(). For i915 there's a window between drm_vblank_off() and marking the crtc as inactive where the current code still allows drm_vblank_get(). v2: Describe what drm_vblank_get() does to explain how a simple refcount bump manages to fix things (Daniel) Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 0de123afdb34..b16a63622bad 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1039,6 +1039,15 @@ void drm_vblank_off(struct drm_device *dev, int crtc) } spin_unlock(&dev->event_lock); + /* + * Prevent subsequent drm_vblank_get() from re-enabling + * the vblank interrupt by bumping the refcount. + */ + if (!dev->vblank[crtc].inmodeset) { + atomic_inc(&dev->vblank[crtc].refcount); + dev->vblank[crtc].inmodeset = 1; + } + spin_unlock_irqrestore(&dev->vbl_lock, irqflags); } EXPORT_SYMBOL(drm_vblank_off); @@ -1079,6 +1088,11 @@ void drm_vblank_on(struct drm_device *dev, int crtc) unsigned long irqflags; spin_lock_irqsave(&dev->vbl_lock, irqflags); + /* Drop our private "prevent drm_vblank_get" refcount */ + if (dev->vblank[crtc].inmodeset) { + atomic_dec(&dev->vblank[crtc].refcount); + dev->vblank[crtc].inmodeset = 0; + } /* re-enable interrupts if there's are users left */ if (atomic_read(&dev->vblank[crtc].refcount) != 0) WARN_ON(drm_vblank_enable(dev, crtc)); -- cgit v1.2.2 From 844b03f27739135fe1fed2fef06da0ffc4c7a081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:46 +0300 Subject: drm: Don't clear vblank timestamps when vblank interrupt is disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clearing the timestamps causes us to send zeroed timestamps to userspace if they get sent out in response to the drm_vblank_off(). It's better to send the very latest timestamp and count instead. Testcase: igt/kms_flip/modeset-vs-vblank-race Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 11 ----------- 1 file changed, 11 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index b16a63622bad..65d2da9b604b 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -55,14 +55,6 @@ */ #define DRM_REDUNDANT_VBLIRQ_THRESH_NS 1000000 -/* - * Clear vblank timestamp buffer for a crtc. - */ -static void clear_vblank_timestamps(struct drm_device *dev, int crtc) -{ - memset(dev->vblank[crtc].time, 0, sizeof(dev->vblank[crtc].time)); -} - /* * Disable vblank irq's on crtc, make sure that last vblank count * of hardware and corresponding consistent software vblank counter @@ -131,9 +123,6 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) smp_mb__after_atomic(); } - /* Invalidate all timestamps while vblank irq's are off. */ - clear_vblank_timestamps(dev, crtc); - spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); } -- cgit v1.2.2 From 13b030af54a5e307cbcccdf5479873fbc4b7f185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:47 +0300 Subject: drm: Move drm_update_vblank_count() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move drm_update_vblank_count() to avoid forward a declaration. No functional change. Reviewed-by: Matt Roper Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 128 +++++++++++++++++++++++----------------------- 1 file changed, 64 insertions(+), 64 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 65d2da9b604b..af965174b083 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -55,6 +55,70 @@ */ #define DRM_REDUNDANT_VBLIRQ_THRESH_NS 1000000 +/** + * drm_update_vblank_count - update the master vblank counter + * @dev: DRM device + * @crtc: counter to update + * + * Call back into the driver to update the appropriate vblank counter + * (specified by @crtc). Deal with wraparound, if it occurred, and + * update the last read value so we can deal with wraparound on the next + * call if necessary. + * + * Only necessary when going from off->on, to account for frames we + * didn't get an interrupt for. + * + * Note: caller must hold dev->vbl_lock since this reads & writes + * device vblank fields. + */ +static void drm_update_vblank_count(struct drm_device *dev, int crtc) +{ + u32 cur_vblank, diff, tslot, rc; + struct timeval t_vblank; + + /* + * Interrupts were disabled prior to this call, so deal with counter + * wrap if needed. + * NOTE! It's possible we lost a full dev->max_vblank_count events + * here if the register is small or we had vblank interrupts off for + * a long time. + * + * We repeat the hardware vblank counter & timestamp query until + * we get consistent results. This to prevent races between gpu + * updating its hardware counter while we are retrieving the + * corresponding vblank timestamp. + */ + do { + cur_vblank = dev->driver->get_vblank_counter(dev, crtc); + rc = drm_get_last_vbltimestamp(dev, crtc, &t_vblank, 0); + } while (cur_vblank != dev->driver->get_vblank_counter(dev, crtc)); + + /* Deal with counter wrap */ + diff = cur_vblank - dev->vblank[crtc].last; + if (cur_vblank < dev->vblank[crtc].last) { + diff += dev->max_vblank_count; + + DRM_DEBUG("last_vblank[%d]=0x%x, cur_vblank=0x%x => diff=0x%x\n", + crtc, dev->vblank[crtc].last, cur_vblank, diff); + } + + DRM_DEBUG("enabling vblank interrupts on crtc %d, missed %d\n", + crtc, diff); + + /* Reinitialize corresponding vblank timestamp if high-precision query + * available. Skip this step if query unsupported or failed. Will + * reinitialize delayed at next vblank interrupt in that case. + */ + if (rc) { + tslot = atomic_read(&dev->vblank[crtc].count) + diff; + vblanktimestamp(dev, crtc, tslot) = t_vblank; + } + + smp_mb__before_atomic(); + atomic_add(diff, &dev->vblank[crtc].count); + smp_mb__after_atomic(); +} + /* * Disable vblank irq's on crtc, make sure that last vblank count * of hardware and corresponding consistent software vblank counter @@ -798,70 +862,6 @@ void drm_send_vblank_event(struct drm_device *dev, int crtc, } EXPORT_SYMBOL(drm_send_vblank_event); -/** - * drm_update_vblank_count - update the master vblank counter - * @dev: DRM device - * @crtc: counter to update - * - * Call back into the driver to update the appropriate vblank counter - * (specified by @crtc). Deal with wraparound, if it occurred, and - * update the last read value so we can deal with wraparound on the next - * call if necessary. - * - * Only necessary when going from off->on, to account for frames we - * didn't get an interrupt for. - * - * Note: caller must hold dev->vbl_lock since this reads & writes - * device vblank fields. - */ -static void drm_update_vblank_count(struct drm_device *dev, int crtc) -{ - u32 cur_vblank, diff, tslot, rc; - struct timeval t_vblank; - - /* - * Interrupts were disabled prior to this call, so deal with counter - * wrap if needed. - * NOTE! It's possible we lost a full dev->max_vblank_count events - * here if the register is small or we had vblank interrupts off for - * a long time. - * - * We repeat the hardware vblank counter & timestamp query until - * we get consistent results. This to prevent races between gpu - * updating its hardware counter while we are retrieving the - * corresponding vblank timestamp. - */ - do { - cur_vblank = dev->driver->get_vblank_counter(dev, crtc); - rc = drm_get_last_vbltimestamp(dev, crtc, &t_vblank, 0); - } while (cur_vblank != dev->driver->get_vblank_counter(dev, crtc)); - - /* Deal with counter wrap */ - diff = cur_vblank - dev->vblank[crtc].last; - if (cur_vblank < dev->vblank[crtc].last) { - diff += dev->max_vblank_count; - - DRM_DEBUG("last_vblank[%d]=0x%x, cur_vblank=0x%x => diff=0x%x\n", - crtc, dev->vblank[crtc].last, cur_vblank, diff); - } - - DRM_DEBUG("enabling vblank interrupts on crtc %d, missed %d\n", - crtc, diff); - - /* Reinitialize corresponding vblank timestamp if high-precision query - * available. Skip this step if query unsupported or failed. Will - * reinitialize delayed at next vblank interrupt in that case. - */ - if (rc) { - tslot = atomic_read(&dev->vblank[crtc].count) + diff; - vblanktimestamp(dev, crtc, tslot) = t_vblank; - } - - smp_mb__before_atomic(); - atomic_add(diff, &dev->vblank[crtc].count); - smp_mb__after_atomic(); -} - /** * drm_vblank_enable - enable the vblank interrupt on a CRTC * @dev: DRM device -- cgit v1.2.2 From 812e7465a7decf3cca0b5f71977a25eecd9626a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:48 +0300 Subject: drm: Have the vblank counter account for the time between vblank irq disable and drm_vblank_off() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the vblank irq has already been disabled (via the disable timer) when we call drm_vblank_off() sample the counter and timestamp one last time. This will make the sure that the user space visible counter will account for time between vblank irq disable and drm_vblank_off(). Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index af965174b083..1f86f6c6ecc6 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -140,6 +140,19 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) */ spin_lock_irqsave(&dev->vblank_time_lock, irqflags); + /* + * If the vblank interrupt was already disbled update the count + * and timestamp to maintain the appearance that the counter + * has been ticking all along until this time. This makes the + * count account for the entire time between drm_vblank_on() and + * drm_vblank_off(). + */ + if (!dev->vblank[crtc].enabled) { + drm_update_vblank_count(dev, crtc); + spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); + return; + } + dev->driver->disable_vblank(dev, crtc); dev->vblank[crtc].enabled = false; -- cgit v1.2.2 From f8ad028cc033f75fc479ca1c30e2ea4ba56e5269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:49 +0300 Subject: drm: Avoid random vblank counter jumps if the hardware counter has been reset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When drm_vblank_on() is called the hardware vblank counter may have been reset, so we can't trust that the old values sampled prior to drm_vblank_off() have anything to do with the new values. So update the .last count in drm_vblank_on() to make the first drm_vblank_enable() consider that as the reference point. This will correct the user space visible counter to account for the time between drm_vblank_on() and the first drm_vblank_enable() calls. For extra safety subtract one from the .last count in drm_vblank_on() to make sure that user space will never see the same counter value before and after modeset. Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 1f86f6c6ecc6..fc1525a499d9 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1095,6 +1095,18 @@ void drm_vblank_on(struct drm_device *dev, int crtc) atomic_dec(&dev->vblank[crtc].refcount); dev->vblank[crtc].inmodeset = 0; } + + /* + * sample the current counter to avoid random jumps + * when drm_vblank_enable() applies the diff + * + * -1 to make sure user will never see the same + * vblank counter value before and after a modeset + */ + dev->vblank[crtc].last = + (dev->driver->get_vblank_counter(dev, crtc) - 1) & + dev->max_vblank_count; + /* re-enable interrupts if there's are users left */ if (atomic_read(&dev->vblank[crtc].refcount) != 0) WARN_ON(drm_vblank_enable(dev, crtc)); -- cgit v1.2.2 From 8a51d5bef07f1c8c59de20089fb27ea39d395f1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:50 +0300 Subject: drm: Reduce the amount of dev->vblank[crtc] in the code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declare a local struct drm_vblank_crtc * and use that instead of having to do dig it out via 'dev->vblank[crtc]' everywhere. Performed with the following coccinelle incantation, and a few manual whitespace cleanups: @@ identifier func,member; expression num_crtcs; struct drm_device *dev; unsigned int crtc; @@ func (...) { + struct drm_vblank_crtc *vblank; ... if (crtc >= num_crtcs) return ...; + vblank = &dev->vblank[crtc]; <+... ( - dev->vblank[crtc].member + vblank->member | - &(dev->vblank[crtc]) + vblank ) ...+> } @@ struct drm_device *dev; int crtc; identifier member; expression num_crtcs; @@ for (crtc = 0; crtc < num_crtcs; crtc++) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; + <+... ( - dev->vblank[crtc].member + vblank->member | - &(dev->vblank[crtc]) + vblank ) ...+> } @@ identifier func,member; @@ func (struct drm_device *dev, int crtc, ...) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; <+... ( - dev->vblank[crtc].member + vblank->member | - &(dev->vblank[crtc]) + vblank ) ...+> } v2: Rebased Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 134 +++++++++++++++++++++++++++------------------- 1 file changed, 79 insertions(+), 55 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index fc1525a499d9..b4460bf0e0e4 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -73,6 +73,7 @@ */ static void drm_update_vblank_count(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; u32 cur_vblank, diff, tslot, rc; struct timeval t_vblank; @@ -94,12 +95,12 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) } while (cur_vblank != dev->driver->get_vblank_counter(dev, crtc)); /* Deal with counter wrap */ - diff = cur_vblank - dev->vblank[crtc].last; - if (cur_vblank < dev->vblank[crtc].last) { + diff = cur_vblank - vblank->last; + if (cur_vblank < vblank->last) { diff += dev->max_vblank_count; DRM_DEBUG("last_vblank[%d]=0x%x, cur_vblank=0x%x => diff=0x%x\n", - crtc, dev->vblank[crtc].last, cur_vblank, diff); + crtc, vblank->last, cur_vblank, diff); } DRM_DEBUG("enabling vblank interrupts on crtc %d, missed %d\n", @@ -110,12 +111,12 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) * reinitialize delayed at next vblank interrupt in that case. */ if (rc) { - tslot = atomic_read(&dev->vblank[crtc].count) + diff; + tslot = atomic_read(&vblank->count) + diff; vblanktimestamp(dev, crtc, tslot) = t_vblank; } smp_mb__before_atomic(); - atomic_add(diff, &dev->vblank[crtc].count); + atomic_add(diff, &vblank->count); smp_mb__after_atomic(); } @@ -127,6 +128,7 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) */ static void vblank_disable_and_save(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; unsigned long irqflags; u32 vblcount; s64 diff_ns; @@ -147,14 +149,14 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * count account for the entire time between drm_vblank_on() and * drm_vblank_off(). */ - if (!dev->vblank[crtc].enabled) { + if (!vblank->enabled) { drm_update_vblank_count(dev, crtc); spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); return; } dev->driver->disable_vblank(dev, crtc); - dev->vblank[crtc].enabled = false; + vblank->enabled = false; /* No further vblank irq's will be processed after * this point. Get current hardware vblank count and @@ -169,9 +171,9 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * delayed gpu counter increment. */ do { - dev->vblank[crtc].last = dev->driver->get_vblank_counter(dev, crtc); + vblank->last = dev->driver->get_vblank_counter(dev, crtc); vblrc = drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0); - } while (dev->vblank[crtc].last != dev->driver->get_vblank_counter(dev, crtc) && (--count) && vblrc); + } while (vblank->last != dev->driver->get_vblank_counter(dev, crtc) && (--count) && vblrc); if (!count) vblrc = 0; @@ -179,7 +181,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) /* Compute time difference to stored timestamp of last vblank * as updated by last invocation of drm_handle_vblank() in vblank irq. */ - vblcount = atomic_read(&dev->vblank[crtc].count); + vblcount = atomic_read(&vblank->count); diff_ns = timeval_to_ns(&tvblank) - timeval_to_ns(&vblanktimestamp(dev, crtc, vblcount)); @@ -196,7 +198,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * hope for the best. */ if ((vblrc > 0) && (abs64(diff_ns) > 1000000)) { - atomic_inc(&dev->vblank[crtc].count); + atomic_inc(&vblank->count); smp_mb__after_atomic(); } @@ -236,8 +238,10 @@ void drm_vblank_cleanup(struct drm_device *dev) return; for (crtc = 0; crtc < dev->num_crtcs; crtc++) { - del_timer_sync(&dev->vblank[crtc].disable_timer); - vblank_disable_fn((unsigned long)&dev->vblank[crtc]); + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; + + del_timer_sync(&vblank->disable_timer); + vblank_disable_fn((unsigned long)vblank); } kfree(dev->vblank); @@ -270,11 +274,13 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) goto err; for (i = 0; i < num_crtcs; i++) { - dev->vblank[i].dev = dev; - dev->vblank[i].crtc = i; - init_waitqueue_head(&dev->vblank[i].queue); - setup_timer(&dev->vblank[i].disable_timer, vblank_disable_fn, - (unsigned long)&dev->vblank[i]); + struct drm_vblank_crtc *vblank = &dev->vblank[i]; + + vblank->dev = dev; + vblank->crtc = i; + init_waitqueue_head(&vblank->queue); + setup_timer(&vblank->disable_timer, vblank_disable_fn, + (unsigned long)vblank); } DRM_INFO("Supports vblank timestamp caching Rev 2 (21.10.2013).\n"); @@ -426,9 +432,11 @@ int drm_irq_uninstall(struct drm_device *dev) if (dev->num_crtcs) { spin_lock_irqsave(&dev->vbl_lock, irqflags); for (i = 0; i < dev->num_crtcs; i++) { - wake_up(&dev->vblank[i].queue); - dev->vblank[i].enabled = false; - dev->vblank[i].last = + struct drm_vblank_crtc *vblank = &dev->vblank[i]; + + wake_up(&vblank->queue); + vblank->enabled = false; + vblank->last = dev->driver->get_vblank_counter(dev, i); } spin_unlock_irqrestore(&dev->vbl_lock, irqflags); @@ -796,7 +804,9 @@ EXPORT_SYMBOL(drm_get_last_vbltimestamp); */ u32 drm_vblank_count(struct drm_device *dev, int crtc) { - return atomic_read(&dev->vblank[crtc].count); + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; + + return atomic_read(&vblank->count); } EXPORT_SYMBOL(drm_vblank_count); @@ -816,6 +826,7 @@ EXPORT_SYMBOL(drm_vblank_count); u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc, struct timeval *vblanktime) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; u32 cur_vblank; /* Read timestamp from slot of _vblank_time ringbuffer @@ -824,10 +835,10 @@ u32 drm_vblank_count_and_time(struct drm_device *dev, int crtc, * a seqlock. */ do { - cur_vblank = atomic_read(&dev->vblank[crtc].count); + cur_vblank = atomic_read(&vblank->count); *vblanktime = vblanktimestamp(dev, crtc, cur_vblank); smp_rmb(); - } while (cur_vblank != atomic_read(&dev->vblank[crtc].count)); + } while (cur_vblank != atomic_read(&vblank->count)); return cur_vblank; } @@ -882,13 +893,14 @@ EXPORT_SYMBOL(drm_send_vblank_event); */ static int drm_vblank_enable(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; int ret = 0; assert_spin_locked(&dev->vbl_lock); spin_lock(&dev->vblank_time_lock); - if (!dev->vblank[crtc].enabled) { + if (!vblank->enabled) { /* * Enable vblank irqs under vblank_time_lock protection. * All vblank count & timestamp updates are held off @@ -899,9 +911,9 @@ static int drm_vblank_enable(struct drm_device *dev, int crtc) ret = dev->driver->enable_vblank(dev, crtc); DRM_DEBUG("enabling vblank on crtc %d, ret: %d\n", crtc, ret); if (ret) - atomic_dec(&dev->vblank[crtc].refcount); + atomic_dec(&vblank->refcount); else { - dev->vblank[crtc].enabled = true; + vblank->enabled = true; drm_update_vblank_count(dev, crtc); } } @@ -926,16 +938,17 @@ static int drm_vblank_enable(struct drm_device *dev, int crtc) */ int drm_vblank_get(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[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[crtc].refcount) == 1) { + if (atomic_add_return(1, &vblank->refcount) == 1) { ret = drm_vblank_enable(dev, crtc); } else { - if (!dev->vblank[crtc].enabled) { - atomic_dec(&dev->vblank[crtc].refcount); + if (!vblank->enabled) { + atomic_dec(&vblank->refcount); ret = -EINVAL; } } @@ -975,12 +988,14 @@ EXPORT_SYMBOL(drm_crtc_vblank_get); */ void drm_vblank_put(struct drm_device *dev, int crtc) { - BUG_ON(atomic_read(&dev->vblank[crtc].refcount) == 0); + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; + + BUG_ON(atomic_read(&vblank->refcount) == 0); /* Last user schedules interrupt disable */ - if (atomic_dec_and_test(&dev->vblank[crtc].refcount) && + if (atomic_dec_and_test(&vblank->refcount) && (drm_vblank_offdelay > 0)) - mod_timer(&dev->vblank[crtc].disable_timer, + mod_timer(&vblank->disable_timer, jiffies + ((drm_vblank_offdelay * HZ)/1000)); } EXPORT_SYMBOL(drm_vblank_put); @@ -1016,6 +1031,7 @@ EXPORT_SYMBOL(drm_crtc_vblank_put); */ void drm_vblank_off(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; struct drm_pending_vblank_event *e, *t; struct timeval now; unsigned long irqflags; @@ -1023,7 +1039,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc) spin_lock_irqsave(&dev->vbl_lock, irqflags); vblank_disable_and_save(dev, crtc); - wake_up(&dev->vblank[crtc].queue); + wake_up(&vblank->queue); /* Send any queued vblank events, lest the natives grow disquiet */ seq = drm_vblank_count_and_time(dev, crtc, &now); @@ -1045,9 +1061,9 @@ void drm_vblank_off(struct drm_device *dev, int crtc) * Prevent subsequent drm_vblank_get() from re-enabling * the vblank interrupt by bumping the refcount. */ - if (!dev->vblank[crtc].inmodeset) { - atomic_inc(&dev->vblank[crtc].refcount); - dev->vblank[crtc].inmodeset = 1; + if (!vblank->inmodeset) { + atomic_inc(&vblank->refcount); + vblank->inmodeset = 1; } spin_unlock_irqrestore(&dev->vbl_lock, irqflags); @@ -1087,13 +1103,14 @@ EXPORT_SYMBOL(drm_crtc_vblank_off); */ void drm_vblank_on(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; unsigned long irqflags; spin_lock_irqsave(&dev->vbl_lock, irqflags); /* Drop our private "prevent drm_vblank_get" refcount */ - if (dev->vblank[crtc].inmodeset) { - atomic_dec(&dev->vblank[crtc].refcount); - dev->vblank[crtc].inmodeset = 0; + if (vblank->inmodeset) { + atomic_dec(&vblank->refcount); + vblank->inmodeset = 0; } /* @@ -1103,12 +1120,12 @@ void drm_vblank_on(struct drm_device *dev, int crtc) * -1 to make sure user will never see the same * vblank counter value before and after a modeset */ - dev->vblank[crtc].last = + vblank->last = (dev->driver->get_vblank_counter(dev, crtc) - 1) & dev->max_vblank_count; /* re-enable interrupts if there's are users left */ - if (atomic_read(&dev->vblank[crtc].refcount) != 0) + if (atomic_read(&vblank->refcount) != 0) WARN_ON(drm_vblank_enable(dev, crtc)); spin_unlock_irqrestore(&dev->vbl_lock, irqflags); } @@ -1156,6 +1173,8 @@ EXPORT_SYMBOL(drm_crtc_vblank_on); */ void drm_vblank_pre_modeset(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; + /* vblank is not initialized (IRQ not installed ?), or has been freed */ if (!dev->num_crtcs) return; @@ -1166,10 +1185,10 @@ void drm_vblank_pre_modeset(struct drm_device *dev, int crtc) * to avoid corrupting the count if multiple, mismatch calls occur), * so that interrupts remain enabled in the interim. */ - if (!dev->vblank[crtc].inmodeset) { - dev->vblank[crtc].inmodeset = 0x1; + if (!vblank->inmodeset) { + vblank->inmodeset = 0x1; if (drm_vblank_get(dev, crtc) == 0) - dev->vblank[crtc].inmodeset |= 0x2; + vblank->inmodeset |= 0x2; } } EXPORT_SYMBOL(drm_vblank_pre_modeset); @@ -1184,21 +1203,22 @@ EXPORT_SYMBOL(drm_vblank_pre_modeset); */ void drm_vblank_post_modeset(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; unsigned long irqflags; /* vblank is not initialized (IRQ not installed ?), or has been freed */ if (!dev->num_crtcs) return; - if (dev->vblank[crtc].inmodeset) { + if (vblank->inmodeset) { spin_lock_irqsave(&dev->vbl_lock, irqflags); dev->vblank_disable_allowed = true; spin_unlock_irqrestore(&dev->vbl_lock, irqflags); - if (dev->vblank[crtc].inmodeset & 0x2) + if (vblank->inmodeset & 0x2) drm_vblank_put(dev, crtc); - dev->vblank[crtc].inmodeset = 0; + vblank->inmodeset = 0; } } EXPORT_SYMBOL(drm_vblank_post_modeset); @@ -1333,6 +1353,7 @@ err_put: int drm_wait_vblank(struct drm_device *dev, void *data, struct drm_file *file_priv) { + struct drm_vblank_crtc *vblank; union drm_wait_vblank *vblwait = data; int ret; unsigned int flags, seq, crtc, high_crtc; @@ -1362,6 +1383,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data, if (crtc >= dev->num_crtcs) return -EINVAL; + vblank = &dev->vblank[crtc]; + ret = drm_vblank_get(dev, crtc); if (ret) { DRM_DEBUG("failed to acquire vblank counter, %d\n", ret); @@ -1394,11 +1417,11 @@ int drm_wait_vblank(struct drm_device *dev, void *data, DRM_DEBUG("waiting on vblank count %d, crtc %d\n", vblwait->request.sequence, crtc); - dev->vblank[crtc].last_wait = vblwait->request.sequence; - DRM_WAIT_ON(ret, dev->vblank[crtc].queue, 3 * HZ, + vblank->last_wait = vblwait->request.sequence; + DRM_WAIT_ON(ret, vblank->queue, 3 * HZ, (((drm_vblank_count(dev, crtc) - vblwait->request.sequence) <= (1 << 23)) || - !dev->vblank[crtc].enabled || + !vblank->enabled || !dev->irq_enabled)); if (ret != -EINTR) { @@ -1459,6 +1482,7 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) */ bool drm_handle_vblank(struct drm_device *dev, int crtc) { + struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; u32 vblcount; s64 diff_ns; struct timeval tvblank; @@ -1474,7 +1498,7 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) spin_lock_irqsave(&dev->vblank_time_lock, irqflags); /* Vblank irq handling disabled. Nothing to do. */ - if (!dev->vblank[crtc].enabled) { + if (!vblank->enabled) { spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); return false; } @@ -1484,7 +1508,7 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) */ /* Get current timestamp and count. */ - vblcount = atomic_read(&dev->vblank[crtc].count); + vblcount = atomic_read(&vblank->count); drm_get_last_vbltimestamp(dev, crtc, &tvblank, DRM_CALLED_FROM_VBLIRQ); /* Compute time difference to timestamp of last vblank */ @@ -1508,14 +1532,14 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) * the timestamp computed above. */ smp_mb__before_atomic(); - atomic_inc(&dev->vblank[crtc].count); + atomic_inc(&vblank->count); smp_mb__after_atomic(); } else { DRM_DEBUG("crtc %d: Redundant vblirq ignored. diff_ns = %d\n", crtc, (int) diff_ns); } - wake_up(&dev->vblank[crtc].queue); + wake_up(&vblank->queue); drm_handle_vblank_events(dev, crtc); spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); -- cgit v1.2.2 From 56cc279b29c7b204fe7d0943509ae209b8b128db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:51 +0300 Subject: drm: Fix deadlock between event_lock and vbl_lock/vblank_time_lock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently both drm_irq.c and several drivers call drm_vblank_put() while holding event_lock. Now that drm_vblank_put() can disable the vblank interrupt directly it may need to grab vbl_lock and vblank_time_lock. That causes deadlocks since we take the locks in the opposite order in two places in drm_irq.c. So let's make sure the locking order is always event_lock->vbl_lock->vblank_time_lock. In drm_vblank_off() pull up event_lock from underneath vbl_lock. Hold the event_lock across the whole operation to make sure we only send out the events that were on the queue when we disabled the interrupt, and not ones that got added just after (assuming drm_vblank_on() already managed to get called somewhere between). To sort the other deadlock pull the event_lock out from drm_handle_vblank_events() into drm_handle_vblank() to be taken outside vblank_time_lock. Add the appropriate assert_spin_locked() to drm_handle_vblank_events(). Reviewed-by: Matt Roper Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index b4460bf0e0e4..9353609c6770 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1037,14 +1037,25 @@ void drm_vblank_off(struct drm_device *dev, int crtc) unsigned long irqflags; unsigned int seq; - spin_lock_irqsave(&dev->vbl_lock, irqflags); + spin_lock_irqsave(&dev->event_lock, irqflags); + + spin_lock(&dev->vbl_lock); vblank_disable_and_save(dev, crtc); wake_up(&vblank->queue); + /* + * Prevent subsequent drm_vblank_get() from re-enabling + * the vblank interrupt by bumping the refcount. + */ + if (!vblank->inmodeset) { + atomic_inc(&vblank->refcount); + vblank->inmodeset = 1; + } + spin_unlock(&dev->vbl_lock); + /* Send any queued vblank events, lest the natives grow disquiet */ seq = drm_vblank_count_and_time(dev, crtc, &now); - spin_lock(&dev->event_lock); list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) { if (e->pipe != crtc) continue; @@ -1055,18 +1066,7 @@ void drm_vblank_off(struct drm_device *dev, int crtc) drm_vblank_put(dev, e->pipe); send_vblank_event(dev, e, seq, &now); } - spin_unlock(&dev->event_lock); - - /* - * Prevent subsequent drm_vblank_get() from re-enabling - * the vblank interrupt by bumping the refcount. - */ - if (!vblank->inmodeset) { - atomic_inc(&vblank->refcount); - vblank->inmodeset = 1; - } - - spin_unlock_irqrestore(&dev->vbl_lock, irqflags); + spin_unlock_irqrestore(&dev->event_lock, irqflags); } EXPORT_SYMBOL(drm_vblank_off); @@ -1446,12 +1446,11 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) { struct drm_pending_vblank_event *e, *t; struct timeval now; - unsigned long flags; unsigned int seq; - seq = drm_vblank_count_and_time(dev, crtc, &now); + assert_spin_locked(&dev->event_lock); - spin_lock_irqsave(&dev->event_lock, flags); + seq = drm_vblank_count_and_time(dev, crtc, &now); list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) { if (e->pipe != crtc) @@ -1467,8 +1466,6 @@ static void drm_handle_vblank_events(struct drm_device *dev, int crtc) send_vblank_event(dev, e, seq, &now); } - spin_unlock_irqrestore(&dev->event_lock, flags); - trace_drm_vblank_event(crtc, seq); } @@ -1491,15 +1488,18 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) if (!dev->num_crtcs) return false; + spin_lock_irqsave(&dev->event_lock, irqflags); + /* Need timestamp lock to prevent concurrent execution with * vblank enable/disable, as this would cause inconsistent * or corrupted timestamps and vblank counts. */ - spin_lock_irqsave(&dev->vblank_time_lock, irqflags); + spin_lock(&dev->vblank_time_lock); /* Vblank irq handling disabled. Nothing to do. */ if (!vblank->enabled) { - spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); + spin_unlock(&dev->vblank_time_lock); + spin_unlock_irqrestore(&dev->event_lock, irqflags); return false; } @@ -1539,10 +1539,13 @@ bool drm_handle_vblank(struct drm_device *dev, int crtc) crtc, (int) diff_ns); } + spin_unlock(&dev->vblank_time_lock); + wake_up(&vblank->queue); drm_handle_vblank_events(dev, crtc); - spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); + spin_unlock_irqrestore(&dev->event_lock, irqflags); + return true; } EXPORT_SYMBOL(drm_handle_vblank); -- cgit v1.2.2 From ffe7c73a8d4f0caeebd5d220ddbf7126a4daca1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:52 +0300 Subject: drm: Fix race between drm_vblank_off() and drm_queue_vblank_event() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently it's possible that the following will happen: 1. drm_wait_vblank() calls drm_vblank_get() 2. drm_vblank_off() gets called 3. drm_wait_vblank() calls drm_queue_vblank_event() which adds the event to the queue event though vblank interrupts are currently disabled (and may not be re-enabled ever again). To fix the problem, add another vblank->enabled check into drm_queue_vblank_event(). drm_vblank_off() holds event_lock around the vblank disable, so no further locking needs to be added to drm_queue_vblank_event(). vblank disable from another source is not possible since drm_wait_vblank() already holds a vblank reference. Reviewed-by: Matt Roper Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 9353609c6770..b2428cb0c64d 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1270,6 +1270,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe, union drm_wait_vblank *vblwait, struct drm_file *file_priv) { + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; struct drm_pending_vblank_event *e; struct timeval now; unsigned long flags; @@ -1293,6 +1294,18 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe, spin_lock_irqsave(&dev->event_lock, flags); + /* + * drm_vblank_off() might have been called after we called + * drm_vblank_get(). drm_vblank_off() holds event_lock + * around the vblank disable, so no need for further locking. + * The reference from drm_vblank_get() protects against + * vblank disable from another source. + */ + if (!vblank->enabled) { + ret = -EINVAL; + goto err_unlock; + } + if (file_priv->event_space < sizeof e->event) { ret = -EBUSY; goto err_unlock; -- cgit v1.2.2 From 4ed0ce3d0bccd74416ba6beb33a8a79d1617e97b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:53 +0300 Subject: drm: Disable vblank interrupt immediately when drm_vblank_offdelay<0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make drm_vblank_put() disable the vblank interrupt immediately when the refcount drops to zero and drm_vblank_offdelay<0. v2: Preserve the current drm_vblank_offdelay==0 'never disable' behaviur Reviewed-by: Matt Roper Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index b2428cb0c64d..99145c4d536b 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -993,10 +993,13 @@ void drm_vblank_put(struct drm_device *dev, int crtc) BUG_ON(atomic_read(&vblank->refcount) == 0); /* Last user schedules interrupt disable */ - if (atomic_dec_and_test(&vblank->refcount) && - (drm_vblank_offdelay > 0)) - mod_timer(&vblank->disable_timer, - jiffies + ((drm_vblank_offdelay * HZ)/1000)); + if (atomic_dec_and_test(&vblank->refcount)) { + if (drm_vblank_offdelay < 0) + vblank_disable_fn((unsigned long)vblank); + else if (drm_vblank_offdelay > 0) + mod_timer(&vblank->disable_timer, + jiffies + ((drm_vblank_offdelay * HZ)/1000)); + } } EXPORT_SYMBOL(drm_vblank_put); -- cgit v1.2.2 From 00185e667009dda907887a4f84fbd02c6e651a49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:54 +0300 Subject: drm: Add dev->vblank_disable_immediate flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a flag to drm_device which will cause the vblank code to bypass the disable timer and always disable the vblank interrupt immediately when the last reference is dropped. v2: Add some notes about the flag to the kernel doc Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 99145c4d536b..8dbcc3f892d5 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -994,7 +994,7 @@ void drm_vblank_put(struct drm_device *dev, int crtc) /* Last user schedules interrupt disable */ if (atomic_dec_and_test(&vblank->refcount)) { - if (drm_vblank_offdelay < 0) + if (dev->vblank_disable_immediate || drm_vblank_offdelay < 0) vblank_disable_fn((unsigned long)vblank); else if (drm_vblank_offdelay > 0) mod_timer(&vblank->disable_timer, -- cgit v1.2.2 From cd19e52aee922ffe5c50b6ed67acd58cc1b2738b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:56 +0300 Subject: drm: Kick start vblank interrupts at drm_vblank_on() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the user is interested in getting accurate vblank sequence numbers all the time they may disable the vblank disable timer entirely. In that case it seems appropriate to kick start the vblank interrupts already from drm_vblank_on(). v2: Adapt to the drm_vblank_offdelay ==0 vs <0 changes Reviewed-by: Matt Roper Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 8dbcc3f892d5..af33df1adc6d 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1126,9 +1126,12 @@ void drm_vblank_on(struct drm_device *dev, int crtc) vblank->last = (dev->driver->get_vblank_counter(dev, crtc) - 1) & dev->max_vblank_count; - - /* re-enable interrupts if there's are users left */ - if (atomic_read(&vblank->refcount) != 0) + /* + * re-enable interrupts if there are users left, or the + * user wishes vblank interrupts to be enabled all the time. + */ + if (atomic_read(&vblank->refcount) != 0 || + (!dev->vblank_disable_immediate && drm_vblank_offdelay == 0)) WARN_ON(drm_vblank_enable(dev, crtc)); spin_unlock_irqrestore(&dev->vbl_lock, irqflags); } -- cgit v1.2.2 From 96a9fdd778037799f63c9ae272ec915dd3ad83dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:50:02 +0300 Subject: drm: Fix confusing debug message in drm_update_vblank_count() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that drm_update_vblank_count() can be called even when we're not about to enable the vblank interrupts we shouldn't print debug messages stating otherwise. Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index af33df1adc6d..62dee812d28a 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -103,7 +103,7 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) crtc, vblank->last, cur_vblank, diff); } - DRM_DEBUG("enabling vblank interrupts on crtc %d, missed %d\n", + DRM_DEBUG("updating vblank count on crtc %d, missed %d\n", crtc, diff); /* Reinitialize corresponding vblank timestamp if high-precision query -- cgit v1.2.2 From c50d7521617d823d769b280bc499e19e364434ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 6 Aug 2014 14:49:59 +0300 Subject: drm: Store the vblank timestamp when adjusting the counter during disable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During vblank disable the code tries to guess based on the timestamps whether we just missed one vblank or not. And if so it increments the counter. However it forgets to store the new timestamp to the approriate slot in our timestamp ring buffer. So anyone querying the timestamp for the resulting sequence number would get a stale timestamp. Fix it up by storing the new timestamp. Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 62dee812d28a..aa9b06495067 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -198,6 +198,13 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * hope for the best. */ if ((vblrc > 0) && (abs64(diff_ns) > 1000000)) { + /* Store new timestamp in ringbuffer. */ + vblanktimestamp(dev, crtc, vblcount + 1) = tvblank; + + /* Increment cooked vblank count. This also atomically commits + * the timestamp computed above. + */ + smp_mb__before_atomic(); atomic_inc(&vblank->count); smp_mb__after_atomic(); } -- cgit v1.2.2 From 79a093aea44f11fda0a5b4dbe4c1e29b2f586f4e Mon Sep 17 00:00:00 2001 From: Mario Kleiner Date: Wed, 6 Aug 2014 03:22:44 +0200 Subject: drm: Remove drm_vblank_cleanup from drm_vblank_init error path. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drm_vblank_cleanup() would operate on non-existent dev->vblank data structure, as failure to allocate that data structure is what triggers the error path in the first place. Signed-off-by: Mario Kleiner Reviewed-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index aa9b06495067..6473089e5fd3 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -303,7 +303,7 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs) return 0; err: - drm_vblank_cleanup(dev); + dev->num_crtcs = 0; return ret; } EXPORT_SYMBOL(drm_vblank_init); -- cgit v1.2.2 From e8450f51a4b39cfe0878b4aee339820b2bfff240 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 25 Jul 2014 23:34:03 +0200 Subject: drm/irq: Implement a generic vblank_wait function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As usual in both a crtc index and a struct drm_crtc * version. The function assumes that no one drivers their display below 10Hz, and it will complain if the vblank wait takes longer than that. v2: Also check dev->max_vblank_counter since some drivers register a fake get_vblank_counter function. v3: Use drm_vblank_count instead of calling the low-level ->get_vblank_counter callback. That way we'll get the sw-cooked counter for platforms without proper vblank support and so can ditch the max_vblank_counter check again. v4: Review from Michel Dänzer: - Restore lost notes about v3: - Spelling in kerneldoc. - Inline wait_event condition. - s/vblank_wait/wait_one_vblank/ Cc: Michel Dänzer Cc: Ville Syrjälä Reviewed-by: Michel Dänzer Reviewed-by: Matt Roper Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 6f16a104d6d0..e64d24951fc2 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1009,6 +1009,50 @@ void drm_crtc_vblank_put(struct drm_crtc *crtc) } EXPORT_SYMBOL(drm_crtc_vblank_put); +/** + * drm_wait_one_vblank - wait for one vblank + * @dev: DRM device + * @crtc: crtc index + * + * This waits for one vblank to pass on @crtc, using the irq driver interfaces. + * It is a failure to call this when the vblank irq for @crtc is disabled, e.g. + * due to lack of driver support or because the crtc is off. + */ +void drm_wait_one_vblank(struct drm_device *dev, int crtc) +{ + int ret; + u32 last; + + ret = drm_vblank_get(dev, crtc); + if (WARN_ON(ret)) + return; + + last = drm_vblank_count(dev, crtc); + + ret = wait_event_timeout(dev->vblank[crtc].queue, + last != drm_vblank_count(dev, crtc), + msecs_to_jiffies(100)); + + WARN_ON(ret == 0); + + drm_vblank_put(dev, crtc); +} +EXPORT_SYMBOL(drm_wait_one_vblank); + +/** + * drm_crtc_wait_one_vblank - wait for one vblank + * @crtc: DRM crtc + * + * This waits for one vblank to pass on @crtc, using the irq driver interfaces. + * It is a failure to call this when the vblank irq for @crtc is disabled, e.g. + * due to lack of driver support or because the crtc is off. + */ +void drm_crtc_wait_one_vblank(struct drm_crtc *crtc) +{ + drm_wait_one_vblank(crtc->dev, drm_crtc_index(crtc)); +} +EXPORT_SYMBOL(drm_crtc_wait_one_vblank); + /** * drm_vblank_off - disable vblank events on a CRTC * @dev: DRM device -- cgit v1.2.2 From 2368ffb18b1d2b04eb80478d225676caa7a3c4c8 Mon Sep 17 00:00:00 2001 From: Mario Kleiner Date: Wed, 6 Aug 2014 03:22:46 +0200 Subject: drm: Use vblank_disable_and_save in drm_vblank_cleanup() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calling vblank_disable_fn() will cause that function to no-op if !dev->vblank_disable_allowed for some kms drivers, e.g., on nouveau-kms. This can cause the gpu vblank irq's to not get disabled before freeing the dev->vblank array, so if a vblank irq fires and calls into drm_handle_vblank() after drm_vblank_cleanup() completes, it will cause use-after-free access to dev->vblank array. Call vblank_disable_and_save unconditionally, so vblank irqs are guaranteed to be off, before we delete the data structures on which they operate. Signed-off-by: Mario Kleiner Reviewed-by: Ville Syrjälä [danvet: Fix subsystem name in patch subject.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 6473089e5fd3..9be760145cb7 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -239,6 +239,7 @@ static void vblank_disable_fn(unsigned long arg) void drm_vblank_cleanup(struct drm_device *dev) { int crtc; + unsigned long irqflags; /* Bail if the driver didn't call drm_vblank_init() */ if (dev->num_crtcs == 0) @@ -248,7 +249,10 @@ void drm_vblank_cleanup(struct drm_device *dev) struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; del_timer_sync(&vblank->disable_timer); - vblank_disable_fn((unsigned long)vblank); + + spin_lock_irqsave(&dev->vbl_lock, irqflags); + vblank_disable_and_save(dev, crtc); + spin_unlock_irqrestore(&dev->vbl_lock, irqflags); } kfree(dev->vblank); -- cgit v1.2.2 From ab8905f1c6a74d695c6096791ec4b349bc985b8a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 10 Sep 2014 17:36:08 +0200 Subject: drm: Really never disable vblank irqs for offdelay==0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the new support for immediate vblank disabling we always disabled the vblank interrupt right away, irrespective of the vblank offdelay setting. But being able to let vblanks run forever is fairly useful for debugging, so restore that behaviour. Suggested-by: Mario Kleiner Cc: Mario Kleiner Cc: Matt Roper Cc: Ville Syrjälä Reviewed-by: Matt Roper Reviewed-by: Mario Kleiner Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 9be760145cb7..87d148cec877 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1005,9 +1005,11 @@ void drm_vblank_put(struct drm_device *dev, int crtc) /* Last user schedules interrupt disable */ if (atomic_dec_and_test(&vblank->refcount)) { - if (dev->vblank_disable_immediate || drm_vblank_offdelay < 0) + if (drm_vblank_offdelay == 0) + return; + else if (dev->vblank_disable_immediate || drm_vblank_offdelay < 0) vblank_disable_fn((unsigned long)vblank); - else if (drm_vblank_offdelay > 0) + else mod_timer(&vblank->disable_timer, jiffies + ((drm_vblank_offdelay * HZ)/1000)); } -- cgit v1.2.2 From 855d30b402b91f09c90f65c34ec91debaae8cf3a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 10 Sep 2014 17:36:09 +0200 Subject: drm: Only update final vblank count when precise ts is available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drivers without a hardware vblank counter simply can't account for the vblanks that happened while the vblank interrupt was off. To check this grab a vblank timestamp and if the result is dubious follow the normal save-and-disable logic. Drivers should prevent this by setting vblank_disable_allowed = false, but since running vblank interrupts constantly is not good for power consumption most drivers lie. Testing for precise vblank timestamps is the next best thing we can check for. Suggested-by: Mario Kleiner Cc: Mario Kleiner Cc: Matt Roper Cc: Ville Syrjälä Reviewed-by: Mario Kleiner Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 87d148cec877..ad699b729278 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -148,8 +148,15 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * has been ticking all along until this time. This makes the * count account for the entire time between drm_vblank_on() and * drm_vblank_off(). + * + * But only do this if precise vblank timestamps are available. + * Otherwise we might read a totally bogus timestamp since drivers + * lacking precise timestamp support rely upon sampling the system clock + * at vblank interrupt time. Which obviously won't work out well if the + * vblank interrupt is disabled. */ - if (!vblank->enabled) { + if (!vblank->enabled && + drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0) > 0) { drm_update_vblank_count(dev, crtc); spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); return; -- cgit v1.2.2 From fb446a1acdb981921de06bfde3a2178da7174481 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 10 Sep 2014 17:36:10 +0200 Subject: drm: Simplify return value of drm_get_last_vbltimestamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Imo u32 hints at a register value, but in reality all callers only care whether the sampled timestamp is precise or not. So give them just a bool. Also move the declaration out of drmP.h, it's only used in drm_irq.c. v2: Also drop the EXPORT_SYMBOL, spotted by Mario. Cc: Mario Kleiner Cc: Ville Syrjälä Reviewed-by: Mario Kleiner Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index ad699b729278..15f748c7fa7e 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -55,6 +55,10 @@ */ #define DRM_REDUNDANT_VBLIRQ_THRESH_NS 1000000 +static bool +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, + struct timeval *tvblank, unsigned flags); + /** * drm_update_vblank_count - update the master vblank counter * @dev: DRM device @@ -74,7 +78,8 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) { struct drm_vblank_crtc *vblank = &dev->vblank[crtc]; - u32 cur_vblank, diff, tslot, rc; + u32 cur_vblank, diff, tslot; + bool rc; struct timeval t_vblank; /* @@ -132,7 +137,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) unsigned long irqflags; u32 vblcount; s64 diff_ns; - int vblrc; + bool vblrc; struct timeval tvblank; int count = DRM_TIMESTAMP_MAXRETRIES; @@ -156,7 +161,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * vblank interrupt is disabled. */ if (!vblank->enabled && - drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0) > 0) { + drm_get_last_vbltimestamp(dev, crtc, &tvblank, 0)) { drm_update_vblank_count(dev, crtc); spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); return; @@ -204,7 +209,7 @@ static void vblank_disable_and_save(struct drm_device *dev, int crtc) * available. In that case we can't account for this and just * hope for the best. */ - if ((vblrc > 0) && (abs64(diff_ns) > 1000000)) { + if (vblrc && (abs64(diff_ns) > 1000000)) { /* Store new timestamp in ringbuffer. */ vblanktimestamp(dev, crtc, vblcount + 1) = tvblank; @@ -781,10 +786,11 @@ static struct timeval get_drm_timestamp(void) * call, i.e., it isn't very precisely locked to the true vblank. * * Returns: - * Non-zero if timestamp is considered to be very precise, zero otherwise. + * True if timestamp is considered to be very precise, false otherwise. */ -u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, - struct timeval *tvblank, unsigned flags) +static bool +drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, + struct timeval *tvblank, unsigned flags) { int ret; @@ -796,7 +802,7 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, ret = dev->driver->get_vblank_timestamp(dev, crtc, &max_error, tvblank, flags); if (ret > 0) - return (u32) ret; + return true; } /* GPU high precision timestamp query unsupported or failed. @@ -804,9 +810,8 @@ u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, */ *tvblank = get_drm_timestamp(); - return 0; + return false; } -EXPORT_SYMBOL(drm_get_last_vbltimestamp); /** * drm_vblank_count - retrieve "cooked" vblank counter value -- cgit v1.2.2 From 3d3cbd84300e7be1e53083cac0f6f9c12978ecb4 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 10 Sep 2014 17:36:11 +0200 Subject: drm: Clarify vblank ts/scanoutpos sampling #defines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I've read INVBL as "invalid backlight" and got mightly confused. The #defines are already fairly long and we can afford to extend them a bit more without resulting in ugly code all over. I'm not sure how useful the complicated bitmask return value of these functions really are since no one checks them. But for now let's keep things as is. Cc: Mario Kleiner Cc: Ville Syrjälä Reviewed-by: Mario Kleiner Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 15f748c7fa7e..79836594030c 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -721,7 +721,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, * within vblank area, counting down the number of lines until * start of scanout. */ - invbl = vbl_status & DRM_SCANOUTPOS_INVBL; + invbl = vbl_status & DRM_SCANOUTPOS_IN_VBLANK; /* Convert scanout position into elapsed time at raw_time query * since start of scanout at first display scanline. delta_ns @@ -751,7 +751,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, vbl_status = DRM_VBLANKTIME_SCANOUTPOS_METHOD; if (invbl) - vbl_status |= DRM_VBLANKTIME_INVBL; + vbl_status |= DRM_VBLANKTIME_IN_VBLANK; return vbl_status; } -- cgit v1.2.2 From 18882995713d2ebdd24d6b07f1853a866a7e1b66 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 10 Sep 2014 08:16:10 +0200 Subject: drm: Move vblank related module options into drm_irq.c This allows us to drop 2 header declarations from drmP.h. The 3rd one is also used in drm_ioctl.c, so for that create a new drm_internal.h header for non-legacy non-kms (since we have internal headers for those parts already) declarations private to drm.ko. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 034297640b48..80ff94ada75e 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -34,6 +34,7 @@ #include #include "drm_trace.h" +#include "drm_internal.h" #include /* For task queue support */ #include @@ -59,6 +60,20 @@ static bool drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, struct timeval *tvblank, unsigned flags); +static unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ + +/* + * Default to use monotonic timestamps for wait-for-vblank and page-flip + * complete events. + */ +unsigned int drm_timestamp_monotonic = 1; + +static int drm_vblank_offdelay = 5000; /* Default to 5000 msecs. */ + +module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600); +module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600); +module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); + /** * drm_update_vblank_count - update the master vblank counter * @dev: DRM device -- cgit v1.2.2 From da8f43962bd323813f7215b00b5da48ad766b9b2 Mon Sep 17 00:00:00 2001 From: Mario Kleiner Date: Sat, 13 Sep 2014 18:25:54 +0200 Subject: drm: Don't update vblank timestamp when the counter didn't change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we already have a timestamp for the current vblank counter, don't update it with a new timestmap. Small errors can creep in between two timestamp queries for the same vblank count, which could be confusing to userspace when it queries the timestamp for the same vblank sequence number twice. This problem gets exposed when the vblank disable timer is not used (or is set to expire quickly) and thus we can get multiple vblank disable<->enable transition during the same frame which would all attempt to update the timestamp with the latest estimate. Testcase: igt/kms_flip/flip-vs-expired-vblank Signed-off-by: Ville Syrjälä Reviewed-by: Mario Kleiner v2:Mario: Trivial rebase on top of current drm-next (13-Sep-2014) Signed-off-by: Mario Kleiner Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 80ff94ada75e..e73cbdaa18df 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -126,6 +126,9 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc) DRM_DEBUG("updating vblank count on crtc %d, missed %d\n", crtc, diff); + if (diff == 0) + return; + /* Reinitialize corresponding vblank timestamp if high-precision query * available. Skip this step if query unsupported or failed. Will * reinitialize delayed at next vblank interrupt in that case. -- cgit v1.2.2 From 80c873b6b7b4de56d0771e3834ea71467937e506 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 15 Sep 2014 14:04:28 +0200 Subject: drm: Improve debug output for drm_wait_one_vblank This replicates what we've done in i915 in commit 31e4b89acbd7b19c9a8557e6e660a583a0b97daa Author: Damien Lespiau Date: Mon Aug 18 13:51:00 2014 +0100 drm/i915: Print the pipe on which the vblank wait times out to make sure that when we switch i915 to drm_wait_one_vblank that the debug output doesn't regress. Cc: Damien Lespiau Cc: Thomas Wood Reviewed-by: Damien Lespiau Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_irq.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/drm_irq.c') diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index e73cbdaa18df..5ef03c216a27 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1077,7 +1077,7 @@ void drm_wait_one_vblank(struct drm_device *dev, int crtc) u32 last; ret = drm_vblank_get(dev, crtc); - if (WARN_ON(ret)) + if (WARN(ret, "vblank not available on crtc %i, ret=%i\n", crtc, ret)) return; last = drm_vblank_count(dev, crtc); @@ -1086,7 +1086,7 @@ void drm_wait_one_vblank(struct drm_device *dev, int crtc) last != drm_vblank_count(dev, crtc), msecs_to_jiffies(100)); - WARN_ON(ret == 0); + WARN(ret == 0, "vblank wait timed out on crtc %i\n", crtc); drm_vblank_put(dev, crtc); } -- cgit v1.2.2