Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Link retraining erases active stream configurations due to overwriting shared register -- commit c62d46d098f37a3918af36fbf87cf140b8d49f60 Author: Yongxing Mou <[email protected]> drm/msm/dp: split link setup from source params msm_dp_ctrl_configure_source_params() should only handle stream-related configuration. Move the link setup out of it so MST can program link and stream settings separately. > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c > b/drivers/gpu/drm/msm/dp/dp_ctrl.c > index 71d45b2c4daf8..1c2eccec6ec6d 100644 > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c [ ... ] > @@ -2551,6 +2547,10 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl > *msm_dp_ctrl, bool force_link_train > */ > reinit_completion(&ctrl->video_comp); > > + msm_dp_ctrl_lane_mapping(ctrl); > + msm_dp_setup_peripheral_flush(ctrl); > + msm_dp_ctrl_config_ctrl_link(ctrl); [Severity: High] This isn't a bug introduced by this patch, but looking at how link setup and stream setup share registers, can msm_dp_ctrl_config_ctrl_link() wipe out active stream configurations? In msm_dp_ctrl_config_ctrl_link(), the REG_DP_CONFIGURATION_CTRL register state is built from 0 rather than reading the existing state: u32 config = 0; const u8 *dpcd = panel->dpcd; /* Default-> LSCLK DIV: 1/4 LCLK */ config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT); ... msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config); When msm_dp_ctrl_link_maintenance() triggers link retraining due to a sink request, it eventually calls msm_dp_ctrl_config_ctrl_link(). Because this function overwrites the shared REG_DP_CONFIGURATION_CTRL register without a read-modify-write, does it erase all stream configurations (like DP_CONFIGURATION_CTRL_RGB_YUV or DP_CONFIGURATION_CTRL_SEND_VSC) previously set by msm_dp_ctrl_config_ctrl_streams()? If the maintenance routine then resumes sending video, this might leave the hardware outputting an incorrect stream format. Would it be safer to use msm_dp_read_link() to preserve the existing register state? > + > msm_dp_ctrl_configure_source_params(ctrl); > > msm_dp_ctrl_config_msa(ctrl, -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
