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

Daniel Vetter
2014-08-11T14:30:03Z

On Thu, Jul 24, 2014 at 05:04:19PM +0100, Thomas Daniel wrote:
> From: Oscar Mateo <oscar.mateo@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@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@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