[Intel-gfx] [PATCH 51/59] drm/i915: Add *_ring_begin() to request allocation

[Chris Wilson]

On Fri, Mar 20, 2015 at 04:23:45PM +0100, Daniel Vetter wrote:
> On Thu, Mar 19, 2015 at 12:30:56PM +0000, John.C.Harrison at Intel.com wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 6f198df..c7dcabd 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -2205,21 +2205,27 @@ int intel_ring_alloc_request_extras(struct drm_i915_gem_request *request)
> >  	return 0;
> >  }
> >  
> > -int intel_ring_reserved_space_reserve(struct intel_ringbuffer *ringbuf, int size)
> > +int legacy_ring_reserved_space_reserve(struct drm_i915_gem_request *request)
> >  {
> > -	/* NB: Until request management is fully tidied up and the OLR is
> > -	 * removed, there are too many ways for get false hits on this
> > -	 * anti-recursion check! */
> > -	/*WARN_ON(ringbuf->reserved_size);*/
> > +	/*
> > +	 * The first call merely notes the reserve request and is common for
> > +	 * all back ends. The subsequent localised _begin() call actually
> > +	 * ensures that the reservation is available. Without the begin, if
> > +	 * the request creator immediately submitted the request without
> > +	 * adding any commands to it then there might not actually be
> > +	 * sufficient room for the submission commands.
> > +	 */
> > +	intel_ring_reserved_space_reserve(request->ringbuf, MIN_SPACE_FOR_ADD_REQUEST);
> > +
> > +	return intel_ring_begin(request, 0);
> 
> This feels a bit convoluted tbh, and would fall aparat if we start adding
> sanity checks to _begin/advance functions. Can't we instead directly call
> ring_wait_for_space? This forgoes the intel_wrap_ring_buffer call, but
> otoh we just need to factor that into our estimates. Wrapping the ring for
> the entire reservartion right away is
> a) way too much - we only wrap individual ring_being calls anyway
> b) not doing any good since all the intermediate intel_ring_emit calls
> might very-well push us into a wrap anyway.
> 
> In the end we just need to increase our reservation with the biggest
> intel_ring_begin we have in the add_request code - that's the worst-case
> of ring space we might waste due to wrapping.

Sorry to tune in here but what? What happened to the transactional
request model?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre