[Intel-gfx] [PATCH 13/18] drm/i915: Stop passing caller's num_dwords to engine->semaphore.signal()

[Chris Wilson]

On Fri, Jul 22, 2016 at 11:15:59AM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-20 at 14:12 +0100, Chris Wilson wrote:
> > Rather than pass in the num_dwords that the caller wishes to use after
> > the signal command packet, split the breadcrumb emission into two phases
> > and have both the signal and breadcrumb individiually acquire space on
> > the ring. This makes the interface simpler for the reader, and will
> > simplify for patches.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 51 ++++++++++++++-------------------
> >  drivers/gpu/drm/i915/intel_ringbuffer.h |  4 +--
> >  2 files changed, 23 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 907d933d62aa..9c66745fc8d7 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1308,10 +1308,8 @@ static void render_ring_cleanup(struct intel_engine_cs *engine)
> >  	intel_fini_pipe_control(engine);
> >  }
> >  
> > -static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
> > -			   unsigned int num_dwords)
> > +static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req)
> >  {
> > -#define MBOX_UPDATE_DWORDS 8
> >  	struct intel_ring *signaller = signaller_req->ring;
> >  	struct drm_i915_private *dev_priv = signaller_req->i915;
> >  	struct intel_engine_cs *waiter;
> > @@ -1319,10 +1317,7 @@ static int gen8_rcs_signal(struct drm_i915_gem_request *signaller_req,
> >  	int ret, num_rings;
> >  
> >  	num_rings = hweight32(INTEL_INFO(dev_priv)->ring_mask);
> > -	num_dwords += (num_rings-1) * MBOX_UPDATE_DWORDS;
> > -#undef MBOX_UPDATE_DWORDS
> > -
> > -	ret = intel_ring_begin(signaller_req, num_dwords);
> > +	ret = intel_ring_begin(signaller_req, (num_rings-1) * 8);
> 
> Magic number. Just make the defines GEN?_?CS_MBOX_UPDATE_DWORDS? 

No. It is important that these are very clear as the reviewer is
required to check the number of dwords emitted.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre