[PATCH weston v2 1/2] compositor, backends: weston_compositor_read_presentation_clock
Bryce Harrington
bryce at osg.samsung.com
Wed Mar 18 10:58:37 PDT 2015
On Wed, Mar 18, 2015 at 03:27:21PM +0200, Pekka Paalanen wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> Create a new function weston_compositor_read_presentation_clock() to
> wrap the clock_gettime() call for the Presentation clock.
>
> Reading the presentation clock is never supposed to fail, but if it
> does, this will notify about it. I have not seen it fail yet, though.
>
> This prepares for new testing features in the future that might allow
> controlling the presentation clock. Right now it is just a convenience
> function for clock_gettime().
>
> All presentation clock readers are converted to call this new function
> except rpi-backend's rpi_flippipe_update_complete(), because it gets its
> clock id via a thread-safe mechanism. There shouldn't be anything really
> thread-unsafe in weston_compositor_read_presentation_clock() at the
> moment, but might be in the future, and weston core is not expected to
> need to be thread-safe.
>
> This is based on the original patch by
> Cc: Derek Foreman <derekf at osg.samsung.com>
>
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> ---
> src/compositor-drm.c | 2 +-
> src/compositor-fbdev.c | 4 ++--
> src/compositor-headless.c | 4 ++--
> src/compositor-rdp.c | 4 ++--
> src/compositor-rpi.c | 2 +-
> src/compositor-x11.c | 4 ++--
> src/compositor.c | 33 +++++++++++++++++++++++++++++++++
> src/compositor.h | 5 +++++
> 8 files changed, 48 insertions(+), 10 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index ed4eabf..15152b0 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -723,7 +723,7 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
>
> finish_frame:
> /* if we cannot page-flip, immediately finish frame */
> - clock_gettime(compositor->base.presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(&compositor->base, &ts);
> weston_output_finish_frame(output_base, &ts,
> PRESENTATION_FEEDBACK_INVALID);
> }
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index eedf252..b4c1390 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -117,7 +117,7 @@ fbdev_output_start_repaint_loop(struct weston_output *output)
> {
> struct timespec ts;
>
> - clock_gettime(output->compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->compositor, &ts);
> weston_output_finish_frame(output, &ts, PRESENTATION_FEEDBACK_INVALID);
> }
>
> @@ -223,7 +223,7 @@ finish_frame_handler(void *data)
> struct fbdev_output *output = data;
> struct timespec ts;
>
> - clock_gettime(output->base.compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->base.compositor, &ts);
> weston_output_finish_frame(&output->base, &ts, 0);
>
> return 1;
> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
> index d4d25fa..1b1d327 100644
> --- a/src/compositor-headless.c
> +++ b/src/compositor-headless.c
> @@ -58,7 +58,7 @@ headless_output_start_repaint_loop(struct weston_output *output)
> {
> struct timespec ts;
>
> - clock_gettime(output->compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->compositor, &ts);
> weston_output_finish_frame(output, &ts, PRESENTATION_FEEDBACK_INVALID);
> }
>
> @@ -68,7 +68,7 @@ finish_frame_handler(void *data)
> struct headless_output *output = data;
> struct timespec ts;
>
> - clock_gettime(output->base.compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->base.compositor, &ts);
> weston_output_finish_frame(&output->base, &ts, 0);
>
> return 1;
> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index 1c3f988..6955d49 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -307,7 +307,7 @@ rdp_output_start_repaint_loop(struct weston_output *output)
> {
> struct timespec ts;
>
> - clock_gettime(output->compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->compositor, &ts);
> weston_output_finish_frame(output, &ts, PRESENTATION_FEEDBACK_INVALID);
> }
>
> @@ -353,7 +353,7 @@ finish_frame_handler(void *data)
> struct rdp_output *output = data;
> struct timespec ts;
>
> - clock_gettime(output->base.compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->base.compositor, &ts);
> weston_output_finish_frame(&output->base, &ts, 0);
>
> return 1;
> diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
> index 2d72686..08d5105 100644
> --- a/src/compositor-rpi.c
> +++ b/src/compositor-rpi.c
> @@ -217,7 +217,7 @@ rpi_output_start_repaint_loop(struct weston_output *output)
> struct timespec ts;
>
> /* XXX: do a phony dispmanx update and trigger on its completion? */
> - clock_gettime(output->compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->compositor, &ts);
> weston_output_finish_frame(output, &ts, PRESENTATION_FEEDBACK_INVALID);
> }
>
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index e9735c5..e89acb7 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -346,7 +346,7 @@ x11_output_start_repaint_loop(struct weston_output *output)
> {
> struct timespec ts;
>
> - clock_gettime(output->compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->compositor, &ts);
> weston_output_finish_frame(output, &ts, PRESENTATION_FEEDBACK_INVALID);
> }
>
> @@ -458,7 +458,7 @@ finish_frame_handler(void *data)
> struct x11_output *output = data;
> struct timespec ts;
>
> - clock_gettime(output->base.compositor->presentation_clock, &ts);
> + weston_compositor_read_presentation_clock(output->base.compositor, &ts);
> weston_output_finish_frame(&output->base, &ts, 0);
>
> return 1;
> diff --git a/src/compositor.c b/src/compositor.c
> index 60b7ee4..e060be1 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -4618,6 +4618,39 @@ weston_compositor_set_presentation_clock_software(
> return -1;
> }
>
> +/** Read the current time from the Presentation clock
> + *
> + * \param compositor
> + * \param ts[out] The current time.
> + *
> + * \note Reading the current time in user space is always imprecise to some
> + * degree.
> + *
> + * This function is never meant to fail. If reading the clock does fail,
> + * an error message is logged and a zero time is returned. Callers are not
> + * supposed to detect or react to failures.
If 0 time is returned, are we certain that's not going to screw up
callers in some unexpected way?
If it's never meant to fail, and we don't want to propagate error codes
from here, why not assert()?
Otherwise, seems to me like there'd be little harm propagating the error
by returning false. The current callers do ignore clock_gettime's
return, but maybe some test someday will want to test conditions where
clock_gettime wouldn't work properly...
> + */
> +WL_EXPORT void
> +weston_compositor_read_presentation_clock(
> + const struct weston_compositor *compositor,
> + struct timespec *ts)
> +{
> + static bool warned;
> + int ret;
> +
> + ret = clock_gettime(compositor->presentation_clock, ts);
> + if (ret < 0) {
> + ts->tv_sec = 0;
> + ts->tv_nsec = 0;
> +
> + if (!warned)
> + weston_log("Error: failure to read "
> + "the presentation clock %#x: '%m' (%d)\n",
> + compositor->presentation_clock, errno);
> + warned = true;
> + }
> +}
> +
> WL_EXPORT void
> weston_version(int *major, int *minor, int *micro)
> {
> diff --git a/src/compositor.h b/src/compositor.h
> index 76b0778..2e6ef9d 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -1343,6 +1343,11 @@ int
> weston_compositor_set_presentation_clock_software(
> struct weston_compositor *compositor);
> void
> +weston_compositor_read_presentation_clock(
> + const struct weston_compositor *compositor,
> + struct timespec *ts);
> +
> +void
> weston_compositor_shutdown(struct weston_compositor *ec);
> void
> weston_compositor_exit_with_code(struct weston_compositor *compositor,
Apart from the error handling, rest looks fine:
Reviewed-by: Bryce Harrington <bryce at osg.samsung.com>
More information about the wayland-devel
mailing list