Hi,

On Tue, Jun 16, 2026 at 02:04:13PM +0200, Nicolas Frattaroli wrote:
> On Monday, 15 June 2026 18:33:11 Central European Summer Time Maxime Ripard 
> wrote:
> > On Mon, Jun 15, 2026 at 10:07:02AM +0200, Nicolas Frattaroli wrote:
> > > On Saturday, 13 June 2026 08:57:29 Central European Summer Time Hans 
> > > Verkuil wrote:
> > > > Hi Nicolas,
> > > > 
> > > > On 11/06/2026 14:57, Nicolas Frattaroli wrote:
> > > > > HDMI uses the DDC I2C bus for communicating various bits of link 
> > > > > status
> > > > > out of band with the actual HDMI video signal. This information can be
> > > > > useful for debugging issues like questionable cables sabotaged by 
> > > > > feline
> > > > > teeth, Enthusiast Grade cables made of cow fencing wire, and other 
> > > > > such
> > > > > problems that ruin one's media viewing plans.
> > > > > 
> > > > > Consequently, this series exposes various bits of pertinent 
> > > > > information
> > > > > from the SCDC protocol in an HDMI connector's debugfs. To continually
> > > > > poll the link status, userspace can poll the debugfs file.
> > > > 
> > > > Something is not quite right: I've been testing this series with my i915
> > > > based laptop with HDMI connector, and I never see the scdc_status file.
> > > > And that's because CONFIG_DRM_BRIDGE_CONNECTOR is not set for my 
> > > > configuration.
> > > 
> > > That's to be expected. i915 does not use bridge connectors, and neither
> > > does amdgpu iirc. I am working on embedded boards that do use bridge
> > > connectors, along with all the rest of the HDMI state helpers.
> > > 
> > > Implementing something new in DRM usually involves having to triplicate
> > > certain parts of the work in order to have i915 and amdgpu behave the
> > > same as everyone else.
> > > 
> > > > So I think you are creating the debugfs entry in the wrong place.
> > > 
> > > I can't create it for all drm_connectors as Maxime suggested due to the
> > > cyclical dependency that would create. If someone has any idea on how
> > > to break that dependency cycle, I'm open to suggestions.
> > 
> > Back when we introduce the audio support, we floated the idea to
> > de-midlayer this and turn it into a debugfs_init helper. We don't have a
> > lot of users yet so it might be the best time to do so, and would solve
> > your issue.
> 
> I agree and will be happy to do that. I need some more concrete info
> though on what "de-midlayer" means here. From what I can tell, the
> core problem is that drm_connector.c directly calls into
> drm_debugfs_connector_add of drm_debugfs.c, which if it did also
> do a direct call into drm_scdc_helper.c would mean drm_scdc_helper.c
> needs drm_connector.c but drm_connector.c needs drm_scdc_helper.c.
> 
> So I guess the de-midlayer and turn it into helper part is that
> drm_connector does an indirect call to a function pointer instead,
> which is default-filled at runtime with the helper debugfs_init
> function? I don't know if we can then do this by default for all
> drivers in drm_connector.c or if it needs to be done per-driver
> like some of the other helpers. If the latter, then we don't
> really gain much over what I'm doing with bridge connector.

I'd promote the hdmi_debugfs_add() call in drm_debugfs_connector_add to
an debugfs_init helper for drivers that use the HDMI state helpers,
because it's only useful for those anyway.

> The other maybe less intrusive change is that
> drm_scdc_debugfs_init and scdc_status_fops travels to drm_debugfs.c,
> while the actual implementation of the read op remains in
> drm_scdc_helper.c. I think that would solve it without a big refactor?

Then, depending on what makes the most sense, you can either create
another debugfs_init helper for scdc that would be called by the new
hdmi state debugfs_init helper or any other relevant driver, or put it
directly into that new helper.

Maxime

Attachment: signature.asc
Description: PGP signature

Reply via email to