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

[Yongxing Mou <yongxing.mou@oss.qualcomm.com>]

On 6/16/2026 8:45 AM, Dmitry Baryshkov wrote:
> 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@quicinc.com>
>>>>
>>>> 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@quicinc.com>
>>>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>>>> ---
>>>>    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.
> 
Got it.
>> It's not a DP spec
>> requirement, and other drivers (drm_dp_mst core, i915, amdgpu, nouveau)
>> don't have an equivalent delay.
> 
> Why?
> 
This comes from the downstream implementation and was added due to 
issues observed with some dongles, which reportedly required a ~100 ms 
delay. However, it’s unclear whether such a long delay is actually 
necessary (e.g. if a shorter delay would be sufficient).
Alternatively, we could follow nouveau and avoid adding the delay for 
now, and only introduce it if it turns out to be required.
>>
>>>> +	}
>>>> +
>>>> +	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.
> 
>