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

[Mateo Lozano, Oscar]

> -----Original Message-----
> From: Jesse Barnes [mailto:jbarnes at virtuousgeek.org]
> Sent: Tuesday, June 24, 2014 6:20 PM
> To: Chris Wilson
> Cc: Mateo Lozano, Oscar; intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 26/53] drm/i915/bdw: New logical ring
> submission mechanism
> 
> On Mon, 23 Jun 2014 14:13:55 +0100
> Chris Wilson <chris at chris-wilson.co.uk> wrote:
> 
> > 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?
> >
> > I'm still baffled by the design. intel_ring_begin() and friends should
> > be able to find their context (logical or legacy) from the ring and
> > dtrt.
> 
> To me, given that the LRC contains the ring, the right thing to do would be to
> do what Oscar did in the first place: pass the context around everywhere.  If
> there were cases where that wasn't ideal (the layering violations you
> mention later?) we should fix them up instead.
> 
> But given that this is a standalone file, it's easy to fix up however we want
> incrementally, as long as things work well to begin with and it's reasonably
> easy to review it...

Hi Jesse,

I spoke with Chris on IRC, and he told me that he is currently rewriting this stuff to show (and prove) what his approach is.

In the meantime, I´ll keep working on the review comments I have received from Brad and Daniel and I´ll also send some early patches with prep-work that (hopefully) can be merged without too much fuss.

-- Oscar