[PATCH weston 2/2] compositor-drm: Fix inconsistency in finish frame timestamps

[Kristian Høgsberg]

On Wed, Feb 13, 2013 at 04:06:38PM +0200, Ander Conselvan de Oliveira wrote:
> The page flip event timestamps comes from the monotonic clock, while
> idle_repaint() gets the time with gettimeofday(). That leads to
> inconsistent timestamps on the frame callbacks.
> 
> Fix this by making the drm backend page flip to the current buffer and
> call weston_output_finish_frame() with the page flip event timestamp,
> instead of using gettimeofday().

Yup, this looks right, but I think we need to require that all
backends provide a start_repaint_loop function.  Only the backend
knows what kind of timestamp comes back in finish_frame, so we can't
assume gttimeofday() as a fallback.  compositor-wayland.c uses the
frame callback timestamp, for example, and if the host weston is
running on KMS, that's a monotonic timestamp from the underlying
pageflip event.

Kristian

> ---
>  src/compositor-drm.c |   34 ++++++++++++++++++++++++++++++----
>  src/compositor.c     |    6 +++++-
>  src/compositor.h     |    1 +
>  3 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index f1f7343..e3ac572 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -629,6 +629,26 @@ drm_output_repaint(struct weston_output *output_base,
>  }
>  
>  static void
> +drm_output_start_repaint_loop(struct weston_output *output_base)
> +{
> +	struct drm_output *output = (struct drm_output *) output_base;
> +	struct drm_compositor *compositor = (struct drm_compositor *)
> +		output_base->compositor;
> +	uint32_t fb_id;
> +
> +	if (output->current)
> +		fb_id = output->current->fb_id;
> +	else
> +		fb_id = output->original_crtc->buffer_id;
> +
> +	if (drmModePageFlip(compositor->drm.fd, output->crtc_id, fb_id,
> +			    DRM_MODE_PAGE_FLIP_EVENT, output) < 0) {
> +		weston_log("queueing pageflip failed: %m\n");
> +		return;
> +	}
> +}
> +
> +static void
>  vblank_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec,
>  	       void *data)
>  {
> @@ -655,11 +675,16 @@ page_flip_handler(int fd, unsigned int frame,
>  	struct drm_output *output = (struct drm_output *) data;
>  	uint32_t msecs;
>  
> -	output->page_flip_pending = 0;
> +	/* We don't set page_flip_pending on start_repaint_loop, in that case
> +	 * we just want to page flip to the current buffer to get an accurate
> +	 * timestamp */
> +	if (output->page_flip_pending) {
> +		drm_output_release_fb(output, output->current);
> +		output->current = output->next;
> +		output->next = NULL;
> +	}
>  
> -	drm_output_release_fb(output, output->current);
> -	output->current = output->next;
> -	output->next = NULL;
> +	output->page_flip_pending = 0;
>  
>  	if (!output->vblank_pending) {
>  		msecs = sec * 1000 + usec / 1000;
> @@ -1636,6 +1661,7 @@ create_output_for_connector(struct drm_compositor *ec,
>  	wl_list_insert(ec->base.output_list.prev, &output->base.link);
>  
>  	output->base.origin = output->base.current;
> +	output->base.start_repaint_loop = drm_output_start_repaint_loop;
>  	output->base.repaint = drm_output_repaint;
>  	output->base.destroy = drm_output_destroy;
>  	output->base.assign_planes = drm_assign_planes;
> diff --git a/src/compositor.c b/src/compositor.c
> index 764d622..a0faba1 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1164,7 +1164,11 @@ idle_repaint(void *data)
>  {
>  	struct weston_output *output = data;
>  
> -	weston_output_finish_frame(output, weston_compositor_get_time());
> +	if (output->start_repaint_loop)
> +		output->start_repaint_loop(output);
> +	else
> +		weston_output_finish_frame(output,
> +					   weston_compositor_get_time());
>  }
>  
>  WL_EXPORT void
> diff --git a/src/compositor.h b/src/compositor.h
> index 004f931..34f81b5 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -181,6 +181,7 @@ struct weston_output {
>  	struct weston_mode *origin;
>  	struct wl_list mode_list;
>  
> +	void (*start_repaint_loop)(struct weston_output *output);
>  	void (*repaint)(struct weston_output *output,
>  			pixman_region32_t *damage);
>  	void (*destroy)(struct weston_output *output);
> -- 
> 1.7.10.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel