LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect()
@ 2020-02-14 17:56 Kai-Heng Feng
  2020-03-11  3:24 ` Kai-Heng Feng
  2020-03-11 11:36 ` Ville Syrjälä
  0 siblings, 2 replies; 3+ messages in thread
From: Kai-Heng Feng @ 2020-02-14 17:56 UTC (permalink / raw)
  To: jani.nikula, ville.syrjala, joonas.lahtinen, rodrigo.vivi
  Cc: Kai-Heng Feng, David Airlie, Daniel Vetter,
	José Roberto de Souza, Lucas De Marchi, Imre Deak,
	Chris Wilson, Manasi Navare, Gwan-gyeong Mun, Matt Roper,
	Maarten Lankhorst, Ramalingam C, Uma Shankar, intel-gfx,
	open list:DRM DRIVERS, open list

On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port
becomes useless and never responds to cable hotplugging:
[    3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
[    3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port D

Seems like the lspcon chip on the system in question only gets powered
after the cable is plugged.

So let's call lspcon_init() dynamically to properly initialize the
lspcon chip and make HDMI port work.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v4:
 - Trust VBT in intel_infoframe_init().
 - Init lspcon in intel_dp_detect().

v3:
 - Make sure it's handled under long HPD case.

v2: 
 - Move lspcon_init() inside of intel_dp_hpd_pulse().

 drivers/gpu/drm/i915/display/intel_ddi.c  | 17 +----------------
 drivers/gpu/drm/i915/display/intel_dp.c   | 13 ++++++++++++-
 drivers/gpu/drm/i915/display/intel_hdmi.c |  2 +-
 3 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 33f1dc3d7c1a..ca717434b406 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -4741,7 +4741,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 		&dev_priv->vbt.ddi_port_info[port];
 	struct intel_digital_port *intel_dig_port;
 	struct intel_encoder *encoder;
-	bool init_hdmi, init_dp, init_lspcon = false;
+	bool init_hdmi, init_dp;
 	enum phy phy = intel_port_to_phy(dev_priv, port);
 
 	init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
@@ -4754,7 +4754,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 		 * is initialized before lspcon.
 		 */
 		init_dp = true;
-		init_lspcon = true;
 		init_hdmi = false;
 		DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port));
 	}
@@ -4833,20 +4832,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 			goto err;
 	}
 
-	if (init_lspcon) {
-		if (lspcon_init(intel_dig_port))
-			/* TODO: handle hdmi info frame part */
-			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
-				port_name(port));
-		else
-			/*
-			 * LSPCON init faied, but DP init was success, so
-			 * lets try to drive as DP++ port.
-			 */
-			DRM_ERROR("LSPCON init failed on port %c\n",
-				port_name(port));
-	}
-
 	intel_infoframe_init(intel_dig_port);
 
 	return;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index c7424e2a04a3..43117aa86292 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5663,8 +5663,19 @@ intel_dp_detect(struct drm_connector *connector,
 	/* Can't disconnect eDP */
 	if (intel_dp_is_edp(intel_dp))
 		status = edp_detect(intel_dp);
-	else if (intel_digital_port_connected(encoder))
+	else if (intel_digital_port_connected(encoder)) {
+		if (intel_bios_is_lspcon_present(dev_priv, dig_port->base.port) &&
+		    !dig_port->lspcon.active) {
+			if (lspcon_init(dig_port))
+				DRM_DEBUG_KMS("LSPCON init success on port %c\n",
+					      port_name(dig_port->base.port));
+			else
+				DRM_DEBUG_KMS("LSPCON init failed on port %c\n",
+					      port_name(dig_port->base.port));
+		}
+
 		status = intel_dp_detect_dpcd(intel_dp);
+	}
 	else
 		status = connector_status_disconnected;
 
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 93ac0f296852..27a5aa8cefc9 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -3100,7 +3100,7 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
 		intel_dig_port->set_infoframes = g4x_set_infoframes;
 		intel_dig_port->infoframes_enabled = g4x_infoframes_enabled;
 	} else if (HAS_DDI(dev_priv)) {
-		if (intel_dig_port->lspcon.active) {
+		if (intel_bios_is_lspcon_present(dev_priv, intel_dig_port->base.port)) {
 			intel_dig_port->write_infoframe = lspcon_write_infoframe;
 			intel_dig_port->read_infoframe = lspcon_read_infoframe;
 			intel_dig_port->set_infoframes = lspcon_set_infoframes;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect()
  2020-02-14 17:56 [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect() Kai-Heng Feng
@ 2020-03-11  3:24 ` Kai-Heng Feng
  2020-03-11 11:36 ` Ville Syrjälä
  1 sibling, 0 replies; 3+ messages in thread
From: Kai-Heng Feng @ 2020-03-11  3:24 UTC (permalink / raw)
  To: Jani Nikula, ville.syrjala, joonas.lahtinen, rodrigo.vivi
  Cc: David Airlie, Daniel Vetter, José Roberto de Souza,
	Lucas De Marchi, Imre Deak, Chris Wilson, Manasi Navare,
	Gwan-gyeong Mun, Matt Roper, Maarten Lankhorst, Ramalingam C,
	Uma Shankar, intel-gfx, open list:DRM DRIVERS, open list



> On Feb 15, 2020, at 01:56, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> 
> On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port
> becomes useless and never responds to cable hotplugging:
> [    3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
> [    3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port D
> 
> Seems like the lspcon chip on the system in question only gets powered
> after the cable is plugged.
> 
> So let's call lspcon_init() dynamically to properly initialize the
> lspcon chip and make HDMI port work.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

A gentle ping.

> ---
> v4:
> - Trust VBT in intel_infoframe_init().
> - Init lspcon in intel_dp_detect().
> 
> v3:
> - Make sure it's handled under long HPD case.
> 
> v2: 
> - Move lspcon_init() inside of intel_dp_hpd_pulse().
> 
> drivers/gpu/drm/i915/display/intel_ddi.c  | 17 +----------------
> drivers/gpu/drm/i915/display/intel_dp.c   | 13 ++++++++++++-
> drivers/gpu/drm/i915/display/intel_hdmi.c |  2 +-
> 3 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 33f1dc3d7c1a..ca717434b406 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4741,7 +4741,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> 		&dev_priv->vbt.ddi_port_info[port];
> 	struct intel_digital_port *intel_dig_port;
> 	struct intel_encoder *encoder;
> -	bool init_hdmi, init_dp, init_lspcon = false;
> +	bool init_hdmi, init_dp;
> 	enum phy phy = intel_port_to_phy(dev_priv, port);
> 
> 	init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
> @@ -4754,7 +4754,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> 		 * is initialized before lspcon.
> 		 */
> 		init_dp = true;
> -		init_lspcon = true;
> 		init_hdmi = false;
> 		DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port));
> 	}
> @@ -4833,20 +4832,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> 			goto err;
> 	}
> 
> -	if (init_lspcon) {
> -		if (lspcon_init(intel_dig_port))
> -			/* TODO: handle hdmi info frame part */
> -			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
> -				port_name(port));
> -		else
> -			/*
> -			 * LSPCON init faied, but DP init was success, so
> -			 * lets try to drive as DP++ port.
> -			 */
> -			DRM_ERROR("LSPCON init failed on port %c\n",
> -				port_name(port));
> -	}
> -
> 	intel_infoframe_init(intel_dig_port);
> 
> 	return;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index c7424e2a04a3..43117aa86292 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5663,8 +5663,19 @@ intel_dp_detect(struct drm_connector *connector,
> 	/* Can't disconnect eDP */
> 	if (intel_dp_is_edp(intel_dp))
> 		status = edp_detect(intel_dp);
> -	else if (intel_digital_port_connected(encoder))
> +	else if (intel_digital_port_connected(encoder)) {
> +		if (intel_bios_is_lspcon_present(dev_priv, dig_port->base.port) &&
> +		    !dig_port->lspcon.active) {
> +			if (lspcon_init(dig_port))
> +				DRM_DEBUG_KMS("LSPCON init success on port %c\n",
> +					      port_name(dig_port->base.port));
> +			else
> +				DRM_DEBUG_KMS("LSPCON init failed on port %c\n",
> +					      port_name(dig_port->base.port));
> +		}
> +
> 		status = intel_dp_detect_dpcd(intel_dp);
> +	}
> 	else
> 		status = connector_status_disconnected;
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 93ac0f296852..27a5aa8cefc9 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3100,7 +3100,7 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
> 		intel_dig_port->set_infoframes = g4x_set_infoframes;
> 		intel_dig_port->infoframes_enabled = g4x_infoframes_enabled;
> 	} else if (HAS_DDI(dev_priv)) {
> -		if (intel_dig_port->lspcon.active) {
> +		if (intel_bios_is_lspcon_present(dev_priv, intel_dig_port->base.port)) {
> 			intel_dig_port->write_infoframe = lspcon_write_infoframe;
> 			intel_dig_port->read_infoframe = lspcon_read_infoframe;
> 			intel_dig_port->set_infoframes = lspcon_set_infoframes;
> -- 
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect()
  2020-02-14 17:56 [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect() Kai-Heng Feng
  2020-03-11  3:24 ` Kai-Heng Feng
@ 2020-03-11 11:36 ` Ville Syrjälä
  1 sibling, 0 replies; 3+ messages in thread
From: Ville Syrjälä @ 2020-03-11 11:36 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: jani.nikula, joonas.lahtinen, rodrigo.vivi, David Airlie,
	Daniel Vetter, José Roberto de Souza, Lucas De Marchi,
	Imre Deak, Chris Wilson, Manasi Navare, Gwan-gyeong Mun,
	Matt Roper, Maarten Lankhorst, Ramalingam C, Uma Shankar,
	intel-gfx, open list:DRM DRIVERS, open list

On Sat, Feb 15, 2020 at 01:56:27AM +0800, Kai-Heng Feng wrote:
> On HP 800 G4 DM, if HDMI cable isn't plugged before boot, the HDMI port
> becomes useless and never responds to cable hotplugging:
> [    3.031904] [drm:lspcon_init [i915]] *ERROR* Failed to probe lspcon
> [    3.031945] [drm:intel_ddi_init [i915]] *ERROR* LSPCON init failed on port D
> 
> Seems like the lspcon chip on the system in question only gets powered
> after the cable is plugged.
> 
> So let's call lspcon_init() dynamically to properly initialize the
> lspcon chip and make HDMI port work.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v4:
>  - Trust VBT in intel_infoframe_init().
>  - Init lspcon in intel_dp_detect().
> 
> v3:
>  - Make sure it's handled under long HPD case.
> 
> v2: 
>  - Move lspcon_init() inside of intel_dp_hpd_pulse().
> 
>  drivers/gpu/drm/i915/display/intel_ddi.c  | 17 +----------------
>  drivers/gpu/drm/i915/display/intel_dp.c   | 13 ++++++++++++-
>  drivers/gpu/drm/i915/display/intel_hdmi.c |  2 +-
>  3 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 33f1dc3d7c1a..ca717434b406 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4741,7 +4741,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  		&dev_priv->vbt.ddi_port_info[port];
>  	struct intel_digital_port *intel_dig_port;
>  	struct intel_encoder *encoder;
> -	bool init_hdmi, init_dp, init_lspcon = false;
> +	bool init_hdmi, init_dp;
>  	enum phy phy = intel_port_to_phy(dev_priv, port);
>  
>  	init_hdmi = port_info->supports_dvi || port_info->supports_hdmi;
> @@ -4754,7 +4754,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  		 * is initialized before lspcon.
>  		 */
>  		init_dp = true;
> -		init_lspcon = true;
>  		init_hdmi = false;
>  		DRM_DEBUG_KMS("VBT says port %c has lspcon\n", port_name(port));
>  	}
> @@ -4833,20 +4832,6 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  			goto err;
>  	}
>  
> -	if (init_lspcon) {
> -		if (lspcon_init(intel_dig_port))
> -			/* TODO: handle hdmi info frame part */
> -			DRM_DEBUG_KMS("LSPCON init success on port %c\n",
> -				port_name(port));
> -		else
> -			/*
> -			 * LSPCON init faied, but DP init was success, so
> -			 * lets try to drive as DP++ port.
> -			 */
> -			DRM_ERROR("LSPCON init failed on port %c\n",
> -				port_name(port));
> -	}
> -
>  	intel_infoframe_init(intel_dig_port);
>  
>  	return;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index c7424e2a04a3..43117aa86292 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5663,8 +5663,19 @@ intel_dp_detect(struct drm_connector *connector,
>  	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
>  		status = edp_detect(intel_dp);
> -	else if (intel_digital_port_connected(encoder))
> +	else if (intel_digital_port_connected(encoder)) {
> +		if (intel_bios_is_lspcon_present(dev_priv, dig_port->base.port) &&
> +		    !dig_port->lspcon.active) {
> +			if (lspcon_init(dig_port))
> +				DRM_DEBUG_KMS("LSPCON init success on port %c\n",
> +					      port_name(dig_port->base.port));
> +			else
> +				DRM_DEBUG_KMS("LSPCON init failed on port %c\n",
> +					      port_name(dig_port->base.port));
> +		}

I was going to ask what happens when you unplug+replug, but looks like
we already have lspcon_resume()in intel_dp_detect_dpcd(). This should
be there as well. In fact I think we should just move all the logic
into the lspcon code and let it decide on its own whether to take the init
path or the resume path (assuming there is even any difference between
the two).

Not sure what we should do with the lspcon_resume() call in
intel_dp_encoder_reset()...

> +
>  		status = intel_dp_detect_dpcd(intel_dp);
> +	}
>  	else
>  		status = connector_status_disconnected;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 93ac0f296852..27a5aa8cefc9 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3100,7 +3100,7 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
>  		intel_dig_port->set_infoframes = g4x_set_infoframes;
>  		intel_dig_port->infoframes_enabled = g4x_infoframes_enabled;
>  	} else if (HAS_DDI(dev_priv)) {
> -		if (intel_dig_port->lspcon.active) {
> +		if (intel_bios_is_lspcon_present(dev_priv, intel_dig_port->base.port)) {
>  			intel_dig_port->write_infoframe = lspcon_write_infoframe;
>  			intel_dig_port->read_infoframe = lspcon_read_infoframe;
>  			intel_dig_port->set_infoframes = lspcon_set_infoframes;
> -- 
> 2.17.1

-- 
Ville Syrjälä
Intel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-03-11 11:36 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 17:56 [PATCH v4] drm/i915: Init lspcon after HPD in intel_dp_detect() Kai-Heng Feng
2020-03-11  3:24 ` Kai-Heng Feng
2020-03-11 11:36 ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).