On Fri, Nov 21, 2025 at 04:36:28PM +0100, Maxime Ripard wrote: > On Tue, Oct 14, 2025 at 06:48:19PM +0300, Dmitry Baryshkov wrote: > > On Tue, Oct 14, 2025 at 02:59:43PM +0200, Maxime Ripard wrote: > > > On Fri, Oct 03, 2025 at 06:54:47PM +0300, Dmitry Baryshkov wrote: > > > > On Fri, Oct 03, 2025 at 03:22:23PM +0200, Maxime Ripard wrote: > > > > > On Tue, Sep 30, 2025 at 10:02:28AM +0300, Dmitry Baryshkov wrote: > > > > > > On Mon, Sep 29, 2025 at 03:00:04PM +0200, Maxime Ripard wrote: > > > > > > > On Thu, Sep 25, 2025 at 05:16:07PM +0300, Dmitry Baryshkov wrote: > > > > > > > > On Thu, Sep 25, 2025 at 03:13:47PM +0200, Maxime Ripard wrote: > > > > > > > > > On Wed, Sep 10, 2025 at 06:26:56PM +0300, Dmitry Baryshkov > > > > > > > > > wrote: > > > > > > > > > > On Wed, Sep 10, 2025 at 09:30:19AM +0200, Maxime Ripard > > > > > > > > > > wrote: > > > > > > > > > > > On Wed, Sep 03, 2025 at 03:03:43AM +0300, Dmitry > > > > > > > > > > > Baryshkov wrote: > > > > > > > > > > > > On Tue, Sep 02, 2025 at 08:06:54PM +0200, Maxime Ripard > > > > > > > > > > > > wrote: > > > > > > > > > > > > > On Tue, Sep 02, 2025 at 06:45:44AM +0300, Dmitry > > > > > > > > > > > > > Baryshkov wrote: > > > > > > > > > > > > > > On Mon, Sep 01, 2025 at 09:07:02AM +0200, Maxime > > > > > > > > > > > > > > Ripard wrote: > > > > > > > > > > > > > > > On Sun, Aug 31, 2025 at 01:29:13AM +0300, Dmitry > > > > > > > > > > > > > > > Baryshkov wrote: > > > > > > > > > > > > > > > > On Sat, Aug 30, 2025 at 09:30:01AM +0200, > > > > > > > > > > > > > > > > Daniel Stone wrote: > > > > > > > > > > > > > > > > > Hi Dmitry, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, 30 Aug 2025 at 02:23, Dmitry Baryshkov > > > > > > > > > > > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > > > > > > > It's not uncommon for the particular device > > > > > > > > > > > > > > > > > > to support only a subset of > > > > > > > > > > > > > > > > > > HDMI InfoFrames. It's not a big problem for > > > > > > > > > > > > > > > > > > the kernel, since we adopted > > > > > > > > > > > > > > > > > > a model of ignoring the unsupported > > > > > > > > > > > > > > > > > > Infoframes, but it's a bigger > > > > > > > > > > > > > > > > > > problem for the userspace: we end up having > > > > > > > > > > > > > > > > > > files in debugfs which do > > > > > > > > > > > > > > > > > > mot match what is being sent on the wire. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Sort that out, making sure that all > > > > > > > > > > > > > > > > > > interfaces are consistent. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the series, it's a really good > > > > > > > > > > > > > > > > > cleanup. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I know that dw-hdmi-qp can support _any_ > > > > > > > > > > > > > > > > > infoframe, by manually > > > > > > > > > > > > > > > > > packing it into the two GHDMI banks. So the > > > > > > > > > > > > > > > > > supported set there is > > > > > > > > > > > > > > > > > 'all of the currently well-known ones, plus > > > > > > > > > > > > > > > > > any two others, but only > > > > > > > > > > > > > > > > > two and not more'. I wonder if that has any > > > > > > > > > > > > > > > > > effect on the interface > > > > > > > > > > > > > > > > > you were thinking about for userspace? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I was mostly concerned with the existing > > > > > > > > > > > > > > > > debugfs interface (as it is > > > > > > > > > > > > > > > > also used e.g. for edid-decode, etc). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It seems "everything + 2 spare" is more or less > > > > > > > > > > > > > > > > common (ADV7511, MSM > > > > > > > > > > > > > > > > HDMI also have those. I don't have at hand the > > > > > > > > > > > > > > > > proper datasheet for > > > > > > > > > > > > > > > > LT9611 (non-UXC one), but I think its > > > > > > > > > > > > > > > > InfoFrames are also more or less > > > > > > > > > > > > > > > > generic). Maybe we should change debugfs > > > > > > > > > > > > > > > > integration to register the > > > > > > > > > > > > > > > > file when the frame is being enabled and > > > > > > > > > > > > > > > > removing it when it gets unset. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > But, like, for what benefit? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > It's a debugfs interface for userspace to > > > > > > > > > > > > > > > consume. The current setup > > > > > > > > > > > > > > > works fine with edid-decode already. Why should > > > > > > > > > > > > > > > we complicate the design > > > > > > > > > > > > > > > that much and create fun races like "I'm running > > > > > > > > > > > > > > > edid-decode in parallel > > > > > > > > > > > > > > > to a modeset that would remove the file I just > > > > > > > > > > > > > > > opened, what is the file > > > > > > > > > > > > > > > now?". > > > > > > > > > > > > > > > > > > > > > > > > > > > > Aren't we trading that with the 'I'm running > > > > > > > > > > > > > > edid-decode in paralle with > > > > > > > > > > > > > > to a modeset and the file suddenly becomes empty'? > > > > > > > > > > > > > > > > > > > > > > > > > > In that case, you know what the file is going to be: > > > > > > > > > > > > > empty. And you went > > > > > > > > > > > > > from a racy, straightforward, design to a racy, > > > > > > > > > > > > > complicated, design. > > > > > > > > > > > > > > > > > > > > > > > > > > It was my question before, but I still don't really > > > > > > > > > > > > > see what benefits it > > > > > > > > > > > > > would have, and why we need to care about it in the > > > > > > > > > > > > > core, when it could > > > > > > > > > > > > > be dealt with in the drivers just fine on a case by > > > > > > > > > > > > > case basis. > > > > > > > > > > > > > > > > > > > > > > > > Actually it can not: debugfs files are registered from > > > > > > > > > > > > the core, not > > > > > > > > > > > > from the drivers. That's why I needed all the > > > > > > > > > > > > supported_infoframes > > > > > > > > > > > > (which later became software_infoframes). > > > > > > > > > > > > > > > > > > > > > > That's one thing we can change then. > > > > > > > > > > > > > > > > > > > > > > > Anyway, I'm fine with having empty files there. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Then in the long run we can add 'slots' and > > > > > > > > > > > > > > > > allocate some of the frames > > > > > > > > > > > > > > > > to the slots. E.g. ADV7511 would get 'software > > > > > > > > > > > > > > > > AVI', 'software SPD', > > > > > > > > > > > > > > > > 'auto AUDIO' + 2 generic slots (and MPEG > > > > > > > > > > > > > > > > InfoFrame which can probably be > > > > > > > > > > > > > > > > salvaged as another generic one)). MSM HDMI > > > > > > > > > > > > > > > > would get 'software AVI', > > > > > > > > > > > > > > > > 'software AUDIO' + 2 generic slots (+MPEG + > > > > > > > > > > > > > > > > obsucre HDMI which I don't > > > > > > > > > > > > > > > > want to use). Then the framework might be able > > > > > > > > > > > > > > > > to prioritize whether to > > > > > > > > > > > > > > > > use generic slots for important data (as DRM > > > > > > > > > > > > > > > > HDR, HDMI) or less important > > > > > > > > > > > > > > > > (SPD). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Why is it something for the framework to deal > > > > > > > > > > > > > > > with? If you want to have > > > > > > > > > > > > > > > extra infoframes in there, just go ahead and > > > > > > > > > > > > > > > create additional debugfs > > > > > > > > > > > > > > > files in your driver. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > If you want to have the slot mechanism, check in > > > > > > > > > > > > > > > your atomic_check that > > > > > > > > > > > > > > > only $NUM_SLOT at most infoframes are set. > > > > > > > > > > > > > > > > > > > > > > > > > > > > The driver can only decide that 'we have VSI, SPD > > > > > > > > > > > > > > and DRM InfoFrames > > > > > > > > > > > > > > which is -ETOOMUCH for 2 generic slots'. The > > > > > > > > > > > > > > framework should be able to > > > > > > > > > > > > > > decide 'the device has 2 generic slots, we have HDR > > > > > > > > > > > > > > data, use VSI and > > > > > > > > > > > > > > DRM InfoFrames and disable SPD for now'. > > > > > > > > > > > > > > > > > > > > > > > > > > I mean... the spec does? The spec says when a > > > > > > > > > > > > > particular feature > > > > > > > > > > > > > requires to send a particular infoframe. If your > > > > > > > > > > > > > device cannot support > > > > > > > > > > > > > to have more than two "features" enabled at the same > > > > > > > > > > > > > time, so be it. It > > > > > > > > > > > > > something that should be checked in that driver > > > > > > > > > > > > > atomic_check. > > > > > > > > > > > > > > > > > > > > > > > > Sounds good to me. Let's have those checks in the > > > > > > > > > > > > drivers until we > > > > > > > > > > > > actually have seveal drivers performing generic frame > > > > > > > > > > > > allocation. > > > > > > > > > > > > > > > > > > > > > > > > > Or just don't register the SPD debugfs file, ignore > > > > > > > > > > > > > it, put a comment > > > > > > > > > > > > > there, and we're done too. > > > > > > > > > > > > > > > > > > > > > > > > It's generic code. > > > > > > > > > > > > > > > > > > > > > > > > > > But... We are not there yet and I don't have clear > > > > > > > > > > > > > > usecase (we support > > > > > > > > > > > > > > HDR neither on ADV7511 nor on MSM HDMI, after > > > > > > > > > > > > > > carefully reading the > > > > > > > > > > > > > > guide I realised that ADV7511 has normal audio > > > > > > > > > > > > > > infoframes). Maybe I > > > > > > > > > > > > > > should drop all the 'auto' features, simplifying > > > > > > > > > > > > > > this series and land > > > > > > > > > > > > > > [1] for LT9611UXC as I wanted origianlly. > > > > > > > > > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > https://lore.kernel.org/dri-devel/[email protected]/ > > > > > > > > > > > > > > > > > > > > > > > > > > Looking back at that series, I think it still has > > > > > > > > > > > > > value to rely on the > > > > > > > > > > > > > HDMI infrastructure at the very least for the > > > > > > > > > > > > > atomic_check sanitization. > > > > > > > > > > > > > > > > > > > > > > > > > > But since you wouldn't use the generated infoframes, > > > > > > > > > > > > > just skip the > > > > > > > > > > > > > debugfs files registration. You're not lying to > > > > > > > > > > > > > userspace anymore, and > > > > > > > > > > > > > you get the benefits of the HDMI framework. > > > > > > > > > > > > > > > > > > > > > > > > We create all infoframe files for all HDMI connectors. > > > > > > > > > > > > > > > > > > > > > > Then we can provide a debugfs_init helper to register all > > > > > > > > > > > of them, or > > > > > > > > > > > only some of them, and let the drivers figure it out. > > > > > > > > > > > > > > > > > > > > > > Worst case scenario, debugfs files will not get created, > > > > > > > > > > > which is a much > > > > > > > > > > > better outcome than having to put boilerplate in every > > > > > > > > > > > driver that will > > > > > > > > > > > get inconsistent over time. > > > > > > > > > > > > > > > > > > > > debugfs_init() for each infoframe or taking some kind of > > > > > > > > > > bitmask? > > > > > > > > > > > > > > > > > > I meant turning hdmi_debugfs_add and > > > > > > > > > create_hdmi_*_infoframe_file into > > > > > > > > > public helpers. That way, drivers that don't care can use the > > > > > > > > > (renamed) > > > > > > > > > hdmi_debugfs_add, and drivers with different constraints can > > > > > > > > > register > > > > > > > > > the relevant infoframes directly. > > > > > > > > > > > > > > > > Doesn't that mean more boilerplate? > > > > > > > > > > > > > > I don't think it would? In the general case, it wouldn't change > > > > > > > anything, and in special cases, then it's probably going to be > > > > > > > different > > > > > > > from one driver to the next so there's not much we can do. > > > > > > > > > > > > > > > In the end, LT9611UXC is a special case, for which I'm totally > > > > > > > > fine > > > > > > > > not to use HDMI helpers at this point: we don't control > > > > > > > > infoframes > > > > > > > > (hopefully that can change), we don't care about the TMDS > > > > > > > > clock, no > > > > > > > > CEC, etc. > > > > > > > > > > > > > > Not using the helpers sound pretty reasonable here too. > > > > > > > > > > > > > > > For all other usecases I'm fine with having atomic_check() > > > > > > > > unset all > > > > > > > > unsupported infoframes and having empty files in debugfs. Then > > > > > > > > we can > > > > > > > > evolve over the time, once we see a pattern. We had several > > > > > > > > drivers > > > > > > > > which had very limited infoframes support, but I think this now > > > > > > > > gets > > > > > > > > sorted over the time. > > > > > > > > > > > > > > I never talked about atomic_check()? You were initially concerned > > > > > > > that > > > > > > > the framework would expose data in debugfs that it's not using. > > > > > > > Not > > > > > > > registering anything in debugfs solves that, but I'm not sure we > > > > > > > need to > > > > > > > special case atomic_check. > > > > > > > > > > > > Well... I ended up with [1], handling infoframes in the > > > > > > atomic_check() > > > > > > rather than registering fewer infoframe debugfs files. This way > > > > > > device > > > > > > state is consistent, we don't have enabled instances, etc. However > > > > > > it > > > > > > results in repetetive code in atomic_check(). > > > > > > > > > > > > [1] > > > > > > https://lore.kernel.org/dri-devel/[email protected]/ > > > > > > > > > > I guess we can continue the discussion there, but I'm not sure we want > > > > > to have more boilerplate in drivers, and especially in the > > > > > atomic_check > > > > > part. If drivers are inconsistent or wrong in the debugfs path, > > > > > there's > > > > > no major issue. If they are wrong in the atomic_check path, it will > > > > > lead > > > > > to regressions, possibly in paths that are pretty hard to test. > > > > > > > > You've responded there and I can drop the extra handling for HDR DRM and > > > > audio infoframes in the atomic_check(). What is your opinion about the > > > > atomic_check() unsetting the infoframe->set for SPD and HDMI infoframes? > > > > > > HDMI infoframes are mandatory, so that's a big no-no. > > > > Nevertheless... There are drivers (sun4i, inno_hdmi, rk3066, dw_hdmi_qp) > > which don't (yet) implement VSI support. > > We should really differentiate drivers that didn't because they were > allowed to, and drivers that can't. > > For the bridge you mentioned earlier in the discussion, it makes sense > not to expose the debugfs file because we simply don't have access to > the actual content. That's fine. > > For the drivers you listed there, I'm pretty confident that it's because > nobody really tried. That's definitely not fine, and we should complain > as loudly as possible for that particular case, and not give them a free > pass. Checking sun4i, I'm pretty sure it can be implemented. Looking at > the incomplete RK3066 TRM, it can be implemented too. And inno_hdmi > looks really similar.
Interesting, according do a10s and a31 user manuals, there is no support for vendor infoframes, but it has SPD infoframe, which seemingly can be reused for HVIF. > So it's not really impossible, you just need some hardware and a day's > worth of work. > > There's no reason these should get a pass, it's breaking the spec for no > reason. > > > > For SPD, It's really not clear to me why atomic_check should do that in > > > the first place. Your initial concern was about exposing infoframes in > > > debugfs that wouldn't be used by the driver. > > > > > > If the driver doesn't register a debugfs file for SPD, and ignores > > > whatever is in the atomic state, what's should we force drivers to do > > > that? > > > > I really don't think that drivers should mess up with debugfs on their > > own. Making atomic_check() disable the unsupported InfoFrames makes the > > picture perfect: the DRM no longer tries to program them to the > > hardware, DebugFS files stay empty, so the whole state becomes > > consistent. > > In the "bridge has no access to infoframes" case, there's really no > infoframe. An empty file is "the infoframe can be there but isn't used", > not "we don't have access to it and can't report them". Only drivers > have those infos. > > If we do split up write_infoframe into multiple functions though, I > guess we could create the debugfs file only if the function pointer is > set, which removes drivers' involvement if you don't like that. I'm fine with not using HDMI connector framework for lt9611uxc. Likewise, I think, it's fine to have empty files for the infoframes which are not being sent over the wire for any reason (hw not supporting it is one of the reasons). I really don't see a good way to implement multiple-callbacks-as-a-supporter-flag inside drm_bridge_connector. So, I really think, disabling unsupported infoframes in atomic_check is the beset course. -- With best wishes Dmitry
