Re: [Intel-gfx] Find bugs in i915 driver
On Tue, Aug 21, 2012 at 09:20:37AM +0200, Daniel Vetter wrote: > On Mon, Aug 13, 2012 at 08:07:43AM +, 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 > > 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 > > 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 > > Acked-by: Paul Menzel > > 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: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Find bugs in i915 driver
On Mon, Aug 13, 2012 at 08:07:43AM +, 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
> 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
> 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
> Acked-by: Paul Menzel
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:paulepan...@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 + 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
> >
> > If Anhua is your first name your name is still switched here.
> >
> > Please do the following.
> >
> > git commit --amend --author="Anhua Xu "
> >
> > > Date: Tue, 31 Jul 2012 17:16:50 +0800
> > >
Re: [Intel-gfx] Find bugs in i915 driver
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
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
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
Acked-by: Paul Menzel
---
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:paulepan...@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 + 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
>
> If Anhua is your first name your name is still switched here.
>
> Please do the following.
>
> git commit --amend --author="Anhua Xu "
>
> > 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
> 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
>
&
Re: [Intel-gfx] Find bugs in i915 driver
On Mon, Aug 13, 2012 at 03:08:33AM +, 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 > 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 Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Find bugs in i915 driver
Am Montag, den 13.08.2012, 03:08 + 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
If Anhua is your first name your name is still switched here.
Please do the following.
git commit --amend --author="Anhua Xu "
> 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
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:
Does this actually fix a bug you are seeing or did you just spot this
reading the code?
> Signed-off-by: Anhua Xu
> ---
> 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
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
signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Find bugs in i915 driver
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
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
---
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.vet...@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 +, Xu, Anhua wrote:
> > > Thanks Chris. I add this in the the commit description. The updated patch
> > > is
> below:
> > >
> > > commit 71c3ff04834a01c81a5843996b87397273eb538d
> > > Author: Xu Anhua
> > > 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: dan...@ffwll.ch
> Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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 +, Xu, Anhua wrote: > > Thanks Chris. I add this in the the commit description. The updated patch > > is below: > > > > commit 71c3ff04834a01c81a5843996b87397273eb538d > > Author: Xu Anhua > > 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: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Find bugs in i915 driver
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 +, Xu, Anhua wrote: > > Thanks Chris. I add this in the the commit description. The updated patch > > is below: > > > > commit 71c3ff04834a01c81a5843996b87397273eb538d > > Author: Xu Anhua 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 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 signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Find bugs in i915 driver
On Tue, Jul 31, 2012 at 09:17:15AM +, Xu, Anhua wrote: > Thanks Chris. I add this in the the commit description. The updated patch is > below: > > commit 71c3ff04834a01c81a5843996b87397273eb538d > Author: Xu Anhua > 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 :( ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Find bugs in i915 driver
Thanks Chris. I add this in the the commit description. The updated patch is
below:
commit 71c3ff04834a01c81a5843996b87397273eb538d
Author: Xu Anhua
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:ch...@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 +, "Xu, Anhua"
> 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, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Find bugs in i915 driver
On Tue, 31 Jul 2012 02:29:01 +, "Xu, Anhua" 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, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
