From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 3/5] drm/i915: forcewake struct mutex locking fixes Date: Thu, 21 Apr 2011 07:18:24 +0100 Message-ID: References: <1303343599-18509-1-git-send-email-ben@bwidawsk.net> <1303343599-18509-4-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id A10E29E7E1 for ; Wed, 20 Apr 2011 23:18:27 -0700 (PDT) In-Reply-To: <1303343599-18509-4-git-send-email-ben@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Ben Widawsky , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, 20 Apr 2011 16:53:17 -0700, Ben Widawsky wrote: > > Signed-off-by: Ben Widawsky 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