| Message ID | 1352884515-10154-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 14 Nov 2012 09:15:14 +0000 Chris Wilson <chris@chris-wilson.co.uk> wrote: > The bspec was recently updated to remove the ability to update the > semaphore using the MI_SEMAPHORE_BOX command, the ability to wait upon > the semaphore value remained. Instead the advice is to update the > register using the MI_LOAD_REGISTER_IMM command. In cursory testing, > semaphores continue to function - the question is whether this fixes > some of the deadlocks where the semaphore registers contained stale > values? > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel J Blueman <daniel@quora.org> Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index ee4abc5..dd2e7db 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -554,12 +554,9 @@ update_mboxes(struct intel_ring_buffer *ring, > u32 seqno, > u32 mmio_offset) > { > - intel_ring_emit(ring, MI_SEMAPHORE_MBOX | > - MI_SEMAPHORE_GLOBAL_GTT | > - MI_SEMAPHORE_REGISTER | > - MI_SEMAPHORE_UPDATE); > - intel_ring_emit(ring, seqno); > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > intel_ring_emit(ring, mmio_offset); > + intel_ring_emit(ring, seqno); > } > > /**
On Wed, 14 Nov 2012 11:06:42 -0800 Ben Widawsky <ben@bwidawsk.net> wrote: > On Wed, 14 Nov 2012 09:15:14 +0000 > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > The bspec was recently updated to remove the ability to update the > > semaphore using the MI_SEMAPHORE_BOX command, the ability to wait upon > > the semaphore value remained. Instead the advice is to update the > > register using the MI_LOAD_REGISTER_IMM command. In cursory testing, > > semaphores continue to function - the question is whether this fixes > > some of the deadlocks where the semaphore registers contained stale > > values? > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniel J Blueman <daniel@quora.org> > Reviewed-by: Ben Widawsky <ben@bwidawsk.net> Tested-by: Ben Widawsky <ben@bwidawsk.net> > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index ee4abc5..dd2e7db 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -554,12 +554,9 @@ update_mboxes(struct intel_ring_buffer *ring, > > u32 seqno, > > u32 mmio_offset) > > { > > - intel_ring_emit(ring, MI_SEMAPHORE_MBOX | > > - MI_SEMAPHORE_GLOBAL_GTT | > > - MI_SEMAPHORE_REGISTER | > > - MI_SEMAPHORE_UPDATE); > > - intel_ring_emit(ring, seqno); > > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > > intel_ring_emit(ring, mmio_offset); > > + intel_ring_emit(ring, seqno); > > } > > > > /** > > >
On Thu, Nov 15, 2012 at 09:14:48AM -0800, Ben Widawsky wrote: > On Wed, 14 Nov 2012 11:06:42 -0800 > Ben Widawsky <ben@bwidawsk.net> wrote: > > > On Wed, 14 Nov 2012 09:15:14 +0000 > > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > > The bspec was recently updated to remove the ability to update the > > > semaphore using the MI_SEMAPHORE_BOX command, the ability to wait upon > > > the semaphore value remained. Instead the advice is to update the > > > register using the MI_LOAD_REGISTER_IMM command. In cursory testing, > > > semaphores continue to function - the question is whether this fixes > > > some of the deadlocks where the semaphore registers contained stale > > > values? > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Daniel J Blueman <daniel@quora.org> > > Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > Tested-by: Ben Widawsky <ben@bwidawsk.net> I seem to have forgotten to mention that this is merged. Bad, bad me ... Thanks for the patch & review. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ee4abc5..dd2e7db 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -554,12 +554,9 @@ update_mboxes(struct intel_ring_buffer *ring, u32 seqno, u32 mmio_offset) { - intel_ring_emit(ring, MI_SEMAPHORE_MBOX | - MI_SEMAPHORE_GLOBAL_GTT | - MI_SEMAPHORE_REGISTER | - MI_SEMAPHORE_UPDATE); - intel_ring_emit(ring, seqno); + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); intel_ring_emit(ring, mmio_offset); + intel_ring_emit(ring, seqno); } /**
The bspec was recently updated to remove the ability to update the semaphore using the MI_SEMAPHORE_BOX command, the ability to wait upon the semaphore value remained. Instead the advice is to update the register using the MI_LOAD_REGISTER_IMM command. In cursory testing, semaphores continue to function - the question is whether this fixes some of the deadlocks where the semaphore registers contained stale values? Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel J Blueman <daniel@quora.org> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)