From 7ffd7a68511c710b84db3548a1997fd2625f580a Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From 08c71e5e817a956389af5da5e99ab3e26d5c673d Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 6 Aug 2014 14:49:45 +0300 Subject: drm/i915: Warn if drm_vblank_get() still works after drm_vblank_off() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v2: Drop the drm_vblank_off() (Daniel) Use drm_crtc_vblank_{get,put}() Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 99eb7cad62a8..8f6b932d8e79 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1341,6 +1341,12 @@ static void assert_sprites_disabled(struct drm_i915_private *dev_priv, } } +static void assert_vblank_disabled(struct drm_crtc *crtc) +{ + if (WARN_ON(drm_crtc_vblank_get(crtc) == 0)) + drm_crtc_vblank_put(crtc); +} + static void ibx_assert_pch_refclk_enabled(struct drm_i915_private *dev_priv) { u32 val; @@ -3905,6 +3911,8 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc) int pipe = intel_crtc->pipe; int plane = intel_crtc->plane; + assert_vblank_disabled(crtc); + drm_vblank_on(dev, pipe); intel_enable_primary_hw_plane(dev_priv, plane, pipe); @@ -3954,6 +3962,8 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc) intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe)); drm_vblank_off(dev, pipe); + + assert_vblank_disabled(crtc); } static void ironlake_crtc_enable(struct drm_crtc *crtc) -- cgit v1.2.3 From 844b03f27739135fe1fed2fef06da0ffc4c7a081 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From 13b030af54a5e307cbcccdf5479873fbc4b7f185 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From 812e7465a7decf3cca0b5f71977a25eecd9626a4 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From f8ad028cc033f75fc479ca1c30e2ea4ba56e5269 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From 8a51d5bef07f1c8c59de20089fb27ea39d395f1b Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From 56cc279b29c7b204fe7d0943509ae209b8b128db Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From ffe7c73a8d4f0caeebd5d220ddbf7126a4daca1f Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From 4ed0ce3d0bccd74416ba6beb33a8a79d1617e97b Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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 --- Documentation/DocBook/drm.tmpl | 1 + drivers/gpu/drm/drm_drv.c | 4 ++-- drivers/gpu/drm/drm_irq.c | 11 +++++++---- include/drm/drmP.h | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 1d3756d3176c..55923d00bd52 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3386,6 +3386,7 @@ void (*disable_vblank) (struct drm_device *dev, int crtc); by scheduling a timer. The delay is accessible through the vblankoffdelay module parameter or the drm_vblank_offdelay global variable and expressed in milliseconds. Its default value is 5000 ms. + Zero means never disable, and a negative value means disable immediately. When a vertical blanking interrupt occurs drivers only need to call the diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 92bc6b1d9646..db03e16ca817 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -39,7 +39,7 @@ unsigned int drm_debug = 0; /* 1 to enable debug output */ EXPORT_SYMBOL(drm_debug); -unsigned int drm_vblank_offdelay = 5000; /* Default to 5000 msecs. */ +int drm_vblank_offdelay = 5000; /* Default to 5000 msecs. */ unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */ @@ -53,7 +53,7 @@ MODULE_AUTHOR(CORE_AUTHOR); MODULE_DESCRIPTION(CORE_DESC); MODULE_LICENSE("GPL and additional rights"); MODULE_PARM_DESC(debug, "Enable debug output"); -MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]"); +MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs] (0: never disable, <0: disable immediately)"); MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]"); MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic timestamps"); 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); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index a57646382086..24b32d453c60 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1345,7 +1345,7 @@ extern void drm_put_dev(struct drm_device *dev); extern void drm_unplug_dev(struct drm_device *dev); extern unsigned int drm_debug; -extern unsigned int drm_vblank_offdelay; +extern int drm_vblank_offdelay; extern unsigned int drm_timestamp_precision; extern unsigned int drm_timestamp_monotonic; -- cgit v1.2.3 From 00185e667009dda907887a4f84fbd02c6e651a49 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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 --- Documentation/DocBook/drm.tmpl | 6 ++++++ drivers/gpu/drm/drm_irq.c | 2 +- include/drm/drmP.h | 10 ++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 55923d00bd52..583edbffff1a 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3387,6 +3387,12 @@ void (*disable_vblank) (struct drm_device *dev, int crtc); module parameter or the drm_vblank_offdelay global variable and expressed in milliseconds. Its default value is 5000 ms. Zero means never disable, and a negative value means disable immediately. + Drivers may override the behaviour by setting the + drm_device + vblank_disable_immediate flag, which when set + causes vblank interrupts to be disabled immediately regardless of the + drm_vblank_offdelay value. The flag should only be set if there's a + properly working hardware vblank counter present. When a vertical blanking interrupt occurs drivers only need to call the 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, diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 24b32d453c60..17a5c10474bd 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1074,6 +1074,16 @@ struct drm_device { */ bool vblank_disable_allowed; + /* + * If true, vblank interrupt will be disabled immediately when the + * refcount drops to zero, as opposed to via the vblank disable + * timer. + * This can be set to true it the hardware has a working vblank + * counter and the driver uses drm_vblank_on() and drm_vblank_off() + * appropriately. + */ + bool vblank_disable_immediate; + /* array of size num_crtcs */ struct drm_vblank_crtc *vblank; -- cgit v1.2.3 From 21da27005f79d72499bb809616b15fd2c5c15319 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 6 Aug 2014 14:49:55 +0300 Subject: drm/i915: Opt out of vblank disable timer on >gen2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the vblank races are plugged, we can opt out of using the vblank disable timer and just let vblank interrupts get disabled immediately when the last reference is dropped. Gen2 is the exception since it has no hardware frame counter. Reviewed-by: Matt Roper Reviewed-by: Daniel Vetter Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_irq.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6ef9d6fabf80..845f0f6c1eeb 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -4680,6 +4680,14 @@ void intel_irq_init(struct drm_device *dev) dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */ } + /* + * Opt out of the vblank disable timer on everything except gen2. + * Gen2 doesn't have a hardware frame counter and so depends on + * vblank interrupts to produce sane vblank seuquence numbers. + */ + if (!IS_GEN2(dev)) + dev->vblank_disable_immediate = true; + if (drm_core_check_feature(dev, DRIVER_MODESET)) { dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp; dev->driver->get_scanout_position = i915_get_crtc_scanoutpos; -- cgit v1.2.3 From cd19e52aee922ffe5c50b6ed67acd58cc1b2738b Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From d297e1037327884fe9545f434d720fd3e8f18c80 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Wed, 6 Aug 2014 14:50:01 +0300 Subject: drm/i915: Update scanline_offset only for active crtcs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit update_scanline_offset() in intel_sanitize_crtc() was supposed to be called only for active crtcs. But due to some underrun patches it now gets updated for all crtcs on gmch platforms. Move the update_scanline_offset() to the very beginning of intel_sanitize_crtc() where we update the vblank state. This seems like a better place anyway since the scanline offset ought to be up to date before we might need to consult it. So before any vblanky stuff happens. Signed-off-by: Ville Syrjälä Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8f6b932d8e79..de40a44e0ca0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12719,9 +12719,10 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK); /* restore vblank interrupts to correct state */ - if (crtc->active) + if (crtc->active) { + update_scanline_offset(crtc); drm_vblank_on(dev, crtc->pipe); - else + } else drm_vblank_off(dev, crtc->pipe); /* We need to sanitize the plane -> pipe mapping first because this will @@ -12820,8 +12821,6 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) */ crtc->cpu_fifo_underrun_disabled = true; crtc->pch_fifo_underrun_disabled = true; - - update_scanline_offset(crtc); } } -- cgit v1.2.3 From 96a9fdd778037799f63c9ae272ec915dd3ad83dc Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 From c50d7521617d823d769b280bc499e19e364434ae Mon Sep 17 00:00:00 2001 From: Ville Syrjälä 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') 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.3 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') 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.3 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') 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.3 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') 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.3 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') 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.3 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 +++++++++++++++---------- include/drm/drmP.h | 2 -- 2 files changed, 15 insertions(+), 12 deletions(-) (limited to 'drivers') 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 diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 17a5c10474bd..a97307525c2d 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -1320,8 +1320,6 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc); extern void drm_crtc_vblank_on(struct drm_crtc *crtc); extern void drm_vblank_cleanup(struct drm_device *dev); -extern u32 drm_get_last_vbltimestamp(struct drm_device *dev, int crtc, - struct timeval *tvblank, unsigned flags); extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, int crtc, int *max_error, struct timeval *vblank_time, -- cgit v1.2.3 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 ++-- drivers/gpu/drm/i915/i915_irq.c | 2 +- drivers/gpu/drm/nouveau/nouveau_display.c | 2 +- drivers/gpu/drm/radeon/radeon_display.c | 2 +- drivers/gpu/drm/radeon/radeon_pm.c | 2 +- include/drm/drmP.h | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers') 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; } diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 845f0f6c1eeb..20dd9e233fc6 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1020,7 +1020,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, /* In vblank? */ if (in_vbl) - ret |= DRM_SCANOUTPOS_INVBL; + ret |= DRM_SCANOUTPOS_IN_VBLANK; return ret; } diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 47ad74255bf1..37a6ab8a97f8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -111,7 +111,7 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, if (etime) *etime = ns_to_ktime(args.time[1]); if (*vpos < 0) - ret |= DRM_SCANOUTPOS_INVBL; + ret |= DRM_SCANOUTPOS_IN_VBLANK; return ret; } diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 3fdf87318069..fd8cd0c3600f 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -1917,7 +1917,7 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int fl /* In vblank? */ if (in_vbl) - ret |= DRM_SCANOUTPOS_INVBL; + ret |= DRM_SCANOUTPOS_IN_VBLANK; /* Is vpos outside nominal vblank area, but less than * 1/100 of a frame height away from start of vblank? diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 23314be49480..7fd665a22908 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -1560,7 +1560,7 @@ static bool radeon_pm_in_vbl(struct radeon_device *rdev) if (rdev->pm.active_crtcs & (1 << crtc)) { vbl_status = radeon_get_crtc_scanoutpos(rdev->ddev, crtc, 0, &vpos, &hpos, NULL, NULL); if ((vbl_status & DRM_SCANOUTPOS_VALID) && - !(vbl_status & DRM_SCANOUTPOS_INVBL)) + !(vbl_status & DRM_SCANOUTPOS_IN_VBLANK)) in_vbl = false; } } diff --git a/include/drm/drmP.h b/include/drm/drmP.h index a97307525c2d..7b87fccff183 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -674,11 +674,11 @@ struct drm_master { /* Flags and return codes for get_vblank_timestamp() driver function. */ #define DRM_CALLED_FROM_VBLIRQ 1 #define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0) -#define DRM_VBLANKTIME_INVBL (1 << 1) +#define DRM_VBLANKTIME_IN_VBLANK (1 << 1) /* get_scanout_position() return flags */ #define DRM_SCANOUTPOS_VALID (1 << 0) -#define DRM_SCANOUTPOS_INVBL (1 << 1) +#define DRM_SCANOUTPOS_IN_VBLANK (1 << 1) #define DRM_SCANOUTPOS_ACCURATE (1 << 2) struct drm_bus { -- cgit v1.2.3