[PATCH v2 0/9] media: synopsys: hdmirx: support an HDMI bridge in front of the receiver

[PATCH v2 0/9] media: synopsys: hdmirx: support an HDMI bridge in front of the receiver

[Sascha Hauer]

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.

Patches 1-2 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 3-4 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 5 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 6-9 are the 5V state itself. Patch 6 stops probe requiring an
interrupt for hpd-gpios. Patch 7 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 8 adds V4L2_DEVICE_NOTIFY_RX_POWER_PRESENT,
and patch 9 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 9 disables the
hotplug work items themselves instead of cancelling them. Patch 7 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>
---
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 (3):
      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   | 300 ++++++++++++++++++---
 drivers/media/v4l2-core/v4l2-device.c              |  43 +++
 include/media/v4l2-device.h                        |  15 +-
 include/media/v4l2-subdev.h                        |  12 +
 7 files changed, 354 insertions(+), 46 deletions(-)
---
base-commit: bc35965f6940a9bf834d54187b6088b8eb09206d
change-id: 20260904-hdmirx-media-fe7eb952b2be

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH v2 1/9] media: synopsys: hdmirx: add media device infrastructure

[Sascha Hauer]

From: Gerald Loacker <gerald.loacker@wolfvision.net>

Initialize the media device to enable integration with HDMI bridge
devices in the video capture pipeline. This provides the foundation
for representing the hardware topology through the media controller
framework.

Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 25f8ca0d6d946..f51a1619b71ec 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -31,6 +31,7 @@
 #include <linux/workqueue.h>
 
 #include <media/cec.h>
+#include <media/media-device.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
@@ -123,6 +124,7 @@ struct snps_hdmirx_dev {
 	struct device *dev;
 	struct hdmirx_stream stream;
 	struct v4l2_device v4l2_dev;
+	struct media_device mdev;
 	struct v4l2_ctrl_handler hdl;
 	struct v4l2_ctrl *detect_tx_5v_ctrl;
 	struct v4l2_ctrl *rgb_range;
@@ -2742,6 +2744,11 @@ static int hdmirx_probe(struct platform_device *pdev)
 		goto err_hdl;
 	}
 
+	hdmirx_dev->mdev.dev = dev;
+	strscpy(hdmirx_dev->mdev.model, "snps-hdmirx", sizeof(hdmirx_dev->mdev.model));
+	media_device_init(&hdmirx_dev->mdev);
+	hdmirx_dev->v4l2_dev.mdev = &hdmirx_dev->mdev;
+
 	stream = &hdmirx_dev->stream;
 	stream->hdmirx_dev = hdmirx_dev;
 	ret = hdmirx_register_stream_vdev(stream);
@@ -2771,6 +2778,7 @@ static int hdmirx_probe(struct platform_device *pdev)
 	vb2_video_unregister_device(&hdmirx_dev->stream.vdev);
 err_unreg_v4l2_dev:
 	v4l2_device_unregister(&hdmirx_dev->v4l2_dev);
+	media_device_cleanup(&hdmirx_dev->mdev);
 err_hdl:
 	v4l2_ctrl_handler_free(&hdmirx_dev->hdl);
 err_pm:
@@ -2794,6 +2802,7 @@ static void hdmirx_remove(struct platform_device *pdev)
 	vb2_video_unregister_device(&hdmirx_dev->stream.vdev);
 	v4l2_ctrl_handler_free(&hdmirx_dev->hdl);
 	v4l2_device_unregister(&hdmirx_dev->v4l2_dev);
+	media_device_cleanup(&hdmirx_dev->mdev);
 
 	/* touched by hdmirx_disable()->hdmirx_plugout() */
 	hdmirx_dev->rgb_range = NULL;

-- 
2.47.3

[PATCH v2 2/9] media: synopsys: hdmirx: add media entity for video capture device

[Sascha Hauer]

From: Gerald Loacker <gerald.loacker@wolfvision.net>

Register the video device as a media entity with a sink pad to integrate
it into the media controller topology. This allows the capture device to
be properly represented in the media graph and enables userspace to
discover the device capabilities through the MC API.

The sink pad indicates that this entity receives data (HDMI video stream)
which is then made available through the V4L2 video capture interface.

Initialize the pad before registering the video device. The previous patch
gave v4l2_dev an mdev, so video_register_device() already registers the
entity, and doing it the other way round would hand
media_device_register_entity() an entity with no pads yet.

Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c   | 26 ++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index f51a1619b71ec..11a9b16586a57 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -125,6 +125,7 @@ struct snps_hdmirx_dev {
 	struct hdmirx_stream stream;
 	struct v4l2_device v4l2_dev;
 	struct media_device mdev;
+	struct media_pad pad;
 	struct v4l2_ctrl_handler hdl;
 	struct v4l2_ctrl *detect_tx_5v_ctrl;
 	struct v4l2_ctrl *rgb_range;
@@ -2751,15 +2752,29 @@ static int hdmirx_probe(struct platform_device *pdev)
 
 	stream = &hdmirx_dev->stream;
 	stream->hdmirx_dev = hdmirx_dev;
+
+	hdmirx_dev->pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_pads_init(&stream->vdev.entity, 1, &hdmirx_dev->pad);
+	if (ret) {
+		dev_err_probe(dev, ret, "media entity pads initialization failed\n");
+		goto err_unreg_v4l2_dev;
+	}
+
 	ret = hdmirx_register_stream_vdev(stream);
 	if (ret < 0) {
 		dev_err_probe(dev, ret, "video device registration failed\n");
-		goto err_unreg_v4l2_dev;
+		goto err_cleanup_entity;
+	}
+
+	ret = media_device_register(&hdmirx_dev->mdev);
+	if (ret) {
+		dev_err_probe(dev, ret, "media device registration failed\n");
+		goto err_unreg_video_dev;
 	}
 
 	ret = hdmirx_register_cec(hdmirx_dev, pdev);
 	if (ret)
-		goto err_unreg_video_dev;
+		goto err_unreg_media;
 
 	hdmirx_load_default_edid(hdmirx_dev);
 
@@ -2774,8 +2789,12 @@ static int hdmirx_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_unreg_media:
+	media_device_unregister(&hdmirx_dev->mdev);
 err_unreg_video_dev:
 	vb2_video_unregister_device(&hdmirx_dev->stream.vdev);
+err_cleanup_entity:
+	media_entity_cleanup(&hdmirx_dev->stream.vdev.entity);
 err_unreg_v4l2_dev:
 	v4l2_device_unregister(&hdmirx_dev->v4l2_dev);
 	media_device_cleanup(&hdmirx_dev->mdev);
@@ -2799,7 +2818,10 @@ static void hdmirx_remove(struct platform_device *pdev)
 
 	hdmirx_disable_irq(dev);
 
+	media_device_unregister(&hdmirx_dev->mdev);
+
 	vb2_video_unregister_device(&hdmirx_dev->stream.vdev);
+	media_entity_cleanup(&hdmirx_dev->stream.vdev.entity);
 	v4l2_ctrl_handler_free(&hdmirx_dev->hdl);
 	v4l2_device_unregister(&hdmirx_dev->v4l2_dev);
 	media_device_cleanup(&hdmirx_dev->mdev);

-- 
2.47.3

[PATCH v2 3/9] dt-bindings: media: snps,dw-hdmi-rx: add a port for a bridge in front

[Sascha Hauer]

Describe the OF graph port connecting the receiver to an HDMI bridge in
front of it. The schema sets additionalProperties: false, so without
this a device tree carrying the endpoint does not validate at all.

hpd-gpios is the +5V line of the HDMI connector, which the source drives
to announce itself. On a board whose HDMI connector belongs to such a
bridge, that line goes to the bridge and there is nothing for the
receiver to sense, so the property cannot be provided. The name is
unfortunate: the hot plug detect line the receiver drives back at the
source is a controller register, not this GPIO. Describe what it
actually carries.

A board has either the connector wired to the receiver or a bridge in
front of it, so require exactly one of hpd-gpios and port.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../devicetree/bindings/media/snps,dw-hdmi-rx.yaml  | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
index b7f6c87d0e06a..896f8ffbb5bf9 100644
--- a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
+++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
@@ -63,9 +63,21 @@ properties:
     maxItems: 1
 
   hpd-gpios:
-    description: GPIO specifier for HPD.
+    description:
+      The +5V line of the HDMI connector, which the source drives to announce
+      itself. Despite the name this is an input to the receiver; the hot plug
+      detect line the receiver drives back at the source is handled inside the
+      controller. Absent when the connector belongs to a bridge in front of
+      this receiver rather than to the receiver itself.
     maxItems: 1
 
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description:
+      Connection to the HDMI source. Present when an HDMI bridge sits in
+      front of this receiver instead of the connector being wired straight
+      to it.
+
   rockchip,grf:
     $ref: /schemas/types.yaml#/definitions/phandle
     description:
@@ -88,7 +100,12 @@ required:
   - power-domains
   - resets
   - pinctrl-0
-  - hpd-gpios
+
+oneOf:
+  - required:
+      - hpd-gpios
+  - required:
+      - port
 
 additionalProperties: false
 

-- 
2.47.3

[PATCH v2 4/9] media: synopsys: hdmirx: add async subdevice support

[Sascha Hauer]

From: Gerald Loacker <gerald.loacker@wolfvision.net>

Add V4L2 async notifier support to enable dynamic binding of connected
subdevices in the video pipeline. The driver parses device-tree endpoints
to discover and register video bridge subdevices.

When a subdevice is bound, a media link is created between its output pad
and the HDMI RX sink pad. Subdevice nodes are registered once binding
completes, making them accessible from userspace.

If no endpoint is defined in the device tree, the driver operates in
standalone mode and registers its subdevice nodes directly, maintaining
backward compatibility with simpler configurations.

The notifier itself is unregistered on the way out whether or not a
subdevice ever bound. Only v4l2_async_nf_unregister() takes the notifier
off the global list, so cleaning up without it would leave an entry
pointing into memory devres is about to free. Both calls are no-ops when
there was no endpoint to watch.

Ask the bound subdevice which of its pads faces us rather than assuming
the second one. media_create_pad_link() warns and fails on an index the
entity does not have, which a bridge with a single pad would hit.

V4L2_ASYNC has to be selected now that the driver calls the notifier
helpers. v4l2-async.h has no stubs for them, and nothing else the driver
already selects pulls the symbol in.

The media device is already registered when the subdevice binds, so
userspace can be walking the graph under graph_mutex while the link is
added. media_create_pad_link() does not take the lock itself; take it
around the call.

Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/media/platform/synopsys/hdmirx/Kconfig     |  1 +
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c   | 92 +++++++++++++++++++++-
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/Kconfig b/drivers/media/platform/synopsys/hdmirx/Kconfig
index 4321f985f6320..598c303eba8d0 100644
--- a/drivers/media/platform/synopsys/hdmirx/Kconfig
+++ b/drivers/media/platform/synopsys/hdmirx/Kconfig
@@ -5,6 +5,7 @@ config VIDEO_SYNOPSYS_HDMIRX
 	depends on ARCH_ROCKCHIP || COMPILE_TEST
 	depends on VIDEO_DEV
 	select MEDIA_CONTROLLER
+	select V4L2_ASYNC
 	select VIDEO_V4L2_SUBDEV_API
 	select VIDEOBUF2_DMA_CONTIG
 	select CEC_CORE
diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 11a9b16586a57..d80143347fe0c 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -20,6 +20,7 @@
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/of_platform.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/pinctrl/consumer.h>
@@ -32,12 +33,14 @@
 
 #include <media/cec.h>
 #include <media/media-device.h>
+#include <media/v4l2-async.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-dv-timings.h>
 #include <media/v4l2-event.h>
 #include <media/v4l2-fh.h>
+#include <media/v4l2-fwnode.h>
 #include <media/v4l2-ioctl.h>
 #include <media/videobuf2-dma-contig.h>
 #include <media/videobuf2-v4l2.h>
@@ -126,6 +129,7 @@ struct snps_hdmirx_dev {
 	struct v4l2_device v4l2_dev;
 	struct media_device mdev;
 	struct media_pad pad;
+	struct v4l2_async_notifier notifier;
 	struct v4l2_ctrl_handler hdl;
 	struct v4l2_ctrl *detect_tx_5v_ctrl;
 	struct v4l2_ctrl *rgb_range;
@@ -2649,6 +2653,50 @@ static int hdmirx_register_cec(struct snps_hdmirx_dev *hdmirx_dev,
 	return 0;
 }
 
+static int hdmirx_fwnode_bound(struct v4l2_async_notifier *notifier,
+			       struct v4l2_subdev *subdev,
+			       struct v4l2_async_connection *asc)
+{
+	struct snps_hdmirx_dev *hdmirx_dev =
+		container_of(notifier, struct snps_hdmirx_dev, notifier);
+	int source_pad;
+	int ret;
+
+	source_pad = media_entity_get_fwnode_pad(&subdev->entity,
+						 asc->match.fwnode,
+						 MEDIA_PAD_FL_SOURCE);
+	if (source_pad < 0) {
+		dev_err(hdmirx_dev->dev, "%s has no source pad for %pfw: %d\n",
+			subdev->name, asc->match.fwnode, source_pad);
+		return source_pad;
+	}
+
+	mutex_lock(&hdmirx_dev->mdev.graph_mutex);
+	ret = media_create_pad_link(&subdev->entity, source_pad,
+				    &hdmirx_dev->stream.vdev.entity, 0,
+				    MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
+	mutex_unlock(&hdmirx_dev->mdev.graph_mutex);
+	if (ret) {
+		dev_err(hdmirx_dev->dev, "Failed to create media link: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int hdmirx_fwnode_complete(struct v4l2_async_notifier *notifier)
+{
+	struct snps_hdmirx_dev *hdmirx_dev =
+		container_of(notifier, struct snps_hdmirx_dev, notifier);
+
+	return v4l2_device_register_subdev_nodes(&hdmirx_dev->v4l2_dev);
+}
+
+static const struct v4l2_async_notifier_operations hdmirx_async_ops = {
+	.bound = hdmirx_fwnode_bound,
+	.complete = hdmirx_fwnode_complete,
+};
+
 static int hdmirx_probe(struct platform_device *pdev)
 {
 	struct snps_hdmirx_dev *hdmirx_dev;
@@ -2656,6 +2704,8 @@ static int hdmirx_probe(struct platform_device *pdev)
 	struct v4l2_ctrl_handler *hdl;
 	struct hdmirx_stream *stream;
 	struct v4l2_device *v4l2_dev;
+	struct device_node *ep_node;
+	struct v4l2_async_connection *asc;
 	int ret;
 
 	hdmirx_dev = devm_kzalloc(dev, sizeof(*hdmirx_dev), GFP_KERNEL);
@@ -2772,9 +2822,38 @@ static int hdmirx_probe(struct platform_device *pdev)
 		goto err_unreg_video_dev;
 	}
 
+	ep_node = of_graph_get_endpoint_by_regs(dev->of_node, 0, -1);
+	if (ep_node) {
+		v4l2_async_nf_init(&hdmirx_dev->notifier, &hdmirx_dev->v4l2_dev);
+		hdmirx_dev->notifier.ops = &hdmirx_async_ops;
+
+		asc = v4l2_async_nf_add_fwnode_remote(&hdmirx_dev->notifier,
+						      of_fwnode_handle(ep_node),
+						      struct v4l2_async_connection);
+		of_node_put(ep_node);
+
+		if (IS_ERR(asc)) {
+			ret = PTR_ERR(asc);
+			dev_err_probe(dev, ret, "Failed to add remote fwnode\n");
+			goto err_unreg_media;
+		}
+
+		ret = v4l2_async_nf_register(&hdmirx_dev->notifier);
+		if (ret) {
+			dev_err_probe(dev, ret, "Failed to register async notifier\n");
+			goto err_cleanup_notifier;
+		}
+	} else {
+		ret = v4l2_device_register_subdev_nodes(&hdmirx_dev->v4l2_dev);
+		if (ret) {
+			dev_err_probe(dev, ret, "Failed to register subdev nodes\n");
+			goto err_unreg_media;
+		}
+	}
+
 	ret = hdmirx_register_cec(hdmirx_dev, pdev);
 	if (ret)
-		goto err_unreg_media;
+		goto err_unreg_notifier;
 
 	hdmirx_load_default_edid(hdmirx_dev);
 
@@ -2789,6 +2868,10 @@ static int hdmirx_probe(struct platform_device *pdev)
 
 	return 0;
 
+err_unreg_notifier:
+	v4l2_async_nf_unregister(&hdmirx_dev->notifier);
+err_cleanup_notifier:
+	v4l2_async_nf_cleanup(&hdmirx_dev->notifier);
 err_unreg_media:
 	media_device_unregister(&hdmirx_dev->mdev);
 err_unreg_video_dev:
@@ -2818,6 +2901,13 @@ static void hdmirx_remove(struct platform_device *pdev)
 
 	hdmirx_disable_irq(dev);
 
+	/*
+	 * Both are no-ops when there was no endpoint to watch: the notifier is
+	 * zeroed and neither v4l2_dev nor waiting_list is set.
+	 */
+	v4l2_async_nf_unregister(&hdmirx_dev->notifier);
+	v4l2_async_nf_cleanup(&hdmirx_dev->notifier);
+
 	media_device_unregister(&hdmirx_dev->mdev);
 
 	vb2_video_unregister_device(&hdmirx_dev->stream.vdev);

-- 
2.47.3

[PATCH v2 5/9] media: synopsys: hdmirx: give the signal lock wait a real timeout

[Sascha Hauer]

From: Gerald Loacker <gerald.loacker@wolfvision.net>

hdmirx_wait_signal_lock() bounded itself by a count of 300 iterations.
How long that came to depended on the debounce inside
tx_5v_power_present(), which the loop calls once per iteration and which
sleeps about 7ms of it, so the effective timeout was around two seconds
by accident rather than by design.

That accident is about to go away. A later patch in this series takes the
5V state from an upstream subdev where the board has no hpd-gpios, which
leaves tx_5v_power_present() a plain read of a flag costing microseconds.
The same 300 iterations would then be over in a few milliseconds and the
wait would give up almost immediately.

Poll on a fixed 10ms interval against an explicit three second deadline.
Check the deadline after the 5V test so a disconnect is still reported as
-ENOLINK rather than a timeout, and report the timeout as -ETIMEDOUT; the
sole caller only tests for non-zero, so neither is a change in behaviour.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c   | 40 +++++++++++++++-------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index d80143347fe0c..9af1e71afb4e5 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -2133,13 +2133,25 @@ static irqreturn_t hdmirx_dma_irq_handler(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+/*
+ * Wait for the HDMI signal to lock: TMDS clock detection and ratio
+ * configuration, then PHY and CMU lock, then DMA lock.
+ *
+ * The wait used to be bounded by a count of 300 iterations. What that came
+ * to in wall clock time was decided by the debounce inside
+ * tx_5v_power_present(), which is called once per iteration and sleeps
+ * about 7ms of it, so the real timeout was an accident of how 5V happens to
+ * be sensed. Poll on a fixed interval against an explicit deadline instead.
+ */
 static int hdmirx_wait_signal_lock(struct snps_hdmirx_dev *hdmirx_dev)
 {
 	struct v4l2_device *v4l2_dev = &hdmirx_dev->v4l2_dev;
 	u32 mu_status, scdc_status, dma_st10, cmu_st;
-	u32 i;
+	unsigned long timeout;
+
+	timeout = jiffies + msecs_to_jiffies(3000);
 
-	for (i = 0; i < 300; i++) {
+	for (;;) {
 		mu_status = hdmirx_readl(hdmirx_dev, MAINUNIT_STATUS);
 		scdc_status = hdmirx_readl(hdmirx_dev, SCDC_REGBANK_STATUS3);
 		dma_st10 = hdmirx_readl(hdmirx_dev, DMA_STATUS10);
@@ -2153,21 +2165,25 @@ static int hdmirx_wait_signal_lock(struct snps_hdmirx_dev *hdmirx_dev)
 		if (!tx_5v_power_present(hdmirx_dev)) {
 			v4l2_dbg(1, debug, v4l2_dev,
 				 "%s: HDMI pull out, return\n", __func__);
-			return -1;
+			return -ENOLINK;
 		}
 
-		hdmirx_tmds_clk_ratio_config(hdmirx_dev);
-	}
+		if (time_after(jiffies, timeout)) {
+			v4l2_err(v4l2_dev,
+				 "%s: signal not lock, tmds_clk_ratio:%d\n",
+				 __func__, hdmirx_dev->tmds_clk_ratio);
+			v4l2_err(v4l2_dev,
+				 "%s: mu_st:%#x, scdc_st:%#x, dma_st10:%#x, cmu_st:%#x\n",
+				 __func__, mu_status, scdc_status, dma_st10,
+				 cmu_st);
+			return -ETIMEDOUT;
+		}
 
-	if (i == 300) {
-		v4l2_err(v4l2_dev, "%s: signal not lock, tmds_clk_ratio:%d\n",
-			 __func__, hdmirx_dev->tmds_clk_ratio);
-		v4l2_err(v4l2_dev, "%s: mu_st:%#x, scdc_st:%#x, dma_st10:%#x\n",
-			 __func__, mu_status, scdc_status, dma_st10);
-		return -1;
+		hdmirx_tmds_clk_ratio_config(hdmirx_dev);
+		usleep_range(10000, 11000);
 	}
 
-	v4l2_dbg(1, debug, v4l2_dev, "%s: signal lock ok, i:%d\n", __func__, i);
+	v4l2_dbg(1, debug, v4l2_dev, "%s: signal lock ok\n", __func__);
 	hdmirx_writel(hdmirx_dev, GLOBAL_SWRESET_REQUEST, DATAPATH_SWRESETREQ);
 
 	reinit_completion(&hdmirx_dev->avi_pkt_rcv);

-- 
2.47.3

[PATCH v2 6/9] media: synopsys: hdmirx: skip the 5V detect interrupt without a GPIO

[Sascha Hauer]

From: Gerald Loacker <gerald.loacker@wolfvision.net>

The hpd GPIO is fetched with devm_gpiod_get_optional(), so a board that
does not wire 5V detect to the receiver leaves detect_5v_gpio NULL.
hdmirx_setup_irq() then calls gpiod_to_irq() on it anyway, which returns
-EINVAL for a NULL descriptor, and probe fails. The GPIO is only
optional as far as the fetch goes.

Set the interrupt up only when the GPIO is there. det_irq then keeps the
zero devm_kzalloc() gave it, so hdmirx_enable_irq() and
hdmirx_disable_irq() skip it as well.

Nothing else has to change for such a board to probe: 5V is then never
reported as present, which is what gpiod_get_value_cansleep() on a NULL
descriptor already returns. Getting the state from somewhere else is a
separate matter.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c   | 34 ++++++++++++----------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 9af1e71afb4e5..4b94e35c912c1 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -2525,7 +2525,8 @@ static void hdmirx_disable_irq(struct device *dev)
 {
 	struct snps_hdmirx_dev *hdmirx_dev = dev_get_drvdata(dev);
 
-	disable_irq(hdmirx_dev->det_irq);
+	if (hdmirx_dev->det_irq > 0)
+		disable_irq(hdmirx_dev->det_irq);
 	disable_irq(hdmirx_dev->dma_irq);
 	disable_irq(hdmirx_dev->hdmi_irq);
 
@@ -2539,7 +2540,8 @@ static void hdmirx_enable_irq(struct device *dev)
 
 	enable_irq(hdmirx_dev->hdmi_irq);
 	enable_irq(hdmirx_dev->dma_irq);
-	enable_irq(hdmirx_dev->det_irq);
+	if (hdmirx_dev->det_irq > 0)
+		enable_irq(hdmirx_dev->det_irq);
 
 	queue_delayed_work(system_dfl_wq,
 			   &hdmirx_dev->delayed_work_hotplug,
@@ -2623,21 +2625,23 @@ static int hdmirx_setup_irq(struct snps_hdmirx_dev *hdmirx_dev,
 		return ret;
 	}
 
-	irq = gpiod_to_irq(hdmirx_dev->detect_5v_gpio);
-	if (irq < 0) {
-		dev_err_probe(dev, irq, "failed to get hdmirx-5v irq\n");
-		return irq;
-	}
+	if (hdmirx_dev->detect_5v_gpio) {
+		irq = gpiod_to_irq(hdmirx_dev->detect_5v_gpio);
+		if (irq < 0) {
+			dev_err_probe(dev, irq, "failed to get hdmirx-5v irq\n");
+			return irq;
+		}
 
-	irq_set_status_flags(irq, IRQ_NOAUTOEN);
+		irq_set_status_flags(irq, IRQ_NOAUTOEN);
 
-	hdmirx_dev->det_irq = irq;
-	ret = devm_request_irq(dev, irq, hdmirx_5v_det_irq_handler,
-			       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
-			       "rk_hdmirx-5v", hdmirx_dev);
-	if (ret) {
-		dev_err_probe(dev, ret, "failed to request hdmirx-5v irq\n");
-		return ret;
+		hdmirx_dev->det_irq = irq;
+		ret = devm_request_irq(dev, irq, hdmirx_5v_det_irq_handler,
+				       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
+				       "rk_hdmirx-5v", hdmirx_dev);
+		if (ret) {
+			dev_err_probe(dev, ret, "failed to request hdmirx-5v irq\n");
+			return ret;
+		}
 	}
 
 	return 0;

-- 
2.47.3

[PATCH v2 7/9] media: v4l2-device: wait for notifications when unregistering a subdev

[Sascha Hauer]

v4l2_subdev_notify() reads sd->v4l2_dev without synchronizing with
v4l2_device_unregister_subdev(), which clears it. A subdev notifies from
its own interrupt handler or work item, and when it is bound
asynchronously that runs independently of the bridge driver, which can
unbind it at any time. A notification racing with the unbind then
either dereferences a NULL v4l2_dev, since the inline helper reloads it
after its check and nearly every notify callback reloads it once more
for container_of(), or calls into a bridge that has already torn down.
adv7180, tc358743 and lt6911uxe send V4L2_EVENT_SOURCE_CHANGE this way
to rcar-vin and rp1-cfe, both of which install a notify callback.

Protect the call with SRCU. A plain RCU read side will not do because
some callbacks sleep, such as cobalt's, which takes a mutex, while
other notifications come from hard interrupt context, cx23885 IR and
imx-media-fim among them. SRCU allows both.

The callbacks read sd->v4l2_dev themselves, so clearing it and then
waiting is not enough. Add sd->notify_enabled, clear that first, wait
for the readers, and only then let v4l2_device_unregister_subdev() go
on to clear v4l2_dev. The registration error path does the same. The
grace period is only waited for when the v4l2_device has a notify
callback, which few do.

The callback runs under the read side while the unregistering side
waits, so it must not wait for anything the unregistering thread may
hold: a mutex held by the caller of v4l2_device_unregister_subdev(),
the driver core's device lock, or v4l2-async's list_lock, which is held
across unbinding. Only sleeping locks matter, as a spinlock cannot be
held across the wait. The only existing callbacks that take one are
cobalt's, which takes pci_lock around a register update, and cx23885's,
which for the cx25840 IR block runs the IR work handler directly and
ends up in cx25840's rx_params_lock. Neither lock is held across subdev
unregistration. Lockdep models the SRCU read side and synchronize_srcu(),
so a callback breaking the rule is reported.

Document the rule, and bring the notify description in v4l2-subdev.rst up
to date: the helper has not been a macro returning an error for a long
time.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/driver-api/media/v4l2-subdev.rst |  8 +++--
 drivers/media/v4l2-core/v4l2-device.c          | 43 ++++++++++++++++++++++++++
 include/media/v4l2-device.h                    | 15 +++++----
 include/media/v4l2-subdev.h                    |  4 +++
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/Documentation/driver-api/media/v4l2-subdev.rst b/Documentation/driver-api/media/v4l2-subdev.rst
index 13aec460e802f..2e8ced276d212 100644
--- a/Documentation/driver-api/media/v4l2-subdev.rst
+++ b/Documentation/driver-api/media/v4l2-subdev.rst
@@ -345,9 +345,11 @@ e.g. AUDIO_CONTROLLER and specify that as the group ID value when calling
 that needs it.
 
 If the sub-device needs to notify its v4l2_device parent of an event, then
-it can call ``v4l2_subdev_notify(sd, notification, arg)``. This macro checks
-whether there is a ``notify()`` callback defined and returns ``-ENODEV`` if not.
-Otherwise the result of the ``notify()`` call is returned.
+it can call ``v4l2_subdev_notify(sd, notification, arg)``. This calls the
+``notify()`` callback of the v4l2_device the sub-device is registered with, if
+there is one, and does nothing otherwise. Once
+``v4l2_device_unregister_subdev()`` returns, the callback is no longer running
+for that sub-device and will not be called for it again.
 
 V4L2 sub-device userspace API
 -----------------------------
diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c
index 67e3073de1321..5d7b7badaf2d0 100644
--- a/drivers/media/v4l2-core/v4l2-device.c
+++ b/drivers/media/v4l2-core/v4l2-device.c
@@ -10,10 +10,17 @@
 #include <linux/ioctl.h>
 #include <linux/module.h>
 #include <linux/slab.h>
+#include <linux/srcu.h>
 #include <linux/videodev2.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
 
+/*
+ * Subdevs notify from their own context, unsynchronized with the bridge
+ * unregistering them. Readers hold this while calling into the bridge.
+ */
+DEFINE_STATIC_SRCU(v4l2_subdev_notify_srcu);
+
 int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
 {
 	if (v4l2_dev == NULL)
@@ -108,6 +115,17 @@ void v4l2_device_unregister(struct v4l2_device *v4l2_dev)
 }
 EXPORT_SYMBOL_GPL(v4l2_device_unregister);
 
+/*
+ * Stop notifications to sd->v4l2_dev and wait for those in progress.
+ * Callbacks read sd->v4l2_dev, so it must stay set until this returns.
+ */
+static void v4l2_subdev_disable_notify(struct v4l2_subdev *sd)
+{
+	WRITE_ONCE(sd->notify_enabled, false);
+	if (sd->v4l2_dev->notify)
+		synchronize_srcu(&v4l2_subdev_notify_srcu);
+}
+
 int __v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 				  struct v4l2_subdev *sd, struct module *module)
 {
@@ -131,6 +149,8 @@ int __v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 		return -ENODEV;
 
 	sd->v4l2_dev = v4l2_dev;
+	/* Pairs with smp_load_acquire() in v4l2_subdev_notify() */
+	smp_store_release(&sd->notify_enabled, true);
 	/* This just returns 0 if either of the two args is NULL */
 	err = v4l2_ctrl_add_handler(v4l2_dev->ctrl_handler, sd->ctrl_handler,
 				    NULL, true);
@@ -165,6 +185,7 @@ int __v4l2_device_register_subdev(struct v4l2_device *v4l2_dev,
 	media_device_unregister_entity(&sd->entity);
 #endif
 error_module:
+	v4l2_subdev_disable_notify(sd);
 	if (!sd->owner_v4l2_dev)
 		module_put(sd->owner);
 	sd->v4l2_dev = NULL;
@@ -274,6 +295,8 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
 	list_del(&sd->list);
 	spin_unlock(&v4l2_dev->lock);
 
+	v4l2_subdev_disable_notify(sd);
+
 	if (sd->internal_ops && sd->internal_ops->unregistered)
 		sd->internal_ops->unregistered(sd);
 	sd->v4l2_dev = NULL;
@@ -293,3 +316,23 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd)
 		v4l2_subdev_release(sd);
 }
 EXPORT_SYMBOL_GPL(v4l2_device_unregister_subdev);
+
+void v4l2_subdev_notify(struct v4l2_subdev *sd, unsigned int notification,
+			void *arg)
+{
+	struct v4l2_device *v4l2_dev;
+	int idx;
+
+	if (!sd)
+		return;
+
+	idx = srcu_read_lock(&v4l2_subdev_notify_srcu);
+	/* Pairs with smp_store_release() in __v4l2_device_register_subdev() */
+	if (smp_load_acquire(&sd->notify_enabled)) {
+		v4l2_dev = sd->v4l2_dev;
+		if (v4l2_dev->notify)
+			v4l2_dev->notify(sd, notification, arg);
+	}
+	srcu_read_unlock(&v4l2_subdev_notify_srcu, idx);
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_notify);
diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
index 25f69b1b8db03..cd3883e911840 100644
--- a/include/media/v4l2-device.h
+++ b/include/media/v4l2-device.h
@@ -234,13 +234,16 @@ v4l2_device_register_ro_subdev_nodes(struct v4l2_device *v4l2_dev)
  *	type is driver-specific.
  * @arg: arguments for the notification. Those are specific to each
  *	notification type.
+ *
+ * May be called from any context, including hard interrupts; the
+ * &v4l2_device.notify callback has to cope with the caller's context.
+ * Unregistering @sd waits for callbacks already running. The callback must
+ * therefore not wait for anything the unregistering thread may hold, such
+ * as a mutex held by the caller of v4l2_device_unregister_subdev() or the
+ * v4l2-async notifier lock.
  */
-static inline void v4l2_subdev_notify(struct v4l2_subdev *sd,
-				      unsigned int notification, void *arg)
-{
-	if (sd && sd->v4l2_dev && sd->v4l2_dev->notify)
-		sd->v4l2_dev->notify(sd, notification, arg);
-}
+void v4l2_subdev_notify(struct v4l2_subdev *sd, unsigned int notification,
+			void *arg);
 
 /**
  * v4l2_device_supports_requests - Test if requests are supported.
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d256b7ec8f848..c1483a85d0c72 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -997,6 +997,9 @@ struct v4l2_subdev_platform_data {
  * @owner: The owner is the same as the driver's &struct device owner.
  * @owner_v4l2_dev: true if the &sd->owner matches the owner of @v4l2_dev->dev
  *	owner. Initialized by v4l2_device_register_subdev().
+ * @notify_enabled: v4l2_subdev_notify() reaches @v4l2_dev. Set on
+ *	registration and cleared before @v4l2_dev is, see
+ *	v4l2_device_unregister_subdev().
  * @flags: subdev flags. Can be:
  *   %V4L2_SUBDEV_FL_IS_I2C - Set this flag if this subdev is a i2c device;
  *   %V4L2_SUBDEV_FL_IS_SPI - Set this flag if this subdev is a spi device;
@@ -1054,6 +1057,7 @@ struct v4l2_subdev {
 	struct list_head list;
 	struct module *owner;
 	bool owner_v4l2_dev;
+	bool notify_enabled;
 	u32 flags;
 	struct v4l2_device *v4l2_dev;
 	const struct v4l2_subdev_ops *ops;

-- 
2.47.3

[PATCH v2 8/9] media: v4l2-subdev: notify the bridge when the source power changes

[Sascha Hauer]

Whether a source is driving the connector's +5V line is published as
V4L2_CID_DV_RX_POWER_PRESENT and nowhere else. adv7604, adv7842,
tc358743 and tda1997x all set that control from their cable detect
interrupt and stop there, so a driver that has to act on the change in
hardware, rather than hand the value to userspace, has nothing to hook.

The in-kernel channel already exists for the neighbouring events. A
format change goes out through v4l2_subdev_notify_event() as
V4L2_EVENT_SOURCE_CHANGE, and adv7604 sends a private ADV76XX_HOTPLUG
when the bridge has to drive HPD back out at the connector, which cobalt
consumes. Only the source power state is missing.

Add a generic notification for it. The argument carries the same
per-input mask as the control, so a device in front of a receiver can
pass on what it sees without the receiver having to reach into its
control handler.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/media/v4l2-subdev.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index c1483a85d0c72..a3ee4866cce69 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -29,6 +29,14 @@
 
 #define	V4L2_DEVICE_NOTIFY_EVENT		_IOW('v', 2, struct v4l2_event)
 
+/*
+ * The source power state a receiver publishes as
+ * V4L2_CID_DV_RX_POWER_PRESENT has changed. The argument points at an
+ * unsigned int holding the same per-input mask as that control, so zero
+ * means no input sees a source.
+ */
+#define	V4L2_DEVICE_NOTIFY_RX_POWER_PRESENT	_IOW('v', 3, unsigned int)
+
 struct v4l2_device;
 struct v4l2_ctrl_handler;
 struct v4l2_event;

-- 
2.47.3

[PATCH v2 9/9] media: synopsys: hdmirx: get the 5V state from the upstream subdev

[Sascha Hauer]

From: Gerald Loacker <gerald.loacker@wolfvision.net>

On a board whose HDMI connector belongs to a device in front of this
receiver, the connector's +5V line goes to that device and hpd-gpios is
absent. tx_5v_power_present() then reads a NULL descriptor, which
gpiod_get_value_cansleep() reports as zero, so the receiver never sees a
source and hdmirx_plugin() never runs.

Ask the device in front instead. Remember the subdev bound through our
async notifier in source_sd, and without a GPIO of our own have
tx_5v_power_present() call its g_input_status(), taking
V4L2_IN_ST_NO_POWER as "no 5V". There is nothing to debounce here; the
subdev does that on its side of the connector. A subdev without that op
is bound anyway, with a warning: a receiver that sees no source beats
one that refuses to probe.

Changes reach us the way they do on the GPIO path, as an edge that makes
the hotplug worker look again. Once the subdev is bound its
sd->v4l2_dev is ours, so v4l2_subdev_notify() lands in the callback
installed here, and V4L2_DEVICE_NOTIFY_RX_POWER_PRESENT stands in for
the 5V interrupt this board does not have. The value the notification
carries is not used. The worker asks the subdev itself, so nothing is
cached that could go stale against bind, unbind, or a notification that
arrives early or late. Bind and unbind queue the work as well, for a
source that was connected all along and for one that leaves with the
subdev. With a GPIO of our own, notifications are ignored.

source_sd is set and cleared under work_lock. Every caller of
tx_5v_power_present() already holds it except port_no_link(), which
VIDIOC_QUERY_DV_TIMINGS reaches without it; take it there.
hdmirx_notify() reads sd->v4l2_dev, which relies on
v4l2_device_unregister_subdev() waiting for running callbacks before it
clears the pointer, see "media: v4l2-device: wait for notifications
when unregistering a subdev".

Without a det_irq there is nothing to disable_irq() across the cancel in
hdmirx_disable_irq(), so a notification could queue the hotplug work
right afterwards - including from hdmirx_suspend(), on its way to gating
the clocks. Disable the work items instead of cancelling them.
disable_delayed_work_sync() also turns every later attempt to queue them
into a no-op, whoever makes it, and hdmirx_enable_irq() enables them
again. They start out disabled at probe, which keeps the worker queued
by hdmirx_fwnode_bound() off the hardware before the EDID is written.
It also covers the way out: hdmirx_fwnode_unbind() queues the work from
inside v4l2_async_nf_unregister(), in hdmirx_remove() and on the probe
error path, just before the devm allocated hdmirx_dev goes away, and
that queue is now ignored.

Assisted-by: Claude:claude-opus-5
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c   | 101 +++++++++++++++++++--
 1 file changed, 95 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 4b94e35c912c1..5fa276706a6e6 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -142,6 +142,7 @@ struct snps_hdmirx_dev {
 	struct mutex phy_rw_lock; /* to protect phy r/w configuration */
 	struct mutex stream_lock; /* to lock video stream capture */
 	struct mutex work_lock; /* to lock the critical section of hotplug event */
+	struct v4l2_subdev *source_sd; /* under work_lock: 5V without hpd-gpios */
 	struct reset_control_bulk_data resets[HDMIRX_NUM_RST];
 	struct clk_bulk_data *clks;
 	struct regmap *grf;
@@ -237,6 +238,23 @@ static bool tx_5v_power_present(struct snps_hdmirx_dev *hdmirx_dev)
 	int val, i, cnt = 0;
 	bool ret;
 
+	/*
+	 * Without a GPIO of our own the connector belongs to the subdev in
+	 * front of us. Ask it; it debounces on its side of the connector.
+	 */
+	if (!hdmirx_dev->detect_5v_gpio) {
+		u32 status;
+
+		lockdep_assert_held(&hdmirx_dev->work_lock);
+
+		if (!hdmirx_dev->source_sd ||
+		    v4l2_subdev_call(hdmirx_dev->source_sd, video,
+				     g_input_status, &status))
+			return false;
+
+		return !(status & V4L2_IN_ST_NO_POWER);
+	}
+
 	for (i = 0; i < 10; i++) {
 		usleep_range(1000, 1100);
 		val = gpiod_get_value_cansleep(hdmirx_dev->detect_5v_gpio);
@@ -465,7 +483,13 @@ static int hdmirx_get_detected_timings(struct snps_hdmirx_dev *hdmirx_dev,
 
 static bool port_no_link(struct snps_hdmirx_dev *hdmirx_dev)
 {
-	return !tx_5v_power_present(hdmirx_dev);
+	bool present;
+
+	mutex_lock(&hdmirx_dev->work_lock);
+	present = tx_5v_power_present(hdmirx_dev);
+	mutex_unlock(&hdmirx_dev->work_lock);
+
+	return !present;
 }
 
 static int hdmirx_query_dv_timings(struct file *file, void *priv,
@@ -2272,13 +2296,22 @@ static void hdmirx_delayed_work_res_change(struct work_struct *work)
 	mutex_unlock(&hdmirx_dev->work_lock);
 }
 
-static irqreturn_t hdmirx_5v_det_irq_handler(int irq, void *dev_id)
+/*
+ * A 5V edge. Neither source of one says more than "look again": the
+ * hotplug worker samples tx_5v_power_present() and acts on what it finds.
+ */
+static void hdmirx_5v_edge(struct snps_hdmirx_dev *hdmirx_dev)
 {
-	struct snps_hdmirx_dev *hdmirx_dev = dev_id;
-
 	queue_delayed_work(system_dfl_wq,
 			   &hdmirx_dev->delayed_work_hotplug,
 			   msecs_to_jiffies(10));
+}
+
+static irqreturn_t hdmirx_5v_det_irq_handler(int irq, void *dev_id)
+{
+	struct snps_hdmirx_dev *hdmirx_dev = dev_id;
+
+	hdmirx_5v_edge(hdmirx_dev);
 
 	return IRQ_HANDLED;
 }
@@ -2530,14 +2563,18 @@ static void hdmirx_disable_irq(struct device *dev)
 	disable_irq(hdmirx_dev->dma_irq);
 	disable_irq(hdmirx_dev->hdmi_irq);
 
-	cancel_delayed_work_sync(&hdmirx_dev->delayed_work_hotplug);
-	cancel_delayed_work_sync(&hdmirx_dev->delayed_work_res_change);
+	/* A subdev in front of us can still notify, keep it from queueing. */
+	disable_delayed_work_sync(&hdmirx_dev->delayed_work_hotplug);
+	disable_delayed_work_sync(&hdmirx_dev->delayed_work_res_change);
 }
 
 static void hdmirx_enable_irq(struct device *dev)
 {
 	struct snps_hdmirx_dev *hdmirx_dev = dev_get_drvdata(dev);
 
+	enable_delayed_work(&hdmirx_dev->delayed_work_hotplug);
+	enable_delayed_work(&hdmirx_dev->delayed_work_res_change);
+
 	enable_irq(hdmirx_dev->hdmi_irq);
 	enable_irq(hdmirx_dev->dma_irq);
 	if (hdmirx_dev->det_irq > 0)
@@ -2673,6 +2710,19 @@ static int hdmirx_register_cec(struct snps_hdmirx_dev *hdmirx_dev,
 	return 0;
 }
 
+static void hdmirx_notify(struct v4l2_subdev *sd, unsigned int notification,
+			  void *arg)
+{
+	struct snps_hdmirx_dev *hdmirx_dev =
+		container_of(sd->v4l2_dev, struct snps_hdmirx_dev, v4l2_dev);
+
+	if (notification != V4L2_DEVICE_NOTIFY_RX_POWER_PRESENT ||
+	    hdmirx_dev->detect_5v_gpio)
+		return;
+
+	hdmirx_5v_edge(hdmirx_dev);
+}
+
 static int hdmirx_fwnode_bound(struct v4l2_async_notifier *notifier,
 			       struct v4l2_subdev *subdev,
 			       struct v4l2_async_connection *asc)
@@ -2701,6 +2751,21 @@ static int hdmirx_fwnode_bound(struct v4l2_async_notifier *notifier,
 		return ret;
 	}
 
+	if (hdmirx_dev->detect_5v_gpio)
+		return 0;
+
+	if (!v4l2_subdev_has_op(subdev, video, g_input_status))
+		dev_warn(hdmirx_dev->dev,
+			 "%s cannot report 5V and there is no hpd-gpios, no source will be detected\n",
+			 subdev->name);
+
+	mutex_lock(&hdmirx_dev->work_lock);
+	hdmirx_dev->source_sd = subdev;
+	mutex_unlock(&hdmirx_dev->work_lock);
+
+	/* The source may have been connected all along. */
+	hdmirx_5v_edge(hdmirx_dev);
+
 	return 0;
 }
 
@@ -2712,8 +2777,28 @@ static int hdmirx_fwnode_complete(struct v4l2_async_notifier *notifier)
 	return v4l2_device_register_subdev_nodes(&hdmirx_dev->v4l2_dev);
 }
 
+static void hdmirx_fwnode_unbind(struct v4l2_async_notifier *notifier,
+				 struct v4l2_subdev *subdev,
+				 struct v4l2_async_connection *asc)
+{
+	struct snps_hdmirx_dev *hdmirx_dev =
+		container_of(notifier, struct snps_hdmirx_dev, notifier);
+
+	/* With a GPIO of our own the connector is ours and stays put. */
+	if (hdmirx_dev->detect_5v_gpio)
+		return;
+
+	mutex_lock(&hdmirx_dev->work_lock);
+	hdmirx_dev->source_sd = NULL;
+	mutex_unlock(&hdmirx_dev->work_lock);
+
+	/* The source went with it. */
+	hdmirx_5v_edge(hdmirx_dev);
+}
+
 static const struct v4l2_async_notifier_operations hdmirx_async_ops = {
 	.bound = hdmirx_fwnode_bound,
+	.unbind = hdmirx_fwnode_unbind,
 	.complete = hdmirx_fwnode_complete,
 };
 
@@ -2770,6 +2855,9 @@ static int hdmirx_probe(struct platform_device *pdev)
 			  hdmirx_delayed_work_hotplug);
 	INIT_DELAYED_WORK(&hdmirx_dev->delayed_work_res_change,
 			  hdmirx_delayed_work_res_change);
+	/* Until hdmirx_enable_irq(), after the EDID is written. */
+	disable_delayed_work(&hdmirx_dev->delayed_work_hotplug);
+	disable_delayed_work(&hdmirx_dev->delayed_work_res_change);
 
 	hdmirx_dev->cur_fmt_fourcc = V4L2_PIX_FMT_BGR24;
 	hdmirx_dev->timings = cea640x480;
@@ -2808,6 +2896,7 @@ static int hdmirx_probe(struct platform_device *pdev)
 		goto err_pm;
 	}
 	hdmirx_dev->v4l2_dev.ctrl_handler = hdl;
+	hdmirx_dev->v4l2_dev.notify = hdmirx_notify;
 
 	ret = v4l2_device_register(dev, &hdmirx_dev->v4l2_dev);
 	if (ret < 0) {

-- 
2.47.3

Re: [PATCH v2 3/9] dt-bindings: media: snps,dw-hdmi-rx: add a port for a bridge in front

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]