[Intel-gfx] [PATCH] drm/i915: freeze display before the interrupts and GT

[Jesse Barnes]

On Thu, 17 Jul 2014 17:43:46 -0300
Paulo Zanoni <przanoni at gmail.com> wrote:

> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> 
> 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 at virtuousgeek.org>
>     drm/i915: don't warn if IRQs are disabled when shutting down display IRQs
> 
> commit e11aa362308f5de467ce355a2a2471321b15a35c
> Author: Jesse Barnes <jbarnes at virtuousgeek.org>
>     drm/i915: use runtime irq suspend/resume in freeze/thaw
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
>  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 at virtuousgeek.org>
-- 
Jesse Barnes, Intel Open Source Technology Center