[PATCH weston v2 10/11] [RFC] Account for very large repaint window misses

[Mario Kleiner]

On 03/30/2017 11:53 AM, Pekka Paalanen wrote:
> On Wed, 29 Mar 2017 22:35:46 +0200
> Mario Kleiner <mario.kleiner.de at gmail.com> wrote:
>
>> On 03/29/2017 12:30 PM, Pekka Paalanen wrote:
>>> On Tue, 28 Mar 2017 20:53:53 +0200
>>> Mario Kleiner <mario.kleiner.de at gmail.com> wrote:
>>>
>> ...
>>>>
>>>> Agreed. The adjustment can never be more than 1 refresh period, ergo
>>>> doesn't need a loop, unless we'd get the very unlikely case of Weston
>>>> getting preempted for a long time exactly during the few instructions
>>>> leading from output->start_repaint_loop to weston_output_finish_frame
>>>> reading the compositor clock. In which case we'd glitch badly in any
>>>> case. I can't think of a way, apart from kms driver bug, that the
>>>> pageflip event could carry a bad timestamp, and for the pageflip driven
>>>> path we don't hit that one-shot adjustment (or potential loop) anyway,
>>>> as the flags aren't WP_PRESENTATION_FEEDBACK_INVALID.
>>>
>>> Oh, that is true about the pageflip fallback path of
>>> start_repaint_loop. Makes me wonder if I should "fix" that, since
>>> start_repaint_loop should never result in sending out presentation
>>> feedback events, and can't assert on that if the flags are not
>>> FEEDBACK_INVALID. Oh well, I've been meaning to get rid of the INVALID
>>> flag by introducing a different function to call in that case.
>>>
>>
>> I think that can't happen, because we only copy the pending feedback
>> events for a surface into the list for an output in the call sequence:
>>
>> weston_output_maybe_repaint -> weston_output_repaint ->
>> weston_output_take_feedback_list
>>
>> That in turn only happens if output->repaint_status ==
>> REPAINT_SCHEDULED, which only gets set at the end of
>> weston_output_finish_frame().
>>
>> Iow. If the repaint loop is restarted in any way, that first
>> weston_output_finish_frame() call from the restart code will call
>> weston_presentation_feedback_present_list() with an empty
>> output->feedback_list - a no-op. Only then we switch to
>> REPAINT_SCHEDULED, sample, take pending feedback events for dispatch in
>> the next call to weston_output_finish_frame().
>>
>> So it doesn't matter if we get called by the pageflip fallback during
>> restart with flags other than WP_PRESENTATION_FEEDBACK_INVALID.
>>
>> But it confused me initially as well when i reread the code.
>
> Assert()s generally are for things that "can't happen". ;-)
>

Maybe i misunderstood what you meant. I meant it can't happen that we 
send out presentation feedback during any restart of the repaint loop, 
regardless if regularly or via the pageflip fallback.

-mario