[PATCH weston v2] compositor-drm: pageflip timeout implementation

[Daniel Stone]

Hi Emmanuel,

On 6 January 2017 at 21:17, Emmanuel Gil Peyrot
<emmanuel.peyrot at collabora.com> 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 <daniels at collabora.com>

Cheers,
Daniel