[Intel-gfx] [PATCH 2/3] drm/i915: reference counted forcewake

[Chris Wilson]

On Thu, 14 Apr 2011 11:13:46 -0700, Ben Widawsky <ben at bwidawsk.net> wrote:
> Provide a reference count to track the forcewake state of the GPU. The
> savings is up to 1 UC read if the unit is already awake, but the main
> goal is to give userspace some mechanism to wake the GPU.
> 
> v2:
> Added a spin_lock for synchronizing access to forcewake. The lock
> should not be heavily contested because any caller will either have
> struct_mutex or config.mutex, and those locks should be much more
> serializing than this. In terms of locking order:
> 
> 1.[config.mutex]
> 2.	[struct_mutex]
> 3.		forcewake_lock
> 
> By default i915_read[bwlq] functions now acquire forcewake_lock. Added a new
> _locked version of the read function for callers wishing to prevent the
> GT from possibly going to sleep in between reads. (We could also use
> these functions to try to get system state if the lock somehow becomes
> stuck).

This patch needs to be split again. I want the update to the existing
interface and users to be separate from the introduction of a new
interface. (This means that should we ever regret that interface we can
rip it out without undoing the fixes.)

Replacing the get()...long sequence of read/writes...put() did make me cry
a little, but it is indeed safer to move the check into the individual
reads (though there is nothing to prevent the rc6-window from closing
during the middle of that sequence... we need to check that is ok) and too
ugly to special case those rare times when we do modify 20 regs at once
(or maybe we have to?).

Again, let's change the macro without removing the existing get()...put()
users and then remove those as a patch+review on a per-case basis.

> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -2025,6 +2025,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	spin_lock_init(&dev_priv->irq_lock);
>  	spin_lock_init(&dev_priv->error_lock);
> +	if (IS_GEN6(dev))
> +		spin_lock_init(&dev_priv->forcewake_lock);

Just do it.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre