[PATCH weston] Make backends always specify output repaint time
Pekka Paalanen
ppaalanen at gmail.com
Sun Apr 7 07:24:51 PDT 2013
On Fri, 5 Apr 2013 23:07:11 +0200
Jonas Ådahl <jadahl at gmail.com> wrote:
> Most backends relies on gettimeofday(2) for output repaint timestamps
> but this is not a requirement. Before this patch repaints coming from
> idle_repaint() always used gettimeofday(2) for timestamps. For backends
> not using that time source this could cause large jumps between
> timestamps.
>
> To fix this, timestamps needs to always come from the backend. This
> means that the backend needs to always be responsible of starting the
> repaint loop in case that the repaint cannot start immediately.
Good idea.
> The drm backend implementation is from the patch by Ander Conselvan de
> Oliveira found here:
> http://lists.freedesktop.org/archives/wayland-devel/2013-February/007393.html
>
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
>
> Hi,
>
> This is a follow up from reviews of Ander's patch[0] from February.
>
> What is done since that patch is to implement the "start_repaint_loop"
> for the current backends found in weston. All but the wayland backend
> implementation are pretty straight forward.
>
> [0] http://lists.freedesktop.org/archives/wayland-devel/2013-February/007393.html
>
> Jonas
>
> src/compositor-drm.c | 34 ++++++++++++++--
> src/compositor-fbdev.c | 18 ++++++---
> src/compositor-headless.c | 12 ++++--
> src/compositor-rdp.c | 19 +++++----
> src/compositor-rpi.c | 27 ++++++++++---
> src/compositor-wayland.c | 97 +++++++++++++++++++++++++++++++++++++++++++++
> src/compositor-x11.c | 20 +++++++---
> src/compositor.c | 2 +-
> src/compositor.h | 1 +
> 9 files changed, 199 insertions(+), 31 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 6e0a126..49f197e 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -623,6 +623,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");
Btw, does this cause one refresh cycle (scanout frame) delay in
starting the repaint loop?
> + return;
> + }
> +}
> +
> +static void
> vblank_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec,
> void *data)
> {
> @@ -649,11 +669,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;
> @@ -1618,6 +1643,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-rpi.c b/src/compositor-rpi.c
> index cae6f7b..43fc14a 100644
> --- a/src/compositor-rpi.c
> +++ b/src/compositor-rpi.c
> @@ -612,19 +612,26 @@ rpi_element_update(struct rpi_element *element,
> return 0;
> }
>
> +static uint64_t
> +rpi_get_current_time(void)
> +{
> + struct timeval tv;
> +
> + /* manufacture flip completion timestamp */
See below.
> + /* XXX: use CLOCK_MONOTONIC instead? */
> + gettimeofday(&tv, NULL);
> + return (uint64_t)tv.tv_sec * 1000 + tv.tv_usec / 1000;
> +}
> +
> static void
> rpi_flippipe_update_complete(DISPMANX_UPDATE_HANDLE_T update, void *data)
> {
> /* This function runs in a different thread. */
> struct rpi_flippipe *flippipe = data;
> - struct timeval tv;
> uint64_t time;
> ssize_t ret;
>
> - /* manufacture flip completion timestamp */
This "manufacture" comment should stay here, not be moved.
Otherwise ok.
> - /* XXX: use CLOCK_MONOTONIC instead? */
> - gettimeofday(&tv, NULL);
> - time = (uint64_t)tv.tv_sec * 1000 + tv.tv_usec / 1000;
> + time = rpi_get_current_time();
>
> ret = write(flippipe->writefd, &time, sizeof time);
> if (ret != sizeof time)
> @@ -885,6 +892,15 @@ rpi_output_destroy_old_elements(struct rpi_output *output)
> }
>
> static void
> +rpi_output_start_repaint_loop(struct weston_output *output)
> +{
> + uint64_t time;
> +
> + time = rpi_get_current_time();
> + weston_output_finish_frame(output, time);
> +}
> +
> +static void
> rpi_output_repaint(struct weston_output *base, pixman_region32_t *damage)
> {
> struct rpi_output *output = to_rpi_output(base);
> @@ -1029,6 +1045,7 @@ rpi_output_create(struct rpi_compositor *compositor)
> output->egl_window.width = modeinfo.width;
> output->egl_window.height = modeinfo.height;
>
> + output->base.start_repaint_loop = rpi_output_start_repaint_loop;
> output->base.repaint = rpi_output_repaint;
> output->base.destroy = rpi_output_destroy;
> if (compositor->max_planes > 0)
The rpi bits look good, apart from that one comment.
> diff --git a/src/compositor.c b/src/compositor.c
> index e7c22db..693df2c 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1308,7 +1308,7 @@ idle_repaint(void *data)
> {
> struct weston_output *output = data;
>
> - weston_output_finish_frame(output, weston_compositor_get_time());
> + output->start_repaint_loop(output);
Very nice to see this get_time() removed.
> }
>
> WL_EXPORT void
> diff --git a/src/compositor.h b/src/compositor.h
> index 7d1d68e..1e999a6 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -186,6 +186,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
I checked only the rpi specific bits, looking good.
Thanks,
pq
More information about the wayland-devel
mailing list