Re: [PATCH] drm-vblank: Always return true vblank count of scheduled vblank event.

On Fri, 10 Dec 2010 16:00:19 +0100 Mario Kleiner mario.kleiner@tuebingen.mpg.de wrote:

-------- Original Message -------- Subject: [PATCH] drm-vblank: Always return true vblank count of scheduled vblank event. Date: Fri, 10 Dec 2010 15:58:10 +0100 From: Mario Kleiner mario.kleiner@tuebingen.mpg.de To: airlied@gmail.com CC: jbarnes@virtuousgeek.org, krh@bitplanet.net, Mario Kleiner mario.kleiner@tuebingen.mpg.de

This patch tries to make sure that the vbl.reply.sequence vblank count for a queued or emitted vblank event always corresponds to the true vblank count of queueing/emission, so the ddx can rely on the returned target_msc for consistency checks and implementation of swap_intervals in glXSwapBuffers().

Without this there is a small race-condition between the userspace ddx queueing a vblank event and the vblank counter incrementing before the event gets queued in the kernel.

Signed-off-by: Mario Kleiner mario.kleiner@tuebingen.mpg.de

drivers/gpu/drm/drm_irq.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 4e82d0d..55160d7 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -1086,15 +1086,18 @@ static int drm_queue_vblank_event(struct drm_device *dev, int pipe,

e->event.sequence = vblwait->request.sequence; if ((seq - vblwait->request.sequence) <= (1 << 23)) {

• 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_add_tail(&e->base.link, &e->base.file_priv->event_list); wake_up_interruptible(&e->base.file_priv->event_wait);

• vblwait->reply.sequence = seq;
  

  trace_drm_vblank_event_delivered(current->pid, pipe, vblwait->request.sequence);

But this actually causes us to return the current count rather than the requested count if the event requested is in the past, right?

} else { list_add_tail(&e->base.link, &dev->vblank_event_list);

• vblwait->reply.sequence = vblwait->request.sequence;
  

  }

This one makes sense; we want to make sure the returned sequence is the one requested (assuming it was in the future at the time of the request anyway).