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

[Chris Wilson]

Quoting Ville Syrjälä (2018-12-17 18:02:25)
> On Mon, Dec 17, 2018 at 03:20:55PM +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
> > v3: Mask off all HWSP writes on driver unload and engine cleanup.
> > v4: And what about the physical hwsp?
> > v5: No, engine->init_hw() is not called from driver_init_hw(), don't be
> > daft. Really scrub HWSTAM as early as we can in driver_init_mmio()
> > v6: Rename set_hwsp as it was setting the mask not the hwsp register.
> > 
> > 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>
> > Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c         |   9 --
> >  drivers/gpu/drm/i915/intel_engine_cs.c  |  23 +++++
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 107 +++++++++++++++---------
> >  3 files changed, 91 insertions(+), 48 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();
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index 6f165f9ad2bf..f1a2aeb34e5e 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -261,6 +261,23 @@ static void __sprint_engine_name(char *name, const struct engine_info *info)
> >                        info->instance) >= INTEL_ENGINE_CS_MAX_NAME);
> >  }
> >  
> > +static void set_hwsp_writemask(struct intel_engine_cs *engine, u32 mask)
> > +{
> > +     struct drm_i915_private *dev_priv = engine->i915;
> > +     i915_reg_t hwstam = RING_HWSTAM(engine->mmio_base);
> 
> g4x/ilk don't have HWSTAM for the bsd. A quick test on my ilk shows that
> the register at that offset stays at 0x0 even if written, so it might
> even be safe. But maybe we shouldn't poke at things that aren't there?

Swines. Did they forget where the copy function was? Or did they use the
one mask for both status pages? Probably just don't maintain a copy of
the iir in hwsp/bsd.

Oh well, another respin.
-Chris