On Thu, Sep 25, 2025 at 05:55:06PM +0300, Dmitry Baryshkov wrote: > > > 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. > > > > > > So, no, sorry. That's still a no for me. Please stop sending that patch > > > > > > Oops :-) > > > > > > > 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
signature.asc
Description: PGP signature
