Re: [PATCH v4 29/39] drm/msm/dp: add an API to initialize MST on sink side

[Dmitry Baryshkov]

On Mon, Jun 15, 2026 at 05:02:27PM +0800, Yongxing Mou wrote:
> 
> 
> On 4/12/2026 8:15 AM, Dmitry Baryshkov wrote:
> > On Fri, Apr 10, 2026 at 05:34:04PM +0800, Yongxing Mou wrote:
> > > From: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > 
> > > If the DP controller is capable of supporting multiple streams
> > > then initialize the DP sink in MST mode by programming the DP_MSTM_CTRL
> > > DPCD register to enable MST mode.
> > > 
> > > Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > Signed-off-by: Yongxing Mou <yongxing.mou@xxxxxxxxxxxxxxxx>
> > > ---
> > >   drivers/gpu/drm/msm/dp/dp_display.c | 57 ++++++++++++++++++++++++++++++++-----
> > >   1 file changed, 50 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index 8ae690ce2b9f..abf26951819a 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -14,6 +14,7 @@
> > >   #include <linux/string_choices.h>
> > >   #include <drm/display/drm_dp_aux_bus.h>
> > >   #include <drm/display/drm_hdmi_audio_helper.h>
> > > +#include <drm/display/drm_dp_mst_helper.h>
> > >   #include <drm/drm_edid.h>
> > >   #include "msm_drv.h"
> > > @@ -270,6 +271,40 @@ static int msm_dp_display_lttpr_init(struct msm_dp_display_private *dp, u8 *dpcd
> > >   	return lttpr_count;
> > >   }
> > > +static void msm_dp_display_mst_init(struct msm_dp_display_private *dp)
> > > +{
> > > +	const unsigned long clear_mstm_ctrl_timeout_us = 100000;
> > > +	u8 old_mstm_ctrl;
> > > +	struct msm_dp *msm_dp = &dp->msm_dp_display;
> > > +	int ret;
> > > +
> > > +	/* clear sink MST state */
> > > +	drm_dp_dpcd_read_byte(dp->aux, DP_MSTM_CTRL, &old_mstm_ctrl);
> > > +
> > > +	ret = drm_dp_dpcd_write_byte(dp->aux, DP_MSTM_CTRL, 0);
> > > +	if (ret < 0) {
> > > +		DRM_ERROR("failed to clear DP_MSTM_CTRL, ret=%d\n", ret);
> > > +		return;
> > > +	}
> > > +
> > > +	/* add extra delay if MST old state is on*/
> > > +	if (old_mstm_ctrl) {
> > > +		drm_dbg_dp(dp->drm_dev, "wait %luus to set DP_MSTM_CTRL set 0\n",
> > > +			   clear_mstm_ctrl_timeout_us);
> > > +		usleep_range(clear_mstm_ctrl_timeout_us,
> > > +			     clear_mstm_ctrl_timeout_us + 1000);
> > 
> > For 100 ms you should be using msleep() instead. But where is that
> > timeout coming from?
> > 
> Will switch to msleep(100).
> 
> The 100 ms is an empirical workaround carried over from downstream — some
> sinks don't exit MST immediately after writing DP_MSTM_CTRL = 0, and
> re-enabling MST too quickly broke topology probe.

Comment.

> It's not a DP spec
> requirement, and other drivers (drm_dp_mst core, i915, amdgpu, nouveau)
> don't have an equivalent delay.

Why?

> 
> > > +	}
> > > +
> > > +	ret = drm_dp_dpcd_write_byte(dp->aux, DP_MSTM_CTRL,
> > > +				     DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC);
> > 
> > Isn't it too early to enable MST? (I don't remember this part of the
> > standard).
> > 
> No, this follows the DP 1.4a MST spec. DP_MSTM_CTRL must be set before
> topology discovery. Topology discovery uses AUX sideband messages, which
> don’t require link training.
> So we enable MST first, then discover topology, and only do link training
> and payload allocation later in atomic_enable().

Ack, thanks.


-- 
With best wishes
Dmitry