[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

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

[Maxime Ripard]

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

On Mon, Jun 08, 2026 at 01:19:07PM +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 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;
> +	}
> +

That might be slightly personal, but I think if (bpc < 8 || bpc > 16 ||
bpc % 2) is more readable

With that fixed
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

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

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

[Nicolas Frattaroli]

On Thursday, 18 June 2026 17:47:09 Central European Summer Time Maxime Ripard wrote:
> On Mon, Jun 08, 2026 at 01:19:07PM +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 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;
> > +	}
> > +
> 
> That might be slightly personal, but I think if (bpc < 8 || bpc > 16 ||
> bpc % 2) is more readable

That would allow 14bpc, which wouldn't be correct.

Kind regards,
Nicolas Frattaroli

> 
> With that fixed
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> 
> Maxime
> 

[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 2/2] drm/display: hdmi: Round odd max_bpc down to even numbers

[Maxime Ripard]

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

On Mon, Jun 08, 2026 at 01:19:08PM +0200, Nicolas Frattaroli wrote:
> 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) {

bpc = round_down(max_bpc, 2) (or rounddown) would be more readable here.

Looks good otherwise, once fixed
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

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

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

[Maxime Ripard]

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

Hi,

On Mon, Jun 08, 2026 at 01:19:06PM +0200, Nicolas Frattaroli wrote:
> 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>

Do you have a bit more details on the world you live in? :)

In particular, why would erroring out on setting an odd value in
atomic_set_property not work?

Maxime

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

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

[Nicolas Frattaroli]

On Tuesday, 9 June 2026 14:51:12 Central European Summer Time Maxime Ripard wrote:
> Hi,
> 
> On Mon, Jun 08, 2026 at 01:19:06PM +0200, Nicolas Frattaroli wrote:
> > 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>
> 
> Do you have a bit more details on the world you live in? :)
> 
> In particular, why would erroring out on setting an odd value in
> atomic_set_property not work?

It would work, but it'd be an inferior solution IMHO. (If the intent
was to point out this is already done then I can't find the code where
such a check is performed.)

It's perfectly fine, albeit weird, for userspace to say it wants a max
bpc of 11. That HDMI does not support 11 bpc isn't really something the
upper end of the range should concern itself with, much like we don't
error out on a max bpc of 14 either even though HDMI does not support
bit depths of 14 bits.

By counting from the next even number, we don't leak our implementation's
choice of trying every other bit depth through the uAPI with an overly
restrictive constraint being placed. In an alternate universe, mirror
world Maxime may have decided to i-- in that for loop instead just in
case, and the second patch wouldn't be needed.

Kind regards,
Nicolas Frattaroli

> 
> Maxime
> 

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

[Maxime Ripard]

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

On Tue, Jun 09, 2026 at 05:46:20PM +0200, Nicolas Frattaroli wrote:
> On Tuesday, 9 June 2026 14:51:12 Central European Summer Time Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Jun 08, 2026 at 01:19:06PM +0200, Nicolas Frattaroli wrote:
> > > 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>
> > 
> > Do you have a bit more details on the world you live in? :)
> > 
> > In particular, why would erroring out on setting an odd value in
> > atomic_set_property not work?
> 
> It would work, but it'd be an inferior solution IMHO. (If the intent
> was to point out this is already done then I can't find the code where
> such a check is performed.)
> 
> It's perfectly fine, albeit weird, for userspace to say it wants a max
> bpc of 11. That HDMI does not support 11 bpc isn't really something the
> upper end of the range should concern itself with, much like we don't
> error out on a max bpc of 14 either even though HDMI does not support
> bit depths of 14 bits.

Yeah, this makes total sense indeed.

> By counting from the next even number, we don't leak our implementation's
> choice of trying every other bit depth through the uAPI with an overly
> restrictive constraint being placed. In an alternate universe, mirror
> world Maxime may have decided to i-- in that for loop instead just in
> case, and the second patch wouldn't be needed.

Damn current world Maxime :D

Thanks for the explanation, the series looks mostly fine and I had two
nits.

Maxime

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

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

[Michel Dänzer]

On 6/9/26 14:51, Maxime Ripard wrote:
> On Mon, Jun 08, 2026 at 01:19:06PM +0200, Nicolas Frattaroli wrote:
>> 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>
> 
> Do you have a bit more details on the world you live in? :)
> 
> In particular, why would erroring out on setting an odd value in
> atomic_set_property not work?

That doesn't make sense, since the "max bpc" property purely defines an upper limit. Setting an odd value for it doesn't mean the kernel has to use an odd effective bpc value, it can use any valid bpc <= the "max bpc" property value.


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast

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

[Maxime Ripard]

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

On Wed, Jun 10, 2026 at 09:45:54AM +0200, Michel Dänzer wrote:
> On 6/9/26 14:51, Maxime Ripard wrote:
> > On Mon, Jun 08, 2026 at 01:19:06PM +0200, Nicolas Frattaroli wrote:
> >> 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>
> > 
> > Do you have a bit more details on the world you live in? :)
> > 
> > In particular, why would erroring out on setting an odd value in
> > atomic_set_property not work?
> 
> That doesn't make sense, since the "max bpc" property purely defines
> an upper limit. Setting an odd value for it doesn't mean the kernel
> has to use an odd effective bpc value, it can use any valid bpc <= the
> "max bpc" property value.

Ah, yes, of course. Thanks!
Maxime

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