Re: [PATCH v3 00/10] media: synopsys: hdmirx: support an HDMI bridge in front of the receiver

[Hans Verkuil <hverkuil+cisco@kernel.org>]

Hi Sascha,

On 05/10/2026 15:28, Sascha Hauer wrote:
> The Synopsys HDMI receiver driver assumes the HDMI connector belongs to
> it: the connector's +5V line arrives on hpd-gpios, and there is nothing
> between the source and the receiver. On the board this series comes from
> that is not the case. An HDMI bridge owns the connector, the receiver
> sits behind it, and the two are described by an OF graph endpoint.
> 
> Three things follow from that, and the series is arranged in that order.
> 
> Patch 1 comes before them and is a fix for the driver as it is. The
> driver data is devm allocated, but an open file descriptor keeps the
> video device alive past unbind, so the last close works on freed memory.
> The media device the series adds would have been torn down the same way,
> which is why the fix leads the series. It stands on its own and is
> marked for stable.
> 
> Patches 2-3 give the driver a media device and register the video device
> as an entity with a sink pad. On their own they add a graph with one node
> in it; they exist so that there is somewhere for the bridge to appear.
> 
> Patches 4-5 add the endpoint to the binding and bind the remote subdev
> through a v4l2 async notifier, linking its source pad to the receiver's
> sink pad. Such a board cannot provide hpd-gpios, so the binding requires
> either hpd-gpios or the port. Without an endpoint in the device tree the
> driver keeps working exactly as before, so boards wired straight to the
> connector are unaffected.
> 
> Patch 6 is fallout from the wait for signal lock, which was bounded by an
> iteration count whose duration came from the 5V debounce it called
> each time round. That debounce goes away on a board with no hpd-gpios
> later in the series, so the wait gets a real timeout first.
> 
> Patches 7-10 are the 5V state itself. Patch 7 stops probe requiring an
> interrupt for hpd-gpios. Patch 8 makes v4l2_device_unregister_subdev()
> wait for notifications already in flight. Nothing kept a subdev from
> notifying a v4l2_device that was unbinding it; that is not new with this
> series, as asynchronously bound subdevs sending V4L2_EVENT_SOURCE_CHANGE
> race the same way today. Patch 9 adds V4L2_DEVICE_NOTIFY_RX_POWER_PRESENT,
> and patch 10 has the receiver set v4l2_dev.notify and take the state from
> whatever is bound in front of it. The notification is only an edge, as
> the 5V interrupt is with hpd-gpios: the hotplug worker then asks the
> subdev through g_input_status(), so nothing is cached that could go
> stale against bind or unbind. Without a det_irq there is nothing to
> disable across suspend and remove either, so patch 10 disables the
> hotplug work items themselves instead of cancelling them. Patch 8 is
> what lets the notify callback read sd->v4l2_dev while the subdev is
> being unbound.
> 
> V4L2_CID_DV_RX_POWER_PRESENT is otherwise the only place this state is
> published - adv7604, adv7842, tc358743 and tda1997x all set that control
> from their cable detect interrupt and stop there - while a format change
> already gets an in-kernel notification through v4l2_subdev_notify_event().
> An earlier version of this series hooked the control with
> v4l2_ctrl_notify() instead. That reaches into another device's control
> handler, and the only thing it bought was working with those four
> drivers unmodified, which turns out to be worth nothing: they are HDMI
> sinks, so none of them could sit in front of an HDMI receiver anyway.
> 
> Nothing in tree sends the new notification yet. The driver for the device
> on this board does, and the alternative was a mechanism that no in-tree
> driver could have used either.
> 
> There is no in-tree device tree using the new port yet: the board is not
> upstream, and the device in front of the receiver, a Lontium LT86102UXE
> 1:2 HDMI splitter, has no driver upstream either. I would rather send the
> infrastructure for review now than sit on it until the board lands.
> 
> Tested on that board, and build tested for arm64 at each patch with W=1.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

As mentioned in my reply to patch 1/10 I'll take that one. But for the remaining
9 patches I need to see the 'v4l2-compliance -m /dev/mediaX -s' output to make
sure the media device topology is sane and streaming works as expected.

Use the v4l2-compliance version from the v4l-utils git repo so you have the latest
code.

I also need more time to review the v4l2 core notify patches. Apologies for not
doing that sooner, I hadn't noticed before that this series made core changes as well.

I haven't looked at that notify code for years, so I will need to refresh my
memory.

My plan is to merge patch 1/10 today in our next branch. It is probably best
if you post a v4 rebased to that and including the v4l2-compliance output in
the cover letter.

Regards,

	Hans

> ---
> Changes in v3:
> - new patch 1: free the driver data from the v4l2_device release callback
>   instead of devres, an open file descriptor keeps the video device alive
>   past unbind. The media device cleanup Sashiko flagged on the old patch
>   1 was the same problem, which predates the series (Sashiko on patch 1)
> - v4l2-device: wait for the SRCU grace period on every subdev
>   unregistration, not only when the v4l2_device has a notify callback,
>   as the reader dereferences v4l2_dev either way (Sashiko on patch 7)
> - Link to v2: https://lore.kernel.org/r/20260924-hdmirx-media-v2-0-c12f641d8b0f@pengutronix.de
> 
> Changes in v2:
> - dt-bindings: squash "add the source port" and "make hpd-gpios optional"
>   into one patch and require exactly one of hpd-gpios and port (Conor)
> - dt-bindings: drop the OS-specific sentence from the port description
>   (Conor)
> - hdmirx: take graph_mutex around media_create_pad_link() in the async
>   bound callback, the media device is already registered (Sashiko)
> - new patch 7: v4l2-device: wait for notifications when unregistering a
>   subdev, so a notification cannot outlive the unbind of the receiver
>   (Sashiko on patch 9)
> - hdmirx: ask the subdev in front for the 5V state on every notification
>   instead of caching what the notification carries, and disable the
>   hotplug work items across probe, suspend and remove instead of gating
>   the workers on a flag and cancelling once more after the notifier is
>   unregistered
> - Link to v1: https://lore.kernel.org/r/20260923-hdmirx-media-v1-0-ea3d77d4a5f3@pengutronix.de
> 
> ---
> Gerald Loacker (6):
>       media: synopsys: hdmirx: add media device infrastructure
>       media: synopsys: hdmirx: add media entity for video capture device
>       media: synopsys: hdmirx: add async subdevice support
>       media: synopsys: hdmirx: give the signal lock wait a real timeout
>       media: synopsys: hdmirx: skip the 5V detect interrupt without a GPIO
>       media: synopsys: hdmirx: get the 5V state from the upstream subdev
> 
> Sascha Hauer (4):
>       media: synopsys: hdmirx: free the driver data when the last user is gone
>       dt-bindings: media: snps,dw-hdmi-rx: add a port for a bridge in front
>       media: v4l2-device: wait for notifications when unregistering a subdev
>       media: v4l2-subdev: notify the bridge when the source power changes
> 
>  .../devicetree/bindings/media/snps,dw-hdmi-rx.yaml |  21 +-
>  Documentation/driver-api/media/v4l2-subdev.rst     |   8 +-
>  drivers/media/platform/synopsys/hdmirx/Kconfig     |   1 +
>  .../media/platform/synopsys/hdmirx/snps_hdmirx.c   | 355 +++++++++++++++++----
>  drivers/media/v4l2-core/v4l2-device.c              |  42 +++
>  include/media/v4l2-device.h                        |  15 +-
>  include/media/v4l2-subdev.h                        |  12 +
>  7 files changed, 386 insertions(+), 68 deletions(-)
> ---
> base-commit: bc35965f6940a9bf834d54187b6088b8eb09206d
> change-id: 20260904-hdmirx-media-fe7eb952b2be
> 
> Best regards,