Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Splitting `msm_dp_ctrl_config_ctrl()` causes stream configuration state to be permanently lost during dynamic link maintenance, leading to display corruption. --
commit 83fda55d30fe1cd098ad8cc76e151a0398a09b6a Author: Yongxing Mou <[email protected]> drm/msm/dp: split msm_dp_ctrl_config_ctrl() into link parts and stream parts The DP_CONFIGURATION_CTRL register contains both link-level and stream-specific fields. Currently, msm_dp_ctrl_config_ctrl() configures all of them together. Separate the configuration into link parts and stream parts to support MST. > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c > b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 86ef8c89ad443..ed2ba47881fd0 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c [ ... ] > +static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl) > +{ > + u32 config = 0; [ ... ] > - 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); > } [ ... ] > @@ -1628,7 +1644,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); [Severity: High] Does splitting this configuration cause the stream state to be permanently lost during dynamic link maintenance? Before this patch, msm_dp_ctrl_config_ctrl() programmed both link and stream parameters. Now, msm_dp_ctrl_config_ctrl_link() initializes config = 0 and overwrites REG_DP_CONFIGURATION_CTRL, which erases any existing stream configuration. While normal initialization restores stream parameters by calling msm_dp_ctrl_configure_source_params() after link training, dynamic link maintenance paths (like msm_dp_ctrl_link_maintenance() calling msm_dp_ctrl_setup_main_link()) perform link training and then transition directly to DP_STATE_CTRL_SEND_VIDEO without re-applying the stream parameters: msm_dp_ctrl_link_maintenance() ... ret = msm_dp_ctrl_setup_main_link(ctrl, &training_step); ... msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, DP_STATE_CTRL_SEND_VIDEO); Since msm_dp_ctrl_setup_main_link() eventually triggers msm_dp_ctrl_link_train() which calls msm_dp_ctrl_config_ctrl_link(), will this result in the DP stream coming back up with critical parameters cleared (e.g., 0 BPC, no VSC, incorrect color format), breaking the display output? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
