On Fri, Apr 22, 2011 at 09:20:17AM -0700, Ben Widawsky wrote: > On Thu, Apr 21, 2011 at 07:18:24AM +0100, Chris Wilson wrote: > > On Wed, 20 Apr 2011 16:53:17 -0700, Ben Widawsky <b...@bwidawsk.net> wrote: > > > > > > Signed-off-by: Ben Widawsky <b...@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
Crap. I wasn't paying attention. You're right, I did this to be symmetric with the other code. I can remove it if you prefer. _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
