Re: [PATCH] drm/i915/dp: clear and restore MSTM_CTRL around MST DPMS off/on

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/dp: clearing DP_MSTM_CTRL on DPMS OFF breaks MST topology
--

commit 7efe593dfb9be6f1dddd0b7978cd417266811e58
Author: Xiao Lu <xiaolu.xie@intel.com>

drm/i915/dp: clear and restore MSTM_CTRL around MST DPMS off/on

This commit modifies the MST stream disable path to clear MSTM_CTRL before
sending D3, and restores it on re-enable. This is done so the hub is back
in MST mode before processing the PHY power-up and retraining downstream
links.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a46ab5d3d70fc..7e4e18345ec17 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3163,6 +3163,8 @@ static void intel_ddi_post_disable_dp(struct intel_atomic_state *state,
>  	 * Power down sink before disabling the port, otherwise we end
>  	 * up getting interrupts from the sink on detecting link loss.
>  	 */
> +	if (is_mst)
> +		drm_dp_dpcd_write_byte(&intel_dp->aux, DP_MSTM_CTRL, 0);

[Severity: High]
Does clearing DP_MSTM_CTRL to 0 here cause the entire MST topology to be
torn down unexpectedly?

By wiping the MST hardware state on the hub without notifying the software
topology manager, if a user-space compositor or sysfs polls the connector
during DPMS OFF, intel_dp_detect() will read DP_MSTM_CTRL as 0.

In intel_dp_mst_verify_dpcd_state():
	ret = drm_dp_dpcd_read_byte(intel_dp->mst.mgr.aux, DP_MSTM_CTRL, &val);

	if (ret < 0 || val != (DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC)) {
		...

This will return false, causing intel_dp_detect() to force a connector
disconnect:

	if (status != connector_status_disconnected &&
	    !intel_dp_mst_verify_dpcd_state(intel_dp))
		...
		status = connector_status_disconnected;

>  	intel_dp_set_power(intel_dp, DP_SET_POWER_D3);
>  
>  	if (DISPLAY_VER(display) >= 12) {

[ ... ]

> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 7023619aaaba9..4109bb9284fd0 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -1270,8 +1270,12 @@ static void mst_stream_pre_enable(struct intel_atomic_state *state,
>  	drm_WARN_ON(display->drm, DISPLAY_VER(display) >= 12 && first_mst_stream &&
>  		    !intel_dp_mst_is_master_trans(pipe_config));
>  
> -	if (first_mst_stream)
> +	if (first_mst_stream) {
>  		intel_dp_set_power(intel_dp, DP_SET_POWER_D0);
> +		drm_dp_dpcd_write_byte(&intel_dp->aux, DP_MSTM_CTRL,
> +				       DP_MST_EN | DP_UP_REQ_EN |
> +				       DP_UPSTREAM_IS_SRC);
> +	}
>  
>  	drm_dp_send_power_updown_phy(&intel_dp->mst.mgr, connector->mst.port, true);

[Severity: High]
Since DP_MSTM_CTRL is restored here on DPMS ON, the hardware returns to MST
mode, but the software manager (drm_dp_mst_topology_mgr) is unaware of the
reset and hasn't been prompted to send a LINK_ADDRESS broadcast.

Will the immediate drm_dp_send_power_updown_phy() sideband message fail
because the hub lacks downstream routing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014081448.2320363-1-xiaolu.xie@intel.com?part=1