[PATCH 08/10] compositor: Respect repaint-window when restarting repaint loop.

[Pekka Paalanen]

On Sun, 21 Jun 2015 21:25:15 +0200
Mario Kleiner <mario.kleiner.de at gmail.com> wrote:

> If a stopped repaint loop gets restarted due to posting of new
> damage, and this restart of the repaint loop happens late in the
> video refresh cycle, ie. already inside the repaint-window and
> thereby after the composition deadline for the current frame,
> then defer the actual output repaint to the composition deadline
> of the next video refresh cycle by setting the repaint timer
> accordingly.
> 
> This tries to make sure that:
> 
> a) Client(s) posting damage timely before the composition deadline
>    (video refresh duration - "repaint-window" duration) of the
>    current refresh cycle will trigger a repaint within the current
>    refresh cycle, thereby avoiding one extra frame of compositor
>    lag due to the needed restart of the repaint loop if the loop
>    was stopped. This allows them to benefit from the earlier
>    "instant repaint restart" commit to keep latency low.
> 
> b) Late clients which post damage close to the end of a refresh
>    cycle can't race other clients if the repaint loop is restarted.
>    Instead they will get deferred to the next compositor cycle,
>    just as if the repaint loop would have been already running -
>    the semantic of the "repaint-window" parameter is preserved.
> 
>    This is especially important to prevent a very late client
>    from triggering a repaint very close to the vblank, which
>    would cause the compositor to certainly miss the vblank and
>    skip one frame and then cause a delay of another frame for
>    other clients which posted their damage in time for the
>    following frame. Iow. this provides clients with a more
>    predictable compositor timing and makes it easier for them
>    to latch onto the compositors repaint cycle.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> ---
>  src/compositor.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/compositor.c b/src/compositor.c
> index 2f89b39..2186692 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -2431,6 +2431,13 @@ weston_output_finish_frame(struct weston_output *output,
>  		msec = 0;
>  	}
>  
> +	/* Called from restart_repaint_loop and restart happens already after
> +	 * the deadline given by repaint_msec? In that case we delay until
> +	 * the deadline of the next frame, to give clients a more predictable
> +	 * timing of the repaint cycle to lock on. */
> +	if (presented_flags == PRESENTATION_FEEDBACK_INVALID && msec < 0)
> +		msec += refresh_nsec / 1000000;
> +
>  	if (msec < 1)
>  		output_repaint_timer_handler(output);
>  	else

Hi Mario,

I have been checking these two patches and testing their behaviour on
the DRM backend. Here is what I found. Note, that I tested the series
with my tweaks, but those should not affect to behaviour.

Code versions:

baseline: patch 9
instant-restart: patch 9 + 7
respect-window: patch 9 + 7 + 8

I attached wesgr plots of each code version, from
'weston-presentation-shm -p -d N', where N was 4 and 12. With 4, the
client has well enough time to make it before the deadline, and with 12
it always missed the deadline but not the vblank.

Figure baseline-p4 shows the usual continuously updating client
profile, where the compositor's repaint loop keeps on.

Figure baseline-p12 shows that the repaint loop stops (gray area)
before the client posts a frame, causing the compositor wait for a
vblank before processing it, which leads to latency of around 20 ms.

Figures instant-restart-p4 and respect-window-p4 are similar to
baseline-p4, as expected.

Figure instant-restart-p12 is interesting. The fast-restart of the
repaint loop works, the client being the first to trigger a repaint
gets processed immediately and achieves <5ms latency. This however has
the client fighting issue I pointed out, which you solved with patch 8.
The client animated smoothly at 60 fps when nothing else happened, but
moving the pointer caused it to drop to 30 fps and this was very
visible.

I also confirmed, that weston_finish_frame() does indeed reliably get
called twice with the exact same timestamp in this case.

Figure respect-window-p12 is basically the same as baseline-p12: the
client gets 30 fps, because it always misses the deadline for the very
next vblank.

So what do these patches actually change?
It took me a while to figure out how I could measure it.

Turned out I need to cover different values of N (the delay, see
above). All the numbers are in milliseconds.

Baseline:
N	latency
100	33
103	30
106	27
109	24
112	21
115	18

respect-window:
N	latency
100	16
103	13
106	10
109	7, 24 (jumps between the two)
112	21
115	18

Latency here is the time presentation-shm reports as "c2p" and "t2p";
commit/target to presentation.

And *that* is the difference. Note, that 60 Hz refresh, 100 ms is just
about 6 periods exactly.

So yes, these patches do indeed seem to shave off one frame period of
latency when starting from dead-still compositor (repaint loop not
active). They do not make a difference to a client that takes too long
to render while trying to get a maximum framerate.

Have I managed to exhaustively analyze these patches, and do you agree
with the analysis? Anything I missed?

I think these look good, and I will post the revised patch set soon.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: baseline-p4.svg
Type: image/svg+xml
Size: 9065 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150706/66ecc2a2/attachment-0006.svg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: baseline-p12.svg
Type: image/svg+xml
Size: 8172 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150706/66ecc2a2/attachment-0007.svg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: instant-restart-p4.svg
Type: image/svg+xml
Size: 8046 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150706/66ecc2a2/attachment-0008.svg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: instant-restart-p12.svg
Type: image/svg+xml
Size: 8750 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150706/66ecc2a2/attachment-0009.svg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: respect-window-p4.svg
Type: image/svg+xml
Size: 8266 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150706/66ecc2a2/attachment-0010.svg>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: respect-window-p12.svg
Type: image/svg+xml
Size: 6938 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150706/66ecc2a2/attachment-0011.svg>