Re: [PATCH 07/12] drm/i915: Write mmio workarounds after gpu reset

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Oct 07, 2015 at 04:52:54PM +0300, Ville Syrjälä wrote:
> On Wed, Oct 07, 2015 at 09:43:07AM +0100, Chris Wilson wrote:
> > On Tue, Oct 06, 2015 at 05:26:48PM +0300, Mika Kuoppala wrote:
> > > @@ -6973,8 +6989,12 @@ void intel_init_clock_gating(struct drm_device *dev)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > >  
> > > +	intel_wa_init(&dev_priv->mmio_workarounds);
> > > +
> > >  	if (dev_priv->display.init_clock_gating)
> > >  		dev_priv->display.init_clock_gating(dev);
> > > +
> > > +	intel_wa_write_mmio(dev_priv, &dev_priv->mmio_workarounds);
> > >  }
> > 
> > Given that we have this function which is in charge of setting the w/a
> > regs and is supposed to be called when initialising the hw after
> > load/reset/resume, why?
> 
> I think ideally we should move all the GT related w/as, or at least the
> ones clobbered by a GPU reset into ring init, or somewhere close by.
> The display stuff (and UCGCTL at least) could stay where they are.
> Although I suspect we should move the init_clock_gating earlier in the
> init/resume sequence anyway. I think now we might be a bit late in doing
> it.

Yes, this is how it's supposed to work. Any w/a which gets clobbered by GT
reset and which is still in the init_clock_gating code is just plain a bug
in our driver. We have them aplenty unfortunately :(

Maybe we should add a big kerneldoc to intel_init_clock_gating explaining
that.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx