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

[Maxime Ripard <mripard@kernel.org>]

[-- 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 --]