[PATCH] drm: Fix HDMI_STATIC_METADATA_TYPE1 constant.

[Mario Kleiner]

On Mon, Jan 25, 2021 at 1:09 PM Ville Syrjälä <ville.syrjala at linux.intel.com>
wrote:

> On Sun, Jan 24, 2021 at 10:04:54PM +0100, Mario Kleiner wrote:
> > On Sun, Jan 24, 2021 at 9:24 PM Simon Ser <contact at emersion.fr> wrote:
> >
> > > On Sunday, January 24th, 2021 at 9:10 PM, Mario Kleiner <
> > > mario.kleiner.de at gmail.com> wrote:
> > >
> > > > But it still needs to be fixed if we want working HDR. I thought
> > > > libdrm copies the definitions from the kernel periodically, so the
> > > > fix should propagate?
> > >
> > > There will always be user-space that sends 1 instead of 0. This
> > > shouldn't fail on more recent kernels or it will be a regression.
> > >
> >
> > Yes, i know, regressing user-space is bad, but in this very specific
> case a
> > "good" one, if ever. At the moment, it wouldn't regress userspace simply
> > because the kernel doesn't actually check for the correct value in its
> HDR
> > metadata handling. But the value itself is sent as HDMI HDR metadata to
> the
> > attached HDR display monitor, so if the monitors firmware checks, it will
> > classify the wrong value of 1 as invalid and disable HDR mode on the
> > display, which is certainly not what a HDR client application wants. And
> > future HDR standards which will actually allocate the value 1 to a
> > different mode of HDR operation will switch to the wrong mode /
> > misinterpret the sent HDR metadata with hillarious results, which is also
> > not in the interest of a HDR client application, or a HDR capable
> > compositor.
> >
> > Iow. if clients continue to use the wrong value 1 then HDR display will
> > break in various ways on correctly implemented HDR displays, but in a
> > mystifying and hard to debug way. The kernel rejecting a wrong setting
> > explicitly and forcing a bug fix in the client would be a blessing in
> this
> > case.
> >
> > I spent weeks last year, going in circles and hunting ghost bugs related
> to
> > HDR because much of the HDR stuff, both drivers and monitor firmware
> seems
> > to be in not a great shape. "Less wrong" would be a big step forward.
> > Especially with the cheaper HDR monitors it is difficult to see when
> things
> > go wrong, because we don't have good expectations on how proper HDR
> should
> > look and the lower end HDR displays don't help.
>
> This is not an uapi defintion anyway so fixing should be fine.
> I don't think we even check any of the client provided values, or do we?
> EOTF I think we do check, but IMO that check should probably just be
> nuked as well if we don't bother checking anything else.
>
>
I think we check only EOTF atm. That check does make sense though, as
userspace getting that wrong will definitely knock out even low-end HDR
monitors. My tests with a - supposed to be pretty good according to tests -
DisplayHDR-600 monitor suggest that that's pretty much the only thing the
monitor actually uses, apart from CRC checking the data packet.


> I was in fact going to suggest nuking this entire hdr_sink_metadata
> parsing as unused, but looks like amdgpu has started to use it for
> some backlight stuff of all things.
>

My gut feeling says we will need this info in the kernel in the future,
independent of current users. Probably especially if one wants to do
interesting things which combine HDR with VRR/DP-Adaptive sync, or future
HDR standards (dynamic HDR metadata?), those infos in the kernel may become
quite useful.
 In some way it would even be nice to have all that info exposed in parsed
form as a connector property or such, so all clients can use the same
parsed data instead of reinventing the wheel. So I'd vote against nuking it.

Thanks,
-mario
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210125/e88e33b4/attachment.htm>