diff mbox series

drm/i915/display: Fixed the main link lost in MST

Message ID 20240422075514.1529260-1-gareth.yu@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: Fixed the main link lost in MST | expand

Commit Message

Yu, Gareth April 22, 2024, 7:55 a.m. UTC
From: Gareth Yu <gareth.yu@intel.com>

Re-train the main link when the sink asserts a HPD for the main lnk 
lost.

Cc : Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
Cc : Matt Roper <matthew.d.roper@intel.com>
Cc : Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Gareth Yu <gareth.yu@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Jani Nikula April 22, 2024, 8:27 a.m. UTC | #1
On Mon, 22 Apr 2024, gareth.yu@intel.com wrote:
> From: Gareth Yu <gareth.yu@intel.com>
>
> Re-train the main link when the sink asserts a HPD for the main lnk 
> lost.

This is a completely inadequate commit message for such a fundamental
change.

Preferrably we'd additionally like a bug filed at fdo gitlab, with debug
logs that show the situation, etc.

> Cc : Tejas Upadhyay <tejaskumarx.surendrakumar.upadhyay@intel.com>
> Cc : Matt Roper <matthew.d.roper@intel.com>
> Cc : Ville Syrjälä <ville.syrjala@linux.intel.com>

Superfluous space before ":".

> Signed-off-by: Gareth Yu <gareth.yu@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index e05e25cd4a94..db5d4fa8340b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5849,8 +5849,13 @@ 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)) {
>  		status = intel_dp_detect_dpcd(intel_dp);
> +		if (status == connector_status_connected && intel_dp->is_mst &&
> +		    !intel_dp_mst_link_status(intel_dp))
> +			if (intel_dp_retrain_link(encoder, ctx))
> +				status = connector_status_disconnected;
> +	}

If we were to add this, this is definitely not the place. You can't look
at ->is_mst for clues here because it's between mst detect and
configure. We might be unconfiguring mst. This also adds unnecessary
checks for status == connected, when it's all handled below with an
early exist for disconnected, etc. And there's a path for retrain too.

All of this has to fit the existing paths nicely instead of just shoving
it here at random.


BR,
Jani.

>  	else
>  		status = connector_status_disconnected;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e05e25cd4a94..db5d4fa8340b 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5849,8 +5849,13 @@  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)) {
 		status = intel_dp_detect_dpcd(intel_dp);
+		if (status == connector_status_connected && intel_dp->is_mst &&
+		    !intel_dp_mst_link_status(intel_dp))
+			if (intel_dp_retrain_link(encoder, ctx))
+				status = connector_status_disconnected;
+	}
 	else
 		status = connector_status_disconnected;