Quoting Ville Syrjala (2018-06-11 21:02:55) > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > On i965/g4x IIR is edge triggered. So in order for IIR to notice that > there is still a pending interrupt we have to force and edge in ISR. > For the ISR/IIR pipe event bits we can do that by temporarily > clearing all the PIPESTAT enable bits when we ack the status bits. > This will force the ISR pipe event bit low, and it can then go back > high when we restore the PIPESTAT enable bits. > > This avoids the following race: > 1. stat = read(PIPESTAT) > 2. an enabled PIPESTAT status bit goes high > 3. write(PIPESTAT, enable|stat); > 4. write(IIR, PIPE_EVENT) > > The end result is IIR==0 and ISR!=0. This can lead to nasty > vblank wait/flip_done timeouts if another interrupt source > doesn't trick us into looking at the PIPESTAT status bits despite > the IIR PIPE_EVENT bit being low. > > Before i965 IIR was level triggered so this problem can't actually > happen there. And curiously VLV/CHV went back to the level triggered > scheme as well. But for simplicity we'll use the same i965/g4x > compatible code for all platforms. > > Cc: stable@xxxxxxxxxxxxxxx > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106033 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105225 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106030 > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_irq.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 2fd92a886789..364e1c85315e 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -1893,9 +1893,17 @@ static void i9xx_pipestat_irq_ack(struct drm_i915_private *dev_priv, > > /* > * Clear the PIPE*STAT regs before the IIR > + * > + * Toggle the enable bits to make sure we get an > + * edge in the ISR pipe event bit if we don't clear > + * all the enabled status bits. Otherwise the edge > + * triggered IIR on i965/g4x wouldn't notice that > + * an interrupt is still pending. > */ > - if (pipe_stats[pipe]) > - I915_WRITE(reg, enable_mask | pipe_stats[pipe]); > + if (pipe_stats[pipe]) { > + I915_WRITE(reg, pipe_stats[pipe]); Ack status, disable all. > + I915_WRITE(reg, enable_mask); Enable, an asserted bit should trigger a new edge. It certainly does as you explain, now I just hope the hw feels the same! Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris
