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