Re: [PATCH 1/2] drm/i915: Split intel_engine allocation and initialisation

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2017-01-23 at 09:57 +0000, Chris Wilson wrote:
> On Mon, Jan 23, 2017 at 11:41:12AM +0200, Joonas Lahtinen wrote:
> > 
> > On la, 2017-01-21 at 14:50 +0000, Chris Wilson wrote:
> > > 
> > > In order to reset the GPU early on in the module load sequence, we need
> > > to allocate the basic engine structs (to populate the mmio offsets etc).
> > > Currently, the engine initialisation allocates both the base struct and
> > > also allocate auxiliary objects, which depend upon state setup quite
> > > late in the load sequence. We split off the allocation callback for
> > > later and allow ourselves to allocate the engine structs themselves
> > > early.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > <SNIP>
> > 
> > > 
> > > +int intel_engines_init(struct drm_i915_private *dev_priv)
> > > +{
> > 
> > <SNIP>
> > 
> > > 
> > > +	for_each_engine(engine, dev_priv, id) {
> > > +		int (*init)(struct intel_engine_cs *engine) = NULL;
> > > +
> > > +		if (!err) {
> > > +			if (i915.enable_execlists)
> > > +				init = intel_engines[id].init_execlists;
> > > +			else
> > > +				init = intel_engines[id].init_legacy;
> > > +		}
> > > +
> > > +		if (!init || (err = init(engine))) {
> > > +			kfree(engine);
> > > +			dev_priv->engine[id] = NULL;
> > > +			continue;
> > > +		}
> > > +
> > > +		mask |= ENGINE_MASK(id);
> > > +	}
> > 
> > As discussed in IRC, this loop is broken after first erroring init.
> 
> As answered, it is not.
> 
> After an err is set, it and all subsequent engines are freed, and then
> all previously initialised engines are run through the cleanup.

You're correct, I managed to ignore the declaration time initialization
a half a dozen times when going through it.

You could check the "!init" as "err" to make my next review round
easier :P

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx