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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 20 Oct 2026, Xiao Lu <xiaolu.xie@intel.com> wrote:
> When the last MST stream is disabled, i915 sends SET_POWER D3 to the
> MST trunk while MSTM_CTRL (DPCD 0x111) still has MST mode enabled.
> On re-enable, POWER_UP_PHY is sent before MSTM_CTRL is restored, so
> the hub may process the PHY power-up in an inconsistent state and
> fail to retrain its downstream links.
>
> Fix this by clearing MST_EN and UP_REQ_EN in MSTM_CTRL before sending
> D3 on the last stream disable. On re-enable, restore MSTM_CTRL after
> writing D0 and before sending POWER_UP_PHY, so the hub is back in MST
> mode when it processes the PHY power-up and retrains its downstream
> links.
>
> Keep UPSTREAM_IS_SRC set while MST mode is disabled this way, and
> accept this value in intel_dp_mst_verify_dpcd_state() while no MST
> stream is active. Otherwise a connector detection during DPMS off,
> for instance after a runtime resume, would see MSTM_CTRL as reset and
> tear down the MST topology. A real DPCD reset still clears
> UPSTREAM_IS_SRC and so it's still detected.

I did not dig deep into this, but my overall comment is that I think
accessing DP_MSTM_CTRL should be done in the topology manager alone, not
split between the topology manager and the drivers.


BR,
Jani.


>
> v2: Write UPSTREAM_IS_SRC instead of 0 and accept it in
>     intel_dp_mst_verify_dpcd_state(), to avoid removing the topology
>     on a connector detection during DPMS off. (Marco)
>
> Cc: Marco Nenciarini <mnencia@kcore.it>
> Signed-off-by: Xiao Lu <xiaolu.xie@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c    |  3 +++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c | 14 +++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index a46ab5d3d..6b7665206 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3163,6 +3163,9 @@ 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,
> +				       DP_UPSTREAM_IS_SRC);
>  	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 7023619aa..da3cf7493 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);
>  
> @@ -2254,6 +2258,14 @@ bool intel_dp_mst_verify_dpcd_state(struct intel_dp *intel_dp)
>  
>  	ret = drm_dp_dpcd_read_byte(intel_dp->mst.mgr.aux, DP_MSTM_CTRL, &val);
>  
> +	/*
> +	 * MST mode is disabled while all MST streams are disabled, keeping
> +	 * UPSTREAM_IS_SRC set to tell this apart from a DPCD reset.
> +	 */
> +	if (ret == 0 && val == DP_UPSTREAM_IS_SRC &&
> +	    intel_dp_mst_active_streams(intel_dp) == 0)
> +		return true;
> +
>  	/* Adjust the expected register value for SST + SideBand. */
>  	if (ret < 0 || val != (DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC)) {
>  		drm_dbg_kms(display->drm,

-- 
Jani Nikula, Intel