Re: [PATCH 2/6] drm/i915: Treat kernel context as initialised

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

On ke, 2016-05-25 at 12:48 +0100, Chris Wilson wrote:
> The kernel context exists simply as a placeholder and should never be
> executed with a render context. It does not need the golden render
> state, as that will always be applied to a user context. By skipping the
> initialisation we can avoid issues in attempting to program the golden
> render context when trying to make the hardware idle.
> 
> Testcase: igt/drm_module_reload_basic #byt
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95634
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 45 +++++++++++++++++++++++----------
>  1 file changed, 32 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index a3b11aac23a4..3e3acd054f05 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -413,7 +413,8 @@ int i915_gem_context_init(struct drm_device *dev)
>  		return PTR_ERR(ctx);
>  	}
>  
> -	if (!i915.enable_execlists && ctx->engine[RCS].state) {
> +	if (!i915.enable_execlists) {
> +		struct intel_engine_cs *engine;
>  		int ret;
>  
>  		/* We may need to do things with the shrinker which
> @@ -423,13 +424,28 @@ int i915_gem_context_init(struct drm_device *dev)
>  		 * be available. To avoid this we always pin the default
>  		 * context.
>  		 */
> -		ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
> -					    get_context_alignment(dev_priv), 0);
> -		if (ret) {
> -			DRM_ERROR("Failed to pinned default global context (error %d)\n",
> -				  ret);
> -			i915_gem_context_unreference(ctx);
> -			return ret;
> +		for_each_engine(engine, dev_priv) {
> +			struct intel_context *ce = &ctx->engine[engine->id];
> +
> +			ret = 0;
> +			if (ce->state)

Ugh, rather have one level of indent and teardown separated, it'll be
easier to read and extend yadda yadda...

> +				ret = i915_gem_obj_ggtt_pin(ce->state,
> +							    get_context_alignment(dev_priv), 0);
> +			if (ret) {
> +				DRM_ERROR("Failed to pinned default global context (error %d)\n",

s/pinned/pin/

> +					  ret);
> +				i915_gem_context_unreference(ctx);
> +				return ret;
> +			}
> +
> +			/* The kernel context is only used as a placeholder
> +			 * for flushing the active context. It is never used
> +			 * for submitting rendering and as such never requires
> +			 * the golden render context, and so we can skip
> +			 * emitting it when we switch to the kernel context
> +			 * (during eviction).
> +			 */
> +			ce->initialised = true;
>  		}
>  	}
>  
> @@ -452,13 +468,16 @@ void i915_gem_context_lost(struct drm_i915_private *dev_priv)
>  			i915_gem_context_unpin(engine->last_context, engine);
>  			engine->last_context = NULL;
>  		}
> -
> -		/* Force the GPU state to be reinitialised on enabling */
> -		dev_priv->kernel_context->engine[engine->id].initialised =
> -			engine->init_context == NULL;
>  	}
>  
> -	/* Force the GPU state to be reinitialised on enabling */
> +	/* Force the GPU state to be restore on enabling */

s/restore/restored/

> +	for_each_engine(engine, dev_priv) {
> +		struct intel_context *ce =
> +			&dev_priv->kernel_context->engine[engine->id];
> +
> +		ce->initialised =
> +			!i915.enable_execlists || engine->init_context == NULL;
> +	}

<NL> here?

>  	dev_priv->kernel_context->remap_slice = ALL_L3_SLICES(dev_priv);
>  }
>  
-- 
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