From 9d1997f72aa431612961cba0e4c2cbf40c07f7c4 Mon Sep 17 00:00:00 2001 From: Sultan Alsawaf Date: Wed, 25 Jan 2023 18:06:20 -0800 Subject: [PATCH] modesetting: Ensure vblank events always run in sequential order It is possible for vblank events to run out of order with respect to one another because the event which was queued to the kernel has the privilege of running before all other events are handled. This allows kernel-queued events to run before other, older events which should've run first. Although this isn't a huge problem now, it will become more problematic after the next change which ties DRI client notifications to TearFree page flips. This increases the likelihood of DRI clients erroneously receiving presentation-completion notifications out of order; i.e., a client could receive a notification for a newer pixmap it submitted *before* receiving a notification for an older pixmap. Ensure vblank events always run in sequential order by removing the bias towards kernel-queued events, and therefore forcing them to run at their sequential position in the queue like other events. Signed-off-by: Sultan Alsawaf Reviewed-by: Martin Roukala --- hw/xfree86/drivers/modesetting/vblank.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/drivers/modesetting/vblank.c b/hw/xfree86/drivers/modesetting/vblank.c index 4d250aa34..1c5f2578f 100644 --- a/hw/xfree86/drivers/modesetting/vblank.c +++ b/hw/xfree86/drivers/modesetting/vblank.c @@ -573,10 +573,15 @@ ms_drm_sequence_handler(int fd, uint64_t frame, uint64_t ns, Bool is64bit, uint6 if (q->seq == seq) { crtc = q->crtc; msc = ms_kernel_msc_to_crtc_msc(crtc, frame, is64bit); - xorg_list_del(&q->list); - if (!q->aborted) - q->handler(msc, ns / 1000, q->data); - free(q); + + /* Write the current MSC to this event to ensure its handler runs in + * the loop below. This is done because we don't want to run the + * handler right now, since we need to ensure all events are handled + * in FIFO order with respect to one another. Otherwise, if this + * event were handled first just because it was queued to the + * kernel, it could run before older events expiring at this MSC. + */ + q->msc = msc; break; } }