* [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-06-19 5:26 [RFC PATCH 0/6] Add Shared Media Graph API Paul Elder
@ 2026-06-19 5:26 ` Paul Elder
2026-06-24 4:47 ` Paul Elder
` (3 more replies)
2026-06-19 5:26 ` [RFC PATCH 2/6] arm64: dts: rockchip: rk3588s-base: Connect vicap and isps Paul Elder
` (4 subsequent siblings)
5 siblings, 4 replies; 14+ messages in thread
From: Paul Elder @ 2026-06-19 5:26 UTC (permalink / raw)
To: laurent.pinchart
Cc: Paul Elder, michael.riesch, xuhf, stefan.klug, kieran.bingham,
dan.scally, jacopo.mondi, linux-media, linux-arm-kernel,
linux-rockchip, linux-kernel, hverkuil+cisco, nicolas.dufresne,
ribalda, sakari.ailus
Currently, a media graph contains a main device whose driver is
responsible for creating the media device. We have however recently run
into devices that have multiple devices that can quality as a main
device. Examples are the RK3588 which has a VICAP and two ISP
instances, and another example is the i.MX8MP which has an ISI and two
ISP instances. As there is currently no way to reconcile who the main
device is in the media device, these setups simple cannot be used
simultaneously.
This patch extends the media controller API with a "shared media graph"
framework. This allows drivers to share a media device, thus enabling
the setups mentioned above. Instead of owning and creating a media
device, drivers can join-or-create a shared media device via the shared
media graph API. The matching is done automatically based on the
detected endpoints in the device tree.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
drivers/media/mc/Makefile | 2 +-
drivers/media/mc/mc-shared-graph.c | 335 +++++++++++++++++++++++++++++
include/media/mc-shared-graph.h | 92 ++++++++
3 files changed, 428 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/mc/mc-shared-graph.c
create mode 100644 include/media/mc-shared-graph.h
diff --git a/drivers/media/mc/Makefile b/drivers/media/mc/Makefile
index 2b7af42ba59c..1d502fdc52ad 100644
--- a/drivers/media/mc/Makefile
+++ b/drivers/media/mc/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
mc-objs := mc-device.o mc-devnode.o mc-entity.o \
- mc-request.o
+ mc-request.o mc-shared-graph.o
ifneq ($(CONFIG_USB),)
mc-objs += mc-dev-allocator.o
diff --git a/drivers/media/mc/mc-shared-graph.c b/drivers/media/mc/mc-shared-graph.c
new file mode 100644
index 000000000000..c4067e5b861d
--- /dev/null
+++ b/drivers/media/mc/mc-shared-graph.c
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mc-shared-graph.c - Media Controller Shared Graph API
+ *
+ * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
+ */
+
+/*
+ * This file adds the Media Controller Shared Graph API. This allows drivers
+ * to create shared media graphs or join existing media graphs from other
+ * drivers, so that they can all be in the same media graph. This allows us to
+ * have more complex media graphs chaining more complex hardware together,
+ * instead of simple async subdevs.
+ */
+
+#include <linux/device.h>
+#include <linux/fwnode.h>
+#include <linux/kref.h>
+#include <linux/property.h>
+
+#include <media/media-device.h>
+
+#include <media/mc-shared-graph.h>
+
+static LIST_HEAD(media_device_shared_list);
+static DEFINE_MUTEX(media_device_shared_lock);
+
+struct media_device_shared_member {
+ struct device *dev;
+ struct fwnode_handle *fwnode;
+ struct list_head list;
+};
+
+struct media_device_shared_link {
+ struct media_entity *source;
+ u16 source_pad;
+ struct media_entity *sink;
+ u16 sink_pad;
+ u32 flags;
+ struct list_head list;
+};
+
+// TODO figure out locking for when multiple drivers touch the media graph;
+// maybe macros for shared versions?
+struct media_device_shared {
+ struct media_device mdev;
+ struct list_head members;
+ struct list_head links;
+
+ struct list_head list;
+ struct kref refcount;
+
+ struct device *removed_device;
+};
+
+static inline struct media_device_shared *
+to_media_device_shared(struct media_device *mdev)
+{
+ return container_of(mdev, struct media_device_shared, mdev);
+}
+
+static void media_device_shared_release(struct kref *kref)
+{
+ struct media_device_shared *mds =
+ container_of(kref, struct media_device_shared, refcount);
+
+ dev_dbg(mds->removed_device, "%s: releasing Media Device\n", __func__);
+
+ mutex_lock(&media_device_shared_lock);
+
+ media_device_unregister(&mds->mdev);
+ media_device_cleanup(&mds->mdev);
+
+ list_del(&mds->list);
+ mutex_unlock(&media_device_shared_lock);
+
+ kfree(mds);
+}
+
+/* Callers should hold media_device_shared_lock when calling this function */
+static bool __media_device_shared_find_match(struct media_device_shared *mds,
+ struct fwnode_handle *fwnode)
+{
+ struct media_device_shared_member *member;
+ struct fwnode_handle *ep;
+ struct fwnode_handle *remote_ep;
+ bool match = false;
+
+ // TODO: parse the device tree endpoints graph instead of finding just the
+ // first-level neighbours
+ fwnode_graph_for_each_endpoint(fwnode, ep) {
+ list_for_each_entry(member, &mds->members, list) {
+ remote_ep = fwnode_graph_get_remote_port_parent(ep);
+ match = (member->fwnode == remote_ep);
+ fwnode_handle_put(remote_ep);
+
+ if (!match)
+ continue;
+
+ goto match_complete;
+ }
+ }
+
+match_complete:
+ fwnode_handle_put(ep);
+ return match;
+}
+
+/* Callers should hold media_device_shared_lock when calling this function */
+static struct media_device *__media_device_shared_get(struct device *dev)
+{
+ struct media_device_shared *mds;
+ struct media_device_shared_member *member;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
+ bool ret;
+
+ dev_dbg(dev, "%s: searching for media device for %pfwf", __func__, fwnode);
+
+ list_for_each_entry(mds, &media_device_shared_list, list) {
+ ret = __media_device_shared_find_match(mds, fwnode);
+ if (ret)
+ break;
+ }
+
+ if (!ret)
+ return NULL;
+
+ member = kzalloc_obj(*member);
+ if (!member)
+ return NULL;
+
+ member->dev = dev;
+ member->fwnode = fwnode;
+ list_add_tail(&member->list, &mds->members);
+ kref_get(&mds->refcount);
+
+ dev_dbg(dev, "%s: %pfwf joined media device of %pfwf",
+ __func__, fwnode,
+ list_first_entry(&mds->members, struct media_device_shared_member, list)->fwnode);
+
+ return &mds->mdev;
+}
+
+/* Callers should hold media_device_shared_lock when calling this function */
+static struct media_device *__media_device_shared_create(struct device *dev)
+{
+ struct media_device_shared *mds;
+ struct media_device_shared_member *member;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
+ int ret;
+
+ mds = kzalloc_obj(*mds);
+ if (!mds)
+ return NULL;
+
+ member = kzalloc_obj(*member);
+ if (!member)
+ goto err_free_mds;
+
+ media_device_init(&mds->mdev);
+
+ ret = media_device_register(&mds->mdev);
+ if (ret)
+ goto err_free_member;
+
+ INIT_LIST_HEAD(&mds->members);
+ member->dev = dev;
+ member->fwnode = fwnode;
+ list_add_tail(&member->list, &mds->members);
+
+ INIT_LIST_HEAD(&mds->links);
+
+ kref_init(&mds->refcount);
+ list_add_tail(&mds->list, &media_device_shared_list);
+
+ // TODO figure out how to reconcile this with multiple members
+ mds->mdev.dev = dev;
+
+ devv_dbg(dev, "%s: Allocated media device with %pfwf at %p\n",
+ __func__, fwnode, &mds->mdev);
+ return &mds->mdev;
+
+err_free_member:
+ kfree(member);
+err_free_mds:
+ kfree(mds);
+ return NULL;
+}
+
+// TODO figure out how to resolve the identifiers (model, driver name, etc);
+// atm it's racy and whoever gets it last wins
+struct media_device *media_device_shared_join(struct device *dev)
+{
+ struct media_device *mdev;
+
+ mutex_lock(&media_device_shared_lock);
+
+ mdev = __media_device_shared_get(dev);
+ if (!!mdev) {
+ dev_dbg(dev, "%s: found media device for %pfwf", __func__, dev_fwnode(dev));
+ mutex_unlock(&media_device_shared_lock);
+ return mdev;
+ }
+
+ mdev = __media_device_shared_create(dev);
+ if (!mdev) {
+ dev_warn(dev, "%s: failed to create media device for %pfwf", __func__, dev_fwnode(dev));
+ mutex_unlock(&media_device_shared_lock);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ dev_dbg(dev, "%s: created media device for %pfwf", __func__, dev_fwnode(dev));
+ mutex_unlock(&media_device_shared_lock);
+ return mdev;
+}
+EXPORT_SYMBOL_GPL(media_device_shared_join);
+
+void media_device_shared_leave(struct media_device *mdev, struct device *dev)
+{
+ struct media_device_shared *mds = to_media_device_shared(mdev);
+ struct media_device_shared_member *member;
+ struct media_device_shared_member *member_tmp;
+ bool removed = false;
+
+ mutex_lock(&media_device_shared_lock);
+
+ list_for_each_entry_safe(member, member_tmp, &mds->members, list) {
+ if (member->dev == dev) {
+ list_del(&member->list);
+ kfree(member);
+ removed = true;
+ }
+ }
+
+ if (!removed)
+ dev_err(dev, "%s: %pfwf trying to leave from graph in which not a member",
+ __func__, dev_fwnode(dev));
+
+ mds->removed_device = dev;
+ mutex_unlock(&media_device_shared_lock);
+ kref_put(&mds->refcount, media_device_shared_release);
+}
+EXPORT_SYMBOL_GPL(media_device_shared_leave);
+
+int media_device_shared_join_link_source(struct media_device *mdev,
+ struct device *dev,
+ struct media_entity *source,
+ u16 source_pad, u32 flags)
+{
+ struct media_device_shared *mds = to_media_device_shared(mdev);
+ struct media_device_shared_link *link;
+ struct media_device_shared_link *link_tmp;
+ int ret = 0;
+
+ mutex_lock(&media_device_shared_lock);
+
+ /*
+ * TODO Figure out flags. Should we use greatest common denominator? Or
+ * prioritize sink? Or whoever wins the race? For now we just take the flags
+ * from the sink.
+ *
+ * TODO Figure out how to actually do the matching. For now we just match
+ * whoever comes in first. This works with the simple example we're running
+ * with now (rkcif + one rkisp2) but with setups with multiple copies of
+ * hardware this will cause problems, like with rkcif + two rkisp2 and
+ * imx8-isi + two rkisp1.
+ */
+ list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
+ if (link->sink) {
+ ret = media_create_pad_link(source, source_pad,
+ link->sink, link->sink_pad,
+ link->flags);
+ list_del(&link->list);
+ kfree(link);
+ goto exit_join_link_source;
+ }
+ }
+
+ link = kzalloc_obj(*link);
+ if (!link) {
+ ret = -ENOMEM;
+ goto exit_join_link_source;
+ }
+
+ link->source = source;
+ link->source_pad = source_pad;
+ link->flags = flags;
+ list_add_tail(&link->list, &mds->links);
+
+exit_join_link_source:
+ mutex_unlock(&media_device_shared_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(media_device_shared_join_link_source);
+
+// TODO deduplicate from above
+int media_device_shared_join_link_sink(struct media_device *mdev,
+ struct device *dev,
+ struct media_entity *sink,
+ u16 sink_pad, u32 flags)
+{
+ struct media_device_shared *mds = to_media_device_shared(mdev);
+ struct media_device_shared_link *link;
+ struct media_device_shared_link *link_tmp;
+ int ret = 0;
+
+ mutex_lock(&media_device_shared_lock);
+
+ list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
+ if (link->source) {
+ ret = media_create_pad_link(link->source, link->source_pad,
+ sink, sink_pad,
+ flags);
+ list_del(&link->list);
+ kfree(link);
+ goto exit_join_link_sink;
+ }
+ }
+
+ link = kzalloc_obj(*link);
+ if (!link) {
+ ret = -ENOMEM;
+ goto exit_join_link_sink;
+ }
+
+ link->sink = sink;
+ link->sink_pad = sink_pad;
+ link->flags = flags;
+ list_add_tail(&link->list, &mds->links);
+
+exit_join_link_sink:
+ mutex_unlock(&media_device_shared_lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(media_device_shared_join_link_sink);
diff --git a/include/media/mc-shared-graph.h b/include/media/mc-shared-graph.h
new file mode 100644
index 000000000000..487325163f84
--- /dev/null
+++ b/include/media/mc-shared-graph.h
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mc-shared-graph.h - Media Controller Shared Graph API
+ *
+ * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
+ */
+
+/*
+ * This file adds the Media Controller Shared Graph API. This allows drivers
+ * to create shared media graphs or join existing media graphs from other
+ * drivers, so that they can all be in the same media graph. This allows us to
+ * have more complex media graphs chaining more complex hardware together,
+ * instead of simple async subdevs.
+ */
+
+#include <linux/types.h>
+
+#ifndef _MEDIA_SHARED_GRAPH_H
+#define _MEDIA_SHARED_GRAPH_H
+
+struct device;
+struct media_device;
+struct media_entity;
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+/**
+ * media_device_shared_join() - Join or create a new shared media device
+ *
+ * @dev: struct &device pointer
+ *
+ * This is the entrance function for a device to join or create a new shared
+ * media device. It searches for an existing shared media device based on the
+ * neighbours in the device's device tree ports node. If found, then this
+ * functions returns the existing shared media device and joins it. If one is
+ * not found then one is created and initialized and returned.
+ */
+struct media_device *media_device_shared_join(struct device *dev);
+
+/**
+ * media_device_shared_leave() - Leave the shared media device.
+ *
+ * @mdev: struct &media_device pointer
+ * @dev: struct &device pointer
+ *
+ * This function makes the device leave the shared media device. When all
+ * members have left the media device it will be freed.
+ */
+void media_device_shared_leave(struct media_device *mdev, struct device *dev);
+
+/**
+ * media_device_shared_join_link_source() - Register a link source in the shared media device
+ *
+ * @mdev: The struct &media_device pointer that is part of a shared media device
+ * @dev: struct &device pointer
+ * @source: The link source
+ * @source_pad: The pad
+ * @flags: The flags
+ *
+ * This function registers with the shared media device the source part of a
+ * link. When the shared media device receives the matching sink part of a link
+ * via media_device_shared_join_link_sink() then the link will be fully created.
+ */
+int media_device_shared_join_link_source(struct media_device *mdev,
+ struct device *dev,
+ struct media_entity *source,
+ u16 source_pad, u32 flags);
+
+/**
+ * media_device_shared_join_link_sink() - Register a link sink in the shared media device
+ *
+ * Same as media_device_shared_join_link_source() but for sink instead of
+ * source.
+ */
+int media_device_shared_join_link_sink(struct media_device *mdev,
+ struct device *dev,
+ struct media_entity *sink,
+ u16 sink_pad, u32 flags);
+#else
+static inline struct media_device *media_device_shared_join(struct device *dev)
+{ return NULL; }
+static inline void media_device_shared_leave(struct media_device *mdev,
+ struct device *dev) { }
+static inline int media_device_shared_join_link_source(struct media_device *mdev,
+ struct device *dev,
+ struct media_entity *source,
+ u16 source_pad, u32 flags) { }
+static inline int media_device_shared_join_link_sink(struct media_device *mdev,
+ struct device *dev,
+ struct media_entity *sink,
+ u16 sink_pad, u32 flags) { }
+#endif /* CONFIG_MEDIA_CONTROLLER */
+#endif /* _MEDIA_DEV_SHARED_GRAPH_H */
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-06-19 5:26 ` [RFC PATCH 1/6] media: mc: Implement shared media graph Paul Elder
@ 2026-06-24 4:47 ` Paul Elder
2026-06-24 10:39 ` Kieran Bingham
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Paul Elder @ 2026-06-24 4:47 UTC (permalink / raw)
To: laurent.pinchart
Cc: michael.riesch, xuhf, stefan.klug, kieran.bingham, dan.scally,
jacopo.mondi, linux-media, linux-arm-kernel, linux-rockchip,
linux-kernel, hverkuil+cisco, nicolas.dufresne, ribalda,
sakari.ailus
Hi me,
You have a typo...
Quoting Paul Elder (2026-06-19 14:26:28)
> Currently, a media graph contains a main device whose driver is
> responsible for creating the media device. We have however recently run
> into devices that have multiple devices that can quality as a main
> device. Examples are the RK3588 which has a VICAP and two ISP
> instances, and another example is the i.MX8MP which has an ISI and two
> ISP instances. As there is currently no way to reconcile who the main
> device is in the media device, these setups simple cannot be used
> simultaneously.
>
> This patch extends the media controller API with a "shared media graph"
> framework. This allows drivers to share a media device, thus enabling
> the setups mentioned above. Instead of owning and creating a media
> device, drivers can join-or-create a shared media device via the shared
> media graph API. The matching is done automatically based on the
> detected endpoints in the device tree.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> drivers/media/mc/Makefile | 2 +-
> drivers/media/mc/mc-shared-graph.c | 335 +++++++++++++++++++++++++++++
> include/media/mc-shared-graph.h | 92 ++++++++
> 3 files changed, 428 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/mc/mc-shared-graph.c
> create mode 100644 include/media/mc-shared-graph.h
>
> diff --git a/drivers/media/mc/Makefile b/drivers/media/mc/Makefile
> index 2b7af42ba59c..1d502fdc52ad 100644
> --- a/drivers/media/mc/Makefile
> +++ b/drivers/media/mc/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> mc-objs := mc-device.o mc-devnode.o mc-entity.o \
> - mc-request.o
> + mc-request.o mc-shared-graph.o
>
> ifneq ($(CONFIG_USB),)
> mc-objs += mc-dev-allocator.o
> diff --git a/drivers/media/mc/mc-shared-graph.c b/drivers/media/mc/mc-shared-graph.c
> new file mode 100644
> index 000000000000..c4067e5b861d
> --- /dev/null
> +++ b/drivers/media/mc/mc-shared-graph.c
> @@ -0,0 +1,335 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mc-shared-graph.c - Media Controller Shared Graph API
> + *
> + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> + */
> +
> +/*
> + * This file adds the Media Controller Shared Graph API. This allows drivers
> + * to create shared media graphs or join existing media graphs from other
> + * drivers, so that they can all be in the same media graph. This allows us to
> + * have more complex media graphs chaining more complex hardware together,
> + * instead of simple async subdevs.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/fwnode.h>
> +#include <linux/kref.h>
> +#include <linux/property.h>
> +
> +#include <media/media-device.h>
> +
> +#include <media/mc-shared-graph.h>
> +
> +static LIST_HEAD(media_device_shared_list);
> +static DEFINE_MUTEX(media_device_shared_lock);
> +
> +struct media_device_shared_member {
> + struct device *dev;
> + struct fwnode_handle *fwnode;
> + struct list_head list;
> +};
> +
> +struct media_device_shared_link {
> + struct media_entity *source;
> + u16 source_pad;
> + struct media_entity *sink;
> + u16 sink_pad;
> + u32 flags;
> + struct list_head list;
> +};
> +
> +// TODO figure out locking for when multiple drivers touch the media graph;
> +// maybe macros for shared versions?
> +struct media_device_shared {
> + struct media_device mdev;
> + struct list_head members;
> + struct list_head links;
> +
> + struct list_head list;
> + struct kref refcount;
> +
> + struct device *removed_device;
> +};
> +
> +static inline struct media_device_shared *
> +to_media_device_shared(struct media_device *mdev)
> +{
> + return container_of(mdev, struct media_device_shared, mdev);
> +}
> +
> +static void media_device_shared_release(struct kref *kref)
> +{
> + struct media_device_shared *mds =
> + container_of(kref, struct media_device_shared, refcount);
> +
> + dev_dbg(mds->removed_device, "%s: releasing Media Device\n", __func__);
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + media_device_unregister(&mds->mdev);
> + media_device_cleanup(&mds->mdev);
> +
> + list_del(&mds->list);
> + mutex_unlock(&media_device_shared_lock);
> +
> + kfree(mds);
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static bool __media_device_shared_find_match(struct media_device_shared *mds,
> + struct fwnode_handle *fwnode)
> +{
> + struct media_device_shared_member *member;
> + struct fwnode_handle *ep;
> + struct fwnode_handle *remote_ep;
> + bool match = false;
> +
> + // TODO: parse the device tree endpoints graph instead of finding just the
> + // first-level neighbours
> + fwnode_graph_for_each_endpoint(fwnode, ep) {
> + list_for_each_entry(member, &mds->members, list) {
> + remote_ep = fwnode_graph_get_remote_port_parent(ep);
> + match = (member->fwnode == remote_ep);
> + fwnode_handle_put(remote_ep);
> +
> + if (!match)
> + continue;
> +
> + goto match_complete;
> + }
> + }
> +
> +match_complete:
> + fwnode_handle_put(ep);
> + return match;
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static struct media_device *__media_device_shared_get(struct device *dev)
> +{
> + struct media_device_shared *mds;
> + struct media_device_shared_member *member;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + bool ret;
> +
> + dev_dbg(dev, "%s: searching for media device for %pfwf", __func__, fwnode);
> +
> + list_for_each_entry(mds, &media_device_shared_list, list) {
> + ret = __media_device_shared_find_match(mds, fwnode);
> + if (ret)
> + break;
> + }
> +
> + if (!ret)
> + return NULL;
> +
> + member = kzalloc_obj(*member);
> + if (!member)
> + return NULL;
> +
> + member->dev = dev;
> + member->fwnode = fwnode;
> + list_add_tail(&member->list, &mds->members);
> + kref_get(&mds->refcount);
> +
> + dev_dbg(dev, "%s: %pfwf joined media device of %pfwf",
> + __func__, fwnode,
> + list_first_entry(&mds->members, struct media_device_shared_member, list)->fwnode);
> +
> + return &mds->mdev;
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static struct media_device *__media_device_shared_create(struct device *dev)
> +{
> + struct media_device_shared *mds;
> + struct media_device_shared_member *member;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + int ret;
> +
> + mds = kzalloc_obj(*mds);
> + if (!mds)
> + return NULL;
> +
> + member = kzalloc_obj(*member);
> + if (!member)
> + goto err_free_mds;
> +
> + media_device_init(&mds->mdev);
> +
> + ret = media_device_register(&mds->mdev);
> + if (ret)
> + goto err_free_member;
> +
> + INIT_LIST_HEAD(&mds->members);
> + member->dev = dev;
> + member->fwnode = fwnode;
> + list_add_tail(&member->list, &mds->members);
> +
> + INIT_LIST_HEAD(&mds->links);
> +
> + kref_init(&mds->refcount);
> + list_add_tail(&mds->list, &media_device_shared_list);
> +
> + // TODO figure out how to reconcile this with multiple members
> + mds->mdev.dev = dev;
> +
> + devv_dbg(dev, "%s: Allocated media device with %pfwf at %p\n",
here:
s/devv/dev/
Paul
> + __func__, fwnode, &mds->mdev);
> + return &mds->mdev;
> +
> +err_free_member:
> + kfree(member);
> +err_free_mds:
> + kfree(mds);
> + return NULL;
> +}
> +
> +// TODO figure out how to resolve the identifiers (model, driver name, etc);
> +// atm it's racy and whoever gets it last wins
> +struct media_device *media_device_shared_join(struct device *dev)
> +{
> + struct media_device *mdev;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + mdev = __media_device_shared_get(dev);
> + if (!!mdev) {
> + dev_dbg(dev, "%s: found media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> + }
> +
> + mdev = __media_device_shared_create(dev);
> + if (!mdev) {
> + dev_warn(dev, "%s: failed to create media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + dev_dbg(dev, "%s: created media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join);
> +
> +void media_device_shared_leave(struct media_device *mdev, struct device *dev)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_member *member;
> + struct media_device_shared_member *member_tmp;
> + bool removed = false;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + list_for_each_entry_safe(member, member_tmp, &mds->members, list) {
> + if (member->dev == dev) {
> + list_del(&member->list);
> + kfree(member);
> + removed = true;
> + }
> + }
> +
> + if (!removed)
> + dev_err(dev, "%s: %pfwf trying to leave from graph in which not a member",
> + __func__, dev_fwnode(dev));
> +
> + mds->removed_device = dev;
> + mutex_unlock(&media_device_shared_lock);
> + kref_put(&mds->refcount, media_device_shared_release);
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_leave);
> +
> +int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_link *link;
> + struct media_device_shared_link *link_tmp;
> + int ret = 0;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + /*
> + * TODO Figure out flags. Should we use greatest common denominator? Or
> + * prioritize sink? Or whoever wins the race? For now we just take the flags
> + * from the sink.
> + *
> + * TODO Figure out how to actually do the matching. For now we just match
> + * whoever comes in first. This works with the simple example we're running
> + * with now (rkcif + one rkisp2) but with setups with multiple copies of
> + * hardware this will cause problems, like with rkcif + two rkisp2 and
> + * imx8-isi + two rkisp1.
> + */
> + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> + if (link->sink) {
> + ret = media_create_pad_link(source, source_pad,
> + link->sink, link->sink_pad,
> + link->flags);
> + list_del(&link->list);
> + kfree(link);
> + goto exit_join_link_source;
> + }
> + }
> +
> + link = kzalloc_obj(*link);
> + if (!link) {
> + ret = -ENOMEM;
> + goto exit_join_link_source;
> + }
> +
> + link->source = source;
> + link->source_pad = source_pad;
> + link->flags = flags;
> + list_add_tail(&link->list, &mds->links);
> +
> +exit_join_link_source:
> + mutex_unlock(&media_device_shared_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join_link_source);
> +
> +// TODO deduplicate from above
> +int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_link *link;
> + struct media_device_shared_link *link_tmp;
> + int ret = 0;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> + if (link->source) {
> + ret = media_create_pad_link(link->source, link->source_pad,
> + sink, sink_pad,
> + flags);
> + list_del(&link->list);
> + kfree(link);
> + goto exit_join_link_sink;
> + }
> + }
> +
> + link = kzalloc_obj(*link);
> + if (!link) {
> + ret = -ENOMEM;
> + goto exit_join_link_sink;
> + }
> +
> + link->sink = sink;
> + link->sink_pad = sink_pad;
> + link->flags = flags;
> + list_add_tail(&link->list, &mds->links);
> +
> +exit_join_link_sink:
> + mutex_unlock(&media_device_shared_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join_link_sink);
> diff --git a/include/media/mc-shared-graph.h b/include/media/mc-shared-graph.h
> new file mode 100644
> index 000000000000..487325163f84
> --- /dev/null
> +++ b/include/media/mc-shared-graph.h
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mc-shared-graph.h - Media Controller Shared Graph API
> + *
> + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> + */
> +
> +/*
> + * This file adds the Media Controller Shared Graph API. This allows drivers
> + * to create shared media graphs or join existing media graphs from other
> + * drivers, so that they can all be in the same media graph. This allows us to
> + * have more complex media graphs chaining more complex hardware together,
> + * instead of simple async subdevs.
> + */
> +
> +#include <linux/types.h>
> +
> +#ifndef _MEDIA_SHARED_GRAPH_H
> +#define _MEDIA_SHARED_GRAPH_H
> +
> +struct device;
> +struct media_device;
> +struct media_entity;
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +/**
> + * media_device_shared_join() - Join or create a new shared media device
> + *
> + * @dev: struct &device pointer
> + *
> + * This is the entrance function for a device to join or create a new shared
> + * media device. It searches for an existing shared media device based on the
> + * neighbours in the device's device tree ports node. If found, then this
> + * functions returns the existing shared media device and joins it. If one is
> + * not found then one is created and initialized and returned.
> + */
> +struct media_device *media_device_shared_join(struct device *dev);
> +
> +/**
> + * media_device_shared_leave() - Leave the shared media device.
> + *
> + * @mdev: struct &media_device pointer
> + * @dev: struct &device pointer
> + *
> + * This function makes the device leave the shared media device. When all
> + * members have left the media device it will be freed.
> + */
> +void media_device_shared_leave(struct media_device *mdev, struct device *dev);
> +
> +/**
> + * media_device_shared_join_link_source() - Register a link source in the shared media device
> + *
> + * @mdev: The struct &media_device pointer that is part of a shared media device
> + * @dev: struct &device pointer
> + * @source: The link source
> + * @source_pad: The pad
> + * @flags: The flags
> + *
> + * This function registers with the shared media device the source part of a
> + * link. When the shared media device receives the matching sink part of a link
> + * via media_device_shared_join_link_sink() then the link will be fully created.
> + */
> +int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags);
> +
> +/**
> + * media_device_shared_join_link_sink() - Register a link sink in the shared media device
> + *
> + * Same as media_device_shared_join_link_source() but for sink instead of
> + * source.
> + */
> +int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags);
> +#else
> +static inline struct media_device *media_device_shared_join(struct device *dev)
> +{ return NULL; }
> +static inline void media_device_shared_leave(struct media_device *mdev,
> + struct device *dev) { }
> +static inline int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags) { }
> +static inline int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags) { }
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> +#endif /* _MEDIA_DEV_SHARED_GRAPH_H */
> --
> 2.47.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-06-19 5:26 ` [RFC PATCH 1/6] media: mc: Implement shared media graph Paul Elder
2026-06-24 4:47 ` Paul Elder
@ 2026-06-24 10:39 ` Kieran Bingham
2026-06-25 8:23 ` Paul Elder
2026-07-10 20:23 ` Michael Riesch
2026-07-10 21:50 ` Nicolas Dufresne
3 siblings, 1 reply; 14+ messages in thread
From: Kieran Bingham @ 2026-06-24 10:39 UTC (permalink / raw)
To: Paul Elder, laurent.pinchart
Cc: Paul Elder, michael.riesch, xuhf, stefan.klug, dan.scally,
jacopo.mondi, linux-media, linux-arm-kernel, linux-rockchip,
linux-kernel, hverkuil+cisco, nicolas.dufresne, ribalda,
sakari.ailus
Hi Paul,
I'm taking a first read through, some comments inline, but not
necessarily a full review:
Quoting Paul Elder (2026-06-19 06:26:28)
> Currently, a media graph contains a main device whose driver is
> responsible for creating the media device. We have however recently run
> into devices that have multiple devices that can quality as a main
s/quality/qualify/
> device. Examples are the RK3588 which has a VICAP and two ISP
> instances, and another example is the i.MX8MP which has an ISI and two
> ISP instances. As there is currently no way to reconcile who the main
> device is in the media device, these setups simple cannot be used
> simultaneously.
I'm very excited to see how we could apply this to the NXP i.MX8MP to
resolve the ISI+ISP issue!
> This patch extends the media controller API with a "shared media graph"
> framework. This allows drivers to share a media device, thus enabling
> the setups mentioned above. Instead of owning and creating a media
> device, drivers can join-or-create a shared media device via the shared
> media graph API. The matching is done automatically based on the
> detected endpoints in the device tree.
Sounds great! I'll read on...
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> drivers/media/mc/Makefile | 2 +-
> drivers/media/mc/mc-shared-graph.c | 335 +++++++++++++++++++++++++++++
> include/media/mc-shared-graph.h | 92 ++++++++
> 3 files changed, 428 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/mc/mc-shared-graph.c
> create mode 100644 include/media/mc-shared-graph.h
>
> diff --git a/drivers/media/mc/Makefile b/drivers/media/mc/Makefile
> index 2b7af42ba59c..1d502fdc52ad 100644
> --- a/drivers/media/mc/Makefile
> +++ b/drivers/media/mc/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> mc-objs := mc-device.o mc-devnode.o mc-entity.o \
> - mc-request.o
> + mc-request.o mc-shared-graph.o
>
> ifneq ($(CONFIG_USB),)
> mc-objs += mc-dev-allocator.o
> diff --git a/drivers/media/mc/mc-shared-graph.c b/drivers/media/mc/mc-shared-graph.c
> new file mode 100644
> index 000000000000..c4067e5b861d
> --- /dev/null
> +++ b/drivers/media/mc/mc-shared-graph.c
> @@ -0,0 +1,335 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mc-shared-graph.c - Media Controller Shared Graph API
> + *
> + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> + */
> +
> +/*
> + * This file adds the Media Controller Shared Graph API. This allows drivers
> + * to create shared media graphs or join existing media graphs from other
> + * drivers, so that they can all be in the same media graph. This allows us to
> + * have more complex media graphs chaining more complex hardware together,
> + * instead of simple async subdevs.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/fwnode.h>
> +#include <linux/kref.h>
> +#include <linux/property.h>
> +
> +#include <media/media-device.h>
> +
> +#include <media/mc-shared-graph.h>
> +
> +static LIST_HEAD(media_device_shared_list);
> +static DEFINE_MUTEX(media_device_shared_lock);
> +
> +struct media_device_shared_member {
> + struct device *dev;
> + struct fwnode_handle *fwnode;
> + struct list_head list;
> +};
> +
> +struct media_device_shared_link {
> + struct media_entity *source;
> + u16 source_pad;
> + struct media_entity *sink;
> + u16 sink_pad;
> + u32 flags;
> + struct list_head list;
> +};
> +
> +// TODO figure out locking for when multiple drivers touch the media graph;
> +// maybe macros for shared versions?
Do you mean for when drivers are trying to change link state directly?
> +struct media_device_shared {
> + struct media_device mdev;
> + struct list_head members;
> + struct list_head links;
> +
> + struct list_head list;
> + struct kref refcount;
> +
> + struct device *removed_device;
> +};
> +
> +static inline struct media_device_shared *
> +to_media_device_shared(struct media_device *mdev)
> +{
> + return container_of(mdev, struct media_device_shared, mdev);
> +}
> +
> +static void media_device_shared_release(struct kref *kref)
> +{
> + struct media_device_shared *mds =
> + container_of(kref, struct media_device_shared, refcount);
> +
> + dev_dbg(mds->removed_device, "%s: releasing Media Device\n", __func__);
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + media_device_unregister(&mds->mdev);
> + media_device_cleanup(&mds->mdev);
> +
> + list_del(&mds->list);
> + mutex_unlock(&media_device_shared_lock);
> +
> + kfree(mds);
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
Lets add a lockdep_assert then?
> +static bool __media_device_shared_find_match(struct media_device_shared *mds,
> + struct fwnode_handle *fwnode)
> +{
> + struct media_device_shared_member *member;
> + struct fwnode_handle *ep;
> + struct fwnode_handle *remote_ep;
> + bool match = false;
> +
is it just as easy as:
lockdep_assert_held(&media_device_shared_lock);
?
> + // TODO: parse the device tree endpoints graph instead of finding just the
> + // first-level neighbours
> + fwnode_graph_for_each_endpoint(fwnode, ep) {
> + list_for_each_entry(member, &mds->members, list) {
> + remote_ep = fwnode_graph_get_remote_port_parent(ep);
> + match = (member->fwnode == remote_ep);
> + fwnode_handle_put(remote_ep);
> +
> + if (!match)
> + continue;
> +
> + goto match_complete;
> + }
> + }
> +
> +match_complete:
> + fwnode_handle_put(ep);
> + return match;
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
Lets add a lockdep check too then :D (same everywhere/anywhere it's
needed)
> +static struct media_device *__media_device_shared_get(struct device *dev)
> +{
> + struct media_device_shared *mds;
> + struct media_device_shared_member *member;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + bool ret;
> +
> + dev_dbg(dev, "%s: searching for media device for %pfwf", __func__, fwnode);
> +
> + list_for_each_entry(mds, &media_device_shared_list, list) {
> + ret = __media_device_shared_find_match(mds, fwnode);
> + if (ret)
> + break;
> + }
> +
> + if (!ret)
> + return NULL;
> +
> + member = kzalloc_obj(*member);
> + if (!member)
> + return NULL;
> +
> + member->dev = dev;
> + member->fwnode = fwnode;
> + list_add_tail(&member->list, &mds->members);
> + kref_get(&mds->refcount);
> +
> + dev_dbg(dev, "%s: %pfwf joined media device of %pfwf",
> + __func__, fwnode,
> + list_first_entry(&mds->members, struct media_device_shared_member, list)->fwnode);
> +
> + return &mds->mdev;
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static struct media_device *__media_device_shared_create(struct device *dev)
> +{
> + struct media_device_shared *mds;
> + struct media_device_shared_member *member;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + int ret;
> +
> + mds = kzalloc_obj(*mds);
> + if (!mds)
> + return NULL;
> +
> + member = kzalloc_obj(*member);
> + if (!member)
> + goto err_free_mds;
> +
> + media_device_init(&mds->mdev);
> +
> + ret = media_device_register(&mds->mdev);
> + if (ret)
> + goto err_free_member;
> +
> + INIT_LIST_HEAD(&mds->members);
> + member->dev = dev;
> + member->fwnode = fwnode;
> + list_add_tail(&member->list, &mds->members);
> +
> + INIT_LIST_HEAD(&mds->links);
> +
> + kref_init(&mds->refcount);
> + list_add_tail(&mds->list, &media_device_shared_list);
> +
> + // TODO figure out how to reconcile this with multiple members
Aha, right - becuse the 'media_device dev' becomes whoever registers first.
I wonder where it's actually used, and maybe that's ok ?
> + mds->mdev.dev = dev;
> +
> + devv_dbg(dev, "%s: Allocated media device with %pfwf at %p\n",
> + __func__, fwnode, &mds->mdev);
> + return &mds->mdev;
> +
> +err_free_member:
> + kfree(member);
> +err_free_mds:
> + kfree(mds);
> + return NULL;
> +}
> +
> +// TODO figure out how to resolve the identifiers (model, driver name, etc);
> +// atm it's racy and whoever gets it last wins
Maybe that's something we need to pass or set up explicitly then, so
it's clear it's a 'specific graph'
Would this give us a way to represent the hardware in a new form - i.e.
thinking about IMX8MP - the existing graph wouldn't be available - so it
wouldn't be any worry from a UABI perspective because it's just a whole
new interface/media-device name....
> +struct media_device *media_device_shared_join(struct device *dev)
> +{
> + struct media_device *mdev;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + mdev = __media_device_shared_get(dev);
> + if (!!mdev) {
> + dev_dbg(dev, "%s: found media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> + }
> +
> + mdev = __media_device_shared_create(dev);
> + if (!mdev) {
> + dev_warn(dev, "%s: failed to create media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + dev_dbg(dev, "%s: created media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join);
> +
> +void media_device_shared_leave(struct media_device *mdev, struct device *dev)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_member *member;
> + struct media_device_shared_member *member_tmp;
> + bool removed = false;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + list_for_each_entry_safe(member, member_tmp, &mds->members, list) {
> + if (member->dev == dev) {
> + list_del(&member->list);
> + kfree(member);
> + removed = true;
> + }
> + }
> +
> + if (!removed)
> + dev_err(dev, "%s: %pfwf trying to leave from graph in which not a member",
> + __func__, dev_fwnode(dev));
> +
> + mds->removed_device = dev;
> + mutex_unlock(&media_device_shared_lock);
> + kref_put(&mds->refcount, media_device_shared_release);
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_leave);
> +
> +int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_link *link;
> + struct media_device_shared_link *link_tmp;
> + int ret = 0;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + /*
> + * TODO Figure out flags. Should we use greatest common denominator? Or
> + * prioritize sink? Or whoever wins the race? For now we just take the flags
> + * from the sink.
> + *
> + * TODO Figure out how to actually do the matching. For now we just match
> + * whoever comes in first. This works with the simple example we're running
> + * with now (rkcif + one rkisp2) but with setups with multiple copies of
> + * hardware this will cause problems, like with rkcif + two rkisp2 and
> + * imx8-isi + two rkisp1.
> + */
> + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> + if (link->sink) {
> + ret = media_create_pad_link(source, source_pad,
> + link->sink, link->sink_pad,
> + link->flags);
> + list_del(&link->list);
> + kfree(link);
> + goto exit_join_link_source;
> + }
> + }
> +
> + link = kzalloc_obj(*link);
> + if (!link) {
> + ret = -ENOMEM;
> + goto exit_join_link_source;
> + }
> +
> + link->source = source;
> + link->source_pad = source_pad;
> + link->flags = flags;
> + list_add_tail(&link->list, &mds->links);
> +
> +exit_join_link_source:
> + mutex_unlock(&media_device_shared_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join_link_source);
> +
> +// TODO deduplicate from above
Indeed, it does look like an opportunity.
> +int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_link *link;
> + struct media_device_shared_link *link_tmp;
> + int ret = 0;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> + if (link->source) {
> + ret = media_create_pad_link(link->source, link->source_pad,
> + sink, sink_pad,
> + flags);
> + list_del(&link->list);
> + kfree(link);
> + goto exit_join_link_sink;
> + }
> + }
> +
> + link = kzalloc_obj(*link);
> + if (!link) {
> + ret = -ENOMEM;
> + goto exit_join_link_sink;
> + }
> +
> + link->sink = sink;
> + link->sink_pad = sink_pad;
> + link->flags = flags;
> + list_add_tail(&link->list, &mds->links);
> +
> +exit_join_link_sink:
> + mutex_unlock(&media_device_shared_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join_link_sink);
> diff --git a/include/media/mc-shared-graph.h b/include/media/mc-shared-graph.h
> new file mode 100644
> index 000000000000..487325163f84
> --- /dev/null
> +++ b/include/media/mc-shared-graph.h
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mc-shared-graph.h - Media Controller Shared Graph API
> + *
> + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> + */
> +
> +/*
> + * This file adds the Media Controller Shared Graph API. This allows drivers
> + * to create shared media graphs or join existing media graphs from other
> + * drivers, so that they can all be in the same media graph. This allows us to
> + * have more complex media graphs chaining more complex hardware together,
> + * instead of simple async subdevs.
> + */
> +
> +#include <linux/types.h>
> +
> +#ifndef _MEDIA_SHARED_GRAPH_H
> +#define _MEDIA_SHARED_GRAPH_H
> +
> +struct device;
> +struct media_device;
> +struct media_entity;
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +/**
> + * media_device_shared_join() - Join or create a new shared media device
> + *
> + * @dev: struct &device pointer
> + *
> + * This is the entrance function for a device to join or create a new shared
> + * media device. It searches for an existing shared media device based on the
> + * neighbours in the device's device tree ports node. If found, then this
> + * functions returns the existing shared media device and joins it. If one is
> + * not found then one is created and initialized and returned.
> + */
> +struct media_device *media_device_shared_join(struct device *dev);
> +
> +/**
> + * media_device_shared_leave() - Leave the shared media device.
> + *
> + * @mdev: struct &media_device pointer
> + * @dev: struct &device pointer
> + *
> + * This function makes the device leave the shared media device. When all
> + * members have left the media device it will be freed.
> + */
> +void media_device_shared_leave(struct media_device *mdev, struct device *dev);
> +
> +/**
> + * media_device_shared_join_link_source() - Register a link source in the shared media device
> + *
> + * @mdev: The struct &media_device pointer that is part of a shared media device
> + * @dev: struct &device pointer
> + * @source: The link source
> + * @source_pad: The pad
> + * @flags: The flags
> + *
> + * This function registers with the shared media device the source part of a
> + * link. When the shared media device receives the matching sink part of a link
> + * via media_device_shared_join_link_sink() then the link will be fully created.
> + */
> +int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags);
> +
> +/**
> + * media_device_shared_join_link_sink() - Register a link sink in the shared media device
> + *
> + * Same as media_device_shared_join_link_source() but for sink instead of
> + * source.
> + */
> +int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags);
> +#else
> +static inline struct media_device *media_device_shared_join(struct device *dev)
> +{ return NULL; }
> +static inline void media_device_shared_leave(struct media_device *mdev,
> + struct device *dev) { }
> +static inline int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags) { }
> +static inline int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags) { }
Should these two stubs which return an int return an error code? -ENODEV or such ?
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> +#endif /* _MEDIA_DEV_SHARED_GRAPH_H */
> --
> 2.47.2
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-06-24 10:39 ` Kieran Bingham
@ 2026-06-25 8:23 ` Paul Elder
2026-07-10 20:42 ` Michael Riesch
0 siblings, 1 reply; 14+ messages in thread
From: Paul Elder @ 2026-06-25 8:23 UTC (permalink / raw)
To: Kieran Bingham, laurent.pinchart
Cc: michael.riesch, xuhf, stefan.klug, dan.scally, jacopo.mondi,
linux-media, linux-arm-kernel, linux-rockchip, linux-kernel,
hverkuil+cisco, nicolas.dufresne, ribalda, sakari.ailus
Hi Kieran,
Thanks for the not-necessarily-a-full review.
Quoting Kieran Bingham (2026-06-24 19:39:44)
> Hi Paul,
>
> I'm taking a first read through, some comments inline, but not
> necessarily a full review:
>
> Quoting Paul Elder (2026-06-19 06:26:28)
> > Currently, a media graph contains a main device whose driver is
> > responsible for creating the media device. We have however recently run
> > into devices that have multiple devices that can quality as a main
>
> s/quality/qualify/
>
> > device. Examples are the RK3588 which has a VICAP and two ISP
> > instances, and another example is the i.MX8MP which has an ISI and two
> > ISP instances. As there is currently no way to reconcile who the main
> > device is in the media device, these setups simple cannot be used
> > simultaneously.
>
> I'm very excited to see how we could apply this to the NXP i.MX8MP to
> resolve the ISI+ISP issue!
\o/
>
>
> > This patch extends the media controller API with a "shared media graph"
> > framework. This allows drivers to share a media device, thus enabling
> > the setups mentioned above. Instead of owning and creating a media
> > device, drivers can join-or-create a shared media device via the shared
> > media graph API. The matching is done automatically based on the
> > detected endpoints in the device tree.
>
> Sounds great! I'll read on...
>
> >
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> > drivers/media/mc/Makefile | 2 +-
> > drivers/media/mc/mc-shared-graph.c | 335 +++++++++++++++++++++++++++++
> > include/media/mc-shared-graph.h | 92 ++++++++
> > 3 files changed, 428 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/media/mc/mc-shared-graph.c
> > create mode 100644 include/media/mc-shared-graph.h
> >
> > diff --git a/drivers/media/mc/Makefile b/drivers/media/mc/Makefile
> > index 2b7af42ba59c..1d502fdc52ad 100644
> > --- a/drivers/media/mc/Makefile
> > +++ b/drivers/media/mc/Makefile
> > @@ -1,7 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0
> >
> > mc-objs := mc-device.o mc-devnode.o mc-entity.o \
> > - mc-request.o
> > + mc-request.o mc-shared-graph.o
> >
> > ifneq ($(CONFIG_USB),)
> > mc-objs += mc-dev-allocator.o
> > diff --git a/drivers/media/mc/mc-shared-graph.c b/drivers/media/mc/mc-shared-graph.c
> > new file mode 100644
> > index 000000000000..c4067e5b861d
> > --- /dev/null
> > +++ b/drivers/media/mc/mc-shared-graph.c
> > @@ -0,0 +1,335 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * mc-shared-graph.c - Media Controller Shared Graph API
> > + *
> > + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> > + */
> > +
> > +/*
> > + * This file adds the Media Controller Shared Graph API. This allows drivers
> > + * to create shared media graphs or join existing media graphs from other
> > + * drivers, so that they can all be in the same media graph. This allows us to
> > + * have more complex media graphs chaining more complex hardware together,
> > + * instead of simple async subdevs.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/fwnode.h>
> > +#include <linux/kref.h>
> > +#include <linux/property.h>
> > +
> > +#include <media/media-device.h>
> > +
> > +#include <media/mc-shared-graph.h>
> > +
> > +static LIST_HEAD(media_device_shared_list);
> > +static DEFINE_MUTEX(media_device_shared_lock);
> > +
> > +struct media_device_shared_member {
> > + struct device *dev;
> > + struct fwnode_handle *fwnode;
> > + struct list_head list;
> > +};
> > +
> > +struct media_device_shared_link {
> > + struct media_entity *source;
> > + u16 source_pad;
> > + struct media_entity *sink;
> > + u16 sink_pad;
> > + u32 flags;
> > + struct list_head list;
> > +};
> > +
> > +// TODO figure out locking for when multiple drivers touch the media graph;
> > +// maybe macros for shared versions?
>
> Do you mean for when drivers are trying to change link state directly?
I meant for all the operations that act on media device. I'm not sure what
there is because I didn't really find anything significant, and I found some
action point from some meeting notes somewhere that said "deprecate media_ops"
(not assigned to me) so...
If there aren't any then it's a non-issue, but if there are then I was
wondering if we need to return the shared media device to the driver (as
opposed to a non-shared regular media device) and use shared versions of media
device functions that have locking.
>
> > +struct media_device_shared {
> > + struct media_device mdev;
> > + struct list_head members;
> > + struct list_head links;
> > +
> > + struct list_head list;
> > + struct kref refcount;
> > +
> > + struct device *removed_device;
> > +};
> > +
> > +static inline struct media_device_shared *
> > +to_media_device_shared(struct media_device *mdev)
> > +{
> > + return container_of(mdev, struct media_device_shared, mdev);
> > +}
> > +
> > +static void media_device_shared_release(struct kref *kref)
> > +{
> > + struct media_device_shared *mds =
> > + container_of(kref, struct media_device_shared, refcount);
> > +
> > + dev_dbg(mds->removed_device, "%s: releasing Media Device\n", __func__);
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + media_device_unregister(&mds->mdev);
> > + media_device_cleanup(&mds->mdev);
> > +
> > + list_del(&mds->list);
> > + mutex_unlock(&media_device_shared_lock);
> > +
> > + kfree(mds);
> > +}
> > +
> > +/* Callers should hold media_device_shared_lock when calling this function */
>
> Lets add a lockdep_assert then?
I learned something new today :)
>
>
> > +static bool __media_device_shared_find_match(struct media_device_shared *mds,
> > + struct fwnode_handle *fwnode)
> > +{
> > + struct media_device_shared_member *member;
> > + struct fwnode_handle *ep;
> > + struct fwnode_handle *remote_ep;
> > + bool match = false;
> > +
>
> is it just as easy as:
>
> lockdep_assert_held(&media_device_shared_lock);
> ?
If I understand correctly how lockdep_assert works, yes.
>
> > + // TODO: parse the device tree endpoints graph instead of finding just the
> > + // first-level neighbours
> > + fwnode_graph_for_each_endpoint(fwnode, ep) {
> > + list_for_each_entry(member, &mds->members, list) {
> > + remote_ep = fwnode_graph_get_remote_port_parent(ep);
> > + match = (member->fwnode == remote_ep);
> > + fwnode_handle_put(remote_ep);
> > +
> > + if (!match)
> > + continue;
> > +
> > + goto match_complete;
> > + }
> > + }
> > +
> > +match_complete:
> > + fwnode_handle_put(ep);
> > + return match;
> > +}
> > +
> > +/* Callers should hold media_device_shared_lock when calling this function */
>
> Lets add a lockdep check too then :D (same everywhere/anywhere it's
> needed)
>
>
> > +static struct media_device *__media_device_shared_get(struct device *dev)
> > +{
> > + struct media_device_shared *mds;
> > + struct media_device_shared_member *member;
> > + struct fwnode_handle *fwnode = dev_fwnode(dev);
> > + bool ret;
> > +
> > + dev_dbg(dev, "%s: searching for media device for %pfwf", __func__, fwnode);
> > +
> > + list_for_each_entry(mds, &media_device_shared_list, list) {
> > + ret = __media_device_shared_find_match(mds, fwnode);
> > + if (ret)
> > + break;
> > + }
> > +
> > + if (!ret)
> > + return NULL;
> > +
> > + member = kzalloc_obj(*member);
> > + if (!member)
> > + return NULL;
> > +
> > + member->dev = dev;
> > + member->fwnode = fwnode;
> > + list_add_tail(&member->list, &mds->members);
> > + kref_get(&mds->refcount);
> > +
> > + dev_dbg(dev, "%s: %pfwf joined media device of %pfwf",
> > + __func__, fwnode,
> > + list_first_entry(&mds->members, struct media_device_shared_member, list)->fwnode);
> > +
> > + return &mds->mdev;
> > +}
> > +
> > +/* Callers should hold media_device_shared_lock when calling this function */
> > +static struct media_device *__media_device_shared_create(struct device *dev)
> > +{
> > + struct media_device_shared *mds;
> > + struct media_device_shared_member *member;
> > + struct fwnode_handle *fwnode = dev_fwnode(dev);
> > + int ret;
> > +
> > + mds = kzalloc_obj(*mds);
> > + if (!mds)
> > + return NULL;
> > +
> > + member = kzalloc_obj(*member);
> > + if (!member)
> > + goto err_free_mds;
> > +
> > + media_device_init(&mds->mdev);
> > +
> > + ret = media_device_register(&mds->mdev);
> > + if (ret)
> > + goto err_free_member;
> > +
> > + INIT_LIST_HEAD(&mds->members);
> > + member->dev = dev;
> > + member->fwnode = fwnode;
> > + list_add_tail(&member->list, &mds->members);
> > +
> > + INIT_LIST_HEAD(&mds->links);
> > +
> > + kref_init(&mds->refcount);
> > + list_add_tail(&mds->list, &media_device_shared_list);
> > +
> > + // TODO figure out how to reconcile this with multiple members
>
> Aha, right - becuse the 'media_device dev' becomes whoever registers first.
> I wonder where it's actually used, and maybe that's ok ?
Yes, exactly. So far all I could find it used for was dev_err etc. In my
testing it hasn't caused any problems, since I tested both loading orders
between the rkcif and rkisp2. Maybe I missed something important and somebody
else knows better.
Although it's not very nice to have prints from the other device. Is making a
thin device an option?
>
> > + mds->mdev.dev = dev;
> > +
> > + devv_dbg(dev, "%s: Allocated media device with %pfwf at %p\n",
> > + __func__, fwnode, &mds->mdev);
> > + return &mds->mdev;
> > +
> > +err_free_member:
> > + kfree(member);
> > +err_free_mds:
> > + kfree(mds);
> > + return NULL;
> > +}
> > +
> > +// TODO figure out how to resolve the identifiers (model, driver name, etc);
> > +// atm it's racy and whoever gets it last wins
>
> Maybe that's something we need to pass or set up explicitly then, so
> it's clear it's a 'specific graph'
Yes. I have no clue where that name would come from though.
Although I thought userspace matches on the entity names of the media device?
Does the name of the graph matter? It needs to be consistent though imo.
I considered having a list of names of members, but I'm not sure how useful
that would be. We don't have a userspace API to check it, plus it can already
just list the member entities of the graph. And the driver doesn't really care
who else is in the graph, since the shared media graph machinery already
automatches based on dt endpoints.
Also imo it's also not too nice for userspace when {driver_name} refers to a
driver that doesn't exist.
>
> Would this give us a way to represent the hardware in a new form - i.e.
> thinking about IMX8MP - the existing graph wouldn't be available - so it
> wouldn't be any worry from a UABI perspective because it's just a whole
> new interface/media-device name....
Hm, maybe the media device name could just be something along the lines of
"shared-{platform_name}-media"? Although I suppose that assumes that each
platform only has one media graph and we'll run into problems when we have
multiple. Or we append the youngest register block address in the shared media
graph to the name?
>
>
>
> > +struct media_device *media_device_shared_join(struct device *dev)
> > +{
> > + struct media_device *mdev;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + mdev = __media_device_shared_get(dev);
> > + if (!!mdev) {
> > + dev_dbg(dev, "%s: found media device for %pfwf", __func__, dev_fwnode(dev));
> > + mutex_unlock(&media_device_shared_lock);
> > + return mdev;
> > + }
> > +
> > + mdev = __media_device_shared_create(dev);
> > + if (!mdev) {
> > + dev_warn(dev, "%s: failed to create media device for %pfwf", __func__, dev_fwnode(dev));
> > + mutex_unlock(&media_device_shared_lock);
> > + return ERR_PTR(-ENOMEM);
> > + }
> > +
> > + dev_dbg(dev, "%s: created media device for %pfwf", __func__, dev_fwnode(dev));
> > + mutex_unlock(&media_device_shared_lock);
> > + return mdev;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_join);
> > +
> > +void media_device_shared_leave(struct media_device *mdev, struct device *dev)
> > +{
> > + struct media_device_shared *mds = to_media_device_shared(mdev);
> > + struct media_device_shared_member *member;
> > + struct media_device_shared_member *member_tmp;
> > + bool removed = false;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + list_for_each_entry_safe(member, member_tmp, &mds->members, list) {
> > + if (member->dev == dev) {
> > + list_del(&member->list);
> > + kfree(member);
> > + removed = true;
> > + }
> > + }
> > +
> > + if (!removed)
> > + dev_err(dev, "%s: %pfwf trying to leave from graph in which not a member",
> > + __func__, dev_fwnode(dev));
> > +
> > + mds->removed_device = dev;
> > + mutex_unlock(&media_device_shared_lock);
> > + kref_put(&mds->refcount, media_device_shared_release);
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_leave);
> > +
> > +int media_device_shared_join_link_source(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *source,
> > + u16 source_pad, u32 flags)
> > +{
> > + struct media_device_shared *mds = to_media_device_shared(mdev);
> > + struct media_device_shared_link *link;
> > + struct media_device_shared_link *link_tmp;
> > + int ret = 0;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + /*
> > + * TODO Figure out flags. Should we use greatest common denominator? Or
> > + * prioritize sink? Or whoever wins the race? For now we just take the flags
> > + * from the sink.
> > + *
> > + * TODO Figure out how to actually do the matching. For now we just match
> > + * whoever comes in first. This works with the simple example we're running
> > + * with now (rkcif + one rkisp2) but with setups with multiple copies of
> > + * hardware this will cause problems, like with rkcif + two rkisp2 and
> > + * imx8-isi + two rkisp1.
> > + */
> > + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> > + if (link->sink) {
> > + ret = media_create_pad_link(source, source_pad,
> > + link->sink, link->sink_pad,
> > + link->flags);
> > + list_del(&link->list);
> > + kfree(link);
> > + goto exit_join_link_source;
> > + }
> > + }
> > +
> > + link = kzalloc_obj(*link);
> > + if (!link) {
> > + ret = -ENOMEM;
> > + goto exit_join_link_source;
> > + }
> > +
> > + link->source = source;
> > + link->source_pad = source_pad;
> > + link->flags = flags;
> > + list_add_tail(&link->list, &mds->links);
> > +
> > +exit_join_link_source:
> > + mutex_unlock(&media_device_shared_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_join_link_source);
> > +
> > +// TODO deduplicate from above
>
> Indeed, it does look like an opportunity.
Ok this is the one exception that was an actual todo and not a topic for
discussion :)
(Unless someone has the opinion "actually we don't need to deduplicate this")
>
> > +int media_device_shared_join_link_sink(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *sink,
> > + u16 sink_pad, u32 flags)
> > +{
> > + struct media_device_shared *mds = to_media_device_shared(mdev);
> > + struct media_device_shared_link *link;
> > + struct media_device_shared_link *link_tmp;
> > + int ret = 0;
> > +
> > + mutex_lock(&media_device_shared_lock);
> > +
> > + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> > + if (link->source) {
> > + ret = media_create_pad_link(link->source, link->source_pad,
> > + sink, sink_pad,
> > + flags);
> > + list_del(&link->list);
> > + kfree(link);
> > + goto exit_join_link_sink;
> > + }
> > + }
> > +
> > + link = kzalloc_obj(*link);
> > + if (!link) {
> > + ret = -ENOMEM;
> > + goto exit_join_link_sink;
> > + }
> > +
> > + link->sink = sink;
> > + link->sink_pad = sink_pad;
> > + link->flags = flags;
> > + list_add_tail(&link->list, &mds->links);
> > +
> > +exit_join_link_sink:
> > + mutex_unlock(&media_device_shared_lock);
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(media_device_shared_join_link_sink);
> > diff --git a/include/media/mc-shared-graph.h b/include/media/mc-shared-graph.h
> > new file mode 100644
> > index 000000000000..487325163f84
> > --- /dev/null
> > +++ b/include/media/mc-shared-graph.h
> > @@ -0,0 +1,92 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * mc-shared-graph.h - Media Controller Shared Graph API
> > + *
> > + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> > + */
> > +
> > +/*
> > + * This file adds the Media Controller Shared Graph API. This allows drivers
> > + * to create shared media graphs or join existing media graphs from other
> > + * drivers, so that they can all be in the same media graph. This allows us to
> > + * have more complex media graphs chaining more complex hardware together,
> > + * instead of simple async subdevs.
> > + */
> > +
> > +#include <linux/types.h>
> > +
> > +#ifndef _MEDIA_SHARED_GRAPH_H
> > +#define _MEDIA_SHARED_GRAPH_H
> > +
> > +struct device;
> > +struct media_device;
> > +struct media_entity;
> > +
> > +#if defined(CONFIG_MEDIA_CONTROLLER)
> > +/**
> > + * media_device_shared_join() - Join or create a new shared media device
> > + *
> > + * @dev: struct &device pointer
> > + *
> > + * This is the entrance function for a device to join or create a new shared
> > + * media device. It searches for an existing shared media device based on the
> > + * neighbours in the device's device tree ports node. If found, then this
> > + * functions returns the existing shared media device and joins it. If one is
> > + * not found then one is created and initialized and returned.
> > + */
> > +struct media_device *media_device_shared_join(struct device *dev);
> > +
> > +/**
> > + * media_device_shared_leave() - Leave the shared media device.
> > + *
> > + * @mdev: struct &media_device pointer
> > + * @dev: struct &device pointer
> > + *
> > + * This function makes the device leave the shared media device. When all
> > + * members have left the media device it will be freed.
> > + */
> > +void media_device_shared_leave(struct media_device *mdev, struct device *dev);
> > +
> > +/**
> > + * media_device_shared_join_link_source() - Register a link source in the shared media device
> > + *
> > + * @mdev: The struct &media_device pointer that is part of a shared media device
> > + * @dev: struct &device pointer
> > + * @source: The link source
> > + * @source_pad: The pad
> > + * @flags: The flags
> > + *
> > + * This function registers with the shared media device the source part of a
> > + * link. When the shared media device receives the matching sink part of a link
> > + * via media_device_shared_join_link_sink() then the link will be fully created.
> > + */
> > +int media_device_shared_join_link_source(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *source,
> > + u16 source_pad, u32 flags);
> > +
> > +/**
> > + * media_device_shared_join_link_sink() - Register a link sink in the shared media device
> > + *
> > + * Same as media_device_shared_join_link_source() but for sink instead of
> > + * source.
> > + */
> > +int media_device_shared_join_link_sink(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *sink,
> > + u16 sink_pad, u32 flags);
> > +#else
> > +static inline struct media_device *media_device_shared_join(struct device *dev)
> > +{ return NULL; }
> > +static inline void media_device_shared_leave(struct media_device *mdev,
> > + struct device *dev) { }
> > +static inline int media_device_shared_join_link_source(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *source,
> > + u16 source_pad, u32 flags) { }
> > +static inline int media_device_shared_join_link_sink(struct media_device *mdev,
> > + struct device *dev,
> > + struct media_entity *sink,
> > + u16 sink_pad, u32 flags) { }
>
> Should these two stubs which return an int return an error code? -ENODEV or such ?
Ah yes they should. I missed that.
Thanks,
Paul
>
>
> > +#endif /* CONFIG_MEDIA_CONTROLLER */
> > +#endif /* _MEDIA_DEV_SHARED_GRAPH_H */
> > --
> > 2.47.2
> >
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-06-25 8:23 ` Paul Elder
@ 2026-07-10 20:42 ` Michael Riesch
2026-07-10 21:28 ` Nicolas Dufresne
0 siblings, 1 reply; 14+ messages in thread
From: Michael Riesch @ 2026-07-10 20:42 UTC (permalink / raw)
To: Paul Elder, Kieran Bingham, laurent.pinchart
Cc: xuhf, stefan.klug, dan.scally, jacopo.mondi, linux-media,
linux-arm-kernel, linux-rockchip, linux-kernel, hverkuil+cisco,
nicolas.dufresne, ribalda, sakari.ailus
Hi Paul,
On 6/25/26 10:23, Paul Elder wrote:
> [...]
>>> +// TODO figure out locking for when multiple drivers touch the media graph;
>>> +// maybe macros for shared versions?
>>
>> Do you mean for when drivers are trying to change link state directly?
>
> I meant for all the operations that act on media device. I'm not sure what
> there is because I didn't really find anything significant, and I found some
> action point from some meeting notes somewhere that said "deprecate media_ops"
> (not assigned to me) so...
Oh yes, for some reason this ball ended up in our side of the field. But
actually I think the media_ops are not much of an issue, as you don't
set them when you create the shared media device. Problem solved, right?
> If there aren't any then it's a non-issue, but if there are then I was
> wondering if we need to return the shared media device to the driver (as
> opposed to a non-shared regular media device) and use shared versions of media
> device functions that have locking.
+1
I would recommend that just to be on the safe side. And I think I would
make the shared media device an opaque pointer to make sure that there
won't be any monkey business.
Thanks and best regards,
Michael
>
>>
>>> +struct media_device_shared {
>>> + struct media_device mdev;
>>> + struct list_head members;
>>> + struct list_head links;
>>> +
>>> + struct list_head list;
>>> + struct kref refcount;
>>> +
>>> + struct device *removed_device;
>>> +};
>>> +[...]
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-07-10 20:42 ` Michael Riesch
@ 2026-07-10 21:28 ` Nicolas Dufresne
0 siblings, 0 replies; 14+ messages in thread
From: Nicolas Dufresne @ 2026-07-10 21:28 UTC (permalink / raw)
To: Michael Riesch, Paul Elder, Kieran Bingham, laurent.pinchart
Cc: xuhf, stefan.klug, dan.scally, jacopo.mondi, linux-media,
linux-arm-kernel, linux-rockchip, linux-kernel, hverkuil+cisco,
ribalda, sakari.ailus
[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]
Le vendredi 10 juillet 2026 à 22:42 +0200, Michael Riesch a écrit :
> Hi Paul,
>
> On 6/25/26 10:23, Paul Elder wrote:
> > [...]
> > > > +// TODO figure out locking for when multiple drivers touch the media graph;
> > > > +// maybe macros for shared versions?
> > >
> > > Do you mean for when drivers are trying to change link state directly?
> >
> > I meant for all the operations that act on media device. I'm not sure what
> > there is because I didn't really find anything significant, and I found some
> > action point from some meeting notes somewhere that said "deprecate media_ops"
> > (not assigned to me) so...
>
> Oh yes, for some reason this ball ended up in our side of the field. But
> actually I think the media_ops are not much of an issue, as you don't
> set them when you create the shared media device. Problem solved, right?
If we effectively don't need them in any drivers using shared context, let's
find a place to add a BUG_ON.
Nicolas
>
> > If there aren't any then it's a non-issue, but if there are then I was
> > wondering if we need to return the shared media device to the driver (as
> > opposed to a non-shared regular media device) and use shared versions of media
> > device functions that have locking.
>
> +1
> I would recommend that just to be on the safe side. And I think I would
> make the shared media device an opaque pointer to make sure that there
> won't be any monkey business.
>
> Thanks and best regards,
> Michael
>
> >
> > >
> > > > +struct media_device_shared {
> > > > + struct media_device mdev;
> > > > + struct list_head members;
> > > > + struct list_head links;
> > > > +
> > > > + struct list_head list;
> > > > + struct kref refcount;
> > > > +
> > > > + struct device *removed_device;
> > > > +};
> > > > +[...]
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-06-19 5:26 ` [RFC PATCH 1/6] media: mc: Implement shared media graph Paul Elder
2026-06-24 4:47 ` Paul Elder
2026-06-24 10:39 ` Kieran Bingham
@ 2026-07-10 20:23 ` Michael Riesch
2026-07-10 21:50 ` Nicolas Dufresne
3 siblings, 0 replies; 14+ messages in thread
From: Michael Riesch @ 2026-07-10 20:23 UTC (permalink / raw)
To: Paul Elder, laurent.pinchart
Cc: xuhf, stefan.klug, kieran.bingham, dan.scally, jacopo.mondi,
linux-media, linux-arm-kernel, linux-rockchip, linux-kernel,
hverkuil+cisco, nicolas.dufresne, ribalda, sakari.ailus
Hi Paul,
Thanks for your work! And sorry for the long silence.
On 6/19/26 07:26, Paul Elder wrote:
> Currently, a media graph contains a main device whose driver is
> responsible for creating the media device. We have however recently run
> into devices that have multiple devices that can quality as a main
> device. Examples are the RK3588 which has a VICAP and two ISP
> instances, and another example is the i.MX8MP which has an ISI and two
> ISP instances. As there is currently no way to reconcile who the main
> device is in the media device, these setups simple cannot be used
> simultaneously.
>
> This patch extends the media controller API with a "shared media graph"
> framework. This allows drivers to share a media device, thus enabling
> the setups mentioned above. Instead of owning and creating a media
> device, drivers can join-or-create a shared media device via the shared
> media graph API. The matching is done automatically based on the
> detected endpoints in the device tree.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Frenetically-cheered-by: Michael Riesch <michael.riesch@collabora.com>
OK, what follows is pretty much a rubberducking session in which I try
to explain myself the concept you came up with. Possibly there are one
or two thoughts and/or questions you can use to develop the concept further.
IIUC this is a new API with four calls. There is _join() and _leave()
that more or less replace the creation and deletion of a media device in
a V4L2 driver. And there is join_link_source() and join_link_sink() that
are called for each cross-device connection on the source and sink side,
respectively.
There is one shared media device per disjoint graph in the device tree.
If a new device calls _join(), the function iterates over all shared
media devices, iterates over all DT endpoints of the new device, and
iterates over all members of each shared media device. If the fwnode of
the parent of the remote endpoint of a certain endpoint of the new
device equals the fwnode of a certain member of a certain shared media
device, we found a match. This certain shared media device is then used
in the driver of the new device. Before that, a member for the new
device is added to the shared media device.
In case no shared media device is found, a new one is created and used.
A member for the new device is added to the shared device in this case, too.
The actual linking takes place in the _join_link_{sink,source}
functions, which iterate over the links of the shared media device and
check whether a link has a pointer to a source (or sink) respectively.
If a source (or sink) is found, a link is created. Otherwise, a link (in
the form of a newly introduced data structure, not a media_link) is
added to the links of a shared media device. The latter somewhat
corresponds to the v4l2 async notifier mechanism.
So far, so good. Even if the genitive construction chain that describes
the matching above is a bit daunting. All in all I think we are on a
reasonable path here.
> [...]
> +// TODO figure out locking for when multiple drivers touch the media graph;
> +// maybe macros for shared versions?
> +struct media_device_shared {
> + struct media_device mdev;
> + struct list_head members;
> + struct list_head links;
> +
> + struct list_head list;
> + struct kref refcount;
> +
> + struct device *removed_device;
> +};
[discussed in other thread]
> [...]
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static bool __media_device_shared_find_match(struct media_device_shared *mds,
> + struct fwnode_handle *fwnode)
> +{
> + struct media_device_shared_member *member;
> + struct fwnode_handle *ep;
> + struct fwnode_handle *remote_ep;
> + bool match = false;
> +
> + // TODO: parse the device tree endpoints graph instead of finding just the
> + // first-level neighbours
> + fwnode_graph_for_each_endpoint(fwnode, ep) {
> + list_for_each_entry(member, &mds->members, list) {
> + remote_ep = fwnode_graph_get_remote_port_parent(ep);
> + match = (member->fwnode == remote_ep);
> + fwnode_handle_put(remote_ep);
> +
> + if (!match)
> + continue;
> +
> + goto match_complete;
> + }
> + }
> +
> +match_complete:
> + fwnode_handle_put(ep);
> + return match;
> +}
<rubberducking>
IIUC there could be devices A -> B -> C that share a DT graph (and thus
should share a media device), but suppose the probe order is driver A,
C, B then A would rightfully create a new shared media device, but C
would not see it and create yet another shared media device. Thus, the
the find_match() function needs to traverse C -> B -> A to find the
correct shared media device.
</rubberducking>
Just saying that with one and only one media device to rule them all we
wouldn't have to deal with this. Another option would be to assume that
each driver knows to which shared media device it belongs. For example,
the rkcif driver could be aware that it belongs to the
{
.id = MEDIA_SHARED_ROCKCHIP_CAMERA,
.name = "Rockchip Camera Subsystem",
}
shared media device. This would reduce the magic above to a lookup
operation. All that said, it should be feasible of course to traverse
and find.
> [...]
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static struct media_device *__media_device_shared_create(struct device *dev)
> +{
> + struct media_device_shared *mds;
> + struct media_device_shared_member *member;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + int ret;
> +
> + mds = kzalloc_obj(*mds);
> + if (!mds)
> + return NULL;
> +
> + member = kzalloc_obj(*member);
> + if (!member)
> + goto err_free_mds;
> +
> + media_device_init(&mds->mdev);
> +
> + ret = media_device_register(&mds->mdev);
> + if (ret)
> + goto err_free_member;
> +
> + INIT_LIST_HEAD(&mds->members);
> + member->dev = dev;
> + member->fwnode = fwnode;
> + list_add_tail(&member->list, &mds->members);
This can be refactored as this is carried out either way, right?
> +
> + INIT_LIST_HEAD(&mds->links);
> +
> + kref_init(&mds->refcount);
> + list_add_tail(&mds->list, &media_device_shared_list);
> +
> + // TODO figure out how to reconcile this with multiple members
> + mds->mdev.dev = dev;
> +
> + devv_dbg(dev, "%s: Allocated media device with %pfwf at %p\n",
> + __func__, fwnode, &mds->mdev);
> + return &mds->mdev;
> +
> +err_free_member:
> + kfree(member);
> +err_free_mds:
> + kfree(mds);
> + return NULL;
> +}
OK, what options do we have?!
We could pick one device of many. But based on what criteria? If there
was a good way to pick one, we could equally pick a driver that
registers the media device and move on. We conclude that apparently
that's not the case, otherwise we wouldn't be doing this exercise. Next.
We could introduce a pseudo device that provides the umbrella. But
again, that would be close to creating an umbrella driver that registers
the media device. And at some point we decided not to do that. Next.
We move from the notion of a media **device** to a media **graph**
(which is in alignment with the subject of this mail). The media graph
is something owned by the subsystem, not by any device/driver. The media
entities therein, however, could be owned by devices/drivers. Maybe
struct media_entity should feature a struct device *dev; member.
I could imagine that the dev member is used mostly for
dev_{info,err,...}, we would need to get rid of them. If there is
actually something with devm_ then this requires more thought.
IMHO the third option is the way to go.
> +
> +// TODO figure out how to resolve the identifiers (model, driver name, etc);
> +// atm it's racy and whoever gets it last wins
> +struct media_device *media_device_shared_join(struct device *dev)
> +{
> + struct media_device *mdev;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + mdev = __media_device_shared_get(dev);
> + if (!!mdev) {
> + dev_dbg(dev, "%s: found media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> + }
> +
> + mdev = __media_device_shared_create(dev);
> + if (!mdev) {
> + dev_warn(dev, "%s: failed to create media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + dev_dbg(dev, "%s: created media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join);
Similarly, we have to bid farewell to the notion of a media **device**.
Can we agree on making
struct device *dev;
char driver_name[32];
char serial[40];
char bus_info[32];
u32 hw_revision;
optional for shared media devices (or, better, graphs)?
I could imagine, though, that we provide some minimally meaningful info
in a struct
{
.id = MEDIA_SHARED_ROCKCHIP_CAMERA,
.name = "Rockchip Camera Subsystem",
.driver_name = "rkcss",
.hw_revision = 42,
...
}
and apply that info here.
> [...]
> +int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_link *link;
> + struct media_device_shared_link *link_tmp;
> + int ret = 0;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + /*
> + * TODO Figure out flags. Should we use greatest common denominator? Or
> + * prioritize sink? Or whoever wins the race? For now we just take the flags
> + * from the sink.
The phrase "whoever wins the race" is surely gold in the documentation
;-) I guess it should be same as with any _notifier_bound setup, and
IIUC it's typically the sink that passes some flags to
v4l2_create_fwnode_links_to_pad. So prioritizing the sink seems reasonable.
> + *
> + * TODO Figure out how to actually do the matching. For now we just match
> + * whoever comes in first. This works with the simple example we're running
> + * with now (rkcif + one rkisp2) but with setups with multiple copies of
> + * hardware this will cause problems, like with rkcif + two rkisp2 and
> + * imx8-isi + two rkisp1.
> + */
In the end, shouldn't this follow whatever v4l2_async does? Not exactly
sure, but I think storing a struct media_entity * and a struct
fwnode_endpoint * for side A should do the trick. Side B would appear
later with the second struct fwnode_endpoint. The check would then be if
fwnode_graph_get_remote_endpoint(fwnode_A) == fwnode_B (+ maybe vice
versa). The link would then be filled with media_entity_{A,B} and
media_entity_{A,B}->ops.get_fwnode_pad(media_entity_{A,B},
fwnode_{A,B}). Obviously we need some selector that tells us whether
A=source and B=sink or vice versa for that.
> + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> + if (link->sink) {
> + ret = media_create_pad_link(source, source_pad,
> + link->sink, link->sink_pad,
> + link->flags);
> + list_del(&link->list);
> + kfree(link);
> + goto exit_join_link_source;
> + }
> + }
> +
> + link = kzalloc_obj(*link);
> + if (!link) {
> + ret = -ENOMEM;
> + goto exit_join_link_source;
> + }
> +
> + link->source = source;
> + link->source_pad = source_pad;
> + link->flags = flags;
> + list_add_tail(&link->list, &mds->links);
> +
> +exit_join_link_source:
> + mutex_unlock(&media_device_shared_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join_link_source);
> +
> +// TODO deduplicate from above
Should be possible to have some helper with (..., bool is_source) and
then two nice functions that are exposed to the public.
To start the bike shedding: Maybe
media_device_shared_register_{source,sink}?
> [...]
Thanks and best regards,
Michael
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-06-19 5:26 ` [RFC PATCH 1/6] media: mc: Implement shared media graph Paul Elder
` (2 preceding siblings ...)
2026-07-10 20:23 ` Michael Riesch
@ 2026-07-10 21:50 ` Nicolas Dufresne
3 siblings, 0 replies; 14+ messages in thread
From: Nicolas Dufresne @ 2026-07-10 21:50 UTC (permalink / raw)
To: Paul Elder, laurent.pinchart
Cc: michael.riesch, xuhf, stefan.klug, kieran.bingham, dan.scally,
jacopo.mondi, linux-media, linux-arm-kernel, linux-rockchip,
linux-kernel, hverkuil+cisco, ribalda, sakari.ailus
[-- Attachment #1: Type: text/plain, Size: 17111 bytes --]
Hi,
Le vendredi 19 juin 2026 à 14:26 +0900, Paul Elder a écrit :
> Currently, a media graph contains a main device whose driver is
> responsible for creating the media device. We have however recently run
> into devices that have multiple devices that can quality as a main
> device. Examples are the RK3588 which has a VICAP and two ISP
> instances, and another example is the i.MX8MP which has an ISI and two
> ISP instances. As there is currently no way to reconcile who the main
> device is in the media device, these setups simple cannot be used
> simultaneously.
>
> This patch extends the media controller API with a "shared media graph"
> framework. This allows drivers to share a media device, thus enabling
> the setups mentioned above. Instead of owning and creating a media
> device, drivers can join-or-create a shared media device via the shared
> media graph API. The matching is done automatically based on the
> detected endpoints in the device tree.
Dropping a note, we should check if it makes sense combine this with
include/linux/component.h
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> drivers/media/mc/Makefile | 2 +-
> drivers/media/mc/mc-shared-graph.c | 335 +++++++++++++++++++++++++++++
> include/media/mc-shared-graph.h | 92 ++++++++
> 3 files changed, 428 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/mc/mc-shared-graph.c
> create mode 100644 include/media/mc-shared-graph.h
>
> diff --git a/drivers/media/mc/Makefile b/drivers/media/mc/Makefile
> index 2b7af42ba59c..1d502fdc52ad 100644
> --- a/drivers/media/mc/Makefile
> +++ b/drivers/media/mc/Makefile
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> mc-objs := mc-device.o mc-devnode.o mc-entity.o \
> - mc-request.o
> + mc-request.o mc-shared-graph.o
>
> ifneq ($(CONFIG_USB),)
> mc-objs += mc-dev-allocator.o
> diff --git a/drivers/media/mc/mc-shared-graph.c b/drivers/media/mc/mc-shared-graph.c
> new file mode 100644
> index 000000000000..c4067e5b861d
> --- /dev/null
> +++ b/drivers/media/mc/mc-shared-graph.c
> @@ -0,0 +1,335 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mc-shared-graph.c - Media Controller Shared Graph API
> + *
> + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> + */
> +
> +/*
> + * This file adds the Media Controller Shared Graph API. This allows drivers
> + * to create shared media graphs or join existing media graphs from other
> + * drivers, so that they can all be in the same media graph. This allows us to
> + * have more complex media graphs chaining more complex hardware together,
> + * instead of simple async subdevs.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/fwnode.h>
> +#include <linux/kref.h>
> +#include <linux/property.h>
> +
> +#include <media/media-device.h>
> +
> +#include <media/mc-shared-graph.h>
> +
> +static LIST_HEAD(media_device_shared_list);
> +static DEFINE_MUTEX(media_device_shared_lock);
> +
> +struct media_device_shared_member {
> + struct device *dev;
> + struct fwnode_handle *fwnode;
> + struct list_head list;
> +};
> +
> +struct media_device_shared_link {
> + struct media_entity *source;
> + u16 source_pad;
> + struct media_entity *sink;
> + u16 sink_pad;
> + u32 flags;
> + struct list_head list;
> +};
> +
> +// TODO figure out locking for when multiple drivers touch the media graph;
> +// maybe macros for shared versions?
> +struct media_device_shared {
> + struct media_device mdev;
> + struct list_head members;
> + struct list_head links;
> +
> + struct list_head list;
> + struct kref refcount;
> +
> + struct device *removed_device;
> +};
> +
> +static inline struct media_device_shared *
> +to_media_device_shared(struct media_device *mdev)
> +{
> + return container_of(mdev, struct media_device_shared, mdev);
> +}
> +
> +static void media_device_shared_release(struct kref *kref)
> +{
> + struct media_device_shared *mds =
> + container_of(kref, struct media_device_shared, refcount);
> +
> + dev_dbg(mds->removed_device, "%s: releasing Media Device\n", __func__);
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + media_device_unregister(&mds->mdev);
> + media_device_cleanup(&mds->mdev);
> +
> + list_del(&mds->list);
> + mutex_unlock(&media_device_shared_lock);
> +
> + kfree(mds);
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static bool __media_device_shared_find_match(struct media_device_shared *mds,
> + struct fwnode_handle *fwnode)
> +{
> + struct media_device_shared_member *member;
> + struct fwnode_handle *ep;
> + struct fwnode_handle *remote_ep;
> + bool match = false;
> +
> + // TODO: parse the device tree endpoints graph instead of finding just the
> + // first-level neighbours
> + fwnode_graph_for_each_endpoint(fwnode, ep) {
> + list_for_each_entry(member, &mds->members, list) {
> + remote_ep = fwnode_graph_get_remote_port_parent(ep);
> + match = (member->fwnode == remote_ep);
> + fwnode_handle_put(remote_ep);
> +
> + if (!match)
> + continue;
> +
> + goto match_complete;
> + }
> + }
> +
> +match_complete:
> + fwnode_handle_put(ep);
> + return match;
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static struct media_device *__media_device_shared_get(struct device *dev)
> +{
> + struct media_device_shared *mds;
> + struct media_device_shared_member *member;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + bool ret;
> +
> + dev_dbg(dev, "%s: searching for media device for %pfwf", __func__, fwnode);
> +
> + list_for_each_entry(mds, &media_device_shared_list, list) {
> + ret = __media_device_shared_find_match(mds, fwnode);
> + if (ret)
> + break;
> + }
> +
> + if (!ret)
> + return NULL;
> +
> + member = kzalloc_obj(*member);
> + if (!member)
> + return NULL;
> +
> + member->dev = dev;
> + member->fwnode = fwnode;
> + list_add_tail(&member->list, &mds->members);
> + kref_get(&mds->refcount);
> +
> + dev_dbg(dev, "%s: %pfwf joined media device of %pfwf",
> + __func__, fwnode,
> + list_first_entry(&mds->members, struct media_device_shared_member, list)->fwnode);
> +
> + return &mds->mdev;
> +}
> +
> +/* Callers should hold media_device_shared_lock when calling this function */
> +static struct media_device *__media_device_shared_create(struct device *dev)
> +{
> + struct media_device_shared *mds;
> + struct media_device_shared_member *member;
> + struct fwnode_handle *fwnode = dev_fwnode(dev);
> + int ret;
> +
> + mds = kzalloc_obj(*mds);
> + if (!mds)
> + return NULL;
> +
> + member = kzalloc_obj(*member);
> + if (!member)
> + goto err_free_mds;
> +
> + media_device_init(&mds->mdev);
> +
> + ret = media_device_register(&mds->mdev);
> + if (ret)
> + goto err_free_member;
> +
> + INIT_LIST_HEAD(&mds->members);
> + member->dev = dev;
> + member->fwnode = fwnode;
> + list_add_tail(&member->list, &mds->members);
> +
> + INIT_LIST_HEAD(&mds->links);
> +
> + kref_init(&mds->refcount);
> + list_add_tail(&mds->list, &media_device_shared_list);
> +
> + // TODO figure out how to reconcile this with multiple members
> + mds->mdev.dev = dev;
> +
> + devv_dbg(dev, "%s: Allocated media device with %pfwf at %p\n",
> + __func__, fwnode, &mds->mdev);
> + return &mds->mdev;
> +
> +err_free_member:
> + kfree(member);
> +err_free_mds:
> + kfree(mds);
> + return NULL;
> +}
> +
> +// TODO figure out how to resolve the identifiers (model, driver name, etc);
> +// atm it's racy and whoever gets it last wins
> +struct media_device *media_device_shared_join(struct device *dev)
> +{
> + struct media_device *mdev;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + mdev = __media_device_shared_get(dev);
> + if (!!mdev) {
Looks fun / geeky, but please don't, just do if (mdev). We really don't care
here if the value passed to the if is exactly 1 or some other non-nul value.
> + dev_dbg(dev, "%s: found media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> + }
> +
> + mdev = __media_device_shared_create(dev);
> + if (!mdev) {
> + dev_warn(dev, "%s: failed to create media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + dev_dbg(dev, "%s: created media device for %pfwf", __func__, dev_fwnode(dev));
> + mutex_unlock(&media_device_shared_lock);
> + return mdev;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join);
> +
> +void media_device_shared_leave(struct media_device *mdev, struct device *dev)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_member *member;
> + struct media_device_shared_member *member_tmp;
> + bool removed = false;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + list_for_each_entry_safe(member, member_tmp, &mds->members, list) {
> + if (member->dev == dev) {
> + list_del(&member->list);
> + kfree(member);
> + removed = true;
> + }
> + }
> +
> + if (!removed)
> + dev_err(dev, "%s: %pfwf trying to leave from graph in which not a member",
> + __func__, dev_fwnode(dev));
> +
> + mds->removed_device = dev;
> + mutex_unlock(&media_device_shared_lock);
> + kref_put(&mds->refcount, media_device_shared_release);
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_leave);
> +
> +int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_link *link;
> + struct media_device_shared_link *link_tmp;
> + int ret = 0;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + /*
> + * TODO Figure out flags. Should we use greatest common denominator? Or
> + * prioritize sink? Or whoever wins the race? For now we just take the flags
> + * from the sink.
> + *
> + * TODO Figure out how to actually do the matching. For now we just match
> + * whoever comes in first. This works with the simple example we're running
> + * with now (rkcif + one rkisp2) but with setups with multiple copies of
> + * hardware this will cause problems, like with rkcif + two rkisp2 and
> + * imx8-isi + two rkisp1.
> + */
> + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> + if (link->sink) {
> + ret = media_create_pad_link(source, source_pad,
> + link->sink, link->sink_pad,
> + link->flags);
> + list_del(&link->list);
> + kfree(link);
> + goto exit_join_link_source;
> + }
> + }
> +
> + link = kzalloc_obj(*link);
> + if (!link) {
> + ret = -ENOMEM;
> + goto exit_join_link_source;
> + }
> +
> + link->source = source;
> + link->source_pad = source_pad;
> + link->flags = flags;
> + list_add_tail(&link->list, &mds->links);
> +
> +exit_join_link_source:
> + mutex_unlock(&media_device_shared_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join_link_source);
> +
> +// TODO deduplicate from above
> +int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags)
> +{
> + struct media_device_shared *mds = to_media_device_shared(mdev);
> + struct media_device_shared_link *link;
> + struct media_device_shared_link *link_tmp;
> + int ret = 0;
> +
> + mutex_lock(&media_device_shared_lock);
> +
> + list_for_each_entry_safe(link, link_tmp, &mds->links, list) {
> + if (link->source) {
> + ret = media_create_pad_link(link->source, link->source_pad,
> + sink, sink_pad,
> + flags);
> + list_del(&link->list);
> + kfree(link);
> + goto exit_join_link_sink;
> + }
> + }
> +
> + link = kzalloc_obj(*link);
> + if (!link) {
> + ret = -ENOMEM;
> + goto exit_join_link_sink;
> + }
> +
> + link->sink = sink;
> + link->sink_pad = sink_pad;
> + link->flags = flags;
> + list_add_tail(&link->list, &mds->links);
> +
> +exit_join_link_sink:
> + mutex_unlock(&media_device_shared_lock);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(media_device_shared_join_link_sink);
> diff --git a/include/media/mc-shared-graph.h b/include/media/mc-shared-graph.h
> new file mode 100644
> index 000000000000..487325163f84
> --- /dev/null
> +++ b/include/media/mc-shared-graph.h
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * mc-shared-graph.h - Media Controller Shared Graph API
> + *
> + * Copyright (c) 2026 Paul Elder <paul.elder@ideasonboard.com>
> + */
> +
> +/*
> + * This file adds the Media Controller Shared Graph API. This allows drivers
> + * to create shared media graphs or join existing media graphs from other
> + * drivers, so that they can all be in the same media graph. This allows us to
> + * have more complex media graphs chaining more complex hardware together,
> + * instead of simple async subdevs.
> + */
> +
> +#include <linux/types.h>
> +
> +#ifndef _MEDIA_SHARED_GRAPH_H
> +#define _MEDIA_SHARED_GRAPH_H
> +
> +struct device;
> +struct media_device;
> +struct media_entity;
> +
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +/**
> + * media_device_shared_join() - Join or create a new shared media device
> + *
> + * @dev: struct &device pointer
> + *
> + * This is the entrance function for a device to join or create a new shared
> + * media device. It searches for an existing shared media device based on the
> + * neighbours in the device's device tree ports node. If found, then this
> + * functions returns the existing shared media device and joins it. If one is
> + * not found then one is created and initialized and returned.
> + */
> +struct media_device *media_device_shared_join(struct device *dev);
> +
> +/**
> + * media_device_shared_leave() - Leave the shared media device.
> + *
> + * @mdev: struct &media_device pointer
> + * @dev: struct &device pointer
> + *
> + * This function makes the device leave the shared media device. When all
> + * members have left the media device it will be freed.
> + */
> +void media_device_shared_leave(struct media_device *mdev, struct device *dev);
> +
> +/**
> + * media_device_shared_join_link_source() - Register a link source in the shared media device
> + *
> + * @mdev: The struct &media_device pointer that is part of a shared media device
> + * @dev: struct &device pointer
> + * @source: The link source
> + * @source_pad: The pad
> + * @flags: The flags
> + *
> + * This function registers with the shared media device the source part of a
> + * link. When the shared media device receives the matching sink part of a link
> + * via media_device_shared_join_link_sink() then the link will be fully created.
> + */
> +int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags);
> +
> +/**
> + * media_device_shared_join_link_sink() - Register a link sink in the shared media device
> + *
> + * Same as media_device_shared_join_link_source() but for sink instead of
> + * source.
> + */
> +int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags);
> +#else
> +static inline struct media_device *media_device_shared_join(struct device *dev)
> +{ return NULL; }
> +static inline void media_device_shared_leave(struct media_device *mdev,
> + struct device *dev) { }
> +static inline int media_device_shared_join_link_source(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *source,
> + u16 source_pad, u32 flags) { }
> +static inline int media_device_shared_join_link_sink(struct media_device *mdev,
> + struct device *dev,
> + struct media_entity *sink,
> + u16 sink_pad, u32 flags) { }
> +#endif /* CONFIG_MEDIA_CONTROLLER */
> +#endif /* _MEDIA_DEV_SHARED_GRAPH_H */
I've checked the request media ops, and tried to project how request could be
placed back into this. The main use of the ops is to extend request structure.
And any kind of scheduling will require that. Though, a scheduler based on
request inside a shared media graph should probably be media graph specific.
All in all, it makes me thing that we should plan for something less
generic/storage like, and perhaps allow for for ops by a non-device driver, that
could then further customize scheduling and implement request ?
Nicolas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 2/6] arm64: dts: rockchip: rk3588s-base: Connect vicap and isps
2026-06-19 5:26 [RFC PATCH 0/6] Add Shared Media Graph API Paul Elder
2026-06-19 5:26 ` [RFC PATCH 1/6] media: mc: Implement shared media graph Paul Elder
@ 2026-06-19 5:26 ` Paul Elder
2026-06-19 5:26 ` [RFC PATCH 3/6] media: rkcif: Use shared media graph Paul Elder
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Paul Elder @ 2026-06-19 5:26 UTC (permalink / raw)
To: laurent.pinchart
Cc: Paul Elder, michael.riesch, xuhf, stefan.klug, kieran.bingham,
dan.scally, jacopo.mondi, linux-media, linux-arm-kernel,
linux-rockchip, linux-kernel, hverkuil+cisco, nicolas.dufresne,
ribalda, sakari.ailus, Heiko Stuebner
Add a connection between VICAP and the two ISPs.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 55c5c603c1e3..f36267720910 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1493,10 +1493,16 @@ vicap_mipi5: port@6 {
vicap_toisp0: port@10 {
reg = <16>;
+ vicap_toisp0_ep: endpoint {
+ remote-endpoint = <&isp0_tovicap>;
+ };
};
vicap_toisp1: port@11 {
reg = <17>;
+ vicap_toisp1_ep: endpoint {
+ remote-endpoint = <&isp1_tovicap>;
+ };
};
};
};
@@ -3551,6 +3557,18 @@ isp0: isp@fdcb0000 {
power-domains = <&power RK3588_PD_VI>;
iommus = <&isp0_mmu>;
status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ isp0_tovicap: endpoint {
+ remote-endpoint = <&vicap_toisp0_ep>;
+ };
+ };
+ };
};
isp0_mmu: iommu@fdcb7f00 {
@@ -3580,6 +3598,18 @@ isp1: isp@fdcc0000 {
power-domains = <&power RK3588_PD_ISP1>;
iommus = <&isp1_mmu>;
status = "disabled";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ isp1_tovicap: endpoint {
+ remote-endpoint = <&vicap_toisp1_ep>;
+ };
+ };
+ };
};
isp1_mmu: iommu@fdcc7f00 {
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 3/6] media: rkcif: Use shared media graph
2026-06-19 5:26 [RFC PATCH 0/6] Add Shared Media Graph API Paul Elder
2026-06-19 5:26 ` [RFC PATCH 1/6] media: mc: Implement shared media graph Paul Elder
2026-06-19 5:26 ` [RFC PATCH 2/6] arm64: dts: rockchip: rk3588s-base: Connect vicap and isps Paul Elder
@ 2026-06-19 5:26 ` Paul Elder
2026-06-19 5:26 ` [RFC PATCH 4/6] media: rkisp2: " Paul Elder
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Paul Elder @ 2026-06-19 5:26 UTC (permalink / raw)
To: laurent.pinchart
Cc: Paul Elder, michael.riesch, xuhf, stefan.klug, kieran.bingham,
dan.scally, jacopo.mondi, linux-media, linux-arm-kernel,
linux-rockchip, linux-kernel, hverkuil+cisco, nicolas.dufresne,
ribalda, sakari.ailus
Make rkcif use shared media graph. This allows it to be in the same
media graph as rkisp2 on the rk3588, opening to door to allowing the
entire capture pipeline to run in inline mode.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
.../platform/rockchip/rkcif/rkcif-common.h | 2 +-
.../media/platform/rockchip/rkcif/rkcif-dev.c | 32 +++++++++----------
.../platform/rockchip/rkcif/rkcif-interface.c | 10 ++++++
3 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-common.h b/drivers/media/platform/rockchip/rkcif/rkcif-common.h
index 4d9211ba9bda..f2989d152ba2 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-common.h
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-common.h
@@ -242,7 +242,7 @@ struct rkcif_device {
struct rkcif_interface interfaces[RKCIF_IF_MAX];
- struct media_device media_dev;
+ struct media_device *media_dev;
struct v4l2_device v4l2_dev;
struct v4l2_async_notifier notifier;
};
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-dev.c b/drivers/media/platform/rockchip/rkcif/rkcif-dev.c
index be3a174b9aab..4c86e3e2f3cd 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-dev.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-dev.c
@@ -20,6 +20,7 @@
#include <linux/pm_runtime.h>
#include <linux/reset.h>
+#include <media/mc-shared-graph.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-mc.h>
@@ -165,6 +166,7 @@ static int rkcif_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct rkcif_device *rkcif;
+ struct media_device *mdev;
int ret, irq;
rkcif = devm_kzalloc(dev, sizeof(*rkcif), GFP_KERNEL);
@@ -212,22 +214,22 @@ static int rkcif_probe(struct platform_device *pdev)
pm_runtime_enable(&pdev->dev);
- rkcif->media_dev.dev = dev;
- strscpy(rkcif->media_dev.model, RKCIF_DRIVER_NAME,
- sizeof(rkcif->media_dev.model));
- media_device_init(&rkcif->media_dev);
+ mdev = media_device_shared_join(rkcif->dev);
+ if (IS_ERR(mdev)) {
+ dev_err(dev, "failed to register media device: %d\n", ret);
+ goto err_pm_runtime_disable;
+ }
+
+ rkcif->media_dev = mdev;
+ rkcif->media_dev->dev = dev;
+ strscpy(rkcif->media_dev->model, RKCIF_DRIVER_NAME,
+ sizeof(rkcif->media_dev->model));
- rkcif->v4l2_dev.mdev = &rkcif->media_dev;
+ rkcif->v4l2_dev.mdev = rkcif->media_dev;
ret = v4l2_device_register(dev, &rkcif->v4l2_dev);
if (ret)
goto err_media_dev_cleanup;
- ret = media_device_register(&rkcif->media_dev);
- if (ret < 0) {
- dev_err(dev, "failed to register media device: %d\n", ret);
- goto err_v4l2_dev_unregister;
- }
-
v4l2_async_nf_init(&rkcif->notifier, &rkcif->v4l2_dev);
rkcif->notifier.ops = &rkcif_notifier_ops;
@@ -247,11 +249,10 @@ static int rkcif_probe(struct platform_device *pdev)
rkcif_unregister(rkcif);
err_notifier_cleanup:
v4l2_async_nf_cleanup(&rkcif->notifier);
- media_device_unregister(&rkcif->media_dev);
-err_v4l2_dev_unregister:
v4l2_device_unregister(&rkcif->v4l2_dev);
err_media_dev_cleanup:
- media_device_cleanup(&rkcif->media_dev);
+ media_device_shared_leave(rkcif->media_dev, rkcif->dev);
+err_pm_runtime_disable:
pm_runtime_disable(&pdev->dev);
return ret;
}
@@ -263,9 +264,8 @@ static void rkcif_remove(struct platform_device *pdev)
v4l2_async_nf_unregister(&rkcif->notifier);
rkcif_unregister(rkcif);
v4l2_async_nf_cleanup(&rkcif->notifier);
- media_device_unregister(&rkcif->media_dev);
v4l2_device_unregister(&rkcif->v4l2_dev);
- media_device_cleanup(&rkcif->media_dev);
+ media_device_shared_leave(rkcif->media_dev, rkcif->dev);
pm_runtime_disable(&pdev->dev);
}
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-interface.c b/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
index 414a9980cf2e..cd791186f224 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
@@ -6,6 +6,7 @@
* Copyright (C) 2025 Collabora, Ltd.
*/
+#include <media/mc-shared-graph.h>
#include <media/v4l2-common.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-mc.h>
@@ -399,6 +400,15 @@ int rkcif_interface_register(struct rkcif_device *rkcif,
if (ret)
goto err_subdev_unregister;
+ ret = media_device_shared_join_link_source(interface->rkcif->media_dev,
+ interface->rkcif->dev,
+ &interface->sd.entity,
+ RKCIF_IF_PAD_SRC,
+ 0);
+ if (ret)
+ goto err_subdev_unregister;
+
+
return 0;
err_subdev_unregister:
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 4/6] media: rkisp2: Use shared media graph
2026-06-19 5:26 [RFC PATCH 0/6] Add Shared Media Graph API Paul Elder
` (2 preceding siblings ...)
2026-06-19 5:26 ` [RFC PATCH 3/6] media: rkcif: Use shared media graph Paul Elder
@ 2026-06-19 5:26 ` Paul Elder
2026-06-19 5:26 ` [RFC PATCH 5/6] media: rkcif: Implement inline mode Paul Elder
2026-06-19 5:26 ` [RFC PATCH 6/6] media: rkisp2: " Paul Elder
5 siblings, 0 replies; 14+ messages in thread
From: Paul Elder @ 2026-06-19 5:26 UTC (permalink / raw)
To: laurent.pinchart
Cc: Paul Elder, michael.riesch, xuhf, stefan.klug, kieran.bingham,
dan.scally, jacopo.mondi, linux-media, linux-arm-kernel,
linux-rockchip, linux-kernel, hverkuil+cisco, nicolas.dufresne,
ribalda, sakari.ailus
Make rkisp2 use shared media graph. This allows it to be in the same
media graph as rkcif on the rk3588, opening to door to allowing the
entire capture pipeline to run in inline mode.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
.../platform/rockchip/rkisp2/rkisp2-common.h | 2 +-
.../platform/rockchip/rkisp2/rkisp2-dev.c | 42 ++++++++++---------
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h b/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h
index 1eafdb5db5d8..ecf0f5e22064 100644
--- a/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h
+++ b/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h
@@ -432,7 +432,7 @@ struct rkisp2_device {
struct regmap *gasket;
unsigned int gasket_id;
struct v4l2_device v4l2_dev;
- struct media_device media_dev;
+ struct media_device *media_dev;
struct v4l2_async_notifier notifier;
struct v4l2_subdev *source;
struct rkisp2_isp isp;
diff --git a/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c b/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c
index 4042bf43d287..f74b7aae3159 100644
--- a/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c
+++ b/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c
@@ -20,6 +20,7 @@
#include <linux/platform_device.h>
#include <linux/pinctrl/consumer.h>
#include <linux/pm_runtime.h>
+#include <media/mc-shared-graph.h>
#include <media/v4l2-fwnode.h>
#include <media/v4l2-mc.h>
@@ -117,7 +118,7 @@ static int rkisp2_create_links(struct rkisp2_device *rkisp2)
ret = media_create_pad_link(
source, 0, &rkisp2->isp.sd.entity,
RKISP2_ISP_PAD_SINK_VIDEO,
- MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE);
+ MEDIA_LNK_FL_ENABLED);
} else {
ret = media_create_pad_link(source, 0,
&rkisp2->isp.sd.entity,
@@ -147,6 +148,12 @@ static int rkisp2_create_links(struct rkisp2_device *rkisp2)
if (ret)
return ret;
+ ret = media_device_shared_join_link_sink(rkisp2->media_dev, rkisp2->dev,
+ &rkisp2->isp.sd.entity,
+ RKISP2_ISP_PAD_SINK_VIDEO, 0);
+ if (ret)
+ return ret;
+
return 0;
}
@@ -233,6 +240,7 @@ static int rkisp2_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct rkisp2_device *rkisp2;
struct v4l2_device *v4l2_dev;
+ struct media_device *mdev;
unsigned int i;
int ret, irq;
u32 cif_id;
@@ -298,29 +306,28 @@ static int rkisp2_probe(struct platform_device *pdev)
pm_runtime_put(&pdev->dev);
- rkisp2->media_dev.hw_revision = info->isp_ver;
- strscpy(rkisp2->media_dev.model, RKISP2_DRIVER_NAME,
- sizeof(rkisp2->media_dev.model));
- rkisp2->media_dev.dev = &pdev->dev;
- strscpy(rkisp2->media_dev.bus_info, RKISP2_BUS_INFO,
- sizeof(rkisp2->media_dev.bus_info));
- media_device_init(&rkisp2->media_dev);
+ mdev = media_device_shared_join(rkisp2->dev);
+ if (IS_ERR(mdev))
+ goto err_pm_runtime_disable;
+
+ rkisp2->media_dev = mdev;
+ rkisp2->media_dev->hw_revision = info->isp_ver;
+ strscpy(rkisp2->media_dev->model, RKISP2_DRIVER_NAME,
+ sizeof(rkisp2->media_dev->model));
+ strscpy(rkisp2->media_dev->bus_info, RKISP2_BUS_INFO,
+ sizeof(rkisp2->media_dev->bus_info));
v4l2_dev = &rkisp2->v4l2_dev;
- v4l2_dev->mdev = &rkisp2->media_dev;
+ v4l2_dev->mdev = rkisp2->media_dev;
strscpy(v4l2_dev->name, RKISP2_DRIVER_NAME, sizeof(v4l2_dev->name));
ret = v4l2_device_register(rkisp2->dev, &rkisp2->v4l2_dev);
if (ret)
goto err_media_dev_cleanup;
- ret = media_device_register(&rkisp2->media_dev);
- if (ret)
- goto err_unreg_v4l2_dev;
-
ret = rkisp2_entities_register(rkisp2);
if (ret)
- goto err_unreg_media_dev;
+ goto err_unreg_v4l2_dev;
ret = v4l2_device_register_subdev_nodes(&rkisp2->v4l2_dev);
if (ret)
@@ -332,12 +339,10 @@ static int rkisp2_probe(struct platform_device *pdev)
err_unreg_entities:
rkisp2_entities_unregister(rkisp2);
-err_unreg_media_dev:
- media_device_unregister(&rkisp2->media_dev);
err_unreg_v4l2_dev:
v4l2_device_unregister(&rkisp2->v4l2_dev);
err_media_dev_cleanup:
- media_device_cleanup(&rkisp2->media_dev);
+ media_device_shared_leave(rkisp2->media_dev, rkisp2->dev);
err_pm_runtime_disable:
pm_runtime_disable(&pdev->dev);
return ret;
@@ -353,10 +358,9 @@ static void rkisp2_remove(struct platform_device *pdev)
rkisp2_entities_unregister(rkisp2);
rkisp2_debug_cleanup(rkisp2);
- media_device_unregister(&rkisp2->media_dev);
v4l2_device_unregister(&rkisp2->v4l2_dev);
- media_device_cleanup(&rkisp2->media_dev);
+ media_device_shared_leave(rkisp2->media_dev, rkisp2->dev);
pm_runtime_disable(&pdev->dev);
}
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 5/6] media: rkcif: Implement inline mode
2026-06-19 5:26 [RFC PATCH 0/6] Add Shared Media Graph API Paul Elder
` (3 preceding siblings ...)
2026-06-19 5:26 ` [RFC PATCH 4/6] media: rkisp2: " Paul Elder
@ 2026-06-19 5:26 ` Paul Elder
2026-06-19 5:26 ` [RFC PATCH 6/6] media: rkisp2: " Paul Elder
5 siblings, 0 replies; 14+ messages in thread
From: Paul Elder @ 2026-06-19 5:26 UTC (permalink / raw)
To: laurent.pinchart
Cc: Paul Elder, michael.riesch, xuhf, stefan.klug, kieran.bingham,
dan.scally, jacopo.mondi, linux-media, linux-arm-kernel,
linux-rockchip, linux-kernel, hverkuil+cisco, nicolas.dufresne,
ribalda, sakari.ailus
Add support to the rkcif for inline mode. Switching between offline mode
and inline mode is done by disabling/enabling the link between the rkcif
and rkisp2.
As the link is on a source pad on rkcif-interface, rkcif-mipi is
bypassed. This is why s_stream is implemented for rkcif_interface and it
calls enable_streams on the remote on the sink pad. Also since
rkcif_mipi_isr is called unconditionally it is skipped when in inline
mode.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
This patch is meant to serve as an example of how one might implement
inline mode. Despite it being "just an example", it has been tested and
captures properly, including loading rkcif and rkisp2 in differing
orders, and swapping between offline mode and inline mode (though
capturing in the wrong mode is still a bit problematic).
The shared media graph that was added in an earlier patch has made media
graph manipulation a non-issue. Although the API for switching between
inline mode and offline can be debated (here it is done with link
manipulation via link_setup), that is not a relevant topic for this
series.
---
.../rockchip/rkcif/rkcif-capture-dvp.c | 2 +-
.../rockchip/rkcif/rkcif-capture-mipi.c | 3 +
.../platform/rockchip/rkcif/rkcif-common.h | 16 +-
.../platform/rockchip/rkcif/rkcif-interface.c | 240 +++++++++++++++++-
.../platform/rockchip/rkcif/rkcif-regs.h | 49 ++++
.../platform/rockchip/rkcif/rkcif-stream.c | 6 +-
6 files changed, 301 insertions(+), 15 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-capture-dvp.c b/drivers/media/platform/rockchip/rkcif/rkcif-capture-dvp.c
index dbaf7636aeeb..6aa1031ac330 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-capture-dvp.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-capture-dvp.c
@@ -666,7 +666,7 @@ static int rkcif_dvp_start_streaming(struct rkcif_stream *stream)
int ret = -EINVAL;
state = v4l2_subdev_lock_and_get_active_state(&interface->sd);
- source_fmt = v4l2_subdev_state_get_format(state, RKCIF_IF_PAD_SRC,
+ source_fmt = v4l2_subdev_state_get_format(state, RKCIF_IF_PAD_SRC_DMA,
stream->id);
if (!source_fmt)
goto out;
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
index bc9518f8db50..50050cfa83b0 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-capture-mipi.c
@@ -821,6 +821,9 @@ irqreturn_t rkcif_mipi_isr(int irq, void *ctx)
enum rkcif_interface_index index = RKCIF_MIPI_BASE + i;
struct rkcif_interface *interface = &rkcif->interfaces[index];
+ if (interface->inline_mode)
+ continue;
+
intstat = rkcif_mipi_read(interface, RKCIF_MIPI_INTSTAT);
rkcif_mipi_write(interface, RKCIF_MIPI_INTSTAT, intstat);
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-common.h b/drivers/media/platform/rockchip/rkcif/rkcif-common.h
index f2989d152ba2..cf9322cae7a9 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-common.h
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-common.h
@@ -58,7 +58,8 @@ enum rkcif_interface_index {
enum rkcif_interface_pad_index {
RKCIF_IF_PAD_SINK,
- RKCIF_IF_PAD_SRC,
+ RKCIF_IF_PAD_SRC_DMA,
+ RKCIF_IF_PAD_SRC_TOISP,
RKCIF_IF_PAD_MAX
};
@@ -194,6 +195,8 @@ struct rkcif_interface {
struct v4l2_fwnode_endpoint vep;
struct v4l2_subdev sd;
+ bool inline_mode;
+
union {
struct rkcif_dvp dvp;
};
@@ -247,4 +250,15 @@ struct rkcif_device {
struct v4l2_async_notifier notifier;
};
+static inline void
+rkcif_write(struct rkcif_device *rkcif, unsigned int addr, u32 val)
+{
+ writel(val, rkcif->base_addr + addr);
+}
+
+static inline u32 rkcif_read(struct rkcif_device *rkcif, unsigned int addr)
+{
+ return readl(rkcif->base_addr + addr);
+}
+
#endif
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-interface.c b/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
index cd791186f224..568835e47f92 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-interface.c
@@ -6,6 +6,8 @@
* Copyright (C) 2025 Collabora, Ltd.
*/
+#include <linux/pm_runtime.h>
+
#include <media/mc-shared-graph.h>
#include <media/v4l2-common.h>
#include <media/v4l2-fwnode.h>
@@ -20,8 +22,118 @@ static inline struct rkcif_interface *to_rkcif_interface(struct v4l2_subdev *sd)
return container_of(sd, struct rkcif_interface, sd);
}
+static u16 rkcif_interface_get_active_source_pad(struct rkcif_interface *interface)
+{
+ struct media_entity *entity = &interface->sd.entity;
+ struct media_link *link;
+
+ list_for_each_entry(link, &entity->links, list) {
+ if (link->source->entity != entity ||
+ (link->source->index != RKCIF_IF_PAD_SRC_DMA &&
+ link->source->index != RKCIF_IF_PAD_SRC_TOISP))
+ continue;
+
+ if (link->flags & MEDIA_LNK_FL_ENABLED) {
+ dev_dbg(interface->rkcif->dev, "%s: active link is %d\n",
+ __func__, link->source->index);
+ return link->source->index;
+ }
+ }
+
+ /* Default to DMA if neither link is active */
+ return RKCIF_IF_PAD_SRC_DMA;
+}
+
+static u32 rkcif_interface_mipi_dt(u32 fourcc)
+{
+ switch (fourcc) {
+ case MEDIA_BUS_FMT_SRGGB8_1X8:
+ case MEDIA_BUS_FMT_SBGGR8_1X8:
+ case MEDIA_BUS_FMT_SGBRG8_1X8:
+ case MEDIA_BUS_FMT_SGRBG8_1X8:
+ return RKCIF_CSI2_DT_RAW8;
+ case MEDIA_BUS_FMT_SRGGB10_1X10:
+ case MEDIA_BUS_FMT_SBGGR10_1X10:
+ case MEDIA_BUS_FMT_SGBRG10_1X10:
+ case MEDIA_BUS_FMT_SGRBG10_1X10:
+ return RKCIF_CSI2_DT_RAW10;
+ case MEDIA_BUS_FMT_SRGGB12_1X12:
+ case MEDIA_BUS_FMT_SBGGR12_1X12:
+ case MEDIA_BUS_FMT_SGBRG12_1X12:
+ case MEDIA_BUS_FMT_SGRBG12_1X12:
+ return RKCIF_CSI2_DT_RAW12;
+ default:
+ return RKCIF_CSI2_DT_RAW10;
+ }
+}
+
+static u32 rkcif_interface_mipi_parse_type(u32 fourcc)
+{
+ switch (fourcc) {
+ case MEDIA_BUS_FMT_SRGGB8_1X8:
+ case MEDIA_BUS_FMT_SBGGR8_1X8:
+ case MEDIA_BUS_FMT_SGBRG8_1X8:
+ case MEDIA_BUS_FMT_SGRBG8_1X8:
+ return RKCIF_MIPI_PARSE_TYPE_RAW8_RGB888;
+ case MEDIA_BUS_FMT_SRGGB10_1X10:
+ case MEDIA_BUS_FMT_SBGGR10_1X10:
+ case MEDIA_BUS_FMT_SGBRG10_1X10:
+ case MEDIA_BUS_FMT_SGRBG10_1X10:
+ return RKCIF_MIPI_PARSE_TYPE_RAW10;
+ case MEDIA_BUS_FMT_SRGGB12_1X12:
+ case MEDIA_BUS_FMT_SBGGR12_1X12:
+ case MEDIA_BUS_FMT_SGBRG12_1X12:
+ case MEDIA_BUS_FMT_SGRBG12_1X12:
+ return RKCIF_MIPI_PARSE_TYPE_RAW12;
+ default:
+ return RKCIF_MIPI_PARSE_TYPE_RAW10;
+ }
+}
+
+static int rkcif_interface_subdev_link_setup(struct media_entity *entity,
+ const struct media_pad *local_pad,
+ const struct media_pad *remote_pad,
+ u32 flags)
+{
+ struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+ struct rkcif_interface *interface = to_rkcif_interface(sd);
+ struct media_link *link;
+ u16 other_source_pad_index;
+
+ dev_dbg(interface->rkcif->dev, "link setup %s -> %s\n",
+ local_pad->entity->name, remote_pad->entity->name);
+
+ /* We only care about links being created on a source pad */
+ if (!(flags & MEDIA_LNK_FL_ENABLED) ||
+ !(local_pad->flags & MEDIA_PAD_FL_SOURCE) ||
+ (local_pad->index != RKCIF_IF_PAD_SRC_DMA &&
+ local_pad->index != RKCIF_IF_PAD_SRC_TOISP))
+ return 0;
+
+ other_source_pad_index =
+ local_pad->index == RKCIF_IF_PAD_SRC_DMA ?
+ RKCIF_IF_PAD_SRC_TOISP :
+ RKCIF_IF_PAD_SRC_DMA;
+
+ list_for_each_entry(link, &entity->links, list) {
+ if (link->source->entity != local_pad->entity ||
+ link->source->index != other_source_pad_index)
+ continue;
+
+ /*
+ * If we are trying to enable DMA source pad but the TOISP
+ * source pad (and vice versa) has an enabled link then return
+ * error
+ */
+ return link->flags & MEDIA_LNK_FL_ENABLED ? -EBUSY : 0;
+ }
+
+ return 0;
+}
+
static const struct media_entity_operations rkcif_interface_media_ops = {
.link_validate = v4l2_subdev_link_validate,
+ .link_setup = rkcif_interface_subdev_link_setup,
.has_pad_interdep = v4l2_subdev_has_pad_interdep,
};
@@ -37,7 +149,8 @@ static int rkcif_interface_set_fmt(struct v4l2_subdev *sd,
int ret;
/* the format on the source pad always matches the sink pad */
- if (format->pad == RKCIF_IF_PAD_SRC)
+ if (format->pad == RKCIF_IF_PAD_SRC_DMA ||
+ format->pad == RKCIF_IF_PAD_SRC_TOISP)
return v4l2_subdev_get_fmt(sd, state, format);
input = rkcif_interface_find_input_fmt(interface, true,
@@ -85,7 +198,8 @@ static int rkcif_interface_get_sel(struct v4l2_subdev *sd,
struct v4l2_rect *crop;
int ret = 0;
- if (sel->pad != RKCIF_IF_PAD_SRC)
+ if (sel->pad != RKCIF_IF_PAD_SRC_DMA &&
+ sel->pad != RKCIF_IF_PAD_SRC_TOISP)
return -EINVAL;
sink = v4l2_subdev_state_get_opposite_stream_format(state, sel->pad,
@@ -122,7 +236,8 @@ static int rkcif_interface_set_sel(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *sink, *src;
struct v4l2_rect *crop;
- if (sel->pad != RKCIF_IF_PAD_SRC || sel->target != V4L2_SEL_TGT_CROP)
+ if ((sel->pad != RKCIF_IF_PAD_SRC_DMA &&
+ sel->pad != RKCIF_IF_PAD_SRC_TOISP) || sel->target != V4L2_SEL_TGT_CROP)
return -EINVAL;
sink = v4l2_subdev_state_get_opposite_stream_format(state, sel->pad,
@@ -176,7 +291,10 @@ static int rkcif_interface_apply_crop(struct rkcif_stream *stream,
struct rkcif_interface *interface = stream->interface;
struct v4l2_rect *crop;
- crop = v4l2_subdev_state_get_crop(state, RKCIF_IF_PAD_SRC, stream->id);
+ crop = v4l2_subdev_state_get_crop(
+ state,
+ rkcif_interface_get_active_source_pad(interface),
+ stream->id);
if (!crop)
return -EINVAL;
@@ -195,8 +313,11 @@ static int rkcif_interface_enable_streams(struct v4l2_subdev *sd,
struct v4l2_subdev_route *route;
struct v4l2_subdev *remote_sd;
struct media_pad *remote_pad;
+ u32 active_pad = rkcif_interface_get_active_source_pad(interface);
u64 mask;
+ interface->inline_mode = (active_pad == RKCIF_IF_PAD_SRC_TOISP);
+
remote_pad =
media_pad_remote_pad_first(&sd->entity.pads[RKCIF_IF_PAD_SINK]);
remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
@@ -213,7 +334,7 @@ static int rkcif_interface_enable_streams(struct v4l2_subdev *sd,
}
mask = v4l2_subdev_state_xlate_streams(state, RKCIF_IF_PAD_SINK,
- RKCIF_IF_PAD_SRC, &streams_mask);
+ active_pad, &streams_mask);
return v4l2_subdev_enable_streams(remote_sd, remote_pad->index, mask);
}
@@ -222,6 +343,7 @@ static int rkcif_interface_disable_streams(struct v4l2_subdev *sd,
struct v4l2_subdev_state *state,
u32 pad, u64 streams_mask)
{
+ struct rkcif_interface *interface = to_rkcif_interface(sd);
struct v4l2_subdev *remote_sd;
struct media_pad *remote_pad;
u64 mask;
@@ -230,8 +352,9 @@ static int rkcif_interface_disable_streams(struct v4l2_subdev *sd,
media_pad_remote_pad_first(&sd->entity.pads[RKCIF_IF_PAD_SINK]);
remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
- mask = v4l2_subdev_state_xlate_streams(state, RKCIF_IF_PAD_SINK,
- RKCIF_IF_PAD_SRC, &streams_mask);
+ mask = v4l2_subdev_state_xlate_streams(
+ state, RKCIF_IF_PAD_SINK,
+ rkcif_interface_get_active_source_pad(interface), &streams_mask);
return v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
}
@@ -246,7 +369,94 @@ static const struct v4l2_subdev_pad_ops rkcif_interface_pad_ops = {
.disable_streams = rkcif_interface_disable_streams,
};
+static int rkcif_interface_s_stream(struct v4l2_subdev *sd, int enable)
+{
+ struct rkcif_interface *interface = to_rkcif_interface(sd);
+ struct rkcif_device *rkcif = interface->rkcif;
+ struct v4l2_subdev_state *state;
+ struct v4l2_subdev *remote_sd;
+ struct media_pad *remote_pad;
+ struct v4l2_rect *crop;
+ struct v4l2_mbus_framefmt *mbus;
+ int ret;
+ u32 active_pad = rkcif_interface_get_active_source_pad(interface);
+ u32 val, crop_val, offset_val;
+
+ interface->inline_mode = (active_pad == RKCIF_IF_PAD_SRC_TOISP);
+
+ remote_pad =
+ media_pad_remote_pad_first(&sd->entity.pads[RKCIF_IF_PAD_SINK]);
+ remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+
+ if (!enable) {
+ rkcif_write(rkcif, RKCIF_MIPI2_ID0_CTRL0, 0);
+ rkcif_write(rkcif, RKCIF_MIPI2_CTRL, 0);
+
+ ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, 1);
+
+ pm_runtime_put(rkcif->dev);
+ return ret;
+ }
+
+ ret = pm_runtime_resume_and_get(rkcif->dev);
+ if (ret < 0) {
+ dev_err(rkcif->dev, "failed to get runtime pm, %d\n", ret);
+ return ret;
+ }
+
+ state = v4l2_subdev_lock_and_get_active_state(sd);
+ crop = v4l2_subdev_state_get_crop(state, active_pad);
+ mbus = v4l2_subdev_state_get_format(state, active_pad);
+ v4l2_subdev_unlock_state(state);
+
+ /*
+ * Enable interrupts:
+ * - toisp0 ch{0,1,2} frame start
+ * - toisp1 ch{0,1,2} frame start
+ * - toisp0 ch{0,1,2} frame end
+ * - toisp1 ch{0,1,2} frame end
+ * - toisp0 fifo overflow
+ * - toisp1 fifo overflow
+ * - axi bus errors
+ */
+ rkcif_write(rkcif, RKCIF_GLB_INTEN, 0xFFFC003);
+
+ /*
+ * - Enable toisp ch0
+ * - Select MIPI2 ID0
+ */
+ val = 0x01;
+ val |= 8 << 3;
+ rkcif_write(rkcif, RKCIF_TOISP0_CH_CTRL, val);
+
+ crop_val = RKCIF_XY_COORD(3840, 2160);
+ offset_val = RKCIF_XY_COORD(0, 0);
+ if (!!crop) {
+ crop_val = RKCIF_XY_COORD(crop->width, crop->height);
+ offset_val = RKCIF_XY_COORD(crop->left, crop->top);
+ }
+
+ rkcif_write(rkcif, RKCIF_TOISP0_CROP_SIZE, crop_val);
+ rkcif_write(rkcif, RKCIF_TOISP0_CROP_START, offset_val);
+
+ val = rkcif_interface_mipi_parse_type(mbus ? mbus->code : 0);
+ val |= RKCIF_MIPI_DATA_SEL_DT(
+ rkcif_interface_mipi_dt(mbus ? mbus->code : 0));
+ val |= RKCIF_MIPI_CAP_EN;
+
+ rkcif_write(rkcif, RKCIF_MIPI2_ID0_CTRL0, val);
+ rkcif_write(rkcif, RKCIF_MIPI2_ID0_CTRL1, crop_val);
+ rkcif_write(rkcif, RKCIF_MIPI2_CTRL, RKCIF_MIPI_CAP_EN);
+
+ return v4l2_subdev_enable_streams(remote_sd, remote_pad->index, 1);
+}
+
+static const struct v4l2_subdev_video_ops rkcif_interface_video_ops = {
+ .s_stream = rkcif_interface_s_stream,
+};
+
static const struct v4l2_subdev_ops rkcif_interface_ops = {
+ .video = &rkcif_interface_video_ops,
.pad = &rkcif_interface_pad_ops,
};
@@ -258,7 +468,14 @@ static int rkcif_interface_init_state(struct v4l2_subdev *sd,
{
.sink_pad = RKCIF_IF_PAD_SINK,
.sink_stream = 0,
- .source_pad = RKCIF_IF_PAD_SRC,
+ .source_pad = RKCIF_IF_PAD_SRC_DMA,
+ .source_stream = 0,
+ .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ },
+ {
+ .sink_pad = RKCIF_IF_PAD_SINK,
+ .sink_stream = 0,
+ .source_pad = RKCIF_IF_PAD_SRC_TOISP,
.source_stream = 0,
.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
},
@@ -373,6 +590,8 @@ int rkcif_interface_register(struct rkcif_device *rkcif,
sd->internal_ops = &rkcif_interface_internal_ops;
sd->owner = THIS_MODULE;
+ interface->inline_mode = false;
+
if (interface->type == RKCIF_IF_DVP)
snprintf(sd->name, sizeof(sd->name), "rkcif-dvp0");
else if (interface->type == RKCIF_IF_MIPI)
@@ -381,7 +600,8 @@ int rkcif_interface_register(struct rkcif_device *rkcif,
pads[RKCIF_IF_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
MEDIA_PAD_FL_MUST_CONNECT;
- pads[RKCIF_IF_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
+ pads[RKCIF_IF_PAD_SRC_DMA].flags = MEDIA_PAD_FL_SOURCE;
+ pads[RKCIF_IF_PAD_SRC_TOISP].flags = MEDIA_PAD_FL_SOURCE;
ret = media_entity_pads_init(&sd->entity, RKCIF_IF_PAD_MAX, pads);
if (ret)
goto err;
@@ -403,7 +623,7 @@ int rkcif_interface_register(struct rkcif_device *rkcif,
ret = media_device_shared_join_link_source(interface->rkcif->media_dev,
interface->rkcif->dev,
&interface->sd.entity,
- RKCIF_IF_PAD_SRC,
+ RKCIF_IF_PAD_SRC_TOISP,
0);
if (ret)
goto err_subdev_unregister;
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-regs.h b/drivers/media/platform/rockchip/rkcif/rkcif-regs.h
index 3cf7ee19de30..3addd2aac691 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-regs.h
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-regs.h
@@ -150,4 +150,53 @@ enum rkcif_mipi_id_register_index {
RKCIF_MIPI_ID_REGISTER_MAX
};
+#define RKCIF_BASE 0x00000000
+#define RKCIF_GLB_CTRL (RKCIF_BASE + 0x00000000)
+#define RKCIF_GLB_INTEN (RKCIF_BASE + 0x00000004)
+#define RKCIF_GLB_INTST (RKCIF_BASE + 0x00000008)
+#define RKCIF_TOISP0_CH_CTRL (RKCIF_BASE + 0x00000780)
+#define RKCIF_TOISP0_CROP_SIZE (RKCIF_BASE + 0x00000784)
+#define RKCIF_TOISP0_CROP_START (RKCIF_BASE + 0x00000788)
+#define RKCIF_TOISP1_CH_CTRL (RKCIF_BASE + 0x0000078c)
+#define RKCIF_TOISP1_CROP_SIZE (RKCIF_BASE + 0x00000790)
+#define RKCIF_TOISP1_CROP_START (RKCIF_BASE + 0x00000794)
+
+#define RKCIF_MIPI2_BASE (RKCIF_BASE + 0x00000300)
+#define RKCIF_MIPI2_ID0_CTRL0 (RKCIF_MIPI2_BASE + 0x00000000)
+#define RKCIF_MIPI2_ID0_CTRL1 (RKCIF_MIPI2_BASE + 0x00000004)
+#define RKCIF_MIPI2_CTRL (RKCIF_MIPI2_BASE + 0x00000020)
+
+#define RKCIF_MIPI_CAP_EN BIT(0)
+#define RKCIF_MIPI_PARSE_TYPE_RAW8_RGB888 (0 << 1)
+#define RKCIF_MIPI_PARSE_TYPE_RAW10 (1 << 1)
+#define RKCIF_MIPI_PARSE_TYPE_RAW12 (2 << 1)
+#define RKCIF_MIPI_PARSE_TYPE_RAW14 (3 << 1)
+#define RKCIF_MIPI_PARSE_TYPE_YUV422_8BIT (4 << 1)
+#define RKCIF_MIPI_CROP_EN BIT(4)
+#define RKCIF_MIPI_WDDR_RAW_COMPACT (0 << 5)
+#define RKCIF_MIPI_WDDR_RAW_UNCOMPACT (1 << 5)
+#define RKCIF_MIPI_WDDR_YUV_PACKET (2 << 5)
+#define RKCIF_MIPI_WDDR_YUV400 (3 << 5)
+#define RKCIF_MIPI_WDDR_YUV422SP (4 << 5)
+#define RKCIF_MIPI_WDDR_YUV420SP (5 << 5)
+
+/* MIPI_DATA_SEL */
+#define RKCIF_MIPI_DATA_SEL_VC(a) (((a) & 0x3) << 8)
+#define RKCIF_MIPI_DATA_SEL_DT(a) (((a) & 0x3F) << 10)
+/* MIPI DATA_TYPE */
+/* These are copied from rkisp2 as they are not in the cif trm */
+#define RKCIF_CSI2_DT_EBD 0x12
+#define RKCIF_CSI2_DT_YUV420_8b 0x18
+#define RKCIF_CSI2_DT_YUV420_10b 0x19
+#define RKCIF_CSI2_DT_YUV422_8b 0x1E
+#define RKCIF_CSI2_DT_YUV422_10b 0x1F
+#define RKCIF_CSI2_DT_RGB565 0x22
+#define RKCIF_CSI2_DT_RGB666 0x23
+#define RKCIF_CSI2_DT_RGB888 0x24
+#define RKCIF_CSI2_DT_RAW8 0x2A
+#define RKCIF_CSI2_DT_RAW10 0x2B
+#define RKCIF_CSI2_DT_RAW12 0x2C
+#define RKCIF_CSI2_DT_RAW16 0x2e
+#define RKCIF_CSI2_DT_SPD 0x2F
+
#endif
diff --git a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
index 3130d420ad55..f173657fd2b4 100644
--- a/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
+++ b/drivers/media/platform/rockchip/rkcif/rkcif-stream.c
@@ -283,7 +283,7 @@ static int rkcif_stream_start_streaming(struct vb2_queue *queue,
mask = BIT_ULL(stream->id);
ret = v4l2_subdev_enable_streams(&stream->interface->sd,
- RKCIF_IF_PAD_SRC, mask);
+ RKCIF_IF_PAD_SRC_DMA, mask);
if (ret < 0)
goto err_stop_stream;
@@ -309,7 +309,7 @@ static void rkcif_stream_stop_streaming(struct vb2_queue *queue)
int ret;
mask = BIT_ULL(stream->id);
- v4l2_subdev_disable_streams(&stream->interface->sd, RKCIF_IF_PAD_SRC,
+ v4l2_subdev_disable_streams(&stream->interface->sd, RKCIF_IF_PAD_SRC_DMA,
mask);
stream->stopping = true;
@@ -589,7 +589,7 @@ int rkcif_stream_register(struct rkcif_device *rkcif,
if (stream->id == RKCIF_ID0)
link_flags |= MEDIA_LNK_FL_ENABLED;
- ret = media_create_pad_link(&interface->sd.entity, RKCIF_IF_PAD_SRC,
+ ret = media_create_pad_link(&interface->sd.entity, RKCIF_IF_PAD_SRC_DMA,
&stream->vdev.entity, 0, link_flags);
if (ret) {
dev_err(rkcif->dev, "failed to link stream media pad: %d\n",
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [RFC PATCH 6/6] media: rkisp2: Implement inline mode
2026-06-19 5:26 [RFC PATCH 0/6] Add Shared Media Graph API Paul Elder
` (4 preceding siblings ...)
2026-06-19 5:26 ` [RFC PATCH 5/6] media: rkcif: Implement inline mode Paul Elder
@ 2026-06-19 5:26 ` Paul Elder
5 siblings, 0 replies; 14+ messages in thread
From: Paul Elder @ 2026-06-19 5:26 UTC (permalink / raw)
To: laurent.pinchart
Cc: Paul Elder, michael.riesch, xuhf, stefan.klug, kieran.bingham,
dan.scally, jacopo.mondi, linux-media, linux-arm-kernel,
linux-rockchip, linux-kernel, hverkuil+cisco, nicolas.dufresne,
ribalda, sakari.ailus
Add support to rkisp2 for inline mode. Switching between offline mode
and inline mode is done by disabling/enabling the link between the
rkisp2 and rkcif.
As the link is on a sink pad on rkisp2-isp, rkisp2-dmarx is bypassed in
inline mode, because the v4l2_subdev_call() on the source will go to rkcif
instead of rkisp2-dmarx. Also DMA read only needs to be configured in
offline mode, while in inline mode there is no configuration necessary,
so the change to implement inline mode for rkisp2 is fairly lean.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
Although this patch is meant to serve as an example of how one might
implement inline mode, I think the actual implementation may very well
resemble this. The API for switching between inline mode and offline
is likely to be heavily debated, though that is not a topic for this
series.
Despite it being "just an example", it has been tested and
captures properly, including loading rkcif and rkisp2 in differing
orders, and swapping between offline mode and inline mode (though
capturing in the wrong mode is still a bit problematic).
The shared media graph that was added in an earlier patch has made
media graph manipulation trivial.
---
.../platform/rockchip/rkisp2/rkisp2-common.h | 3 +-
.../platform/rockchip/rkisp2/rkisp2-dev.c | 6 +-
.../platform/rockchip/rkisp2/rkisp2-isp.c | 155 ++++++++++++++----
3 files changed, 124 insertions(+), 40 deletions(-)
diff --git a/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h b/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h
index ecf0f5e22064..91ccb84b5a7a 100644
--- a/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h
+++ b/drivers/media/platform/rockchip/rkisp2/rkisp2-common.h
@@ -88,10 +88,11 @@ enum rkisp2_fmt_raw_pat_type {
/* enum for the isp pads */
enum rkisp2_isp_pad {
- RKISP2_ISP_PAD_SINK_VIDEO,
+ RKISP2_ISP_PAD_SINK_VIDEO_DMA,
RKISP2_ISP_PAD_SINK_PARAMS,
RKISP2_ISP_PAD_SOURCE_VIDEO,
RKISP2_ISP_PAD_SOURCE_STATS,
+ RKISP2_ISP_PAD_SINK_VIDEO_CIF,
RKISP2_ISP_PAD_MAX
};
diff --git a/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c b/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c
index f74b7aae3159..2b6b7ee31f1d 100644
--- a/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c
+++ b/drivers/media/platform/rockchip/rkisp2/rkisp2-dev.c
@@ -117,12 +117,12 @@ static int rkisp2_create_links(struct rkisp2_device *rkisp2)
if (i == RKISP2_RAWRD0) {
ret = media_create_pad_link(
source, 0, &rkisp2->isp.sd.entity,
- RKISP2_ISP_PAD_SINK_VIDEO,
+ RKISP2_ISP_PAD_SINK_VIDEO_DMA,
MEDIA_LNK_FL_ENABLED);
} else {
ret = media_create_pad_link(source, 0,
&rkisp2->isp.sd.entity,
- RKISP2_ISP_PAD_SINK_VIDEO,
+ RKISP2_ISP_PAD_SINK_VIDEO_DMA,
0);
}
@@ -150,7 +150,7 @@ static int rkisp2_create_links(struct rkisp2_device *rkisp2)
ret = media_device_shared_join_link_sink(rkisp2->media_dev, rkisp2->dev,
&rkisp2->isp.sd.entity,
- RKISP2_ISP_PAD_SINK_VIDEO, 0);
+ RKISP2_ISP_PAD_SINK_VIDEO_CIF, 0);
if (ret)
return ret;
diff --git a/drivers/media/platform/rockchip/rkisp2/rkisp2-isp.c b/drivers/media/platform/rockchip/rkisp2/rkisp2-isp.c
index 36c1aeed272f..9be5fb4cbd14 100644
--- a/drivers/media/platform/rockchip/rkisp2/rkisp2-isp.c
+++ b/drivers/media/platform/rockchip/rkisp2/rkisp2-isp.c
@@ -24,6 +24,27 @@
#define RKISP2_ISP_DEV_NAME RKISP2_DRIVER_NAME "_isp"
+static u16 rkisp2_isp_get_active_sink_pad(struct rkisp2_isp *isp)
+{
+ struct media_entity *entity = &isp->sd.entity;
+ struct media_link *link;
+
+ list_for_each_entry(link, &entity->links, list) {
+ if (link->sink->entity != entity ||
+ (link->sink->index != RKISP2_ISP_PAD_SINK_VIDEO_DMA &&
+ link->sink->index != RKISP2_ISP_PAD_SINK_VIDEO_CIF))
+ continue;
+
+ if (link->flags & MEDIA_LNK_FL_ENABLED) {
+ dev_dbg(isp->rkisp2->dev, "%s: active link is %d\n",
+ __func__, link->sink->index);
+ return link->sink->index;
+ }
+ }
+
+ /* Default to DMA if neither link is active */
+ return RKISP2_ISP_PAD_SINK_VIDEO_DMA;
+}
/* ----------------------------------------------------------------------------
* Camera Interface registers configurations
@@ -70,9 +91,9 @@ static int rkisp2_config_isp(struct rkisp2_isp *isp,
const struct v4l2_rect *sink_crop;
sink_frm = v4l2_subdev_state_get_format(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ rkisp2_isp_get_active_sink_pad(isp));
sink_crop = v4l2_subdev_state_get_crop(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ rkisp2_isp_get_active_sink_pad(isp));
src_frm = v4l2_subdev_state_get_format(sd_state,
RKISP2_ISP_PAD_SOURCE_VIDEO);
@@ -116,6 +137,12 @@ static int rkisp2_config_isp(struct rkisp2_isp *isp,
rkisp2_write(rkisp2, RKISP2_CIF_ISP_OUT_H_SIZE, sink_crop->width);
rkisp2_write(rkisp2, RKISP2_CIF_ISP_OUT_V_SIZE, sink_crop->height);
+ /*
+ * I think we don't need to configure cif source here because it seems
+ * like offline mode needs to explicitly configure CSI2RX in offline
+ * mode, but the default (all zero) is inline mode
+ */
+
irq_mask |= RKISP2_CIF_ISP_FRAME | RKISP2_CIF_ISP_V_START |
RKISP2_CIF_ISP_PIC_SIZE_ERROR;
rkisp2_write(rkisp2, RKISP2_CIF_ISP_IMSC, irq_mask);
@@ -273,7 +300,8 @@ static int rkisp2_isp_enum_mbus_code(struct v4l2_subdev *sd,
unsigned int i, dir;
int pos = 0;
- if (code->pad == RKISP2_ISP_PAD_SINK_VIDEO) {
+ if (code->pad == RKISP2_ISP_PAD_SINK_VIDEO_DMA ||
+ code->pad == RKISP2_ISP_PAD_SINK_VIDEO_CIF) {
dir = RKISP2_ISP_SD_SINK;
} else if (code->pad == RKISP2_ISP_PAD_SOURCE_VIDEO) {
dir = RKISP2_ISP_SD_SRC;
@@ -322,7 +350,8 @@ static int rkisp2_isp_enum_frame_size(struct v4l2_subdev *sd,
return -EINVAL;
if (!(mbus_info->direction & RKISP2_ISP_SD_SINK) &&
- fse->pad == RKISP2_ISP_PAD_SINK_VIDEO)
+ (fse->pad == RKISP2_ISP_PAD_SINK_VIDEO_DMA ||
+ fse->pad == RKISP2_ISP_PAD_SINK_VIDEO_CIF))
return -EINVAL;
if (!(mbus_info->direction & RKISP2_ISP_SD_SRC) &&
@@ -337,15 +366,14 @@ static int rkisp2_isp_enum_frame_size(struct v4l2_subdev *sd,
return 0;
}
-static int rkisp2_isp_init_state(struct v4l2_subdev *sd,
- struct v4l2_subdev_state *sd_state)
+static void rkisp2_isp_init_state_sink(struct v4l2_subdev_state *sd_state,
+ u16 pad)
{
- struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
- struct v4l2_rect *sink_crop, *src_crop;
+ struct v4l2_mbus_framefmt *sink_fmt;
+ struct v4l2_rect *sink_crop;
+
+ sink_fmt = v4l2_subdev_state_get_format(sd_state, pad);
- /* Video. */
- sink_fmt = v4l2_subdev_state_get_format(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
sink_fmt->width = RKISP2_DEFAULT_WIDTH;
sink_fmt->height = RKISP2_DEFAULT_HEIGHT;
sink_fmt->field = V4L2_FIELD_NONE;
@@ -355,13 +383,27 @@ static int rkisp2_isp_init_state(struct v4l2_subdev *sd,
sink_fmt->ycbcr_enc = V4L2_YCBCR_ENC_601;
sink_fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
- sink_crop = v4l2_subdev_state_get_crop(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ sink_crop = v4l2_subdev_state_get_crop(sd_state, pad);
sink_crop->width = RKISP2_DEFAULT_WIDTH;
sink_crop->height = RKISP2_DEFAULT_HEIGHT;
sink_crop->left = 0;
sink_crop->top = 0;
+}
+
+static int rkisp2_isp_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state)
+{
+ struct v4l2_mbus_framefmt *sink_fmt, *src_fmt;
+ struct v4l2_rect *sink_crop, *src_crop;
+
+ /* Video. */
+ rkisp2_isp_init_state_sink(sd_state, RKISP2_ISP_PAD_SINK_VIDEO_DMA);
+ rkisp2_isp_init_state_sink(sd_state, RKISP2_ISP_PAD_SINK_VIDEO_CIF);
+ sink_fmt = v4l2_subdev_state_get_format(sd_state,
+ RKISP2_ISP_PAD_SINK_VIDEO_DMA);
+ sink_crop = v4l2_subdev_state_get_crop(sd_state,
+ RKISP2_ISP_PAD_SINK_VIDEO_DMA);
src_fmt = v4l2_subdev_state_get_format(sd_state,
RKISP2_ISP_PAD_SOURCE_VIDEO);
*src_fmt = *sink_fmt;
@@ -390,7 +432,7 @@ static void rkisp2_isp_set_src_fmt(struct rkisp2_isp *isp,
bool set_csc;
sink_fmt = v4l2_subdev_state_get_format(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ rkisp2_isp_get_active_sink_pad(isp));
src_fmt = v4l2_subdev_state_get_format(sd_state,
RKISP2_ISP_PAD_SOURCE_VIDEO);
src_crop = v4l2_subdev_state_get_crop(sd_state,
@@ -490,7 +532,7 @@ static void rkisp2_isp_set_src_crop(struct rkisp2_isp *isp,
src_crop = v4l2_subdev_state_get_crop(sd_state,
RKISP2_ISP_PAD_SOURCE_VIDEO);
sink_crop = v4l2_subdev_state_get_crop(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ rkisp2_isp_get_active_sink_pad(isp));
src_crop->left = ALIGN(r->left, 2);
src_crop->width = ALIGN(r->width, 2);
@@ -508,15 +550,13 @@ static void rkisp2_isp_set_src_crop(struct rkisp2_isp *isp,
static void rkisp2_isp_set_sink_crop(struct rkisp2_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_rect *r)
+ struct v4l2_rect *r, u16 pad)
{
struct v4l2_rect *sink_crop, *src_crop;
const struct v4l2_mbus_framefmt *sink_fmt;
- sink_crop = v4l2_subdev_state_get_crop(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
- sink_fmt = v4l2_subdev_state_get_format(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ sink_crop = v4l2_subdev_state_get_crop(sd_state, pad);
+ sink_fmt = v4l2_subdev_state_get_format(sd_state, pad);
sink_crop->left = ALIGN(r->left, 2);
sink_crop->width = ALIGN(r->width, 2);
@@ -534,15 +574,14 @@ static void rkisp2_isp_set_sink_crop(struct rkisp2_isp *isp,
static void rkisp2_isp_set_sink_fmt(struct rkisp2_isp *isp,
struct v4l2_subdev_state *sd_state,
- struct v4l2_mbus_framefmt *format)
+ struct v4l2_mbus_framefmt *format, u16 pad)
{
const struct rkisp2_mbus_info *mbus_info;
struct v4l2_mbus_framefmt *sink_fmt;
struct v4l2_rect *sink_crop;
bool is_yuv;
- sink_fmt = v4l2_subdev_state_get_format(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ sink_fmt = v4l2_subdev_state_get_format(sd_state, pad);
sink_fmt->code = format->code;
mbus_info = rkisp2_mbus_info_get_by_code(sink_fmt->code);
if (!mbus_info || !(mbus_info->direction & RKISP2_ISP_SD_SINK)) {
@@ -590,9 +629,8 @@ static void rkisp2_isp_set_sink_fmt(struct rkisp2_isp *isp,
*format = *sink_fmt;
/* Propagate to in crop */
- sink_crop = v4l2_subdev_state_get_crop(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
- rkisp2_isp_set_sink_crop(isp, sd_state, sink_crop);
+ sink_crop = v4l2_subdev_state_get_crop(sd_state, pad);
+ rkisp2_isp_set_sink_crop(isp, sd_state, sink_crop, pad);
}
static int rkisp2_isp_set_fmt(struct v4l2_subdev *sd,
@@ -601,8 +639,9 @@ static int rkisp2_isp_set_fmt(struct v4l2_subdev *sd,
{
struct rkisp2_isp *isp = to_rkisp2_isp(sd);
- if (fmt->pad == RKISP2_ISP_PAD_SINK_VIDEO)
- rkisp2_isp_set_sink_fmt(isp, sd_state, &fmt->format);
+ if (fmt->pad == RKISP2_ISP_PAD_SINK_VIDEO_DMA ||
+ fmt->pad == RKISP2_ISP_PAD_SINK_VIDEO_CIF)
+ rkisp2_isp_set_sink_fmt(isp, sd_state, &fmt->format, fmt->pad);
else if (fmt->pad == RKISP2_ISP_PAD_SOURCE_VIDEO)
rkisp2_isp_set_src_fmt(isp, sd_state, &fmt->format);
else
@@ -619,12 +658,14 @@ static int rkisp2_isp_get_selection(struct v4l2_subdev *sd,
int ret = 0;
if (sel->pad != RKISP2_ISP_PAD_SOURCE_VIDEO &&
- sel->pad != RKISP2_ISP_PAD_SINK_VIDEO)
+ sel->pad != RKISP2_ISP_PAD_SINK_VIDEO_DMA &&
+ sel->pad != RKISP2_ISP_PAD_SINK_VIDEO_CIF)
return -EINVAL;
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
- if (sel->pad == RKISP2_ISP_PAD_SINK_VIDEO) {
+ if (sel->pad == RKISP2_ISP_PAD_SINK_VIDEO_DMA ||
+ sel->pad == RKISP2_ISP_PAD_SINK_VIDEO_CIF) {
struct v4l2_mbus_framefmt *fmt;
fmt = v4l2_subdev_state_get_format(sd_state, sel->pad);
@@ -634,7 +675,7 @@ static int rkisp2_isp_get_selection(struct v4l2_subdev *sd,
sel->r.top = 0;
} else {
sel->r = *v4l2_subdev_state_get_crop(sd_state,
- RKISP2_ISP_PAD_SINK_VIDEO);
+ RKISP2_ISP_PAD_SINK_VIDEO_DMA);
}
break;
@@ -663,8 +704,9 @@ static int rkisp2_isp_set_selection(struct v4l2_subdev *sd,
dev_dbg(isp->rkisp2->dev, "%s: pad: %d sel(%d,%d)/%ux%u\n", __func__,
sel->pad, sel->r.left, sel->r.top, sel->r.width, sel->r.height);
- if (sel->pad == RKISP2_ISP_PAD_SINK_VIDEO)
- rkisp2_isp_set_sink_crop(isp, sd_state, &sel->r);
+ if (sel->pad == RKISP2_ISP_PAD_SINK_VIDEO_DMA ||
+ sel->pad == RKISP2_ISP_PAD_SINK_VIDEO_CIF)
+ rkisp2_isp_set_sink_crop(isp, sd_state, &sel->r, sel->pad);
else if (sel->pad == RKISP2_ISP_PAD_SOURCE_VIDEO)
rkisp2_isp_set_src_crop(isp, sd_state, &sel->r);
else
@@ -678,6 +720,46 @@ static int rkisp2_subdev_link_validate(struct media_link *link)
return v4l2_subdev_link_validate(link);
}
+static int rkisp2_subdev_link_setup(struct media_entity *entity,
+ const struct media_pad *local_pad,
+ const struct media_pad *remote_pad, u32 flags)
+{
+ struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+ struct rkisp2_isp *isp = to_rkisp2_isp(sd);
+ struct media_link *link;
+ u16 other_sink_pad_index;
+
+ dev_dbg(isp->rkisp2->dev, "link setup %s -> %s\n", remote_pad->entity->name,
+ local_pad->entity->name);
+
+ /* We only care about links being created on a sink pad */
+ if (!(flags & MEDIA_LNK_FL_ENABLED) ||
+ !(local_pad->flags & MEDIA_PAD_FL_SINK) ||
+ (local_pad->index != RKISP2_ISP_PAD_SINK_VIDEO_DMA &&
+ local_pad->index != RKISP2_ISP_PAD_SINK_VIDEO_CIF))
+ return 0;
+
+ other_sink_pad_index =
+ local_pad->index == RKISP2_ISP_PAD_SINK_VIDEO_DMA ?
+ RKISP2_ISP_PAD_SINK_VIDEO_CIF :
+ RKISP2_ISP_PAD_SINK_VIDEO_DMA;
+
+ list_for_each_entry(link, &entity->links, list) {
+ if (link->sink->entity != local_pad->entity ||
+ link->sink->index != other_sink_pad_index)
+ continue;
+
+ /*
+ * If we are trying to enable DMA sink pad but the CIF
+ * sink pad (and vice versa) has an enabled link then return
+ * error
+ */
+ return link->flags & MEDIA_LNK_FL_ENABLED ? -EBUSY : 0;
+ }
+
+ return 0;
+}
+
static const struct v4l2_subdev_pad_ops rkisp2_isp_pad_ops = {
.enum_mbus_code = rkisp2_isp_enum_mbus_code,
.enum_frame_size = rkisp2_isp_enum_frame_size,
@@ -709,7 +791,7 @@ static int rkisp2_isp_s_stream(struct v4l2_subdev *sd, int enable)
return 0;
}
- sink_pad = &isp->pads[RKISP2_ISP_PAD_SINK_VIDEO];
+ sink_pad = &isp->pads[rkisp2_isp_get_active_sink_pad(isp)];
source_pad = media_pad_remote_pad_unique(sink_pad);
if (IS_ERR(source_pad)) {
dev_dbg(rkisp2->dev, "Failed to get source for ISP: %ld\n",
@@ -769,6 +851,7 @@ static int rkisp2_isp_subs_evt(struct v4l2_subdev *sd, struct v4l2_fh *fh,
static const struct media_entity_operations rkisp2_isp_media_ops = {
.link_validate = rkisp2_subdev_link_validate,
+ .link_setup = rkisp2_subdev_link_setup,
};
static const struct v4l2_subdev_video_ops rkisp2_isp_video_ops = {
@@ -808,8 +891,8 @@ int rkisp2_isp_register(struct rkisp2_device *rkisp2)
sd->owner = THIS_MODULE;
strscpy(sd->name, RKISP2_ISP_DEV_NAME, sizeof(sd->name));
- pads[RKISP2_ISP_PAD_SINK_VIDEO].flags = MEDIA_PAD_FL_SINK |
- MEDIA_PAD_FL_MUST_CONNECT;
+ pads[RKISP2_ISP_PAD_SINK_VIDEO_DMA].flags = MEDIA_PAD_FL_SINK;
+ pads[RKISP2_ISP_PAD_SINK_VIDEO_CIF].flags = MEDIA_PAD_FL_SINK;
pads[RKISP2_ISP_PAD_SINK_PARAMS].flags = MEDIA_PAD_FL_SINK;
pads[RKISP2_ISP_PAD_SOURCE_VIDEO].flags = MEDIA_PAD_FL_SOURCE;
pads[RKISP2_ISP_PAD_SOURCE_STATS].flags = MEDIA_PAD_FL_SOURCE;
--
2.47.2
^ permalink raw reply related [flat|nested] 14+ messages in thread