[PATCH] remove damagePostOp() from DamageDamageRegion()

[Aaron Plattner]

On Wed, Aug 27, 2008 at 02:19:59PM -0400, Adam Jackson wrote:
> On Wed, 2008-08-27 at 09:56 -0700, Aaron Plattner wrote:
> > Now might be a good time to reopen the discussion about fixing the Damage
> > extension for asynchronous hardware.  There are two ways to solve this:
> > 
> >   1. Add extensions to allow GL client rendering to wait for X rendering
> >      associated with a particular Damage event using some sort of sync
> >      barrier.
> >   2. Defer sending Damage events until the damage has actually occurred.
> > 
> > While 1 would be ideal, it's probably too complicated to spec and implement
> > in a reasonable timeframe so it might be a good idea to think about
> > implementing 2.  What we need for that is
> > 
> >   a. reportAfter-like semantics for all damage objects, not just ones that
> >      were explicitly marked with DamageSetReportAfterOp.
> >   b. Deferring damageReportPostOp to after the rendering has actually
> >      occurred, not just after it's been submitted to the hardware.
> 
> As Michel notes, we need report-before too.  Software cursor is the big
> user of this, which does among other things:
> 
> if the incoming damage overlaps the glyph position
>     mark cursor removed
>     restore the pixels under the glyph
> call lower rendering layer
> if cursor was removed
>     re-paint the cursor
> 
> This is overkill in the common case of GXcopy, but any rop that reads
> out the destination needs this for correctness.  Software cursor is not
> really something we can break, since hardware still rarely has more than
> one and MPX will require arbitrarily many.  Cursor plane plz.
> 
> I think the tension here is that there's actually three events of
> interest - damage pending, damage posted to hardware, and damage
> completed - and we only expose two.  Of course, for software and
> single-queue hardware, the latter two are pretty much identical.
> 
> I lean towards defining the existing DamageNotify event - and the
> embarassingly named ReportPostOp inside the server - as "posted to
> hardware", on the basis that naïve hardware shouldn't have to care, and
> complicated hardware will have to do something custom to get the
> completion event anyway.  The question is how to add the completion
> event to the protocol.  And yes, I realize this is walking straight into
> the "too complicated to spec and implement" warning above, but I don't
> think it's actually that bad.  My first thought would be to add it as a
> new sync counter, but I think that fails because you'd need one per
> drawable.  I think this feels more like a flag in the high bit of the
> damage report level: DamageReportNonEmpty | DamageReportCompletion would
> give you a simple edge trigger when rendering hit the framebuffer.  As a
> bonus, this gives us per-drawable completions, and a way to know about
> them that isn't XGetImage.
> 
> The internal API could go a couple of ways.  My first guess is something
> like a DamageRegisterCompletionCallback() for the screen, which if
> present causes miext/damage/ to register a WakeupHandler that calls into
> the screen for events (which it then pulls from the DRM or whatever).
> There's possibly a wrapping order issue there but I think it's
> manageable.  Screens without such a callback would just generate
> completion-class events immediately.  Maybe this is bad API though?  I'm
> open to suggestions.

Yeah, that about sums it up.  To try to make everything crystal clear, let
me pedantically summarize the problem as it exists today.

There several different classes of users of the Damage extension currently.
Let me know if I missed or misunderstood any:

 1. Internal X modules that rely on early reporting
    E.g.: software cursor
    Doesn't care about when the damage actually lands

 2. Internal X modules that rely on reportAfter
    E.g.: EXA
    Doesn't care about when the damage actually lands

 3. RENDER-based composite managers
    E.g.: xcompmgr
    Wants damage events as early as possible
    Doesn't care about when the damage actually lands

 4. OpenGL-based composite managers
    E.g.: compiz
    Renders asynchronously w.r.t. the X server
    Wants damage events after damage actually lands
    Currently broken due to early damage reporting

 5. OpenGL-based composite managers + some magical new sync extension
    E.g.: doesn't exist yet
    Renders asynchronously w.r.t. the X server
    Wants damage events as early as possible
    Needs HW semaphores to synchronize with X rendering

Here's the problem with 4 that we have today:

 a. Compiz registers for damage events.
 b. Some X client sends a rendering request.
 c. The X server sends Compiz a damage event.
 d. Compiz processes the request and sends commands to the HW.
 e. HW processes Compiz's commands using stale bits.
 f. X driver processes the rendering request.
 g. HW processes X's commands, causing the damage that Compiz thinks it's
    already processed.

Just having reportAfter-like semantics for Compiz's damage object isn't enough:

 a. Compiz registers for damage events.
 b. Some X client sends a rendering request.
 c. The X server enqueues a damage region but doesn't send it yet.
 d. X driver processes the rendering request.
 e. X sends the damage event to Compiz.
 f. Compiz processes the request and sends commands to the HW.
 g. HW processes Compiz's commands using stale bits.
 h. HW processes X's commands, causing the damage that Compiz thinks it's
    already processed.


The big question here is whether we want to try to solve 4 without going
straight to 5.  Solving 4 by delaying events until the rendering occurs has
the unfortunate effect of synchronizing the CPU with the GPU: Compiz can't
start its CPU work until the GPU is done.


For solving 4:
    Something like your DamageRegisterCompletionCallback seems like it
would work.  We'd just need to hash out a server API for it and a way for
clients to opt in.  We already do something similar today for OpenGL
rendering to redirected windows: The OpenGL client schedules a completion
interrupt from the hardware, which wakes up the X driver through a callback
registered through xf86AddGeneralHandler, which calls DamageDamageRegion.
(This is one of the reasons I'd like DamageDamageRegion to continue to call
damageReportPostOp).

For implementing 5:
    It seems like it would be possible to add a new request that's similar
to DamageSubtract but that also inserts a sync marker into X's channel and
returns the counter value to the client.  The client would then use some
new extension to insert a waitSync on that counter value into its own
channel before continuing.  This would ensure that the client's rendering
happens after X's, while allowing both X and the client to continue to
enqueue more commands.  I'll need to get more feedback from other nvidians
to hash out the details, but does that sounds reasonable from an X/Mesa/DRI
perspective?

-- Aaron