[PATCH weston 2/2] compositor-drm: Fix inconsistency in finish frame timestamps

[Ander Conselvan de Oliveira]

On 02/15/2013 10:27 AM, Pekka Paalanen wrote:
> On Thu, 14 Feb 2013 11:51:15 -0500
> Kristian Høgsberg <hoegsberg at gmail.com> wrote:
>
>> On Wed, Feb 13, 2013 at 04:06:38PM +0200, Ander Conselvan de Oliveira wrote:
>>> The page flip event timestamps comes from the monotonic clock, while
>>> idle_repaint() gets the time with gettimeofday(). That leads to
>>> inconsistent timestamps on the frame callbacks.
>>>
>>> Fix this by making the drm backend page flip to the current buffer and
>>> call weston_output_finish_frame() with the page flip event timestamp,
>>> instead of using gettimeofday().
>>
>> Yup, this looks right, but I think we need to require that all
>> backends provide a start_repaint_loop function.  Only the backend
>> knows what kind of timestamp comes back in finish_frame, so we can't
>> assume gttimeofday() as a fallback.  compositor-wayland.c uses the
>> frame callback timestamp, for example, and if the host weston is
>> running on KMS, that's a monotonic timestamp from the underlying
>> pageflip event.
>
> Hi,
>
> while you are making the timestamps consistent, could also make them,
> err... correct?
>
> I think the current code in weston_output_repaint() is sending out the
> frame callbacks at the moment the rendering has been started, not when
> when it has hit the screen. Do you agree?

That's a tricky one. I agree that the timestamp of a frame callback does 
not represent the moment the last attached buffer reached the screen. 
But the timestamp is that of the latest completed flip.

The protocol description for wl_surface::frame reads: "Request 
notification when the next frame is displayed". It does not define if 
the next frame need to contain any newly attached buffer. So except for 
the case of idle_repaint(), one could consider the current 
implementation correct.

If that is what we want, that's a different story. But because of our 
latency, it seems providing accurate timestamps could hinder our ability 
to drive repaint using frame callbacks.

The time between a client issuing an attach and the flip for it 
completing can be almost two frames, since the client renders and 
signals that to the compositor, which will render again after a pending 
flip completes and queue another flip. Only when the second flip 
completes the new buffer hits the screen. If the callback fires only at 
that point, the client will seat idle for the time of two frames.

I believe we would have to change Weston's repaint loop to fix that. In 
the attached patch, weston_output_repaint() is called from a timer armed 
on finish_frame() to fire closer to the next vsync. We could derive an 
appropriate delay from the current mode refresh rate (I just hardcoded 
it for now).

With that patch I was able to keep gears running at 60 fps while 
providing accurate timestamps, while just moving the send_callback() 
calls to finish_frame() would result in a 30 fps framerate.

The patch also solves the different clock problem of the timestamps by 
removing the msec parameters from output_repaint() and just calling 
straight into it from idle_repaint().

Any thoughts?

Cheers,
Ander

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-compositor-Reduce-latency-by-delaying-repaint-closer.patch
Type: text/x-patch
Size: 6062 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20130219/9bf03ddc/attachment.bin>