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? 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. 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. -- With best wishes Dmitry
