present: Handle wraparound when comparing MSC values

When a window moves from one CRTC to another, present_window_to_crtc_msc
updates window_priv->msc_offset according to the delta between the
current MSC values of the old and new CRTC:

            window_priv->msc_offset += new_msc - old_msc;

window_priv->msc_offset is initially 0, so if new_msc < old_msc,
window_priv->msc_offset wraps around and becomes a large number. If the
window_msc parameter passed in is small (in particular if it's 0, such as
is the case when the client just wants to know the current window MSC
value), the returned CRTC MSC value may still be a large number. In that
case, the existing MSC comparisons in pixmap_present weren't working as
intended, resulting in scheduling a wait far into the future when the
target MSC had actually already passed. This would result in the client
(e.g. the Chromium browser) hanging when moving its window between CRTCs.

In order to fix this, introduce msc_is_(equal_or_)after helper functions
which take the wraparound into account for comparing two MSC values.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
Signed-off-by: Keith Packard <keithp@keithp.com>

1 files changed, 27 insertions, 5 deletions

diff --git a/present/present.c b/present/present.c
index 66e0f21b3..8cf3b6f77 100644
--- a/present/present.c
+++ b/present/present.c
@@ -717,6 +717,28 @@ present_execute(present_vblank_ptr vblank, uint64_t ust, uint64_t crtc_msc)
     present_vblank_destroy(vblank);
 }
 
+/*
+ * Returns:
+ * TRUE if the first MSC value is after the second one
+ * FALSE if the first MSC value is equal to or before the second one
+ */
+static Bool
+msc_is_after(uint64_t test, uint64_t reference)
+{
+    return (int64_t)(test - reference) > 0;
+}
+
+/*
+ * Returns:
+ * TRUE if the first MSC value is equal to or after the second one
+ * FALSE if the first MSC value is before the second one
+ */
+static Bool
+msc_is_equal_or_after(uint64_t test, uint64_t reference)
+{
+    return (int64_t)(test - reference) >= 0;
+}
+
 int
 present_pixmap(WindowPtr window,
                PixmapPtr pixmap,
@@ -771,14 +793,14 @@ present_pixmap(WindowPtr window,
 
     /* Adjust target_msc to match modulus
      */
-    if (crtc_msc >= target_msc) {
+    if (msc_is_equal_or_after(crtc_msc, target_msc)) {
         if (divisor != 0) {
             target_msc = crtc_msc - (crtc_msc % divisor) + remainder;
             if (options & PresentOptionAsync) {
-                if (target_msc < crtc_msc)
+                if (msc_is_after(crtc_msc, target_msc))
                     target_msc += divisor;
             } else {
-                if (target_msc <= crtc_msc)
+                if (msc_is_equal_or_after(crtc_msc, target_msc))
                     target_msc += divisor;
             }
         } else {
@@ -864,7 +886,7 @@ present_pixmap(WindowPtr window,
     if (pixmap != NULL &&
         !(options & PresentOptionCopy) &&
         screen_priv->info) {
-        if (target_msc > crtc_msc &&
+        if (msc_is_after(target_msc, crtc_msc) &&
             present_check_flip (target_crtc, window, pixmap, TRUE, valid, x_off, y_off))
         {
             vblank->flip = TRUE;
@@ -897,7 +919,7 @@ present_pixmap(WindowPtr window,
 
     xorg_list_add(&vblank->event_queue, &present_exec_queue);
     vblank->queued = TRUE;
-    if (target_msc > crtc_msc) {
+    if (msc_is_after(target_msc, crtc_msc)) {
         ret = present_queue_vblank(screen, target_crtc, vblank->event_id, target_msc);
         if (ret == Success)
             return Success;