[Mesa-dev] DRI3/Present fixes for Mesa 10.3+ (v2)

[Mario Kleiner]

On 12/05/2014 03:41 AM, Eric Anholt wrote:
> Mario Kleiner <mario.kleiner.de at gmail.com> writes:
>
>> A slightly updated and extended series of the dri3/present fixes for Mesa i
>> sent last week.
>>
>> Patch 1 and 2 are same as before. Patch 3 now has signed off by Frank Binns
>> and reviewed by Chris Wilson. Patch 4 and 5 are additional fixes. The last
>> one makes INTEL_swap_events behave properly again when swap completion events
>> come in out of order due to skipped present requests. Before the first out
>> of sequence sbc event caused the INTEL_swap_events extension to become completely
>> unuseable for the rest of a session.
> Sent out review for 3 of them (time for me to head out instead of
> reviewing more code), and thanks for working on this.  2/5 it's not
> clear to me from my first read of the spec that you shouldn't expect to
> get the most recent values from either cause in your return.  5/5 my
> first thoughts were "I hate wraparound logic, I'll review this later.
> Also, should we even be saving off msc/ust for an out-of-order sbc
> event?  Needs more thought."
>
> I'll try to think more about these later, but I wouldn't block on me.

Wrt. 2/5: It's a bit ambiguous how to read that bit of the spec, and i 
agree that one could read it in a way that the current mesa dri3 
behaviour is not (completely) violating the spec. When we implemented 
the DRI2 version, we understood it in the way i want to restore with 
2/5. The first reason is because the DRI2 / patch 2/5 interpretation 
makes the OML_sync_control extension very useful and robust for swap 
timing, whereas the alternative reading makes the extension borderline 
useless / its use somewhat fragile due to the race described in the 
commit. The second reason is backwards compatibility: It would be 
awesome not to break timing sensitive apps written against DRI2, 
especially given that users of those apps will certainly not understand 
why a simple distribution upgrade or graphics stack update pushed by the 
distro can suddenly cause such regressions.

In new app releases one could sort of work around such breakage by use 
of the INTEL_swap_events extension assuming everything would be nicely 
bug free. Unfortunately there were quite a few bugs and omissions and 
limitations in various ddx and kms drivers even until very recently 
which require funky workarounds, which require funky use of the 
different bits of the OML_sync_control extension,  in ways that would be 
difficult to do without 2/5 restoring the DRI2 semantic.

As an example see 
https://github.com/kleinerm/Psychtoolbox-3/blob/master/PsychSourceGL/Source/Linux/Screen/PsychWindowGlue.c#L1372
for my own toolkits backend implementation. It's a nice horror-show of 
all the relevant bugs in DRI2/DRI3 since about 2010 with the collection 
of workarounds needed to keep stuff working reasonably well on currently 
shipping distros.

Wrt. 5/5: As a little helper, attached a little C test thingy for the 
wraparound logic, running through a large range of simulated swap 
events, exercising the logic for both overflow and underflow wraparound, 
which i used to convince myself that i didn't screw up there with 
integer overflows etc., apart from testing on the actual server for 
normal use. In practice i don't think the wraparound logic in its 
revised form will ever trigger. It takes a lot of time for a running app 
to accumulate 2^32 bufferswaps.

As far as saving mst/ust for out-of-order sbc events: Yes, imho! The 
Present extension allows to complete swaps in a order different from 
their submission, e.g., you could submit a present request for target 
msc 10000, then for 9000, then for 8000..., and Present would complete 
them in order 8000, 9000, 10000 - this would cause the completion events 
to come in in reverse order with decrementing sbc, instead of 
incrementing. A valid case for dri3/present, and as INTEL_swap_events is 
not restricted to ascending order, it should be able to handle that 
case. Another case where one can see smaller inversions of the order is 
when switching between vsynced and non-vsynced swaps, possibly when the 
Present extension skips presents. I think a client could get mightily 
confused if it would try to collect swap events for submitted swaps and 
they don't show up.

However, what would be useful would be to extend INTEL_swap_events with 
new enums for completion type. Something like GLX_ERROR_INTEL for a 
present/swap that failed due to some error, e.g., out of memory, gpu 
wedged, whatever, and then GLX_SKIPPED_INTEL for a skipped present. 
Currently PresentCompleteModeSkip gets mapped to 
GLX_COPY_COMPLETE_INTEL, losing that information, which would be 
valuable for client applications like mine, which really need to know 
for sure if specific content made it to the actual display unharmed, and 
in which order.

Another thought i had: It would be good if we could expose somewhere if 
mesa is using DRI2 or DRI3/Present in a given session, e.g., in the 
GL_RENDERER string or maybe MESA_query_renderer extension. There are 
some workarounds one needs for DRI2 which are not needed for DRI3, 
potentially even harmful. There are things i (hope) i can do when i know 
i'm running under a well working DRI3/Present backend, which i couldn't 
do under DRI2. So i'd ideally need two different code paths and then 
some way to decide which one to choose based on the mesa backend in use.

Ok, brain totally crashing, need to sleep.

Thanks,
-mario

-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.c
Type: text/x-csrc
Size: 963 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141205/3abea13a/attachment.c>