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 <[email protected]> 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/[email protected]?part=4
