On Thu, Apr 28, 2016 at 03:31:00PM +0100, Chris Wilson wrote: > 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@xxxxxxxxxxxxxxxxxx> > > >Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > >Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> > > >Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > >--- > > > 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? +1 Bonus points if it comes with a bit of kerneldoc explaining the concepts and main interfaces ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
