* [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; 16+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ 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; 16+ 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
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ 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; 16+ 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
>
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ 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; 16+ 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
> >
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ 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; 16+ 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;
>>> +};
>>> +[...]
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ 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; 16+ 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.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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ 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-13 1:55 ` Paul Elder
2026-07-10 21:50 ` Nicolas Dufresne
3 siblings, 1 reply; 16+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-07-10 20:23 ` Michael Riesch
@ 2026-07-13 1:55 ` Paul Elder
2026-07-13 8:26 ` Michael Riesch
0 siblings, 1 reply; 16+ messages in thread
From: Paul Elder @ 2026-07-13 1:55 UTC (permalink / raw)
To: Michael Riesch, 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 Michael,
Thanks for the review!
Quoting Michael Riesch (2026-07-11 05:23:50)
> 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.
\o/
Yes the overview is right.
>
> > [...]
> > +// 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>
Yeah I guess traversing the graph is kind of a critical feature.
>
> 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
Hm yeah that's true; we'd just have a sea of media entities and we can link
whoever we want...
We didn't really discuss that part very hard last time.
> 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",
> }
The problem is where would this information go? We can't put it in the driver;
it *must* come from the device tree. And I don't think we can just add a new
property that defines a media subsystem. So I think our only option is to
traverse the endpoint graph in the device tree.
> 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?
Yes.
>
> > +
> > + 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?!
(Just in general or with respect to this function...?)
>
> 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.
Yes; given that both rkcif and rkisp2 could potentially exist on devices where
the other does not, this is not realistically doable. (I mean if we really want
to we could but it would be a high-cost mess)
>
> 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.
Similar problem as above (can't choose main driver), the one above that (no
place to put group member list).
>
> 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
I always thought that media device and media graph were the
same/interchangable...
> 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
afaict that's the case.
> actually something with devm_ then this requires more thought.
>
> IMHO the third option is the way to go.
I'm glad we agree :)
>
> > +
> > +// 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)?
Well I think driver_name and (maybe) hw_revision are still useful to have. The
only problem is I have no clue where to put/get that information.
The only good-ish idea I could come up with was something like
"shared_media_graph_{board compatible}_{smallest base address of media graph
member}". So like "shared_media_graph_rockchip,rk3588_fdcb0000" for the rk3588
since between the csi receivers and vicap and isp, the isp has the smallest
address. That way:
- the information comes from the device tree
- the name is persistent
- it can identify separate shared media graphs on the same platform
idk about the hardware revision though. I think probably only driver match data
can hold that information. But then we'd probably have duplicate information
since otherwise if one of the member drivers doesn't probe then we'd be missing
the hw_revision. Or maybe we don't care about hardware revision since it
doesn't make sense for the shared media graph part and it only makes sense for
the individual IP cores. In which case that information is lost unless we move
it somewhere else. Or maybe it's not used anyway so we can indeed just drop it.
>
> 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,
> ...
> }
Same problem as above: there's no reasonable place to put this information.
> 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
Hehe :)
> IIUC it's typically the sink that passes some flags to
> v4l2_create_fwnode_links_to_pad. So prioritizing the sink seems reasonable.
Ok good.
>
> > + *
> > + * 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.
Oh, good idea!
>
> > + 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}?
Yeah that sounds like a good idea.
Thanks,
Paul
>
> > [...]
>
> Thanks and best regards,
> Michael
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC PATCH 1/6] media: mc: Implement shared media graph
2026-07-13 1:55 ` Paul Elder
@ 2026-07-13 8:26 ` Michael Riesch
0 siblings, 0 replies; 16+ messages in thread
From: Michael Riesch @ 2026-07-13 8:26 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,
On 7/13/26 03:55, Paul Elder wrote:
> [...]
>>
>> <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>
>
> Yeah I guess traversing the graph is kind of a critical feature.
>
>>
>> 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
>
> Hm yeah that's true; we'd just have a sea of media entities and we can link
> whoever we want...
>
> We didn't really discuss that part very hard last time.
>
>> 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",
>> }
>
> The problem is where would this information go? We can't put it in the driver;
> it *must* come from the device tree. And I don't think we can just add a new
> property that defines a media subsystem. So I think our only option is to
> traverse the endpoint graph in the device tree.
Just to sketch the rough idea: A list of known shared media graphs in
drivers/media/mc/mc-shared-graph.c:
struct mcshgr_info { ... } known_graphs = {
{
.id = MEDIA_SHARED_ROCKCHIP_RK3588_CAMERA,
.name = "Rockchip Camera Subsystem",
// TODO fill me with other things
},
{
.id = MEDIA_SHARED_ROCKCHIP_RK3576_CAMERA,
.name = "whatever",
// TODO fill me with other things
},
};
The IDs need to go into a enum in include/media/mc-shared-graph.h.
The drivers would then call e.g.
mdev = media_device_shared_join(rkcif->dev,
MEDIA_SHARED_ROCKCHIP_RK3588_CAMERA);
and of course this parameter could vary with the individual driver DT
compatible, e.g., "rockchip,rk3588-vicap" =>
MEDIA_SHARED_ROCKCHIP_RK3588_CAMERA.
I am sketching this for the sake of completeness. This would provide a
place to store information, but I am not convinced at all that this is a
nice solution.
Alternatively, the list of known shared media graphs could store some DT
compatibles
struct mcshgr_info { ... } known_graphs = {
{
.id = MEDIA_SHARED_ROCKCHIP_RK3588_CAMERA,
.name = "Rockchip Camera Subsystem",
.compatibles = {
"rockchip,rk3588-isp",
"rockchip,rk3588-vicap",
},
// TODO fill me with other things
},
};
and I guess there could be a way to get the compatible from struct
device, then
mdev = media_device_shared_join(rkcif->dev);
would remain the same.
This just occurred to me -- not sure whether I like it.
> [...]
>>
>> OK, what options do we have?!
>
> (Just in general or with respect to this function...?)
How to get information that the media device requires or at least
contains as of today.
>> 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.
>
> Yes; given that both rkcif and rkisp2 could potentially exist on devices where
> the other does not, this is not realistically doable. (I mean if we really want
> to we could but it would be a high-cost mess)
>
>>
>> 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.
>
> Similar problem as above (can't choose main driver), the one above that (no
> place to put group member list).
>
>>
>> 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
>
> I always thought that media device and media graph were the
> same/interchangable...
They may be, at least as of today. I was asking myself how our view on
the new (shared) media device should look like. And I think that the
concept of the graph is important, and that the focus should be on which
entities there are and how the are connected (i.e, the topology, the
graph). This also means that we should not rely on other information and
assumptions (other information such as media device name, hw version,
... and assumptions such as "there is a single v4l2 driver that owns the
graph", "there is a single media device that owns the graph", ...)
In the end, what should user space do? Does libcamera actually do
anything with e.g. the HW revision of a media device? Is the name of the
media device actually used?
Or does it simply iterate over all media devices and scan for entities
with a certain name (e.g. look for a RK3588 ISP)? Because if it does, or
if it could do it that way, there is no point in crafting together this
additional information in the kernel.
>
>> 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
>
> afaict that's the case.
>
>> actually something with devm_ then this requires more thought.
>>
>> IMHO the third option is the way to go.
>
> I'm glad we agree :)
Then we set the dev member of the media_device to NULL, right?
>
>>
>>> +
>>> +// 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)?
>
> Well I think driver_name and (maybe) hw_revision are still useful to have. The
> only problem is I have no clue where to put/get that information.
>
> The only good-ish idea I could come up with was something like
> "shared_media_graph_{board compatible}_{smallest base address of media graph
> member}". So like "shared_media_graph_rockchip,rk3588_fdcb0000" for the rk3588
> since between the csi receivers and vicap and isp, the isp has the smallest
> address. That way:
> - the information comes from the device tree
> - the name is persistent
> - it can identify separate shared media graphs on the same platform
>
> idk about the hardware revision though. I think probably only driver match data
> can hold that information. But then we'd probably have duplicate information
> since otherwise if one of the member drivers doesn't probe then we'd be missing
> the hw_revision. Or maybe we don't care about hardware revision since it
> doesn't make sense for the shared media graph part and it only makes sense for
> the individual IP cores. In which case that information is lost unless we move
> it somewhere else. Or maybe it's not used anyway so we can indeed just drop it.
Crafting together a name from device tree information... could work..
and could be reasonable I guess. Just saying that if some joker removes
the connection between ISP and VICAP in device tree, the media graph
could end up with the same name for different things or different things
with the same name and... I just think I this all is not something I
would rely on from user space, and I would look at the graph rather than
making assumptions based on the graph name.
The real question is whether this is actually needed. As you stated
above, maybe this simply doesn't make sense for a shared media graph.
And we still have the individual IP core information.
I would recommend to ignore the fields listed above for shared media
devices.
> [...]
Thanks and best regards,
Michael
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ 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; 16+ 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.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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 16+ 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; 16+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ 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; 16+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ 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; 16+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ 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; 16+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ 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; 16+ 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
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply related [flat|nested] 16+ messages in thread