From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 04/13] drm/i915: Make semaphore updates more precise Date: Thu, 30 Jan 2014 13:25:42 +0200 Message-ID: <20140130112542.GI9454@intel.com> References: <1391025333-31587-1-git-send-email-benjamin.widawsky@intel.com> <1391025333-31587-5-git-send-email-benjamin.widawsky@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 178BD105DDE for ; Thu, 30 Jan 2014 03:25:47 -0800 (PST) Content-Disposition: inline In-Reply-To: <1391025333-31587-5-git-send-email-benjamin.widawsky@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Ben Widawsky Cc: Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Wed, Jan 29, 2014 at 11:55:24AM -0800, 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. > = > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 43 +++++++++++++++++----------= ------ > 1 file changed, 22 insertions(+), 21 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i9= 15/intel_ringbuffer.c > index 70f7190..97789ff 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -635,24 +635,20 @@ static void render_ring_cleanup(struct intel_ring_b= uffer *ring) > static int gen6_signal(struct intel_ring_buffer *signaller, > unsigned int num_dwords) > { > +#define MBOX_UPDATE_DWORDS 4 > struct drm_device *dev =3D signaller->dev; > struct drm_i915_private *dev_priv =3D 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 +=3D ((I915_NUM_RINGS-1) * MBOX_UPDATE_DWORDS); > + num_rings =3D hweight_long(INTEL_INFO(dev)->ring_mask); > + num_dwords =3D round_up((num_rings-1) * MBOX_UPDATE_DWORDS, 2); num_dwords +=3D Also round_up() is useless since it's already a multiple of 4. Or did you mean to change it to emit only 3 dwords per mbox? > +#undef MBOX_UPDATE_DWORDS > = > - ret =3D intel_ring_begin(signaller, num_dwords); > + /* XXX: + 4 for the caller */ > + ret =3D intel_ring_begin(signaller, num_dwords + 4); The +=3D earlier gets rid of the +4 here. > if (ret) > return ret; > -#undef MBOX_UPDATE_DWORDS > = > for_each_ring(useless, dev_priv, i) { > u32 mbox_reg =3D signaller->semaphore.signal_mbox[i]; > @@ -661,14 +657,11 @@ static int gen6_signal(struct intel_ring_buffer *si= gnaller, > 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); > } > } > = > + WARN_ON(i !=3D num_rings); So we're not expecting dev_priv->ring[] to be sparsely populated ever? > + > return 0; > } > = > @@ -686,7 +679,11 @@ gen6_add_request(struct intel_ring_buffer *ring) > { > int ret; > = > - ret =3D ring->semaphore.signal(ring, 4); > + if (ring->semaphore.signal) > + ret =3D ring->semaphore.signal(ring, 4); > + else > + ret =3D intel_ring_begin(ring, 4); > + > if (ret) > return ret; > = > @@ -1881,7 +1878,8 @@ int intel_init_render_ring_buffer(struct drm_device= *dev) > ring->get_seqno =3D gen6_ring_get_seqno; > ring->set_seqno =3D ring_set_seqno; > ring->semaphore.sync_to =3D gen6_ring_sync; > - ring->semaphore.signal =3D gen6_signal; > + if (i915_semaphore_is_enabled(dev)) > + ring->semaphore.signal =3D gen6_signal; I guess we could also set .sync_to conditionally, but doesn't really matter since we won't call it anyway w/o semaphores enabled. > ring->semaphore.mbox[RCS] =3D MI_SEMAPHORE_SYNC_INVALID; > ring->semaphore.mbox[VCS] =3D MI_SEMAPHORE_SYNC_RV; > ring->semaphore.mbox[BCS] =3D MI_SEMAPHORE_SYNC_RB; > @@ -2058,7 +2056,8 @@ int intel_init_bsd_ring_buffer(struct drm_device *d= ev) > gen6_ring_dispatch_execbuffer; > } > ring->semaphore.sync_to =3D gen6_ring_sync; > - ring->semaphore.signal =3D gen6_signal; > + if (i915_semaphore_is_enabled(dev)) > + ring->semaphore.signal =3D gen6_signal; > ring->semaphore.mbox[RCS] =3D MI_SEMAPHORE_SYNC_VR; > ring->semaphore.mbox[VCS] =3D MI_SEMAPHORE_SYNC_INVALID; > ring->semaphore.mbox[BCS] =3D MI_SEMAPHORE_SYNC_VB; > @@ -2116,7 +2115,8 @@ int intel_init_blt_ring_buffer(struct drm_device *d= ev) > ring->dispatch_execbuffer =3D gen6_ring_dispatch_execbuffer; > } > ring->semaphore.sync_to =3D gen6_ring_sync; > - ring->semaphore.signal =3D gen6_signal; > + if (i915_semaphore_is_enabled(dev)) > + ring->semaphore.signal =3D gen6_signal; > ring->semaphore.mbox[RCS] =3D MI_SEMAPHORE_SYNC_BR; > ring->semaphore.mbox[VCS] =3D MI_SEMAPHORE_SYNC_BV; > ring->semaphore.mbox[BCS] =3D MI_SEMAPHORE_SYNC_INVALID; > @@ -2158,7 +2158,8 @@ int intel_init_vebox_ring_buffer(struct drm_device = *dev) > ring->dispatch_execbuffer =3D gen6_ring_dispatch_execbuffer; > } > ring->semaphore.sync_to =3D gen6_ring_sync; > - ring->semaphore.signal =3D gen6_signal; > + if (i915_semaphore_is_enabled(dev)) > + ring->semaphore.signal =3D gen6_signal; > ring->semaphore.mbox[RCS] =3D MI_SEMAPHORE_SYNC_VER; > ring->semaphore.mbox[VCS] =3D MI_SEMAPHORE_SYNC_VEV; > ring->semaphore.mbox[BCS] =3D MI_SEMAPHORE_SYNC_VEB; > -- = > 1.8.5.3 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC