On Thu,  2 Apr 2015 07:10:50 +0200
Mario Kleiner <[email protected]> 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 <[email protected]>
> ---
>  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 <[email protected]>
for now.


Thanks,
pq
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to