Re: [PATCH 3/5] drm/i915: forcewake struct mutex locking fixes

[Chris Wilson <chris@chris-wilson.co.uk>]

On Wed, 20 Apr 2011 16:53:17 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Just to annoy you, this needs to be split up into the various categories
of fixes. Because...

>  static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode)
> @@ -3067,9 +3074,12 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>  	intel_disable_pll(dev_priv, pipe);
>  
>  	intel_crtc->active = false;
> +
> +	mutex_lock(&dev->struct_mutex);
>  	intel_update_fbc(dev);
>  	intel_update_watermarks(dev);
>  	intel_clear_scanline_wait(dev);
> +	mutex_unlock(&dev->struct_mutex);
>  }

This is overly correct. You can put a comment here to say that we will
never attempt to use FORCEWAKE here and that these registers are protected
by the mode_config lock. Except for intel_clear_scanline_wait, but that
itself is is longing to be killed now. If we haven't fixed the underlying
bug that we were working around by now, we have been too lax.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre