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 <[email protected]>
---
 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

_______________________________________________
xorg-devel mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to