On Mon, Jun 11, 2018 at 09:53:52PM +0100, Chris Wilson wrote:
Quoting Ville Syrjala (2018-06-11 21:02:55)
From: Ville Syrjälä ville.syrjala@linux.intel.com
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:
- stat = read(PIPESTAT)
- an enabled PIPESTAT status bit goes high
- write(PIPESTAT, enable|stat);
- 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@vger.kernel.org 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@linux.intel.com
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@chris-wilson.co.uk
s/PIPESTATE/PIPESTAT/ (can't count how many times I've done that one wrong) and pushed to dinq. Thanks for the review.