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

[PATCH 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>
---
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: be3e81d74654555fff2779d78848267bd50dcf51
change-id: 20260608-hdmi-max-bpc-fix-703019763834

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

[PATCH 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 a331ebdd65af..8303475ec021 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

[PATCH 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 subtracting max_bpc modulo 2 from max_bpc 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")
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 8303475ec021..9fbf88054ad8 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -648,7 +648,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 = max_bpc - 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 e89e1af7a811..dd1043ef8804 100644
--- a/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c
@@ -1327,6 +1327,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.
@@ -2227,6 +2297,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),
 	/*
 	 * TODO: We should have tests to check that a change in the
 	 * format triggers a CRTC mode change just like we do for the

-- 
2.54.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The function `sink_supports_format_bpc` explicitly permits 16 BPC for RGB444 and YCBCR444 formats without verifying if the sink's EDID actually supports 48-bit deep color.
--

commit 767fa3cc832e8b93f697ad91bcc66e5b3b1ed85d
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

drm/display: hdmi: Only allow BPC values of 8, 10, 12 and 16

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.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 4867edbf2622b..b06499dbfe8b3 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -419,6 +419,16 @@ sink_supports_format_bpc(const struct drm_connector *connector,
>  		return false;
>  	}
>  
> +	switch (bpc) {
> +	case 8:
> +	case 10:
> +	case 12:
> +	case 16:

[Severity: High]
This is a pre-existing issue, but does explicitly allowing 16 BPC here
expose a missing validation check for 48-bit deep color further down in
sink_supports_format_bpc()?

When checking DRM_OUTPUT_COLOR_FORMAT_RGB444 and
DRM_OUTPUT_COLOR_FORMAT_YCBCR444, the code validates 10 BPC and 12 BPC
against the EDID flags, but completely omits checking 16 BPC against
DRM_EDID_HDMI_DC_48:

drivers/gpu/drm/display/drm_hdmi_state_helper.c:sink_supports_format_bpc() {
	...
	case DRM_OUTPUT_COLOR_FORMAT_RGB444:
		...
		if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30)) {
			drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
			return false;
		}

		if (bpc == 12 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36)) {
			drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
			return false;
		}

		drm_dbg_kms(dev, "RGB format supported in that configuration.\n");
		return true;
	...
}

If the TMDS clock limit permits it, the driver might incorrectly conclude
that the sink supports 16 BPC and program the hardware accordingly, which
could result in a black screen if the monitor doesn't actually support
48-bit deep color.

Should a check for (info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_48)
be added for the RGB444 and YCBCR444 cases?

> +		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");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608-hdmi-max-bpc-fix-v1-0-6e8dcebc7274@collabora.com?part=1

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

[Nicolas Frattaroli]

On Monday, 8 June 2026 13:30:22 Central European Summer Time sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The function `sink_supports_format_bpc` explicitly permits 16 BPC for RGB444 and YCBCR444 formats without verifying if the sink's EDID actually supports 48-bit deep color.
> --
> 
> commit 767fa3cc832e8b93f697ad91bcc66e5b3b1ed85d
> Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> drm/display: hdmi: Only allow BPC values of 8, 10, 12 and 16
> 
> 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.
> 
> > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > index 4867edbf2622b..b06499dbfe8b3 100644
> > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > @@ -419,6 +419,16 @@ sink_supports_format_bpc(const struct drm_connector *connector,
> >  		return false;
> >  	}
> >  
> > +	switch (bpc) {
> > +	case 8:
> > +	case 10:
> > +	case 12:
> > +	case 16:
> 
> [Severity: High]
> This is a pre-existing issue, but does explicitly allowing 16 BPC here
> expose a missing validation check for 48-bit deep color further down in
> sink_supports_format_bpc()?
> 
> When checking DRM_OUTPUT_COLOR_FORMAT_RGB444 and
> DRM_OUTPUT_COLOR_FORMAT_YCBCR444, the code validates 10 BPC and 12 BPC
> against the EDID flags, but completely omits checking 16 BPC against
> DRM_EDID_HDMI_DC_48:
> 
> drivers/gpu/drm/display/drm_hdmi_state_helper.c:sink_supports_format_bpc() {
> 	...
> 	case DRM_OUTPUT_COLOR_FORMAT_RGB444:
> 		...
> 		if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30)) {
> 			drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
> 			return false;
> 		}
> 
> 		if (bpc == 12 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36)) {
> 			drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
> 			return false;
> 		}
> 
> 		drm_dbg_kms(dev, "RGB format supported in that configuration.\n");
> 		return true;
> 	...
> }
> 
> If the TMDS clock limit permits it, the driver might incorrectly conclude
> that the sink supports 16 BPC and program the hardware accordingly, which
> could result in a black screen if the monitor doesn't actually support
> 48-bit deep color.
> 
> Should a check for (info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_48)
> be added for the RGB444 and YCBCR444 cases?

Finally, we have automated the ultimate Linux kernel review response:
"while you're at it, can you fix everything else wrong in this code?"

(No, this does not expose any pre-existing issue; the validation is
more restrictive than it was before. That it's still not restrictive
enough in all cases does not introduce a new problem.)

> 
> > +		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");
> 
> 

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

[Nicolas Frattaroli]

On Monday, 8 June 2026 13:46:27 Central European Summer Time Nicolas Frattaroli wrote:
> On Monday, 8 June 2026 13:30:22 Central European Summer Time sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > 
> > Pre-existing issues:
> > - [High] The function `sink_supports_format_bpc` explicitly permits 16 BPC for RGB444 and YCBCR444 formats without verifying if the sink's EDID actually supports 48-bit deep color.
> > --
> > 
> > commit 767fa3cc832e8b93f697ad91bcc66e5b3b1ed85d
> > Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > 
> > drm/display: hdmi: Only allow BPC values of 8, 10, 12 and 16
> > 
> > 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.
> > 
> > > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > > index 4867edbf2622b..b06499dbfe8b3 100644
> > > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > > @@ -419,6 +419,16 @@ sink_supports_format_bpc(const struct drm_connector *connector,
> > >  		return false;
> > >  	}
> > >  
> > > +	switch (bpc) {
> > > +	case 8:
> > > +	case 10:
> > > +	case 12:
> > > +	case 16:
> > 
> > [Severity: High]
> > This is a pre-existing issue, but does explicitly allowing 16 BPC here
> > expose a missing validation check for 48-bit deep color further down in
> > sink_supports_format_bpc()?
> > 
> > When checking DRM_OUTPUT_COLOR_FORMAT_RGB444 and
> > DRM_OUTPUT_COLOR_FORMAT_YCBCR444, the code validates 10 BPC and 12 BPC
> > against the EDID flags, but completely omits checking 16 BPC against
> > DRM_EDID_HDMI_DC_48:
> > 
> > drivers/gpu/drm/display/drm_hdmi_state_helper.c:sink_supports_format_bpc() {
> > 	...
> > 	case DRM_OUTPUT_COLOR_FORMAT_RGB444:
> > 		...
> > 		if (bpc == 10 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_30)) {
> > 			drm_dbg_kms(dev, "10 BPC but sink doesn't support Deep Color 30.\n");
> > 			return false;
> > 		}
> > 
> > 		if (bpc == 12 && !(info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_36)) {
> > 			drm_dbg_kms(dev, "12 BPC but sink doesn't support Deep Color 36.\n");
> > 			return false;
> > 		}
> > 
> > 		drm_dbg_kms(dev, "RGB format supported in that configuration.\n");
> > 		return true;
> > 	...
> > }
> > 
> > If the TMDS clock limit permits it, the driver might incorrectly conclude
> > that the sink supports 16 BPC and program the hardware accordingly, which
> > could result in a black screen if the monitor doesn't actually support
> > 48-bit deep color.
> > 
> > Should a check for (info->edid_hdmi_rgb444_dc_modes & DRM_EDID_HDMI_DC_48)
> > be added for the RGB444 and YCBCR444 cases?
> 
> Finally, we have automated the ultimate Linux kernel review response:
> "while you're at it, can you fix everything else wrong in this code?"
> 
> (No, this does not expose any pre-existing issue; the validation is
> more restrictive than it was before. That it's still not restrictive
> enough in all cases does not introduce a new problem.)

Correction: there isn't even any problem here. DC48 is the highest
bpc, so the check is redundant anyway, which is likely also why it
was omitted.

Bad clanker! Bad bad bad!

> 
> > 
> > > +		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");
> > 
> > 
> 
>