On Mon, Jun 29, 2026 at 10:14:44PM +0800, Yongxing Mou wrote: > drm/msm/dp: introduce dp_mst_drm for MST stream management
duplicate subject? > > Add a dp_mst_drm layer to manage DP MST streams with a clear ownership > model between encoder, panel and connector. > > Each MST stream is represented by a dedicated drm_encoder. At modeset > initialization time, one (encoder, dp_panel) pair is created per > stream_id and remains fixed for the lifetime of the driver. The > dp_panel thus carries a stable stream context, including stream_id > and pixel mapping. > > MST connectors are created and destroyed dynamically on hotplug and > are attached to a dp_panel through atomic routing. During an atomic > commit, connectors are associated with encoders via > atomic_best_encoder(), forming a temporary binding for the duration > of the commit. This doesnt't seem to be true any longer. > > Encoder helper callbacks drive the MST stream lifecycle, including > timeslot allocation, link enable/disable and payload programming. > > A per-MST-instance lock serializes operations on shared link state > across multiple streams. > > Signed-off-by: Abhinav Kumar <[email protected]> > Signed-off-by: Yongxing Mou <[email protected]> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 + > drivers/gpu/drm/msm/dp/dp_display.c | 9 ++ > drivers/gpu/drm/msm/dp/dp_display.h | 2 + > drivers/gpu/drm/msm/dp/dp_mst_drm.c | 245 > ++++++++++++++++++++++++++++++++ > drivers/gpu/drm/msm/msm_drv.h | 7 + > 5 files changed, 269 insertions(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index b32ecd5b0777..ac5dc844fead 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > @@ -696,6 +696,12 @@ static int _dpu_kms_initialize_displayport(struct > drm_device *dev, > DPU_ERROR("encoder init failed for dp > mst display\n"); > return PTR_ERR(encoder); > } > + > + rc = > msm_dp_mst_attach_encoder(priv->kms->dp[i], encoder); > + if (rc) { > + DPU_ERROR("dp_mst attach_encoder > failed, rc = %d\n", rc); > + return rc; > + } Reoder, this should be a part of the patch, adding the encoder. > } > } > } > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index fc9c1e3e57ab..6eac390af2e0 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -601,6 +601,15 @@ struct msm_dp_panel *msm_dp_display_get_panel(struct > msm_dp *msm_dp_display, > return dp_panel; > } > > +void msm_dp_display_set_link_info(struct msm_dp *msm_dp_display, > + struct msm_dp_link_info *dst) > +{ > + struct msm_dp_display_private *dp = > + container_of(msm_dp_display, struct msm_dp_display_private, > msm_dp_display); > + > + memcpy(dst, &dp->panel->link_info, sizeof(*dst)); > +} > + > static void msm_dp_display_deinit_sub_modules(struct msm_dp_display_private > *dp) > { > msm_dp_audio_put(dp->audio); > diff --git a/drivers/gpu/drm/msm/dp/dp_display.h > b/drivers/gpu/drm/msm/dp/dp_display.h > index a185819ec57e..fb6bdd372b52 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.h > +++ b/drivers/gpu/drm/msm/dp/dp_display.h > @@ -63,4 +63,6 @@ void msm_dp_display_unprepare(struct msm_dp *dp); > > struct msm_dp_panel *msm_dp_display_get_panel(struct msm_dp *msm_dp_display, > enum msm_dp_stream_id stream_id); > +void msm_dp_display_set_link_info(struct msm_dp *msm_dp_display, > + struct msm_dp_link_info *dst); > #endif /* _DP_DISPLAY_H_ */ > diff --git a/drivers/gpu/drm/msm/dp/dp_mst_drm.c > b/drivers/gpu/drm/msm/dp/dp_mst_drm.c > index 78b8dffe111b..6a77fdef85e9 100644 > --- a/drivers/gpu/drm/msm/dp/dp_mst_drm.c > +++ b/drivers/gpu/drm/msm/dp/dp_mst_drm.c > @@ -4,18 +4,259 @@ > */ > > #include <drm/drm_edid.h> > +#include <drm/drm_fixed.h> > +#include <drm/drm_atomic_helper.h> > #include <drm/display/drm_dp_mst_helper.h> > > #include "dp_mst_drm.h" > #include "dp_panel.h" > +#include "dpu_encoder.h" > + > +#define to_dp_mst_connector(x) \ > + container_of((x), struct msm_dp_mst_connector, connector) > + > +struct msm_dp_mst_encoder { > + struct drm_encoder *enc; > + int stream_id; > + struct msm_dp_panel *dp_panel; > +}; > + > +struct msm_dp_mst_connector { > + struct drm_connector connector; > + struct drm_dp_mst_port *mst_port; > + struct msm_dp_mst *dp_mst; > +}; > + Extra empty line. > > struct msm_dp_mst { > struct drm_dp_mst_topology_mgr mst_mgr; > + struct msm_dp_mst_encoder mst_encoders[DP_STREAM_MAX]; > struct msm_dp *msm_dp; > struct drm_dp_aux *dp_aux; > u32 max_streams; > + struct mutex mst_lock; > + struct msm_dp_link_info link_info; > }; > > +static struct msm_dp_panel *msm_dp_mst_panel_from_encoder(struct msm_dp_mst > *mst, > + struct drm_encoder > *enc) > +{ > + int i; > + > + for (i = 0; i < mst->max_streams; i++) { > + if (mst->mst_encoders[i].enc == enc) > + return mst->mst_encoders[i].dp_panel; > + } > + return NULL; > +} > + > +static void msm_dp_mst_update_timeslots(struct msm_dp_mst *mst, > + struct msm_dp_panel *panel, > + struct drm_dp_mst_atomic_payload > *payload) > +{ > + if (payload->vc_start_slot < 0) > + msm_dp_display_set_stream_info(mst->msm_dp, panel, 1, 0, 0); > + else > + msm_dp_display_set_stream_info(mst->msm_dp, panel, > + payload->vc_start_slot, > + payload->time_slots, > payload->pbn); Can you pass payload as is? Then you don't need a wrapping function. > + > + drm_dbg_kms(mst->msm_dp->drm_dev, > + "[MST] stream:%u timeslots vc_start:%d slots:%d pbn:%d\n", > + panel->stream_id, payload->vc_start_slot, > + payload->time_slots, payload->pbn); > +} > + > +static void msm_dp_mst_stream_enable(struct drm_encoder *encoder, > + struct drm_atomic_commit *state) > +{ > + struct drm_connector *connector = > + drm_atomic_get_new_connector_for_encoder(state, encoder); > + struct msm_dp_mst_connector *mst_conn = to_dp_mst_connector(connector); > + struct msm_dp_mst *mst = mst_conn->dp_mst; > + struct msm_dp *dp_display = mst->msm_dp; > + struct msm_dp_panel *panel = msm_dp_mst_panel_from_encoder(mst, > encoder); > + struct drm_dp_mst_port *port = mst_conn->mst_port; > + struct drm_dp_mst_topology_state *mst_state = > + drm_atomic_get_new_mst_topology_state(state, &mst->mst_mgr); > + struct drm_dp_mst_atomic_payload *payload = > + drm_atomic_get_mst_payload_state(mst_state, port); > + int rc; > + > + panel->connector = connector; The connector should be _get before assigning. > + > + guard(mutex)(&mst->mst_lock); > + > + rc = msm_dp_display_set_mode_helper(dp_display, state, encoder, panel); > + if (rc) { > + drm_err(dp_display->drm_dev, > + "[MST] stream:%u set_mode failed rc=%d\n", > panel->stream_id, rc); > + goto out; > + } > + > + rc = msm_dp_display_prepare_link(dp_display); > + if (rc) { > + drm_err(dp_display->drm_dev, > + "[MST] stream:%u prepare_link failed rc=%d\n", > panel->stream_id, rc); > + msm_dp_display_unprepare(dp_display); > + goto out; > + } > + > + drm_dp_mst_update_slots(mst_state, DP_CAP_ANSI_8B10B); > + > + rc = drm_dp_add_payload_part1(&mst->mst_mgr, mst_state, payload); > + if (rc) { > + drm_err(dp_display->drm_dev, > + "[MST] payload allocation failure for conn:%d\n", > connector->base.id); > + msm_dp_display_unprepare(dp_display); > + goto out; > + } > + > + msm_dp_mst_update_timeslots(mst, panel, payload); > + > + msm_dp_display_enable_helper(dp_display, panel); > + > + drm_dp_check_act_status(&mst->mst_mgr); > + > + drm_dp_add_payload_part2(&mst->mst_mgr, payload); > + > +out: > + drm_connector_get(connector); > +} > + > +static void msm_dp_mst_stream_disable(struct drm_encoder *encoder, > + struct drm_atomic_commit *state) > +{ > + struct drm_connector *connector = > drm_atomic_get_old_connector_for_encoder(state, encoder); > + struct msm_dp_mst_connector *mst_conn = to_dp_mst_connector(connector); > + struct msm_dp_mst *mst = mst_conn->dp_mst; > + struct msm_dp_panel *panel = msm_dp_mst_panel_from_encoder(mst, > encoder); > + struct drm_dp_mst_topology_state *old_mst_state = > + drm_atomic_get_old_mst_topology_state(state, &mst->mst_mgr); > + struct drm_dp_mst_topology_state *new_mst_state = > + drm_atomic_get_new_mst_topology_state(state, &mst->mst_mgr); > + struct drm_dp_mst_atomic_payload *old_payload = > + drm_atomic_get_mst_payload_state(old_mst_state, > mst_conn->mst_port); > + struct drm_dp_mst_atomic_payload *new_payload = > + drm_atomic_get_mst_payload_state(new_mst_state, > mst_conn->mst_port); > + > + guard(mutex)(&mst->mst_lock); > + > + drm_dp_remove_payload_part1(&mst->mst_mgr, new_mst_state, new_payload); > + > + drm_dp_remove_payload_part2(&mst->mst_mgr, new_mst_state, old_payload, > new_payload); > + > + msm_dp_mst_update_timeslots(mst, panel, new_payload); > + > + msm_dp_display_disable_helper(mst->msm_dp, panel); > + > + drm_dp_check_act_status(&mst->mst_mgr); > +} > + > +static void msm_dp_mst_stream_post_disable(struct drm_encoder *encoder, > + struct drm_atomic_commit *state) > +{ > + struct drm_connector *connector = > drm_atomic_get_old_connector_for_encoder(state, encoder); > + struct msm_dp_mst_connector *mst_conn = to_dp_mst_connector(connector); > + struct msm_dp_mst *mst = mst_conn->dp_mst; > + struct msm_dp_panel *panel = msm_dp_mst_panel_from_encoder(mst, > encoder); > + > + guard(mutex)(&mst->mst_lock); > + > + msm_dp_display_atomic_post_disable_helper(mst->msm_dp, panel); > + > + if (!mst->msm_dp->mst_active) > + msm_dp_display_unprepare(mst->msm_dp); > + > + panel->connector = NULL; > + > + drm_connector_put(connector); > +} > + > +static int msm_dp_mst_enc_atomic_check(struct drm_encoder *enc, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct msm_dp_mst_connector *mst_conn = > to_dp_mst_connector(conn_state->connector); > + struct msm_dp_mst *mst = mst_conn->dp_mst; > + struct drm_dp_mst_topology_state *mst_state; > + int bpp, pbn, slots; > + > + if (!conn_state->crtc) > + return 0; > + > + if (!drm_atomic_crtc_needs_modeset(crtc_state) || !crtc_state->active) > + return 0; > + > + bpp = (conn_state->connector->display_info.bpc * 3) ?: 24; /* fallback: > assume 8bpc */ > + pbn = drm_dp_calc_pbn_mode(crtc_state->mode.clock, bpp << 4); > + > + mst_state = drm_atomic_get_mst_topology_state(crtc_state->state, > &mst->mst_mgr); > + if (IS_ERR(mst_state)) > + return PTR_ERR(mst_state); > + > + if (!dfixed_trunc(mst_state->pbn_div)) { > + mst_state->pbn_div = > + drm_dp_get_vc_payload_bw(mst->link_info.rate, > + mst->link_info.num_lanes); > + } > + > + slots = drm_dp_atomic_find_time_slots(crtc_state->state, &mst->mst_mgr, > + mst_conn->mst_port, pbn); > + if (slots < 0) > + return slots; > + > + return 0; > +} > + > +static void msm_dp_mst_enc_atomic_enable(struct drm_encoder *enc, > + struct drm_atomic_commit *state) > +{ > + msm_dp_mst_stream_enable(enc, state); > + dpu_encoder_phys_enable(enc, state); Looking at this, no, squashing the bridge and the encoder was not a good idea, excuse me. The encoder is an internal implement of the DPU layer and we should leave it there as is. DPU can use DP functions, but not other way around. > +} > + > +static void msm_dp_mst_enc_atomic_disable(struct drm_encoder *enc, > + struct drm_atomic_commit *state) > +{ > + msm_dp_mst_stream_disable(enc, state); > + dpu_encoder_phys_disable(enc, state); > + msm_dp_mst_stream_post_disable(enc, state); > +} > + > +static const struct drm_encoder_helper_funcs msm_dp_mst_encoder_helper_funcs > = { > + .atomic_check = msm_dp_mst_enc_atomic_check, > + .atomic_mode_set = dpu_encoder_atomic_mode_set, > + .atomic_enable = msm_dp_mst_enc_atomic_enable, > + .atomic_disable = msm_dp_mst_enc_atomic_disable, > +}; > + > +int msm_dp_mst_attach_encoder(struct msm_dp *dp_display, struct drm_encoder > *encoder) > +{ > + struct msm_dp_mst *mst = dp_display->msm_dp_mst; > + struct msm_dp_panel *dp_panel; > + int i; > + > + for (i = 0; i < mst->max_streams; i++) { > + if (!mst->mst_encoders[i].enc) > + break; > + } > + > + dp_panel = msm_dp_display_get_panel(dp_display, i); > + if (!dp_panel) { > + drm_err(dp_display->drm_dev, > + "[MST] failed to allocate panel for stream %d\n", i); > + return -ENOMEM; > + } > + > + mst->mst_encoders[i].enc = encoder; > + mst->mst_encoders[i].stream_id = i; > + mst->mst_encoders[i].dp_panel = dp_panel; > + drm_encoder_helper_add(encoder, &msm_dp_mst_encoder_helper_funcs); > + > + return 0; > +} > + > int msm_dp_mst_display_set_mgr_state(struct msm_dp *dp_display, bool state) > { > struct msm_dp_mst *mst = dp_display->msm_dp_mst; > @@ -27,6 +268,9 @@ int msm_dp_mst_display_set_mgr_state(struct msm_dp > *dp_display, bool state) > "[MST] failed to set topology mgr state to %d rc:%d\n", > state, rc); > } > > + if (state) Why? > + msm_dp_display_set_link_info(dp_display, &mst->link_info); > + > drm_dbg_kms(dp_display->drm_dev, "[MST] set_mgr_state state:%d\n", > state); > return rc; > } > @@ -55,6 +299,7 @@ int msm_dp_mst_init(struct msm_dp *dp_display, u32 > max_streams, struct drm_dp_au > return ret; > } > > + mutex_init(&mst->mst_lock); > dp_display->msm_dp_mst = mst; > return 0; > } > diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h > index f71200a790f3..854dd08eede2 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -358,6 +358,8 @@ bool msm_dp_needs_periph_flush(const struct msm_dp > *dp_display, > bool msm_dp_wide_bus_available(const struct msm_dp *dp_display); > int msm_dp_get_mst_max_stream(struct msm_dp *dp_display); > int msm_dp_mst_register(struct msm_dp *dp_display); > +int msm_dp_mst_attach_encoder(struct msm_dp *dp_display, struct drm_encoder > *encoder); > + > #else > static inline int __init msm_dp_register(void) > { > @@ -384,6 +386,11 @@ static inline int msm_dp_mst_register(struct msm_dp > *dp_display) > return -EINVAL; > } > > +static inline int msm_dp_mst_attach_encoder(struct msm_dp *dp_display, > struct drm_encoder *encoder) > +{ > + return -EINVAL; > +} > + > static inline void msm_dp_snapshot(struct msm_disp_state *disp_state, struct > msm_dp *dp_display) > { > } > > -- > 2.43.0 > -- With best wishes Dmitry
