Re: [PATCH] drm/i915/display: Replace bw_constrained with max_link_bpp_x16

[sashiko-bot@kernel.org]

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 <taterraster@gmail.com>

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/20260919161927.98829-1-mail@taterr.org?part=1