Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/msm/dpu: incomplete blendstage reset in dpu_encoder_helper_reset_mixers() --
commit d3a43e897b19cb72f6d34115067a2c68f2bef217 Author: Xi Zhang <[email protected]> drm/msm/dpu: save LMs in encoder mode_set for cleanup use later This commit caches layer mixers in dpu_encoder_virt during atomic_mode_set so that dpu_encoder_helper_reset_mixers() cleans up the previously assigned mixers rather than querying the post-swap global state. It also removes the ctl->ops.setup_blendstage() call from dpu_encoder_helper_reset_mixers(). > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 8d2b4d2726f27..09afc0681248f 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c [ ... ] > @@ -2183,42 +2194,34 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc) > > static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys > *phys_enc) > { > - int i, num_lm; > - struct dpu_global_state *global_state; > - struct dpu_hw_blk *hw_lm[2]; > - struct dpu_hw_mixer *hw_mixer[2]; > + int i; > struct dpu_hw_ctl *ctl = phys_enc->hw_ctl; > + struct dpu_encoder_virt *dpu_enc = > to_dpu_encoder_virt(phys_enc->parent); > > /* reset all mixers for this encoder */ > if (ctl->ops.clear_all_blendstages) > ctl->ops.clear_all_blendstages(ctl); > > - global_state = dpu_kms_get_existing_global_state(phys_enc->dpu_kms); > + for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_mixer); i++) { > + if (dpu_enc->hw_mixer[i]) { > + if (ctl->ops.update_pending_flush_mixer) > + ctl->ops.update_pending_flush_mixer(ctl, > dpu_enc->hw_mixer[i]->idx); > > - num_lm = dpu_rm_get_assigned_resources(&phys_enc->dpu_kms->rm, > global_state, > - phys_enc->parent->crtc, DPU_HW_BLK_LM, hw_lm, > ARRAY_SIZE(hw_lm)); > - > - for (i = 0; i < num_lm; i++) { > - hw_mixer[i] = to_dpu_hw_mixer(hw_lm[i]); > - if (ctl->ops.update_pending_flush_mixer) > - ctl->ops.update_pending_flush_mixer(ctl, > hw_mixer[i]->idx); > - > - /* clear all blendstages */ > - if (ctl->ops.setup_blendstage) > - ctl->ops.setup_blendstage(ctl, hw_mixer[i]->idx, NULL); > - > - if (hw_mixer[i]->ops.clear_all_blendstages) > - hw_mixer[i]->ops.clear_all_blendstages(hw_mixer[i]); > + /* clear all blendstages */ > + if (dpu_enc->hw_mixer[i]->ops.clear_all_blendstages) > + dpu_enc->hw_mixer[i]->ops.clear_all_blendstages( > + dpu_enc->hw_mixer[i]); > + } > + } [Severity: Medium] Does ctl->ops.clear_all_blendstages() completely cover what ctl->ops.setup_blendstage(ctl, hw_mixer[i]->idx, NULL) was doing here? When dpu_encoder_virt_atomic_disable() -> dpu_encoder_helper_phys_cleanup() -> dpu_encoder_helper_reset_mixers() runs on DPU hardware with core_major_ver < 12, ctl->ops.clear_all_blendstages invokes dpu_hw_ctl_clear_all_blendstages() in drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c: static void dpu_hw_ctl_clear_all_blendstages(struct dpu_hw_ctl *ctx) { struct dpu_hw_blk_reg_map *c = &ctx->hw; int i; for (i = 0; i < ctx->mixer_count; i++) { enum dpu_lm mixer_id = ctx->mixer_hw_caps[i].id; DPU_REG_WRITE(c, CTL_LAYER(mixer_id), 0); DPU_REG_WRITE(c, CTL_LAYER_EXT(mixer_id), 0); DPU_REG_WRITE(c, CTL_LAYER_EXT2(mixer_id), 0); DPU_REG_WRITE(c, CTL_LAYER_EXT3(mixer_id), 0); } DPU_REG_WRITE(c, CTL_FETCH_PIPE_ACTIVE, 0); } By contrast, dpu_hw_ctl_setup_blendstage() with stage_cfg == NULL did: mixercfg[0] = CTL_MIXER_BORDER_OUT; /* always set BORDER_OUT */ if (!stage_cfg) goto exit; ... exit: DPU_REG_WRITE(c, CTL_LAYER(lm), mixercfg[0]); DPU_REG_WRITE(c, CTL_LAYER_EXT(lm), mixercfg[1]); DPU_REG_WRITE(c, CTL_LAYER_EXT2(lm), mixercfg[2]); DPU_REG_WRITE(c, CTL_LAYER_EXT3(lm), mixercfg[3]); if (ctx->mdss_ver->core_major_ver >= 9) DPU_REG_WRITE(c, CTL_LAYER_EXT4(lm), mixercfg[4]); Without ctl->ops.setup_blendstage(), on DPU v9.x through v11.x platforms CTL_LAYER_EXT4(lm) is never cleared during mixer reset (leaving any staged SSPP_DMA4 or SSPP_DMA5 bits set), and on DPU < v5 (where ctl->ops.reset_intf_cfg is not implemented) CTL_LAYER(lm) is left as 0 instead of having CTL_MIXER_BORDER_OUT set. Should dpu_hw_ctl_clear_all_blendstages() be updated to also clear CTL_LAYER_EXT4(mixer_id) on core_major_ver >= 9 (and preserve CTL_MIXER_BORDER_OUT if needed), or should the setup_blendstage() call be kept? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
