On Tue, Apr 29, 2014 at 02:52:31PM -0700, Ben Widawsky wrote:
> With the ring mask we now have an easy way to know the number of rings
> in the system, and therefore can accurately predict the number of dwords
> to emit for semaphore signalling. This was not possible (easily)
> previously.
>
> There should be no functional impact, simply fewer instructions emitted.
>
> While we're here, simply do the round up to 2 instead of the fancier
> rounding we did before, which rounding up per mbox, ie 4. This also
> allows us to drop the unnecessary MI_NOOP, so not really 4, 3.
>
> v2: Use 3 dwords instead of 4 (Ville)
> Do the proper calculation to get the number of dwords to emit (Ville)
> Conditionally set .sync_to when semaphores are enabled (Ville)
>
> v3: Rebased on VCS2
> Replace hweight_long with hweight32 (Ville)
>
> Reviewed-by: Ville Syrjälä <[email protected]> (v1)
> Signed-off-by: Ben Widawsky <[email protected]>
> ---
> drivers/gpu/drm/i915/intel_ringbuffer.c | 173
> +++++++++++++++++---------------
> 1 file changed, 90 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index e0c7bf2..7aedc0c 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -666,24 +666,19 @@ static void render_ring_cleanup(struct
> intel_ring_buffer *ring)
> static int gen6_signal(struct intel_ring_buffer *signaller,
> unsigned int num_dwords)
> {
> +#define MBOX_UPDATE_DWORDS 3
> struct drm_device *dev = signaller->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_ring_buffer *useless;
> - int i, ret;
> + int i, ret, num_rings;
>
> - /* NB: In order to be able to do semaphore MBOX updates for varying
> - * number of rings, it's easiest if we round up each individual update
> - * to a multiple of 2 (since ring updates must always be a multiple of
> - * 2) even though the actual update only requires 3 dwords.
> - */
> -#define MBOX_UPDATE_DWORDS 4
> - if (i915_semaphore_is_enabled(dev))
> - num_dwords += ((I915_NUM_RINGS-1) * MBOX_UPDATE_DWORDS);
> + num_rings = hweight32(INTEL_INFO(dev)->ring_mask);
> + num_dwords += round_up((num_rings-1) * MBOX_UPDATE_DWORDS, 2);
> +#undef MBOX_UPDATE_DWORDS
>
> ret = intel_ring_begin(signaller, num_dwords);
> if (ret)
> return ret;
> -#undef MBOX_UPDATE_DWORDS
>
> for_each_ring(useless, dev_priv, i) {
> u32 mbox_reg = signaller->semaphore.mbox.signal[i];
> @@ -691,15 +686,13 @@ static int gen6_signal(struct intel_ring_buffer
> *signaller,
> intel_ring_emit(signaller, MI_LOAD_REGISTER_IMM(1));
> intel_ring_emit(signaller, mbox_reg);
> intel_ring_emit(signaller,
> signaller->outstanding_lazy_seqno);
> - intel_ring_emit(signaller, MI_NOOP);
> - } else {
> - intel_ring_emit(signaller, MI_NOOP);
> - intel_ring_emit(signaller, MI_NOOP);
> - intel_ring_emit(signaller, MI_NOOP);
> - intel_ring_emit(signaller, MI_NOOP);
> }
> }
>
> + /* If num_dwords was rounded, make sure the tail pointer is correct */
> + if (num_rings % 2 == 0)
> + intel_ring_emit(signaller, MI_NOOP);
> +
> return 0;
> }
>
> @@ -717,7 +710,11 @@ gen6_add_request(struct intel_ring_buffer *ring)
> {
> int ret;
>
> - ret = ring->semaphore.signal(ring, 4);
> + if (ring->semaphore.signal)
> + ret = ring->semaphore.signal(ring, 4);
> + else
> + ret = intel_ring_begin(ring, 4);
> +
> if (ret)
> return ret;
>
The hunks below look like a different patch. Accidental squash while
rebasing?
I've merged patches 1-3 of this series already.
-Daniel
> @@ -1928,24 +1925,27 @@ int intel_init_render_ring_buffer(struct drm_device
> *dev)
> ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
> ring->get_seqno = gen6_ring_get_seqno;
> ring->set_seqno = ring_set_seqno;
> - ring->semaphore.sync_to = gen6_ring_sync;
> - ring->semaphore.signal = gen6_signal;
> - /*
> - * The current semaphore is only applied on pre-gen8 platform.
> - * And there is no VCS2 ring on the pre-gen8 platform. So the
> - * semaphore between RCS and VCS2 is initialized as INVALID.
> - * Gen8 will initialize the sema between VCS2 and RCS later.
> - */
> - ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_RV;
> - ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_RB;
> - ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_RVE;
> - ring->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.signal[RCS] = GEN6_NOSYNC;
> - ring->semaphore.mbox.signal[VCS] = GEN6_VRSYNC;
> - ring->semaphore.mbox.signal[BCS] = GEN6_BRSYNC;
> - ring->semaphore.mbox.signal[VECS] = GEN6_VERSYNC;
> - ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + if (i915_semaphore_is_enabled(dev)) {
> + ring->semaphore.sync_to = gen6_ring_sync;
> + ring->semaphore.signal = gen6_signal;
> + /*
> + * The current semaphore is only applied on pre-gen8
> + * platform. And there is no VCS2 ring on the pre-gen8
> + * platform. So the semaphore between RCS and VCS2 is
> + * initialized as INVALID. Gen8 will initialize the
> + * sema between VCS2 and RCS later.
> + */
> + ring->semaphore.mbox.wait[RCS] =
> MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_RV;
> + ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_RB;
> + ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_RVE;
> + ring->semaphore.mbox.wait[VCS2] =
> MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.signal[RCS] = GEN6_NOSYNC;
> + ring->semaphore.mbox.signal[VCS] = GEN6_VRSYNC;
> + ring->semaphore.mbox.signal[BCS] = GEN6_BRSYNC;
> + ring->semaphore.mbox.signal[VECS] = GEN6_VERSYNC;
> + ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + }
> } else if (IS_GEN5(dev)) {
> ring->add_request = pc_render_add_request;
> ring->flush = gen4_render_ring_flush;
> @@ -2113,24 +2113,27 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
> ring->dispatch_execbuffer =
> gen6_ring_dispatch_execbuffer;
> }
> - ring->semaphore.sync_to = gen6_ring_sync;
> - ring->semaphore.signal = gen6_signal;
> - /*
> - * The current semaphore is only applied on pre-gen8 platform.
> - * And there is no VCS2 ring on the pre-gen8 platform. So the
> - * semaphore between VCS and VCS2 is initialized as INVALID.
> - * Gen8 will initialize the sema between VCS2 and VCS later.
> - */
> - ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VR;
> - ring->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VB;
> - ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_VVE;
> - ring->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.signal[RCS] = GEN6_RVSYNC;
> - ring->semaphore.mbox.signal[VCS] = GEN6_NOSYNC;
> - ring->semaphore.mbox.signal[BCS] = GEN6_BVSYNC;
> - ring->semaphore.mbox.signal[VECS] = GEN6_VEVSYNC;
> - ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + if (i915_semaphore_is_enabled(dev)) {
> + ring->semaphore.sync_to = gen6_ring_sync;
> + ring->semaphore.signal = gen6_signal;
> + /*
> + * The current semaphore is only applied on pre-gen8
> + * platform. And there is no VCS2 ring on the pre-gen8
> + * platform. So the semaphore between VCS and VCS2 is
> + * initialized as INVALID. Gen8 will initialize the
> + * sema between VCS2 and VCS later.
> + */
> + ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VR;
> + ring->semaphore.mbox.wait[VCS] =
> MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VB;
> + ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_VVE;
> + ring->semaphore.mbox.wait[VCS2] =
> MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.signal[RCS] = GEN6_RVSYNC;
> + ring->semaphore.mbox.signal[VCS] = GEN6_NOSYNC;
> + ring->semaphore.mbox.signal[BCS] = GEN6_BVSYNC;
> + ring->semaphore.mbox.signal[VECS] = GEN6_VEVSYNC;
> + ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + }
> } else {
> ring->mmio_base = BSD_RING_BASE;
> ring->flush = bsd_ring_flush;
> @@ -2231,24 +2234,26 @@ 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->semaphore.sync_to = gen6_ring_sync;
> - ring->semaphore.signal = gen6_signal;
> - /*
> - * The current semaphore is only applied on pre-gen8 platform. And
> - * there is no VCS2 ring on the pre-gen8 platform. So the semaphore
> - * between BCS and VCS2 is initialized as INVALID.
> - * Gen8 will initialize the sema between BCS and VCS2 later.
> - */
> - ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_BR;
> - ring->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_BV;
> - ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_BVE;
> - ring->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.signal[RCS] = GEN6_RBSYNC;
> - ring->semaphore.mbox.signal[VCS] = GEN6_VBSYNC;
> - ring->semaphore.mbox.signal[BCS] = GEN6_NOSYNC;
> - ring->semaphore.mbox.signal[VECS] = GEN6_VEBSYNC;
> - ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + if (i915_semaphore_is_enabled(dev)) {
> + ring->semaphore.signal = gen6_signal;
> + ring->semaphore.sync_to = gen6_ring_sync;
> + /*
> + * The current semaphore is only applied on pre-gen8 platform.
> + * And there is no VCS2 ring on the pre-gen8 platform. So the
> + * semaphore between BCS and VCS2 is initialized as INVALID.
> + * Gen8 will initialize the sema between BCS and VCS2 later.
> + */
> + ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_BR;
> + ring->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_BV;
> + ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_BVE;
> + ring->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.signal[RCS] = GEN6_RBSYNC;
> + ring->semaphore.mbox.signal[VCS] = GEN6_VBSYNC;
> + ring->semaphore.mbox.signal[BCS] = GEN6_NOSYNC;
> + ring->semaphore.mbox.signal[VECS] = GEN6_VEBSYNC;
> + ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + }
> ring->init = init_ring_common;
>
> return intel_init_ring_buffer(dev, ring);
> @@ -2281,18 +2286,20 @@ int intel_init_vebox_ring_buffer(struct drm_device
> *dev)
> ring->irq_put = hsw_vebox_put_irq;
> ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
> }
> - ring->semaphore.sync_to = gen6_ring_sync;
> - ring->semaphore.signal = gen6_signal;
> - ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VER;
> - ring->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_VEV;
> - ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VEB;
> - ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> - ring->semaphore.mbox.signal[RCS] = GEN6_RVESYNC;
> - ring->semaphore.mbox.signal[VCS] = GEN6_VVESYNC;
> - ring->semaphore.mbox.signal[BCS] = GEN6_BVESYNC;
> - ring->semaphore.mbox.signal[VECS] = GEN6_NOSYNC;
> - ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + if (i915_semaphore_is_enabled(dev)) {
> + ring->semaphore.sync_to = gen6_ring_sync;
> + ring->semaphore.signal = gen6_signal;
> + ring->semaphore.mbox.wait[RCS] = MI_SEMAPHORE_SYNC_VER;
> + ring->semaphore.mbox.wait[VCS] = MI_SEMAPHORE_SYNC_VEV;
> + ring->semaphore.mbox.wait[BCS] = MI_SEMAPHORE_SYNC_VEB;
> + ring->semaphore.mbox.wait[VECS] = MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.wait[VCS2] = MI_SEMAPHORE_SYNC_INVALID;
> + ring->semaphore.mbox.signal[RCS] = GEN6_RVESYNC;
> + ring->semaphore.mbox.signal[VCS] = GEN6_VVESYNC;
> + ring->semaphore.mbox.signal[BCS] = GEN6_BVESYNC;
> + ring->semaphore.mbox.signal[VECS] = GEN6_NOSYNC;
> + ring->semaphore.mbox.signal[VCS2] = GEN6_NOSYNC;
> + }
> ring->init = init_ring_common;
>
> return intel_init_ring_buffer(dev, ring);
> --
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
