[PATCH weston v2 2/2] compositor: add repaint delay timer
Pekka Paalanen
ppaalanen at gmail.com
Thu Mar 19 03:01:08 PDT 2015
On Wed, 18 Mar 2015 14:45:34 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:
> On Wed, Mar 18, 2015 at 03:27:22PM +0200, Pekka Paalanen wrote:
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >
> > This timer delays the output_repaint towards the end of the refresh
> > period, reducing the time from repaint to present.
> >
> > The length of the repaint window can be set in weston.ini.
> >
> > The call to weston_output_schedule_repaint_reset() is delayed by one
> > more period. If we exit the continuous repaint loop (set
> > output->repaint_scheduled to false) in finish_frame, we may call
> > start_repaint_loop() unnecessarily. The problem case was actually
> > observed with two outputs on the DRM backend at 60 Hz, and 7 ms
> > repaint-window. During a window move, one output was constantly falling
> > off the continuous repaint loop and introducing additional one frame
> > latency, leading to jerky window motion. This code now avoids the
> > problem.
> >
> > Changes in v2:
> >
> > - Rename repaint_delay_timer to repaint_timer and
> > output_repaint_delay_handler to output_repaint_timer_handler.
> >
> > - When computing the delay, take the current time into account. The timer
> > uses a relative timeout, so we have to subtract any time already gone.
> >
> > Note, that 'gone' may also be negative. DRM has a habit of predicting
> > the page flip timestamp so it may be still in the future when we get the
> > completion event.
> >
> > - Do also a sanity check 'msec > 1000'. In the unlikely case that
> > something fails to provide a good timestamp, never delay for more than
> > one second.
> >
> > Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> Integer 32 math with numbers up in the billions kind of worries me about
> out-of-range errors, but the math all appears to work out. Might be
> nice to have tests for timespec_sub and timespec_to_nsec anyway just to
> be sure, but the code itself looks ok to land, so:
>
> Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
Yeah, I lifted timespec_sub() from wesgr. There I have much more
rigorous testing of timestamp validity. tv_nsec is of type 'long' which
may be 32-bit (tv_sec *might* be 32-bit too, we have no guarantees).
For valid timestamps, tv_nsec is always in the range [0, 999999999] so
the math is safe for valid timestamps, and produces valid timestamps.
I've been thinking of putting all timespec helpers in a new shared file
with tests and exact documentation, but there isn't enough of them yet.
Thanks,
pq
> > ---
> > man/weston.ini.man | 10 +++++++
> > src/compositor.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++--------
> > src/compositor.h | 2 ++
> > 3 files changed, 84 insertions(+), 11 deletions(-)
> >
> > diff --git a/man/weston.ini.man b/man/weston.ini.man
> > index 7a353c9..d7df0e4 100644
> > --- a/man/weston.ini.man
> > +++ b/man/weston.ini.man
> > @@ -137,6 +137,16 @@ directory are:
> > .fi
> > .RE
> > .TP 7
> > +.BI "repaint-window=" N
> > +Set the approximate length of the repaint window in milliseconds. The repaint
> > +window is used to control and reduce the output latency for clients. If the
> > +window is longer than the output refresh period, the repaint will be done
> > +immediately when the previous repaint finishes, not processing client requests
> > +in between. If the repaint window is too short, the compositor may miss the
> > +target vertical blank, increasing output latency. The default value is 7
> > +milliseconds. The allowed range is from -10 to 1000 milliseconds. Using a
> > +negative value will force the compositor to always miss the target vblank.
> > +.TP 7
> > .BI "gbm-format="format
> > sets the GBM format used for the framebuffer for the GBM backend. Can be
> > .B xrgb8888,
> > diff --git a/src/compositor.c b/src/compositor.c
> > index e060be1..3f6aa7d 100644
> > --- a/src/compositor.c
> > +++ b/src/compositor.c
> > @@ -62,6 +62,28 @@
> > #include "git-version.h"
> > #include "version.h"
> >
> > +#define DEFAULT_REPAINT_WINDOW 7 /* milliseconds */
> > +
> > +#define NSEC_PER_SEC 1000000000
> > +
> > +static void
> > +timespec_sub(struct timespec *r,
> > + const struct timespec *a, const struct timespec *b)
> > +{
> > + r->tv_sec = a->tv_sec - b->tv_sec;
> > + r->tv_nsec = a->tv_nsec - b->tv_nsec;
> > + if (r->tv_nsec < 0) {
> > + r->tv_sec--;
> > + r->tv_nsec += NSEC_PER_SEC;
> > + }
> > +}
> > +
> > +static int64_t
> > +timespec_to_nsec(const struct timespec *a)
> > +{
> > + return (int64_t)a->tv_sec * NSEC_PER_SEC + a->tv_nsec;
> > +}
> > +
> > static struct wl_list child_process_list;
> > static struct weston_compositor *segv_compositor;
> >
> > @@ -2346,19 +2368,38 @@ weston_output_schedule_repaint_reset(struct weston_output *output)
> > weston_compositor_read_input, compositor);
> > }
> >
> > +static int
> > +output_repaint_timer_handler(void *data)
> > +{
> > + struct weston_output *output = data;
> > + struct weston_compositor *compositor = output->compositor;
> > +
> > + if (output->repaint_needed &&
> > + compositor->state != WESTON_COMPOSITOR_SLEEPING &&
> > + compositor->state != WESTON_COMPOSITOR_OFFSCREEN &&
> > + weston_output_repaint(output) == 0)
> > + return 0;
> > +
> > + weston_output_schedule_repaint_reset(output);
> > +
> > + return 0;
> > +}
> > +
> > WL_EXPORT void
> > weston_output_finish_frame(struct weston_output *output,
> > const struct timespec *stamp,
> > uint32_t presented_flags)
> > {
> > struct weston_compositor *compositor = output->compositor;
> > - int r;
> > - uint32_t refresh_nsec;
> > + int32_t refresh_nsec;
> > + struct timespec now;
> > + struct timespec gone;
> > + int msec;
> >
> > TL_POINT("core_repaint_finished", TLP_OUTPUT(output),
> > TLP_VBLANK(stamp), TLP_END);
> >
> > - refresh_nsec = 1000000000000UL / output->current_mode->refresh;
> > + refresh_nsec = 1000000000000LL / output->current_mode->refresh;
> > weston_presentation_feedback_present_list(&output->feedback_list,
> > output, refresh_nsec, stamp,
> > output->msc,
> > @@ -2366,15 +2407,16 @@ weston_output_finish_frame(struct weston_output *output,
> >
> > output->frame_time = stamp->tv_sec * 1000 + stamp->tv_nsec / 1000000;
> >
> > - if (output->repaint_needed &&
> > - compositor->state != WESTON_COMPOSITOR_SLEEPING &&
> > - compositor->state != WESTON_COMPOSITOR_OFFSCREEN) {
> > - r = weston_output_repaint(output);
> > - if (!r)
> > - return;
> > - }
> > + weston_compositor_read_presentation_clock(compositor, &now);
> > + timespec_sub(&gone, &now, stamp);
> > + msec = (refresh_nsec - timespec_to_nsec(&gone)) / 1000000; /* floor */
> > + msec -= compositor->repaint_msec;
> >
> > - weston_output_schedule_repaint_reset(output);
> > + /* Also sanity check. */
> > + if (msec < 1 || msec > 1000)
> > + output_repaint_timer_handler(output);
> > + else
> > + wl_event_source_timer_update(output->repaint_timer, msec);
> > }
> >
> > static void
> > @@ -3873,6 +3915,8 @@ weston_output_destroy(struct weston_output *output)
> >
> > output->destroying = 1;
> >
> > + wl_event_source_remove(output->repaint_timer);
> > +
> > weston_presentation_feedback_discard_list(&output->feedback_list);
> >
> > weston_compositor_remove_output(output->compositor, output);
> > @@ -4033,6 +4077,8 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c,
> > int x, int y, int mm_width, int mm_height, uint32_t transform,
> > int32_t scale)
> > {
> > + struct wl_event_loop *loop;
> > +
> > output->compositor = c;
> > output->x = x;
> > output->y = y;
> > @@ -4053,6 +4099,10 @@ weston_output_init(struct weston_output *output, struct weston_compositor *c,
> > wl_list_init(&output->resource_list);
> > wl_list_init(&output->feedback_list);
> >
> > + loop = wl_display_get_event_loop(c->wl_display);
> > + output->repaint_timer = wl_event_loop_add_timer(loop,
> > + output_repaint_timer_handler, output);
> > +
> > output->id = ffs(~output->compositor->output_id_pool) - 1;
> > output->compositor->output_id_pool |= 1 << output->id;
> >
> > @@ -4517,6 +4567,17 @@ weston_compositor_init(struct weston_compositor *ec,
> > weston_compositor_add_debug_binding(ec, KEY_T,
> > timeline_key_binding_handler, ec);
> >
> > + s = weston_config_get_section(ec->config, "core", NULL, NULL);
> > + weston_config_section_get_int(s, "repaint-window", &ec->repaint_msec,
> > + DEFAULT_REPAINT_WINDOW);
> > + if (ec->repaint_msec < -10 || ec->repaint_msec > 1000) {
> > + weston_log("Invalid repaint_window value in config: %d\n",
> > + ec->repaint_msec);
> > + ec->repaint_msec = DEFAULT_REPAINT_WINDOW;
> > + }
> > + weston_log("Output repaint window is %d ms maximum.\n",
> > + ec->repaint_msec);
> > +
> > weston_compositor_schedule_repaint(ec);
> >
> > return 0;
> > diff --git a/src/compositor.h b/src/compositor.h
> > index 2e6ef9d..a451ba3 100644
> > --- a/src/compositor.h
> > +++ b/src/compositor.h
> > @@ -196,6 +196,7 @@ struct weston_output {
> > pixman_region32_t previous_damage;
> > int repaint_needed;
> > int repaint_scheduled;
> > + struct wl_event_source *repaint_timer;
> > struct weston_output_zoom zoom;
> > int dirty;
> > struct wl_signal frame_signal;
> > @@ -683,6 +684,7 @@ struct weston_compositor {
> > int32_t kb_repeat_delay;
> >
> > clockid_t presentation_clock;
> > + int32_t repaint_msec;
> >
> > int exit_code;
> > };
> > --
> > 2.0.5
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list