Re: [PATCH] drm/i915: specify the address of the semaphore register for comparison

[Ben Widawsky <ben@bwidawsk.net>]

On Wed, 21 Sep 2011 12:42:22 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> In addition to telling the GPU that we are comparing with a named MMIO
> register, we also apparently have to tell the GPU the address of that
> register.
> 
> The lack of giving the GPU the duplicate information it seems to
> desire leads to such fun confusion as:
> 
> Blitter command stream:
>   seqno: 0x00001c6c
> Render command stream:
>   seqno: 0x00001c7a
> 
> Active batch on BLT (seqno 1c6e):
>   0x03b55000:      0x30222221: UNKNOWN
>   0x03b55004:      0xffb7b4b1:    UNKNOWN
>   0x03b55008:      0xffb7b4b1:    UNKNOWN
>   0x03b5500c:      0xffb7b4b1:    UNKNOWN
>   0x03b55010:      0xffb7b4b1:    UNKNOWN
>   0x03b55014:      0xffb7b4b1:    UNKNOWN
>   0x03b55018:      0xffb7b4b1:    UNKNOWN
>   0x03b5501c:      0xffb7b4b1:    UNKNOWN
>   0x03b55020:      0xffb7b4b1:    UNKNOWN
>   0x03b55024:      0xffb7b4b1:    UNKNOWN
>   0x03b55028:      0xffb7b4b1:    UNKNOWN
>   0x03b5502c:      0xffb7b4b1:    UNKNOWN
>   ... [an antialiased rgba box]
> 
> Render ringbuffer:
>   ...
>   0x00007e70:      0x0b160001: MI_SEMAPHORE_MBOX
>   0x00007e74:      0x00001c6e:    dword 1
>   0x00007e78:      0x00000000:    dword 2
>   0x00007e7c:      0x00000000: MI_NOOP
>   0x00007e80:      0x18800180: MI_BATCH_BUFFER_START
>   0x00007e84:      0x03b5b000:    dword 1
>   0x00007e88:      0x02000004: MI_FLUSH
>   0x00007e8c:      0x00000000: MI_NOOP
>   0x00007e90:      0x0b240001: MI_SEMAPHORE_MBOX
>   0x00007e94:      0x00001c70:    dword 1
>   0x00007e98:      0x00022040:    dword 2
>   0x00007e9c:      0x0b240001: MI_SEMAPHORE_MBOX
>   0x00007ea0:      0x00001c70:    dword 1
>   0x00007ea4:      0x00012044:    dword 2
>   0x00007ea8:      0x10800001: MI_STORE_DATA_INDEX
>   0x00007eac:      0x00000080:    dword 1
>   0x00007eb0:      0x00001c70:    dword 2
>   0x00007eb4:      0x01000000: MI_USER_INTERRUPT
>   0x00007eb8:      0x02000000: MI_FLUSH
>   ... [wait on results of batch 1cde from BLT, then execute 1c70 on
> RENDER]
> 
> As the RENDER ring has already completed batch 1c7a before the BLT
> began processing 1c6e, it is obvious then that the GPU did not heed
> the semaphore wait instruction. Adding the symmetric register address
> from the signal instruction to the compare instruction prevents the
> hang and also prevents visual corruption whilst running KDE.
> 
> Reported-by: Andrew Lutomirski <luto@mit.edu>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andrew Lutomirski <luto@mit.edu>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c index 5fa3d99..6b9790e
> 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -359,7 +359,8 @@ intel_ring_sync(struct intel_ring_buffer *ring,
>  			intel_ring_sync_index(ring, to) << 17 |
>  			MI_SEMAPHORE_COMPARE);
>  	intel_ring_emit(ring, seqno);
> -	intel_ring_emit(ring, 0);
> +	intel_ring_emit(ring,
> +			RING_SYNC_0(to->mmio_base) +
> 4*intel_ring_sync_index(ring,to)); intel_ring_emit(ring, MI_NOOP);
>  	intel_ring_advance(ring);
>  

I'm sort of afraid this is just papering over the issue, ie. the mmio
access is just adding delay that happens to make it work. I think we
should follow up with devs on this one.

Ben