Re: [PATCH 1/3] drm: Send pending vblank events before disabling vblank.

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed, 27 Apr 2011 16:10:57 +1000
christopher.halse.rogers@canonical.com wrote:

> From: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
> 
> This is the least-bad behaviour.  It means that we signal the
> vblank event before it actually happens, but since we're disabling
> vblanks there's no guarantee that it will *ever* happen otherwise.
> 
> This prevents GL applications which use WaitMSC from hanging
> indefinitely.
> 
> Signed-off-by: Christopher James Halse Rogers <christopher.halse.rogers@canonical.com>
> ---
>  drivers/gpu/drm/drm_irq.c |   23 +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 741457b..a1f12cb 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -932,11 +932,34 @@ EXPORT_SYMBOL(drm_vblank_put);
>  
>  void drm_vblank_off(struct drm_device *dev, int crtc)
>  {
> +	struct drm_pending_vblank_event *e, *t;
> +	struct timeval now;
>  	unsigned long irqflags;
> +	unsigned int seq;
>  
>  	spin_lock_irqsave(&dev->vbl_lock, irqflags);
>  	vblank_disable_and_save(dev, crtc);
>  	DRM_WAKEUP(&dev->vbl_queue[crtc]);
> +
> +	/* Send any queued vblank events, lest the natives grow disquiet */
> +	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)
> +			continue;
> +		DRM_DEBUG("Sending premature vblank event on disable: \
> +			  wanted %d, current %d\n",
> +			  e->event.sequence, seq);
> +
> +		e->event.sequence = seq;
> +		e->event.tv_sec = now.tv_sec;
> +		e->event.tv_usec = now.tv_usec;
> +		drm_vblank_put(dev, e->pipe);
> +		list_move_tail(&e->base.link, &e->base.file_priv->event_list);
> +		wake_up_interruptible(&e->base.file_priv->event_wait);
> +		trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> +						 e->event.sequence);
> +	}
> +
>  	spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
>  }
>  EXPORT_SYMBOL(drm_vblank_off);

Yeah, this matches what we do for the blocking waits, and it's probably
a good idea.  Userspace can decide what to do with apps running against
a disabled CRTC.

Would be nice to share the code with drm_handle_vblank_events though
somehow.  Looks like

		e->event.sequence = seq;
		e->event.tv_sec = now.tv_sec;
		e->event.tv_usec = now.tv_usec;
		drm_vblank_put(dev, e->pipe);
		list_move_tail(&e->base.link, &e->base.file_priv->event_list);
		wake_up_interruptible(&e->base.file_priv->event_wait);
		trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
						 e->event.sequence);

could be pulled out into a separate function for both to use.

-- 
Jesse Barnes, Intel Open Source Technology Center