[Intel-gfx] [PATCH 3/5] drm/i915: Unify engine init loop

[Chris Wilson]

On Wed, Jul 06, 2016 at 11:52:09AM +0100, Tvrtko Ursulin wrote:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index fcff30998227..5bebcb16e488 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2115,6 +2115,7 @@ static const struct engine_info {
>  	u32 mmio_base;
>  	unsigned irq_shift;
>  	int (*init)(struct intel_engine_cs *engine);
> +	int (*init_ringbuf)(struct intel_engine_cs *engine);

Blatant favouritisim!

The consensus (all of 2, perhas 3, people) name was to call them

	intel_legacy_submission.c
	intel_execlists_submission.c
	intel_guc_submission.c

from that we would then have

	int (*init_legacy_sumission)(struct intel_engine_cs *engine);
	int (*init_execlists_sumission)(struct intel_engine_cs *engine);

init_ringbuf() means to me to initialise the ringbuffer structure
associated both with execlists and legacy.

What I tried a couple of years ago was doing all the immediate setup
inside intel_ringbuffer.c and then dispatching to intel_execlists.c for
it to override as needed.

> @@ -2201,10 +2203,15 @@ int intel_logical_rings_init(struct drm_device *dev)
>  		if (!HAS_ENGINE(dev_priv, i))
>  			continue;
>  
> -		if (!intel_engines[i].init)
> +		if (i915.enable_execlists)
> +			init = intel_engines[i].init;
> +		else
> +			init = intel_engines[i].init_ringbuf;

I think I prefer your patch, but I'd like to some all the common setup in
one place.  intel_engine_cs.c ?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre