[PATCH v2 0/2] Make HDMI state helpers handle odd max bpc requests

[PATCH v2 0/2] Make HDMI state helpers handle odd max bpc requests

[Nicolas Frattaroli]

With the "max bpc" KMS connector property, userspace can arbitrarily
restrict the upper end of the bits-per-component range. This is fine and
good, except the HDMI state helpers never considered that max_bpc could
be influenced by a userspace setting, so assumed it'll always be an even
value from the HDMI standards.

This, unfortunately, is not the world we live in anymore. Patch 1
corrects sink_supports_format_bpc to return false on BPCs outside of
what HDMI allows. Patch 2 then corrects handling of odd-numbered max
bpcs by rounding the loop start value down to an even number instead. It
also adds a KUnit test to make sure nobody breaks this again in the
future.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v2:
- Use rounddown() from linux/math.h instead of the bespoke rounding. No
  functional changes.
- Note: switch statement in patch 1 kept as-is, as 14bpc is not allowed
  by HDMI it seems.
- Link to v1: https://patch.msgid.link/20260608-hdmi-max-bpc-fix-v1-0-6e8dcebc7274@collabora.com

---
Nicolas Frattaroli (2):
      drm/display: hdmi: Only allow BPC values of 8, 10, 12 and 16
      drm/display: hdmi: Round odd max_bpc down to even numbers

 drivers/gpu/drm/display/drm_hdmi_state_helper.c    | 12 +++-
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 71 ++++++++++++++++++++++
 2 files changed, 82 insertions(+), 1 deletion(-)
---
base-commit: 24209d838338d162bb25aadfd637b11747a357ca
change-id: 20260608-hdmi-max-bpc-fix-703019763834

Best regards,
--  
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v2 1/2] drm/display: hdmi: Only allow BPC values of 8, 10, 12 and 16

[Nicolas Frattaroli]

As per the comment in sink_supports_format_bpc(), CTA-861-F defines that
only bits-per-channel values of 8, 10, 12 and 16 are allowed for HDMI.
Allowing more than this has surprising consequences for the atomic check
phase. The HDMI state helpers may accidentally conclude that a sink
supports 11bpc if a caller asks for it.

Fix this by exiting early if the bpc value doesn't match one of those
given in the standard.

Fixes: 26ff1c38fc29 ("drm/connector: hdmi: Compute bpc and format automatically")
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index ce17eeefc2da..0bf6102244f7 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -420,6 +420,16 @@ sink_supports_format_bpc(const struct drm_connector *connector,
 		return false;
 	}
 
+	switch (bpc) {
+	case 8:
+	case 10:
+	case 12:
+	case 16:
+		break;
+	default:
+		return false;
+	}
+
 	if (!info->is_hdmi &&
 	    (format != DRM_OUTPUT_COLOR_FORMAT_RGB444 || bpc != 8)) {
 		drm_dbg_kms(dev, "DVI Monitors require an RGB output at 8 bpc\n");

-- 
2.54.0

Re: [PATCH v2 1/2] drm/display: hdmi: Only allow BPC values of 8, 10, 12 and 16

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1639 bytes --]

Hi Nicolas,

On Thu, Jun 18, 2026 at 08:54:23PM +0200, Nicolas Frattaroli wrote:
> As per the comment in sink_supports_format_bpc(), CTA-861-F defines that
> only bits-per-channel values of 8, 10, 12 and 16 are allowed for HDMI.
> Allowing more than this has surprising consequences for the atomic check
> phase. The HDMI state helpers may accidentally conclude that a sink
> supports 11bpc if a caller asks for it.
> 
> Fix this by exiting early if the bpc value doesn't match one of those
> given in the standard.
> 
> Fixes: 26ff1c38fc29 ("drm/connector: hdmi: Compute bpc and format automatically")
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index ce17eeefc2da..0bf6102244f7 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -420,6 +420,16 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>  		return false;
>  	}
>  
> +	switch (bpc) {
> +	case 8:
> +	case 10:
> +	case 12:
> +	case 16:
> +		break;
> +	default:
> +		return false;
> +	}
> +

Sorry for not noticing in your first version, but I'd move this before
the VIC 1 check, and would add a comment to refer to the spec (possibly
by splitting the comment already there for the VIC 1 check).

We should also log debug message on failure like we do for every other
rejection path.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

[PATCH v2 2/2] drm/display: hdmi: Round odd max_bpc down to even numbers

[Nicolas Frattaroli]

The HDMI state helpers will count down from the max bpc to 8 in steps of
2, trying each value as a possible output bpc. This goes awry if max bpc
is restricted by userspace to an odd number with the "max bpc" connector
property.

Prevent this, without introducing any additional bpc format trial steps,
by simply rounding down to the next multiple of 2 as the starting point
for the for loop.

Additionally, add a KUnit test to validate the handling of this.

Fixes: 26ff1c38fc29 ("drm/connector: hdmi: Compute bpc and format automatically")
Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c    |  2 +-
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 71 ++++++++++++++++++++++
 2 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index 0bf6102244f7..e9553fab99b5 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -649,7 +649,7 @@ hdmi_compute_format_bpc(const struct drm_connector *connector,
 	unsigned int bpc;
 	int ret;
 
-	for (bpc = max_bpc; bpc >= 8; bpc -= 2) {
+	for (bpc = rounddown(max_bpc, 2); bpc >= 8; bpc -= 2) {
 		ret = hdmi_try_format_bpc(connector, conn_state, mode, bpc, fmt);
 		if (!ret)
 			continue;
diff --git a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
index 353a261d42da..31ac93d9be98 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -1361,6 +1361,76 @@ static void drm_test_check_tmds_char_rate_rgb_12bpc(struct kunit *test)
 	drm_modeset_acquire_fini(&ctx);
 }
 
+/*
+ * Test that given a request for an odd-numbered max bpc, the HDMI state helpers
+ * will succeed an atomic check but round down to the even-numbered bpc on the
+ * output, while leaving the requested value alone.
+ */
+static void drm_test_check_odd_max_bpc(struct kunit *test)
+{
+	struct drm_atomic_helper_connector_hdmi_priv *priv;
+	struct drm_connector_state *conn_state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_display_mode *preferred;
+	struct drm_atomic_commit *state;
+	struct drm_connector *conn;
+	struct drm_device *drm;
+	int ret;
+
+	priv = drm_kunit_helper_connector_hdmi_init_with_edid_funcs(test,
+				BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444),
+				12,
+				&dummy_connector_hdmi_funcs,
+				test_edid_hdmi_1080p_rgb_yuv_dc_max_340mhz);
+	KUNIT_ASSERT_NOT_NULL(test, priv);
+
+	drm = &priv->drm;
+	conn = &priv->connector;
+	preferred = find_preferred_mode(conn);
+	KUNIT_ASSERT_NOT_NULL(test, preferred);
+
+	KUNIT_ASSERT_TRUE(test, conn->display_info.is_hdmi);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+retry_conn_enable:
+	ret = drm_kunit_helper_enable_crtc_connector(test, drm, priv->crtc,
+						     conn, preferred, &ctx);
+	if (ret == -EDEADLK) {
+		ret = drm_modeset_backoff(&ctx);
+		if (!ret)
+			goto retry_conn_enable;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	state = drm_kunit_helper_atomic_state_alloc(test, drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+retry_conn_state:
+	conn_state = drm_atomic_get_connector_state(state, conn);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	conn_state->max_requested_bpc = 11;
+
+	ret = drm_atomic_check_only(state);
+	if (ret == -EDEADLK) {
+		drm_atomic_commit_clear(state);
+		ret = drm_modeset_backoff(&ctx);
+		if (!ret)
+			goto retry_conn_state;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	conn_state = drm_atomic_get_new_connector_state(state, conn);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, conn_state);
+
+	KUNIT_EXPECT_EQ(test, conn_state->hdmi.output_bpc, 10);
+	KUNIT_EXPECT_EQ(test, conn_state->max_requested_bpc, 11);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
 /*
  * Test that if we filter a rate through our hook, it's indeed rejected
  * by the whole atomic_check logic.
@@ -2476,6 +2546,7 @@ static struct kunit_case drm_atomic_helper_connector_hdmi_check_tests[] = {
 	KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_8bpc),
 	KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_10bpc),
 	KUNIT_CASE(drm_test_check_tmds_char_rate_rgb_12bpc),
+	KUNIT_CASE(drm_test_check_odd_max_bpc),
 	KUNIT_CASE_PARAM(drm_test_check_hdmi_color_format,
 			 check_hdmi_color_format_gen_params),
 	KUNIT_CASE_PARAM(drm_test_check_hdmi_color_format_420_only,

-- 
2.54.0