On Mon, 23 Jun 2014 14:13:55 +0100 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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... Jesse _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
