On 22.04.2015 09:58, Kenneth Graunke wrote: > 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. > > Signed-off-by: Kenneth Graunke <kenn...@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..3e8e72a 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;
AFAICT the only functional change of this patch is that this case of poll() returning <= 0 is now treated as failure, whereas it was previously treated as success. Not sure that warrants the API change, but anyway I think it would be good to have that one-liner bugfix in a separate change instead of buried in the API change. > + /* 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", > -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel