[Intel-gfx] [PATCH v2] drm/i915: Apply missed interrupt after reset w/a to all ringbuffer gen

[Chris Wilson]

Quoting Ville Syrjälä (2018-12-13 11:59:28)
> On Thu, Dec 13, 2018 at 11:01:05AM +0000, Chris Wilson wrote:
> > Having completed a test run of gem_eio across all machines in CI we also
> > observe the phenomenon (of lost interrupts after resetting the GPU) on
> > gen3 machines as well as the previously sighted gen6/gen7. Let's apply
> > the same HWSTAM workaround that was effective for gen6+ for all, as
> > although we haven't seen the same failure on gen4/5 it seems prudent to
> > keep the code the same.
> > 
> > As a consequence we can remove the extra setting of HWSTAM and apply the
> > register from a single site.
> > 
> > v2: Delazy and move the HWSTAM into its own function
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=108735
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c         |  9 ------
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 41 ++++++++++++++++---------
> >  2 files changed, 27 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index e2dac9b5f4ce..0c7fc9890891 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -3586,9 +3586,6 @@ static void ironlake_irq_reset(struct drm_device *dev)
> >  {
> >       struct drm_i915_private *dev_priv = to_i915(dev);
> >  
> > -     if (IS_GEN(dev_priv, 5))
> > -             I915_WRITE(HWSTAM, 0xffffffff);
> > -
> >       GEN3_IRQ_RESET(DE);
> >       if (IS_GEN(dev_priv, 7))
> >               I915_WRITE(GEN7_ERR_INT, 0xffffffff);
> > @@ -4368,8 +4365,6 @@ static void i8xx_irq_reset(struct drm_device *dev)
> >  
> >       i9xx_pipestat_irq_reset(dev_priv);
> >  
> > -     I915_WRITE16(HWSTAM, 0xffff);
> > -
> >       GEN2_IRQ_RESET();
> >  }
> >  
> > @@ -4537,8 +4532,6 @@ static void i915_irq_reset(struct drm_device *dev)
> >  
> >       i9xx_pipestat_irq_reset(dev_priv);
> >  
> > -     I915_WRITE(HWSTAM, 0xffffffff);
> > -
> >       GEN3_IRQ_RESET();
> >  }
> >  
> > @@ -4648,8 +4641,6 @@ static void i965_irq_reset(struct drm_device *dev)
> >  
> >       i9xx_pipestat_irq_reset(dev_priv);
> >  
> > -     I915_WRITE(HWSTAM, 0xffffffff);
> > -
> >       GEN3_IRQ_RESET();
> >  }
> 
> So we're not worried about enabling interrupts and having
> something unmasked in HWSTAM by accident before we have the
> status page set up?

Sanitization of the HWSP setup would be off during early engine setup.
We do the irq install & reset during i915_load_modeset_init after we do
the status page setup. Unless I'm mistaken, moving the HWSTAM alongside
HWSP moves the sanitisation earlier.

The only question then is order inside the HWSP setup, and the
suggestion would be to make sure the HWSP is set before the HWSTAM to be
safe on the bits we control.
-Chris