On Mon, Nov 07, 2016 at 02:35:38PM +0200, Mika Kuoppala wrote:
> We do take execlist spinlock on thread context when
> we declare gpu to be wedged. Avoid the need to change
> the spinlock type just for the sake of wedging by
> removing the spinlock. Keep irqs disabled during reset
> phase and only enable on success path. Also add explicit
> disable to setting wedged so that we leave irqs off
> if we fail init.
Technically that spinlock already needs irqsafe.
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 20 ++------------------
> drivers/gpu/drm/i915/i915_drv.h | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/i915_gem.c | 6 ++++--
> 3 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 0213a30..2ed81f1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1725,22 +1725,6 @@ int i915_resume_switcheroo(struct drm_device *dev)
> return i915_drm_resume(dev);
> }
>
> -static void disable_engines_irq(struct drm_i915_private *dev_priv)
> -{
> - struct intel_engine_cs *engine;
> - enum intel_engine_id id;
> -
> - /* Ensure irq handler finishes, and not run again. */
> - disable_irq(dev_priv->drm.irq);
> - for_each_engine(engine, dev_priv, id)
> - tasklet_kill(&engine->irq_tasklet);
> -}
> -
> -static void enable_engines_irq(struct drm_i915_private *dev_priv)
> -{
> - enable_irq(dev_priv->drm.irq);
> -}
> -
> /**
> * i915_reset - reset chip after a hang
> * @dev: drm device to reset
> @@ -1775,9 +1759,8 @@ void i915_reset(struct drm_i915_private *dev_priv)
>
> pr_notice("drm/i915: Resetting chip after gpu hang\n");
>
> - disable_engines_irq(dev_priv);
> + i915_disable_engine_irqs(dev_priv);
> ret = intel_gpu_reset(dev_priv, ALL_ENGINES);
> - enable_engines_irq(dev_priv);
>
> if (ret) {
> if (ret != -ENODEV)
> @@ -1812,6 +1795,7 @@ void i915_reset(struct drm_i915_private *dev_priv)
>
> wakeup:
> wake_up_bit(&error->flags, I915_RESET_IN_PROGRESS);
> + i915_enable_engine_irqs(dev_priv);
To make this symmetric with set-wedged from init, it needs to be before
the wakeup:
Except that would be bad...
i915_disable_engine_irqs() doesn't just disable the GT interrupt, but
GMBUS, HPD, etc.
Since we already are planning to change this to use irqsafe spinlock
here, I think we just go forward with that plan. I thought there was
merit to having disable_engines_irq() for set-wedged, but it is
dangerously named!
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
