modesetting: fix reference issues with flip countpageflip-v6

This ports code from xf86-video-ati mostly, it moves
to tracking flips via an allocated structure instead
of hiding them at the modesetting driver level,

4 files changed, 144 insertions, 124 deletions

diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h
index 2d63caebc..9ae4966fd 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -105,10 +105,6 @@ typedef struct _modesettingRec {
      * Page flipping stuff.
      *  @{
      */
-    int flip_count;
-    uint64_t fe_msc;
-    uint64_t fe_usec;
-    struct ms_present_vblank_event *flip_vblank_event;
     /** @} */
 
     DamagePtr damage;
diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h
index 9ceddf2d0..fe363c577 100644
--- a/hw/xfree86/drivers/modesetting/drmmode_display.h
+++ b/hw/xfree86/drivers/modesetting/drmmode_display.h
@@ -46,7 +46,6 @@ typedef struct {
 typedef struct {
     int fd;
     unsigned fb_id;
-    unsigned old_fb_id;
     drmModeFBPtr mode_fb;
     int cpp;
     ScrnInfoPtr scrn;
diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index 82aeab035..1df65549e 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -225,63 +225,49 @@ ms_present_flush(WindowPtr window)
 }
 
 #ifdef GLAMOR
-struct ms_pageflip {
+
+/*
+ * Event data for an in progress flip.
+ * This contains a pointer to the vblank event,
+ * and information about the flip in progress.
+ * a reference to this is stored in the per-crtc
+ * flips.
+ */
+struct ms_flipdata {
     ScreenPtr screen;
-    Bool on_reference_crtc;
+    struct ms_present_vblank_event *event;
+    /* number of CRTC events referencing this */
+    int flip_count;
+    uint64_t fe_msc;
+    uint64_t fe_usec;
+    uint32_t old_fb_id;
 };
 
-/**
- * Notify Present that the flip is complete
+/*
+ * Per crtc pageflipping infomation,
+ * These are submitted to the queuing code
+ * one of them per crtc per flip.
  */
-static void
-ms_pageflip_complete(ScreenPtr screen)
-{
-    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    modesettingPtr ms = modesettingPTR(scrn);
-    uint64_t event_id = ms->flip_vblank_event->event_id;
-
-    DebugPresent(("\t\tms:fc %lld %p c %d msc %llu ust %llu\n",
-                  (long long) event_id, ms->flip_vblank_event,
-                  ms->flip_count,
-                  (long long) ms->fe_msc, (long long) ms->fe_usec));
-
-    free(ms->flip_vblank_event);
-    ms->flip_vblank_event = NULL;
-
-    /* Release framebuffer */
-    drmModeRmFB(ms->fd, ms->drmmode.old_fb_id);
-
-    /* Notify Present that the flip is complete. */
-    present_event_notify(event_id, ms->fe_usec, ms->fe_msc);
-}
+struct ms_crtc_pageflip {
+    Bool on_reference_crtc;
+    /* reference to the ms_flipdata */
+    struct ms_flipdata *flipdata;
+};
 
 /**
- * Called after processing a pageflip complete event from DRM.
+ * Free an ms_crtc_pageflip.
  *
- * Update the saved msc/ust values as needed, then check to see if the
- * whole set of events are complete and notify the application at that
- * point.
+ * Drops the reference count on the flipdata.
  */
-static Bool
-ms_handle_pageflip(struct ms_pageflip *flip, uint64_t msc, uint64_t usec)
+static void
+ms_present_flip_free(struct ms_crtc_pageflip *flip)
 {
-    ScreenPtr screen = flip->screen;
-    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-    modesettingPtr ms = modesettingPTR(scrn);
+    struct ms_flipdata *flipdata = flip->flipdata;
 
-    if (flip->on_reference_crtc) {
-        /* Cache msc, ust for later delivery with a Present event or
-         * GLX reply.
-         */
-        ms->fe_msc = msc;
-        ms->fe_usec = usec;
-    }
     free(flip);
-
-    ms->flip_count--;
-
-    /* Tell the caller if this was the last DRM flip complete event expected. */
-    return ms->flip_count == 0;
+    if (--flipdata->flip_count > 0)
+        return;
+    free(flipdata);
 }
 
 /**
@@ -293,19 +279,36 @@ ms_handle_pageflip(struct ms_pageflip *flip, uint64_t msc, uint64_t usec)
 static void
 ms_flip_handler(uint64_t msc, uint64_t ust, void *data)
 {
-    struct ms_pageflip *flip = data;
-    ScreenPtr screen = flip->screen;
+    struct ms_crtc_pageflip *flip = data;
+    ScreenPtr screen = flip->flipdata->screen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
     modesettingPtr ms = modesettingPTR(scrn);
-    (void) ms;
+    struct ms_flipdata *flipdata = flip->flipdata;
+
+    DebugPresent(("\t\tms:fh %lld c %d msc %llu ust %llu\n",
+                  (long long) flipdata->event->event_id,
+                  flipdata->flip_count,
+                  (long long) msc, (long long) ust));
+
+    if (flip->on_reference_crtc) {
+        flipdata->fe_msc = msc;
+        flipdata->fe_usec = ust;
+    }
+
+    if (flipdata->flip_count == 1) {
+        DebugPresent(("\t\tms:fc %lld c %d msc %llu ust %llu\n",
+                      (long long) flipdata->event->event_id,
+                      flipdata->flip_count,
+                      (long long) flipdata->fe_msc, (long long) flipdata->fe_usec));
 
-    DebugPresent(("\t\tms:fh %lld %p c %d msc %llu ust %llu\n",
-                 (long long) ms->flip_vblank_event->event_id,
-                 ms->flip_vblank_event, ms->flip_count,
-                 (long long) msc, (long long) ust));
 
-    if (ms_handle_pageflip(flip, msc, ust))
-        ms_pageflip_complete(screen);
+        ms_present_vblank_handler(flipdata->fe_msc,
+                                  flipdata->fe_usec,
+                                  flipdata->event);
+
+        drmModeRmFB(ms->fd, flipdata->old_fb_id);
+    }
+    ms_present_flip_free(flip);
 }
 
 /*
@@ -314,36 +317,34 @@ ms_flip_handler(uint64_t msc, uint64_t ust, void *data)
 static void
 ms_present_flip_abort(void *data)
 {
-    struct ms_pageflip *flip = data;
-    ScreenPtr screen = flip->screen;
+    struct ms_crtc_pageflip *flip = data;
+    ScreenPtr screen = flip->flipdata->screen;
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
     modesettingPtr ms = modesettingPTR(scrn);
-    struct ms_present_vblank_event *event = ms->flip_vblank_event;
+    struct ms_flipdata *flipdata = flip->flipdata;
+    struct ms_present_vblank_event *event = flipdata->event;
 
-    DebugPresent(("\t\tms:fa %lld\n", (long long) event->event_id));
+    DebugPresent(("\t\tms:fa %lld c %d\n", event->event_id, flipdata->flip_count));
 
-    if (ms_handle_pageflip(flip, 0, 0)) {
-        /* Present abort handling */
-        free(event);
-        ms->flip_vblank_event = NULL;
+    if (flipdata->flip_count == 1)
+        free(flipdata->event);
 
-        /* Release framebuffer */
-        drmModeRmFB(ms->drmmode.fd, ms->drmmode.old_fb_id);
-    }
+    ms_present_flip_free(flip);
 }
 
 static Bool
 queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
-                   int ref_crtc_vblank_pipe, int new_fb_id, uint32_t flags)
+                   struct ms_flipdata *flipdata,
+                   int ref_crtc_vblank_pipe, uint32_t flags)
 {
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
     modesettingPtr ms = modesettingPTR(scrn);
     drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
-    struct ms_pageflip *flip;
+    struct ms_crtc_pageflip *flip;
     uint32_t seq;
     int err;
 
-    flip = calloc(1, sizeof(struct ms_pageflip));
+    flip = calloc(1, sizeof(struct ms_crtc_pageflip));
     if (flip == NULL) {
         xf86DrvMsg(scrn->scrnIndex, X_WARNING,
                    "flip queue: carrier alloc failed.\n");
@@ -354,7 +355,7 @@ queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
      * completion event. All other crtc's events will be discarded.
      */
     flip->on_reference_crtc = (drmmode_crtc->vblank_pipe == ref_crtc_vblank_pipe);
-    flip->screen = screen;
+    flip->flipdata = flipdata;
 
     seq = ms_drm_queue_alloc(crtc, flip, ms_flip_handler, ms_present_flip_abort);
     if (!seq) {
@@ -362,15 +363,16 @@ queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
         return FALSE;
     }
 
-    DebugPresent(("\t\tms:fq %lld %p c %d -> %d seq %llu\n",
-                  (long long) ms->flip_vblank_event->event_id,
-                  ms->flip_vblank_event, ms->flip_count, ms->flip_count + 1,
+    DebugPresent(("\t\tms:fq %lld c %d -> %d seq %llu\n",
+                  (long long) flipdata->event->event_id,
+                  flipdata->flip_count, flipdata->flip_count + 1,
                   (long long) seq));
-    assert(ms->flip_count >= 0);
-    ms->flip_count++;
+
+    /* take a reference on flipdata for use in flip */
+    flipdata->flip_count++;
 
     while (drmModePageFlip(ms->fd, drmmode_crtc->mode_crtc->crtc_id,
-                           new_fb_id, flags, (void *) (uintptr_t) seq)) {
+                           ms->drmmode.fb_id, flags, (void *) (uintptr_t) seq)) {
         err = errno;
         /* We may have failed because the event queue was full.  Flush it
          * and retry.  If there was nothing to flush, then we failed for
@@ -396,41 +398,55 @@ queue_flip_on_crtc(ScreenPtr screen, xf86CrtcPtr crtc,
 static Bool
 ms_do_pageflip(ScreenPtr screen,
                PixmapPtr new_front,
+               struct ms_present_vblank_event *event,
                int ref_crtc_vblank_pipe,
-               Bool async,
-               uint64_t event_id)
+               Bool async)
 {
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
     modesettingPtr ms = modesettingPTR(scrn);
     xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
     drmmode_bo new_front_bo;
-    uint32_t new_fb_id;
     uint32_t flags;
     int i;
-
+    struct ms_flipdata *flipdata;
     glamor_block_handler(screen);
 
-    ms->flip_vblank_event = calloc(1, sizeof(struct ms_present_vblank_event));
-    if (!ms->flip_vblank_event)
-        return FALSE;
-    ms->flip_vblank_event->event_id = event_id;
-
     new_front_bo.gbm = glamor_gbm_bo_from_pixmap(screen, new_front);
     new_front_bo.dumb = NULL;
     if (!new_front_bo.gbm) {
         xf86DrvMsg(scrn->scrnIndex, X_ERROR,
                    "Failed to get GBM bo for flip to new front.\n");
-        free(ms->flip_vblank_event);
-        ms->flip_vblank_event = NULL;
         return FALSE;
     }
 
+    flipdata = calloc(1, sizeof(struct ms_flipdata));
+    if (!flipdata) {
+        drmmode_bo_destroy(&ms->drmmode, &new_front_bo);
+        xf86DrvMsg(scrn->scrnIndex, X_ERROR,
+                   "Failed to allocate flipdata.\n");
+        return FALSE;
+    }
+
+    flipdata->event = event;
+    flipdata->screen = screen;
+
+    /*
+     * Take a local reference on flipdata.
+     * if the first flip fails, the sequence abort
+     * code will free the crtc flip data, and drop
+     * it's reference which would cause this to be
+     * freed when we still required it.
+     */
+    flipdata->flip_count++;
+
     /* Create a new handle for the back buffer */
+    flipdata->old_fb_id = ms->drmmode.fb_id;
     if (drmModeAddFB(ms->fd, scrn->virtualX, scrn->virtualY,
                      scrn->depth, scrn->bitsPerPixel,
                      drmmode_bo_get_pitch(&new_front_bo),
-                     drmmode_bo_get_handle(&new_front_bo), &new_fb_id))
+                     drmmode_bo_get_handle(&new_front_bo), &ms->drmmode.fb_id)) {
         goto error_out;
+    }
 
     drmmode_bo_destroy(&ms->drmmode, &new_front_bo);
 
@@ -447,49 +463,51 @@ ms_do_pageflip(ScreenPtr screen,
      * Also, flips queued on disabled or incorrectly configured displays
      * may never complete; this is a configuration error.
      */
-    ms->fe_msc = 0;
-    ms->fe_usec = 0;
-
     for (i = 0; i < config->num_crtc; i++) {
         xf86CrtcPtr crtc = config->crtc[i];
 
         if (!ms_crtc_on(crtc))
             continue;
 
-        if (!queue_flip_on_crtc(screen, crtc, ref_crtc_vblank_pipe, new_fb_id,
+        if (!queue_flip_on_crtc(screen, crtc, flipdata,
+                                ref_crtc_vblank_pipe,
                                 flags)) {
             goto error_undo;
         }
     }
 
-    ms->drmmode.old_fb_id = ms->drmmode.fb_id;
-    ms->drmmode.fb_id = new_fb_id;
-
-    assert(ms->flip_count >= 0);
-    if (ms->flip_count == 0) {
-        ms_pageflip_complete(screen);
+    /*
+     * Do we have more than our local reference,
+     * if so and no errors, then drop our local
+     * reference and return now.
+     */
+    if (flipdata->flip_count > 1) {
+        flipdata->flip_count--;
+        return TRUE;
     }
 
-    return TRUE;
-
 error_undo:
-    drmModeRmFB(ms->fd, new_fb_id);
-    for (i = 0; i < config->num_crtc; i++) {
-        if (config->crtc[i]->enabled) {
-            ErrorF("XXX: crtc apply\n");
-            /* intel_crtc_apply(config->crtc[i]); */
-        }
+
+    /*
+     * Have we just got the local reference?
+     * free the framebuffer if so since nobody successfully
+     * submitted anything
+     */
+    if (flipdata->flip_count == 1) {
+        drmModeRmFB(ms->fd, ms->drmmode.fb_id);
+        ms->drmmode.fb_id = flipdata->old_fb_id;
     }
 
 error_out:
     xf86DrvMsg(scrn->scrnIndex, X_WARNING, "Page flip failed: %s\n",
                strerror(errno));
+    /* if only the local reference - free the structure,
+     * else drop the local reference and return */
+    if (flipdata->flip_count == 1)
+        free(flipdata);
+    else
+        flipdata->flip_count--;
 
-    free(ms->flip_vblank_event);
-    ms->flip_vblank_event = NULL;
-
-    assert(ms->flip_count >= 0);
-    ms->flip_count = 0;
     return FALSE;
 }
 
@@ -559,12 +577,17 @@ ms_present_flip(RRCrtcPtr crtc,
     xf86CrtcPtr xf86_crtc = crtc->devPrivate;
     drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
     Bool ret;
+    struct ms_present_vblank_event *event;
 
     if (!ms_present_check_flip(crtc, screen->root, pixmap, sync_flip))
         return FALSE;
 
-    ret = ms_do_pageflip(screen, pixmap, drmmode_crtc->vblank_pipe, !sync_flip,
-                         event_id);
+    event = calloc(1, sizeof(struct ms_present_vblank_event));
+    if (!event)
+        return FALSE;
+
+    event->event_id = event_id;
+    ret = ms_do_pageflip(screen, pixmap, event, drmmode_crtc->vblank_pipe, !sync_flip);
     if (!ret)
         xf86DrvMsg(scrn->scrnIndex, X_ERROR, "present flip failed\n");
 
@@ -582,9 +605,16 @@ ms_present_unflip(ScreenPtr screen, uint64_t event_id)
     Bool ret;
     xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
     int i;
+    struct ms_present_vblank_event *event;
+
+    event = calloc(1, sizeof(struct ms_present_vblank_event));
+    if (!event)
+        return;
+
+    event->event_id = event_id;
 
     if (ms_present_check_flip(NULL, screen->root, pixmap, TRUE) &&
-        ms_do_pageflip(screen, pixmap, -1, FALSE, event_id)) {
+        ms_do_pageflip(screen, pixmap, event, -1, FALSE)) {
         return;
     }
 
diff --git a/hw/xfree86/drivers/modesetting/vblank.c b/hw/xfree86/drivers/modesetting/vblank.c
index 776dcef9b..0b7bf9d73 100644
--- a/hw/xfree86/drivers/modesetting/vblank.c
+++ b/hw/xfree86/drivers/modesetting/vblank.c
@@ -50,11 +50,6 @@
 static struct xorg_list ms_drm_queue;
 static uint32_t ms_drm_seq;
 
-struct ms_pageflip {
-    ScreenPtr screen;
-    Bool crtc_for_msc_ust;
-};
-
 static void ms_box_intersect(BoxPtr dest, BoxPtr a, BoxPtr b)
 {
     dest->x1 = a->x1 > b->x1 ? a->x1 : b->x1;