[Intel-gfx] [PATCH 2/6] drm/i915: Treat kernel context as initialised

[Chris Wilson]

On Wed, May 25, 2016 at 05:16:56PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > [ text/plain ]
> > 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 at 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)
> > +				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",
> > +					  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;
> 
> My concern here is that the bdw had/has problem with entering and
> exiting from rc6 with a vanilla hw state. And if you have not
> submitted anything, you need a working context to sleep on.

This patch doesn't alter the fact that no context is set prior to rc6
being enabled. Nor have we been setting one for bdw for almost 2 years.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre