[Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring submission mechanism

[Mateo Lozano, Oscar]

> -----Original Message-----
> From: Volkin, Bradley D
> Sent: Monday, June 23, 2014 8:10 PM
> To: Mateo Lozano, Oscar
> Cc: Chris Wilson; intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring
> submission mechanism
> 
> On Mon, Jun 23, 2014 at 07:35:38AM -0700, Mateo Lozano, Oscar wrote:
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > > Sent: Monday, June 23, 2014 2:42 PM
> > > To: Mateo Lozano, Oscar
> > > Cc: Volkin, Bradley D; intel-gfx at lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical
> > > ring submission mechanism
> > >
> > > On Mon, Jun 23, 2014 at 01:36:07PM +0000, Mateo Lozano, Oscar
> wrote:
> > > > > -----Original Message-----
> > > > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > > > > Sent: Monday, June 23, 2014 2:27 PM
> > > > > To: Mateo Lozano, Oscar
> > > > > Cc: Volkin, Bradley D; intel-gfx at lists.freedesktop.org
> > > > > Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical
> > > > > ring submission mechanism
> > > > >
> > > > > On Mon, Jun 23, 2014 at 01:18:35PM +0000, Mateo Lozano, Oscar
> > > wrote:
> > > > > > > -----Original Message-----
> > > > > > > From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
> > > > > > > Sent: Monday, June 23, 2014 2:14 PM
> > > > > > > To: Mateo Lozano, Oscar
> > > > > > > Cc: Volkin, Bradley D; intel-gfx at lists.freedesktop.org
> > > > > > > Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New
> > > > > > > logical ring submission mechanism
> > > > > > >
> > > > > > > On Mon, Jun 23, 2014 at 01:09:37PM +0000, Mateo Lozano,
> > > > > > > Oscar
> > > > > wrote:
> > > > > > > > So far, yes, but that´s only because I artificially made
> > > > > > > > intel_lrc.c self-
> > > > > > > contained, as Daniel requested. What if we need to execute
> > > > > > > commands from somewhere else, like in intel_gen7_queue_flip()?
> > > > > > > >
> > > > > > > > And this takes me to another discussion: this logical ring
> > > > > > > > vs legacy ring split
> > > > > > > is probably a good idea (time will tell), but we should
> > > > > > > provide a way of sending commands for execution without
> > > > > > > knowing if Execlists are enabled or not. In the early series
> > > > > > > that was easy because we reused the ring_begin, ring_emit &
> > > > > > > ring_advance functions, but this is not the case anymore.
> > > > > > > And without this, sooner or later somebody will break legacy
> > > > > > > or execlists (this already happened last week, when somebody
> > > > > > > here was implementing native sync without knowing
> > > > > about Execlists).
> > > > > > > >
> > > > > > > > So, the questions is: how do you feel about a dev_priv.gt
> > > > > > > > vfunc that takes a
> > > > > > > context, a ring, an array of DWORDS and a BB length and does
> > > > > > > the intel_(logical)_ring_begin/emit/advance based on
> > > i915.enable_execlists?
> 
> There are 3 cases of non-execbuffer submissions that I can think of: flips,
> render state, and clear-buffer (proposed patches on the list). I wonder if the
> right approach might be to use batchbuffers with a small wrapper around the
> dispatch_execbuffer/emit_bb_start vfuncs. Basically the rule would be to
> only touch a ringbuffer from within the intel_engine_cs vfuncs, which always
> know which set of functions to use.
> 
> For flips, we could use MMIO flips. Render state already uses the existing
> dispatch_execbuffer() and add_request(). The clear code could potentially do
> the same. There would obviously be some overhead in using a batch buffer
> for what could end up being just a few commands. Perhaps the batch buffer
> pool code from the command parser would help though.

This has another positive side-effect: the scheduler guys do not like things inside the ring without a proper batchbuffer & request, because it makes their life more complex.