On Tue, Sep 02, 2025 at 06:12:39AM +0300, Dmitry Baryshkov wrote: > 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.
We can play semantics if you want, but the line is *very* fine between "mangling data" and "lying". If the mode has been mangled, and you don't report that it has been to userspace, are you lying? I guess you could argue both ways, but it's really the same than what we're discussing here. Except for the fact that one is a central part of the uapi, that the userspace depends on, and one is a debugfs file that *isn't* a uAPI, and the userspace really shouldn't depend on. > > > 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. Then if you don't have access to it, we can just not register the debugfs files. Maxime
signature.asc
Description: PGP signature
