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 <sultan@kerneltoast.com>
Reviewed-by: Martin Roukala <martin.roukala@mupuf.org>

1 files 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;
         }
     }