[Intel-gfx] [PATCH xf86-video-intel] sna: Use correct struct sna in sna_mode_wakeup

[Thomas Preston]

On 21/01/2020 11:27, Chris Wilson wrote:
> Quoting Thomas Preston (2020-01-21 10:28:58)
>> When deciding if we should defer_vblanks we should reference the event's
>> struct sna, rather than the caller's struct sna. In order to do this, we
>> must grab a new struct sna for each event in the buffer. Move this logic
>> out of `case DRM_EVENT_FLIP_COMPLETE` and create a new variable
>> sna_event, so that it is clear which struct sna we are referring to.
>> Also add another ZaphodHead comment by the struct sna argument, in case
>> someone misses the comment below.
>>
>> Fixes issue #184 with ZaphodHead and TearFree, introduced in this commit:
>>
>>         12db28ab sna: Reorder vblank/flip event handling to avoid TearFree recursion
>>
>> Signed-off-by: Thomas Preston <thomas.preston at codethink.co.uk>
>> ---
>>  src/sna/sna_display.c | 48 +++++++++++++++++++++++--------------------
>>  1 file changed, 26 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/sna/sna_display.c b/src/sna/sna_display.c
>> index 874292bc..b40a6c4a 100644
>> --- a/src/sna/sna_display.c
>> +++ b/src/sna/sna_display.c
>> @@ -9711,9 +9711,12 @@ fixup_flip:
>>         RegionEmpty(region);
>>  }
>>  
>> +/* In the case of ZaphodHead, there is only one event queue in the main
>> + * struct sna. Only refer to this struct sna when dealing with the event queue.
>> + * Otherwise, extract the struct sna from the event user_data.
>> + */
>>  int sna_mode_wakeup(struct sna *sna)
>>  {
>> -       bool defer_vblanks = sna->mode.flip_active && sna->mode.shadow_enabled;
> 
> My thinking was that I only cared about re-entrancy on the local sna for
> processing this event queue. And there is no threading, so only one
> sna is processed at a time... Hmm, I don't think we can in the situation
> of being inside one shadow flip and care much about the other.
> 

If it helps, the hanging display described in #184 only happens when
TearFree (shadow_enabled) is enabled for both displays. Maybe
sna_mode_wakeup is called on a :0.1 head event, but sna_dri2_vblank_handler
is never called because :0.0 flip_active >= 1, causing us to always defer
the event.

It's not entirely clear to me why this would stop :0.0 flip_active changing
though.

Anyway, paying more attention to which event's sna has
flip_active/shadow_enabled (as the original comments describe) makes the
problem go away.

We're actually debugging this to close-in on *another* ZaphodHead+TearFree
issue which appears on a 4.14 kernel (among other userland upgrades). At
some point :0.0 head gets stuck between two buffers (current + shadow) and
switches between the two causing a flicker or ghosting effect. It's
possibly got something to do with these patches:

	8bfac0f2 sna/dri2: Only force the TearFree/swcursor hack when using TearFree
	26f8ab54 sna: Restore local damage processing for TearFree/DRI2/swcursor early
	7cf67022 sna/dri2: Prevent the sw cursor from copyig to a buffer as we discard it

and it goes away when `-Dasync-swap=true` (APPLY_DAMAGE is 0) or we set:

	 sna->ignore_copy_area = false; //sna->flags & SNA_TEAR_FREE;