[Intel-gfx] [PATCH 5/5] drm/i915: Flatten engine init control flow

[Dave Gordon]

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?

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 at intel.com>