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

[Yongxing Mou <yongxing.mou@oss.qualcomm.com>]

On 6/16/2026 8:44 AM, Dmitry Baryshkov wrote:
> 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@quicinc.com>
>>>>
>>>> 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@quicinc.com>
>>>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>>> ---
>>>>    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().
> 
Got it.
>>>> -	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.
> 
This function is only used in the SST path, which is why ctrl->panel is 
passed in.
The only reason for the change here is the interface update of 
msm_dp_ctrl_push_idle().
If needed, we could keep msm_dp_ctrl_push_idle() for SST and introduce a 
new msm_dp_ctrl_push_vcpf() for MST. Since the SST behavior hasn’t 
changed, no modification is needed here.
>>>>    	ctrl->link->phy_params.p_level = 0;
>>>>    	ctrl->link->phy_params.v_level = 0;
>