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 at gmail.com>
> Cc: stable at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrj?l?
Intel OTC

Reply via email to