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

[Volkin, Bradley D]

On Tue, Jun 24, 2014 at 04:45:05AM -0700, Mateo Lozano, Oscar wrote:
> Ok, let´s try to extract something positive out of all this.
> 
> OPTION A (Ben´s proposal):
> 
> > I think the only solution for what Chris is asking for is to implement this as 1
> > context per engine, as opposed to 1 context with a context object per
> > engine. As you correctly stated, I think we all agreed the latter was fine when
> > we met. Functionally, I see no difference, but it does allow you to always use
> > a context as the sole mechanism for making any decisions and performing
> > any operations. Now without writing all the code, I can't promise it actually
> > will look better, but I think it's likely going to be a lot cleaner. Before you do
> > any changes though...
> 
> We agreed on this early on (v1), yes, but then the idea was frowned upon by Brad and then by Daniel. I cannot recall exactly why anymore, but one big reason was that the idr mechanism makes it difficult to track several contexts with the same id (and userspace only has one context handle) and something about ctx->hang_stats. From v2 on, we agreed to multiplex different engines inside one intel_context (and that´s why we renamed i915_hw_context to intel_context).

Yeah, at least for me, the reason was that the multiple structs per context id
code felt awkward given that most/all of the fields in a struct intel_context are
logically per-context rather than per-engine (vm, hang_stats, etc). It didn't
seem like the right approach to me at the time.

Brad

> 
> OPTION B (Brad´s proposal):
> 
> > So I suggested that we:
> > 
> > - Add a back pointer from struct intel_rinbuffer to intel_context (would only
> >   be valid for lrc mode)
> > - Move the intel_ringbuffer_get(engine, context) calls up to the callers
> > - Pass (engine, ringbuf) instead of (engine, context) to intel_ring_* functions
> > - Have the vfunc implemenations get the context from the ringbuffer where
> >   needed and ignore it where not
> > 
> > Looking again, we could probably add a back pointer to the intel_engine_cs
> > as well and then just pass around the ringbuf.
> 
> Sounds fine by me: intel_ringbuffer is only related to exactly one intel_engine_cs and one intel_context, so having pointers to those two makes sense.
> As before, this could be easily done within the existing code (passing intel_rinbgbuffer instead of intel_engine_cs), but Daniel wants a code split, so I can only do it for the logical ring functions.