[Intel-gfx] [PATCH] drm/i915: Convert the forcewake worker into a timer func

[Chris Wilson]

On Mon, Mar 03, 2014 at 04:46:20PM +0200, Ville Syrjälä wrote:
> On Fri, Feb 28, 2014 at 06:44:03PM +0000, Chris Wilson wrote:
> > @@ -405,9 +403,8 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine)
> >  	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> >  	if (--dev_priv->uncore.forcewake_count == 0) {
> >  		dev_priv->uncore.forcewake_count++;
> > -		mod_delayed_work(dev_priv->wq,
> > -				 &dev_priv->uncore.force_wake_work,
> > -				 1);
> > +		mod_timer_pinned(&dev_priv->uncore.force_wake_timer,
> > +				 jiffies + 1);
> 
> This could expire more or less immediately, but it should be fine. We'd
> just end up doing two forcewake_get()s instead of one, which should
> still be better than >2 if the theory of the timer holds.

When does the timer actually fire? I am presuming that the callbacks are
run from a soft-interrupt at the next scheduling point. Using a pinned
timer here should mean that the earlier that it could fire is after this
sequence of register writes (or at least in the next mutex_lock() etc).
At any rate, we only care that we reduce the number of times we do the
forcewake dance.

> > @@ -794,7 +790,7 @@ void intel_uncore_fini(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > -	flush_delayed_work(&dev_priv->uncore.force_wake_work);
> > +	del_timer_sync(&dev_priv->uncore.force_wake_timer);
> 
> This could leave force wake enabled.

Bah humbug. Not our problem ;-)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre