[2/2] drm/i915: Use HOTPLUG_INT_STATUS_G4X on VLV/CHV
diff mbox

Message ID 1431366550-25659-2-git-send-email-ville.syrjala@linux.intel.com
State New
Headers show

Commit Message

Ville Syrjälä May 11, 2015, 5:49 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use HOTPLUG_INT_STATUS_G4X instead of HOTPLUG_INT_STATUS_I915 on VLV/CHV
so that we don't confuse the AUX status bits with SDVO status bits.

Avoid pointless log spam as below while handling AUX interrupts:
[drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
[drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
[drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
[drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
[drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
[drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x71450064

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jani Nikula May 12, 2015, 8:54 a.m. UTC | #1
On Mon, 11 May 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Use HOTPLUG_INT_STATUS_G4X instead of HOTPLUG_INT_STATUS_I915 on VLV/CHV
> so that we don't confuse the AUX status bits with SDVO status bits.
>
> Avoid pointless log spam as below while handling AUX interrupts:
> [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x71450064
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Shouldn't we also have bugs about missing hotplug on BYT...? Like,
there's a lot of talk about power management on

https://bugs.freedesktop.org/show_bug.cgi?id=89008

but could this make a difference?

BR,
Jani.


> ---
>  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 9c5371b..557af88 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -79,7 +79,7 @@ static const u32 hpd_status_g4x[HPD_NUM_PINS] = {
>  	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
>  };
>  
> -static const u32 hpd_status_i915[HPD_NUM_PINS] = { /* i915 and valleyview are the same */
> +static const u32 hpd_status_i915[HPD_NUM_PINS] = {
>  	[HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
>  	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915,
>  	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915,
> @@ -1743,7 +1743,7 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
>  		 */
>  		POSTING_READ(PORT_HOTPLUG_STAT);
>  
> -		if (IS_G4X(dev)) {
> +		if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
>  			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
>  
>  			intel_hpd_irq_handler(dev, hotplug_trigger, 0, hpd_status_g4x);
> -- 
> 2.0.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter May 12, 2015, 10:24 a.m. UTC | #2
On Tue, May 12, 2015 at 11:54:23AM +0300, Jani Nikula wrote:
> On Mon, 11 May 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Use HOTPLUG_INT_STATUS_G4X instead of HOTPLUG_INT_STATUS_I915 on VLV/CHV
> > so that we don't confuse the AUX status bits with SDVO status bits.
> >
> > Avoid pointless log spam as below while handling AUX interrupts:
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x71450064
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Both applied to dinq, thanks.
-Daniel

> Shouldn't we also have bugs about missing hotplug on BYT...? Like,
> there's a lot of talk about power management on
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=89008
> 
> but could this make a difference?
> 
> BR,
> Jani.
> 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 9c5371b..557af88 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -79,7 +79,7 @@ static const u32 hpd_status_g4x[HPD_NUM_PINS] = {
> >  	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
> >  };
> >  
> > -static const u32 hpd_status_i915[HPD_NUM_PINS] = { /* i915 and valleyview are the same */
> > +static const u32 hpd_status_i915[HPD_NUM_PINS] = {
> >  	[HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
> >  	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915,
> >  	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915,
> > @@ -1743,7 +1743,7 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
> >  		 */
> >  		POSTING_READ(PORT_HOTPLUG_STAT);
> >  
> > -		if (IS_G4X(dev)) {
> > +		if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> >  			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> >  
> >  			intel_hpd_irq_handler(dev, hotplug_trigger, 0, hpd_status_g4x);
> > -- 
> > 2.0.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä May 12, 2015, 10:27 a.m. UTC | #3
On Tue, May 12, 2015 at 11:54:23AM +0300, Jani Nikula wrote:
> On Mon, 11 May 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Use HOTPLUG_INT_STATUS_G4X instead of HOTPLUG_INT_STATUS_I915 on VLV/CHV
> > so that we don't confuse the AUX status bits with SDVO status bits.
> >
> > Avoid pointless log spam as below while handling AUX interrupts:
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x71450064
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> Shouldn't we also have bugs about missing hotplug on BYT...? Like,
> there's a lot of talk about power management on
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=89008
> 
> but could this make a difference?

Both HOTPLUG_INT_STATUS_G4X and HOTPLUG_INT_STATUS_I915 have the
required bits to have working hpd on VLV/CHV. Only the SDVO bits
differ, and we don't care about those. So the only issue is that
the i915 SDVO bits overlap with the AUX bits, so every time we
get an AUX interrupt we also think we get an SDVO hpd.

> 
> BR,
> Jani.
> 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index 9c5371b..557af88 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -79,7 +79,7 @@ static const u32 hpd_status_g4x[HPD_NUM_PINS] = {
> >  	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
> >  };
> >  
> > -static const u32 hpd_status_i915[HPD_NUM_PINS] = { /* i915 and valleyview are the same */
> > +static const u32 hpd_status_i915[HPD_NUM_PINS] = {
> >  	[HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
> >  	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915,
> >  	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915,
> > @@ -1743,7 +1743,7 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
> >  		 */
> >  		POSTING_READ(PORT_HOTPLUG_STAT);
> >  
> > -		if (IS_G4X(dev)) {
> > +		if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> >  			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> >  
> >  			intel_hpd_irq_handler(dev, hotplug_trigger, 0, hpd_status_g4x);
> > -- 
> > 2.0.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
Daniel Vetter May 12, 2015, 11:42 a.m. UTC | #4
On Tue, May 12, 2015 at 01:27:08PM +0300, Ville Syrjälä wrote:
> On Tue, May 12, 2015 at 11:54:23AM +0300, Jani Nikula wrote:
> > On Mon, 11 May 2015, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Use HOTPLUG_INT_STATUS_G4X instead of HOTPLUG_INT_STATUS_I915 on VLV/CHV
> > > so that we don't confuse the AUX status bits with SDVO status bits.
> > >
> > > Avoid pointless log spam as below while handling AUX interrupts:
> > > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > > [drm:intel_hpd_irq_handler] hotplug event received, stat 0x00000040, dig 0x00000000
> > > [drm:intel_dp_aux_ch] dp_aux_ch timeout status 0x71450064
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> > 
> > Shouldn't we also have bugs about missing hotplug on BYT...? Like,
> > there's a lot of talk about power management on
> > 
> > https://bugs.freedesktop.org/show_bug.cgi?id=89008
> > 
> > but could this make a difference?
> 
> Both HOTPLUG_INT_STATUS_G4X and HOTPLUG_INT_STATUS_I915 have the
> required bits to have working hpd on VLV/CHV. Only the SDVO bits
> differ, and we don't care about those. So the only issue is that
> the i915 SDVO bits overlap with the AUX bits, so every time we
> get an AUX interrupt we also think we get an SDVO hpd.

I've added this to the commit message as a note.
-Daniel

> 
> > 
> > BR,
> > Jani.
> > 
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > > index 9c5371b..557af88 100644
> > > --- a/drivers/gpu/drm/i915/i915_irq.c
> > > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > > @@ -79,7 +79,7 @@ static const u32 hpd_status_g4x[HPD_NUM_PINS] = {
> > >  	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
> > >  };
> > >  
> > > -static const u32 hpd_status_i915[HPD_NUM_PINS] = { /* i915 and valleyview are the same */
> > > +static const u32 hpd_status_i915[HPD_NUM_PINS] = {
> > >  	[HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
> > >  	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915,
> > >  	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915,
> > > @@ -1743,7 +1743,7 @@ static void i9xx_hpd_irq_handler(struct drm_device *dev)
> > >  		 */
> > >  		POSTING_READ(PORT_HOTPLUG_STAT);
> > >  
> > > -		if (IS_G4X(dev)) {
> > > +		if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> > >  			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
> > >  
> > >  			intel_hpd_irq_handler(dev, hotplug_trigger, 0, hpd_status_g4x);
> > > -- 
> > > 2.0.5
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Jani Nikula, Intel Open Source Technology Center
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shuang He May 15, 2015, 4:40 a.m. UTC | #5
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6382
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                 -1              314/314              313/314
IVB                                  338/338              338/338
BYT                                  286/286              286/286
BDW                                  320/320              320/320
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 SNB  igt@pm_rpm@dpms-mode-unset-non-lpsp      DMESG_WARN(13)PASS(1)      DMESG_WARN(1)
(dmesg patch applied)WARNING:at_drivers/gpu/drm/i915/intel_uncore.c:#assert_device_not_suspended[i915]()@WARNING:.* at .* assert_device_not_suspended+0x
Note: You need to pay more attention to line start with '*'

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9c5371b..557af88 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -79,7 +79,7 @@  static const u32 hpd_status_g4x[HPD_NUM_PINS] = {
 	[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
 };
 
-static const u32 hpd_status_i915[HPD_NUM_PINS] = { /* i915 and valleyview are the same */
+static const u32 hpd_status_i915[HPD_NUM_PINS] = {
 	[HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
 	[HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915,
 	[HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915,
@@ -1743,7 +1743,7 @@  static void i9xx_hpd_irq_handler(struct drm_device *dev)
 		 */
 		POSTING_READ(PORT_HOTPLUG_STAT);
 
-		if (IS_G4X(dev)) {
+		if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
 			u32 hotplug_trigger = hotplug_status & HOTPLUG_INT_STATUS_G4X;
 
 			intel_hpd_irq_handler(dev, hotplug_trigger, 0, hpd_status_g4x);