ddx/ati: Fix reporting of pageflip completion events on multi-head.

When a drawable is page-flipped on multiple crtc's (fullscreen
drawable on mirror-mode or multi-head x-screen), only one pageflip
event is finally delivered, after the last participating crtc signals
flip completion, this to avoid visual corruption.

Old code returned vblank count and timestamps of flip completion
of this last crtc, instead of the values of the "master crtc", the
one that was used for initially scheduling/triggering the pagflip
via vblank events. (master = radeon_dri2_drawable_crtc(drawable))

This patch makes sure that the pageflip completion values of the
"master" crtc are returned, otherwise client applications will
get confused by the random (msc, ust) values returned by whichever
crtc was the last to complete its flip. Without this, the returned
values change randomly and jump forward and backward in time and
count.

The patch also implements a consistency check on returned vblank
count values of pageflip completion. Impossible values are detected,
a x-warning is logged and returned (msc,ust) values are marked invalid,
so clients could perform error handling. Such a warning would indicate
bugs in the pageflip completion routine of future kms drivers or the
ddx to aid driver debugging.

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

3 files changed, 84 insertions, 26 deletions

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 1310da7c..9248cb0e 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -1284,18 +1284,32 @@ static void
 drmmode_flip_handler(int fd, unsigned int frame, unsigned int tv_sec,
 		     unsigned int tv_usec, void *event_data)
 {
-	drmmode_ptr drmmode = event_data;
+	drmmode_flipevtcarrier_ptr flipcarrier = event_data;
+	drmmode_ptr drmmode = flipcarrier->drmmode;
+
+	/* Is this the event whose info shall be delivered to higher level? */
+	if (flipcarrier->dispatch_me) {
+		/* Yes: Cache msc, ust for later delivery. */
+		drmmode->fe_frame = frame;
+		drmmode->fe_tv_sec = tv_sec;
+		drmmode->fe_tv_usec = tv_usec;
+	}
+	free(flipcarrier);
 
+	/* Last crtc completed flip? */
 	drmmode->flip_count--;
 	if (drmmode->flip_count > 0)
 		return;
 
+	/* Release framebuffer */
 	drmModeRmFB(drmmode->fd, drmmode->old_fb_id);
 
 	if (drmmode->event_data == NULL)
 		return;
 
-	radeon_dri2_flip_event_handler(frame, tv_sec, tv_usec, drmmode->event_data);
+	/* Deliver cached msc, ust from reference crtc to flip event handler */
+	radeon_dri2_flip_event_handler(drmmode->fe_frame, drmmode->fe_tv_sec,
+				       drmmode->fe_tv_usec, drmmode->event_data);
 }
 
 
@@ -1584,7 +1598,7 @@ void drmmode_uevent_fini(ScrnInfoPtr scrn, drmmode_ptr drmmode)
 #endif
 }
 
-Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data)
+Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data, int ref_crtc_hw_id)
 {
 	RADEONInfoPtr info = RADEONPTR(scrn);
 	xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -1594,6 +1608,7 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *dat
 	int i, old_fb_id;
 	uint32_t tiling_flags = 0;
 	int height;
+	drmmode_flipevtcarrier_ptr flipcarrier;
 
 	if (info->allowColorTiling) {
 		if (info->ChipFamily >= CHIP_FAMILY_R600)
@@ -1623,6 +1638,10 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *dat
 	 * Also, flips queued on disabled or incorrectly configured displays
 	 * may never complete; this is a configuration error.
 	 */
+	drmmode->fe_frame = 0;
+	drmmode->fe_tv_sec = 0;
+	drmmode->fe_tv_usec = 0;
+
 	for (i = 0; i < config->num_crtc; i++) {
 		if (!config->crtc[i]->enabled)
 			continue;
@@ -1630,10 +1649,25 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *dat
 		drmmode->event_data = data;
 		drmmode->flip_count++;
 		drmmode_crtc = config->crtc[i]->driver_private;
+
+		flipcarrier = calloc(1, sizeof(drmmode_flipevtcarrier_rec));
+		if (!flipcarrier) {
+			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+				   "flip queue: carrier alloc failed.\n");
+			goto error_undo;
+		}
+
+		/* Only the reference crtc will finally deliver its page flip
+		 * completion event. All other crtc's events will be discarded.
+		 */
+		flipcarrier->dispatch_me = (drmmode_crtc->hw_id == ref_crtc_hw_id);
+		flipcarrier->drmmode = drmmode;
+
 		if (drmModePageFlip(drmmode->fd, drmmode_crtc->mode_crtc->crtc_id,
-				    drmmode->fb_id, DRM_MODE_PAGE_FLIP_EVENT, drmmode)) {
+				    drmmode->fb_id, DRM_MODE_PAGE_FLIP_EVENT, flipcarrier)) {
 			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
 				   "flip queue failed: %s\n", strerror(errno));
+			free(flipcarrier);
 			goto error_undo;
 		}
 	}
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index c3d3561f..548907bb 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -52,9 +52,17 @@ typedef struct {
   drmEventContext event_context;
   int flip_count;
   void *event_data;
+  unsigned int fe_frame;
+  unsigned int fe_tv_sec;
+  unsigned int fe_tv_usec;
 } drmmode_rec, *drmmode_ptr;
 
 typedef struct {
+  drmmode_ptr drmmode;
+  Bool dispatch_me;
+} drmmode_flipevtcarrier_rec, *drmmode_flipevtcarrier_ptr;
+
+typedef struct {
     drmmode_ptr drmmode;
     drmModeCrtcPtr mode_crtc;
     int hw_id;
@@ -101,7 +109,7 @@ extern int drmmode_get_height_align(ScrnInfoPtr scrn, uint32_t tiling);
 extern int drmmode_get_pitch_align(ScrnInfoPtr scrn, int bpe, uint32_t tiling);
 extern int drmmode_get_base_align(ScrnInfoPtr scrn, int bpe, uint32_t tiling);
 
-Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data);
+Bool radeon_do_pageflip(ScrnInfoPtr scrn, struct radeon_bo *new_front, void *data, int ref_crtc_hw_id);
 
 #endif
 
diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c
index c28017cd..8b12872a 100644
--- a/src/radeon_dri2.c
+++ b/src/radeon_dri2.c
@@ -571,6 +571,26 @@ radeon_dri2_unref_buffer(BufferPtr buffer)
     }
 }
 
+static int radeon_dri2_drawable_crtc(DrawablePtr pDraw)
+{
+    ScreenPtr pScreen = pDraw->pScreen;
+    ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
+    xf86CrtcPtr crtc;
+    int crtc_id = -1;
+
+    crtc = radeon_pick_best_crtc(pScrn,
+				 pDraw->x,
+				 pDraw->x + pDraw->width,
+				 pDraw->y,
+				 pDraw->y + pDraw->height);
+
+    /* Make sure the CRTC is valid and this is the real front buffer */
+    if (crtc != NULL && !crtc->rotatedData) {
+        crtc_id = drmmode_get_crtc_id(crtc);
+    }
+    return crtc_id;
+}
+
 static Bool
 radeon_dri2_schedule_flip(ScrnInfoPtr scrn, ClientPtr client,
 			  DrawablePtr draw, DRI2BufferPtr front,
@@ -581,6 +601,9 @@ radeon_dri2_schedule_flip(ScrnInfoPtr scrn, ClientPtr client,
     struct radeon_exa_pixmap_priv *exa_priv;
     DRI2FrameEventPtr flip_info;
 
+    /* Main crtc for this drawable shall finally deliver pageflip event. */
+    int ref_crtc_hw_id = radeon_dri2_drawable_crtc(draw);
+
     flip_info = calloc(1, sizeof(DRI2FrameEventRec));
     if (!flip_info)
 	return FALSE;
@@ -596,7 +619,8 @@ radeon_dri2_schedule_flip(ScrnInfoPtr scrn, ClientPtr client,
     /* Page flip the full screen buffer */
     back_priv = back->driverPrivate;
     exa_priv = exaGetPixmapDriverPrivate(back_priv->pixmap);
-    return radeon_do_pageflip(scrn, exa_priv->bo, flip_info);
+
+    return radeon_do_pageflip(scrn, exa_priv->bo, flip_info, ref_crtc_hw_id);
 }
 
 static Bool
@@ -735,26 +759,6 @@ cleanup:
     free(event);
 }
 
-static int radeon_dri2_drawable_crtc(DrawablePtr pDraw)
-{
-    ScreenPtr pScreen = pDraw->pScreen;
-    ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
-    xf86CrtcPtr crtc;
-    int crtc_id = -1;
-
-    crtc = radeon_pick_best_crtc(pScrn,
-				 pDraw->x,
-				 pDraw->x + pDraw->width,
-				 pDraw->y,
-				 pDraw->y + pDraw->height);
-
-    /* Make sure the CRTC is valid and this is the real front buffer */
-    if (crtc != NULL && !crtc->rotatedData) {
-        crtc_id = drmmode_get_crtc_id(crtc);
-    }
-    return crtc_id;
-}
-
 /*
  * Get current frame count and frame count timestamp, based on drawable's
  * crtc.
@@ -952,6 +956,18 @@ void radeon_dri2_flip_event_handler(unsigned int frame, unsigned int tv_sec,
     /* We assume our flips arrive in order, so we don't check the frame */
     switch (flip->type) {
     case DRI2_SWAP:
+	/* Check for too small vblank count of pageflip completion, taking wraparound
+	 * into account. This usually means some defective kms pageflip completion,
+	 * causing wrong (msc, ust) return values and possible visual corruption.
+	 */
+	if ((frame < flip->frame) && (flip->frame - frame < 5)) {
+	    xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+		       "%s: Pageflip completion event has impossible msc %d < target_msc %d\n",
+		        __func__, frame, flip->frame);
+	    /* All-Zero values signal failure of (msc, ust) timestamping to client. */
+	    frame = tv_sec = tv_usec = 0;
+	}
+
 	DRI2SwapComplete(flip->client, drawable, frame, tv_sec, tv_usec,
 			 DRI2_FLIP_COMPLETE, flip->event_complete,
 			 flip->event_data);