On Fri, Jun 30, 2017 at 12:41:53PM +0100, Chris Wilson wrote: > Quoting Ville Syrjälä (2017-06-30 12:34:15) > > On Thu, Jun 22, 2017 at 01:39:47PM +0100, Chris Wilson wrote: > > > Quoting ville.syrjala@xxxxxxxxxxxxxxx (2017-06-22 12:55:39) > > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > > > We have a lot of different ways of clearing the PIPESTAT registers. > > > > Let's unify it all into one function. There's no magic in PIPESTAT > > > > that would require any of the double clearing and whatnot that > > > > some of the code tries to do. All we can really do is clear the status > > > > bits and disable the enable bits. There is no way to mask anything > > > > so as soon as another event happens the status bit will become set > > > > again, and trying to clear them twice or something can't protect > > > > against that. > > > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > --- > > > > drivers/gpu/drm/i915/i915_irq.c | 67 ++++++++++++++++++----------------------- > > > > 1 file changed, 30 insertions(+), 37 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > > > index b1c7d1a04612..6daaf47482d4 100644 > > > > --- a/drivers/gpu/drm/i915/i915_irq.c > > > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > > > @@ -1732,6 +1732,19 @@ static bool intel_pipe_handle_vblank(struct drm_i915_private *dev_priv, > > > > return ret; > > > > } > > > > > > > > +static void i9xx_pipestat_irq_reset(struct drm_i915_private *dev_priv) > > > > +{ > > > > + enum pipe pipe; > > > > + > > > > + for_each_pipe(dev_priv, pipe) { > > > > + I915_WRITE(PIPESTAT(pipe), > > > > + PIPESTAT_INT_STATUS_MASK | > > > > + PIPE_FIFO_UNDERRUN_STATUS); > > > > > > Hmm, is this change for i915/i965 significant? Maybe explain it away in > > > the changelog? > > > > Sorry missed your question. Which change are we concerned about here > > specifically? > > We didn't set PIPESTAT_INT_STATUS_MASK | PIPE_FIFO_UNDERRUN_STATUS > previously afaics. Ah. Those are the sticky status bits, so the earlier I915_WRITE(PIPESTAT, I915_READ(PIPESTAT)) did the same thing effectively. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx