[Intel-gfx] [PATCH 4/9] drm/i915: Reduce duplicated forcewake logic

[Chris Wilson]

On Tue, Dec 16, 2014 at 05:20:55PM +0200, Mika Kuoppala wrote:
> +void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore)
>  {
> -	if (FORCEWAKE_RENDER & fw_engine) {
> -		WARN_ON(dev_priv->uncore.fw_rendercount == 0);
> -		if (--dev_priv->uncore.fw_rendercount == 0)
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv,
> -							FORCEWAKE_RENDER);
> -	}
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	unsigned long irqflags, fw = 0;
> +	struct intel_uncore_forcewake_domain *domain;
> +	int id, active_domains, retry_count = 100;
>  
> -	if (FORCEWAKE_MEDIA & fw_engine) {
> -		WARN_ON(dev_priv->uncore.fw_mediacount == 0);
> -		if (--dev_priv->uncore.fw_mediacount == 0)
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv,
> -							FORCEWAKE_MEDIA);
> -	}
> +	/* Hold uncore.lock across reset to prevent any register access
> +	 * with forcewake not set correctly. Wait until all pending
> +	 * timers are run before holding.
> +	 */
> +	while (1) {
> +		active_domains = 0;
>  
> -	if (FORCEWAKE_BLITTER & fw_engine) {
> -		WARN_ON(dev_priv->uncore.fw_blittercount == 0);
> -		if (--dev_priv->uncore.fw_blittercount == 0)
> -			dev_priv->uncore.funcs.force_wake_put(dev_priv,
> -							FORCEWAKE_BLITTER);
> -	}
> -}
> +		for_each_fw_domain(domain, dev_priv, id)
> +			del_timer_sync(&domain->timer);
>  
> -static void gen6_force_wake_timer(unsigned long arg)
> -{
> -	struct drm_i915_private *dev_priv = (void *)arg;
> -	unsigned long irqflags;
> +		spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
>  
> -	assert_device_not_suspended(dev_priv);
> +		for_each_fw_domain(domain, dev_priv, id) {
> +			if (timer_pending(&domain->timer))
> +				active_domains |= (1 << id);
> +		}
>  
> -	spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
> -	WARN_ON(!dev_priv->uncore.forcewake_count);
> +		if (active_domains == 0 || --retry_count == 0)
> +			break;
>  
> -	if (--dev_priv->uncore.forcewake_count == 0)
> -		dev_priv->uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL);
> -	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> -}
> +		spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
> +	}

One minor thing that occurs to me is that we should put a cond_resched()
here to encourage the timers to clear.

Also:

if (active_domains == 0)
	break;

if (--retry_count == 0) {
	DRM_ERROR("Timed out waiting for forcewake timers to finish\n");
	break;
}

would be prudent in case we hit the race condition and do go ahead in
the confused state.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre