On Mon, Sep 01, 2025 at 08:54:18AM +0200, Maxime Ripard wrote: > On Wed, Aug 27, 2025 at 05:04:53PM +0300, Dmitry Baryshkov wrote: > > On Wed, Aug 27, 2025 at 09:30:20AM +0200, Maxime Ripard wrote: > > > On Wed, Aug 20, 2025 at 12:52:44PM +0300, Dmitry Baryshkov wrote: > > > > On Wed, Aug 20, 2025 at 09:15:36AM +0200, Maxime Ripard wrote: > > > > > Hi, > > > > > > > > > > On Tue, Aug 19, 2025 at 09:57:30PM +0300, Dmitry Baryshkov wrote: > > > > > > Currently DRM framework expects that the HDMI connector driver > > > > > > supports > > > > > > all infoframe types: it generates the data as required and calls > > > > > > into > > > > > > the driver to program all of them, letting the driver to soft-fail > > > > > > if > > > > > > the infoframe is unsupported. This has a major drawback on userspace > > > > > > API: the framework also registers debugfs files for all Infoframe > > > > > > types, > > > > > > possibly surprising the users when infoframe is visible in the > > > > > > debugfs > > > > > > file, but it is not visible on the wire. > > > > > > > > > > > > Let drivers declare that they support only a subset of infoframes, > > > > > > creating a more consistent interface. > > > > > > > > > > > > Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com> > > > > > > > > > > I'm not really convinced. Infoframes aren't really something you > > > > > should > > > > > ignore, AVI is effectively mandatory, HDMI kind of is too, AUDIO is if > > > > > audio support is enabled, DRM is mandatory if HDR is used. > > > > > > > > Nevertheless, sun4i, innohdmi, adv7511, it6263 and rk3066 drivers > > > > provide support only for the AVI infoframe. > > > > > > Yes, but it's still something we shouldn't paper over. The spec mandates > > > it, if drivers want to deviate from it it's something we should warn > > > about, not silence. > > > > > > sun4i is a good example, to me at least since I have the doc. The > > > hardware supports AVI, Audio, ACP, and SPD. HDR isn't supported, so DRM > > > isn't either. The only missing one is HDMI, but the documentation isn't > > > the best so it might still be supported. In short, it's a driver issue. > > > > > > adv7511 supports AVI, Audio, ACP, SPD, ACP, and looks to have a > > > mechanism to send any infoframe as is. So, again, driver issue. > > > > I've send a patch, enabling SPD and VSI (HDMI) InfoFrames on ADV7511. > > > > > > > > I couldn't find the other datasheet, but I'd be very surprised if it > > > wasn't the case for these too. > > > > > > > Some of them can be extended to support other infoframe kinds (e.g. > > > > ADV7511 has two spare infoframes which can be used for HDMI and SPD). > > > > > > > > > SPD is indeed optional though. > > > > > > > > > > So, it's really dynamic in essence, and not really something we should > > > > > expect drivers to ignore. > > > > > > > > > > I do acknowledge that a lot of drivers just silently ignore the > > > > > infoframes they don't support at the moment, which isn't great either. > > > > > > > > > > Maybe we should standardize and document what drivers should do when > > > > > they don't support a given infoframe type? > > > > > > > > The chips might be generating infoframes internally. This series was > > > > triggered by LT9611UXC, which does all HDMI work under the hood in the > > > > firmware. See [1]. The series I posted hooks HDMI audio directly into > > > > the bridge driver, but I'd really prefer to be able to use > > > > drm_atomic_helper_connector_hdmi_hotplug(), especially if I ever get to > > > > implementing CEC support for it. > > > > > > > > ADV7511 likewise generates audio infoframe without Linux > > > > help (audio-related fields are programmed, but it's not the > > > > infoframe itself). > > > > > > Implementing the write_infoframe hooks as a nop with a comment in those > > > case is totally reasonable to me. > > > > > > I'd still like to document that drivers should only return 0 if they > > > programmed the infoframe, and -ENOTSUPP (and the core logging a warning) > > > otherwise. > > > > > > That way, we would be able to differentiate between the legimitate > > > LT9611UXC case, and the "driver is broken" sun4i (and others) case. > > > > I don't want to end up in a sitation where userspace has a different > > idea of the InfoFrame being sent than the actual one being present on > > the wire. > > It's not ideal, sure, but also, what's wrong with it? We're doing it > *all the time*. Modes programmed by userspace are adjusted for the > hardware, and thus the mode reported by the CRTC turns out different > than the one actually used in hardware. Audio sampling rates might not > match exactly what we're doing. The quirks infrastructure disables part > of the EDID the userspace has access to, etc. > > And all those are under the userspace control, which the infoframes > aren't.
I think there is a differnece between 'change userspace input', 'knowingly mangle data' and 'lie to userspace because the driver doesn't care'. This is especially important e.g. if a user is trying to debug AV issues which can be caused by wrong information in the infoframe. > > > It seems, we need several states per the infoframe: > > > > - Not supported > > Honestly, I'm not sure we need a state for that one. If that infoframe > was set by the framework, then the driver must support it. And if it > wasn't, then there's nothing in debugfs. Yes, I ended up dropping this and having two separate flags. > > > - Autogenerated > > Do we have any way to read them back on those? Usually not. E.g. I don't think I can read back Audio InfoFrame on ADV7511. Nor can I read InfoFrames on LT9611UXC. > > > - Generated by software > > > > E.g. in case of ADV7511 we can declare that Audio InfofFrame is > > autogenerated, AVI, HDMI and SPD as 'software-generated' and DRM (HDR) > > as unsupported. LT9611UXC will declare all (need to check) frame types > > as auto. > > > > This way we can implement the checks and still keep userspace from > > having irrelevant data in debugfs. > > If the only thing you're after is to prevent inconsistent data in > userpace for devices that can generate it automatically, then I guess we > could just implement an (optional) callback to read an infoframe from > the hardware when reading from debugfs. Would that work? As I wrote, this is not always possible, so I'd skip this. > > > I will update my patchset to implement this, but I have another question > > beforehand: should we just declare VSI support or should it be more exact, > > specifying that the driver support HVS (00:0c:03), HVFS (c4:5d:d8), etc? > > I guess you're talking about HDMI 1.4 Vendor specific Infoframe vs HDMI > 2.0 HF-VSIF here? Yes. H14v-VSIF vs HF-VSIF. > > If so, the toggle should be HDMI 2.0 support. We'll need that toggle for > other things anyway (scrambler, YUV420, etc.) > > > I'm asking, because e.g. MSM HDMI controller has hardware support for > > generating HVS frames (but only HVS, the OUI is not programmed, register > > format doesn't match 1:1 frame contents, etc). I instead ended up using > > GENERIC0, because it was more flexible (it's like SPARE packets on > > ADV7511, the contents is being sent as is). However if we ever need to > > send DRM infoframes, we might need to switch from GENERIC0 to HVS, for > > the price of being unable to send HVFS frames. > > Section 10.2 of the HDMI 2.0 states: > > Transmission of the HF-VSIF by Source Devices is optional unless one (or > more) of the features listed in Table 10-1 is active 1. If such features > are active, transmission of the HF-VSIF is mandatory. > > The features in question being 3d. Or ALLM. It's not on my todo list though. > So unless you're supporting 3d, suppporting VSI only seems ok to me. MSM HDMI controllers can support some bits and pieces of 3D. Nobody bothered to implement that though. Maybe I should try getting a monitor which supports stereo output. -- With best wishes Dmitry
