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.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Changes in v3:
- Fixed supported infoframes initialization (Liu Ying)
- Implemented separate flags for DRM-generated InfoFrames and
  hardware-generated ones
- Warn if required InfoFrames are not supported by the driver (Maxime)
- Changed drivers to error out if the DRM framework requires the
  unsupported InfoFrame (Maxime)
- Implemented DRM_BRIDGE_OP_HDMI and DRM_BRIDGE_OP_HDMI_AUDIO for
  Lontium lt9611uxc bridge.
- Link to v2: https://lore.kernel.org/r/20250819-drm-limit-infoframes-v2-0-7595dda24fbd@oss.qualcomm.com

Changes in v2:
- Corrected whitespace / newline issues & other small syntax fixes (Liu
  Ying)
- Added audio to the list of InfoFrames supported by the VC4 driver (Liu
  Ying)
- Changed drm_dbg_kms() to drm_warn_once() in the Audio InfoFrame update
  code (Liu Ying)
- Corrected subject for ITE IT6263 patch (Liu Ying)
- Added patch, dropping default list of InfoFrames in
  drm_bridge_connector.
- Link to v1: https://lore.kernel.org/r/20250816-drm-limit-infoframes-v1-0-6dc17d5f07e9@oss.qualcomm.com

---
Dmitry Baryshkov (11):
      drm/connector: let drivers declare infoframes as unsupported
      drm/bridge: adv7511: declare supported infoframes
      drm/bridge: ite-it6263: declare supported infoframes
      drm/bridge: lontium-lt9611: declare supported infoframes
      drm/bridge: synopsys/dw-hdmi-qp: declare supported infoframes
      drm/msm: hdmi: declare supported infoframes
      drm/rockchip: rk3066: declare supported infoframes
      drm/display: bridge_connector: drop default list for HDMI Infoframes
      drm/connector: verify that HDMI connectors support necessary InfoFrames
      drm/display: hdmi-audio: warn if HDMI connector doesn't support Audio IF
      drm/bridge: lontium-lt9611uxc: switch to HDMI audio helpers

 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c       |  10 +-
 drivers/gpu/drm/bridge/ite-it6263.c                |  11 +-
 drivers/gpu/drm/bridge/lontium-lt9611.c            |  11 +-
 drivers/gpu/drm/bridge/lontium-lt9611uxc.c         | 130 +++++++++------------
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-qp.c       |   7 +-
 drivers/gpu/drm/display/drm_bridge_connector.c     |   2 +
 drivers/gpu/drm/display/drm_hdmi_audio_helper.c    |  12 ++
 drivers/gpu/drm/display/drm_hdmi_state_helper.c    |  53 ++++++++-
 drivers/gpu/drm/drm_connector.c                    |  17 +++
 drivers/gpu/drm/drm_debugfs.c                      |  16 ++-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c             |   8 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c               |   6 +-
 drivers/gpu/drm/rockchip/rk3066_hdmi.c             |   5 +-
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             |   4 +-
 drivers/gpu/drm/tests/drm_connector_test.c         |  49 ++++++++
 drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c |   9 ++
 drivers/gpu/drm/vc4/vc4_hdmi.c                     |   6 +
 include/drm/drm_bridge.h                           |  14 +++
 include/drm/drm_connector.h                        |  39 +++++++
 19 files changed, 306 insertions(+), 103 deletions(-)
---
base-commit: f4028ef65dc4fe9010bcc017597fe0fea6479097
change-id: 20250815-drm-limit-infoframes-e782fa7f3360

Best regards,
-- 
With best wishes
Dmitry

Hi Dmitry,

On Sat, 30 Aug 2025 at 02:23, Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> 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?

Cheers,
Daniel

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
> <dmitry.baryshkov@oss.qualcomm.com> 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.

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).

-- 
With best wishes
Dmitry

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
> > <dmitry.baryshkov@oss.qualcomm.com> 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?".

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

Maxime

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
> > > <dmitry.baryshkov@oss.qualcomm.com> 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'?

> > 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'.

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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/

-- 
With best wishes
Dmitry

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
> > > > <dmitry.baryshkov@oss.qualcomm.com> 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.

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

Or just don't register the SPD debugfs file, ignore it, put a comment
there, and we're done too.

> 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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/

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.

Maxime

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
> > > > > <dmitry.baryshkov@oss.qualcomm.com> 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).

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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/
> 
> 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.

-- 
With best wishes
Dmitry

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
> > > > > > <dmitry.baryshkov@oss.qualcomm.com> 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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/
> > 
> > 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.

Maxime

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
> > > > > > > <dmitry.baryshkov@oss.qualcomm.com> 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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/
> > > 
> > > 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?

-- 
With best wishes
Dmitry

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
> > > > > > > > <dmitry.baryshkov@oss.qualcomm.com> 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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/
> > > > 
> > > > 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.

Maxime

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
> > > > > > > > > <dmitry.baryshkov@oss.qualcomm.com> 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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/
> > > > > 
> > > > > 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

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
> > > > > > > > > > <dmitry.baryshkov@oss.qualcomm.com> 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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/
> > > > > > 
> > > > > > 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.

Worst case scenario, we're going to generate infoframes the driver will
ignore.

Maxime

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
> > > > > > > > > > > <dmitry.baryshkov@oss.qualcomm.com> 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/20250803-lt9611uxc-hdmi-v1-2-cb9ce1793acf@oss.qualcomm.com/
> > > > > > > 
> > > > > > > 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/20250928-limit-infoframes-2-v2-0-6f8f5fd04214@oss.qualcomm.com/

> Worst case scenario, we're going to generate infoframes the driver will
> ignore.

Yes.

-- 
With best wishes
Dmitry

Hi Dmitry,

On Sun, 31 Aug 2025 at 00:29, Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
> On Sat, Aug 30, 2025 at 09:30:01AM +0200, Daniel Stone wrote:
> > 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.

Oh right, that's really interesting.

> 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).

Yeah, that sounds good! I wonder if we want to surface that from the
debugfs interface: maybe just files named 0..$numslots with the
content of the infoframes, one named 'slots' with the integer index,
which infoframe type it's currently carrying, and which types it's
capable of carrying. Then to preserve the current interface, symlinks
from the infoframe type name to the slot index.

Cheers,
Daniel

On Sat, 30 Aug 2025 at 09:30, Daniel Stone <daniel@fooishbar.org> wrote:
> On Sat, 30 Aug 2025 at 02:23, Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> 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.

FWIW, the rest of the series is:
Acked-by: Daniel Stone <daniels@collabora.com>