Re: [PATCH v6 12/29] drm/msm/dp: add support for sending VCPF packets in DP controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Deterministic timeout during VCPF push for MST streams 2 and 3 due to unhandled interrupts.

Pre-existing issues:
- [High] `msm_dp_ctrl_push_vcpf` calls a sleeping function (`wait_for_completion_timeout`) from the atomic disable path.
--

commit 507d49b6852332d2246a4d09ccd9c56c895bd874
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>

drm/msm/dp: add support for sending VCPF packets in DP controller

The VC Payload Fill (VCPF) sequence is inserted by the DP controller
when stream symbols are absent, typically before a stream is disabled.
Add support for triggering the VCPF sequence in the MSM DP controller.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index 9a0c9737e6d00..109a841675997 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c

[ ... ]

> @@ -478,6 +500,34 @@ void msm_dp_ctrl_push_idle(struct msm_dp_ctrl *msm_dp_ctrl)
>  	drm_dbg_dp(ctrl->drm_dev, "mainlink off\n");
>  }
>  
> +/* Must be called with msm_dp_mst::mst_lock held */
> +void msm_dp_ctrl_push_vcpf(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_STATE_CTRL_DP0_PUSH_VCPF;
> +	else if (msm_dp_panel->stream_id == DP_STREAM_1)
> +		state |= DP_STATE_CTRL_DP1_PUSH_VCPF;
> +	else
> +		state |= DP_MSTLINK_PUSH_VCPF;
> +
> +	reinit_completion(&ctrl->idle_comp);
> +
> +	msm_dp_write_stream_link(ctrl, msm_dp_panel->stream_id, REG_DP_STATE_CTRL, state);
> +
> +	if (!wait_for_completion_timeout(&ctrl->idle_comp,
> +			IDLE_PATTERN_COMPLETION_TIMEOUT_JIFFIES))
> +		pr_warn("PUSH_VCPF pattern timedout\n");

[Severity: High]
This is a pre-existing issue, but wait_for_completion_timeout() is a
sleeping function, and it is being invoked from an atomic context.

The DRM atomic commit tail framework calls the bridge's atomic_disable
callback (msm_dp_display_atomic_disable()), which directly calls this
function. Since the atomic_disable callback is an atomic context path
where sleeping is strictly forbidden, could this cause kernel warnings
or potential deadlocks during display teardown?

[Severity: Medium]
Will this wait result in a deterministic timeout for MST streams 2 and 3?

When the stream is assigned to DP_STREAM_2 or DP_STREAM_3, the hardware is
triggered to send VCPF via DP_MSTLINK_PUSH_VCPF, and the code blocks here.

However, looking at the interrupt handler in this patch:

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

The ISR only checks for stream 0 and 1 completion events. DP2 and DP3
are ignored. Since the interrupt for DP2/DP3 is neither unmasked in
DP_INTERRUPT_STATUS5_MASK nor handled in msm_dp_ctrl_isr(), the
completion won't happen.

Does this cause an unnecessary 30ms delay and a timeout warning every time
a secondary MST stream is disabled?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-msm-dp-mst-v6-0-c91d35d6fb9e@oss.qualcomm.com?part=12