[Intel-gfx] [PATCH] drm/i915: vlv: fix IRQ masking when uninstalling interrupts

[Imre Deak]

On Thu, 2014-11-20 at 15:11 +0100, Daniel Vetter wrote:
> On Thu, Nov 20, 2014 at 04:05:55PM +0200, Imre Deak wrote:
> > irq_mask should include all IRQ bits that we want to mask, but atm we
> > set it incorrectly to the inverse of this. If the mask is used
> > subsequently to enable/disable some IRQ bits, we may unintentionally
> > unmask unrelated IRQs. I can't see any way that this can lead to a real
> > problem in the current -nightly code, since the first place the mask
> > will be used next (after a suspend/resume cycle) is in
> > valleyview_irq_postinstall(), but the mask is reset there to its proper
> > value.
> > 
> > This causes a problem in the upstream kernel though, where - due to another
> > issue - the mask is used in the above way to disable only the display IRQs.
> > This other issue is fixed by:
> > 
> > commit 950eabaf5a87257040e0c207be09487954113f54
> > Author: Imre Deak <imre.deak at intel.com>
> > Date:   Mon Sep 8 15:21:09 2014 +0300
> > 
> >     drm/i915: vlv: fix display IRQ enable/disable
> > 
> > Interestingly, even with the above two bugs, we shouldn't in theory have
> > any real problems (arguably a famous last sentence:). That's because
> > even if we unmask something unintentionally via the VLV_IMR/VLV_IER
> > register the master IRQ masking bit in VLV_MASTER_IER is still set and
> > should prevent all i915 interrupts. According to my testing on an ASUS
> > T100 with DSI output this isn't the case at least with the
> > MIPIA_INTERRUPT. Leaving this one unmasked in IMR/IER, while having
> > VLV_MASTER_IER set to 0 may lead to a lockup during system suspend as
> > shown in the bugzilla ticket below. This fix should get rid of the
> > problem reported there in upstream and older kernels.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85920
> > Cc: stable at vger.kernel.org (v3.15+)
> > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 8d169e1..4601f53 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3597,7 +3597,7 @@ static void vlv_display_irq_uninstall(struct drm_i915_private *dev_priv)
> >  
> >  	vlv_display_irq_reset(dev_priv);
> >  
> > -	dev_priv->irq_mask = 0;
> > +	dev_priv->irq_mask = ~0;
> 
> vlv seems to be the only platform which resets irq masks in the uninstall
> hook. That smells fishy - can't we just remove this completely?

Yes, we can look into removing it. But on older kernels the irq_mask was
used in the way I described in the commit log, so I'd suggest this fix
for -stable at least, together with commit 950eabaf5 which isn't
upstream yet.

> -Daniel