Re: [PATCH 01/17] drm/i915: Clear pipestat consistently

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@linux.intel.com (2017-06-22 12:55:39)
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > 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@linux.intel.com>
> > > > ---
> > > >  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx