[PATCH v2 2/2] modesetting: Add vblank synchronization support when using Present.

Fredrik Höglund fredrik at kde.org
Tue Dec 16 15:37:55 PST 2014


On Tuesday 16 December 2014, Jason Ekstrand wrote:
> Yeah... Still not working for me.  Not sure why mutter is different than
> KWin, but I still get black after the screen lock and, when I restart
> gnome-shell it's tearing like crazy.
> --Jason

KWin doesn't use GLX_INTEL_swap_event (although that's about to change),
so it's possible it's still an issue with the swap events.

> On Tue, Dec 16, 2014 at 12:40 AM, Kenneth Graunke <kenneth at whitecape.org>
> wrote:
> >
> > modesetting hooked up vblank support for DRI2, but was missing support
> > for vblanks in Present.
> >
> > This is mostly copy and pasted from Keith's code in the intel driver.
> >
> > v2: Use ms_crtc_msc_to_kernel_msc in ms_present_queue_vblank to hook
> >     up the vblank_offset workaround for bogus MSC values (which the
> >     DRI2 code already did).  Without this, Jason Ekstrand and I saw
> >     compositor lockups (stuck waiting for an event that never came)
> >     when doing lid close or suspend.
> >
> >     Also simplify the ms_present_get_crtc function.  vblank.c already
> >     implements the functionality; we just need to convert types.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > Reviewed-by: Keith Packard <keithp at keithp.com> [v1]
> > ---
> >  hw/xfree86/drivers/modesetting/Makefile.am |   1 +
> >  hw/xfree86/drivers/modesetting/driver.c    |   5 +
> >  hw/xfree86/drivers/modesetting/driver.h    |   6 +
> >  hw/xfree86/drivers/modesetting/present.c   | 221
> > +++++++++++++++++++++++++++++
> >  hw/xfree86/drivers/modesetting/vblank.c    |  18 +++
> >  5 files changed, 251 insertions(+)
> >  create mode 100644 hw/xfree86/drivers/modesetting/present.c
> >
> > This could use more testing.  I could have sworn the vblank_offset
> > workarounds didn't help when I tested them yesterday, but everything
> > seems to be working fine.  Jason, can you try this out and see if it
> > helps you at all?  Thanks!
> >
> > If anyone else wants to give it a try, I'd appreciate it.
> >
> > These patches are also available in the 'ms-present-vblank' branch of
> > git://people.freedesktop.org:~kwg/xserver
> >
> > diff --git a/hw/xfree86/drivers/modesetting/Makefile.am
> > b/hw/xfree86/drivers/modesetting/Makefile.am
> > index 921ca00..82c4f2f 100644
> > --- a/hw/xfree86/drivers/modesetting/Makefile.am
> > +++ b/hw/xfree86/drivers/modesetting/Makefile.am
> > @@ -50,6 +50,7 @@ modesetting_drv_la_SOURCES = \
> >          drmmode_display.h \
> >          dumb_bo.c \
> >          dumb_bo.h \
> > +        present.c \
> >          vblank.c \
> >          $(NULL)
> >
> > diff --git a/hw/xfree86/drivers/modesetting/driver.c
> > b/hw/xfree86/drivers/modesetting/driver.c
> > index 476c814..5dda96b 100644
> > --- a/hw/xfree86/drivers/modesetting/driver.c
> > +++ b/hw/xfree86/drivers/modesetting/driver.c
> > @@ -1113,6 +1113,11 @@ ScreenInit(ScreenPtr pScreen, int argc, char **argv)
> >              xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
> >                         "Failed to initialize the DRI2 extension.\n");
> >          }
> > +
> > +        if (!ms_present_screen_init(pScreen)) {
> > +            xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
> > +                       "Failed to initialize the Present extension.\n");
> > +        }
> >      }
> >  #endif
> >
> > diff --git a/hw/xfree86/drivers/modesetting/driver.h
> > b/hw/xfree86/drivers/modesetting/driver.h
> > index 4267fa1..3decc3e 100644
> > --- a/hw/xfree86/drivers/modesetting/driver.h
> > +++ b/hw/xfree86/drivers/modesetting/driver.h
> > @@ -114,6 +114,10 @@ uint32_t ms_drm_queue_alloc(xf86CrtcPtr crtc,
> >                              ms_drm_handler_proc handler,
> >                              ms_drm_abort_proc abort);
> >
> > +void ms_drm_abort(ScrnInfoPtr scrn,
> > +                  Bool (*match)(void *data, void *match_data),
> > +                  void *match_data);
> > +
> >  xf86CrtcPtr ms_dri2_crtc_covering_drawable(DrawablePtr pDraw);
> >  xf86CrtcPtr ms_covering_crtc(ScrnInfoPtr scrn, BoxPtr box,
> >                               xf86CrtcPtr desired, BoxPtr crtc_box_ret);
> > @@ -129,3 +133,5 @@ void ms_dri2_close_screen(ScreenPtr screen);
> >
> >  Bool ms_vblank_screen_init(ScreenPtr screen);
> >  void ms_vblank_close_screen(ScreenPtr screen);
> > +
> > +Bool ms_present_screen_init(ScreenPtr screen);
> > diff --git a/hw/xfree86/drivers/modesetting/present.c
> > b/hw/xfree86/drivers/modesetting/present.c
> > new file mode 100644
> > index 0000000..55c80e9
> > --- /dev/null
> > +++ b/hw/xfree86/drivers/modesetting/present.c
> > @@ -0,0 +1,221 @@
> > +/*
> > + * Copyright © 2014 Intel Corporation
> > + *
> > + * Permission to use, copy, modify, distribute, and sell this software
> > and its
> > + * documentation for any purpose is hereby granted without fee, provided
> > that
> > + * the above copyright notice appear in all copies and that both that
> > copyright
> > + * notice and this permission notice appear in supporting documentation,
> > and
> > + * that the name of the copyright holders not be used in advertising or
> > + * publicity pertaining to distribution of the software without specific,
> > + * written prior permission.  The copyright holders make no
> > representations
> > + * about the suitability of this software for any purpose.  It is
> > provided "as
> > + * is" without express or implied warranty.
> > + *
> > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > SOFTWARE,
> > + * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
> > + * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT
> > OR
> > + * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF
> > USE,
> > + * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
> > + * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
> > PERFORMANCE
> > + * OF THIS SOFTWARE.
> > + */
> > +
> > +#ifdef HAVE_DIX_CONFIG_H
> > +#include "dix-config.h"
> > +#endif
> > +
> > +#include <assert.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <poll.h>
> > +#include <unistd.h>
> > +#include <stdio.h>
> > +#include <stdint.h>
> > +#include <string.h>
> > +#include <sys/ioctl.h>
> > +#include <sys/time.h>
> > +#include <sys/types.h>
> > +#include <time.h>
> > +
> > +#include <xf86.h>
> > +#include <xf86Crtc.h>
> > +#include <xf86drm.h>
> > +#include <xf86str.h>
> > +#include <present.h>
> > +
> > +#include "driver.h"
> > +
> > +#if 0
> > +#define DebugPresent(x) ErrorF x
> > +#else
> > +#define DebugPresent(x)
> > +#endif
> > +
> > +struct ms_present_vblank_event {
> > +    uint64_t        event_id;
> > +};
> > +
> > +static RRCrtcPtr
> > +ms_present_get_crtc(WindowPtr window)
> > +{
> > +    xf86CrtcPtr xf86_crtc =
> > ms_dri2_crtc_covering_drawable(&window->drawable);
> > +    return xf86_crtc ? xf86_crtc->randr_crtc : NULL;
> > +}
> > +
> > +static int
> > +ms_present_get_ust_msc(RRCrtcPtr crtc, CARD64 *ust, CARD64 *msc)
> > +{
> > +    xf86CrtcPtr xf86_crtc = crtc->devPrivate;
> > +
> > +    return ms_get_crtc_ust_msc(xf86_crtc, ust, msc);
> > +}
> > +
> > +/*
> > + * Flush the DRM event queue when full; makes space for new events.
> > + */
> > +static Bool
> > +ms_flush_drm_events(ScreenPtr screen)
> > +{
> > +    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
> > +    modesettingPtr ms = modesettingPTR(scrn);
> > +
> > +    struct pollfd p = { .fd = ms->fd, .events = POLLIN };
> > +    int r;
> > +
> > +    do {
> > +            r = poll(&p, 1, 0);
> > +    } while (r == -1 && (errno == EINTR || errno == EAGAIN));
> > +
> > +    if (r <= 0)
> > +        return 0;
> > +
> > +    return drmHandleEvent(ms->fd, &ms->event_context);
> > +}
> > +
> > +/*
> > + * Called when the queued vblank event has occurred
> > + */
> > +static void
> > +ms_present_vblank_handler(uint64_t msc, uint64_t usec, void *data)
> > +{
> > +    struct ms_present_vblank_event *event = data;
> > +
> > +    present_event_notify(event->event_id, usec, msc);
> > +    free(event);
> > +}
> > +
> > +/*
> > + * Called when the queued vblank is aborted
> > + */
> > +static void
> > +ms_present_vblank_abort(void *data)
> > +{
> > +    struct ms_present_vblank_event *event = data;
> > +
> > +    free(event);
> > +}
> > +
> > +/*
> > + * Queue an event to report back to the Present extension when the
> > specified
> > + * MSC has past
> > + */
> > +static int
> > +ms_present_queue_vblank(RRCrtcPtr crtc,
> > +                        uint64_t event_id,
> > +                        uint64_t msc)
> > +{
> > +    xf86CrtcPtr xf86_crtc = crtc->devPrivate;
> > +    ScreenPtr screen = crtc->pScreen;
> > +    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
> > +    modesettingPtr ms = modesettingPTR(scrn);
> > +    drmmode_crtc_private_ptr drmmode_crtc = xf86_crtc->driver_private;
> > +    struct ms_present_vblank_event *event;
> > +    drmVBlank vbl;
> > +    int ret;
> > +    uint32_t seq;
> > +
> > +    event = calloc(sizeof(struct ms_present_vblank_event), 1);
> > +    if (!event)
> > +        return BadAlloc;
> > +    event->event_id = event_id;
> > +    seq = ms_drm_queue_alloc(xf86_crtc, event,
> > +                             ms_present_vblank_handler,
> > +                             ms_present_vblank_abort);
> > +    if (!seq) {
> > +        free(event);
> > +        return BadAlloc;
> > +    }
> > +
> > +    vbl.request.type =
> > +        DRM_VBLANK_ABSOLUTE | DRM_VBLANK_EVENT |
> > drmmode_crtc->vblank_pipe;
> > +    vbl.request.sequence = ms_crtc_msc_to_kernel_msc(xf86_crtc, msc);
> > +    vbl.request.signal = seq;
> > +    for (;;) {
> > +        ret = drmWaitVBlank(ms->fd, &vbl);
> > +        if (!ret)
> > +            break;
> > +        if (errno != EBUSY || !ms_flush_drm_events(screen))
> > +            return BadAlloc;
> > +    }
> > +    DebugPresent(("\t\tiq %lld seq %u msc %llu (hw msc %u)\n",
> > +                 (long long) event_id, seq, (long long) msc,
> > +                 vbl.request.sequence));
> > +    return Success;
> > +}
> > +
> > +static Bool
> > +ms_present_event_match(void *data, void *match_data)
> > +{
> > +    struct ms_present_vblank_event *event = data;
> > +    uint64_t *match = match_data;
> > +
> > +    return *match == event->event_id;
> > +}
> > +
> > +/*
> > + * Remove a pending vblank event from the DRM queue so that it is not
> > reported
> > + * to the extension
> > + */
> > +static void
> > +ms_present_abort_vblank(RRCrtcPtr crtc, uint64_t event_id, uint64_t msc)
> > +{
> > +    ScreenPtr screen = crtc->pScreen;
> > +    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
> > +
> > +    ms_drm_abort(scrn, ms_present_event_match, &event_id);
> > +}
> > +
> > +/*
> > + * Flush our batch buffer when requested by the Present extension.
> > + */
> > +static void
> > +ms_present_flush(WindowPtr window)
> > +{
> > +    ScreenPtr screen = window->drawable.pScreen;
> > +    ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
> > +    modesettingPtr ms = modesettingPTR(scrn);
> > +
> > +    if (ms->drmmode.glamor)
> > +        glamor_block_handler(screen);
> > +}
> > +
> > +static present_screen_info_rec ms_present_screen_info = {
> > +    .version = PRESENT_SCREEN_INFO_VERSION,
> > +
> > +    .get_crtc = ms_present_get_crtc,
> > +    .get_ust_msc = ms_present_get_ust_msc,
> > +    .queue_vblank = ms_present_queue_vblank,
> > +    .abort_vblank = ms_present_abort_vblank,
> > +    .flush = ms_present_flush,
> > +
> > +    .capabilities = PresentCapabilityNone,
> > +    .check_flip = 0,
> > +    .flip = 0,
> > +    .unflip = 0,
> > +};
> > +
> > +Bool
> > +ms_present_screen_init(ScreenPtr screen)
> > +{
> > +    return present_screen_init(screen, &ms_present_screen_info);
> > +}
> > diff --git a/hw/xfree86/drivers/modesetting/vblank.c
> > b/hw/xfree86/drivers/modesetting/vblank.c
> > index 5031ef8..c123205 100644
> > --- a/hw/xfree86/drivers/modesetting/vblank.c
> > +++ b/hw/xfree86/drivers/modesetting/vblank.c
> > @@ -323,6 +323,24 @@ ms_drm_abort_scrn(ScrnInfoPtr scrn)
> >  }
> >
> >  /*
> > + * Externally usable abort function that uses a callback to match a single
> > + * queued entry to abort
> > + */
> > +void
> > +ms_drm_abort(ScrnInfoPtr scrn, Bool (*match)(void *data, void
> > *match_data),
> > +             void *match_data)
> > +{
> > +    struct ms_drm_queue *q;
> > +
> > +    xorg_list_for_each_entry(q, &ms_drm_queue, list) {
> > +        if (match(q->data, match_data)) {
> > +            ms_drm_abort_one(q);
> > +            break;
> > +        }
> > +    }
> > +}
> > +
> > +/*
> >   * General DRM kernel handler. Looks for the matching sequence number in
> > the
> >   * drm event queue and calls the handler for it.
> >   */
> > --
> > 2.1.3
> >
> >
> 



More information about the xorg-devel mailing list