Re: [PATCH v2 4/6] drm/msm/dpu: clear the DSPP pointer when no DSPP is assigned

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Stale pointers for hardware blocks (hw_dsc, hw_cwb) in persistent encoder state are not cleared when resource counts shrink, leading to incorrect hardware configuration and kernel panics.
--

commit 6954c50ce10cab5ddfb3ee7cbf148810fc5b6d78
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/msm/dpu: clear the DSPP pointer when no DSPP is assigned

This commit fixes a bug where stale DSPP pointers are retained in the CRTC
state across modesets. It assigns NULL to the DSPP pointer when there is no
DSPP for the mixer.

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index bf593020e8e45..5dc0c9aeaa5d7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1480,8 +1480,8 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc,
>  
>  		cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
>  		cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
> -		if (i < num_dspp)
> -			cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
> +		cstate->mixers[i].hw_dspp = i < num_dspp ?
> +			to_dpu_hw_dspp(hw_dspp[i]) : NULL;
>  	}

[Severity: High]
This is a pre-existing issue, but does this same stale pointer bug also
happen with hw_cwb and hw_dsc in dpu_encoder_virt_atomic_mode_set()?

Looking at drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c, if a modeset drops
CWB or DSC features, the loops only iterate up to the new smaller num_cwb
or num_dsc:

	for (i = 0; i < num_cwb; i++) {
		dpu_enc->hw_cwb[i] = to_dpu_hw_cwb(hw_cwb[i]);
		cwb_mask |= BIT(dpu_enc->hw_cwb[i]->idx - CWB_0);
	}

Since trailing elements aren't cleared, dpu_enc->hw_cwb[i] and
dpu_enc->hw_dsc[i] retain stale pointers.

When dpu_encoder_helper_phys_setup_cwb() later iterates over
MAX_CWB_PER_ENC, it finds the stale pointer and attempts to read from
the uninitialized stack array rt_pp_list[i]:

	for (int i = 0; i < MAX_CWB_PER_ENC; i++) {
		hw_cwb = dpu_enc->hw_cwb[i];
		if (!hw_cwb)
			continue;

		if (enable) {
			struct dpu_hw_pingpong *hw_pp =
					to_dpu_hw_pingpong(rt_pp_list[i]);
			cwb_cfg.pp_idx = hw_pp->idx;
		...

This dereferences hw_pp->idx from stack garbage, causing a kernel panic.

Similarly, in dpu_encoder_use_dsc_merge(), stale hw_dsc pointers artificially
inflate num_dsc, which breaks topology decisions like 3D blend mode
selection:

	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
		if (dpu_enc->hw_dsc[i])
			num_dsc++;

Should similar clearing logic be added to dpu_encoder_virt_atomic_mode_set()
to clear trailing hw_cwb and hw_dsc pointers?

>  
>  	cstate->num_mixers = num_lm;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-fd-kms-fix-smmu-v2-0-9391815742a8@oss.qualcomm.com?part=4