On Mon, Aug 25, 2025 at 10:16:12PM +0800, Yongxing Mou wrote:
> From: Abhinav Kumar <quic_abhin...@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_abhin...@quicinc.com>
> Signed-off-by: Yongxing Mou <yongxing....@oss.qualcomm.com>
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 55
> ++++++++++++++++++++++++++++++-------
> 1 file changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index
> cb433103d439ac6b8089bdecf0ee6be35c914db1..84df34306fb557341bea288ea8c13b0c81b11919
> 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"
> @@ -297,6 +298,35 @@ 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_readb(dp->aux, DP_MSTM_CTRL, &old_mstm_ctrl);
> + drm_dp_dpcd_writeb(dp->aux, DP_MSTM_CTRL, 0);
> +
> + /* 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);
> + }
> +
> + ret = drm_dp_dpcd_writeb(dp->aux, DP_MSTM_CTRL,
> + DP_MST_EN | DP_UP_REQ_EN | DP_UPSTREAM_IS_SRC);
> + if (ret < 0) {
I didn't catch this up in the previous patches. We have two sets of DPCD
accessors: the older ones which can return error or the size of the data
that was actually read / written (which might be less than the size of
the buffer passed to the function) and newer ones, which return error or
0. drm_dp_dpcd_writeb() is from the first group, so if it was
successful, it should be returning 1. It's all a pain to handle, so
please start using newer accessors in your patches (the full conversion
of the MSM driver is on my todo list, but it's intrusive, so was
delaying it...).
TL;DR: inside your code please use drm_dp_dpcd_read_byte() /
drm_dp_dpcd_write_byte() / drm_dp_dpcd_read_data() /
drm_dp_dpcd_write_data().
> + DRM_ERROR("sink mst enablement failed\n");
> + return;
> + }
> +
> + msm_dp->mst_active = true;
> +}
> +
> static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
> {
> struct drm_connector *connector = dp->msm_dp_display.connector;
> @@ -315,18 +345,20 @@ static int msm_dp_display_process_hpd_high(struct
> msm_dp_display_private *dp)
> if (rc)
> goto end;
>
> - drm_edid = drm_edid_read_ddc(connector, &dp->aux->ddc);
> - drm_edid_connector_update(connector, drm_edid);
> + if (!dp->mst_supported || !drm_dp_read_mst_cap(dp->aux,
> dp->panel->dpcd)) {
In several previous patches, the functions initializing MST can
return an error, leaving the user with half-initialized MST. Are those
errors a safety coding or do they have some real-use usecase? In the
latter case, it would be nice to keep SST wokring if enabling MST fails.
> + drm_edid = drm_edid_read_ddc(connector, &dp->aux->ddc);
> + drm_edid_connector_update(connector, drm_edid);
>
> - if (!drm_edid) {
> - DRM_ERROR("panel edid read failed\n");
> - /* check edid read fail is due to unplug */
> - if (!msm_dp_aux_is_link_connected(dp->aux))
> - return -ETIMEDOUT;
> - }
> + if (!drm_edid) {
> + DRM_ERROR("panel edid read failed\n");
> + /* check edid read fail is due to unplug */
> + if (!msm_dp_aux_is_link_connected(dp->aux))
> + return -ETIMEDOUT;
> + }
>
> - if (rc)
> - goto end;
> + if (rc)
> + goto end;
> + }
>
> msm_dp_link_process_request(dp->link);
>
> @@ -349,6 +381,9 @@ static int msm_dp_display_process_hpd_high(struct
> msm_dp_display_private *dp)
> */
> msm_dp_link_psm_config(dp->link, &dp->panel->link_info, false);
>
> + if (dp->mst_supported && drm_dp_read_mst_cap(dp->aux, dp->panel->dpcd))
> + msm_dp_display_mst_init(dp);
> +
> msm_dp_link_reset_phy_params_vx_px(dp->link);
>
> if (!dp->msm_dp_display.internal_hpd)
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
