Hi Emmanuel,

On 6 January 2017 at 21:17, Emmanuel Gil Peyrot
<[email protected]> wrote:
> @@ -858,6 +907,10 @@ vblank_handler(int fd, unsigned int frame, unsigned int 
> sec, unsigned int usec,
>         s->current = s->next;
>         s->next = NULL;
>
> +       /* Stop the pageflip timer instead of rearming it here */
> +       if (output->pageflip_timer)
> +               wl_event_source_timer_update(output->pageflip_timer, 0);
> +
>         if (!output->page_flip_pending) {
>                 ts.tv_sec = sec;
>                 ts.tv_nsec = usec * 1000;
> @@ -891,6 +944,10 @@ page_flip_handler(int fd, unsigned int frame,
>
>         output->page_flip_pending = 0;
>
> +       /* Stop the pageflip timer instead of rearming it here */
> +       if (output->pageflip_timer)
> +               wl_event_source_timer_update(output->pageflip_timer, 0);
> +
>         if (output->destroy_pending)
>                 drm_output_destroy(&output->base);
>         else if (output->disable_pending)

Can you please move both of these into the bottom negative conditional
where we call weston_output_finish_frame? i.e. in vblank_handler, we
should only disarm the timer if (!output->page_flip_pending), and in
the pageflip handler, we should only disarm if
(!output->vblank_pending). This makes sure that if using legacy plane
updates (sigh), we wait for both of the pageflip event for the primary
plane, and the vblank events for the planes, to arrive before
disarming the timer. Without doing that, we could get a false
negative: if the pageflip completion arrives but not the vblank event,
we'll disarm the event, but not actually resume repaint.

With that fixed:
Reviewed-by: Daniel Stone <[email protected]>

Cheers,
Daniel
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to