dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock
@ 2017-03-17 20:20 Chris Wilson
  2017-03-17 20:20 ` [PATCH v2 2/4] drm: vblank cannot be enabled if dev->irq_enabled is false Chris Wilson
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chris Wilson @ 2017-03-17 20:20 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, intel-gfx

Order the update to vblank->enabled after the timestamp is primed so
that a concurrent unlocked reader will only see the vblank->enabled with
the current timestamp.

v2: vblank->enable is guarded by dev->vbl_lock not
dev->vblank_time_lock, update the READ_ONCE accordingly.

Do not add a READ_ONCE(vblank->enabled) inside the interrupt handler to
avoid missing an interrupt whilst racing with enable_vblank()

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_irq.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 53a526c7b24d..c47e07c89136 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -325,6 +325,8 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
 	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 	unsigned long irqflags;
 
+	assert_spin_locked(&dev->vbl_lock);
+
 	/* Prevent vblank irq processing while disabling vblank irqs,
 	 * so no updates of timestamps or count can happen after we've
 	 * disabled. Needed to prevent races in case of delayed irq's.
@@ -336,10 +338,8 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
 	 * calling the ->disable_vblank() operation in atomic context with the
 	 * hardware potentially runtime suspended.
 	 */
-	if (vblank->enabled) {
+	if (cmpxchg_relaxed(&vblank->enabled, true, false))
 		__disable_vblank(dev, pipe);
-		vblank->enabled = false;
-	}
 
 	/*
 	 * Always update the count and timestamp to maintain the
@@ -384,7 +384,7 @@ void drm_vblank_cleanup(struct drm_device *dev)
 	for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
 		struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 
-		WARN_ON(vblank->enabled &&
+		WARN_ON(READ_ONCE(vblank->enabled) &&
 			drm_core_check_feature(dev, DRIVER_MODESET));
 
 		del_timer_sync(&vblank->disable_timer);
@@ -1105,11 +1105,16 @@ static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
 		 */
 		ret = __enable_vblank(dev, pipe);
 		DRM_DEBUG("enabling vblank on crtc %u, ret: %d\n", pipe, ret);
-		if (ret)
+		if (ret) {
 			atomic_dec(&vblank->refcount);
-		else {
-			vblank->enabled = true;
+		} else {
 			drm_update_vblank_count(dev, pipe, 0);
+			/* drm_update_vblank_count() includes a wmb so we just
+			 * need to ensure that the compiler emits the write
+			 * to mark the vblank as enabled after the call
+			 * to drm_update_vblank_count().
+			 */
+			WRITE_ONCE(vblank->enabled, true);
 		}
 	}
 
@@ -1517,7 +1522,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 	 * vblank disable, so no need for further locking.  The reference from
 	 * drm_vblank_get() protects against vblank disable from another source.
 	 */
-	if (!vblank->enabled) {
+	if (!READ_ONCE(vblank->enabled)) {
 		ret = -EINVAL;
 		goto err_unlock;
 	}
@@ -1644,7 +1649,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
 		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
 			    (((drm_vblank_count(dev, pipe) -
 			       vblwait->request.sequence) <= (1 << 23)) ||
-			     !vblank->enabled ||
+			     !READ_ONCE(vblank->enabled) ||
 			     !dev->irq_enabled));
 	}
 
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/4] drm: vblank cannot be enabled if dev->irq_enabled is false
  2017-03-17 20:20 [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Chris Wilson
@ 2017-03-17 20:20 ` Chris Wilson
  2017-03-17 20:20 ` [PATCH v2 3/4] drm: Refactor vblank sequence number comparison Chris Wilson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-03-17 20:20 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

Since we cannot enable the vblank if !dev->irq_enabled, we assert that
checking for both !vblank->enabled and !dev->irq_enabled is tautological
and only need the former. The only time it may differ is when racing
with drm_irq_uninstall(), but that will then disable the vblank and
wakeup the waiters.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_irq.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c47e07c89136..a164cf51d093 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1647,10 +1647,9 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
 		DRM_DEBUG("waiting on vblank count %u, crtc %u\n",
 			  vblwait->request.sequence, pipe);
 		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
-			    (((drm_vblank_count(dev, pipe) -
-			       vblwait->request.sequence) <= (1 << 23)) ||
-			     !READ_ONCE(vblank->enabled) ||
-			     !dev->irq_enabled));
+			    (drm_vblank_count(dev, pipe) -
+			     vblwait->request.sequence) <= (1 << 23) ||
+			    !READ_ONCE(vblank->enabled));
 	}
 
 	if (ret != -EINTR) {
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/4] drm: Refactor vblank sequence number comparison
  2017-03-17 20:20 [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Chris Wilson
  2017-03-17 20:20 ` [PATCH v2 2/4] drm: vblank cannot be enabled if dev->irq_enabled is false Chris Wilson
@ 2017-03-17 20:20 ` Chris Wilson
  2017-03-22  8:54   ` Michel Dänzer
  2017-03-22 10:06   ` [PATCH v2] " Chris Wilson
  2017-03-17 20:20 ` [PATCH v2 4/4] drm: Peek at the current counter/timestamp for vblank queries Chris Wilson
  2017-03-17 20:44 ` [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Ville Syrjälä
  3 siblings, 2 replies; 9+ messages in thread
From: Chris Wilson @ 2017-03-17 20:20 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, intel-gfx

Move the repeated (a - b) <= (1 << 23) to its own function.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_irq.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index a164cf51d093..253505da57bd 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1492,6 +1492,11 @@ int drm_legacy_modeset_ctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
+static inline bool vblank_passed(u32 seq, u32 ref)
+{
+	return (seq - ref) <= (1 << 23);
+}
+
 static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 				  union drm_wait_vblank *vblwait,
 				  struct drm_file *file_priv)
@@ -1542,7 +1547,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 				      vblwait->request.sequence);
 
 	e->event.sequence = vblwait->request.sequence;
-	if ((seq - vblwait->request.sequence) <= (1 << 23)) {
+	if (vblank_passed(seq, vblwait->request.sequence)) {
 		drm_vblank_put(dev, pipe);
 		send_vblank_event(dev, e, seq, &now);
 		vblwait->reply.sequence = seq;
@@ -1632,9 +1637,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
 	}
 
 	if ((flags & _DRM_VBLANK_NEXTONMISS) &&
-	    (seq - vblwait->request.sequence) <= (1 << 23)) {
+	    vblank_passed(seq, vblwait->request.sequence))
 		vblwait->request.sequence = seq + 1;
-	}
 
 	if (flags & _DRM_VBLANK_EVENT) {
 		/* must hold on to the vblank ref until the event fires
@@ -1647,8 +1651,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
 		DRM_DEBUG("waiting on vblank count %u, crtc %u\n",
 			  vblwait->request.sequence, pipe);
 		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
-			    (drm_vblank_count(dev, pipe) -
-			     vblwait->request.sequence) <= (1 << 23) ||
+			    vblank_passed(drm_vblank_count(dev, pipe),
+					  vblwait->request.sequence) ||
 			    !READ_ONCE(vblank->enabled));
 	}
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/4] drm: Peek at the current counter/timestamp for vblank queries
  2017-03-17 20:20 [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Chris Wilson
  2017-03-17 20:20 ` [PATCH v2 2/4] drm: vblank cannot be enabled if dev->irq_enabled is false Chris Wilson
  2017-03-17 20:20 ` [PATCH v2 3/4] drm: Refactor vblank sequence number comparison Chris Wilson
@ 2017-03-17 20:20 ` Chris Wilson
  2017-03-17 20:44 ` [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Ville Syrjälä
  3 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2017-03-17 20:20 UTC (permalink / raw)
  To: dri-devel; +Cc: Michel Dänzer, Laurent Pinchart, Dave Airlie, intel-gfx

Bypass all the spinlocks and return the last timestamp and counter from
the last vblank if the driver delcares that it is accurate (and stable
across on/off), and the vblank is currently enabled.

This is dependent upon the both the hardware and driver to provide the
proper barriers to facilitate reading our bookkeeping outside of the
vblank interrupt and outside of the explicit vblank locks.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Michel Dänzer <michel@daenzer.net>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Dave Airlie <airlied@redhat.com>,
Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 drivers/gpu/drm/drm_irq.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 253505da57bd..846401548ec9 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1569,6 +1569,17 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 	return ret;
 }
 
+static bool drm_wait_vblank_is_query(union drm_wait_vblank *vblwait)
+{
+	if (vblwait->request.sequence)
+		return false;
+
+	return _DRM_VBLANK_RELATIVE ==
+		(vblwait->request.type & (_DRM_VBLANK_TYPES_MASK |
+					  _DRM_VBLANK_EVENT |
+					  _DRM_VBLANK_NEXTONMISS));
+}
+
 /*
  * Wait for VBLANK.
  *
@@ -1618,6 +1629,21 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
 
 	vblank = &dev->vblank[pipe];
 
+	/* If the counter is currently enabled and accurate, short-circuit
+	 * queries to return the cached timestamp of the last vblank.
+	 */
+	if (dev->vblank_disable_immediate &&
+	    drm_wait_vblank_is_query(vblwait) &&
+	    READ_ONCE(vblank->enabled)) {
+		struct timeval now;
+
+		vblwait->reply.sequence =
+			drm_vblank_count_and_time(dev, pipe, &now);
+		vblwait->reply.tval_sec = now.tv_sec;
+		vblwait->reply.tval_usec = now.tv_usec;
+		return 0;
+	}
+
 	ret = drm_vblank_get(dev, pipe);
 	if (ret) {
 		DRM_DEBUG("crtc %d failed to acquire vblank counter, %d\n", pipe, ret);
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock
  2017-03-17 20:20 [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Chris Wilson
                   ` (2 preceding siblings ...)
  2017-03-17 20:20 ` [PATCH v2 4/4] drm: Peek at the current counter/timestamp for vblank queries Chris Wilson
@ 2017-03-17 20:44 ` Ville Syrjälä
  3 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2017-03-17 20:44 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, dri-devel

On Fri, Mar 17, 2017 at 08:20:27PM +0000, Chris Wilson wrote:
> Order the update to vblank->enabled after the timestamp is primed so
> that a concurrent unlocked reader will only see the vblank->enabled with
> the current timestamp.
> 
> v2: vblank->enable is guarded by dev->vbl_lock not
> dev->vblank_time_lock, update the READ_ONCE accordingly.

The locking is indeed very confusing, and I don't know if it even makes
sense anymore. But this looks at least as sane as anything else here :)

For the series:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Do not add a READ_ONCE(vblank->enabled) inside the interrupt handler to
> avoid missing an interrupt whilst racing with enable_vblank()
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_irq.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 53a526c7b24d..c47e07c89136 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -325,6 +325,8 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
>  	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>  	unsigned long irqflags;
>  
> +	assert_spin_locked(&dev->vbl_lock);
> +
>  	/* Prevent vblank irq processing while disabling vblank irqs,
>  	 * so no updates of timestamps or count can happen after we've
>  	 * disabled. Needed to prevent races in case of delayed irq's.
> @@ -336,10 +338,8 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
>  	 * calling the ->disable_vblank() operation in atomic context with the
>  	 * hardware potentially runtime suspended.
>  	 */
> -	if (vblank->enabled) {
> +	if (cmpxchg_relaxed(&vblank->enabled, true, false))
>  		__disable_vblank(dev, pipe);
> -		vblank->enabled = false;
> -	}
>  
>  	/*
>  	 * Always update the count and timestamp to maintain the
> @@ -384,7 +384,7 @@ void drm_vblank_cleanup(struct drm_device *dev)
>  	for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
>  		struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>  
> -		WARN_ON(vblank->enabled &&
> +		WARN_ON(READ_ONCE(vblank->enabled) &&
>  			drm_core_check_feature(dev, DRIVER_MODESET));
>  
>  		del_timer_sync(&vblank->disable_timer);
> @@ -1105,11 +1105,16 @@ static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
>  		 */
>  		ret = __enable_vblank(dev, pipe);
>  		DRM_DEBUG("enabling vblank on crtc %u, ret: %d\n", pipe, ret);
> -		if (ret)
> +		if (ret) {
>  			atomic_dec(&vblank->refcount);
> -		else {
> -			vblank->enabled = true;
> +		} else {
>  			drm_update_vblank_count(dev, pipe, 0);
> +			/* drm_update_vblank_count() includes a wmb so we just
> +			 * need to ensure that the compiler emits the write
> +			 * to mark the vblank as enabled after the call
> +			 * to drm_update_vblank_count().
> +			 */
> +			WRITE_ONCE(vblank->enabled, true);
>  		}
>  	}
>  
> @@ -1517,7 +1522,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
>  	 * vblank disable, so no need for further locking.  The reference from
>  	 * drm_vblank_get() protects against vblank disable from another source.
>  	 */
> -	if (!vblank->enabled) {
> +	if (!READ_ONCE(vblank->enabled)) {
>  		ret = -EINVAL;
>  		goto err_unlock;
>  	}
> @@ -1644,7 +1649,7 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
>  		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
>  			    (((drm_vblank_count(dev, pipe) -
>  			       vblwait->request.sequence) <= (1 << 23)) ||
> -			     !vblank->enabled ||
> +			     !READ_ONCE(vblank->enabled) ||
>  			     !dev->irq_enabled));
>  	}
>  
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 3/4] drm: Refactor vblank sequence number comparison
  2017-03-17 20:20 ` [PATCH v2 3/4] drm: Refactor vblank sequence number comparison Chris Wilson
@ 2017-03-22  8:54   ` Michel Dänzer
  2017-03-22 10:06   ` [PATCH v2] " Chris Wilson
  1 sibling, 0 replies; 9+ messages in thread
From: Michel Dänzer @ 2017-03-22  8:54 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, dri-devel

On 18/03/17 05:20 AM, Chris Wilson wrote:
> Move the repeated (a - b) <= (1 << 23) to its own function.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_irq.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index a164cf51d093..253505da57bd 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1492,6 +1492,11 @@ int drm_legacy_modeset_ctl(struct drm_device *dev, void *data,
>  	return 0;
>  }
>  
> +static inline bool vblank_passed(u32 seq, u32 ref)
> +{
> +	return (seq - ref) <= (1 << 23);
> +}

As a followup, maybe we could try changing this to

    return (s32)(seq - ref) >= 0;


> @@ -1542,7 +1547,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
>  				      vblwait->request.sequence);
>  
>  	e->event.sequence = vblwait->request.sequence;
> -	if ((seq - vblwait->request.sequence) <= (1 << 23)) {
> +	if (vblank_passed(seq, vblwait->request.sequence)) {
>  		drm_vblank_put(dev, pipe);
>  		send_vblank_event(dev, e, seq, &now);
>  		vblwait->reply.sequence = seq;
> @@ -1632,9 +1637,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
>  	}
>  
>  	if ((flags & _DRM_VBLANK_NEXTONMISS) &&
> -	    (seq - vblwait->request.sequence) <= (1 << 23)) {
> +	    vblank_passed(seq, vblwait->request.sequence))
>  		vblwait->request.sequence = seq + 1;
> -	}
>  
>  	if (flags & _DRM_VBLANK_EVENT) {
>  		/* must hold on to the vblank ref until the event fires
> @@ -1647,8 +1651,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
>  		DRM_DEBUG("waiting on vblank count %u, crtc %u\n",
>  			  vblwait->request.sequence, pipe);
>  		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
> -			    (drm_vblank_count(dev, pipe) -
> -			     vblwait->request.sequence) <= (1 << 23) ||
> +			    vblank_passed(drm_vblank_count(dev, pipe),
> +					  vblwait->request.sequence) ||
>  			    !READ_ONCE(vblank->enabled));
>  	}
>  
> 

There's another instance in drm_handle_vblank_events.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] drm: Refactor vblank sequence number comparison
  2017-03-17 20:20 ` [PATCH v2 3/4] drm: Refactor vblank sequence number comparison Chris Wilson
  2017-03-22  8:54   ` Michel Dänzer
@ 2017-03-22 10:06   ` Chris Wilson
  2017-03-23  3:23     ` Michel Dänzer
  1 sibling, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2017-03-22 10:06 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter, intel-gfx, Michel Dänzer

Move the repeated (a - b) <= (1 << 23) to its own function.

v2: Catch the '1<<23' inside drm_handle_vblank() as well

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Michel Dänzer <michel@daenzer.net>
---
 drivers/gpu/drm/drm_irq.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index a164cf51d093..9efe1eacabe7 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1492,6 +1492,11 @@ int drm_legacy_modeset_ctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
+static inline bool vblank_passed(u32 seq, u32 ref)
+{
+	return (seq - ref) <= (1 << 23);
+}
+
 static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 				  union drm_wait_vblank *vblwait,
 				  struct drm_file *file_priv)
@@ -1542,7 +1547,7 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
 				      vblwait->request.sequence);
 
 	e->event.sequence = vblwait->request.sequence;
-	if ((seq - vblwait->request.sequence) <= (1 << 23)) {
+	if (vblank_passed(seq, vblwait->request.sequence)) {
 		drm_vblank_put(dev, pipe);
 		send_vblank_event(dev, e, seq, &now);
 		vblwait->reply.sequence = seq;
@@ -1632,9 +1637,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
 	}
 
 	if ((flags & _DRM_VBLANK_NEXTONMISS) &&
-	    (seq - vblwait->request.sequence) <= (1 << 23)) {
+	    vblank_passed(seq, vblwait->request.sequence))
 		vblwait->request.sequence = seq + 1;
-	}
 
 	if (flags & _DRM_VBLANK_EVENT) {
 		/* must hold on to the vblank ref until the event fires
@@ -1647,8 +1651,8 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
 		DRM_DEBUG("waiting on vblank count %u, crtc %u\n",
 			  vblwait->request.sequence, pipe);
 		DRM_WAIT_ON(ret, vblank->queue, 3 * HZ,
-			    (drm_vblank_count(dev, pipe) -
-			     vblwait->request.sequence) <= (1 << 23) ||
+			    vblank_passed(drm_vblank_count(dev, pipe),
+					  vblwait->request.sequence) ||
 			    !READ_ONCE(vblank->enabled));
 	}
 
@@ -1683,7 +1687,7 @@ static void drm_handle_vblank_events(struct drm_device *dev, unsigned int pipe)
 	list_for_each_entry_safe(e, t, &dev->vblank_event_list, base.link) {
 		if (e->pipe != pipe)
 			continue;
-		if ((seq - e->event.sequence) > (1<<23))
+		if (!vblank_passed(seq, e->event.sequence))
 			continue;
 
 		DRM_DEBUG("vblank event on %u, current %u\n",
-- 
2.11.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm: Refactor vblank sequence number comparison
  2017-03-22 10:06   ` [PATCH v2] " Chris Wilson
@ 2017-03-23  3:23     ` Michel Dänzer
  2017-03-29 11:32       ` Ville Syrjälä
  0 siblings, 1 reply; 9+ messages in thread
From: Michel Dänzer @ 2017-03-23  3:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx, dri-devel

On 22/03/17 07:06 PM, Chris Wilson wrote:
> Move the repeated (a - b) <= (1 << 23) to its own function.
> 
> v2: Catch the '1<<23' inside drm_handle_vblank() as well
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm: Refactor vblank sequence number comparison
  2017-03-23  3:23     ` Michel Dänzer
@ 2017-03-29 11:32       ` Ville Syrjälä
  0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2017-03-29 11:32 UTC (permalink / raw)
  To: Michel Dänzer; +Cc: Daniel Vetter, intel-gfx, dri-devel

On Thu, Mar 23, 2017 at 12:23:34PM +0900, Michel Dänzer wrote:
> On 22/03/17 07:06 PM, Chris Wilson wrote:
> > Move the repeated (a - b) <= (1 << 23) to its own function.
> > 
> > v2: Catch the '1<<23' inside drm_handle_vblank() as well
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>

Entire series pushed to drm-misc-next. Thanks for the patches and
review.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-03-29 11:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-17 20:20 [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Chris Wilson
2017-03-17 20:20 ` [PATCH v2 2/4] drm: vblank cannot be enabled if dev->irq_enabled is false Chris Wilson
2017-03-17 20:20 ` [PATCH v2 3/4] drm: Refactor vblank sequence number comparison Chris Wilson
2017-03-22  8:54   ` Michel Dänzer
2017-03-22 10:06   ` [PATCH v2] " Chris Wilson
2017-03-23  3:23     ` Michel Dänzer
2017-03-29 11:32       ` Ville Syrjälä
2017-03-17 20:20 ` [PATCH v2 4/4] drm: Peek at the current counter/timestamp for vblank queries Chris Wilson
2017-03-17 20:44 ` [PATCH v2 1/4] drm: Mark up accesses of vblank->enabled outside of its spinlock Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).