Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> 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!
Yeah it disables more than engines. Lets wait for irqsave flavour to land
and rethink. This patch can be ignored.
-Mika
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
