[Intel-gfx] [PATCH 3/5] drm/i915: forcewake struct mutex locking fixes

[Ben Widawsky]

On Thu, Apr 21, 2011 at 07:18:24AM +0100, Chris Wilson wrote:
> On Wed, 20 Apr 2011 16:53:17 -0700, Ben Widawsky <ben at bwidawsk.net> wrote:
> > 
> > Signed-off-by: Ben Widawsky <ben at 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

I don't understand what you're asking for. I'm pretty convinced I need
the mutex protected intel_update_fbc, because the call trace could be:

intel_update_fbc()
intel_enable_fbc()
ironlake_enable_fbc()
sandybridge_blit_fbc_update()
gen6_gt_force_wake_get()


Could you elaborate?

Ben