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

[Daniel Vetter]

On Tue, Jul 29, 2014 at 11:13:03AM -0700, Jesse Barnes wrote:
> 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>

There was a bit of conflict with one of Dave's fixes for the dp mst work,
but I think that was just partial of this patch here. But please
double-check.

Queued for -next, thanks for the patch.
-Daniel
> -- 
> Jesse Barnes, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at 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