[PATCH weston 5/7] Don't delay initial output paint

[Pekka Paalanen]

On Tue, 14 Feb 2017 13:18:05 +0000
Daniel Stone <daniels at collabora.com> wrote:

> On startup, we cannot lock on to the repaint timer because it is unknown
> to us. We deal with this by claiming that the moment of entry into the
> repaint loop is the moment a frame returned, causing finish_frame to
> delay our initial repaint to (refresh_time - repaint_delay), typically
> around 9ms of utterly wasted time.

Hi,

I wonder. The reason the repaint_delay exists is that clients would not
race to who gets to submit its frame first and kick everyone else to
the following refresh cycle. That's a concern when weston is in
continuous update mode.

This patch is for repainting from idle instead. Do we still have the
same concern? I suppose one can say it makes no difference, because it's
not the compositor triggering client actions with e.g. frame callbacks,
clients wake up at arbitrary times.

> Add an explicit stamp == NULL, to determine that we are just beginning
> our repaint loop, that the timings are in fact totally invalid, and that
> it would be beneficial to repaint the output immediately.

This is probably the more important justifaction for you, to get rid of
fake timestamps?

> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  libweston/compositor-drm.c | 3 +--
>  libweston/compositor.c     | 7 +++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 7376b3e5b..aeaba83fa 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -862,8 +862,7 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
>  
>  finish_frame:
>  	/* if we cannot page-flip, immediately finish frame */
> -	weston_compositor_read_presentation_clock(output_base->compositor, &ts);
> -	weston_output_finish_frame(output_base, &ts,
> +	weston_output_finish_frame(output_base, NULL,
>  				   WP_PRESENTATION_FEEDBACK_INVALID);

This is a pattern used by practically every backend. Shouldn't they all
be fixed alike?

OTOH, for DRM-backend this is the fallback of the fallback path, so
hardly important.

>  }
>  
> diff --git a/libweston/compositor.c b/libweston/compositor.c
> index 9eab0e298..ed8ef90fd 100644
> --- a/libweston/compositor.c
> +++ b/libweston/compositor.c
> @@ -2383,6 +2383,12 @@ weston_output_finish_frame(struct weston_output *output,
>  	TL_POINT("core_repaint_finished", TLP_OUTPUT(output),
>  		 TLP_VBLANK(stamp), TLP_END);

I suppose there could be an
assert(stamp || (presented_flags & WP_PRESENTATION_FEEDBACK_INVALID))?

>  
> +	/* If we haven't been supplied any timestamp at all, we don't have a
> +	 * timebase to work against, so any delay just wastes time. Push a
> +	 * repaint as soon as possible so we can get on with it. */
> +	if (!stamp)
> +		goto out;
> +
>  	refresh_nsec = millihz_to_nsec(output->current_mode->refresh);
>  	weston_presentation_feedback_present_list(&output->feedback_list,
>  						  output, refresh_nsec, stamp,
> @@ -2415,6 +2421,7 @@ weston_output_finish_frame(struct weston_output *output,
>  	if (presented_flags == WP_PRESENTATION_FEEDBACK_INVALID && msec_rel < 0)
>  		msec_rel += refresh_nsec / 1000000;
>  
> +out:
>  	if (msec_rel < 1)
>  		output_repaint_timer_handler(output);
>  	else

Right. Feel free to ignore my thinking-out-loud, and slap a:
Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170216/8d0c6fe0/attachment.sig>