[Intel-gfx] [PATCH] drm/i915: Consolidate forcewake resetting to a single function

[Chris Wilson]

On Thu, Mar 13, 2014 at 03:00:58PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> > @@ -957,13 +986,6 @@ static int gen6_do_reset(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	int	ret;
> > -	unsigned long irqflags;
> > -	u32 fw_engine = 0;
> > -
> > -	/* Hold uncore.lock across reset to prevent any register access
> > -	 * with forcewake not set correctly
> > -	 */
> > -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >  
> >  	/* Reset the chip */
> >  
> > @@ -976,29 +998,8 @@ static int gen6_do_reset(struct drm_device *dev)
> >  	/* Spin waiting for the device to ack the reset request */
> >  	ret = wait_for((__raw_i915_read32(dev_priv, GEN6_GDRST) & GEN6_GRDOM_FULL) == 0, 500);
> 
> We used to go through the reset with uncore lock held,
> which is not the case anymore. Will it create problems?

I don't think so. gen6_do_reset() is itself serialised and there should
be no other access to GDRST anyway, so from that perspective we won't
trigger hw errors. So the only thing that the uncore lock was protecting
in this function is the forcewake dance.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre