[PATCH] Revert "Set DamageSetReportAfterOp to true for the damage extension" (#30260)

[Aaron Plattner]

On Wed, Oct 20, 2010 at 02:57:27PM -0700, Keith Packard wrote:
> On Wed, 20 Oct 2010 14:29:33 -0700, Aaron Plattner <aplattner at nvidia.com> wrote:
> 
> > Then you're implementing what sounds like a nice vendor-specific
> > feature.
> 
> There isn't any other mechanism provided today to perform the necessary
> synchronization; the event must not be seen by the client before the
> hardware ever sees the rendering request.

The premise is correct, but the conclusion doesn't follow except for a
certain set of drivers.

> > The driver can do that by enabling ReportAfter from a damage create
> > hook.
> 
> No, it must see the rendering request before the damage extension writes
> the damage event to the client, hence the damage extension itself must
> pend the delivery of this event until after the rendering operation has
> been delivered to the client.

... for your driver, and only until we have a sync extension.

> > Forcing the compositor to wait until the X driver has processed the
> > request, and especially (at least on our hardware) until the commands have
> > been flushed to the GPU reduces the amount of parallelism between the CPU
> > and the GPU.
> 
> As Kristian has said, there is no effect 99.99% of the time - the X
> server will always hold the event until it decides to flush events,
> which can only happen before the rendering operation is complete when
> the output buffer is already full before the event is written. Hence, no
> performance difference for the two cases, just a simple change to ensure
> that synchronization is possible, even in the absence of proposed
> additions to the protocol to permit more complicated synchronization
> semantics.
> 
> > Yes, I agree that there's some additional bug that needs to be fixed,
> > regardless.  What I'm saying is that this patch should be reverted on
> > principle anyway and moved into the appropriate drivers (or the DRI
> > module) instead, because it's a vendor-specific requirement that is not
> > applicable to all drivers and reduces CPU/GPU parallelism.
> 
> I've got two competing bugs -- on one hand, we get occasionally
> incorrect rendering with *any* compositing manager, on the other hand we
> get consistent incorrect rendering with a specific compiz plugin.
> 
> I don't see any way to win here.

Fine, but will you be willing to move this call to the drivers that need it
when we have a real sync extension?

> -- 
> keith.packard at intel.com