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

[Keith Packard]

On Wed, 20 Oct 2010 11:35:46 -0700, Aaron Plattner <aplattner at nvidia.com> wrote:

> By sending X protocol to explicitly fence the GL rendering with the X
> rendering, which is the whole purpose of James's XDamageSubtractAndTrigger
> request.  Otherwise, the driver has to implicitly fence all rendering (and
> probably all CUDA kernels that interoperate with X) for damage events,
> which will not perform well.

The rendering we're interested in is coming from an X pixmap -- the
backing pixmap for a redirected window. That you're using that with GL
is immaterial; you must talk about the X resource somewhere. Yes, it's
no longer on the X protocol connection, but as an X API, it seems
incumbent on the implementation to retain the basic X ordering
guarantees.

The Intel DRI driver has specific fencing operations that pend
operations from one client wrt another client to ensure correct ordering
in these cases.

> In the context of the Damage extension, "future rendering requests" means
> future *X* rendering requests -- this is how you told me it worked when I
> asked you about it.  Implicitly fencing all other rendering through any API
> when a Damage event is generated is a spec change that we could make, but
> it would require discussion.

I don't see it as a change in the specification, just a natural
consequence of the X ordering guarantees, and as we're dealing only with
X objects here, I'm not sure why the API used to reference those objects
is relevant.

> If your driver model can use the ReportAfter semantics to get implicit
> fencing across APIs, then that seems like something you could enable in the
> driver via a DamageCreate or DamageRegister hook.  It doesn't seem like it
> belongs in the server.

I guess the point of the change was that the change ensures that the
driver *can* serialize access to the hardware correctly, without the
need for changes in applications. If there is a more efficient
mechanism, I'd love to hear suggestions.

> The flush callback sounds nifty, but it won't solve the problem for
> multi-queue hardware.

Right, it seems like what you want is to know which drawables have
delivered damage events so that you can correctly serialize rendering
From them by other applications using direct rendering.

> The old (and new, with this change) Damage semantics just plain don't work
> on multi-queue hardware with direct-rendering clients; it's a limitation in
> the specification that's addressed by James's sync extension.

The patch in questions should have *no effect* except in the rare case
when the events end up being delivered before the rendering is flushed
to the hardware. Given that, and given that there is no other existing
mechanism to prevent this race condition in the server code, I'm
convinced that we cannot remove the change until another solution is
proposed.

Fixing the problem with the compiz plugin should be our priority at this point.

-- 
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/0c951730/attachment.pgp>