[01/18] drm/i915: Comments for semaphore clarification

Message ID	1367110769-1306-2-git-send-email-ben@bwidawsk.net (mailing list archive)
State	New, archived
Headers	show Return-Path: <intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org> From: Ben Widawsky <ben@bwidawsk.net> To: Intel-GFX <intel-gfx@lists.freedesktop.org> Date: Sat, 27 Apr 2013 17:59:12 -0700 Message-Id: <1367110769-1306-2-git-send-email-ben@bwidawsk.net> In-Reply-To: <1367110769-1306-1-git-send-email-ben@bwidawsk.net> References: <1367110769-1306-1-git-send-email-ben@bwidawsk.net> Cc: Ben Widawsky <ben@bwidawsk.net> Subject: [Intel-gfx] [PATCH 01/18] drm/i915: Comments for semaphore clarification Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org

Message ID

1367110769-1306-2-git-send-email-ben@bwidawsk.net (mailing list archive)

State

New, archived

Headers

From: Ben Widawsky <ben@bwidawsk.net>
To: Intel-GFX <intel-gfx@lists.freedesktop.org>
Date: Sat, 27 Apr 2013 17:59:12 -0700
Message-Id: <1367110769-1306-2-git-send-email-ben@bwidawsk.net>
In-Reply-To: <1367110769-1306-1-git-send-email-ben@bwidawsk.net>
References: <1367110769-1306-1-git-send-email-ben@bwidawsk.net>
Cc: Ben Widawsky <ben@bwidawsk.net>
Subject: [Intel-gfx] [PATCH 01/18] drm/i915: Comments for semaphore
	clarification
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org
Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org

Commit Message

Ben Widawsky April 28, 2013, 12:59 a.m. UTC

Semaphores are tied very closely to the rings in the GPU. Trivial patch
adds comments to the existing code so that when we add new rings we can
include comments there as well. It also helps distinguish the ring to
semaphore mailbox interactions by using the ringname in the semaphore
data structures.

This patch should have no functional impact.

A subset of this patch was:
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++---------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
 3 files changed, 16 insertions(+), 16 deletions(-)

Comments

Lespiau, Damien May 7, 2013, 1:54 p.m. UTC | #1

On Sat, Apr 27, 2013 at 05:59:12PM -0700, Ben Widawsky wrote:
> Semaphores are tied very closely to the rings in the GPU. Trivial patch
> adds comments to the existing code so that when we add new rings we can
> include comments there as well. It also helps distinguish the ring to
> semaphore mailbox interactions by using the ringname in the semaphore
> data structures.
> 
> This patch should have no functional impact.
> 
> A subset of this patch was:
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++---------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
>  3 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4d66898..767aa32 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -267,12 +267,12 @@
>  #define  MI_SEMAPHORE_UPDATE	    (1<<21)
>  #define  MI_SEMAPHORE_COMPARE	    (1<<20)
>  #define  MI_SEMAPHORE_REGISTER	    (1<<18)
> -#define  MI_SEMAPHORE_SYNC_RV	    (2<<16)
> -#define  MI_SEMAPHORE_SYNC_RB	    (0<<16)
> -#define  MI_SEMAPHORE_SYNC_VR	    (0<<16)
> -#define  MI_SEMAPHORE_SYNC_VB	    (2<<16)
> -#define  MI_SEMAPHORE_SYNC_BR	    (2<<16)
> -#define  MI_SEMAPHORE_SYNC_BV	    (0<<16)
> +#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS wait for BCS  (BRSYNC) */
> +#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS wait for VCS  (VRSYNC) */
> +#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS wait for RCS  (RVSYNC) */
> +#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS wait for BCS  (BVSYNC) */
> +#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS wait for VCS  (VBSYNC) */
> +#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS wait for RCS  (RBSYNC) */

Huum aren't the register names inverted in the comment? RCS waiting for
BCS would be RBSYNC? (Register read by RCS, written by BCS)

Ben Widawsky May 7, 2013, 4:51 p.m. UTC | #2

On Tue, May 07, 2013 at 02:54:06PM +0100, Damien Lespiau wrote:
> On Sat, Apr 27, 2013 at 05:59:12PM -0700, Ben Widawsky wrote:
> > Semaphores are tied very closely to the rings in the GPU. Trivial patch
> > adds comments to the existing code so that when we add new rings we can
> > include comments there as well. It also helps distinguish the ring to
> > semaphore mailbox interactions by using the ringname in the semaphore
> > data structures.
> > 
> > This patch should have no functional impact.
> > 
> > A subset of this patch was:
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++------
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++---------
> >  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
> >  3 files changed, 16 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 4d66898..767aa32 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -267,12 +267,12 @@
> >  #define  MI_SEMAPHORE_UPDATE	    (1<<21)
> >  #define  MI_SEMAPHORE_COMPARE	    (1<<20)
> >  #define  MI_SEMAPHORE_REGISTER	    (1<<18)
> > -#define  MI_SEMAPHORE_SYNC_RV	    (2<<16)
> > -#define  MI_SEMAPHORE_SYNC_RB	    (0<<16)
> > -#define  MI_SEMAPHORE_SYNC_VR	    (0<<16)
> > -#define  MI_SEMAPHORE_SYNC_VB	    (2<<16)
> > -#define  MI_SEMAPHORE_SYNC_BR	    (2<<16)
> > -#define  MI_SEMAPHORE_SYNC_BV	    (0<<16)
> > +#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS wait for BCS  (BRSYNC) */
> > +#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS wait for VCS  (VRSYNC) */
> > +#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS wait for RCS  (RVSYNC) */
> > +#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS wait for BCS  (BVSYNC) */
> > +#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS wait for VCS  (VBSYNC) */
> > +#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS wait for RCS  (RBSYNC) */
> 
> Huum aren't the register names inverted in the comment? RCS waiting for
> BCS would be RBSYNC? (Register read by RCS, written by BCS)
> 
> -- 
> Damien

Admittedly this code is quite confusing. So I won't go past, I *think*
The current comments are correct.

Using the example of the render waiting on the blitter:

gen6_ring_sync(render, blitter, seqno)
iender emits a wait on:
blitter->semaphore_register[RCS] (MI_SEMAPHORE_SYNC_BR)

gen6_add_request(blitter)
update_mboxes(GEN6_RBSYNC)
update_mboxes(GEN6_VBSYNC)

So in this case:
MI_SEMAPHORE_SYNC_BR <==> GEN6_RBSYNC



> 
> >  #define  MI_SEMAPHORE_SYNC_INVALID  (1<<0)
> >  /*
> >   * 3D instructions used by the kernel
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 1d5d613..38751a7 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1666,9 +1666,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
> >  		ring->get_seqno = gen6_ring_get_seqno;
> >  		ring->set_seqno = ring_set_seqno;
> >  		ring->sync_to = gen6_ring_sync;
> > -		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID;
> > -		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_RV;
> > -		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_RB;
> > +		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
> > +		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
> > +		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
> >  		ring->signal_mbox[0] = GEN6_VRSYNC;
> >  		ring->signal_mbox[1] = GEN6_BRSYNC;
> >  	} else if (IS_GEN5(dev)) {
> > @@ -1825,9 +1825,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
> >  		ring->irq_put = gen6_ring_put_irq;
> >  		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
> >  		ring->sync_to = gen6_ring_sync;
> > -		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_VR;
> > -		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_INVALID;
> > -		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_VB;
> > +		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
> > +		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
> > +		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
> >  		ring->signal_mbox[0] = GEN6_RVSYNC;
> >  		ring->signal_mbox[1] = GEN6_BVSYNC;
> >  	} else {
> > @@ -1871,9 +1871,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
> >  	ring->irq_put = gen6_ring_put_irq;
> >  	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
> >  	ring->sync_to = gen6_ring_sync;
> > -	ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_BR;
> > -	ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_BV;
> > -	ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_INVALID;
> > +	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
> > +	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
> > +	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> >  	ring->signal_mbox[0] = GEN6_RBSYNC;
> >  	ring->signal_mbox[1] = GEN6_VBSYNC;
> >  	ring->init = init_ring_common;
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index d66208c..785df13 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -102,7 +102,7 @@ struct  intel_ring_buffer {
> >  				   struct intel_ring_buffer *to,
> >  				   u32 seqno);
> >  
> > -	u32		semaphore_register[3]; /*our mbox written by others */
> > +	u32		semaphore_register[I915_NUM_RINGS]; /*our mbox written by others */
> >  	u32		signal_mbox[2]; /* mboxes this ring signals to */
> >  	/**
> >  	 * List of objects currently involved in rendering from the
> > -- 
> > 1.8.2.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Ben Widawsky May 7, 2013, 5 p.m. UTC | #3

On Tue, May 07, 2013 at 09:51:16AM -0700, Ben Widawsky wrote:
> On Tue, May 07, 2013 at 02:54:06PM +0100, Damien Lespiau wrote:
> > On Sat, Apr 27, 2013 at 05:59:12PM -0700, Ben Widawsky wrote:
> > > Semaphores are tied very closely to the rings in the GPU. Trivial patch
> > > adds comments to the existing code so that when we add new rings we can
> > > include comments there as well. It also helps distinguish the ring to
> > > semaphore mailbox interactions by using the ringname in the semaphore
> > > data structures.
> > > 
> > > This patch should have no functional impact.
> > > 
> > > A subset of this patch was:
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++------
> > >  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++---------
> > >  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
> > >  3 files changed, 16 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index 4d66898..767aa32 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -267,12 +267,12 @@
> > >  #define  MI_SEMAPHORE_UPDATE	    (1<<21)
> > >  #define  MI_SEMAPHORE_COMPARE	    (1<<20)
> > >  #define  MI_SEMAPHORE_REGISTER	    (1<<18)
> > > -#define  MI_SEMAPHORE_SYNC_RV	    (2<<16)
> > > -#define  MI_SEMAPHORE_SYNC_RB	    (0<<16)
> > > -#define  MI_SEMAPHORE_SYNC_VR	    (0<<16)
> > > -#define  MI_SEMAPHORE_SYNC_VB	    (2<<16)
> > > -#define  MI_SEMAPHORE_SYNC_BR	    (2<<16)
> > > -#define  MI_SEMAPHORE_SYNC_BV	    (0<<16)
> > > +#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS wait for BCS  (BRSYNC) */
> > > +#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS wait for VCS  (VRSYNC) */
> > > +#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS wait for RCS  (RVSYNC) */
> > > +#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS wait for BCS  (BVSYNC) */
> > > +#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS wait for VCS  (VBSYNC) */
> > > +#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS wait for RCS  (RBSYNC) */
> > 
> > Huum aren't the register names inverted in the comment? RCS waiting for
> > BCS would be RBSYNC? (Register read by RCS, written by BCS)
> > 
> > -- 
> > Damien
> 
> Admittedly this code is quite confusing. So I won't go past, I *think*
> The current comments are correct.
> 
> Using the example of the render waiting on the blitter:
> 
> gen6_ring_sync(render, blitter, seqno)
> iender emits a wait on:
> blitter->semaphore_register[RCS] (MI_SEMAPHORE_SYNC_BR)
> 
> gen6_add_request(blitter)
> update_mboxes(GEN6_RBSYNC)
> update_mboxes(GEN6_VBSYNC)
> 
> So in this case:
> MI_SEMAPHORE_SYNC_BR <==> GEN6_RBSYNC
> 
Okay, I've fixed this locally. Thanks for catching this.
To embarrass myself less, I got confused is the names are actually
correct, the string <FOO> wait for <BAR> is the part that's incorrect.

> 
> 
> > 
> > >  #define  MI_SEMAPHORE_SYNC_INVALID  (1<<0)
> > >  /*
> > >   * 3D instructions used by the kernel
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > index 1d5d613..38751a7 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > @@ -1666,9 +1666,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
> > >  		ring->get_seqno = gen6_ring_get_seqno;
> > >  		ring->set_seqno = ring_set_seqno;
> > >  		ring->sync_to = gen6_ring_sync;
> > > -		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID;
> > > -		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_RV;
> > > -		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_RB;
> > > +		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
> > > +		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
> > > +		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
> > >  		ring->signal_mbox[0] = GEN6_VRSYNC;
> > >  		ring->signal_mbox[1] = GEN6_BRSYNC;
> > >  	} else if (IS_GEN5(dev)) {
> > > @@ -1825,9 +1825,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
> > >  		ring->irq_put = gen6_ring_put_irq;
> > >  		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
> > >  		ring->sync_to = gen6_ring_sync;
> > > -		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_VR;
> > > -		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_INVALID;
> > > -		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_VB;
> > > +		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
> > > +		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
> > > +		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
> > >  		ring->signal_mbox[0] = GEN6_RVSYNC;
> > >  		ring->signal_mbox[1] = GEN6_BVSYNC;
> > >  	} else {
> > > @@ -1871,9 +1871,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
> > >  	ring->irq_put = gen6_ring_put_irq;
> > >  	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
> > >  	ring->sync_to = gen6_ring_sync;
> > > -	ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_BR;
> > > -	ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_BV;
> > > -	ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_INVALID;
> > > +	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
> > > +	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
> > > +	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> > >  	ring->signal_mbox[0] = GEN6_RBSYNC;
> > >  	ring->signal_mbox[1] = GEN6_VBSYNC;
> > >  	ring->init = init_ring_common;
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > index d66208c..785df13 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > > @@ -102,7 +102,7 @@ struct  intel_ring_buffer {
> > >  				   struct intel_ring_buffer *to,
> > >  				   u32 seqno);
> > >  
> > > -	u32		semaphore_register[3]; /*our mbox written by others */
> > > +	u32		semaphore_register[I915_NUM_RINGS]; /*our mbox written by others */
> > >  	u32		signal_mbox[2]; /* mboxes this ring signals to */
> > >  	/**
> > >  	 * List of objects currently involved in rendering from the
> > > -- 
> > > 1.8.2.1
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4d66898..767aa32 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -267,12 +267,12 @@ 
 #define  MI_SEMAPHORE_UPDATE	    (1<<21)
 #define  MI_SEMAPHORE_COMPARE	    (1<<20)
 #define  MI_SEMAPHORE_REGISTER	    (1<<18)
-#define  MI_SEMAPHORE_SYNC_RV	    (2<<16)
-#define  MI_SEMAPHORE_SYNC_RB	    (0<<16)
-#define  MI_SEMAPHORE_SYNC_VR	    (0<<16)
-#define  MI_SEMAPHORE_SYNC_VB	    (2<<16)
-#define  MI_SEMAPHORE_SYNC_BR	    (2<<16)
-#define  MI_SEMAPHORE_SYNC_BV	    (0<<16)
+#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS wait for BCS  (BRSYNC) */
+#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS wait for VCS  (VRSYNC) */
+#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS wait for RCS  (RVSYNC) */
+#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS wait for BCS  (BVSYNC) */
+#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS wait for VCS  (VBSYNC) */
+#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS wait for RCS  (RBSYNC) */
 #define  MI_SEMAPHORE_SYNC_INVALID  (1<<0)
 /*
  * 3D instructions used by the kernel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1d5d613..38751a7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1666,9 +1666,9 @@  int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->get_seqno = gen6_ring_get_seqno;
 		ring->set_seqno = ring_set_seqno;
 		ring->sync_to = gen6_ring_sync;
-		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID;
-		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_RV;
-		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_RB;
+		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
+		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
+		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
 		ring->signal_mbox[0] = GEN6_VRSYNC;
 		ring->signal_mbox[1] = GEN6_BRSYNC;
 	} else if (IS_GEN5(dev)) {
@@ -1825,9 +1825,9 @@  int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->irq_put = gen6_ring_put_irq;
 		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		ring->sync_to = gen6_ring_sync;
-		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_VR;
-		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_INVALID;
-		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_VB;
+		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
+		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
+		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
 		ring->signal_mbox[0] = GEN6_RVSYNC;
 		ring->signal_mbox[1] = GEN6_BVSYNC;
 	} else {
@@ -1871,9 +1871,9 @@  int intel_init_blt_ring_buffer(struct drm_device *dev)
 	ring->irq_put = gen6_ring_put_irq;
 	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 	ring->sync_to = gen6_ring_sync;
-	ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_BR;
-	ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_BV;
-	ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
+	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
+	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
 	ring->signal_mbox[0] = GEN6_RBSYNC;
 	ring->signal_mbox[1] = GEN6_VBSYNC;
 	ring->init = init_ring_common;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d66208c..785df13 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -102,7 +102,7 @@  struct  intel_ring_buffer {
 				   struct intel_ring_buffer *to,
 				   u32 seqno);
 
-	u32		semaphore_register[3]; /*our mbox written by others */
+	u32		semaphore_register[I915_NUM_RINGS]; /*our mbox written by others */
 	u32		signal_mbox[2]; /* mboxes this ring signals to */
 	/**
 	 * List of objects currently involved in rendering from the

[01/18] drm/i915: Comments for semaphore clarification

Commit Message

Comments

Patch