On Mon, Aug 13, 2012 at 08:07:43AM +0000, Xu, Anhua wrote: > Hi, Paul > > Thanks for your advice. I update my patch. Please review, for your question, please see my reply below. > > From d11080eda81c0503b5035ea40667b06fe2ee0fb5 Mon Sep 17 00:00:00 2001 > From: Anhua Xu <anhua.xu at intel.com> > Date: Tue, 31 Jul 2012 17:16:50 +0800 > Subject: [PATCH v3] drm/i915: fix wrong order of parameters in port checking functions > > Wrong order of parameters passed-in when calling hdmi/adpa > /lvds_pipe_enabled(), 2nd and 3rd parameters are reversed. > This bug was indroduced by below commit: > > commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37 > Author: Keith Packard <keithp at keithp.com> > Date: Sat Aug 6 10:35:34 2011 -0700 > > drm/i915: Fix PCH port pipe select in CPT disable paths > > The reachable tag for this commit is v3.1-rc1-3-g1519b99 > > Signed-off-by: Anhua Xu <anhua.xu at intel.com> > Acked-by: Paul Menzel <paulepanter at users.sourceforge.net> I've just remembered that we have a bug report that bisects to the above commit: https://bugs.freedesktop.org/show_bug.cgi?id=44876 On a quick check, the symptoms seem to match your fix here ... -Daniel > --- > drivers/gpu/drm/i915/intel_display.c | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f615976..5fc8c8d 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1383,7 +1383,7 @@ static void assert_pch_hdmi_disabled(struct drm_i915_private *dev_priv, > enum pipe pipe, int reg) > { > u32 val = I915_READ(reg); > - WARN(hdmi_pipe_enabled(dev_priv, val, pipe), > + WARN(hdmi_pipe_enabled(dev_priv, pipe, val), > "PCH HDMI (0x%08x) enabled on transcoder %c, should be disabled\n", > reg, pipe_name(pipe)); > > @@ -1403,13 +1403,13 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, > > reg = PCH_ADPA; > val = I915_READ(reg); > - WARN(adpa_pipe_enabled(dev_priv, val, pipe), > + WARN(adpa_pipe_enabled(dev_priv, pipe, val), > "PCH VGA enabled on transcoder %c, should be disabled\n", > pipe_name(pipe)); > > reg = PCH_LVDS; > val = I915_READ(reg); > - WARN(lvds_pipe_enabled(dev_priv, val, pipe), > + WARN(lvds_pipe_enabled(dev_priv, pipe, val), > "PCH LVDS enabled on transcoder %c, should be disabled\n", > pipe_name(pipe)); > > @@ -1871,7 +1871,7 @@ static void disable_pch_hdmi(struct drm_i915_private *dev_priv, > enum pipe pipe, int reg) > { > u32 val = I915_READ(reg); > - if (hdmi_pipe_enabled(dev_priv, val, pipe)) { > + if (hdmi_pipe_enabled(dev_priv, pipe, val)) { > DRM_DEBUG_KMS("Disabling pch HDMI %x on pipe %d\n", > reg, pipe); > I915_WRITE(reg, val & ~PORT_ENABLE); > @@ -1893,12 +1893,12 @@ static void intel_disable_pch_ports(struct drm_i915_private *dev_priv, > > reg = PCH_ADPA; > val = I915_READ(reg); > - if (adpa_pipe_enabled(dev_priv, val, pipe)) > + if (adpa_pipe_enabled(dev_priv, pipe, val)) > I915_WRITE(reg, val & ~ADPA_DAC_ENABLE); > > reg = PCH_LVDS; > val = I915_READ(reg); > - if (lvds_pipe_enabled(dev_priv, val, pipe)) { > + if (lvds_pipe_enabled(dev_priv, pipe, val)) { > DRM_DEBUG_KMS("disable lvds on pipe %d val 0x%08x\n", pipe, val); > I915_WRITE(reg, val & ~LVDS_PORT_EN); > POSTING_READ(reg); > -- > 1.7.1 > > > > -----Original Message----- > > From: Paul Menzel [mailto:paulepanter at users.sourceforge.net] > > Sent: Monday, August 13, 2012 3:11 PM > > To: Xu, Anhua > > Cc: Daniel Vetter; Greg KH; intel-gfx at lists.freedesktop.org > > Subject: Re: Find bugs in i915 driver > > > > Am Montag, den 13.08.2012, 03:08 +0000 schrieb Xu, Anhua: > > > Sorry, Deniel/Greg, late response for your email because of a busy work last > > work. > > > I will response to you guys ASAP :), below is the updated patch: > > > > > > > > > From 33eb95a3a711b2354985361ff208ea150a5ba235 Mon Sep 17 00:00:00 > > 2001 > > > From: Xu Anhua <anhua.xu at intel.com> > > > > If Anhua is your first name your name is still switched here. > > > > Please do the following. > > > > git commit --amend --author="Anhua Xu <anhua.xu at intel.com>" > > > > > Date: Tue, 31 Jul 2012 17:16:50 +0800 > > > Subject: [PATCH] drm/i915: fix wrong order of parameters in port > > > checking functions > > > > > > Wrong order of parameters passed-in when calling hdmi/adpa > > > /lvds_pipe_enabled(), 2nd and 3rd parameters are reversed. > > > This bug was indroduced by commit > > > 1519b9956eb4b4180fa3f47c73341463cdcfaa37 > > > > Since it is hard to remember commit hashes, you should add the following > > summary > > > > commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37 > > Author: Keith Packard <keithp at keithp.com> > > Date: Sat Aug 6 10:35:34 2011 -0700 > > > > drm/i915: Fix PCH port pipe select in CPT disable paths > > > > or just the following. > > > > 1519b995 drm/i915: Fix PCH port pipe select in CPT disable paths > > > > > The reachable tag for this commit is v3.1-rc1-3-g1519b99 > > > > Then this should be sent to stable [1] too? > > > > Cc: <stable at vger.kernel.org> > > > > Does this actually fix a bug you are seeing or did you just spot this reading the > > code? > > No, I did not met a bug before fixing these issue. This bug was found when I referenced i915 driver code. > But I think these issue can cause potential bugs someday anyway. > > > > > > > Signed-off-by: Anhua Xu <anhua.xu at intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 12 ++++++------ > > > 1 files changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > > b/drivers/gpu/drm/i915/intel_display.c > > > index f615976..5fc8c8d 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -1383,7 +1383,7 @@ static void assert_pch_hdmi_disabled(struct > > drm_i915_private *dev_priv, > > > enum pipe pipe, int reg) > > > { > > > u32 val = I915_READ(reg); > > > - WARN(hdmi_pipe_enabled(dev_priv, val, pipe), > > > + WARN(hdmi_pipe_enabled(dev_priv, pipe, val), > > > "PCH HDMI (0x%08x) enabled on transcoder %c, should be > > disabled\n", > > > reg, pipe_name(pipe)); > > > > > > @@ -1403,13 +1403,13 @@ static void assert_pch_ports_disabled(struct > > > drm_i915_private *dev_priv, > > > > > > reg = PCH_ADPA; > > > val = I915_READ(reg); > > > - WARN(adpa_pipe_enabled(dev_priv, val, pipe), > > > + WARN(adpa_pipe_enabled(dev_priv, pipe, val), > > > "PCH VGA enabled on transcoder %c, should be disabled\n", > > > pipe_name(pipe)); > > > > > > reg = PCH_LVDS; > > > val = I915_READ(reg); > > > - WARN(lvds_pipe_enabled(dev_priv, val, pipe), > > > + WARN(lvds_pipe_enabled(dev_priv, pipe, val), > > > "PCH LVDS enabled on transcoder %c, should be disabled\n", > > > pipe_name(pipe)); > > > > > > @@ -1871,7 +1871,7 @@ static void disable_pch_hdmi(struct > > drm_i915_private *dev_priv, > > > enum pipe pipe, int reg) > > > { > > > u32 val = I915_READ(reg); > > > - if (hdmi_pipe_enabled(dev_priv, val, pipe)) { > > > + if (hdmi_pipe_enabled(dev_priv, pipe, val)) { > > > DRM_DEBUG_KMS("Disabling pch HDMI %x on pipe %d\n", > > > reg, pipe); > > > I915_WRITE(reg, val & ~PORT_ENABLE); @@ -1893,12 +1893,12 > > @@ static > > > void intel_disable_pch_ports(struct drm_i915_private *dev_priv, > > > > > > reg = PCH_ADPA; > > > val = I915_READ(reg); > > > - if (adpa_pipe_enabled(dev_priv, val, pipe)) > > > + if (adpa_pipe_enabled(dev_priv, pipe, val)) > > > I915_WRITE(reg, val & ~ADPA_DAC_ENABLE); > > > > > > reg = PCH_LVDS; > > > val = I915_READ(reg); > > > - if (lvds_pipe_enabled(dev_priv, val, pipe)) { > > > + if (lvds_pipe_enabled(dev_priv, pipe, val)) { > > > DRM_DEBUG_KMS("disable lvds on pipe %d val 0x%08x\n", pipe, > > val); > > > I915_WRITE(reg, val & ~LVDS_PORT_EN); > > > POSTING_READ(reg); > > > > With the changes addressed above, please add > > > > Acked-by: Paul Menzel <paulepanter at users.sourceforge.net> > > > > when sending [PATCH v3] as documented in [2]. > > > > > > Thanks, > > > > Paul > > > > > > [1] > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=Document > > ation/stable_kernel_rules.txt;h=b0714d8f678ac51d0c280a4f5f2980196052421 > > f;hb=HEAD > > [2] > > http://wireless.kernel.org/en/developers/Documentation/git-guide#Annotatin > > g_new_revision -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
