xf86-video-modesetting: Add ms_queue_vblank helper [v3]

This provides an API wrapper around the kernel interface for queueing
a vblank event, simplifying all of the callers.

v2: Fix missing '|' in computing vbl.request.type

v3: Remove spurious bit of next patch (thanks, Michel Dänzer)

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>

hw/xfree86/drivers/modesetting/dri2.c

@@ -695,19 +695,16 @@ ms_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, CARD64 target_msc,
 {
     ScreenPtr screen = draw->pScreen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    modesettingPtr ms = modesettingPTR(scrn);
     ms_dri2_frame_event_ptr wait_info;
-    drmVBlank vbl;
     int ret;
     xf86CrtcPtr crtc = ms_dri2_crtc_covering_drawable(draw);
-    drmmode_crtc_private_ptr drmmode_crtc;
     CARD64 current_msc, current_ust, request_msc;
     uint32_t seq;
+    uint64_t queued_msc;
 
     /* Drawable not visible, return immediately */
     if (!crtc)
         goto out_complete;
-    drmmode_crtc = crtc->driver_private;
 
     wait_info = calloc(1, sizeof(*wait_info));
     if (!wait_info)
@@ -747,13 +744,8 @@ ms_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, CARD64 target_msc,
 
         if (current_msc >= target_msc)
             target_msc = current_msc;
-        vbl.request.type = (DRM_VBLANK_ABSOLUTE |
-                            DRM_VBLANK_EVENT |
-                            drmmode_crtc->vblank_pipe);
-        vbl.request.sequence = ms_crtc_msc_to_kernel_msc(crtc, target_msc);
-        vbl.request.signal = (unsigned long)seq;
 
-        ret = drmWaitVBlank(ms->fd, &vbl);
+        ret = ms_queue_vblank(crtc, MS_QUEUE_ABSOLUTE, target_msc, &queued_msc, seq);
         if (ret) {
             static int limit = 5;
             if (limit) {
@@ -766,7 +758,7 @@ ms_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, CARD64 target_msc,
             goto out_free;
         }
 
-        wait_info->frame = ms_kernel_msc_to_crtc_msc(crtc, vbl.reply.sequence);
+        wait_info->frame = queued_msc;
         DRI2BlockClient(client, draw);
         return TRUE;
     }
@@ -775,9 +767,6 @@ ms_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, CARD64 target_msc,
      * If we get here, target_msc has already passed or we don't have one,
      * so we queue an event that will satisfy the divisor/remainder equation.
      */
-    vbl.request.type =
-        DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | drmmode_crtc->vblank_pipe;
-
     request_msc = current_msc - (current_msc % divisor) +
         remainder;
     /*
@@ -795,11 +784,7 @@ ms_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, CARD64 target_msc,
     if (!seq)
         goto out_free;
 
-    vbl.request.sequence = ms_crtc_msc_to_kernel_msc(crtc, request_msc);
+    if (!ms_queue_vblank(crtc, MS_QUEUE_ABSOLUTE, request_msc, &queued_msc, seq)) {
-    vbl.request.signal = (unsigned long)seq;
-
-    ret = drmWaitVBlank(ms->fd, &vbl);
-    if (ret) {
         static int limit = 5;
         if (limit) {
             xf86DrvMsg(scrn->scrnIndex, X_WARNING,
@@ -811,7 +796,8 @@ ms_dri2_schedule_wait_msc(ClientPtr client, DrawablePtr draw, CARD64 target_msc,
         goto out_free;
     }
 
-    wait_info->frame = ms_kernel_msc_to_crtc_msc(crtc, vbl.reply.sequence);
+    wait_info->frame = queued_msc;
+
     DRI2BlockClient(client, draw);
 
     return TRUE;
@@ -839,20 +825,18 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
 {
     ScreenPtr screen = draw->pScreen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    modesettingPtr ms = modesettingPTR(scrn);
-    drmVBlank vbl;
     int ret, flip = 0;
     xf86CrtcPtr crtc = ms_dri2_crtc_covering_drawable(draw);
-    drmmode_crtc_private_ptr drmmode_crtc;
     ms_dri2_frame_event_ptr frame_info = NULL;
     uint64_t current_msc, current_ust;
     uint64_t request_msc;
     uint32_t seq;
+    ms_queue_flag ms_flag = MS_QUEUE_ABSOLUTE;
+    uint64_t queued_msc;
 
     /* Drawable not displayed... just complete the swap */
     if (!crtc)
         goto blit_fallback;
-    drmmode_crtc = crtc->driver_private;
 
     frame_info = calloc(1, sizeof(*frame_info));
     if (!frame_info)
@@ -878,6 +862,8 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
     ms_dri2_reference_buffer(back);
 
     ret = ms_get_crtc_ust_msc(crtc, &current_ust, &current_msc);
+    if (ret != Success)
+        goto blit_fallback;
 
     /* Flips need to be submitted one frame before */
     if (can_flip(scrn, draw, front, back)) {
@@ -892,22 +878,19 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
     if (*target_msc > 0)
         *target_msc -= flip;
 
+    /* If non-pageflipping, but blitting/exchanging, we need to use
+     * DRM_VBLANK_NEXTONMISS to avoid unreliable timestamping later
+     * on.
+     */
+    if (flip == 0)
+        ms_flag |= MS_QUEUE_NEXT_ON_MISS;
+
     /*
      * If divisor is zero, or current_msc is smaller than target_msc
      * we just need to make sure target_msc passes before initiating
      * the swap.
      */
     if (divisor == 0 || current_msc < *target_msc) {
-        vbl.request.type = (DRM_VBLANK_ABSOLUTE |
-                            DRM_VBLANK_EVENT |
-                            drmmode_crtc->vblank_pipe);
-
-        /* If non-pageflipping, but blitting/exchanging, we need to use
-         * DRM_VBLANK_NEXTONMISS to avoid unreliable timestamping later
-         * on.
-         */
-        if (flip == 0)
-            vbl.request.type |= DRM_VBLANK_NEXTONMISS;
 
         /* If target_msc already reached or passed, set it to
          * current_msc to ensure we return a reasonable value back
@@ -922,19 +905,14 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
         if (!seq)
             goto blit_fallback;
 
-        vbl.request.sequence = ms_crtc_msc_to_kernel_msc(crtc, *target_msc);
+        if (!ms_queue_vblank(crtc, ms_flag, *target_msc, &queued_msc, seq)) {
-        vbl.request.signal = (unsigned long)seq;
-
-        ret = drmWaitVBlank(ms->fd, &vbl);
-        if (ret) {
             xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                        "divisor 0 get vblank counter failed: %s\n",
                        strerror(errno));
             goto blit_fallback;
         }
 
-        *target_msc = ms_kernel_msc_to_crtc_msc(crtc,
+        *target_msc = queued_msc + flip;
-                                                vbl.reply.sequence + flip);
         frame_info->frame = *target_msc;
 
         return TRUE;
@@ -945,11 +923,6 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
      * and we need to queue an event that will satisfy the divisor/remainder
      * equation.
      */
-    vbl.request.type = (DRM_VBLANK_ABSOLUTE |
-                        DRM_VBLANK_EVENT |
-                        drmmode_crtc->vblank_pipe);
-    if (flip == 0)
-        vbl.request.type |= DRM_VBLANK_NEXTONMISS;
 
     request_msc = current_msc - (current_msc % divisor) +
         remainder;
@@ -966,7 +939,6 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
     if (request_msc <= current_msc)
         request_msc += divisor;
 
-
     seq = ms_drm_queue_alloc(crtc, frame_info,
                              ms_dri2_frame_event_handler,
                              ms_dri2_frame_event_abort);
@@ -974,11 +946,7 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
         goto blit_fallback;
 
     /* Account for 1 frame extra pageflip delay if flip > 0 */
-    vbl.request.sequence = ms_crtc_msc_to_kernel_msc(crtc, request_msc) - flip;
+    if (!ms_queue_vblank(crtc, ms_flag, request_msc - flip, &queued_msc, seq)) {
-    vbl.request.signal = (unsigned long)seq;
-
-    ret = drmWaitVBlank(ms->fd, &vbl);
-    if (ret) {
         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                    "final get vblank counter failed: %s\n",
                    strerror(errno));
@@ -986,7 +954,7 @@ ms_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
     }
 
     /* Adjust returned value for 1 fame pageflip offset of flip > 0 */
-    *target_msc = ms_kernel_msc_to_crtc_msc(crtc, vbl.reply.sequence + flip);
+    *target_msc = queued_msc + flip;
     frame_info->frame = *target_msc;
 
     return TRUE;

hw/xfree86/drivers/modesetting/driver.h

@@ -111,6 +111,10 @@ typedef struct _modesettingRec {
     Bool dirty_enabled;
 
     uint32_t cursor_width, cursor_height;
+
+    Bool has_queue_sequence;
+    Bool tried_queue_sequence;
+
 } modesettingRec, *modesettingPtr;
 
 #define modesettingPTR(p) ((modesettingPtr)((p)->driverPrivate))
@@ -121,6 +125,15 @@ uint32_t ms_drm_queue_alloc(xf86CrtcPtr crtc,
                             ms_drm_handler_proc handler,
                             ms_drm_abort_proc abort);
 
+typedef enum ms_queue_flag {
+    MS_QUEUE_ABSOLUTE = 0,
+    MS_QUEUE_RELATIVE = 1,
+    MS_QUEUE_NEXT_ON_MISS = 2
+} ms_queue_flag;
+
+Bool ms_queue_vblank(xf86CrtcPtr crtc, ms_queue_flag flags,
+                     uint64_t msc, uint64_t *msc_queued, uint32_t seq);
+
 void ms_drm_abort(ScrnInfoPtr scrn,
                   Bool (*match)(void *data, void *match_data),
                   void *match_data);
@@ -132,8 +145,8 @@ xf86CrtcPtr ms_dri2_crtc_covering_drawable(DrawablePtr pDraw);
 
 int ms_get_crtc_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc);
 
-uint32_t ms_crtc_msc_to_kernel_msc(xf86CrtcPtr crtc, uint64_t expect);
+uint64_t ms_crtc_msc_to_kernel_msc(xf86CrtcPtr crtc, uint64_t expect);
-uint64_t ms_kernel_msc_to_crtc_msc(xf86CrtcPtr crtc, uint32_t sequence);
+uint64_t ms_kernel_msc_to_crtc_msc(xf86CrtcPtr crtc, uint64_t sequence);
 
 
 Bool ms_dri2_screen_init(ScreenPtr screen);

hw/xfree86/drivers/modesetting/drmmode_display.c

@@ -279,8 +279,6 @@ drmmode_SharedPixmapPresentOnVBlank(PixmapPtr ppix, xf86CrtcPtr crtc,
 {
     drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
     msPixmapPrivPtr ppriv = msGetPixmapPriv(drmmode, ppix);
-
-    drmVBlank vbl;
     struct vblank_event_args *event_args;
 
     if (ppix == drmmode_crtc->prime_pixmap)
@@ -303,12 +301,7 @@ drmmode_SharedPixmapPresentOnVBlank(PixmapPtr ppix, xf86CrtcPtr crtc,
                            drmmode_SharedPixmapVBlankEventHandler,
                            drmmode_SharedPixmapVBlankEventAbort);
 
-    vbl.request.type =
+    return ms_queue_vblank(crtc, MS_QUEUE_RELATIVE, 1, NULL, ppriv->flip_seq);
-        DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT | drmmode_crtc->vblank_pipe;
-    vbl.request.sequence = 1;
-    vbl.request.signal = (unsigned long) ppriv->flip_seq;
-
-    return drmWaitVBlank(drmmode->fd, &vbl) >= 0;
 }
 
 Bool

hw/xfree86/drivers/modesetting/present.c

@@ -109,13 +109,7 @@ ms_present_queue_vblank(RRCrtcPtr crtc,
                         uint64_t msc)
 {
     xf86CrtcPtr xf86_crtc = crtc->devPrivate;
-    ScreenPtr screen = crtc->pScreen;
-    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    modesettingPtr ms = modesettingPTR(scrn);
-    drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
     struct ms_present_vblank_event *event;
-    drmVBlank vbl;
-    int ret;
     uint32_t seq;
 
     event = calloc(sizeof(struct ms_present_vblank_event), 1);
@@ -130,22 +124,9 @@ ms_present_queue_vblank(RRCrtcPtr crtc,
         return BadAlloc;
     }
 
-    vbl.request.type =
+    if (!ms_queue_vblank(xf86_crtc, MS_QUEUE_ABSOLUTE, msc, NULL, seq))
-        DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT | drmmode_crtc->vblank_pipe;
+        return BadAlloc;
-    vbl.request.sequence = ms_crtc_msc_to_kernel_msc(xf86_crtc, msc);
+
-    vbl.request.signal = seq;
-    for (;;) {
-        ret = drmWaitVBlank(ms->fd, &vbl);
-        if (!ret)
-            break;
-        /* If we hit EBUSY, then try to flush events.  If we can't, then
-         * this is an error.
-         */
-        if (errno != EBUSY || ms_flush_drm_events(screen) < 0) {
-	    ms_drm_abort_seq(scrn, seq);
-            return BadAlloc;
-        }
-    }
     DebugPresent(("\t\tmq %lld seq %u msc %llu (hw msc %u)\n",
                  (long long) event_id, seq, (long long) msc,
                  vbl.request.sequence));

hw/xfree86/drivers/modesetting/vblank.c

@@ -173,7 +173,7 @@ ms_dri2_crtc_covering_drawable(DrawablePtr pDraw)
 
 static Bool
 ms_get_kernel_ust_msc(xf86CrtcPtr crtc,
-                      uint32_t *msc, uint64_t *ust)
+                      uint64_t *msc, uint64_t *ust)
 {
     ScreenPtr screen = crtc->randr_crtc->pScreen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
@@ -198,13 +198,50 @@ ms_get_kernel_ust_msc(xf86CrtcPtr crtc,
     }
 }
 
+Bool
+ms_queue_vblank(xf86CrtcPtr crtc, ms_queue_flag flags,
+                uint64_t msc, uint64_t *msc_queued, uint32_t seq)
+{
+    ScreenPtr screen = crtc->randr_crtc->pScreen;
+    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+    modesettingPtr ms = modesettingPTR(scrn);
+    drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+    drmVBlank vbl;
+    int ret;
+
+    for (;;) {
+        /* Queue an event at the specified sequence */
+        vbl.request.type = DRM_VBLANK_EVENT | drmmode_crtc->vblank_pipe;
+        if (flags & MS_QUEUE_RELATIVE)
+            vbl.request.type |= DRM_VBLANK_RELATIVE;
+        else
+            vbl.request.type |= DRM_VBLANK_ABSOLUTE;
+        if (flags & MS_QUEUE_NEXT_ON_MISS)
+            vbl.request.type |= DRM_VBLANK_NEXTONMISS;
+
+        vbl.request.sequence = ms_crtc_msc_to_kernel_msc(crtc, msc);
+        vbl.request.signal = seq;
+        ret = drmWaitVBlank(ms->fd, &vbl);
+        if (ret == 0) {
+            if (msc_queued)
+                *msc_queued = ms_kernel_msc_to_crtc_msc(crtc, vbl.reply.sequence);
+            return TRUE;
+        }
+        if (errno != EBUSY) {
+            ms_drm_abort_seq(scrn, msc);
+            return FALSE;
+        }
+        ms_flush_drm_events(screen);
+    }
+}
+
 /**
  * Convert a 32-bit kernel MSC sequence number to a 64-bit local sequence
  * number, adding in the vblank_offset and high 32 bits, and dealing
  * with 64-bit wrapping
  */
 uint64_t
-ms_kernel_msc_to_crtc_msc(xf86CrtcPtr crtc, uint32_t sequence)
+ms_kernel_msc_to_crtc_msc(xf86CrtcPtr crtc, uint64_t sequence)
 {
     drmmode_crtc_private_rec *drmmode_crtc = crtc->driver_private;
     sequence += drmmode_crtc->vblank_offset;
@@ -218,7 +255,7 @@ ms_kernel_msc_to_crtc_msc(xf86CrtcPtr crtc, uint32_t sequence)
 int
 ms_get_crtc_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc)
 {
-    uint32_t kernel_msc;
+    uint64_t kernel_msc;
 
     if (!ms_get_kernel_ust_msc(crtc, &kernel_msc, ust))
         return BadMatch;
@@ -230,13 +267,13 @@ ms_get_crtc_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc)
 #define MAX_VBLANK_OFFSET       1000
 
 /**
- * Convert a 64-bit adjusted MSC value into a 32-bit kernel sequence number,
+ * Convert a 64-bit adjusted MSC value into a 64-bit kernel sequence number,
- * removing the high 32 bits and subtracting out the vblank_offset term.
+ * by subtracting out the vblank_offset term.
  *
  * This also updates the vblank_offset when it notices that the value should
  * change.
  */
-uint32_t
+uint64_t
 ms_crtc_msc_to_kernel_msc(xf86CrtcPtr crtc, uint64_t expect)
 {
     drmmode_crtc_private_rec *drmmode_crtc = crtc->driver_private;
@@ -257,7 +294,7 @@ ms_crtc_msc_to_kernel_msc(xf86CrtcPtr crtc, uint64_t expect)
                 drmmode_crtc->vblank_offset = 0;
         }
     }
-    return (uint32_t) (expect - drmmode_crtc->vblank_offset);
+    return (expect - drmmode_crtc->vblank_offset);
 }
 
 /**
@@ -375,25 +412,31 @@ ms_drm_abort(ScrnInfoPtr scrn, Bool (*match)(void *data, void *match_data),
  * drm event queue and calls the handler for it.
  */
 static void
-ms_drm_handler(int fd, uint32_t frame, uint32_t sec, uint32_t usec,
+ms_drm_sequence_handler(int fd, uint64_t frame, uint64_t ns, uint64_t user_data)
-               void *user_ptr)
 {
     struct ms_drm_queue *q, *tmp;
-    uint32_t user_data = (uint32_t) (intptr_t) user_ptr;
+    uint32_t seq = (uint32_t) user_data;
 
     xorg_list_for_each_entry_safe(q, tmp, &ms_drm_queue, list) {
-        if (q->seq == user_data) {
+        if (q->seq == seq) {
             uint64_t msc;
 
             msc = ms_kernel_msc_to_crtc_msc(q->crtc, frame);
             xorg_list_del(&q->list);
-            q->handler(msc, (uint64_t) sec * 1000000 + usec, q->data);
+            q->handler(msc, ns / 1000, q->data);
             free(q);
             break;
         }
     }
 }
 
+static void
+ms_drm_handler(int fd, uint32_t frame, uint32_t sec, uint32_t usec,
+               void *user_ptr)
+{
+    ms_drm_sequence_handler(fd, frame, ((uint64_t) sec * 1000000 + usec) * 1000, (uint32_t) (uintptr_t) user_ptr);
+}
+
 Bool
 ms_vblank_screen_init(ScreenPtr screen)
 {