On Wed, 1 Mar 2017 11:34:08 +0000 Daniel Stone <[email protected]> wrote:
> In preparation for grouping output repaint together where possible, > switch the per-output repaint timer, to a global timer which iterates > across all outputs. > > This is implemented by storing the absolute time for the next repaint > for each output locally, and maintaining a global timer which iterates > all of them, scheduling the repaint for the first available time. > > Signed-off-by: Daniel Stone <[email protected]> > Cc: Mario Kleiner <[email protected]> > Cc: Pekka Paalanen <[email protected]> > --- > libweston/compositor.c | 113 > ++++++++++++++++++++++++++++++++++++------------- > libweston/compositor.h | 6 ++- > 2 files changed, 88 insertions(+), 31 deletions(-) > > v2: Rebased on top of previous changes (repaint_status enum). > Suggestions from Pekka: > Keep absolute target repaint time as struct timespec, only using > int64_t nsec for relative comparison to current time, both in > maybe_repaint and finish_frame. > Remove unnecessary output_repaint_timer_arm from schedule_repaint > early-exit path. > Use explict bool rather than 0/~0 time to determine if any outputs > are eligible for repaint in the timer. > > diff --git a/libweston/compositor.c b/libweston/compositor.c > index 4aa30da..a64e695 100644 > --- a/libweston/compositor.c > +++ b/libweston/compositor.c > +static void > +output_repaint_timer_arm(struct weston_compositor *compositor) > +{ > + struct weston_output *output; > + bool any_should_repaint = false; > + struct timespec now; > + int64_t msec_to_next; > + > + weston_compositor_read_presentation_clock(compositor, &now); > + > + wl_list_for_each(output, &compositor->output_list, link) { > + int64_t msec_to_this; > + > + if (output->repaint_status != REPAINT_SCHEDULED) > + continue; > + > + msec_to_this = timespec_sub_to_msec(&output->next_repaint, > + &now); > + if (!any_should_repaint || msec_to_this < msec_to_next) > + msec_to_next = msec_to_this; > + > + any_should_repaint = true; > + } > + > + if (!any_should_repaint) > + return; > + > + /* XXX: This can keep postponing forever, maybe? */ > + if (msec_to_next < 1) > + msec_to_next = 1; Hi, I think there is no longer a risk of postponing indefinitely, because output_repaint_timer_arm() is no longer called from weston_output_schedule_repaint(). The other call sites are well throttled. I'm curious about the reason to have at least 1 ms delay, even if weston_output_finish_frame() deemed that we should paint ASAP. Is it to allow coalescing several outputs' finish_frame needing immediate repaint into single call through output_repaint_timer_handler()? Would be really nice to see some explanation in a comment here. Not that it actually matters much, since we are talking about the time instant to start painting, which is not that crucial to begin with. Although, I suppose if one constructed a 16-head monster, each head adding another 1 ms delay could ruin everything depending on how finish_frame calls get timed. That is the only comment I have for this patch, otherwise it is: Reviewed-by: Pekka Paalanen <[email protected]> If there is just a change or removal of that comment, I can do that while merging. > + > + wl_event_source_timer_update(compositor->repaint_timer, msec_to_next); > +} Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
