Re: [PATCH v4 05/39] drm/msm/dp: splite msm_dp_ctrl_config_ctrl() into link parts and stream parts

[Yongxing Mou]

On 4/12/2026 1:34 AM, Dmitry Baryshkov wrote:

On Fri, Apr 10, 2026 at 05:33:40PM +0800, Yongxing Mou wrote:

The DP_CONFIGURATION_CTRL register contains both link-level and
stream-specific fields. Currently, msm_dp_ctrl_config_ctrl() configures
all of them together. Separates the configuration into link parts and
streams part for support MST.

Signed-off-by: Yongxing Mou <yongxing....@oss.qualcomm.com>
---
  drivers/gpu/drm/msm/dp/dp_ctrl.c | 43 ++++++++++++++++++++++++++--------------
  1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 476346e3ac19..85315467b5d0 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
@@ -388,26 +388,41 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl 
*msm_dp_ctrl)
        drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
  }

-static void msm_dp_ctrl_config_ctrl(struct msm_dp_ctrl_private *ctrl)

+static void msm_dp_ctrl_config_ctrl_streams(struct msm_dp_ctrl_private *ctrl,
+                                           struct msm_dp_panel *msm_dp_panel)
  {
        u32 config = 0, tbd;
+
+       config = msm_dp_read_link(ctrl, REG_DP_CONFIGURATION_CTRL);
+
+       if (msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420)
+               config |= DP_CONFIGURATION_CTRL_RGB_YUV; /* YUV420 */
+
+       tbd = msm_dp_link_get_test_bits_depth(ctrl->link,
+                                             msm_dp_panel->msm_dp_mode.bpp);
+
+       config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
+
+       if (msm_dp_panel->psr_cap.version)
+               config |= DP_CONFIGURATION_CTRL_SEND_VSC;
+
+       drm_dbg_dp(ctrl->drm_dev, "stream DP_CONFIGURATION_CTRL=0x%x\n", 
config);
+
+       msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);

You have an RMW cycle here. Please document what prevents it from racing
with the concurrent msm_dp_ctrl_config_ctrl_link().

Here protected by mst_lock in MST case. Will add a comment.

+}
+
+static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl)
+{
+       u32 config = 0;
        const u8 *dpcd = ctrl->panel->dpcd;

/* Default-> LSCLK DIV: 1/4 LCLK */

        config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT);

- if (ctrl->panel->msm_dp_mode.out_fmt_is_yuv_420)

-               config |= DP_CONFIGURATION_CTRL_RGB_YUV; /* YUV420 */
-
        /* Scrambler reset enable */
        if (drm_dp_alternate_scrambler_reset_cap(dpcd))
                config |= DP_CONFIGURATION_CTRL_ASSR;

- tbd = msm_dp_link_get_test_bits_depth(ctrl->link,

-                       ctrl->panel->msm_dp_mode.bpp);
-
-       config |= tbd << DP_CONFIGURATION_CTRL_BPC_SHIFT;
-
        /* Num of Lanes */
        config |= ((ctrl->link->link_params.num_lanes - 1)
                        << DP_CONFIGURATION_CTRL_NUM_OF_LANES_SHIFT);
@@ -421,10 +436,7 @@ static void msm_dp_ctrl_config_ctrl(struct 
msm_dp_ctrl_private *ctrl)
        config |= DP_CONFIGURATION_CTRL_STATIC_DYNAMIC_CN;
        config |= DP_CONFIGURATION_CTRL_SYNC_ASYNC_CLK;

- if (ctrl->panel->psr_cap.version)

-               config |= DP_CONFIGURATION_CTRL_SEND_VSC;
-
-       drm_dbg_dp(ctrl->drm_dev, "DP_CONFIGURATION_CTRL=0x%x\n", config);
+       drm_dbg_dp(ctrl->drm_dev, "link DP_CONFIGURATION_CTRL=0x%x\n", config);

msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);

  }
@@ -450,7 +462,8 @@ static void msm_dp_ctrl_configure_source_params(struct 
msm_dp_ctrl_private *ctrl
        msm_dp_ctrl_lane_mapping(ctrl);
        msm_dp_setup_peripheral_flush(ctrl);

- msm_dp_ctrl_config_ctrl(ctrl);

+       msm_dp_ctrl_config_ctrl_link(ctrl);
+       msm_dp_ctrl_config_ctrl_streams(ctrl, ctrl->panel);

test_bits_depth = msm_dp_link_get_test_bits_depth(ctrl->link, ctrl->panel->msm_dp_mode.bpp);

        colorimetry_cfg = msm_dp_link_get_colorimetry_config(ctrl->link);
@@ -1628,7 +1641,7 @@ static int msm_dp_ctrl_link_train(struct 
msm_dp_ctrl_private *ctrl,
        u8 assr;
        struct msm_dp_link_info link_info = {0};

- msm_dp_ctrl_config_ctrl(ctrl);

+       msm_dp_ctrl_config_ctrl_link(ctrl);

link_info.num_lanes = ctrl->link->link_params.num_lanes;

        link_info.rate = ctrl->link->link_params.rate;

--
2.43.0