[Intel-gfx] Find bugs in i915 driver
Xu, Anhua
anhua.xu at intel.com
Mon Aug 13 10:07:43 CEST 2012
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>
---
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: [Intel-gfx] 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
More information about the Intel-gfx
mailing list