On Thu, 17 Jul 2014 17:43:46 -0300
Paulo Zanoni <przanoni@xxxxxxxxx> wrote:
> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> Since we started using intel_runtime_pm_disable_interrupts() at normal
> (non-runtime) suspend/resume, we had to remove a WARN from
> ironlake_disable_display_irq to avoid a case where we were doing the
> correct thing and the WARN was not really needed. The problem is that
> the WARN was useful in other cases, and its removal can hide some bugs
> that we would catch automatically.
>
> To be able to add back the WARN, we have to call intel_crtc_control()
> before interrupts are disabled, which is what this patch currently
> does.
>
> Also notice that Ville's patch from the Watermarks series "drm/i915:
> Leave interrupts enabled while disabling crtcs during suspend" also
> did a change that's equivalent to the one we're doing on this patch,
> with the exception that its original patch, when applied to the
> current tree, procduces a WARN.
>
> Related commits:
>
> commit daa390e5ee45cc051d6bf37b296901f2f92b002d
> Author: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> drm/i915: don't warn if IRQs are disabled when shutting down display IRQs
>
> commit e11aa362308f5de467ce355a2a2471321b15a35c
> Author: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> drm/i915: use runtime irq suspend/resume in freeze/thaw
>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_drv.c | 11 +++++------
> drivers/gpu/drm/i915/i915_irq.c | 2 +-
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> No need to revert anything :)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 3315358..63675bf 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -520,12 +520,6 @@ static int i915_drm_freeze(struct drm_device *dev)
> return error;
> }
>
> - flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> -
> - intel_runtime_pm_disable_interrupts(dev);
> -
> - intel_suspend_gt_powersave(dev);
> -
> /*
> * Disable CRTCs directly since we want to preserve sw state
> * for _thaw. Also, power gate the CRTC power wells.
> @@ -535,6 +529,11 @@ static int i915_drm_freeze(struct drm_device *dev)
> intel_crtc_control(crtc, false);
> drm_modeset_unlock_all(dev);
>
> + flush_delayed_work(&dev_priv->rps.delayed_resume_work);
> + intel_runtime_pm_disable_interrupts(dev);
> +
> + intel_suspend_gt_powersave(dev);
> +
> intel_modeset_suspend_hw(dev);
> }
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 20c777c..af231ba 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -151,7 +151,7 @@ ironlake_disable_display_irq(struct drm_i915_private *dev_priv, u32 mask)
> {
> assert_spin_locked(&dev_priv->irq_lock);
>
> - if (!intel_irqs_enabled(dev_priv))
> + if (WARN_ON(!intel_irqs_enabled(dev_priv)))
> return;
>
> if ((dev_priv->irq_mask & mask) != mask) {
So with this added, we get the WARN back but don't need/want to revert
any other patches?
Did you check boot, suspend/resume, and runtime PM for warnings with
this change applied? If so, looks fine:
Reviewed-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
