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 <ch...@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
