[PATCH 10/16] modesetting: Improve the ms_flush_drm_events() API.

Dave Airlie airlied at gmail.com
Thu Jun 25 16:51:13 PDT 2015


From: Kenneth Graunke <kenneth at whitecape.org>

Previously, ms_flush_drm_events() returned a boolean value, and it was
very easy to interpret the meaning incorrectly.  Now, we return an
integer value.

The possible outcomes of this call are:
- poll() raised an error (formerly TRUE, now -1 - poll's return value)
- poll() said there are no events (formerly TRUE, now 0).
- drmHandleEvent() raised an error (formerly FALSE, now the negative
  value returned by drmHandleEvent).
- An event was successfully handled (formerly TRUE, now 1).

The nice part is that this allows you to distinguish errors (< 0),
nothing to do (= 0), and success (1).  We no longer conflate errors
with success.

v2: Change ms_present_queue_vblank to < 0 instead of <= 0, fixing an
    unintentional behavior change.  libdrm may return EBUSY if it's
    received EINTR for more than a second straight; just keep retrying
    in that case.  Suggested by Jasper St. Pierre.

Reviewed-by: Dave Airlie <airlied at redhat.com>
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 hw/xfree86/drivers/modesetting/present.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/hw/xfree86/drivers/modesetting/present.c b/hw/xfree86/drivers/modesetting/present.c
index 359e113..43df148 100644
--- a/hw/xfree86/drivers/modesetting/present.c
+++ b/hw/xfree86/drivers/modesetting/present.c
@@ -72,8 +72,11 @@ ms_present_get_ust_msc(RRCrtcPtr crtc, CARD64 *ust, CARD64 *msc)
 
 /*
  * Flush the DRM event queue when full; makes space for new events.
+ *
+ * Returns a negative value on error, 0 if there was nothing to process,
+ * or 1 if we handled any events.
  */
-static Bool
+static int
 ms_flush_drm_events(ScreenPtr screen)
 {
     ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
@@ -86,10 +89,19 @@ ms_flush_drm_events(ScreenPtr screen)
             r = poll(&p, 1, 0);
     } while (r == -1 && (errno == EINTR || errno == EAGAIN));
 
+    /* If there was an error, r will be < 0.  Return that.  If there was
+     * nothing to process, r == 0.  Return that.
+     */
     if (r <= 0)
-        return TRUE;
+        return r;
+
+    /* Try to handle the event.  If there was an error, return it. */
+    r = drmHandleEvent(ms->fd, &ms->event_context);
+    if (r < 0)
+        return r;
 
-    return drmHandleEvent(ms->fd, &ms->event_context) >= 0;
+    /* Otherwise return 1 to indicate that we handled an event. */
+    return 1;
 }
 
 /*
@@ -159,7 +171,10 @@ ms_present_queue_vblank(RRCrtcPtr crtc,
         ret = drmWaitVBlank(ms->fd, &vbl);
         if (!ret)
             break;
-        if (errno != EBUSY || !ms_flush_drm_events(screen))
+        /* If we hit EBUSY, then try to flush events.  If we can't, then
+         * this is an error.
+         */
+        if (errno != EBUSY || ms_flush_drm_events(screen) < 0)
             return BadAlloc;
     }
     DebugPresent(("\t\tmq %lld seq %u msc %llu (hw msc %u)\n",
-- 
2.4.3



More information about the xorg-devel mailing list