On Mon, Jul 20, 2026 at 06:24:05AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [High] Secondary stream panels fail to have their `hw_revision`
> initialized, silently breaking controller-specific hardware workarounds in
> MST setups.
> - [High] Stream-specific panels are incorrectly passed to mainlink APIs,
> corrupting shared link state and misconfiguring the link based on leaf
> monitor capabilities instead of the MST hub.
> --
>
> --- Patch [10]: [PATCH v8 10/15] drm/msm/dp: allow dp_ctrl stream APIs to use
> any panel passed to it ---
> commit 07d3f4b80ae5875a958a8aceb22c9ffab07db108
> Author: Abhinav Kumar <[email protected]>
>
> drm/msm/dp: allow dp_ctrl stream APIs to use any panel passed to it
>
> With MST, multiple sinks share a single DP controller, so a cached
> panel in msm_dp_ctrl_private can no longer represent the per-stream
> sink. Drop the cache and pass panel explicitly to all stream-related
> dp_ctrl APIs.
>
> > diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > index ae79cbec26d19..207643fa73063 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> > @@ -219,7 +219,7 @@ void msm_dp_ctrl_reset(struct msm_dp_ctrl *msm_dp_ctrl)
> >
> > if (!ctrl->hw_revision) {
> > ctrl->hw_revision = msm_dp_read_ahb(ctrl, REG_DP_HW_VERSION);
> > - ctrl->panel->hw_revision = ctrl->hw_revision;
> > + panel->hw_revision = ctrl->hw_revision;
> > }
> > }
>
> [Severity: High]
> Does this leave the hw_revision uninitialized for secondary stream panels in
> an
> MST setup?
>
> Since ctrl->hw_revision is populated during the host initialization for the
> primary panel, any subsequent stream panels passed to this function will
> bypass
> the assignment. Could this silently break hardware workarounds that depend on
> panel->hw_revision being set correctly?
It does. This should be fixed.
>
> [ ... ]
>
> > @@ -414,10 +414,11 @@ static void msm_dp_ctrl_config_ctrl_streams(struct
> > msm_dp_ctrl_private *ctrl,
> > msm_dp_write_link(ctrl, REG_DP_CONFIGURATION_CTRL, config);
> > }
> >
> > -static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl)
> > +static void msm_dp_ctrl_config_ctrl_link(struct msm_dp_ctrl_private *ctrl,
> > + struct msm_dp_panel *panel)
> > {
> > u32 config = 0;
> > - const u8 *dpcd = ctrl->panel->dpcd;
> > + const u8 *dpcd = panel->dpcd;
> >
> > /* Default-> LSCLK DIV: 1/4 LCLK */
> > config |= (2 << DP_CONFIGURATION_CTRL_LSCLK_DIV_SHIFT);
>
> [Severity: High]
> Can the direct write to REG_DP_CONFIGURATION_CTRL at the end of this function
> erase stream-specific configurations established by previously enabled
> streams?
>
> Since msm_dp_ctrl_config_ctrl_link() is called on every stream enable from
> msm_dp_ctrl_on_stream(), it seems this might overwrite the shared register and
> corrupt the state of active streams.
Hmm?
>
> [ ... ]
>
> > @@ -1810,11 +1821,12 @@ static void msm_dp_ctrl_link_clk_disable(struct
> > msm_dp_ctrl *msm_dp_ctrl)
> > str_on_off(ctrl->core_clks_on));
> > }
> >
> > -static int msm_dp_ctrl_enable_mainlink_clocks(struct msm_dp_ctrl_private
> > *ctrl)
> > +static int msm_dp_ctrl_enable_mainlink_clocks(struct msm_dp_ctrl_private
> > *ctrl,
> > + struct msm_dp_panel *panel)
> > {
> > int ret = 0;
> > struct phy *phy = ctrl->phy;
> > - const u8 *dpcd = ctrl->panel->dpcd;
> > + const u8 *dpcd = panel->dpcd;
> >
> > ctrl->phy_opts.dp.lanes = ctrl->link->link_params.num_lanes;
> > ctrl->phy_opts.dp.link_rate = ctrl->link->link_params.rate / 100;
>
> [Severity: High]
> Is it correct to use the stream-specific (leaf) DPCD to configure shared
> mainlink properties here and in msm_dp_ctrl_link_train()?
>
> This appears to misconfigure the link based on leaf monitor capabilities
> instead
> of the MST hub. If a leaf monitor doesn't support features like enhanced
> framing or downspread, but the hub does, could this break the link to the hub?
>
> --
> Sashiko AI review ยท
> https://sashiko.dev/#/patchset/[email protected]?part=10
--
With best wishes
Dmitry
