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 <[email protected]> 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/[email protected]?part=1
