Re: [PATCH v4 22/39] drm/msm/dp: Add support for sending VCPF packets in DP controller

[Dmitry Baryshkov]

On Mon, Jun 15, 2026 at 04:35:06PM +0800, Yongxing Mou wrote:
> 
> 
> On 4/12/2026 3:24 AM, Dmitry Baryshkov wrote:
> > On Fri, Apr 10, 2026 at 05:33:57PM +0800, Yongxing Mou wrote:
> > > From: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > 
> > > The VC Payload Fill (VCPF) sequence is inserted by the DP controller
> > > when stream symbols are absent, typically before a stream is disabled.
> > > This patch adds support for triggering the VCPF sequence in the MSM DP
> > > controller.
> > > 
> > > Signed-off-by: Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>
> > > Signed-off-by: Yongxing Mou <yongxing.mou@xxxxxxxxxxxxxxxx>
> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > > ---
> > >   drivers/gpu/drm/msm/dp/dp_ctrl.c    | 55 ++++++++++++++++++++++++++++++++++---
> > >   drivers/gpu/drm/msm/dp/dp_ctrl.h    |  2 +-
> > >   drivers/gpu/drm/msm/dp/dp_display.c |  2 +-
> > >   drivers/gpu/drm/msm/dp/dp_reg.h     |  5 ++++
> > >   4 files changed, 58 insertions(+), 6 deletions(-)
> > > 
> > > @@ -516,14 +542,28 @@ static bool msm_dp_ctrl_mainlink_ready(struct msm_dp_ctrl_private *ctrl)
> > >   	return true;
> > >   }
> > > -void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl)
> > > +void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *msm_dp_panel)
> > >   {
> > >   	struct msm_dp_ctrl_private *ctrl;
> > > +	u32 state = 0x0;
> > >   	ctrl = container_of(msm_dp_ctrl, struct msm_dp_ctrl_private, msm_dp_ctrl);
> > > +	if (!ctrl->mst_active)
> > > +		state |= DP_STATE_CTRL_PUSH_IDLE;
> > > +	else if (msm_dp_panel->stream_id == DP_STREAM_0)
> > > +		state |= DP_DP0_PUSH_VCPF;
> > > +	else if (msm_dp_panel->stream_id == DP_STREAM_1)
> > > +		state |= DP_DP1_PUSH_VCPF;
> > > +	else
> > > +		state |= DP_MSTLINK_PUSH_VCPF;
> > > +
> > >   	reinit_completion(&ctrl->idle_comp);
> > 
> > And there can't be two streams wanting to push idle at the same time?
> > 
> In MST, msm_dp_ctrl_push_idle() is only reached from
> msm_dp_display_disable_helper(), which is called from
> msm_dp_mst_stream_disable() / msm_dp_mst_stream_post_disable() in
> dp_mst_drm.c. Both of those
>    callers hold mst->mst_lock for the duration of the disable sequence,
> which serializes push_idle (and the wait on idle_comp) across streams. So
> sharing a single idle_comp on the ctrl is safe.

Comment, protected by mst_lock in msm_dp_foo().

> > > -	msm_dp_write_link(ctrl, 0, REG_DP_STATE_CTRL, DP_STATE_CTRL_PUSH_IDLE);
> > > +
> > > +	msm_dp_write_link(ctrl, msm_dp_panel->stream_id,
> > > +			  msm_dp_panel->stream_id > 1 ?
> > > +			  REG_DP_MSTLINK_STATE_CTRL : REG_DP_STATE_CTRL,
> > > +			  state);
> > >   	if (!wait_for_completion_timeout(&ctrl->idle_comp,
> > >   			IDLE_PATTERN_COMPLETION_TIMEOUT_JIFFIES))

> > > @@ -2183,7 +2223,7 @@ static int msm_dp_ctrl_link_maintenance(struct msm_dp_ctrl_private *ctrl)
> > >   	int ret = 0;
> > >   	int training_step = DP_TRAINING_NONE;
> > > -	msm_dp_ctrl_push_idle(&ctrl->msm_dp_ctrl);
> > > +	msm_dp_ctrl_push_idle(&ctrl->msm_dp_ctrl, ctrl->panel);
> > 
> > Which panel are we passing and why? It feels to me that we have two
> > different cases, one for the MST stream and another one for the SST
> > link. Can we handle them separately? (note: I might be wrong here,
> > please correct me if I'm wrong).
> > 
> For SST, we push to bit 8 of MDSS_0_DPTX_0_STATE_CTRL.
> For MST, stream0 and stream1 use bit 12 and bit 14 respectively.
> For MST stream2 and stream3 use REG_DP_MSTLINK_STATE_CTRL.
> Do we need to handle MST and SST separately here?

Where is the MST panel coming from? You are pushing ctrl->panel, which
is the SST one, if I'm not mistaken.

> > >   	ctrl->link->phy_params.p_level = 0;
> > >   	ctrl->link->phy_params.v_level = 0;

-- 
With best wishes
Dmitry