From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Revert "drm/msm/dpu: try reserving the DSPP-less LM first" Date: Sat, 14 Feb 2026 13:59:52 +1000 Message-ID: In-Reply-To: <20260214-revert-dspp-less-v1-1-be0d636a2a6e@oss.qualcomm.com> References: <20260214-revert-dspp-less-v1-1-be0d636a2a6e@oss.qualcomm.com> <20260214-revert-dspp-less-v1-1-be0d636a2a6e@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review The commit message is clear about the motivation: blue screens on X1E80100 la= ptops, with a proper Fixes tag and Reported-by/Closes links. The revert collapses the two-function design (`dpu_rm_find_lms` + `_dpu_rm_re= serve_lms`) back into a single `_dpu_rm_reserve_lms` function. Looking at the= diff: > -static bool dpu_rm_find_lms(struct dpu_rm *rm, > - struct dpu_global_state *global_state, > - uint32_t crtc_id, bool skip_dspp, > - struct msm_display_topology *topology, > - int *lm_idx, int *pp_idx, int *dspp_idx) > +static int _dpu_rm_reserve_lms(struct dpu_rm *rm, > + struct dpu_global_state *global_state, > + uint32_t crtc_id, > + struct msm_display_topology *topology) The separate `dpu_rm_find_lms` function is removed and its loop body is folde= d back into `_dpu_rm_reserve_lms`, with the `skip_dspp` filtering removed ent= irely: > - if (skip_dspp && to_dpu_hw_mixer(rm->mixer_blks[i])->cap->dspp) { > - DPU_DEBUG("Skipping LM_%d, skipping LMs with DSPPs\n", i); > - continue; > - } This was the core of the optimization -- skipping DSPP-capable LMs when the t= opology doesn't need DSPPs. Removing this restores the original behavior of a= llocating LMs in their natural order. > - /* Try using non-DSPP LM blocks first */ > - found =3D dpu_rm_find_lms(rm, global_state, crtc_id, !topology->num_dspp, > - topology, lm_idx, pp_idx, dspp_idx); > - if (!found && !topology->num_dspp) > - found =3D dpu_rm_find_lms(rm, global_state, crtc_id, false, > - topology, lm_idx, pp_idx, dspp_idx); The two-pass fallback strategy (try without DSPP-capable LMs first, then fall= back to any LM) is eliminated. > - for (i =3D 0; i < topology->num_lm; i++) { > + for (i =3D 0; i < lm_count; i++) { This change in the reservation loop from `topology->num_lm` to `lm_count` is = equivalent since the code reaches this point only when `lm_count =3D=3D topol= ogy->num_lm` (verified by the check just above). No functional difference. One minor note on the commit message: "It seems on later DPUs using higher LM= s require some additional setup or conflicts with the hardware defaults" -- t= his is slightly awkward grammar ("using higher LMs require" could be "using h= igher LMs requires"), but it's understandable and this is a cosmetic observat= ion. No correctness issues found. The revert is a clean, mechanical undo of the or= iginal optimization and correctly restores the previous LM allocation behavio= r. --- Generated by Claude Code Patch Reviewer