[PATCH 2/6] compositor-drm: Allow instant start of repaint loop.

[Pekka Paalanen]

On Thu,  2 Apr 2015 07:10:50 +0200
Mario Kleiner <mario.kleiner.de at gmail.com> wrote:

> drm_output_start_repaint_loop() incurred a delay of
> one refresh cycle by using a no-op page-flip to get
> an accurate vblank timestamp as reference. This causes
> unwanted lag whenever Weston exited its repaint loop, e.g.,
> whenever an application wants to repaint with less than
> full video refresh rate but still minimum lag.
> 
> Try to use the drmWaitVblank ioctl to get a proper
> timestamp instantaneously without lag. If that does
> not work, fall back to the old method of idle page-flip.
> 
> This optimization should work on any drm/kms driver
> which supports high precision vblank timestamping.
> As of Linux 4.0 these would be intel, radeon and
> nouveau on all supported gpu's.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> ---
>  src/compositor-drm.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index fe59811..4a7baa1 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -225,6 +225,9 @@ static const char default_seat[] = "seat0";
>  static void
>  drm_output_set_cursor(struct drm_output *output);
>  
> +static void
> +drm_output_update_msc(struct drm_output *output, unsigned int seq);
> +
>  static int
>  drm_sprite_crtc_supported(struct drm_output *output, uint32_t supported)
>  {
> @@ -704,6 +707,12 @@ err_pageflip:
>  	return -1;
>  }
>  
> +static int64_t
> +timespec_to_nsec(const struct timespec *a)
> +{
> +	return (int64_t)a->tv_sec * 1000000000000LL + a->tv_nsec;

Hi,

are you sure this cannot overflow? I think tv_sec could be a int64_t.

That no-one gets the idea of initializing the clock in the kernel to
some huge value just to fish out this kind of overflows?

> +}
> +
>  static void
>  drm_output_start_repaint_loop(struct weston_output *output_base)
>  {
> @@ -711,7 +720,13 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
>  	struct drm_compositor *compositor = (struct drm_compositor *)
>  		output_base->compositor;
>  	uint32_t fb_id;
> -	struct timespec ts;
> +	struct timespec ts, tnow;
> +	int ret;
> +	drmVBlank vbl = {
> +		.request.type = DRM_VBLANK_RELATIVE,
> +		.request.sequence = 0,
> +		.request.signal = 0,
> +	};
>  
>  	if (output->destroy_pending)
>  		return;
> @@ -721,6 +736,30 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
>  		goto finish_frame;
>  	}
>  
> +	/* Try to get current msc and timestamp via instant query */
> +	vbl.request.type |= drm_waitvblank_pipe(output);
> +	ret = drmWaitVBlank(compositor->drm.fd, &vbl);
> +
> +	/* Error return or zero timestamp means failure to get valid timestamp */
> +	if ((ret == 0) && (vbl.reply.tval_sec > 0)) {

No need to check tval_usec?

> +		ts.tv_sec = vbl.reply.tval_sec;
> +		ts.tv_nsec = vbl.reply.tval_usec * 1000;
> +
> +		/* Valid timestamp for most recent vblank - not stale? Stale ts could
> +		 * happen on Linux 3.17+, so make sure it is not older than 1 refresh
> +		 * duration since now.
> +		 */
> +		weston_compositor_read_presentation_clock(&compositor->base, &tnow);
> +		if (timespec_to_nsec(&tnow) - timespec_to_nsec(&ts) <

I'd write this difference in terms of timespec first and convert to
nsec afterwards, but I can do that also as a follow-up when I happen to
visit this part the next time. I have quite some timespec helpers lying
around that I should collect into one place at some point.

> +			(1000000000000LL / output_base->current_mode->refresh)) {
> +			drm_output_update_msc(output, vbl.reply.sequence);
> +			weston_output_finish_frame(output_base, &ts,
> +									   PRESENTATION_FEEDBACK_INVALID);
> +			return;
> +		}
> +	}
> +
> +	/* Immediate query didn't provide valid timestamp. Use pageflip fallback */
>  	fb_id = output->current->fb_id;
>  
>  	if (drmModePageFlip(compositor->drm.fd, output->crtc_id, fb_id,

At least
Acked-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
for now.


Thanks,
pq