Find bugs in i915 driver

Find bugs in i915 driver

[Xu, Anhua]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

Hi, I found some bugs in i915 driver when reviewing intel_display.c The parameter passed-in for function hdmi/adpa/lvds_pipe_enabled() are not coherent with functions' definition. 
This is the patch. 

Thanks
Eric


commit 9128c5c29649a1eea4e7db9282d0aa57e4885ba8
Author: Xu Anhua <anhua.xu@intel.com>
Date:   Mon Jul 30 20:59:48 2012 +0800

    i915: fix wrong parameter sequence when calling
    	  hdmi/adpa/lvds_pipe_enabled()
    
    When calling hdmi/adpa/lvds_pipe_enabled(),make the
    parameters passed-in coherent with function definitions

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);

[-- Attachment #2: intel-display-wrong-para-sequence.patch --]
[-- Type: application/octet-stream, Size: 2370 bytes --]

commit 9128c5c29649a1eea4e7db9282d0aa57e4885ba8
Author: Xu Anhua <anhua.xu@intel.com>
Date:   Mon Jul 30 20:59:48 2012 +0800

    i915: fix wrong parameter sequence when calling
    	  hdmi/adpa/lvds_pipe_enabled()
    
    When calling hdmi/adpa/lvds_pipe_enabled(),make the
    parameters passed-in coherent with function definitions

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);

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Find bugs in i915 driver

[Chris Wilson]

On Tue, 31 Jul 2012 02:29:01 +0000, "Xu, Anhua" <anhua.xu@intel.com> wrote:
> Hi, I found some bugs in i915 driver when reviewing intel_display.c The parameter passed-in for function hdmi/adpa/lvds_pipe_enabled() are not coherent with functions' definition. 
> This is the patch. 

Patch looks good, you might want to mention that the regression was
introduced with commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: Find bugs in i915 driver

[Xu, Anhua]

Thanks Chris. I add this in the the commit description. The updated patch is below: 

commit 71c3ff04834a01c81a5843996b87397273eb538d
Author: Xu Anhua <anhua.xu@intel.com>
Date:   Tue Jul 31 17:16:50 2012 +0800

    i915: make the parameters passed-in coherent with functions'
    	definition when calling hdmi/adpa/lvds_pipe_enabled()
    
    This bug is indroduced by commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37

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);

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Tuesday, July 31, 2012 3:58 PM
> To: Xu, Anhua; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] Find bugs in i915 driver
> 
> On Tue, 31 Jul 2012 02:29:01 +0000, "Xu, Anhua" <anhua.xu@intel.com>
> wrote:
> > Hi, I found some bugs in i915 driver when reviewing intel_display.c The
> parameter passed-in for function hdmi/adpa/lvds_pipe_enabled() are not
> coherent with functions' definition.
> > This is the patch.
> 
> Patch looks good, you might want to mention that the regression was
> introduced with commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre

Re: Find bugs in i915 driver

[Greg KH]

On Tue, Jul 31, 2012 at 09:17:15AM +0000, Xu, Anhua wrote:
> Thanks Chris. I add this in the the commit description. The updated patch is below: 
> 
> commit 71c3ff04834a01c81a5843996b87397273eb538d
> Author: Xu Anhua <anhua.xu@intel.com>
> Date:   Tue Jul 31 17:16:50 2012 +0800
> 
>     i915: make the parameters passed-in coherent with functions'
>     	definition when calling hdmi/adpa/lvds_pipe_enabled()
>     
>     This bug is indroduced by commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c

No signed-off-by?  No tag for inclusion in the stable tree?

This patch isn't going very far :(

Re: Find bugs in i915 driver

[Paul Menzel]

[-- Attachment #1.1: Type: text/plain, Size: 2533 bytes --]

Dear Anhua,


five additional comments to Greg’s ones.


Am Dienstag, den 31.07.2012, 07:23 -0700 schrieb Greg KH:
> On Tue, Jul 31, 2012 at 09:17:15AM +0000, Xu, Anhua wrote:
> > Thanks Chris. I add this in the the commit description. The updated patch is below: 
> > 
> > commit 71c3ff04834a01c81a5843996b87397273eb538d
> > Author: Xu Anhua <anhua.xu@intel.com>

1. Looking at your address in the From field of this message there is
"Xu, Anhua", so I am not sure which is your first name.

If it is Anhua, please do the following.

    git config --global author.name "Anhua Xu"

> > Date:   Tue Jul 31 17:16:50 2012 +0800
> > 
> >     i915: make the parameters passed-in coherent with functions'
> >     	definition when calling hdmi/adpa/lvds_pipe_enabled()
> >     
> >     This bug is indroduced by commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37

1. There is a typo: in*t*roduced. The spell checker of your email
program should mention that. Or you hook up Git to check that [1][2][3].

> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> 
> No signed-off-by?  No tag for inclusion in the stable tree?
> 
> This patch isn't going very far :(

2. Please do not just mention the hash of the commit but also the
summary. Only a few people have memorized all hashes. ;-)

        commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37
        Author: Keith Packard <keithp@keithp.com>
        Date:   Sat Aug 6 10:35:34 2011 -0700
        
            drm/i915: Fix PCH port pipe select in CPT disable paths

You can use `git commit --amend --author="Anhua Xu" to edit the last
commit.

3. To include the patch you seem to copy the output of `git log` or `git
show`. The recommended way is to use `git format-patch -s -1`. This way
the commit message is indented correctly.

4. Your first message of this thread included a HTML part. Luckily your
reply did not which suggests that your mail program supports writing
just plain text messages. In the future it would be great if you could
write just plain text messages which is the recommended way for mailing
lists [4].


Thanks,

Paul


[1] http://blog.mpdaugherty.com/2010/04/06/how-to-include-git-hooks-in-a-repository-and-still-personalize-your-machine/
[2] http://stackoverflow.com/questions/1691060/vim-set-spell-in-file-git-commit-editmsg
[3] http://petereisentraut.blogspot.de/2011/01/git-commit-mode.html
[4] http://en.opensuse.org/openSUSE:Mailing_list_netiquette

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Find bugs in i915 driver

[Daniel Vetter]

On Tue, Jul 31, 2012 at 07:23:18AM -0700, Greg KH wrote:
> On Tue, Jul 31, 2012 at 09:17:15AM +0000, Xu, Anhua wrote:
> > Thanks Chris. I add this in the the commit description. The updated patch is below: 
> > 
> > commit 71c3ff04834a01c81a5843996b87397273eb538d
> > Author: Xu Anhua <anhua.xu@intel.com>
> > Date:   Tue Jul 31 17:16:50 2012 +0800
> > 
> >     i915: make the parameters passed-in coherent with functions'
> >     	definition when calling hdmi/adpa/lvds_pipe_enabled()
> >     
> >     This bug is indroduced by commit 1519b9956eb4b4180fa3f47c73341463cdcfaa37
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> 
> No signed-off-by?  No tag for inclusion in the stable tree?
> 
> This patch isn't going very far :(

Xu Anhua, can you please update your patch with signed-off-by and cc:
stable? Also, your commit headline is a bit long, it should fit on one
line of at most 70 chars (or thereabouts).
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: Find bugs in i915 driver

[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@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
The reachable tag for this commit is v3.1-rc1-3-g1519b99

Signed-off-by: Anhua Xu <anhua.xu@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);
-- 
1.7.1



> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Friday, August 10, 2012 7:41 PM
> To: Greg KH
> Cc: Xu, Anhua; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] Find bugs in i915 driver
> 
> On Tue, Jul 31, 2012 at 07:23:18AM -0700, Greg KH wrote:
> > On Tue, Jul 31, 2012 at 09:17:15AM +0000, Xu, Anhua wrote:
> > > Thanks Chris. I add this in the the commit description. The updated patch is
> below:
> > >
> > > commit 71c3ff04834a01c81a5843996b87397273eb538d
> > > Author: Xu Anhua <anhua.xu@intel.com>
> > > Date:   Tue Jul 31 17:16:50 2012 +0800
> > >
> > >     i915: make the parameters passed-in coherent with functions'
> > >     	definition when calling hdmi/adpa/lvds_pipe_enabled()
> > >
> > >     This bug is indroduced by commit
> 1519b9956eb4b4180fa3f47c73341463cdcfaa37
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> >
> > No signed-off-by?  No tag for inclusion in the stable tree?
> >
> > This patch isn't going very far :(
> 
> Xu Anhua, can you please update your patch with signed-off-by and cc:
> stable? Also, your commit headline is a bit long, it should fit on one
> line of at most 70 chars (or thereabouts).
> -Daniel
> --
> Daniel Vetter
> Mail: daniel@ffwll.ch
> Mobile: +41 (0)79 365 57 48

Re: Find bugs in i915 driver

[Paul Menzel]

[-- Attachment #1.1: Type: text/plain, Size: 4307 bytes --]

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@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@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@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@vger.kernel.org>

Does this actually fix a bug you are seeing or did you just spot this
reading the code?

> Signed-off-by: Anhua Xu <anhua.xu@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@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=Documentation/stable_kernel_rules.txt;h=b0714d8f678ac51d0c280a4f5f2980196052421f;hb=HEAD
[2] http://wireless.kernel.org/en/developers/Documentation/git-guide#Annotating_new_revision

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: Find bugs in i915 driver

[Xu, Anhua]

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@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@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@intel.com>
Acked-by: Paul Menzel <paulepanter@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@users.sourceforge.net]
> Sent: Monday, August 13, 2012 3:11 PM
> To: Xu, Anhua
> Cc: Daniel Vetter; Greg KH; intel-gfx@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@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@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@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@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@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@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

Re: Find bugs in i915 driver

[Daniel Vetter]

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@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@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@intel.com>
> Acked-by: Paul Menzel <paulepanter@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@users.sourceforge.net]
> > Sent: Monday, August 13, 2012 3:11 PM
> > To: Xu, Anhua
> > Cc: Daniel Vetter; Greg KH; intel-gfx@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@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@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@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@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@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@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@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: Find bugs in i915 driver

[Daniel Vetter]

On Tue, Aug 21, 2012 at 09:20:37AM +0200, Daniel Vetter wrote:
> 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@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@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@intel.com>
> > Acked-by: Paul Menzel <paulepanter@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

And this does indeeed fix the bug! I've moved the patch to -fixes, with cc
stable added.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: Find bugs in i915 driver

[Daniel Vetter]

On Mon, Aug 13, 2012 at 03:08:33AM +0000, Xu, Anhua wrote:
> 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@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
> The reachable tag for this commit is v3.1-rc1-3-g1519b99
> 
> Signed-off-by: Anhua Xu <anhua.xu@intel.com>
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48