Re: [PATCH 0/2] Make HDMI state helpers handle odd max bpc requests
From: Maxime Ripard
Date: Thu Jun 18 2026 - 11:50:49 EST
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@xxxxxxxxxxxxx>
> >
> > 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:
signature.asc
Description: PGP signature