On Mon, Dec 01, 2014 at 04:11:15PM +0000, Dave Gordon wrote: > On 20/11/14 09:19, Daniel Vetter wrote: > > On Thu, Nov 20, 2014 at 08:10:27AM +0000, Chris Wilson wrote: > >> On Thu, Nov 20, 2014 at 12:33:08AM +0100, Daniel Vetter wrote: > >>> Now that sanity prevails and we have the clean split between software > >>> init and starting the engines we can drop all the "have we allocate > >>> this struct already?" nonsense. > >> > >> Actually, I was hoping for something completely different when you sad > >> flattening the init... > > > > I presume you've hoped for more ;-) Essentially this is just the legacy > > ringbuffer blueprint, applying to execlist left as an exercise for the > > reader. And it's just a very small patch. > > > >> Compare with > >> http://cgit.freedesktop.org/~ickle/linux-2.6/tree/drivers/gpu/drm/i915/intel_ringbuffer.c#n1983 > >> > >> There is a lot of duplicated code in the ringbuffer setup, and the code > >> movement here wants to be split differently so that the sequence is > >> identical to execlists. (The only difference is that we only use one > >> ring with legacy, rather than one ring in each context [per-engine].) > >> Making sure the logic is foolproof for both is esssential. > > > > Imo we first need to beat execlist into shape (with small patches, not by > > rewriting it all). Once stuff settles we can take another look at whether > > unification makes sense, but for now I want a big piece of insulation > > between execlist and stuff we actually depend on. Or at least stuff Dave > > might rip my head off over. > > > > So for now I still want to embrace the duplication. > > -Daniel > > As a matter of policy: while we have the duplication and the split > between legacy ringbuffer and lrc versions, should developers try to > submit corresponding changes to both paths together (e.g. in the same > patchset). Or is it OK to submit improvements only to one of the two > paths, causing them to diverge and hence possibly making deduplication > more difficult? Yeah in this case I've figured I'll cause more trouble than benefit when I trample all over intel_lrc.c. Also, I don't have a machine for testing execlist ;-) > > Anyway, this change looks OK in itself, so subject to the "exercise for > the reader" approach being acceptable (on which point I defer to your > and others judgement), then: > > Reviewed-by: Dave Gordon <david.s.gordon@xxxxxxxxx> Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
