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

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

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.

> +
>  #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? 

> -	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).

>  
>  	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
> 

-- 
With best wishes
Dmitry