[PATCH xf86-video-intel 2/2] Fix handling of target_msc, divisor, remainder in I830DRI2ScheduleSwap()

[Mario Kleiner]

The current code in I830DRI2ScheduleSwap() only schedules the
correct vblank events for the case divisor == 0, i.e., the
simple glXSwapBuffers() case.

In a glXSwapBuffersMscOML() request, divisor can be > 0, which would go
wrong.

This modified code should handle target_msc, divisor, remainder and
the different cases defined in the OML_sync_control extension
correctly for the divisor > 0 case.

It also tries to make sure that the effective framecount of swap satisfies
all constraints, taking the 1 frame delay in pageflipping mode and possible
delays in blitting/exchange mode due to DRM_VBLANK_NEXTONMISS into account.

The swap_interval logic in the X-Servers DRI2SwapBuffers() call expects
the returned swap_target from the DDX to be reasonably accurate, otherwise
implementation of swap_interval for the glXSwapBuffers() as defined in the
SGI_swap_interval extension may become unreliable.

For non-pageflipped mode, the returned swap_target is always correct due to the
adjustments done by drmWaitVBlank(), as DRM_VBLANK_NEXTONMISS is set.

In pageflipped mode, DRM_VBLANK_NEXTONMISS can't be used without
severe impact on performance, so the code in I830DRI2ScheduleSwap()
must make manual adjustments to the returned vbl.reply.sequence number.

This patch adds the needed adjustments.

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>
---
 src/i830_dri.c |  110 +++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 70 insertions(+), 40 deletions(-)

diff --git a/src/i830_dri.c b/src/i830_dri.c
index d5e085a..f3cd739 100644
--- a/src/i830_dri.c
+++ b/src/i830_dri.c
@@ -586,6 +586,7 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 	int ret, pipe = I830DRI2DrawablePipe(draw), flip = 0;
 	DRI2FrameEventPtr swap_info;
 	enum DRI2FrameEventType swap_type = DRI2_SWAP;
+	CARD64 current_msc;
 
 	swap_info = xcalloc(1, sizeof(DRI2FrameEventRec));
 
@@ -629,6 +630,8 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 		return FALSE;
 	}
 
+	current_msc = vbl.reply.sequence;
+
 	/* Flips need to be submitted one frame before */
 	if (DRI2CanFlip(draw) && !intel->shadow_present &&
 	    intel->use_pageflipping) {
@@ -638,55 +641,81 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 
 	swap_info->type = swap_type;
 
+	/* Correct target_msc by 'flip' if swap_type == DRI2_FLIP.
+	 * Do it early, so handling of different timing constraints
+	 * for divisor, remainder and msc vs. target_msc works.
+	 */
+	if (*target_msc > 0)
+	    *target_msc -= flip;
+	
 	/*
-	 * If divisor is zero, we just need to make sure target_msc passes
-	 * before waking up the client.
+	 * 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) {
-		vbl.request.type = DRM_VBLANK_NEXTONMISS |
-		    DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
-		if (pipe > 0)
-			vbl.request.type |= DRM_VBLANK_SECONDARY;
-
-		vbl.request.sequence = *target_msc;
-		vbl.request.sequence -= flip;
-		vbl.request.signal = (unsigned long)swap_info;
-		ret = drmWaitVBlank(intel->drmSubFD, &vbl);
-		if (ret) {
-			xf86DrvMsg(scrn->scrnIndex, X_WARNING,
-				   "divisor 0 get vblank counter failed: %s\n",
-				   strerror(errno));
-			return FALSE;
-		}
-
-		*target_msc = vbl.reply.sequence;
-		swap_info->frame = *target_msc;
-
-		return TRUE;
+	if (divisor == 0 || current_msc < *target_msc) {
+	    vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
+	    /* 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 (pipe > 0)
+	        vbl.request.type |= DRM_VBLANK_SECONDARY;
+	    
+	    /* If target_msc already reached or passed, set it to
+	     * current_msc to ensure we return a reasonable value back
+	     * to the caller. This makes swap_interval logic more robust.
+	     */
+	    if (current_msc >= *target_msc)
+	        *target_msc = current_msc;
+	    
+	    vbl.request.sequence = *target_msc;
+	    vbl.request.signal = (unsigned long)swap_info;
+	    ret = drmWaitVBlank(intel->drmSubFD, &vbl);
+	    if (ret) {
+	        xf86DrvMsg(scrn->scrnIndex, X_WARNING,
+			   "divisor 0 get vblank counter failed: %s\n",
+			   strerror(errno));
+		return FALSE;
+	    }
+	    
+	    /* Adjust returned value for 1 frame pageflip offset if flip > 0 */
+	    *target_msc = vbl.reply.sequence + flip;
+	    swap_info->frame = *target_msc;
+	    
+	    return TRUE;
 	}
-
+	
 	/*
 	 * 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/remainderequation.
+	 * and we need to queue an event that will satisfy the divisor/remainder
+	 * equation.
 	 */
-	if ((vbl.reply.sequence % divisor) == remainder)
-		return FALSE;
-
+	
 	vbl.request.type = DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT;
+	if (flip == 0)
+	    vbl.request.type |= DRM_VBLANK_NEXTONMISS;
 	if (pipe > 0)
-		vbl.request.type |= DRM_VBLANK_SECONDARY;
+	    vbl.request.type |= DRM_VBLANK_SECONDARY;
 
+	vbl.request.sequence = current_msc - (current_msc % divisor) + remainder;
+	
 	/*
-	 * If we have no remainder, and the test above failed, it means we've
-	 * passed the last point where seq % divisor == remainder, so we need
-	 * to wait for the next time that will happen.
+	 * If calculated deadline vbl.request.sequence is smaller than or equal to
+	 * current_msc, it means we've passed the last point when effective
+	 * onset frame seq could satisfy seq % divisor == remainder,
+	 * so we need to wait for the next time that this will happen.
+	 *
+	 * This comparison takes the 1 frame swap delay in pageflipping mode into
+	 * account, as well as a potential DRM_VBLANK_NEXTONMISS delay if we are
+	 * blitting/exchanging instead of flipping.
 	 */
-	if (!remainder)
-		vbl.request.sequence += divisor;
-
-	vbl.request.sequence = vbl.reply.sequence -
-	    (vbl.reply.sequence % divisor) + remainder;
+	if (vbl.request.sequence <= current_msc)
+	    vbl.request.sequence += divisor;
+	
+	/* Account for 1 frame extra pageflip delay if flip > 0 */
 	vbl.request.sequence -= flip;
+	
 	vbl.request.signal = (unsigned long)swap_info;
 	ret = drmWaitVBlank(intel->drmSubFD, &vbl);
 	if (ret) {
@@ -695,10 +724,11 @@ I830DRI2ScheduleSwap(ClientPtr client, DrawablePtr draw, DRI2BufferPtr front,
 			   strerror(errno));
 		return FALSE;
 	}
-
-	*target_msc = vbl.reply.sequence;
+	
+	/* Adjust returned value for 1 frame pageflip offset if flip > 0 */
+	*target_msc = vbl.reply.sequence + flip;
 	swap_info->frame = *target_msc;
-
+	
 	return TRUE;
 }
 
-- 
1.6.6