[Intel-gfx] [PATCH v2] drm/i915/execlists: Refactor common engine setup

[Chris Wilson]

On Fri, Apr 29, 2016 at 10:25:41AM +0100, Tvrtko Ursulin wrote:
> On 29/04/16 10:15, Chris Wilson wrote:
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 2e0eaa9fa240..2c94072ab085 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -2016,14 +2016,17 @@ logical_ring_setup(struct drm_device *dev, enum intel_engine_id id)
> >         struct intel_engine_cs *engine = &dev_priv->engine[id];
> >         enum forcewake_domains fw_domains;
> >
> >-       engine->dev = dev;
> >-
> >         engine->id = id;
> >         engine->name = info->name;
> >         engine->exec_id = info->exec_id;
> >         engine->guc_id = info->guc_id;
> >         engine->mmio_base = info->mmio_base;
> >
> >+       /* disable interrupts to this engine before we install ourselves*/
> >+       I915_WRITE_IMR(engine, ~0);
> >+
> >+       engine->dev = dev;
> >+
> >         /* Intentionally left blank. */
> >         engine->buffer = NULL;
> >
> >Make sense?
> 
> Not the most elegant because all the hw access we have so far is in
> engine->init_hw. Why can't we just make intel_engine_initialized
> return false until the very last thing in engine constructors?

In my defence sanitizing the hw before we are ready is common practice
across the driver. The unfun part is that irq install is before gem_init
(because modeset init wants irq enabled for GMBUS/dp-aux). gem init
itself could be split up and moved around so that the setup and init_hw
phases are separate (which would be next on the init ordering hitlist I
hope).

I want engine->dev/engine->i915 set early so we can use it during setup
and init-hw and so that for_each_engine() works as expected in that
time.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre