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

[Keith Packard]

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 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.

> 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.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20101020/bf673ae3/attachment.pgp>