Re: [PATCH v4 3/6] drm/msm/dpu: check mode against PINGPONG or DSC max width

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm/dpu: DSC max width limits are bypassed and threshold logic is dead code
--

commit f43daa2877be8ec1ed9bc6b650068210b82d6ffb
Author: Jessica Zhang <jesszhan0024@gmail.com>

drm/msm/dpu: check mode against PINGPONG or DSC max width

This commit replaces the hardcoded max_mixer_width and MAX_HDISPLAY_SPLIT
limits with dynamically determined pingpong or DSC buffer widths. It aims to
support higher refresh rate modes by passing the derived split threshold to
the layer-mixer count logic.

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 209b25dbeb6c8..41f986ca2a683 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -765,13 +781,14 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc,
>  	struct drm_display_mode *adj_mode = &state->adjusted_mode;
>  	u32 crtc_split_width = adj_mode->hdisplay / cstate->num_mixers;
>  	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> +	int max_pingpong_width = msm_display_get_max_pingpong_width(dpu_kms);
>  	int i;
>  
>  	/* if we cannot merge 2 LMs (no 3d mux) better to fail earlier
>  	 * before even checking the width after the split
>  	 */
>  	if (!dpu_kms->catalog->caps->has_3d_merge &&
> -	    adj_mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
> +	    adj_mode->hdisplay > max_pingpong_width)
>  		return -E2BIG;
>  
>  	for (i = 0; i < cstate->num_mixers; i++) {
> @@ -783,7 +800,7 @@ static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc,
>  
>  		trace_dpu_crtc_setup_lm_bounds(DRMID(crtc), i, r);
>  
> -		if (drm_rect_width(r) > dpu_kms->catalog->caps->max_mixer_width)
> +		if (drm_rect_width(r) > max_pingpong_width)
>  			return -E2BIG;
>  	}

[Severity: High]
Does this inadvertently allow DSC modes to exceed their narrower hardware
limits?

Because the bounds checks in _dpu_crtc_check_and_setup_lm_bounds() no longer
consider max_dsc_encoder_width, a DSC mode with a split width exceeding the
DSC hardware limit (e.g., width 3000 on a single DSC) will incorrectly pass
validation against the wider max_pingpong_width instead of returning -E2BIG.

This could lead to hardware configuration failures when programmed. None of
the follow-up patches in the series appear to resolve the bypassed DSC bounds.

[ ... ]

> @@ -1410,6 +1428,19 @@ static struct msm_display_topology dpu_crtc_get_topology(
>  
>  	topology.cwb_enabled = drm_crtc_in_clone_mode(crtc_state);
>  
> +	max_hdisplay_split = msm_display_get_max_pingpong_width(dpu_kms);
> +
> +	if (topology.num_dsc > 0) {
> +		u32 max_dsc_encoder_width;
> +
> +		if (dpu_kms->catalog->mdss_ver->core_major_ver < 6)
> +			max_dsc_encoder_width = DPU_1_x_MAX_DSC_ENCODER_WIDTH;
> +		else
> +			max_dsc_encoder_width = DPU_8_x_MAX_DSC_ENCODER_WIDTH;
> +
> +		max_hdisplay_split = min(max_hdisplay_split, max_dsc_encoder_width);
> +	}
> +
>  	/*
>  	 * Datapath topology selection
>  	 *
> @@ -1438,7 +1469,8 @@ static struct msm_display_topology dpu_crtc_get_topology(
>  		topology.num_lm = 2;
>  	else if (dpu_kms->catalog->caps->has_3d_merge &&
>  		 topology.num_dsc == 0)
> -		topology.num_lm = dpu_crtc_num_lm_for_mode(dpu_kms, mode);
> +		topology.num_lm = dpu_crtc_num_lm_for_mode(dpu_kms, mode,
> +							   max_hdisplay_split);
>  	else
>  		topology.num_lm = 1;

[Severity: High]
Is the max_hdisplay_split threshold logic for DSC topologies dead code here?

In dpu_crtc_get_topology(), max_hdisplay_split is clamped to
max_dsc_encoder_width when topology.num_dsc > 0. However, this updated
variable is exclusively passed to dpu_crtc_num_lm_for_mode() which is only
called in the branch where topology.num_dsc == 0.

As a result, the computed max_hdisplay_split for DSC topologies is entirely
dead code and never actually utilized.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-lm_fixes_final-v4-0-fa986071c3c1@oss.qualcomm.com?part=3