[Intel-gfx] [PATCH 3/5] drm/i915: forcewake struct mutex locking fixes
Ben Widawsky
ben at bwidawsk.net
Fri Apr 22 18:20:17 CEST 2011
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
More information about the Intel-gfx
mailing list