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

[Mario Kleiner]

On 07/06/2015 03:41 PM, Pekka Paalanen wrote:
> 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?
>

Yes, your analysis is correct and confirms what i intended the patches 
to do. The low latency from a dead-still compositor is important if one 
wants to run an animation with less than full video refresh rate, or 
present images at specific time intervals, as in that case the 
compositor goes idle.

Wrt. "They do not make a difference to a client that takes too long
 > to render while trying to get a maximum framerate."

I have some patch that tries to reduce that problem a bit as well here, 
if you want to have a look:

https://github.com/kleinerm/weston/commits/unredirectexp

The one called "compositor: Optimize repaint scheduling for fullscreen 
surfaces." - It introduces a 2nd repaint-window deadline for fast 
updates - when the update can be done with a flip of the client 
fullscreen buffer - which is shorter. Seems to work pretty ok in 
practice. It will need some modification to rebase onto current master.

thanks,
-mario