[Mesa-dev] [PATCHv3] r600g: Use a fake reloc to sleep for fences

[Marek Olšák]

2012/2/9 Simon Farnsworth <simon.farnsworth at onelan.co.uk>:
> On Wednesday 8 February 2012 18:28:05 Michel Dänzer wrote:
>> On Fre, 2012-02-03 at 17:32 +0000, Simon Farnsworth wrote:
>> > r300g is able to sleep until a fence completes rather than busywait
>> > because it creates a special buffer object and relocation that stays
>> > busy until the CS containing the fence is finished.
>> >
>> > Copy the idea into r600g, and use it to sleep if the user asked for an
>> > infinite wait, falling back to busywaiting if the user provided a
>> > timeout.
>> >
>> > Signed-off-by: Simon Farnsworth <simon.farnsworth at onelan.co.uk>
>> > ---
>> >
>> > v3: At Dave Airlie's suggestion on dri-devel, make use of the BO to give
>> > us a fighting chance of recovering after a GPU reset.
>> >
>> > We know that the fence will never be signalled by hardware if the dummy
>> > BO has gone idle - we use that information to escape the loop. This
>> > might be a useful addition to the 8.0 branch.
>>
>> That's a nifty trick. :)
>>
> It's just a shame that the rest of recovering from a GPU reset isn't as simple
> - at least this stops us busywaiting eternally after the kernel's detected a
> CP stall.
>
>> > diff --git a/src/gallium/drivers/r600/r600_hw_context.c
>> > b/src/gallium/drivers/r600/r600_hw_context.c index 8eb8e6d..acfa494
>> > 100644
>> > --- a/src/gallium/drivers/r600/r600_hw_context.c
>> > +++ b/src/gallium/drivers/r600/r600_hw_context.c
>> > @@ -1623,6 +1623,13 @@ void r600_context_emit_fence(struct r600_context
>> > *ctx, struct r600_resource *fen>
>> >     ctx->pm4[ctx->pm4_cdwords++] = 0;                       /* DATA_HI
>> >     */
>> >     ctx->pm4[ctx->pm4_cdwords++] = PKT3(PKT3_NOP, 0, 0);
>> >     ctx->pm4[ctx->pm4_cdwords++] = r600_context_bo_reloc(ctx, fence_bo,
>> >     RADEON_USAGE_WRITE);>
>> > +
>> > +   /* Create a dummy BO so that fence_finish without a timeout can sleep
>> > waiting for completion */ + *sleep_bo = (struct r600_resource*)
>> > +                   pipe_buffer_create(&ctx->screen->screen, PIPE_BIND_CUSTOM,
>> > +                                      PIPE_USAGE_STAGING, 1);
>> > +   /* Add the fence as a dummy relocation. */
>> > +   r600_context_bo_reloc(ctx, *sleep_bo, RADEON_USAGE_READWRITE);
>>
>> Sorry for only thinking of this now, but what's the advantage of doing
>> this here, rather than in r600_create_fence()? Seems like that would be
>> simpler and cleaner.
>
> I've done it here because this is the bit of code that deals with all the
> hardware-related stuff, and it already knows about writing out relocations. I
> have no particular opinion either way, though, and wouldn't get upset about
> moving it. Thoughts?

FWIW, the "hw_context" files are there for historical reasons only. There is
nothing wrong with adding relocations and emitting commands from the
other places of the driver. Eventually, r600.h, r600_pipe.h, and
r600_hw_context_priv.h will be merged together.

Marek