Hi, On 21 June 2015 at 20:25, Mario Kleiner <[email protected]> wrote: > drm_output_start_repaint_loop() incurred a delay of > one refresh cycle by using a no-op page-flip to get > an accurate vblank timestamp as reference. This causes > unwanted lag whenever Weston exited its repaint loop, e.g., > whenever an application wants to repaint with less than > full video refresh rate but still minimum lag. > > Try to use the drmWaitVblank ioctl to get a proper > timestamp instantaneously without lag. If that does > not work, fall back to the old method of idle page-flip. > > This optimization will work on any drm/kms driver > which supports high precision vblank timestamping. > As of Linux 4.0 these would be intel, radeon and > nouveau on all their supported gpu's. > > On kms drivers without instant high precision timestamping > support, the kernel is supposed to return a timestamp > of zero when calling drmWaitVblank() to query the current > vblank count and time iff vblank irqs are currently > disabled, because the only way to get a valid timestamp > on such kms drivers is to enable vblank interrupts and > then wait a bit for the next vblank irq to take a new valid > timestamp. The caller is supposed to poll until at next > vblank irq it gets a valid non-zero timestamp if it needs > a timestamp. > > This zero-timestamp signalling works up to Linux 3.17, but > got broken due to a regression in Linux 3.18 and later. On > Linux 3.18+ with kms drivers that don't have high precision > timestamping, the kernel erroneously returns a stale timestamp > from an earlier vblank, ie. the vblank count and timestamp are > mismatched. A patch is under way to fix this, but to deal with > broken kernels, we also check non-zero timestamps if they are > more than one refresh duration in the past, as this indicates > a stale/invalid timestamp, so we need to take the page-flip > fallback for restarting the repaint loop. > > v2: Implement review suggestions by Pekka Paalanen, especially > extend the commit message to describe when and why the > instant restart won't work due to missing Linux kernel > functionality or a Linux kernel regression. > > Signed-off-by: Mario Kleiner <[email protected]> > --- > src/compositor-drm.c | 41 ++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 40 insertions(+), 1 deletion(-) > > diff --git a/src/compositor-drm.c b/src/compositor-drm.c > index aa6d010..5035611 100644 > --- a/src/compositor-drm.c > +++ b/src/compositor-drm.c > @@ -229,6 +229,9 @@ static const char default_seat[] = "seat0"; > static void > drm_output_set_cursor(struct drm_output *output); > > +static void > +drm_output_update_msc(struct drm_output *output, unsigned int seq); > + > static int > drm_sprite_crtc_supported(struct drm_output *output, uint32_t supported) > { > @@ -708,6 +711,12 @@ err_pageflip: > return -1; > } > > +static int64_t > +timespec_to_nsec(const struct timespec *a) > +{ > + return (int64_t)a->tv_sec * 1000000000000LL + a->tv_nsec; > +} > + > static void > drm_output_start_repaint_loop(struct weston_output *output_base) > { > @@ -715,7 +724,13 @@ drm_output_start_repaint_loop(struct weston_output > *output_base) > struct drm_compositor *compositor = (struct drm_compositor *) > output_base->compositor; > uint32_t fb_id; > - struct timespec ts; > + struct timespec ts, tnow; > + int ret; > + drmVBlank vbl = { > + .request.type = DRM_VBLANK_RELATIVE, > + .request.sequence = 0, > + .request.signal = 0, > + }; > > if (output->destroy_pending) > return; > @@ -725,6 +740,30 @@ drm_output_start_repaint_loop(struct weston_output > *output_base) > goto finish_frame; > } > > + /* Try to get current msc and timestamp via instant query */ > + vbl.request.type |= drm_waitvblank_pipe(output); > + ret = drmWaitVBlank(compositor->drm.fd, &vbl); > + > + /* Error return or zero timestamp means failure to get valid > timestamp */ > + if ((ret == 0) && (vbl.reply.tval_sec > 0 || vbl.reply.tval_usec > > 0)) { > + ts.tv_sec = vbl.reply.tval_sec; > + ts.tv_nsec = vbl.reply.tval_usec * 1000; > + > + /* Valid timestamp for most recent vblank - not stale? Stale > ts could > + * happen on Linux 3.17+, so make sure it is not older than 1 > refresh > + * duration since now. > + */ > + weston_compositor_read_presentation_clock(&compositor->base, > &tnow); > + if (timespec_to_nsec(&tnow) - timespec_to_nsec(&ts) < > + (1000000000000LL / > output_base->current_mode->refresh)) {
I must admit I lose track of which elements are in which units, and the conversions between them with huge hardcoded numbers. I'd be far more comfortable with, e.g. millihz_to_nsec(output_base->current_mode->refresh). That notwithstanding: Reviewed-by: Daniel Stone <[email protected]> I don't know the shell well enough to review those patches, and I'd also like Pekka to look at the repaint-deadline one if possible. But this one seems OK, as well as the others that Bryce pushed. Thanks! Cheers, Daniel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
