[Intel-gfx] [CI 13/25] drm/i915: Remove the identical implementations of request space reservation

[Chris Wilson]

On Thu, Apr 28, 2016 at 03:02:18PM +0100, Dave Gordon wrote:
> On 28/04/16 09:56, Chris Wilson wrote:
> >Now that we share intel_ring_begin(), reserving space for the tail of
> >the request is identical between legacy/execlists and so the tautology
> >can be removed. In the process, we move the reserved space tracking
> >from the ringbuffer on to the request. This is to enable us to reorder
> >the reserved space allocation in the next patch.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> >Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_drv.h         |  3 +++
> >  drivers/gpu/drm/i915/i915_gem.c         | 23 ++++++++++-------
> >  drivers/gpu/drm/i915/intel_lrc.c        | 15 -----------
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 44 +++------------------------------
> >  drivers/gpu/drm/i915/intel_ringbuffer.h | 17 -------------
> >  5 files changed, 20 insertions(+), 82 deletions(-)
> 
> While you're doing all this reconvergence of the different
> submission mechanisms, how about splitting intel_ringbuffer.c into
> one file concerned with operations on actual ringbuffers (e.g. all
> the reserve, wrap, fill, emit stuff) independent of the submission
> code, and a separate one for the legacy ringbuffer submission
> mechanism.
> 
> Ideally, we could also do the same to intel_lrc.c, with only those
> operations independent of submission mechanism but unique to Logical
> Ring Contexts (as opposed to just ringbuffers) remaining in that
> file, with a separate file again for the execlists submission code.
> 
> That would give us five files in total, split like this:
> * ringbuffer.c           common to *all* ring manipulation
> * lrc.c                  common code for logical contexts
> 
> * legacy_submission.c    TAIL, UHPTR, MI_SWITCH_CONTEXT, etc
> * execlist_submission.c  ELSP, CSB interrupts, etc
> * guc_submission.c       GuC WQ, doorbells, etc
> 
> Or would this just be too disruptive?

That split matches the organisation and flow of the code quite well. You
don't mind if I do this towards the end of a long series of patches?

I am in favour. Any one else?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre