* Re: [PATCH] drm/i915/display: Replace bw_constrained with max_link_bpp_x16
2026-09-19 16:19 [PATCH] drm/i915/display: Replace bw_constrained with max_link_bpp_x16 George Karagounis
@ 2026-09-21 7:10 ` sashiko-bot
2026-09-21 16:48 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-09-21 7:10 UTC (permalink / raw)
To: George Karagounis; +Cc: intel-gfx, dri-devel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH v2] drm/i915/display: Replace bw_constrained with max_link_bpp_x16
2026-09-19 16:19 [PATCH] drm/i915/display: Replace bw_constrained with max_link_bpp_x16 George Karagounis
2026-09-21 7:10 ` sashiko-bot
2026-09-21 16:48 ` ✗ LGCI.VerificationFailed: failure for " Patchwork
@ 2026-09-24 16:49 ` George Karagounis
2026-09-25 20:58 ` ✗ LGCI.VerificationFailed: failure for drm/i915/display: Replace bw_constrained with max_link_bpp_x16 (rev2) Patchwork
3 siblings, 0 replies; 5+ messages in thread
From: George Karagounis @ 2026-09-24 16:49 UTC (permalink / raw)
To: Jani Nikula
Cc: Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin, David Airlie,
Simona Vetter, intel-gfx, intel-xe, dri-devel, linux-kernel, mail
In intel_crt.c (VGA displays) and intel_lvds.c (old laptop displays),
both files were using a legacy boolean flag crtc_state->bw_constrained
to guess if the display link was bandwidth limited.
To fix this, replaced the bw_constrained boolean checks with direct
comparisons against the max_link_bpp_x16 fixed-point API.
Signed-off-by: George Karagounis <mail@taterr.org>
---
v2: Remove the dead bw_constrained flag definition and writer entirely,
as pointed out by the sashiko bot reviewer
drivers/gpu/drm/i915/display/intel_crt.c | 5 ++---
drivers/gpu/drm/i915/display/intel_display.c | 1 -
drivers/gpu/drm/i915/display/intel_display_types.h | 1 -
drivers/gpu/drm/i915/display/intel_lvds.c | 6 +++---
4 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_crt.c b/drivers/gpu/drm/i915/display/intel_crt.c
index 5b8968197fbc..3da9547f2c6e 100644
--- a/drivers/gpu/drm/i915/display/intel_crt.c
+++ b/drivers/gpu/drm/i915/display/intel_crt.c
@@ -459,10 +459,9 @@ static int hsw_crt_compute_config(struct intel_atomic_state *state,
/* LPT FDI RX only supports 8bpc. */
if (HAS_PCH_LPT(display)) {
- /* TODO: Check crtc_state->max_link_bpp_x16 instead of bw_constrained */
- if (crtc_state->bw_constrained && crtc_state->pipe_bpp < 24) {
+ if (crtc_state->max_link_bpp_x16 < fxp_q4_from_int(24)) {
drm_dbg_kms(display->drm,
- "LPT only supports 24bpp\n");
+ "LPT only supports 24bpp\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 9dfd095f325f..fa6b11c4df47 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4767,7 +4767,6 @@ intel_modeset_pipe_config(struct intel_atomic_state *state,
"[CRTC:%d:%s] Link bpp limited to " FXP_Q4_FMT "\n",
crtc->base.base.id, crtc->base.name,
FXP_Q4_ARGS(crtc_state->max_link_bpp_x16));
- crtc_state->bw_constrained = true;
}
base_bpp = crtc_state->pipe_bpp;
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 43d53a98dae7..2166c84596c6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1146,7 +1146,6 @@ struct intel_crtc_state {
* required. This is set in the 2nd loop of calling encoder's
* ->compute_config if the first pick doesn't work out.
*/
- bool bw_constrained;
/* Settings for the intel dpll used on pretty much everything but
* haswell. */
diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index 872753478cf2..6bbd13ab5da7 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)) {
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;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 5+ messages in thread