From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 1/3] drm/i915: fix up the IBX transcoder B check Date: Wed, 12 Sep 2012 08:20:12 -0700 Message-ID: <20120912082012.785ee304@jbarnes-desktop> References: <1347307111-6887-1-git-send-email-daniel.vetter@ffwll.ch> <1347307111-6887-2-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy11-pub.bluehost.com (oproxy11-pub.bluehost.com [173.254.64.10]) by gabe.freedesktop.org (Postfix) with SMTP id 012949E762 for ; Wed, 12 Sep 2012 08:20:00 -0700 (PDT) In-Reply-To: <1347307111-6887-2-git-send-email-daniel.vetter@ffwll.ch> 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: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Mon, 10 Sep 2012 21:58:29 +0200 Daniel Vetter wrote: > This has been added in > > commit de9a35abb3b343a25065449234e47a76c4f3454a > Author: Daniel Vetter > Date: Tue Jun 5 11:03:40 2012 +0200 > > drm/i915: assert that the IBX port transcoder select w/a is implemented > > Unfortunately I've failed to notice that these checks are not just > called for the port that is about to be disabled, but for all (which > makes sense for an assert ...), and the WARN missfired when disabling > another pipe than the one with the dp port. > > Hence also check whether the port is actually disabled. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=54688 > Signed-Off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/intel_display.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f26fb3f..b8e5a51 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1376,7 +1376,8 @@ static void assert_pch_dp_disabled(struct drm_i915_private *dev_priv, > "PCH DP (0x%08x) enabled on transcoder %c, should be disabled\n", > reg, pipe_name(pipe)); > > - WARN(HAS_PCH_IBX(dev_priv->dev) && (val & SDVO_PIPE_B_SELECT), > + WARN(HAS_PCH_IBX(dev_priv->dev) && (val & DP_PORT_EN) == 0 > + && (val & DP_PIPEB_SELECT), > "IBX PCH dp port still using transcoder B\n"); > } > > @@ -1388,7 +1389,8 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv, > "PCH HDMI (0x%08x) enabled on transcoder %c, should be disabled\n", > reg, pipe_name(pipe)); > > - WARN(HAS_PCH_IBX(dev_priv->dev) && (val & SDVO_PIPE_B_SELECT), > + WARN(HAS_PCH_IBX(dev_priv->dev) && (val & PORT_ENABLE) == 0 > + && (val & SDVO_PIPE_B_SELECT), > "IBX PCH hdmi port still using transcoder B\n"); > } > Won't these warn if the port is disabled rather than enabled? Shouldn't we be checking for (val & PORT_ENABLE) != 0 *and* pipe B is selected? -- Jesse Barnes, Intel Open Source Technology Center