Re: [PATCH 14/19] drm/i915: Move context initialisation to first-use

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

On to, 2016-04-21 at 08:56 +0100, Chris Wilson wrote:
> On Thu, Apr 21, 2016 at 10:47:30AM +0300, Joonas Lahtinen wrote:
> > 
> > On to, 2016-04-21 at 08:08 +0100, Chris Wilson wrote:
> > > 
> > > On Thu, Apr 21, 2016 at 09:57:03AM +0300, Joonas Lahtinen wrote:
> > > > 
> > > > 
> > > > On ke, 2016-04-20 at 19:42 +0100, Chris Wilson wrote:
> > > > > 
> > > > > 
> > > > > +	if (!request->ctx->engine[engine->id].initialised) {
> > > > > +		ret = engine->init_context(request);
> > > > > +		if (ret) {
> > > > > +			intel_lr_context_unpin(request->ctx, engine);
> > > > I prefer the goto teardown path, it's easy to read and modify later on.
> > Meant something like this which is functionally the same but less
> > nesting;
> > 
> > 	if (request->ctx->engine[engine->id].initialised)
> > 		return 0;
> > 
> > 	ret = engine->init_context(request);
> > 	if (ret)
> > 		goto out_unpin;
> > 
> > 	request->ctx->engine[engine->id].initialised = true;
> > 
> > 	return 0;
> > 
> > out_unpin:
> > 	intel_lr_context_unpin(request->ctx, engine);
> > 
> > 	return ret;
> Yes, I am arguing that this leaves the next person thinking it will be
> safe to extend the unwind. And so by not conforming to the idiom, it
> should be more of a danger signal.
> 
> Compromise?

Fair enough.

> 
>         if (!request->ctx->engine[engine->id].initialised) {
>                 ret = engine->init_context(request);
>                 if (ret)
>                         goto err_unpin;
> 
>                 request->ctx->engine[engine->id].initialised = true;
>         }
> 
>         /* Note that after this point, we have committed to using
>          * this request as it is being used to both track the
>          * state of engine initialisation and liveness of the
>          * golden renderstate above. Think twice before you try
>          * to cancel/unwind this request now.
>          */
> 
>         return 0;
> 
> err_unpin:
>         intel_lr_context_unpin(request->ctx, engine);
>         return ret;
> }
> 
> 
-- 
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