[Intel-gfx] [PATCH 11/43] drm/i915/bdw: Render moot context reset and switch with Execlists

Daniel Vetter daniel at ffwll.ch
Mon Aug 11 16:30:15 CEST 2014


On Thu, Jul 24, 2014 at 05:04:19PM +0100, Thomas Daniel wrote:
> From: Oscar Mateo <oscar.mateo at intel.com>
> 
> These two functions make no sense in an Logical Ring Context & Execlists
> world.
> 
> v2: We got rid of lrc_enabled and centralized everything in the sanitized
> i915.enbale_execlists instead.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index fbe7278..288f5de 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -380,6 +380,9 @@ void i915_gem_context_reset(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int i;
>  
> +	if (i915.enable_execlists)
> +		return;

This will conflict badly with Alistair's patch at a functional level. I'm
pretty sure that we want _some_ form of reset for the context state, since
the hw didn't just magically load the previously running context. So NACK
on this hunk.

> +
>  	/* Prevent the hardware from restoring the last context (which hung) on
>  	 * the next switch */
>  	for (i = 0; i < I915_NUM_RINGS; i++) {
> @@ -514,6 +517,9 @@ int i915_gem_context_enable(struct drm_i915_private *dev_priv)
>  		ppgtt->enable(ppgtt);
>  	}
>  
> +	if (i915.enable_execlists)
> +		return 0;

Again this conflicts with Alistair's patch. Furthermore it looks redudant
since you no-op out i915_switch_context separately.

> +
>  	/* FIXME: We should make this work, even in reset */
>  	if (i915_reset_in_progress(&dev_priv->gpu_error))
>  		return 0;
> @@ -769,6 +775,9 @@ int i915_switch_context(struct intel_engine_cs *ring,
>  {
>  	struct drm_i915_private *dev_priv = ring->dev->dev_private;
>  
> +	if (i915.enable_execlists)
> +		return 0;

I've hoped we don't need this with the legacy ringbuffer cmdsubmission
fuly split out. If there are other paths (resume, gpu reset) where this
comes into play then I guess we need to look at where the best place is to
make this call. So until this comes with a bit a better justification I'll
punt on this for now.
-Daniel

> +
>  	WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex));
>  
>  	if (to->legacy_hw_ctx.rcs_state == NULL) { /* We have the fake context */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch



More information about the Intel-gfx mailing list