On Tue, May 07, 2013 at 03:49:18PM +0100, Damien Lespiau wrote:
> On Sat, Apr 27, 2013 at 05:59:15PM -0700, Ben Widawsky wrote:
> > Like the other rings, the VECS supports semaphores. The semaphore stuff
> > is a bit wonky so this patch on it's own should be nice for review.
> >
> > This patch should have no functional impact.
> >
> > Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 40
> > ++++++++++++++++++++++-----------
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++---
> > 2 files changed, 33 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 5be4a75..3899f71 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -267,13 +267,19 @@
> > #define MI_SEMAPHORE_UPDATE (1<<21)
> > #define MI_SEMAPHORE_COMPARE (1<<20)
> > #define MI_SEMAPHORE_REGISTER (1<<18)
> > -#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)
> > +#define MI_SEMAPHORE_SYNC_VR (0<<16) /* VCS wait for RCS
> > (RVSYNC) */
> > +#define MI_SEMAPHORE_SYNC_VER (1<<16) /* VECS wait for RCS
> > (RVESYNC) */
> > +#define MI_SEMAPHORE_SYNC_BR (2<<16) /* BCS wait for RCS
> > (RBSYNC) */
> > +#define MI_SEMAPHORE_SYNC_BV (0<<16) /* BCS wait for VCS
> > (VBSYNC) */
> > +#define MI_SEMAPHORE_SYNC_VEV (1<<16) /* VECS wait for VCS
> > (VVESYNC) */
> > +#define MI_SEMAPHORE_SYNC_RV (2<<16) /* RCS wait for VCS
> > (VRSYNC) */
> > +#define MI_SEMAPHORE_SYNC_RB (0<<16) /* RCS wait for BCS
> > (BRSYNC) */
> > +#define MI_SEMAPHORE_SYNC_VEB (1<<16) /* VECS wait for BCS
> > (BVESYNC) */
> > +#define MI_SEMAPHORE_SYNC_VB (2<<16) /* VCS wait for BCS
> > (BVSYNC) */
> > +#define MI_SEMAPHORE_SYNC_BVE (0<<16) /* BCS wait for VECS
> > (VEBSYNC) */
> > +#define MI_SEMAPHORE_SYNC_VVE (1<<16) /* VCS wait for VECS
> > (VEVSYNC) */
> > +#define MI_SEMAPHORE_SYNC_RVE (2<<16) /* RCS wait for VECS
> > (VERSYNC) */
> > +#define MI_SEMAPHORE_SYNC_INVALID (3<<16)
>
> Same thing as in patch 1, I think the registers are the other way
> around.
Yep, fixed.
>
> > /*
> > * 3D instructions used by the kernel
> > */
> > @@ -556,6 +562,7 @@
> > #define RENDER_RING_BASE 0x02000
> > #define BSD_RING_BASE 0x04000
> > #define GEN6_BSD_RING_BASE 0x12000
> > +#define VEBOX_RING_BASE 0x1a000
> > #define BLT_RING_BASE 0x22000
> > #define RING_TAIL(base) ((base)+0x30)
> > #define RING_HEAD(base) ((base)+0x34)
> > @@ -563,13 +570,20 @@
> > #define RING_CTL(base) ((base)+0x3c)
> > #define RING_SYNC_0(base) ((base)+0x40)
> > #define RING_SYNC_1(base) ((base)+0x44)
> > -#define GEN6_RVSYNC (RING_SYNC_0(RENDER_RING_BASE))
> > -#define GEN6_RBSYNC (RING_SYNC_1(RENDER_RING_BASE))
> > -#define GEN6_VRSYNC (RING_SYNC_1(GEN6_BSD_RING_BASE))
> > -#define GEN6_VBSYNC (RING_SYNC_0(GEN6_BSD_RING_BASE))
> > -#define GEN6_BRSYNC (RING_SYNC_0(BLT_RING_BASE))
> > +#define RING_SYNC_2(base) ((base)+0x48)
> > +#define GEN6_RVSYNC (RING_SYNC_0(RENDER_RING_BASE))
> > +#define GEN6_RBSYNC (RING_SYNC_1(RENDER_RING_BASE))
> > +#define GEN6_RVESYNC (RING_SYNC_2(RENDER_RING_BASE))
> > +#define GEN6_VBSYNC (RING_SYNC_0(GEN6_BSD_RING_BASE))
> > +#define GEN6_VRSYNC (RING_SYNC_1(GEN6_BSD_RING_BASE))
> > +#define GEN6_VVESYNC (RING_SYNC_2(GEN6_BSD_RING_BASE))
> > +#define GEN6_BRSYNC (RING_SYNC_0(BLT_RING_BASE))
> > +#define GEN6_BVSYNC (RING_SYNC_1(BLT_RING_BASE))
> > +#define GEN6_BVESYNC (RING_SYNC_2(BLT_RING_BASE))
> > +#define GEN6_VEBSYNC (RING_SYNC_0(VEBOX_RING_BASE))
> > +#define GEN6_VERSYNC (RING_SYNC_1(VEBOX_RING_BASE))
> > +#define GEN6_VEVSYNC (RING_SYNC_2(VEBOX_RING_BASE))
> > #define GEN6_NOSYNC 0
> > -#define GEN6_BVSYNC (RING_SYNC_1(BLT_RING_BASE))
> > #define RING_MAX_IDLE(base) ((base)+0x54)
> > #define RING_HWS_PGA(base) ((base)+0x80)
> > #define RING_HWS_PGA_GEN6(base) ((base)+0x2080)
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 555f8b8..b597d1e 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -654,9 +654,6 @@ gen6_ring_sync(struct intel_ring_buffer *waiter,
> > */
> > seqno -= 1;
> >
> > - WARN_ON(signaller->semaphore_register[waiter->id] ==
> > - MI_SEMAPHORE_SYNC_INVALID);
> > -
>
> Hum, this WARN_ON() is still valid isn't it?
I must have killed this by accident while rebasing. It is indeed valid. Nice
catch.
>
> > ret = intel_ring_begin(waiter, 4);
> > if (ret)
> > return ret;
> > @@ -1678,9 +1675,11 @@ int intel_init_render_ring_buffer(struct drm_device
> > *dev)
> > 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->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_RVE;
> > ring->signal_mbox[RCS] = GEN6_NOSYNC;
> > ring->signal_mbox[VCS] = GEN6_VRSYNC;
> > ring->signal_mbox[BCS] = GEN6_BRSYNC;
> > + ring->signal_mbox[VECS] = GEN6_VERSYNC;
> > } else if (IS_GEN5(dev)) {
> > ring->add_request = pc_render_add_request;
> > ring->flush = gen4_render_ring_flush;
> > @@ -1838,9 +1837,11 @@ int intel_init_bsd_ring_buffer(struct drm_device
> > *dev)
> > 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->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_VEV;
>
> MI_SEMAPHORE_SYNC_VVE here?
Weird. I remember Haihao had caught this typo earlier, and I fixed it
earlier. I'm beginning to fear I may have rebased the wrong branch...
Anyway, thanks for finding it again.
Got all the fixes locally. Will send when I'm done with the other
changes.
>
> > ring->signal_mbox[RCS] = GEN6_RVSYNC;
> > ring->signal_mbox[VCS] = GEN6_NOSYNC;
> > ring->signal_mbox[BCS] = GEN6_BVSYNC;
> > + ring->signal_mbox[VECS] = GEN6_VEVSYNC;
> > } else {
> > ring->mmio_base = BSD_RING_BASE;
> > ring->flush = bsd_ring_flush;
> > @@ -1885,9 +1886,11 @@ int intel_init_blt_ring_buffer(struct drm_device
> > *dev)
> > 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->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_BVE;
> > ring->signal_mbox[RCS] = GEN6_RBSYNC;
> > ring->signal_mbox[VCS] = GEN6_VBSYNC;
> > ring->signal_mbox[BCS] = GEN6_NOSYNC;
> > + ring->signal_mbox[VECS] = GEN6_VEBSYNC;
> > ring->init = init_ring_common;
> >
> > return intel_init_ring_buffer(dev, ring);
> > --
> > 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
