diff mbox

[2/3] drm: Fix emitted vblank timestamps in drm_vblank_off()

Message ID 1407288166-19881-3-git-send-email-mario.kleiner.de@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mario Kleiner Aug. 6, 2014, 1:22 a.m. UTC
Move the query for vblank count and time before the
vblank_disable_and_save(), because the disable fn
will invalidate the vblank timestamps, so all emitted
events would carry an invalid zero timestamp instead of
the timestamp of the vblank of vblank disable. This could
confuse clients.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/drm_irq.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Ville Syrjala Aug. 6, 2014, 10:53 a.m. UTC | #1
On Wed, Aug 06, 2014 at 03:22:45AM +0200, Mario Kleiner wrote:
> Move the query for vblank count and time before the
> vblank_disable_and_save(), because the disable fn
> will invalidate the vblank timestamps, so all emitted
> events would carry an invalid zero timestamp instead of
> the timestamp of the vblank of vblank disable. This could
> confuse clients.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/drm_irq.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 553a58c..89e91e3 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -1019,13 +1019,14 @@ void drm_vblank_off(struct drm_device *dev, int crtc)
>  	unsigned long irqflags;
>  	unsigned int seq;
>  
> +	/* Get 'now' vblank ts before it gets cleared by vblank disable */
> +	seq = drm_vblank_count_and_time(dev, crtc, &now);
> +

This will cause us to send out a potentially stale vblank seq/ts. I had
a different solution to this (not clearing the timestamps) in my pending
vblank series. I have a few more patches on top of that series lined up.
Let me send out the full series so we can discuss it if needed...

>  	spin_lock_irqsave(&dev->vbl_lock, irqflags);
>  	vblank_disable_and_save(dev, crtc);
>  	wake_up(&dev->vblank[crtc].queue);
>  
>  	/* 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)
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 553a58c..89e91e3 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -1019,13 +1019,14 @@  void drm_vblank_off(struct drm_device *dev, int crtc)
 	unsigned long irqflags;
 	unsigned int seq;
 
+	/* Get 'now' vblank ts before it gets cleared by vblank disable */
+	seq = drm_vblank_count_and_time(dev, crtc, &now);
+
 	spin_lock_irqsave(&dev->vbl_lock, irqflags);
 	vblank_disable_and_save(dev, crtc);
 	wake_up(&dev->vblank[crtc].queue);
 
 	/* 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)