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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel OTC