Re: [PATCH v6 10/29] drm/msm/dp: no need to update tu calculation for mst

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] If memory allocation fails during TU calculation, the function returns early without initializing its output parameters, leading to uninitialized stack memory being written to hardware registers.
- [High] Calling sleeping and blocking functions from an atomic context causes system instability and deadlocks.
--

commit 17eb62d850306afdfafe9f15909840906306ec84
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>

drm/msm/dp: no need to update tu calculation for mst

This commit skips the calculation and programming of TU parameters for MST
cases, as the DP stream is transmitted in transfer units only for SST.

> diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> index cd6ae0d5372c9..77506e8408832 100644
> --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
> +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
> @@ -2703,7 +2703,8 @@ int msm_dp_ctrl_on_stream(struct msm_dp_ctrl *msm_dp_ctrl, struct msm_dp_panel *
>  
>  	msm_dp_panel_clear_dsc_dto(panel);
>  
> -	msm_dp_ctrl_setup_tr_unit(ctrl, panel);
> +	if (!ctrl->mst_active)
> +		msm_dp_ctrl_setup_tr_unit(ctrl, panel);

[Severity: High]
This isn't a bug introduced by this patch, but can we write uninitialized
stack memory to MMIO link registers if memory allocation fails?

Looking at msm_dp_ctrl_setup_tr_unit() in drivers/gpu/drm/msm/dp/dp_ctrl.c,
it allocates tu_calc_table on the stack and passes it down for initialization:

drivers/gpu/drm/msm/dp/dp_ctrl.c:msm_dp_ctrl_setup_tr_unit() {
	struct msm_dp_vc_tu_mapping_table tu_calc_table;

	msm_dp_ctrl_calc_tu_parameters(ctrl, panel, &tu_calc_table);

	msm_dp_tu |= tu_calc_table.tu_size_minus1;
	...
}

However, if _dp_ctrl_calc_tu() fails to allocate memory, it returns early
without initializing its output parameter:

drivers/gpu/drm/msm/dp/dp_ctrl.c:_dp_ctrl_calc_tu() {
	...
	tu = kzalloc_obj(*tu);
	if (!tu)
		return;
	...
}

Will this cause msm_dp_ctrl_setup_tr_unit() to use the uninitialized
tu_calc_table stack memory and program it into hardware registers like
REG_DP_VALID_BOUNDARY and REG_DP_TU, potentially leading to display
corruption or kernel panics?

[Severity: High]
This is a pre-existing issue, but does msm_dp_ctrl_on_stream() violate
atomic context constraints by calling sleeping or blocking functions?

The DRM guidelines state that encoder/bridge atomic enable callbacks, such as
msm_dp_display_atomic_enable(), execute in a non-sleeping atomic context.

However, msm_dp_display_atomic_enable() calls msm_dp_ctrl_on_stream(), which
appears to trigger several functions that can sleep or block:

- msm_dp_ctrl_mst_send_act() in drivers/gpu/drm/msm/dp/dp_ctrl.c directly
sleeps:

  msleep(frame_time_ms);

- msm_dp_ctrl_wait4video_ready() uses a blocking completion wait:

  if (!wait_for_completion_timeout(&ctrl->video_comp,
                                   WAIT_FOR_VIDEO_READY_TIMEOUT_JIFFIES)) {

- msm_dp_ctrl_setup_tr_unit() triggers a memory allocation that can sleep to
reclaim memory:

  tu = kzalloc_obj(*tu);

Could these calls cause system deadlocks or lockups when the stream is
activated during atomic commits?

>  
>  	msm_dp_write_link(ctrl, REG_DP_STATE_CTRL, DP_STATE_CTRL_SEND_VIDEO);
>

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