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 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(-)
>>
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> index e64f81bc8c36..9907f2e56e65 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
>> @@ -65,9 +65,18 @@
>>   	(PSR_UPDATE_MASK | PSR_CAPTURE_MASK | PSR_EXIT_MASK | \
>>   	PSR_UPDATE_ERROR_MASK | PSR_WAKE_ERROR_MASK)
>>   
>> +#define DP_INTERRUPT_STATUS5 \
>> +	(DP_INTR_DP0_VCPF_SENT | DP_INTR_DP1_VCPF_SENT)
>> +#define DP_INTERRUPT_STATUS5_MASK \
>> +	(DP_INTERRUPT_STATUS5 << DP_INTERRUPT_STATUS_MASK_SHIFT)
>> +
>>   #define DP_CTRL_INTR_READY_FOR_VIDEO     BIT(0)
>>   #define DP_CTRL_INTR_IDLE_PATTERN_SENT  BIT(3)
>>   
>> +#define DP_DP0_PUSH_VCPF		BIT(12)
>> +#define DP_DP1_PUSH_VCPF		BIT(14)
>> +#define DP_MSTLINK_PUSH_VCPF		BIT(12)
> 
> dp_reg.h, under corresponding registers.
> 
Got it.
>> +
>>   #define MR_LINK_TRAINING1  0x8
>>   #define MR_LINK_SYMBOL_ERM 0x80
>>   #define MR_LINK_PRBS7 0x100
>> @@ -405,6 +414,8 @@ void msm_dp_ctrl_enable_irq(struct msm_dp_ctrl *msm_dp_ctrl)
>>   			DP_INTERRUPT_STATUS1_MASK);
>>   	msm_dp_write_ahb(ctrl, REG_DP_INTR_STATUS2,
>>   			DP_INTERRUPT_STATUS2_MASK);
>> +	msm_dp_write_ahb(ctrl, REG_DP_INTR_STATUS5,
>> +			 DP_INTERRUPT_STATUS5_MASK);
>>   }
>>   
>>   void msm_dp_ctrl_disable_irq(struct msm_dp_ctrl *msm_dp_ctrl)
>> @@ -414,6 +425,7 @@ void msm_dp_ctrl_disable_irq(struct msm_dp_ctrl *msm_dp_ctrl)
>>   
>>   	msm_dp_write_ahb(ctrl, REG_DP_INTR_STATUS, 0x00);
>>   	msm_dp_write_ahb(ctrl, REG_DP_INTR_STATUS2, 0x00);
>> +	msm_dp_write_ahb(ctrl, REG_DP_INTR_STATUS5, 0x00);
>>   }
>>   
>>   static u32 msm_dp_ctrl_get_psr_interrupt(struct msm_dp_ctrl_private *ctrl)
>> @@ -433,6 +445,20 @@ static void msm_dp_ctrl_config_psr_interrupt(struct msm_dp_ctrl_private *ctrl)
>>   	msm_dp_write_ahb(ctrl, REG_DP_INTR_MASK4, DP_INTERRUPT_MASK4);
>>   }
>>   
>> +static u32 msm_dp_ctrl_get_mst_interrupt(struct msm_dp_ctrl_private *ctrl)
>> +{
>> +	u32 intr, intr_ack;
>> +
>> +	intr = msm_dp_read_ahb(ctrl, REG_DP_INTR_STATUS5);
>> +	intr &= ~DP_INTERRUPT_STATUS5_MASK;
>> +	intr_ack = (intr & DP_INTERRUPT_STATUS5)
>> +			<< DP_INTERRUPT_STATUS_ACK_SHIFT;
>> +	msm_dp_write_ahb(ctrl, REG_DP_INTR_STATUS5,
>> +			 intr_ack | DP_INTERRUPT_STATUS5_MASK);
>> +
>> +	return intr;
>> +}
>> +
>>   static void msm_dp_ctrl_psr_mainlink_enable(struct msm_dp_ctrl_private *ctrl)
>>   {
>>   	u32 val;
>> @@ -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.
>> -	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))
>> @@ -2073,7 +2113,7 @@ void msm_dp_ctrl_set_psr(struct msm_dp_ctrl *msm_dp_ctrl, bool enter)
>>   			return;
>>   		}
>>   
>> -		msm_dp_ctrl_push_idle(msm_dp_ctrl);
>> +		msm_dp_ctrl_push_idle(msm_dp_ctrl, ctrl->panel);
>>   		msm_dp_write_link(ctrl, 0, REG_DP_STATE_CTRL, 0);
>>   
>>   		msm_dp_ctrl_psr_mainlink_disable(ctrl);
>> @@ -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?
>>   
>>   	ctrl->link->phy_params.p_level = 0;
>>   	ctrl->link->phy_params.v_level = 0;
>> @@ -3005,6 +3045,13 @@ irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl)
>>   		ret = IRQ_HANDLED;
>>   	}
>>   
>> +	isr = msm_dp_ctrl_get_mst_interrupt(ctrl);
>> +	if (isr & (DP_INTR_DP0_VCPF_SENT | DP_INTR_DP1_VCPF_SENT)) {
>> +		drm_dbg_dp(ctrl->drm_dev, "vcpf sent\n");
>> +		complete(&ctrl->idle_comp);
>> +		ret = IRQ_HANDLED;
>> +	}
>> +
>>   	/* DP aux isr */
>>   	isr = msm_dp_ctrl_get_aux_interrupt(ctrl);
>>   	if (isr)
>> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
>> index c59338199399..cfe7e4496943 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
>> @@ -22,7 +22,7 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl,
>>   int msm_dp_ctrl_prepare_stream_on(struct msm_dp_ctrl *msm_dp_ctrl, bool force_link_train);
>>   void msm_dp_ctrl_off_link(struct msm_dp_ctrl *msm_dp_ctrl);
>>   void msm_dp_ctrl_off_pixel_clk(struct msm_dp_ctrl *msm_dp_ctrl, enum msm_dp_stream_id stream_id);
>> -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);
>>   irqreturn_t msm_dp_ctrl_isr(struct msm_dp_ctrl *msm_dp_ctrl);
>>   void msm_dp_ctrl_handle_sink_request(struct msm_dp_ctrl *msm_dp_ctrl);
>>   struct msm_dp_ctrl *msm_dp_ctrl_get(struct device *dev,
>> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
>> index e0bf4dffa6af..e8028402f748 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> @@ -1557,7 +1557,7 @@ void msm_dp_display_atomic_disable(struct msm_dp *msm_dp_display)
>>   
>>   	dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
>>   
>> -	msm_dp_ctrl_push_idle(dp->ctrl);
>> +	msm_dp_ctrl_push_idle(dp->ctrl, dp->panel);
>>   	msm_dp_ctrl_mst_stream_channel_slot_setup(dp->ctrl);
>>   	msm_dp_ctrl_mst_send_act(dp->ctrl);
>>   }
>> diff --git a/drivers/gpu/drm/msm/dp/dp_reg.h b/drivers/gpu/drm/msm/dp/dp_reg.h
>> index 835a55446868..65695fcb48d0 100644
>> --- a/drivers/gpu/drm/msm/dp/dp_reg.h
>> +++ b/drivers/gpu/drm/msm/dp/dp_reg.h
>> @@ -42,9 +42,13 @@
>>   #define DP_INTR_FRAME_END		BIT(6)
>>   #define DP_INTR_CRC_UPDATED		BIT(9)
>>   
>> +#define DP_INTR_DP0_VCPF_SENT		BIT(0)
>> +#define DP_INTR_DP1_VCPF_SENT		BIT(3)
>> +
>>   #define REG_DP_INTR_STATUS3			(0x00000028)
>>   
>>   #define REG_DP_INTR_STATUS4			(0x0000002C)
>> +#define REG_DP_INTR_STATUS5			(0x00000034)
>>   #define PSR_UPDATE_INT				(0x00000001)
>>   #define PSR_CAPTURE_INT				(0x00000004)
>>   #define PSR_EXIT_INT				(0x00000010)
>> @@ -356,6 +360,7 @@
>>   #define REG_DP_DP0_RG				(0x000004F8)
>>   #define REG_DP_DP1_RG				(0x000004FC)
>>   
>> +#define REG_DP_MSTLINK_STATE_CTRL		(0x00000000)
>>   #define REG_DP_MSTLINK_CONFIGURATION_CTRL	(0x00000034)
>>   #define REG_DP_MSTLINK_TIMESLOT_1_32		(0x00000038)
>>   #define REG_DP_MSTLINK_TIMESLOT_33_63		(0x0000003C)
>>
>> -- 
>> 2.43.0
>>
>