Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] drm/i915/display: incomplete cleanup of bw_constrained flag --
commit f4c7087cb31d33cb453437886f7788a5ffe134a2 Author: George Karagounis <[email protected]> drm/i915/display: Replace bw_constrained with max_link_bpp_x16 This commit replaces the legacy bw_constrained boolean flag checks with direct comparisons against the max_link_bpp_x16 fixed-point API in both intel_crt.c and intel_lvds.c. > diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c > b/drivers/gpu/drm/i915/display/intel_lvds.c > index 872753478cf21..6bbd13ab5da7b 100644 > --- a/drivers/gpu/drm/i915/display/intel_lvds.c > +++ b/drivers/gpu/drm/i915/display/intel_lvds.c [ ... ] > @@ -443,10 +443,10 @@ static int intel_lvds_compute_config(struct > intel_atomic_state *state, > else > lvds_bpp = 6*3; > > - /* TODO: Check crtc_state->max_link_bpp_x16 instead of bw_constrained */ > - if (lvds_bpp != crtc_state->pipe_bpp && !crtc_state->bw_constrained) { > + if (lvds_bpp != crtc_state->pipe_bpp && > + crtc_state->max_link_bpp_x16 >= fxp_q4_from_int(lvds_bpp)) { [Severity: Low] Does this commit intentionally leave the bw_constrained flag definition and other writes intact? While this patch removes the readers of bw_constrained, the flag remains defined in struct intel_crtc_state: drivers/gpu/drm/i915/display/intel_display_types.h:struct intel_crtc_state { ... bool bw_constrained; ... } It also still appears to be stored whenever crtc_state->pipe_bpp > fxp_q4_to_int(crtc_state->max_link_bpp_x16) during mode setting: drivers/gpu/drm/i915/display/intel_display.c:intel_modeset_pipe_config() { ... crtc_state->bw_constrained = true; ... } Because the readers were removed, this causes unnecessary code complexity and leaves behind a dead store. Is there a plan to remove the flag entirely, or was this an oversight? > drm_dbg_kms(display->drm, > - "forcing display bpp (was %d) to LVDS (%d)\n", > + "forcing display bpp (was %d) to LVDS (%d)\n", > crtc_state->pipe_bpp, lvds_bpp); > crtc_state->pipe_bpp = lvds_bpp; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
