Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/i915/dp_mst: missing topology check in mst_connector_stream_disconnected() --
commit b28e17eee0029b86851cad8de209d49fbc2f0399 Author: Xiao Lu <[email protected]> drm/i915/dp_mst: Skip link retrain and the ACT wait for disconnected MST streams This commit skips link retraining when all active pipes on an MST link are bound to disconnected MST sinks, and avoids waiting for a branch ACT during stream enable or disable when no payload change was sent to the branch. Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/17741 > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 7023619aaaba9..ecc7b105e6408 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c [ ... ] > @@ -2172,6 +2178,20 @@ bool intel_dp_mst_crtc_needs_modeset(struct > intel_atomic_state *state, > return false; > } > > +static bool mst_connector_stream_disconnected(struct intel_connector > *connector) > +{ > + if (!connector->mst.dp) > + return false; > + > + if (!connector->mst.dp->mst.mgr.mst_state) > + return true; > + > + if (drm_connector_is_unregistered(&connector->base)) > + return true; > + > + return false; > +} > + > bool intel_dp_mst_stream_disconnected(struct intel_atomic_state *state, > const struct intel_crtc *crtc) > { > @@ -2181,16 +2201,39 @@ bool intel_dp_mst_stream_disconnected(struct > intel_atomic_state *state, > if (!connector) > return false; > > - if (!connector->mst.dp) > - return false; > + return mst_connector_stream_disconnected(connector); > +} > > - if (!connector->mst.dp->mst.mgr.mst_state) > - return true; > +/** > + * intel_dp_mst_disconnected_pipes - get the pipes driving disconnected MST > streams > + * @intel_dp: DP port object > + * > + * Returns the mask of pipes whose MST connector has left the topology, so > + * there is no sink behind them. The caller must hold > + * &drm_mode_config.connection_mutex. > + */ > +u8 intel_dp_mst_disconnected_pipes(struct intel_dp *intel_dp) > +{ > + struct intel_display *display = to_intel_display(intel_dp); > + struct drm_connector_list_iter conn_iter; > + struct intel_connector *connector; > + u8 pipe_mask = 0; > > - if (drm_connector_is_unregistered(&connector->base)) > - return true; > + drm_connector_list_iter_begin(display->drm, &conn_iter); > + for_each_intel_connector_iter(connector, &conn_iter) { > + struct drm_connector_state *conn_state = connector->base.state; > > - return false; > + if (connector->mst.dp != intel_dp || !conn_state->crtc) > + continue; > + > + if (mst_connector_stream_disconnected(connector) || > + !drm_dp_mst_port_downstream_of_parent(&intel_dp->mst.mgr, > + connector->mst.port, > NULL)) [Severity: Medium] As reported by Sashiko on v1 [1], should the drm_dp_mst_port_downstream_of_parent() check live inside mst_connector_stream_disconnected() instead of only in intel_dp_mst_disconnected_pipes()? When an MST port or branch leaves the topology while another stream on the link stays connected (or after upstream HPD reconnects with a new primary branch), connector->mst.port is unlinked from mgr.mst_primary before mgr->delayed_destroy_work asynchronously unregisters the connector. If intel_dp_retrain_link() or a modeset runs during that window, intel_link_bw_init_limits() calls intel_dp_mst_stream_disconnected(): drivers/gpu/drm/i915/display/intel_link_bw.c:intel_link_bw_init_limits() { ... if ((state->base.duplicated && crtc_state) || intel_dp_mst_stream_disconnected(state, crtc)) { limits->max_bpp_x16[pipe] = crtc_state->max_link_bpp_x16; if (intel_dsc_enabled_on_link(crtc_state)) limits->link_dsc_pipes |= BIT(pipe); } else { limits->max_bpp_x16[pipe] = INT_MAX; } ... } Because mst_connector_stream_disconnected() does not check drm_dp_mst_port_downstream_of_parent(), intel_dp_mst_stream_disconnected() returns false while mgr.mst_state is true and the connector is not yet unregistered. Could that reset limits->max_bpp_x16[pipe] to INT_MAX and drop limits->link_dsc_pipes for the disconnected stream, allowing its recomputed TU allocation to exceed mst_state->total_avail_slots in drm_dp_mst_atomic_check_payload_alloc_limits()? [1] https://lore.kernel.org/r/[email protected] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
