From f6853baebaec6a68078fa7f90486e9547d59dcbd Mon Sep 17 00:00:00 2001 From: Kenneth Graunke Date: Sun, 8 Feb 2015 17:21:58 -0800 Subject: [PATCH] modesetting: Improve the ms_flush_drm_events() API. 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 Signed-off-by: Kenneth Graunke --- 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 359e11316..43df148d1 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; - return drmHandleEvent(ms->fd, &ms->event_context) >= 0; + /* Try to handle the event. If there was an error, return it. */ + r = drmHandleEvent(ms->fd, &ms->event_context); + if (r < 0) + return r; + + /* 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",