Message-ID: <hyfwwfiazs4rspjre24zgf6n4uq3267qawnyxrksy7n4wfqiuq@46lcdlmdik3s>
Date: Tue, 2 Sep 2025 19:38:44 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Andrzej Hajda <andrzej.hajda@...el.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, 
	Laurent Pinchart <Laurent.pinchart@...asonboard.com>, Jonas Karlman <jonas@...boo.se>, 
	Jernej Skrabec <jernej.skrabec@...il.com>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Sandy Huang <hjc@...k-chips.com>, 
	Heiko Stübner <heiko@...ech.de>, Andy Yan <andy.yan@...k-chips.com>, Chen-Yu Tsai <wens@...e.org>, 
	Samuel Holland <samuel@...lland.org>, Dave Stevenson <dave.stevenson@...pberrypi.com>, 
	Maíra Canal <mcanal@...lia.com>, Raspberry Pi Kernel Maintenance <kernel-list@...pberrypi.com>, 
	Liu Ying <victor.liu@....com>, Rob Clark <robin.clark@....qualcomm.com>, 
	Dmitry Baryshkov <lumag@...nel.org>, Abhinav Kumar <abhinav.kumar@...ux.dev>, 
	Jessica Zhang <jessica.zhang@....qualcomm.com>, Sean Paul <sean@...rly.run>, 
	Marijn Suijten <marijn.suijten@...ainline.org>, dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, linux-sunxi@...ts.linux.dev, 
	linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org
Subject: Re: [PATCH v2 1/8] drm/connector: let drivers declare infoframes as
 unsupported

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.baryshkov@....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

Download attachment "signature.asc" of type "application/pgp-signature" (274 bytes)

Powered by blists - more mailing lists