Fence Sync patches

[James Jones]

On Friday 03 December 2010 3:11:54 pm Keith Packard wrote:
> * PGP Signed by an unknown key
> 
> On Fri, 3 Dec 2010 14:14:34 -0800, James Jones <jajones at nvidia.com> wrote:
> > I do think fence objects will be generally
> > useful on all platforms.
> 
> Right, I think there's pretty general agreement that integrating OpenGL
> fencing into the X protocol is a good idea. And, this is the bulk of the
> changes that you've proposed, from an X server perspective.

OK, good.

> > The open source drivers have chosen to perform this implicit
> > synchronization.  However, it isn't required by any specification.
> 
> The other drivers have worked hard to provide synchronization when
> reporting damage to a client. This is not any kind of general
> synchronization operation, it happens at a well defined point in the
> operation of the server, and a point which occurs reasonably
> infrequently during normal server execution (once per frame for each
> application).
> 
> GLX and EGL are not the only direct rendering APIs.  We've also got
> several media APIs to deal with. While it would be nice to get fencing
> added to these, it's also a practical reality that our kernel drivers
> will need to provide synchronization between applications without the
> need for API-level fencing at least in the near term.

I agree it's important to balance design principles and reality.

> > Just because new applications and X extensions introduce new
> > synchronization needs doesn't mean new forms of implicit
> > synchronization should be shoe-horned in at the driver level.
> 
> Given support for general fence operations, providing implicit fence
> management means that existing compositing applications would just work,
> and would not require additional code and potentially additional latency
> to the screen. That seems like a pretty compelling argument to me.

Argh. I wish this had come up sooner, there's precious little time to make 
some measurements now to determine what level of latency this introduces, so I 
can't argue with you here.  I'll try to come up with numbers though.

> > In theory, implicit synchronization would be possible in our driver
> > model.
> 
> Given that implicit synchronization is possible, I'd suggest that a
> compromise could be:
> 
>  1) Explicitly state that the usual damage tracking causes synchronous
>     rendering between X and all direct rendering APIs supported by the
>     system.
> 
>  2) Add the new fence-based synchronization mechanism which eliminates
>     this guarantee. This means that compositing managers could choose to
>     continue to use the existing mechanism and get correct behaviour, or
>     they could choose to switch to the new request and gain a potential
>     performance advantage.
> 
>     I imagine this would require an additional request to set the
>     synchronization behaviour of specific Damage objects.
> 
> > However, it would almost certainly be more work than adding and
> > maintaining this X extension and modifying a half-dozen composite
> > managers to use it
> 
> It places the burden of implementation on the people who want the
> change. That seems like a good balance to me. There are also more than
> half a dozen compositing managers, most of which will never get fixed
> leaving them open to future failure.
> 
> Please consider my compromise above; accepting the existing usage as
> correct and sufficient but also encouraging compositing manager
> developers to use fences as a more hardware-friendly synchronization
> method.

Again, this would have been a lot easier to stomach if it had been raised 
sooner, but as I mentioned, I respect your reasons for withholding criticism.  
I think your compromise is acceptable with a couple of caveats:

-I've been trying to fix this "bug" for literally years.  I've been scraping 
together reviews for all the various pieces for months.  We're essentially 
arguing over semantics now, so how about we push this version for 1.10, and do 
a Damage 1.4 as part of 1.11 or even as a bugfix for 1.10 that basically just 
conditionalizes the language I added saying damage requests are not implicitly 
synchronized based on the proposed new request that sets the behavior in the 
server.  If it turns out there's very little or no latency added by 
DamageSubtractAndTrigger, we can go push new composite managers to use it by 
default.  Maybe an open source driver will even implement the explicit mode 
and we can see which is actually faster.

-Our drivers are going to be non-compliant in regard to the implicitly 
synchronized behavior for the foreseeable future.  It is truly a mountain of 
work to implement it with reasonable performance in our current architecture.  
We're slowly adapting to an architecture where it'd be easier, and we could 
fix it at that time, but I doubt I'll get time to before then.  I can live 
with being no compliant.  Apps have grudgingly accepted the quasi-defined 
behavior of texture-from-pixmap "loose binding" mode for years to get the 
performance benefits it offers.

Thanks,
-James