diff mbox

drm/i915/bxt: Fix irq_port for eDP

Message ID 1441011932-5112-1-git-send-email-sonika.jindal@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sonika.jindal@intel.com Aug. 31, 2015, 9:05 a.m. UTC
From: Durgadoss R <durgadoss.r@intel.com>

Currently, HDMI hotplug with eDP as local panel is failing
because the HDMI hpd is detected as a long hpd for eDP; and is
thus rightfully ignored. But, it should really be handled as
an interrupt on port B for HDMI (due to BXT A1 platform having
HPD pins A and B swapped). This patch sets the irq_port[PORT_A]
to NULL in case eDP is on port A so that irq handler does not
treat it as a 'dig_port' interrupt.

v2 (Sonika): Moving the setting of irq_port for BXT WA outside so that this
can be set for both hdmi or dp ports. For HDMI this is required
because we get interrupts for portB on the hpd line of portA for
BXT A0/A1.
This issue occurred because hpd on edp was not disabled
which was done as part of "drm/i915: Dont enable hpd for eDP" from
the series:
http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.html

This patch can be squashed to :
commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
Author: Sonika Jindal <sonika.jindal@intel.com>
Date:   Mon Aug 10 10:35:36 2015 +0530

    drm/i915/bxt: WA for swapped HPD pins in A stepping

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |   21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Daniel Vetter Sept. 2, 2015, 11:46 a.m. UTC | #1
On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> From: Durgadoss R <durgadoss.r@intel.com>
> 
> Currently, HDMI hotplug with eDP as local panel is failing
> because the HDMI hpd is detected as a long hpd for eDP; and is
> thus rightfully ignored. But, it should really be handled as
> an interrupt on port B for HDMI (due to BXT A1 platform having
> HPD pins A and B swapped). This patch sets the irq_port[PORT_A]
> to NULL in case eDP is on port A so that irq handler does not
> treat it as a 'dig_port' interrupt.
> 
> v2 (Sonika): Moving the setting of irq_port for BXT WA outside so that this
> can be set for both hdmi or dp ports. For HDMI this is required
> because we get interrupts for portB on the hpd line of portA for
> BXT A0/A1.
> This issue occurred because hpd on edp was not disabled
> which was done as part of "drm/i915: Dont enable hpd for eDP" from
> the series:
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.html
> 
> This patch can be squashed to :
> commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> Author: Sonika Jindal <sonika.jindal@intel.com>
> Date:   Mon Aug 10 10:35:36 2015 +0530
> 
>     drm/i915/bxt: WA for swapped HPD pins in A stepping
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |   21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 56d778f..bba0cb6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  			goto err;
>  
>  		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> -		/*
> -		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> -		 * interrupts to check the external panel connection.
> -		 */
> -		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> -					 && port == PORT_B)
> -			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> -		else
> -			dev_priv->hotplug.irq_port[port] = intel_dig_port;
> +		dev_priv->hotplug.irq_port[port] = intel_dig_port;
>  	}
>  
>  	/* In theory we don't need the encoder->type check, but leave it just in
> @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  		if (!intel_ddi_init_hdmi_connector(intel_dig_port))
>  			goto err;
>  	}
> +	/*
> +	 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> +	 * interrupts to check the external panel connection.
> +	 */
> +	if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> +		if (port == PORT_B) {
> +			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> +			intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> +		} else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> +			dev_priv->hotplug.irq_port[port] = NULL;
> +	}
>  
>  	return;
>  
> -- 
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
sonika.jindal@intel.com Sept. 2, 2015, 11:48 a.m. UTC | #2
:( This had a hole..
Please drop this patch..

I am going to send another patch tested with hdmi optimization series for bxt.

Regards,
Sonika

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, September 2, 2015 5:17 PM
To: Jindal, Sonika
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP

On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> From: Durgadoss R <durgadoss.r@intel.com>
> 
> Currently, HDMI hotplug with eDP as local panel is failing because the 
> HDMI hpd is detected as a long hpd for eDP; and is thus rightfully 
> ignored. But, it should really be handled as an interrupt on port B 
> for HDMI (due to BXT A1 platform having HPD pins A and B swapped). 
> This patch sets the irq_port[PORT_A] to NULL in case eDP is on port A 
> so that irq handler does not treat it as a 'dig_port' interrupt.
> 
> v2 (Sonika): Moving the setting of irq_port for BXT WA outside so that 
> this can be set for both hdmi or dp ports. For HDMI this is required 
> because we get interrupts for portB on the hpd line of portA for BXT 
> A0/A1.
> This issue occurred because hpd on edp was not disabled which was done 
> as part of "drm/i915: Dont enable hpd for eDP" from the series:
> http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.htm
> l
> 
> This patch can be squashed to :
> commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> Author: Sonika Jindal <sonika.jindal@intel.com>
> Date:   Mon Aug 10 10:35:36 2015 +0530
> 
>     drm/i915/bxt: WA for swapped HPD pins in A stepping
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_ddi.c |   21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 56d778f..bba0cb6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  			goto err;
>  
>  		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> -		/*
> -		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> -		 * interrupts to check the external panel connection.
> -		 */
> -		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> -					 && port == PORT_B)
> -			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> -		else
> -			dev_priv->hotplug.irq_port[port] = intel_dig_port;
> +		dev_priv->hotplug.irq_port[port] = intel_dig_port;
>  	}
>  
>  	/* In theory we don't need the encoder->type check, but leave it 
> just in @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>  		if (!intel_ddi_init_hdmi_connector(intel_dig_port))
>  			goto err;
>  	}
> +	/*
> +	 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> +	 * interrupts to check the external panel connection.
> +	 */
> +	if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> +		if (port == PORT_B) {
> +			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> +			intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> +		} else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> +			dev_priv->hotplug.irq_port[port] = NULL;
> +	}
>  
>  	return;
>  
> --
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Daniel Vetter Sept. 2, 2015, 11:51 a.m. UTC | #3
On Wed, Sep 02, 2015 at 11:48:36AM +0000, Jindal, Sonika wrote:
> :( This had a hole..
> Please drop this patch..

What kind of hole? It sounds like we need this to avoid blowing up when we
handle the edp hpd interrupt everywhere? Please explain so I can
understand what I've missed here ...

Thanks, Daniel

> 
> I am going to send another patch tested with hdmi optimization series for bxt.
> 
> Regards,
> Sonika
> 
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Wednesday, September 2, 2015 5:17 PM
> To: Jindal, Sonika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP
> 
> On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> > From: Durgadoss R <durgadoss.r@intel.com>
> > 
> > Currently, HDMI hotplug with eDP as local panel is failing because the 
> > HDMI hpd is detected as a long hpd for eDP; and is thus rightfully 
> > ignored. But, it should really be handled as an interrupt on port B 
> > for HDMI (due to BXT A1 platform having HPD pins A and B swapped). 
> > This patch sets the irq_port[PORT_A] to NULL in case eDP is on port A 
> > so that irq handler does not treat it as a 'dig_port' interrupt.
> > 
> > v2 (Sonika): Moving the setting of irq_port for BXT WA outside so that 
> > this can be set for both hdmi or dp ports. For HDMI this is required 
> > because we get interrupts for portB on the hpd line of portA for BXT 
> > A0/A1.
> > This issue occurred because hpd on edp was not disabled which was done 
> > as part of "drm/i915: Dont enable hpd for eDP" from the series:
> > http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.htm
> > l
> > 
> > This patch can be squashed to :
> > commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> > Author: Sonika Jindal <sonika.jindal@intel.com>
> > Date:   Mon Aug 10 10:35:36 2015 +0530
> > 
> >     drm/i915/bxt: WA for swapped HPD pins in A stepping
> > 
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> 
> Queued for -next, thanks for the patch.
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |   21 ++++++++++++---------
> >  1 file changed, 12 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 56d778f..bba0cb6 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >  			goto err;
> >  
> >  		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > -		/*
> > -		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > -		 * interrupts to check the external panel connection.
> > -		 */
> > -		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> > -					 && port == PORT_B)
> > -			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > -		else
> > -			dev_priv->hotplug.irq_port[port] = intel_dig_port;
> > +		dev_priv->hotplug.irq_port[port] = intel_dig_port;
> >  	}
> >  
> >  	/* In theory we don't need the encoder->type check, but leave it 
> > just in @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >  		if (!intel_ddi_init_hdmi_connector(intel_dig_port))
> >  			goto err;
> >  	}
> > +	/*
> > +	 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > +	 * interrupts to check the external panel connection.
> > +	 */
> > +	if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> > +		if (port == PORT_B) {
> > +			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > +			intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > +		} else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> > +			dev_priv->hotplug.irq_port[port] = NULL;
> > +	}
> >  
> >  	return;
> >  
> > --
> > 1.7.10.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
sonika.jindal@intel.com Sept. 2, 2015, 11:59 a.m. UTC | #4
When hpd occurs on port_b, due to the below assignment, it calls digital_port_wok_func first.
There it tries to check the connector status for port B (actually checking port A's HPD pin due to the BXT WA).
It finds it connected and continues to read the dpcd. Since this port was only initialized as HDMI and not initialized as DP, we haven't initialized the aux and causes crash in the case when the port is only enumerated as HDMI alone.
So, this part needs to be moved under the init_dp check itself.
I will be posting Durga's original patch along with hdmi patches because this is required for hdmi to work on bxt.

Regards,
Sonika

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Wednesday, September 2, 2015 5:21 PM
To: Jindal, Sonika
Cc: Daniel Vetter; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP

On Wed, Sep 02, 2015 at 11:48:36AM +0000, Jindal, Sonika wrote:
> :( This had a hole..
> Please drop this patch..

What kind of hole? It sounds like we need this to avoid blowing up when we handle the edp hpd interrupt everywhere? Please explain so I can understand what I've missed here ...

Thanks, Daniel

> 
> I am going to send another patch tested with hdmi optimization series for bxt.
> 
> Regards,
> Sonika
> 
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of 
> Daniel Vetter
> Sent: Wednesday, September 2, 2015 5:17 PM
> To: Jindal, Sonika
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Fix irq_port for eDP
> 
> On Mon, Aug 31, 2015 at 02:35:32PM +0530, Sonika Jindal wrote:
> > From: Durgadoss R <durgadoss.r@intel.com>
> > 
> > Currently, HDMI hotplug with eDP as local panel is failing because 
> > the HDMI hpd is detected as a long hpd for eDP; and is thus 
> > rightfully ignored. But, it should really be handled as an interrupt 
> > on port B for HDMI (due to BXT A1 platform having HPD pins A and B swapped).
> > This patch sets the irq_port[PORT_A] to NULL in case eDP is on port 
> > A so that irq handler does not treat it as a 'dig_port' interrupt.
> > 
> > v2 (Sonika): Moving the setting of irq_port for BXT WA outside so 
> > that this can be set for both hdmi or dp ports. For HDMI this is 
> > required because we get interrupts for portB on the hpd line of 
> > portA for BXT A0/A1.
> > This issue occurred because hpd on edp was not disabled which was 
> > done as part of "drm/i915: Dont enable hpd for eDP" from the series:
> > http://lists.freedesktop.org/archives/intel-gfx/2015-August/073266.h
> > tm
> > l
> > 
> > This patch can be squashed to :
> > commit cf1d58833f07afbb4534b15caa3fd48baa313b2c
> > Author: Sonika Jindal <sonika.jindal@intel.com>
> > Date:   Mon Aug 10 10:35:36 2015 +0530
> > 
> >     drm/i915/bxt: WA for swapped HPD pins in A stepping
> > 
> > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>
> 
> Queued for -next, thanks for the patch.
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |   21 ++++++++++++---------
> >  1 file changed, 12 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 56d778f..bba0cb6 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3242,15 +3242,7 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >  			goto err;
> >  
> >  		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > -		/*
> > -		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > -		 * interrupts to check the external panel connection.
> > -		 */
> > -		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> > -					 && port == PORT_B)
> > -			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > -		else
> > -			dev_priv->hotplug.irq_port[port] = intel_dig_port;
> > +		dev_priv->hotplug.irq_port[port] = intel_dig_port;
> >  	}
> >  
> >  	/* In theory we don't need the encoder->type check, but leave it 
> > just in @@ -3259,6 +3251,17 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
> >  		if (!intel_ddi_init_hdmi_connector(intel_dig_port))
> >  			goto err;
> >  	}
> > +	/*
> > +	 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> > +	 * interrupts to check the external panel connection.
> > +	 */
> > +	if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> > +		if (port == PORT_B) {
> > +			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> > +			intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> > +		} else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> > +			dev_priv->hotplug.irq_port[port] = NULL;
> > +	}
> >  
> >  	return;
> >  
> > --
> > 1.7.10.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 56d778f..bba0cb6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3242,15 +3242,7 @@  void intel_ddi_init(struct drm_device *dev, enum port port)
 			goto err;
 
 		intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
-		/*
-		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
-		 * interrupts to check the external panel connection.
-		 */
-		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
-					 && port == PORT_B)
-			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
-		else
-			dev_priv->hotplug.irq_port[port] = intel_dig_port;
+		dev_priv->hotplug.irq_port[port] = intel_dig_port;
 	}
 
 	/* In theory we don't need the encoder->type check, but leave it just in
@@ -3259,6 +3251,17 @@  void intel_ddi_init(struct drm_device *dev, enum port port)
 		if (!intel_ddi_init_hdmi_connector(intel_dig_port))
 			goto err;
 	}
+	/*
+	 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+	 * interrupts to check the external panel connection.
+	 */
+	if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
+		if (port == PORT_B) {
+			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
+			intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
+		} else if (intel_encoder->type == INTEL_OUTPUT_EDP)
+			dev_priv->hotplug.irq_port[port] = NULL;
+	}
 
 	return;