On Thu, Sep 25, 2025 at 05:55:06PM +0300, Dmitry Baryshkov wrote:

quoted

quoted

As we will be getting more and more features, some of the InfoFrames
or data packets will be 'good to have, but not required'.

And drivers would be free to ignore those.

quoted

quoted

So, no, sorry. That's still a no for me. Please stop sending that patch

Oops :-)

quoted

unless we have a discussion about it and you convince me that it's
actually something that we'd need.

My main concern is that the drivers should not opt-out of the features.
E.g. if we start supporting ISRC packets or MPEG or NTSC VBI InfoFrames
(yes, stupid examples), it should not be required to go through all the
drivers, making sure that they disable those. Instead the DRM framework
should be able to make decisions like:

- The driver supports SPD and the VSDB defines SPD, enable this
  InfoFrame (BTW, this needs to be done anyway, we should not be sending
  SPD if it's not defined in VSDB, if I read it correctly).

- The driver hints that the pixel data has only 10 meaninful bits of
  data per component (e.g. out of 12 for DeepColor 36), the Sink has
  HF-VSDB, send HF-VSIF.

- The driver has enabled 3D stereo mode, but it doesn't declare support
  for HF-VSIF. Send only H14b-VSIF.

Similarly (no, I don't have these on my TODO list, these are just
examples):
- The driver defines support for NTSC VBI, register a VBI device.

- The driver defines support for ISRC packets, register ISRC-related
  properties.

- The driver defines support for MPEG Source InfoFrame, provide a way
  for media players to report frame type and bit rate.

- The driver provides limited support for Extended HDR DM InfoFrames,
  select the correct frame type according to driver capabilities.

Without the 'supported' information we should change atomic_check()
functions to set infoframe->set to false for all unsupported InfoFrames
_and_ go through all the drivers again each time we add support for a
feature (e.g. after adding HF-VSIF support).

From what you described here, I think we share a similar goal and have
somewhat similar concerns (thanks, btw, it wasn't obvious to me before),
we just disagree on the trade-offs and ideal solution :)

I agree that we need to sanity check the drivers, and I don't want to go
back to the situation we had before where drivers could just ignore
infoframes and take the easy way out.

It should be hard, and easy to catch during review.

I don't think bitflag are a solution because, to me, it kind of fails
both.

What if, just like the debugfs discussion, we split write_infoframe into
write_avi_infoframe (mandatory), write_spd_infoframe (optional),
write_audio_infoframe (checked by drm_connector_hdmi_audio_init?) and
write_hdr_infoframe (checked in drmm_connector_hdmi_init if max_bpc > 8)

How does that sound?

I'd say, I really like the single function to be called for writing the
infoframes. It makes it much harder for drivers to misbehave or to skip
something.

From a driver PoV, I believe we should still have that single function
indeed. It would be drm_atomic_helper_connector_hdmi_update_infoframes's
job to fan out and call the multiple callbacks, not the drivers.

Maxime