[PATCH v5 00/12] media: rcar-vin: Unify notifiers and enable MC on Gen2

[PATCH v5 00/12] media: rcar-vin: Unify notifiers and enable MC on Gen2

[Niklas Söderlund]

Hello,

This series completes the conversion of the soc_camera VIN driver to a
full fledge media-graph enabled driver for R-Car Gen2 devices, Gen3 and
later have been media-graph centric from the start. Having a single
driver supporting both MC and non-MC operation have lead do odd design
decisions in the driver, and it have prevented improving the driver over
all.

New features and bug fixes have always been more important then fixing
this old generation to be MC-centric. But in order to start to play with
libcamera support for the R-Car pipeline it have become more pressing to
make take the time to make this driver MC-only, and more importantly
test it to make sure nothing really breaks.

Patch 1/12, 2/12 and 3/12 are drive-by fixes correcting issues in the 
existing design. Patch 4/12 prepares for Gen2 MC by making sure each VIN 
instance on Gen2 gets a unique ID which will be needed to support VIN 
groups. Compared to Gen3 and later the group ID does not match what it 
can do in the group and does not need to be set from DT, all that 
matters is that each VIN instance have a unique ID.

Patch 5/12, 6/12 and 7/12 uses the fact that VIN instances on Gen2 now 
have unique IDs and greatly simplifies the unnecessarily complex 
vl4-async notifier usage in the VIN driver. This have in the past lead 
to some subtle bugs and having only a single notifier for all VIN will 
remove a lot of possibilities for this to go wrong in the future.

Patch 8/12 9/12 and 10/12 make to adapt Gen2 to MC in incremental steps 
to ease review. These two patches where previously part of a larger 
one-patch to change it all (now 11/12). There is on state of the series 
with 10/12 applied where controls from the sub-device are not exposed to 
user-space that is then addressed in 11/12.

Finally patch 11/12 removes all non MC code paths and have the Gen2
devices register a media device and configure links. While patch 12/12 
is a small cleanup that was previously part of 11/12.

This have been tested on Gen3 and Gen4 devices without any regressions.
And on Gen2 to make sure the media-graph behaves as it should. As a
bonus the Gen2 devices can now join the VIN CI and any future issues
should be caught as they are for Gen3 and Gen4.

See individual patches for changelog. Compared to v4 the patch [v4 6/6] 
have been broken apart into smaller bits to ease review.

Niklas Söderlund (12):
  media: rcar-vin: Use correct count of remote subdevices
  media: rcar-vin: Store platform info with group structure
  media: rcar-vin: Change link setup argument
  media: rcar-vin: Generate a VIN group ID for Gen2
  media: rcar-vin: Prepare for unifying all v4l-async notifiers
  media: rcar-vin: Improve error paths for parallel devices
  media: rcar-vin: Merge all notifiers
  media: rcar-vin: Always create a media pad
  media: rcar-vin: Remove NTSC workaround
  media: rcar-vin: Only expose VIN controls
  media: rcar-vin: Enable media-graph on Gen2
  media: rcar-vin: Fold event notifier into only user

 .../platform/renesas/rcar-vin/rcar-core.c     | 677 +++++++-----------
 .../platform/renesas/rcar-vin/rcar-dma.c      |  23 +-
 .../platform/renesas/rcar-vin/rcar-v4l2.c     | 488 +------------
 .../platform/renesas/rcar-vin/rcar-vin.h      |  16 +-
 4 files changed, 281 insertions(+), 923 deletions(-)

-- 
2.49.0

[PATCH v5 01/12] media: rcar-vin: Use correct count of remote subdevices

[Niklas Söderlund]

When extending the driver with Gen4 support the iteration of over
possible remote subdevices changed from being R-Car CSI-2 Rx only to
also cover R-Car CSISP instances. In two loops updating the bounds
variable was missed.

This had no ill effect as the count the two values have always been the
same in the past. Fix it by looking at the array size.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
* Changes since v4
- Use ARRAY_SIZE() instead of updating the incorrect define to
  RVIN_REMOTES_MAX.
---
 drivers/media/platform/renesas/rcar-vin/rcar-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index 846ae7989b1d..cf5830d7d7b1 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -213,7 +213,7 @@ static int rvin_group_entity_to_remote_id(struct rvin_group *group,
 
 	sd = media_entity_to_v4l2_subdev(entity);
 
-	for (i = 0; i < RVIN_REMOTES_MAX; i++)
+	for (i = 0; i < ARRAY_SIZE(group->remotes); i++)
 		if (group->remotes[i].subdev == sd)
 			return i;
 
@@ -262,7 +262,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 
 	mutex_lock(&vin->group->lock);
 
-	for (i = 0; i < RVIN_CSI_MAX; i++) {
+	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
 		if (vin->group->remotes[i].asc != asc)
 			continue;
 		vin->group->remotes[i].subdev = NULL;
@@ -284,7 +284,7 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
 
 	mutex_lock(&vin->group->lock);
 
-	for (i = 0; i < RVIN_CSI_MAX; i++) {
+	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
 		if (vin->group->remotes[i].asc != asc)
 			continue;
 		vin->group->remotes[i].subdev = subdev;
-- 
2.49.0

[PATCH v5 02/12] media: rcar-vin: Store platform info with group structure

[Niklas Söderlund]

When the transition of Gen2 to use groups are complete the platform
specific information can be retrieved from the group instead of being
duplicated in each VIN's private data structure.

Prepare for this by already adding the information to the group
structure so it can be used without first having to find the group from
a VIN instances private data.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v4
- New in v5.
---
 drivers/media/platform/renesas/rcar-vin/rcar-core.c | 1 +
 drivers/media/platform/renesas/rcar-vin/rcar-vin.h  | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index cf5830d7d7b1..66efe075adae 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -156,6 +156,7 @@ static int rvin_group_get(struct rvin_dev *vin,
 		}
 
 		kref_init(&group->refcount);
+		group->info = vin->info;
 
 		rvin_group_data = group;
 	}
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
index 83d1b2734c41..313703cd1103 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
@@ -242,6 +242,7 @@ struct rvin_dev {
  * @lock:		protects the count, notifier, vin and csi members
  * @count:		number of enabled VIN instances found in DT
  * @notifier:		group notifier for CSI-2 async connections
+ * @info:		Platform dependent information about the VIN instances
  * @vin:		VIN instances which are part of the group
  * @link_setup:		Callback to create all links for the media graph
  * @remotes:		array of pairs of async connection and subdev pointers
@@ -255,6 +256,7 @@ struct rvin_group {
 	struct mutex lock;
 	unsigned int count;
 	struct v4l2_async_notifier notifier;
+	const struct rvin_info *info;
 	struct rvin_dev *vin[RCAR_VIN_NUM];
 
 	int (*link_setup)(struct rvin_dev *vin);
-- 
2.49.0

[PATCH v5 03/12] media: rcar-vin: Change link setup argument

[Niklas Söderlund]

The link setup callback once acted on each VIN instance, and expected to
be called once for each VIN instance. This have changed as the driver
grew support for later hardware generations and the callback is now
expected to setup links for all VIN in the group.

The argument to the callback have however remained a pointer to a single
VIN instance. This pointer was then used to get the group structure. Fix
this and pass the group as the single argument to the link setup
callback making the expectation of the function clear.

There is no intentional change in behavior.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
* Changes since v4
- Use the group->info structure added in previous patch instead of
  iterating over all VIN to find one that is instantiated to get the
  same information.
- Condense variable declaration in rvin_isp_setup_links().
---
 .../platform/renesas/rcar-vin/rcar-core.c     | 37 ++++++++++---------
 .../platform/renesas/rcar-vin/rcar-vin.h      |  2 +-
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index 66efe075adae..73d713868391 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -65,7 +65,7 @@ static void rvin_group_cleanup(struct rvin_group *group)
 }
 
 static int rvin_group_init(struct rvin_group *group, struct rvin_dev *vin,
-			   int (*link_setup)(struct rvin_dev *),
+			   int (*link_setup)(struct rvin_group *),
 			   const struct media_device_ops *ops)
 {
 	struct media_device *mdev = &group->mdev;
@@ -115,7 +115,7 @@ static void rvin_group_release(struct kref *kref)
 }
 
 static int rvin_group_get(struct rvin_dev *vin,
-			  int (*link_setup)(struct rvin_dev *),
+			  int (*link_setup)(struct rvin_group *),
 			  const struct media_device_ops *ops)
 {
 	struct rvin_group *group;
@@ -247,7 +247,7 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
 		}
 	}
 
-	return vin->group->link_setup(vin);
+	return vin->group->link_setup(vin->group);
 }
 
 static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
@@ -910,35 +910,35 @@ static int rvin_csi2_create_link(struct rvin_group *group, unsigned int id,
 	return 0;
 }
 
-static int rvin_csi2_setup_links(struct rvin_dev *vin)
+static int rvin_csi2_setup_links(struct rvin_group *group)
 {
 	const struct rvin_group_route *route;
 	unsigned int id;
 	int ret = -EINVAL;
 
 	/* Create all media device links between VINs and CSI-2's. */
-	mutex_lock(&vin->group->lock);
-	for (route = vin->info->routes; route->chsel; route++) {
+	mutex_lock(&group->lock);
+	for (route = group->info->routes; route->chsel; route++) {
 		/* Check that VIN' master is part of the group. */
-		if (!vin->group->vin[route->master])
+		if (!group->vin[route->master])
 			continue;
 
 		/* Check that CSI-2 is part of the group. */
-		if (!vin->group->remotes[route->csi].subdev)
+		if (!group->remotes[route->csi].subdev)
 			continue;
 
 		for (id = route->master; id < route->master + 4; id++) {
 			/* Check that VIN is part of the group. */
-			if (!vin->group->vin[id])
+			if (!group->vin[id])
 				continue;
 
-			ret = rvin_csi2_create_link(vin->group, id, route);
+			ret = rvin_csi2_create_link(group, id, route);
 			if (ret)
 				goto out;
 		}
 	}
 out:
-	mutex_unlock(&vin->group->lock);
+	mutex_unlock(&group->lock);
 
 	return ret;
 }
@@ -992,30 +992,31 @@ static int rvin_csi2_init(struct rvin_dev *vin)
  * ISP
  */
 
-static int rvin_isp_setup_links(struct rvin_dev *vin)
+static int rvin_isp_setup_links(struct rvin_group *group)
 {
 	unsigned int i;
 	int ret = -EINVAL;
 
 	/* Create all media device links between VINs and ISP's. */
-	mutex_lock(&vin->group->lock);
+	mutex_lock(&group->lock);
 	for (i = 0; i < RCAR_VIN_NUM; i++) {
 		struct media_pad *source_pad, *sink_pad;
 		struct media_entity *source, *sink;
 		unsigned int source_slot = i / 8;
 		unsigned int source_idx = i % 8 + 1;
+		struct rvin_dev *vin = group->vin[i];
 
-		if (!vin->group->vin[i])
+		if (!vin)
 			continue;
 
 		/* Check that ISP is part of the group. */
-		if (!vin->group->remotes[source_slot].subdev)
+		if (!group->remotes[source_slot].subdev)
 			continue;
 
-		source = &vin->group->remotes[source_slot].subdev->entity;
+		source = &group->remotes[source_slot].subdev->entity;
 		source_pad = &source->pads[source_idx];
 
-		sink = &vin->group->vin[i]->vdev.entity;
+		sink = &vin->vdev.entity;
 		sink_pad = &sink->pads[0];
 
 		/* Skip if link already exists. */
@@ -1031,7 +1032,7 @@ static int rvin_isp_setup_links(struct rvin_dev *vin)
 			break;
 		}
 	}
-	mutex_unlock(&vin->group->lock);
+	mutex_unlock(&group->lock);
 
 	return ret;
 }
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
index 313703cd1103..cb8e8fa54f96 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
@@ -259,7 +259,7 @@ struct rvin_group {
 	const struct rvin_info *info;
 	struct rvin_dev *vin[RCAR_VIN_NUM];
 
-	int (*link_setup)(struct rvin_dev *vin);
+	int (*link_setup)(struct rvin_group *group);
 
 	struct {
 		struct v4l2_async_connection *asc;
-- 
2.49.0

[PATCH v5 04/12] media: rcar-vin: Generate a VIN group ID for Gen2

[Niklas Söderlund]

Prepare to move Gen2 and earlier models to media controller by
generating a unique VIN group id for each VIN instance. On Gen3 and Gen4
it is important to have a specific id in the group as media graph routes
depend on this. On Gen2 and earlier models all that will matter is to
have a unique id in the range.

Break out the id generation to a own function keeping the logic for Gen3
and Gen4 while generating a sequential id for Gen2 models.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
* Changes since v4
- Add explicit error messages for error cases getting a group ID.

* Changes since v1
- Move ID allocation to probe.
- Use ida_alloc_range() instead of implementing our own schema by
  counting DT nodes.
---
 .../platform/renesas/rcar-vin/rcar-core.c     | 80 ++++++++++++++-----
 1 file changed, 61 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index 73d713868391..e50cea9c04d3 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -10,6 +10,7 @@
  * Based on the soc-camera rcar_vin driver
  */
 
+#include <linux/idr.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_graph.h>
@@ -55,6 +56,7 @@
  * be only one group for all instances.
  */
 
+static DEFINE_IDA(rvin_ida);
 static DEFINE_MUTEX(rvin_group_lock);
 static struct rvin_group *rvin_group_data;
 
@@ -119,23 +121,8 @@ static int rvin_group_get(struct rvin_dev *vin,
 			  const struct media_device_ops *ops)
 {
 	struct rvin_group *group;
-	u32 id;
 	int ret;
 
-	/* Make sure VIN id is present and sane */
-	ret = of_property_read_u32(vin->dev->of_node, "renesas,id", &id);
-	if (ret) {
-		vin_err(vin, "%pOF: No renesas,id property found\n",
-			vin->dev->of_node);
-		return -EINVAL;
-	}
-
-	if (id >= RCAR_VIN_NUM) {
-		vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
-			vin->dev->of_node, id);
-		return -EINVAL;
-	}
-
 	/* Join or create a VIN group */
 	mutex_lock(&rvin_group_lock);
 	if (rvin_group_data) {
@@ -165,16 +152,15 @@ static int rvin_group_get(struct rvin_dev *vin,
 	/* Add VIN to group */
 	mutex_lock(&group->lock);
 
-	if (group->vin[id]) {
-		vin_err(vin, "Duplicate renesas,id property value %u\n", id);
+	if (group->vin[vin->id]) {
+		vin_err(vin, "Duplicate renesas,id property value %u\n", vin->id);
 		mutex_unlock(&group->lock);
 		kref_put(&group->refcount, rvin_group_release);
 		return -EINVAL;
 	}
 
-	group->vin[id] = vin;
+	group->vin[vin->id] = vin;
 
-	vin->id = id;
 	vin->group = group;
 	vin->v4l2_dev.mdev = &group->mdev;
 
@@ -1363,6 +1349,56 @@ static const struct of_device_id rvin_of_id_table[] = {
 };
 MODULE_DEVICE_TABLE(of, rvin_of_id_table);
 
+static int rvin_id_get(struct rvin_dev *vin)
+{
+	u32 oid;
+	int id;
+
+	switch (vin->info->model) {
+	case RCAR_GEN3:
+	case RCAR_GEN4:
+		if (of_property_read_u32(vin->dev->of_node, "renesas,id", &oid)) {
+			vin_err(vin, "%pOF: No renesas,id property found\n",
+				vin->dev->of_node);
+			return -EINVAL;
+		}
+
+		if (oid < 0 || oid >= RCAR_VIN_NUM) {
+			vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
+				vin->dev->of_node, oid);
+			return -EINVAL;
+		}
+
+		vin->id = oid;
+		break;
+	default:
+		id = ida_alloc_range(&rvin_ida, 0, RCAR_VIN_NUM - 1,
+				     GFP_KERNEL);
+		if (id < 0) {
+			vin_err(vin, "%pOF: Failed to allocate VIN group ID\n",
+				vin->dev->of_node);
+			return -EINVAL;
+		}
+
+		vin->id = id;
+		break;
+	}
+
+	return 0;
+}
+
+static void rvin_id_put(struct rvin_dev *vin)
+{
+	switch (vin->info->model) {
+	case RCAR_GEN3:
+	case RCAR_GEN4:
+		break;
+	default:
+		ida_free(&rvin_ida, vin->id);
+		break;
+	}
+}
+
 static int rcar_vin_probe(struct platform_device *pdev)
 {
 	struct rvin_dev *vin;
@@ -1390,6 +1426,9 @@ static int rcar_vin_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, vin);
 
+	if (rvin_id_get(vin))
+		return -EINVAL;
+
 	if (vin->info->use_isp) {
 		ret = rvin_isp_init(vin);
 	} else if (vin->info->use_mc) {
@@ -1407,6 +1446,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
 
 	if (ret) {
 		rvin_dma_unregister(vin);
+		rvin_id_put(vin);
 		return ret;
 	}
 
@@ -1431,6 +1471,8 @@ static void rcar_vin_remove(struct platform_device *pdev)
 	else
 		rvin_parallel_cleanup(vin);
 
+	rvin_id_put(vin);
+
 	rvin_dma_unregister(vin);
 }
 
-- 
2.49.0

[PATCH v5 05/12] media: rcar-vin: Prepare for unifying all v4l-async notifiers

[Niklas Söderlund]

The R-Car VIN driver is needlessly complex and uses more then one
v4l-async notifier to attach to all its subdevices. Prepare for unifying
them by moving rvin_parallel_parse_of() to where it needs to be when
they are unified.

The function is moved verbatim and there is no change in behavior.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
* Changes since v4
- Fix spelling in commit message.
---
 .../platform/renesas/rcar-vin/rcar-core.c     | 106 +++++++++---------
 1 file changed, 53 insertions(+), 53 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index e50cea9c04d3..df3f15bd95a4 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -338,6 +338,59 @@ static void rvin_group_notifier_cleanup(struct rvin_dev *vin)
 	}
 }
 
+static int rvin_parallel_parse_of(struct rvin_dev *vin)
+{
+	struct fwnode_handle *ep, *fwnode;
+	struct v4l2_fwnode_endpoint vep = {
+		.bus_type = V4L2_MBUS_UNKNOWN,
+	};
+	struct v4l2_async_connection *asc;
+	int ret;
+
+	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(vin->dev), 0, 0, 0);
+	if (!ep)
+		return 0;
+
+	fwnode = fwnode_graph_get_remote_endpoint(ep);
+	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
+	fwnode_handle_put(ep);
+	if (ret) {
+		vin_err(vin, "Failed to parse %pOF\n", to_of_node(fwnode));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	switch (vep.bus_type) {
+	case V4L2_MBUS_PARALLEL:
+	case V4L2_MBUS_BT656:
+		vin_dbg(vin, "Found %s media bus\n",
+			vep.bus_type == V4L2_MBUS_PARALLEL ?
+			"PARALLEL" : "BT656");
+		vin->parallel.mbus_type = vep.bus_type;
+		vin->parallel.bus = vep.bus.parallel;
+		break;
+	default:
+		vin_err(vin, "Unknown media bus type\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	asc = v4l2_async_nf_add_fwnode(&vin->notifier, fwnode,
+				       struct v4l2_async_connection);
+	if (IS_ERR(asc)) {
+		ret = PTR_ERR(asc);
+		goto out;
+	}
+
+	vin->parallel.asc = asc;
+
+	vin_dbg(vin, "Add parallel OF device %pOF\n", to_of_node(fwnode));
+out:
+	fwnode_handle_put(fwnode);
+
+	return ret;
+}
+
 static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
 				    unsigned int max_id)
 {
@@ -636,59 +689,6 @@ static const struct v4l2_async_notifier_operations rvin_parallel_notify_ops = {
 	.complete = rvin_parallel_notify_complete,
 };
 
-static int rvin_parallel_parse_of(struct rvin_dev *vin)
-{
-	struct fwnode_handle *ep, *fwnode;
-	struct v4l2_fwnode_endpoint vep = {
-		.bus_type = V4L2_MBUS_UNKNOWN,
-	};
-	struct v4l2_async_connection *asc;
-	int ret;
-
-	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(vin->dev), 0, 0, 0);
-	if (!ep)
-		return 0;
-
-	fwnode = fwnode_graph_get_remote_endpoint(ep);
-	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
-	fwnode_handle_put(ep);
-	if (ret) {
-		vin_err(vin, "Failed to parse %pOF\n", to_of_node(fwnode));
-		ret = -EINVAL;
-		goto out;
-	}
-
-	switch (vep.bus_type) {
-	case V4L2_MBUS_PARALLEL:
-	case V4L2_MBUS_BT656:
-		vin_dbg(vin, "Found %s media bus\n",
-			vep.bus_type == V4L2_MBUS_PARALLEL ?
-			"PARALLEL" : "BT656");
-		vin->parallel.mbus_type = vep.bus_type;
-		vin->parallel.bus = vep.bus.parallel;
-		break;
-	default:
-		vin_err(vin, "Unknown media bus type\n");
-		ret = -EINVAL;
-		goto out;
-	}
-
-	asc = v4l2_async_nf_add_fwnode(&vin->notifier, fwnode,
-				       struct v4l2_async_connection);
-	if (IS_ERR(asc)) {
-		ret = PTR_ERR(asc);
-		goto out;
-	}
-
-	vin->parallel.asc = asc;
-
-	vin_dbg(vin, "Add parallel OF device %pOF\n", to_of_node(fwnode));
-out:
-	fwnode_handle_put(fwnode);
-
-	return ret;
-}
-
 static void rvin_parallel_cleanup(struct rvin_dev *vin)
 {
 	v4l2_async_nf_unregister(&vin->notifier);
-- 
2.49.0

[PATCH v5 06/12] media: rcar-vin: Improve error paths for parallel devices

[Niklas Söderlund]

Use the __free(fwnode_handle) hooks to free the endpoints when the
function exits to simplify the error paths and make the intent more
clear.

While at it correct the error message when failing to parse an endpoint
to report the correct node.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v4
- New in v5, improvement suggested by Sakari and Laurent in review of v4.
---
 .../platform/renesas/rcar-vin/rcar-core.c     | 28 +++++++------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index df3f15bd95a4..100432080ad7 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -340,24 +340,20 @@ static void rvin_group_notifier_cleanup(struct rvin_dev *vin)
 
 static int rvin_parallel_parse_of(struct rvin_dev *vin)
 {
-	struct fwnode_handle *ep, *fwnode;
+	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
+	struct fwnode_handle *ep __free(fwnode_handle) = NULL;
 	struct v4l2_fwnode_endpoint vep = {
 		.bus_type = V4L2_MBUS_UNKNOWN,
 	};
 	struct v4l2_async_connection *asc;
-	int ret;
 
 	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(vin->dev), 0, 0, 0);
 	if (!ep)
 		return 0;
 
-	fwnode = fwnode_graph_get_remote_endpoint(ep);
-	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
-	fwnode_handle_put(ep);
-	if (ret) {
-		vin_err(vin, "Failed to parse %pOF\n", to_of_node(fwnode));
-		ret = -EINVAL;
-		goto out;
+	if (v4l2_fwnode_endpoint_parse(ep, &vep)) {
+		vin_err(vin, "Failed to parse %pOF\n", to_of_node(ep));
+		return -EINVAL;
 	}
 
 	switch (vep.bus_type) {
@@ -371,24 +367,20 @@ static int rvin_parallel_parse_of(struct rvin_dev *vin)
 		break;
 	default:
 		vin_err(vin, "Unknown media bus type\n");
-		ret = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
+	fwnode = fwnode_graph_get_remote_endpoint(ep);
 	asc = v4l2_async_nf_add_fwnode(&vin->notifier, fwnode,
 				       struct v4l2_async_connection);
-	if (IS_ERR(asc)) {
-		ret = PTR_ERR(asc);
-		goto out;
-	}
+	if (IS_ERR(asc))
+		return PTR_ERR(asc);
 
 	vin->parallel.asc = asc;
 
 	vin_dbg(vin, "Add parallel OF device %pOF\n", to_of_node(fwnode));
-out:
-	fwnode_handle_put(fwnode);
 
-	return ret;
+	return 0;
 }
 
 static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
-- 
2.49.0

[PATCH v5 07/12] media: rcar-vin: Merge all notifiers

[Niklas Söderlund]

The VIN usage of v4l-async is complex and stems from organic growth of
the driver of supporting both private local subdevices (Gen2, Gen3) and
subdevices shared between all VIN instances (Gen3 and Gen4).

The driver used a separate notifier for each VIN for the private local
ones, and a shared group notifier for the shared ones. This was complex
and lead to subtle bugs when unbinding and later rebinding subdevices in
one of the notifiers having to handle different edge cases depending on
if it also had subdevices in the other notifiers etc.

To simplify this have the Gen2 devices allocate and form a VIN group
too. This way all subdevices on all models can be collect in a
single group notifier. Then there is only a single complete callback for
all where the video devices and subdevice nodes can be registered etc.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
---
* Changes since v4
- Fix spelling in commit message and comments.
- Use mutex guard to simplify error path.
- Fix code style, add braces and blank lines.
---
 .../platform/renesas/rcar-vin/rcar-core.c     | 256 ++++++++----------
 .../platform/renesas/rcar-vin/rcar-vin.h      |   2 -
 2 files changed, 106 insertions(+), 152 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index 100432080ad7..c5510e3b258f 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -43,6 +43,9 @@
 
 #define v4l2_dev_to_vin(d)	container_of(d, struct rvin_dev, v4l2_dev)
 
+static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
+					  struct v4l2_subdev *subdev);
+
 /* -----------------------------------------------------------------------------
  * Gen3 Group Allocator
  */
@@ -233,7 +236,10 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
 		}
 	}
 
-	return vin->group->link_setup(vin->group);
+	if (vin->group->link_setup)
+		return vin->group->link_setup(vin->group);
+
+	return  0;
 }
 
 static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
@@ -241,20 +247,32 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 				     struct v4l2_async_connection *asc)
 {
 	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
-	unsigned int i;
+	struct rvin_group *group = vin->group;
 
-	for (i = 0; i < RCAR_VIN_NUM; i++)
-		if (vin->group->vin[i])
-			rvin_v4l2_unregister(vin->group->vin[i]);
+	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
+		if (group->vin[i])
+			rvin_v4l2_unregister(group->vin[i]);
+	}
 
 	mutex_lock(&vin->group->lock);
 
-	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
-		if (vin->group->remotes[i].asc != asc)
+	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
+		if (!group->vin[i] || group->vin[i]->parallel.asc != asc)
 			continue;
-		vin->group->remotes[i].subdev = NULL;
+
+		group->vin[i]->parallel.subdev = NULL;
+
+		vin_dbg(group->vin[i], "Unbind parallel subdev %s\n",
+			subdev->name);
+	}
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(group->remotes); i++) {
+		if (group->remotes[i].asc != asc)
+			continue;
+
+		group->remotes[i].subdev = NULL;
+
 		vin_dbg(vin, "Unbind %s from slot %u\n", subdev->name, i);
-		break;
 	}
 
 	mutex_unlock(&vin->group->lock);
@@ -267,21 +285,38 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
 				   struct v4l2_async_connection *asc)
 {
 	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
-	unsigned int i;
+	struct rvin_group *group = vin->group;
 
-	mutex_lock(&vin->group->lock);
+	guard(mutex)(&group->lock);
 
-	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
+	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
+		int ret;
+
+		if (!group->vin[i] || group->vin[i]->parallel.asc != asc)
+			continue;
+
+		ret = rvin_parallel_subdevice_attach(group->vin[i], subdev);
+		if (ret)
+			return ret;
+
+		v4l2_set_subdev_hostdata(subdev, group->vin[i]);
+
+		vin_dbg(group->vin[i], "Bound subdev %s\n", subdev->name);
+
+		return 0;
+	}
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(group->remotes); i++) {
 		if (vin->group->remotes[i].asc != asc)
 			continue;
+
 		vin->group->remotes[i].subdev = subdev;
 		vin_dbg(vin, "Bound %s to slot %u\n", subdev->name, i);
-		break;
+
+		return 0;
 	}
 
-	mutex_unlock(&vin->group->lock);
-
-	return 0;
+	return -ENODEV;
 }
 
 static const struct v4l2_async_notifier_operations rvin_group_notify_ops = {
@@ -371,7 +406,7 @@ static int rvin_parallel_parse_of(struct rvin_dev *vin)
 	}
 
 	fwnode = fwnode_graph_get_remote_endpoint(ep);
-	asc = v4l2_async_nf_add_fwnode(&vin->notifier, fwnode,
+	asc = v4l2_async_nf_add_fwnode(&vin->group->notifier, fwnode,
 				       struct v4l2_async_connection);
 	if (IS_ERR(asc))
 		return PTR_ERR(asc);
@@ -417,6 +452,12 @@ static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
 		if (!(vin_mask & BIT(i)))
 			continue;
 
+		/* Parse local subdevice. */
+		ret = rvin_parallel_parse_of(vin->group->vin[i]);
+		if (ret)
+			return ret;
+
+		/* Prase shared subdevices. */
 		for (id = 0; id < max_id; id++) {
 			if (vin->group->remotes[id].asc)
 				continue;
@@ -596,124 +637,6 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
 	return 0;
 }
 
-static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
-{
-	rvin_v4l2_unregister(vin);
-	vin->parallel.subdev = NULL;
-
-	if (!vin->info->use_mc)
-		rvin_free_controls(vin);
-}
-
-static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
-{
-	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
-	struct media_entity *source;
-	struct media_entity *sink;
-	int ret;
-
-	ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
-	if (ret < 0) {
-		vin_err(vin, "Failed to register subdev nodes\n");
-		return ret;
-	}
-
-	if (!video_is_registered(&vin->vdev)) {
-		ret = rvin_v4l2_register(vin);
-		if (ret < 0)
-			return ret;
-	}
-
-	if (!vin->info->use_mc)
-		return 0;
-
-	/* If we're running with media-controller, link the subdevs. */
-	source = &vin->parallel.subdev->entity;
-	sink = &vin->vdev.entity;
-
-	ret = media_create_pad_link(source, vin->parallel.source_pad,
-				    sink, vin->parallel.sink_pad, 0);
-	if (ret)
-		vin_err(vin, "Error adding link from %s to %s: %d\n",
-			source->name, sink->name, ret);
-
-	return ret;
-}
-
-static void rvin_parallel_notify_unbind(struct v4l2_async_notifier *notifier,
-					struct v4l2_subdev *subdev,
-					struct v4l2_async_connection *asc)
-{
-	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
-
-	vin_dbg(vin, "unbind parallel subdev %s\n", subdev->name);
-
-	mutex_lock(&vin->lock);
-	rvin_parallel_subdevice_detach(vin);
-	mutex_unlock(&vin->lock);
-}
-
-static int rvin_parallel_notify_bound(struct v4l2_async_notifier *notifier,
-				      struct v4l2_subdev *subdev,
-				      struct v4l2_async_connection *asc)
-{
-	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
-	int ret;
-
-	mutex_lock(&vin->lock);
-	ret = rvin_parallel_subdevice_attach(vin, subdev);
-	mutex_unlock(&vin->lock);
-	if (ret)
-		return ret;
-
-	v4l2_set_subdev_hostdata(subdev, vin);
-
-	vin_dbg(vin, "bound subdev %s source pad: %u sink pad: %u\n",
-		subdev->name, vin->parallel.source_pad,
-		vin->parallel.sink_pad);
-
-	return 0;
-}
-
-static const struct v4l2_async_notifier_operations rvin_parallel_notify_ops = {
-	.bound = rvin_parallel_notify_bound,
-	.unbind = rvin_parallel_notify_unbind,
-	.complete = rvin_parallel_notify_complete,
-};
-
-static void rvin_parallel_cleanup(struct rvin_dev *vin)
-{
-	v4l2_async_nf_unregister(&vin->notifier);
-	v4l2_async_nf_cleanup(&vin->notifier);
-}
-
-static int rvin_parallel_init(struct rvin_dev *vin)
-{
-	int ret;
-
-	v4l2_async_nf_init(&vin->notifier, &vin->v4l2_dev);
-
-	ret = rvin_parallel_parse_of(vin);
-	if (ret)
-		return ret;
-
-	if (!vin->parallel.asc)
-		return -ENODEV;
-
-	vin_dbg(vin, "Found parallel subdevice %pOF\n",
-		to_of_node(vin->parallel.asc->match.fwnode));
-
-	vin->notifier.ops = &rvin_parallel_notify_ops;
-	ret = v4l2_async_nf_register(&vin->notifier);
-	if (ret < 0) {
-		vin_err(vin, "Notifier registration failed\n");
-		v4l2_async_nf_cleanup(&vin->notifier);
-		return ret;
-	}
-
-	return 0;
-}
-
 /* -----------------------------------------------------------------------------
  * CSI-2
  */
@@ -888,11 +811,52 @@ static int rvin_csi2_create_link(struct rvin_group *group, unsigned int id,
 	return 0;
 }
 
+static int rvin_parallel_setup_links(struct rvin_group *group)
+{
+	u32 flags = MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE;
+
+	guard(mutex)(&group->lock);
+
+	/* If the group also has links don't enable the link. */
+	for (unsigned int i = 0; i < ARRAY_SIZE(group->remotes); i++) {
+		if (group->remotes[i].subdev) {
+			flags = 0;
+			break;
+		}
+	}
+
+	/* Create links. */
+	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
+		struct rvin_dev *vin = group->vin[i];
+		struct media_entity *source;
+		struct media_entity *sink;
+		int ret;
+
+		/* Nothing to do if there is no VIN or parallel subdev. */
+		if (!vin || !vin->parallel.subdev)
+			continue;
+
+		source = &vin->parallel.subdev->entity;
+		sink = &vin->vdev.entity;
+
+		ret = media_create_pad_link(source, vin->parallel.source_pad,
+					    sink, 0, flags);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int rvin_csi2_setup_links(struct rvin_group *group)
 {
 	const struct rvin_group_route *route;
 	unsigned int id;
-	int ret = -EINVAL;
+	int ret;
+
+	ret = rvin_parallel_setup_links(group);
+	if (ret)
+		return ret;
 
 	/* Create all media device links between VINs and CSI-2's. */
 	mutex_lock(&group->lock);
@@ -923,9 +887,7 @@ static int rvin_csi2_setup_links(struct rvin_group *group)
 
 static void rvin_csi2_cleanup(struct rvin_dev *vin)
 {
-	rvin_parallel_cleanup(vin);
 	rvin_group_notifier_cleanup(vin);
-	rvin_group_put(vin);
 	rvin_free_controls(vin);
 }
 
@@ -946,18 +908,11 @@ static int rvin_csi2_init(struct rvin_dev *vin)
 	if (ret)
 		goto err_controls;
 
-	/* It's OK to not have a parallel subdevice. */
-	ret = rvin_parallel_init(vin);
-	if (ret && ret != -ENODEV)
-		goto err_group;
-
 	ret = rvin_group_notifier_init(vin, 1, RVIN_CSI_MAX);
 	if (ret)
-		goto err_parallel;
+		goto err_group;
 
 	return 0;
-err_parallel:
-	rvin_parallel_cleanup(vin);
 err_group:
 	rvin_group_put(vin);
 err_controls:
@@ -1018,7 +973,6 @@ static int rvin_isp_setup_links(struct rvin_group *group)
 static void rvin_isp_cleanup(struct rvin_dev *vin)
 {
 	rvin_group_notifier_cleanup(vin);
-	rvin_group_put(vin);
 	rvin_free_controls(vin);
 }
 
@@ -1430,7 +1384,9 @@ static int rcar_vin_probe(struct platform_device *pdev)
 		    rvin_group_id_to_master(vin->id) == vin->id)
 			vin->scaler = vin->info->scaler;
 	} else {
-		ret = rvin_parallel_init(vin);
+		ret = rvin_group_get(vin, NULL, NULL);
+		if (!ret)
+			ret = rvin_group_notifier_init(vin, 0, 0);
 
 		if (vin->info->scaler)
 			vin->scaler = vin->info->scaler;
@@ -1460,8 +1416,8 @@ static void rcar_vin_remove(struct platform_device *pdev)
 		rvin_isp_cleanup(vin);
 	else if (vin->info->use_mc)
 		rvin_csi2_cleanup(vin);
-	else
-		rvin_parallel_cleanup(vin);
+
+	rvin_group_put(vin);
 
 	rvin_id_put(vin);
 
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
index cb8e8fa54f96..38ae2bd20b72 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
@@ -149,7 +149,6 @@ struct rvin_info {
  * @vdev:		V4L2 video device associated with VIN
  * @v4l2_dev:		V4L2 device
  * @ctrl_handler:	V4L2 control handler
- * @notifier:		V4L2 asynchronous subdevs notifier
  *
  * @parallel:		parallel input subdevice descriptor
  *
@@ -189,7 +188,6 @@ struct rvin_dev {
 	struct video_device vdev;
 	struct v4l2_device v4l2_dev;
 	struct v4l2_ctrl_handler ctrl_handler;
-	struct v4l2_async_notifier notifier;
 
 	struct rvin_parallel_entity parallel;
 
-- 
2.49.0

[PATCH v5 08/12] media: rcar-vin: Always create a media pad

[Niklas Söderlund]

Prepare for Gen2 media graph support by always initializing a media pad
for the VIN device.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v4
- New in v5, broken out from a larger patch in v4.
---
 drivers/media/platform/renesas/rcar-vin/rcar-core.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index c5510e3b258f..8cb3d0a3520f 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -895,10 +895,6 @@ static int rvin_csi2_init(struct rvin_dev *vin)
 {
 	int ret;
 
-	vin->pad.flags = MEDIA_PAD_FL_SINK;
-	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
-	if (ret)
-		return ret;
 
 	ret = rvin_create_controls(vin, NULL);
 	if (ret < 0)
@@ -980,10 +976,6 @@ static int rvin_isp_init(struct rvin_dev *vin)
 {
 	int ret;
 
-	vin->pad.flags = MEDIA_PAD_FL_SINK;
-	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
-	if (ret)
-		return ret;
 
 	ret = rvin_create_controls(vin, NULL);
 	if (ret < 0)
@@ -1375,6 +1367,11 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	if (rvin_id_get(vin))
 		return -EINVAL;
 
+	vin->pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
+	if (ret)
+		return ret;
+
 	if (vin->info->use_isp) {
 		ret = rvin_isp_init(vin);
 	} else if (vin->info->use_mc) {
-- 
2.49.0

[PATCH v5 09/12] media: rcar-vin: Remove NTSC workaround

[Niklas Söderlund]

On Gen2 where sub-devices where not exposed to user-space the filed
TB/BT ordering was controlled by a hack in the VIN driver. Before
converting it to media device model where the subdevice is exposed
remove that hack.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v4
- Broken out from larger patch.
---
 drivers/media/platform/renesas/rcar-vin/rcar-dma.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
index 5c08ee2c9807..4fb33359bb0f 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
@@ -700,9 +700,6 @@ static int rvin_setup(struct rvin_dev *vin)
 	case V4L2_FIELD_INTERLACED:
 		/* Default to TB */
 		vnmc = VNMC_IM_FULL;
-		/* Use BT if video standard can be read and is 60 Hz format */
-		if (!vin->info->use_mc && vin->std & V4L2_STD_525_60)
-			vnmc = VNMC_IM_FULL | VNMC_FOC;
 		break;
 	case V4L2_FIELD_INTERLACED_TB:
 		vnmc = VNMC_IM_FULL;
-- 
2.49.0

[PATCH v5 10/12] media: rcar-vin: Only expose VIN controls

[Niklas Söderlund]

Before moving Gen2 to media controller simplify the creation of controls
by not exposing the sub-device controls on the video device. This could
be done while enabling media controller but doing it separately reduces
the changes needed to do so.

The rework also allows the cleanup and remove paths to be simplified by
folding all special cases into the only remaining call site.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v4
- Broken out from a larger patch.
---
 .../platform/renesas/rcar-vin/rcar-core.c     | 84 ++++---------------
 1 file changed, 18 insertions(+), 66 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index 8cb3d0a3520f..597e868a6bc5 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -365,14 +365,6 @@ static int rvin_group_parse_of(struct rvin_dev *vin, unsigned int port,
 	return ret;
 }
 
-static void rvin_group_notifier_cleanup(struct rvin_dev *vin)
-{
-	if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
-		v4l2_async_nf_unregister(&vin->group->notifier);
-		v4l2_async_nf_cleanup(&vin->group->notifier);
-	}
-}
-
 static int rvin_parallel_parse_of(struct rvin_dev *vin)
 {
 	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
@@ -510,7 +502,7 @@ static void rvin_free_controls(struct rvin_dev *vin)
 	vin->vdev.ctrl_handler = NULL;
 }
 
-static int rvin_create_controls(struct rvin_dev *vin, struct v4l2_subdev *subdev)
+static int rvin_create_controls(struct rvin_dev *vin)
 {
 	int ret;
 
@@ -528,16 +520,6 @@ static int rvin_create_controls(struct rvin_dev *vin, struct v4l2_subdev *subdev
 		return ret;
 	}
 
-	/* For the non-MC mode add controls from the subdevice. */
-	if (subdev) {
-		ret = v4l2_ctrl_add_handler(&vin->ctrl_handler,
-					    subdev->ctrl_handler, NULL, true);
-		if (ret < 0) {
-			rvin_free_controls(vin);
-			return ret;
-		}
-	}
-
 	vin->vdev.ctrl_handler = &vin->ctrl_handler;
 
 	return 0;
@@ -627,11 +609,6 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
 	if (ret < 0 && ret != -ENOIOCTLCMD)
 		return ret;
 
-	/* Add the controls */
-	ret = rvin_create_controls(vin, subdev);
-	if (ret < 0)
-		return ret;
-
 	vin->parallel.subdev = subdev;
 
 	return 0;
@@ -885,34 +862,17 @@ static int rvin_csi2_setup_links(struct rvin_group *group)
 	return ret;
 }
 
-static void rvin_csi2_cleanup(struct rvin_dev *vin)
-{
-	rvin_group_notifier_cleanup(vin);
-	rvin_free_controls(vin);
-}
-
 static int rvin_csi2_init(struct rvin_dev *vin)
 {
 	int ret;
 
-
-	ret = rvin_create_controls(vin, NULL);
-	if (ret < 0)
-		return ret;
-
 	ret = rvin_group_get(vin, rvin_csi2_setup_links, &rvin_csi2_media_ops);
 	if (ret)
-		goto err_controls;
+		return ret;
 
 	ret = rvin_group_notifier_init(vin, 1, RVIN_CSI_MAX);
 	if (ret)
-		goto err_group;
-
-	return 0;
-err_group:
-	rvin_group_put(vin);
-err_controls:
-	rvin_free_controls(vin);
+		rvin_group_put(vin);
 
 	return ret;
 }
@@ -966,34 +926,17 @@ static int rvin_isp_setup_links(struct rvin_group *group)
 	return ret;
 }
 
-static void rvin_isp_cleanup(struct rvin_dev *vin)
-{
-	rvin_group_notifier_cleanup(vin);
-	rvin_free_controls(vin);
-}
-
 static int rvin_isp_init(struct rvin_dev *vin)
 {
 	int ret;
 
-
-	ret = rvin_create_controls(vin, NULL);
-	if (ret < 0)
-		return ret;
-
 	ret = rvin_group_get(vin, rvin_isp_setup_links, NULL);
 	if (ret)
-		goto err_controls;
+		return ret;
 
 	ret = rvin_group_notifier_init(vin, 2, RVIN_ISP_MAX);
 	if (ret)
-		goto err_group;
-
-	return 0;
-err_group:
-	rvin_group_put(vin);
-err_controls:
-	rvin_free_controls(vin);
+		rvin_group_put(vin);
 
 	return ret;
 }
@@ -1372,6 +1315,10 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = rvin_create_controls(vin);
+	if (ret < 0)
+		return ret;
+
 	if (vin->info->use_isp) {
 		ret = rvin_isp_init(vin);
 	} else if (vin->info->use_mc) {
@@ -1390,6 +1337,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	}
 
 	if (ret) {
+		rvin_free_controls(vin);
 		rvin_dma_unregister(vin);
 		rvin_id_put(vin);
 		return ret;
@@ -1409,13 +1357,17 @@ static void rcar_vin_remove(struct platform_device *pdev)
 
 	rvin_v4l2_unregister(vin);
 
-	if (vin->info->use_isp)
-		rvin_isp_cleanup(vin);
-	else if (vin->info->use_mc)
-		rvin_csi2_cleanup(vin);
+	if (vin->info->use_isp || vin->info->use_mc) {
+		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
+			v4l2_async_nf_unregister(&vin->group->notifier);
+			v4l2_async_nf_cleanup(&vin->group->notifier);
+		}
+	}
 
 	rvin_group_put(vin);
 
+	rvin_free_controls(vin);
+
 	rvin_id_put(vin);
 
 	rvin_dma_unregister(vin);
-- 
2.49.0

[PATCH v5 11/12] media: rcar-vin: Enable media-graph on Gen2

[Niklas Söderlund]

Complete the conversion from soc_camera to a full fledge media
controller enabled devices for all supported generations of the device.
All work is already done as this is already supported on Gen3, and
later.

All that is missing is removing all special cases for the non
media-graph call paths and use the common ones in their place.

The one change that stands out is dropping the doubling of the height in
the Gen2 scaler setup, rvin_scaler_gen2(). In the Gen2 non-MC world the
VIN size was set to match the video source subdevices, and if that was a
TOP/BOTTOM video source it needed to be doubled for the scaler to
function properly. In the MC world this is now handled by user-space
configuration of the pipeline and the adjustment is not needed.

Mark the completion of converting from soc_camera by injecting an
attribution of myself in the header.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v3
- Break out small things to ease review.
- Add rationale of odd looking change in rvin_scaler_gen2().

* Changes since v3
- Resolve conflicts with other VIN work merged a head of this series.
---
 .../platform/renesas/rcar-vin/rcar-core.c     | 158 +-----
 .../platform/renesas/rcar-vin/rcar-dma.c      |  20 +-
 .../platform/renesas/rcar-vin/rcar-v4l2.c     | 468 +-----------------
 .../platform/renesas/rcar-vin/rcar-vin.h      |  10 +-
 4 files changed, 41 insertions(+), 615 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
index 597e868a6bc5..6c653a8a8c27 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
@@ -2,12 +2,11 @@
 /*
  * Driver for Renesas R-Car VIN
  *
+ * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
  * Copyright (C) 2016 Renesas Electronics Corp.
  * Copyright (C) 2011-2013 Renesas Solutions Corp.
  * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
  * Copyright (C) 2008 Magnus Damm
- *
- * Based on the soc-camera rcar_vin driver
  */
 
 #include <linux/idr.h>
@@ -43,9 +42,6 @@
 
 #define v4l2_dev_to_vin(d)	container_of(d, struct rvin_dev, v4l2_dev)
 
-static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
-					  struct v4l2_subdev *subdev);
-
 /* -----------------------------------------------------------------------------
  * Gen3 Group Allocator
  */
@@ -236,10 +232,7 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
 		}
 	}
 
-	if (vin->group->link_setup)
-		return vin->group->link_setup(vin->group);
-
-	return  0;
+	return vin->group->link_setup(vin->group);
 }
 
 static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
@@ -290,17 +283,15 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
 	guard(mutex)(&group->lock);
 
 	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
-		int ret;
-
 		if (!group->vin[i] || group->vin[i]->parallel.asc != asc)
 			continue;
 
-		ret = rvin_parallel_subdevice_attach(group->vin[i], subdev);
-		if (ret)
-			return ret;
-
-		v4l2_set_subdev_hostdata(subdev, group->vin[i]);
+		group->vin[i]->parallel.source_pad = 0;
+		for (unsigned int pad = 0; pad < subdev->entity.num_pads; pad++)
+			if (subdev->entity.pads[pad].flags & MEDIA_PAD_FL_SOURCE)
+				group->vin[i]->parallel.source_pad = pad;
 
+		group->vin[i]->parallel.subdev = subdev;
 		vin_dbg(group->vin[i], "Bound subdev %s\n", subdev->name);
 
 		return 0;
@@ -525,95 +516,6 @@ static int rvin_create_controls(struct rvin_dev *vin)
 	return 0;
 }
 
-/* -----------------------------------------------------------------------------
- * Async notifier
- */
-
-static int rvin_find_pad(struct v4l2_subdev *sd, int direction)
-{
-	unsigned int pad;
-
-	if (sd->entity.num_pads <= 1)
-		return 0;
-
-	for (pad = 0; pad < sd->entity.num_pads; pad++)
-		if (sd->entity.pads[pad].flags & direction)
-			return pad;
-
-	return -EINVAL;
-}
-
-/* -----------------------------------------------------------------------------
- * Parallel async notifier
- */
-
-/* The vin lock should be held when calling the subdevice attach and detach */
-static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
-					  struct v4l2_subdev *subdev)
-{
-	struct v4l2_subdev_mbus_code_enum code = {
-		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
-	};
-	int ret;
-
-	/* Find source and sink pad of remote subdevice */
-	ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
-	if (ret < 0)
-		return ret;
-	vin->parallel.source_pad = ret;
-
-	ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
-	vin->parallel.sink_pad = ret < 0 ? 0 : ret;
-
-	if (vin->info->use_mc) {
-		vin->parallel.subdev = subdev;
-		return 0;
-	}
-
-	/* Find compatible subdevices mbus format */
-	vin->mbus_code = 0;
-	code.index = 0;
-	code.pad = vin->parallel.source_pad;
-	while (!vin->mbus_code &&
-	       !v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
-		code.index++;
-		switch (code.code) {
-		case MEDIA_BUS_FMT_YUYV8_1X16:
-		case MEDIA_BUS_FMT_UYVY8_1X16:
-		case MEDIA_BUS_FMT_UYVY8_2X8:
-		case MEDIA_BUS_FMT_UYVY10_2X10:
-		case MEDIA_BUS_FMT_RGB888_1X24:
-			vin->mbus_code = code.code;
-			vin_dbg(vin, "Found media bus format for %s: %d\n",
-				subdev->name, vin->mbus_code);
-			break;
-		default:
-			break;
-		}
-	}
-
-	if (!vin->mbus_code) {
-		vin_err(vin, "Unsupported media bus format for %s\n",
-			subdev->name);
-		return -EINVAL;
-	}
-
-	/* Read tvnorms */
-	ret = v4l2_subdev_call(subdev, video, g_tvnorms, &vin->vdev.tvnorms);
-	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
-		return ret;
-
-	/* Read standard */
-	vin->std = V4L2_STD_UNKNOWN;
-	ret = v4l2_subdev_call(subdev, video, g_std, &vin->std);
-	if (ret < 0 && ret != -ENOIOCTLCMD)
-		return ret;
-
-	vin->parallel.subdev = subdev;
-
-	return 0;
-}
-
 /* -----------------------------------------------------------------------------
  * CSI-2
  */
@@ -971,7 +873,7 @@ static int __maybe_unused rvin_resume(struct device *dev)
 	 * as we don't know if and in which order the master VINs will
 	 * be resumed.
 	 */
-	if (vin->info->use_mc) {
+	if (vin->info->model == RCAR_GEN3) {
 		unsigned int master_id = rvin_group_id_to_master(vin->id);
 		struct rvin_dev *master = vin->group->vin[master_id];
 		int ret;
@@ -993,7 +895,6 @@ static int __maybe_unused rvin_resume(struct device *dev)
 
 static const struct rvin_info rcar_info_h1 = {
 	.model = RCAR_H1,
-	.use_mc = false,
 	.max_width = 2048,
 	.max_height = 2048,
 	.scaler = rvin_scaler_gen2,
@@ -1001,7 +902,6 @@ static const struct rvin_info rcar_info_h1 = {
 
 static const struct rvin_info rcar_info_m1 = {
 	.model = RCAR_M1,
-	.use_mc = false,
 	.max_width = 2048,
 	.max_height = 2048,
 	.scaler = rvin_scaler_gen2,
@@ -1009,7 +909,6 @@ static const struct rvin_info rcar_info_m1 = {
 
 static const struct rvin_info rcar_info_gen2 = {
 	.model = RCAR_GEN2,
-	.use_mc = false,
 	.max_width = 2048,
 	.max_height = 2048,
 	.scaler = rvin_scaler_gen2,
@@ -1024,7 +923,6 @@ static const struct rvin_group_route rcar_info_r8a774e1_routes[] = {
 
 static const struct rvin_info rcar_info_r8a774e1 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a774e1_routes,
@@ -1040,7 +938,6 @@ static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
 
 static const struct rvin_info rcar_info_r8a7795 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.nv12 = true,
 	.max_width = 4096,
 	.max_height = 4096,
@@ -1058,7 +955,6 @@ static const struct rvin_group_route rcar_info_r8a7796_routes[] = {
 
 static const struct rvin_info rcar_info_r8a7796 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.nv12 = true,
 	.max_width = 4096,
 	.max_height = 4096,
@@ -1076,7 +972,6 @@ static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
 
 static const struct rvin_info rcar_info_r8a77965 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.nv12 = true,
 	.max_width = 4096,
 	.max_height = 4096,
@@ -1091,7 +986,6 @@ static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
 
 static const struct rvin_info rcar_info_r8a77970 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77970_routes,
@@ -1105,7 +999,6 @@ static const struct rvin_group_route rcar_info_r8a77980_routes[] = {
 
 static const struct rvin_info rcar_info_r8a77980 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.nv12 = true,
 	.max_width = 4096,
 	.max_height = 4096,
@@ -1119,7 +1012,6 @@ static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
 
 static const struct rvin_info rcar_info_r8a77990 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.nv12 = true,
 	.max_width = 4096,
 	.max_height = 4096,
@@ -1133,7 +1025,6 @@ static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
 
 static const struct rvin_info rcar_info_r8a77995 = {
 	.model = RCAR_GEN3,
-	.use_mc = true,
 	.nv12 = true,
 	.max_width = 4096,
 	.max_height = 4096,
@@ -1143,7 +1034,6 @@ static const struct rvin_info rcar_info_r8a77995 = {
 
 static const struct rvin_info rcar_info_gen4 = {
 	.model = RCAR_GEN4,
-	.use_mc = true,
 	.use_isp = true,
 	.nv12 = true,
 	.raw10 = true,
@@ -1319,21 +1209,27 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	if (vin->info->use_isp) {
-		ret = rvin_isp_init(vin);
-	} else if (vin->info->use_mc) {
-		ret = rvin_csi2_init(vin);
+	switch (vin->info->model) {
+	case RCAR_GEN3:
+	case RCAR_GEN4:
+		if (vin->info->use_isp) {
+			ret = rvin_isp_init(vin);
+		} else {
+			ret = rvin_csi2_init(vin);
 
-		if (vin->info->scaler &&
-		    rvin_group_id_to_master(vin->id) == vin->id)
-			vin->scaler = vin->info->scaler;
-	} else {
-		ret = rvin_group_get(vin, NULL, NULL);
+			if (vin->info->scaler &&
+			    rvin_group_id_to_master(vin->id) == vin->id)
+				vin->scaler = vin->info->scaler;
+		}
+		break;
+	default:
+		ret = rvin_group_get(vin, rvin_parallel_setup_links, NULL);
 		if (!ret)
 			ret = rvin_group_notifier_init(vin, 0, 0);
 
 		if (vin->info->scaler)
 			vin->scaler = vin->info->scaler;
+		break;
 	}
 
 	if (ret) {
@@ -1357,11 +1253,9 @@ static void rcar_vin_remove(struct platform_device *pdev)
 
 	rvin_v4l2_unregister(vin);
 
-	if (vin->info->use_isp || vin->info->use_mc) {
-		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
-			v4l2_async_nf_unregister(&vin->group->notifier);
-			v4l2_async_nf_cleanup(&vin->group->notifier);
-		}
+	if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
+		v4l2_async_nf_unregister(&vin->group->notifier);
+		v4l2_async_nf_cleanup(&vin->group->notifier);
 	}
 
 	rvin_group_put(vin);
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
index 4fb33359bb0f..d4faa5a4e757 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
@@ -2,12 +2,11 @@
 /*
  * Driver for Renesas R-Car VIN
  *
+ * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
  * Copyright (C) 2016 Renesas Electronics Corp.
  * Copyright (C) 2011-2013 Renesas Solutions Corp.
  * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
  * Copyright (C) 2008 Magnus Damm
- *
- * Based on the soc-camera rcar_vin driver
  */
 
 #include <linux/delay.h>
@@ -555,17 +554,12 @@ static void rvin_set_coeff(struct rvin_dev *vin, unsigned short xs)
 
 void rvin_scaler_gen2(struct rvin_dev *vin)
 {
-	unsigned int crop_height;
 	u32 xs, ys;
 
 	/* Set scaling coefficient */
-	crop_height = vin->crop.height;
-	if (V4L2_FIELD_HAS_BOTH(vin->format.field))
-		crop_height *= 2;
-
 	ys = 0;
-	if (crop_height != vin->compose.height)
-		ys = (4096 * crop_height) / vin->compose.height;
+	if (vin->crop.height != vin->compose.height)
+		ys = (4096 * vin->crop.height) / vin->compose.height;
 	rvin_write(vin, ys, VNYS_REG);
 
 	xs = 0;
@@ -1294,14 +1288,6 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
 	struct media_pad *pad;
 	int ret;
 
-	/* No media controller used, simply pass operation to subdevice. */
-	if (!vin->info->use_mc) {
-		ret = v4l2_subdev_call(vin->parallel.subdev, video, s_stream,
-				       on);
-
-		return ret == -ENOIOCTLCMD ? 0 : ret;
-	}
-
 	pad = media_pad_remote_pad_first(&vin->pad);
 	if (!pad)
 		return -EPIPE;
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
index db091af57c19..2bf94bd77c24 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
@@ -2,12 +2,11 @@
 /*
  * Driver for Renesas R-Car VIN
  *
+ * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
  * Copyright (C) 2016 Renesas Electronics Corp.
  * Copyright (C) 2011-2013 Renesas Solutions Corp.
  * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
  * Copyright (C) 2008 Magnus Damm
- *
- * Based on the soc-camera rcar_vin driver
  */
 
 #include <linux/pm_runtime.h>
@@ -230,101 +229,6 @@ static void rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format *pix)
  * V4L2
  */
 
-static int rvin_reset_format(struct rvin_dev *vin)
-{
-	struct v4l2_subdev_format fmt = {
-		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
-		.pad = vin->parallel.source_pad,
-	};
-	int ret;
-
-	ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);
-	if (ret)
-		return ret;
-
-	v4l2_fill_pix_format(&vin->format, &fmt.format);
-
-	vin->crop.top = 0;
-	vin->crop.left = 0;
-	vin->crop.width = vin->format.width;
-	vin->crop.height = vin->format.height;
-
-	/*  Make use of the hardware interlacer by default. */
-	if (vin->format.field == V4L2_FIELD_ALTERNATE) {
-		vin->format.field = V4L2_FIELD_INTERLACED;
-		vin->format.height *= 2;
-	}
-
-	rvin_format_align(vin, &vin->format);
-
-	vin->compose.top = 0;
-	vin->compose.left = 0;
-	vin->compose.width = vin->format.width;
-	vin->compose.height = vin->format.height;
-
-	return 0;
-}
-
-static int rvin_try_format(struct rvin_dev *vin, u32 which,
-			   struct v4l2_pix_format *pix,
-			   struct v4l2_rect *src_rect)
-{
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	struct v4l2_subdev_state *sd_state;
-	static struct lock_class_key key;
-	struct v4l2_subdev_format format = {
-		.which = which,
-		.pad = vin->parallel.source_pad,
-	};
-	enum v4l2_field field;
-	u32 width, height;
-	int ret;
-
-	/*
-	 * FIXME: Drop this call, drivers are not supposed to use
-	 * __v4l2_subdev_state_alloc().
-	 */
-	sd_state = __v4l2_subdev_state_alloc(sd, "rvin:state->lock", &key);
-	if (IS_ERR(sd_state))
-		return PTR_ERR(sd_state);
-
-	if (!rvin_format_from_pixel(vin, pix->pixelformat))
-		pix->pixelformat = RVIN_DEFAULT_FORMAT;
-
-	v4l2_fill_mbus_format(&format.format, pix, vin->mbus_code);
-
-	/* Allow the video device to override field and to scale */
-	field = pix->field;
-	width = pix->width;
-	height = pix->height;
-
-	ret = v4l2_subdev_call(sd, pad, set_fmt, sd_state, &format);
-	if (ret < 0 && ret != -ENOIOCTLCMD)
-		goto done;
-	ret = 0;
-
-	v4l2_fill_pix_format(pix, &format.format);
-
-	if (src_rect) {
-		src_rect->top = 0;
-		src_rect->left = 0;
-		src_rect->width = pix->width;
-		src_rect->height = pix->height;
-	}
-
-	if (field != V4L2_FIELD_ANY)
-		pix->field = field;
-
-	pix->width = width;
-	pix->height = height;
-
-	rvin_format_align(vin, pix);
-done:
-	__v4l2_subdev_state_free(sd_state);
-
-	return ret;
-}
-
 static int rvin_querycap(struct file *file, void *priv,
 			 struct v4l2_capability *cap)
 {
@@ -333,42 +237,6 @@ static int rvin_querycap(struct file *file, void *priv,
 	return 0;
 }
 
-static int rvin_try_fmt_vid_cap(struct file *file, void *priv,
-				struct v4l2_format *f)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-
-	return rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix, NULL);
-}
-
-static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
-			      struct v4l2_format *f)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_rect fmt_rect, src_rect;
-	int ret;
-
-	if (vb2_is_busy(&vin->queue))
-		return -EBUSY;
-
-	ret = rvin_try_format(vin, V4L2_SUBDEV_FORMAT_ACTIVE, &f->fmt.pix,
-			      &src_rect);
-	if (ret)
-		return ret;
-
-	vin->format = f->fmt.pix;
-
-	fmt_rect.top = 0;
-	fmt_rect.left = 0;
-	fmt_rect.width = vin->format.width;
-	fmt_rect.height = vin->format.height;
-
-	v4l2_rect_map_inside(&vin->crop, &src_rect);
-	v4l2_rect_map_inside(&vin->compose, &fmt_rect);
-
-	return 0;
-}
-
 static int rvin_g_fmt_vid_cap(struct file *file, void *priv,
 			      struct v4l2_format *f)
 {
@@ -465,6 +333,7 @@ static int rvin_enum_fmt_vid_cap(struct file *file, void *priv,
 
 static int rvin_remote_rectangle(struct rvin_dev *vin, struct v4l2_rect *rect)
 {
+	struct media_pad *pad = media_pad_remote_pad_first(&vin->pad);
 	struct v4l2_subdev_format fmt = {
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
@@ -472,18 +341,11 @@ static int rvin_remote_rectangle(struct rvin_dev *vin, struct v4l2_rect *rect)
 	unsigned int index;
 	int ret;
 
-	if (vin->info->use_mc) {
-		struct media_pad *pad = media_pad_remote_pad_first(&vin->pad);
+	if (!pad)
+		return -EINVAL;
 
-		if (!pad)
-			return -EINVAL;
-
-		sd = media_entity_to_v4l2_subdev(pad->entity);
-		index = pad->index;
-	} else {
-		sd = vin_to_source(vin);
-		index = vin->parallel.source_pad;
-	}
+	sd = media_entity_to_v4l2_subdev(pad->entity);
+	index = pad->index;
 
 	fmt.pad = index;
 	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
@@ -623,113 +485,6 @@ static int rvin_s_selection(struct file *file, void *fh,
 	return 0;
 }
 
-static int rvin_g_parm(struct file *file, void *priv,
-		       struct v4l2_streamparm *parm)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-
-	return v4l2_g_parm_cap(&vin->vdev, sd, parm);
-}
-
-static int rvin_s_parm(struct file *file, void *priv,
-		       struct v4l2_streamparm *parm)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-
-	return v4l2_s_parm_cap(&vin->vdev, sd, parm);
-}
-
-static int rvin_g_pixelaspect(struct file *file, void *priv,
-			      int type, struct v4l2_fract *f)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-
-	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		return -EINVAL;
-
-	return v4l2_subdev_call(sd, video, g_pixelaspect, f);
-}
-
-static int rvin_enum_input(struct file *file, void *priv,
-			   struct v4l2_input *i)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	int ret;
-
-	if (i->index != 0)
-		return -EINVAL;
-
-	ret = v4l2_subdev_call(sd, video, g_input_status, &i->status);
-	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
-		return ret;
-
-	i->type = V4L2_INPUT_TYPE_CAMERA;
-
-	if (v4l2_subdev_has_op(sd, pad, dv_timings_cap)) {
-		i->capabilities = V4L2_IN_CAP_DV_TIMINGS;
-		i->std = 0;
-	} else {
-		i->capabilities = V4L2_IN_CAP_STD;
-		i->std = vin->vdev.tvnorms;
-	}
-
-	strscpy(i->name, "Camera", sizeof(i->name));
-
-	return 0;
-}
-
-static int rvin_g_input(struct file *file, void *priv, unsigned int *i)
-{
-	*i = 0;
-	return 0;
-}
-
-static int rvin_s_input(struct file *file, void *priv, unsigned int i)
-{
-	if (i > 0)
-		return -EINVAL;
-	return 0;
-}
-
-static int rvin_querystd(struct file *file, void *priv, v4l2_std_id *a)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-
-	return v4l2_subdev_call(sd, video, querystd, a);
-}
-
-static int rvin_s_std(struct file *file, void *priv, v4l2_std_id a)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	int ret;
-
-	ret = v4l2_subdev_call(vin_to_source(vin), video, s_std, a);
-	if (ret < 0)
-		return ret;
-
-	vin->std = a;
-
-	/* Changing the standard will change the width/height */
-	return rvin_reset_format(vin);
-}
-
-static int rvin_g_std(struct file *file, void *priv, v4l2_std_id *a)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-
-	if (v4l2_subdev_has_op(vin_to_source(vin), pad, dv_timings_cap))
-		return -ENOIOCTLCMD;
-
-	*a = vin->std;
-
-	return 0;
-}
-
 static int rvin_subscribe_event(struct v4l2_fh *fh,
 				const struct v4l2_event_subscription *sub)
 {
@@ -740,167 +495,6 @@ static int rvin_subscribe_event(struct v4l2_fh *fh,
 	return v4l2_ctrl_subscribe_event(fh, sub);
 }
 
-static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
-				struct v4l2_enum_dv_timings *timings)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	int ret;
-
-	if (timings->pad)
-		return -EINVAL;
-
-	timings->pad = vin->parallel.sink_pad;
-
-	ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
-
-	timings->pad = 0;
-
-	return ret;
-}
-
-static int rvin_s_dv_timings(struct file *file, void *priv_fh,
-			     struct v4l2_dv_timings *timings)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	int ret;
-
-	ret = v4l2_subdev_call(sd, pad, s_dv_timings,
-			       vin->parallel.sink_pad, timings);
-	if (ret)
-		return ret;
-
-	/* Changing the timings will change the width/height */
-	return rvin_reset_format(vin);
-}
-
-static int rvin_g_dv_timings(struct file *file, void *priv_fh,
-			     struct v4l2_dv_timings *timings)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-
-	return v4l2_subdev_call(sd, pad, g_dv_timings,
-				vin->parallel.sink_pad, timings);
-}
-
-static int rvin_query_dv_timings(struct file *file, void *priv_fh,
-				 struct v4l2_dv_timings *timings)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-
-	return v4l2_subdev_call(sd, pad, query_dv_timings,
-				vin->parallel.sink_pad, timings);
-}
-
-static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
-			       struct v4l2_dv_timings_cap *cap)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	int ret;
-
-	if (cap->pad)
-		return -EINVAL;
-
-	cap->pad = vin->parallel.sink_pad;
-
-	ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
-
-	cap->pad = 0;
-
-	return ret;
-}
-
-static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	int ret;
-
-	if (edid->pad)
-		return -EINVAL;
-
-	edid->pad = vin->parallel.sink_pad;
-
-	ret = v4l2_subdev_call(sd, pad, get_edid, edid);
-
-	edid->pad = 0;
-
-	return ret;
-}
-
-static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
-{
-	struct rvin_dev *vin = video_drvdata(file);
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	int ret;
-
-	if (edid->pad)
-		return -EINVAL;
-
-	edid->pad = vin->parallel.sink_pad;
-
-	ret = v4l2_subdev_call(sd, pad, set_edid, edid);
-
-	edid->pad = 0;
-
-	return ret;
-}
-
-static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
-	.vidioc_querycap		= rvin_querycap,
-	.vidioc_try_fmt_vid_cap		= rvin_try_fmt_vid_cap,
-	.vidioc_g_fmt_vid_cap		= rvin_g_fmt_vid_cap,
-	.vidioc_s_fmt_vid_cap		= rvin_s_fmt_vid_cap,
-	.vidioc_enum_fmt_vid_cap	= rvin_enum_fmt_vid_cap,
-
-	.vidioc_g_selection		= rvin_g_selection,
-	.vidioc_s_selection		= rvin_s_selection,
-
-	.vidioc_g_parm			= rvin_g_parm,
-	.vidioc_s_parm			= rvin_s_parm,
-
-	.vidioc_g_pixelaspect		= rvin_g_pixelaspect,
-
-	.vidioc_enum_input		= rvin_enum_input,
-	.vidioc_g_input			= rvin_g_input,
-	.vidioc_s_input			= rvin_s_input,
-
-	.vidioc_dv_timings_cap		= rvin_dv_timings_cap,
-	.vidioc_enum_dv_timings		= rvin_enum_dv_timings,
-	.vidioc_g_dv_timings		= rvin_g_dv_timings,
-	.vidioc_s_dv_timings		= rvin_s_dv_timings,
-	.vidioc_query_dv_timings	= rvin_query_dv_timings,
-
-	.vidioc_g_edid			= rvin_g_edid,
-	.vidioc_s_edid			= rvin_s_edid,
-
-	.vidioc_querystd		= rvin_querystd,
-	.vidioc_g_std			= rvin_g_std,
-	.vidioc_s_std			= rvin_s_std,
-
-	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
-	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
-	.vidioc_querybuf		= vb2_ioctl_querybuf,
-	.vidioc_qbuf			= vb2_ioctl_qbuf,
-	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
-	.vidioc_expbuf			= vb2_ioctl_expbuf,
-	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
-	.vidioc_streamon		= vb2_ioctl_streamon,
-	.vidioc_streamoff		= vb2_ioctl_streamoff,
-
-	.vidioc_log_status		= v4l2_ctrl_log_status,
-	.vidioc_subscribe_event		= rvin_subscribe_event,
-	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
-};
-
-/* -----------------------------------------------------------------------------
- * V4L2 Media Controller
- */
-
 static void rvin_mc_try_format(struct rvin_dev *vin,
 			       struct v4l2_pix_format *pix)
 {
@@ -979,19 +573,6 @@ static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = {
  * File Operations
  */
 
-static int rvin_power_parallel(struct rvin_dev *vin, bool on)
-{
-	struct v4l2_subdev *sd = vin_to_source(vin);
-	int power = on ? 1 : 0;
-	int ret;
-
-	ret = v4l2_subdev_call(sd, core, s_power, power);
-	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
-		return ret;
-
-	return 0;
-}
-
 static int rvin_open(struct file *file)
 {
 	struct rvin_dev *vin = video_drvdata(file);
@@ -1011,11 +592,7 @@ static int rvin_open(struct file *file)
 	if (ret)
 		goto err_unlock;
 
-	if (vin->info->use_mc)
-		ret = v4l2_pipeline_pm_get(&vin->vdev.entity);
-	else if (v4l2_fh_is_singular_file(file))
-		ret = rvin_power_parallel(vin, true);
-
+	ret = v4l2_pipeline_pm_get(&vin->vdev.entity);
 	if (ret < 0)
 		goto err_open;
 
@@ -1027,10 +604,7 @@ static int rvin_open(struct file *file)
 
 	return 0;
 err_power:
-	if (vin->info->use_mc)
-		v4l2_pipeline_pm_put(&vin->vdev.entity);
-	else if (v4l2_fh_is_singular_file(file))
-		rvin_power_parallel(vin, false);
+	v4l2_pipeline_pm_put(&vin->vdev.entity);
 err_open:
 	v4l2_fh_release(file);
 err_unlock:
@@ -1044,23 +618,14 @@ static int rvin_open(struct file *file)
 static int rvin_release(struct file *file)
 {
 	struct rvin_dev *vin = video_drvdata(file);
-	bool fh_singular;
 	int ret;
 
 	mutex_lock(&vin->lock);
 
-	/* Save the singular status before we call the clean-up helper */
-	fh_singular = v4l2_fh_is_singular_file(file);
-
 	/* the release helper will cleanup any on-going streaming */
 	ret = _vb2_fop_release(file, NULL);
 
-	if (vin->info->use_mc) {
-		v4l2_pipeline_pm_put(&vin->vdev.entity);
-	} else {
-		if (fh_singular)
-			rvin_power_parallel(vin, false);
-	}
+	v4l2_pipeline_pm_put(&vin->vdev.entity);
 
 	mutex_unlock(&vin->lock);
 
@@ -1113,12 +678,6 @@ static void rvin_notify(struct v4l2_subdev *sd,
 		container_of(sd->v4l2_dev, struct rvin_dev, v4l2_dev);
 	unsigned int i;
 
-	/* If no media controller, no need to route the event. */
-	if (!vin->info->use_mc) {
-		rvin_notify_video_device(vin, notification, arg);
-		return;
-	}
-
 	group = vin->group;
 
 	for (i = 0; i < RCAR_VIN_NUM; i++) {
@@ -1162,13 +721,8 @@ int rvin_v4l2_register(struct rvin_dev *vin)
 	vin->format.field = RVIN_DEFAULT_FIELD;
 	vin->format.colorspace = RVIN_DEFAULT_COLORSPACE;
 
-	if (vin->info->use_mc) {
-		vdev->device_caps |= V4L2_CAP_IO_MC;
-		vdev->ioctl_ops = &rvin_mc_ioctl_ops;
-	} else {
-		vdev->ioctl_ops = &rvin_ioctl_ops;
-		rvin_reset_format(vin);
-	}
+	vdev->device_caps |= V4L2_CAP_IO_MC;
+	vdev->ioctl_ops = &rvin_mc_ioctl_ops;
 
 	rvin_format_align(vin, &vin->format);
 
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
index 38ae2bd20b72..74bef5b8adad 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
@@ -2,12 +2,11 @@
 /*
  * Driver for Renesas R-Car VIN
  *
+ * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
  * Copyright (C) 2016 Renesas Electronics Corp.
  * Copyright (C) 2011-2013 Renesas Solutions Corp.
  * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
  * Copyright (C) 2008 Magnus Damm
- *
- * Based on the soc-camera rcar_vin driver
  */
 
 #ifndef __RCAR_VIN__
@@ -79,8 +78,6 @@ struct rvin_video_format {
  * @mbus_type:	media bus type
  * @bus:	media bus parallel configuration
  * @source_pad:	source pad of remote subdevice
- * @sink_pad:	sink pad of remote subdevice
- *
  */
 struct rvin_parallel_entity {
 	struct v4l2_async_connection *asc;
@@ -90,7 +87,6 @@ struct rvin_parallel_entity {
 	struct v4l2_mbus_config_parallel bus;
 
 	unsigned int source_pad;
-	unsigned int sink_pad;
 };
 
 /**
@@ -117,7 +113,6 @@ struct rvin_group_route {
 /**
  * struct rvin_info - Information about the particular VIN implementation
  * @model:		VIN model
- * @use_mc:		use media controller instead of controlling subdevice
  * @use_isp:		the VIN is connected to the ISP and not to the CSI-2
  * @nv12:		support outputting NV12 pixel format
  * @raw10:		support outputting RAW10 pixel format
@@ -129,7 +124,6 @@ struct rvin_group_route {
  */
 struct rvin_info {
 	enum model_id model;
-	bool use_mc;
 	bool use_isp;
 	bool nv12;
 	bool raw10;
@@ -176,7 +170,6 @@ struct rvin_info {
  * @crop:		active cropping
  * @compose:		active composing
  * @scaler:		Optional scaler
- * @std:		active video standard of the video source
  *
  * @alpha:		Alpha component to fill in for supported pixel formats
  */
@@ -218,7 +211,6 @@ struct rvin_dev {
 	struct v4l2_rect crop;
 	struct v4l2_rect compose;
 	void (*scaler)(struct rvin_dev *vin);
-	v4l2_std_id std;
 
 	unsigned int alpha;
 };
-- 
2.49.0

[PATCH v5 12/12] media: rcar-vin: Fold event notifier into only user

[Niklas Söderlund]

With Gen2 converted to use the common media device there is only one
caller left for the helper to notify a video device of an event, fold it
in.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v4
- Broken out from larger patch.
---
 .../platform/renesas/rcar-vin/rcar-v4l2.c     | 20 +++++++------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
index 2bf94bd77c24..59b01cb0628a 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
@@ -656,18 +656,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
 	video_unregister_device(&vin->vdev);
 }
 
-static void rvin_notify_video_device(struct rvin_dev *vin,
-				     unsigned int notification, void *arg)
-{
-	switch (notification) {
-	case V4L2_DEVICE_NOTIFY_EVENT:
-		v4l2_event_queue(&vin->vdev, arg);
-		break;
-	default:
-		break;
-	}
-}
-
 static void rvin_notify(struct v4l2_subdev *sd,
 			unsigned int notification, void *arg)
 {
@@ -693,7 +681,13 @@ static void rvin_notify(struct v4l2_subdev *sd,
 		if (remote != sd)
 			continue;
 
-		rvin_notify_video_device(vin, notification, arg);
+		switch (notification) {
+		case V4L2_DEVICE_NOTIFY_EVENT:
+			v4l2_event_queue(&vin->vdev, arg);
+			break;
+		default:
+			break;
+		}
 	}
 }
 
-- 
2.49.0

Re: [PATCH v5 01/12] media: rcar-vin: Use correct count of remote subdevices

[Sakari Ailus]

Hejssan,

On Fri, Jun 06, 2025 at 08:25:55PM +0200, Niklas Söderlund wrote:
> When extending the driver with Gen4 support the iteration of over
> possible remote subdevices changed from being R-Car CSI-2 Rx only to
> also cover R-Car CSISP instances. In two loops updating the bounds
> variable was missed.
> 
> This had no ill effect as the count the two values have always been the
> same in the past. Fix it by looking at the array size.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> * Changes since v4
> - Use ARRAY_SIZE() instead of updating the incorrect define to
>   RVIN_REMOTES_MAX.

Do you still need RVIN_REMOTES_MAX()? Couldn't you use ARRAY_SIZE()
elsewhere, too?

> ---
>  drivers/media/platform/renesas/rcar-vin/rcar-core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index 846ae7989b1d..cf5830d7d7b1 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -213,7 +213,7 @@ static int rvin_group_entity_to_remote_id(struct rvin_group *group,
>  
>  	sd = media_entity_to_v4l2_subdev(entity);
>  
> -	for (i = 0; i < RVIN_REMOTES_MAX; i++)
> +	for (i = 0; i < ARRAY_SIZE(group->remotes); i++)
>  		if (group->remotes[i].subdev == sd)
>  			return i;
>  
> @@ -262,7 +262,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  
>  	mutex_lock(&vin->group->lock);
>  
> -	for (i = 0; i < RVIN_CSI_MAX; i++) {
> +	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
>  		if (vin->group->remotes[i].asc != asc)
>  			continue;
>  		vin->group->remotes[i].subdev = NULL;
> @@ -284,7 +284,7 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
>  
>  	mutex_lock(&vin->group->lock);
>  
> -	for (i = 0; i < RVIN_CSI_MAX; i++) {
> +	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
>  		if (vin->group->remotes[i].asc != asc)
>  			continue;
>  		vin->group->remotes[i].subdev = subdev;

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v5 01/12] media: rcar-vin: Use correct count of remote subdevices

[Sakari Ailus]

On Mon, Jun 09, 2025 at 09:39:40AM +0000, Sakari Ailus wrote:
> > * Changes since v4
> > - Use ARRAY_SIZE() instead of updating the incorrect define to
> >   RVIN_REMOTES_MAX.
> 
> Do you still need RVIN_REMOTES_MAX()? Couldn't you use ARRAY_SIZE()
> elsewhere, too?

I checked rcar-vin.h, please ignore the above comment.

-- 
Sakari Ailus

Re: [PATCH v5 02/12] media: rcar-vin: Store platform info with group structure

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:25:56PM +0200, Niklas Söderlund wrote:
> When the transition of Gen2 to use groups are complete the platform
> specific information can be retrieved from the group instead of being
> duplicated in each VIN's private data structure.
> 
> Prepare for this by already adding the information to the group
> structure so it can be used without first having to find the group from
> a VIN instances private data.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v4
> - New in v5.
> ---
>  drivers/media/platform/renesas/rcar-vin/rcar-core.c | 1 +
>  drivers/media/platform/renesas/rcar-vin/rcar-vin.h  | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index cf5830d7d7b1..66efe075adae 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -156,6 +156,7 @@ static int rvin_group_get(struct rvin_dev *vin,
>  		}
>  
>  		kref_init(&group->refcount);
> +		group->info = vin->info;
>  
>  		rvin_group_data = group;
>  	}
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> index 83d1b2734c41..313703cd1103 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> @@ -242,6 +242,7 @@ struct rvin_dev {
>   * @lock:		protects the count, notifier, vin and csi members
>   * @count:		number of enabled VIN instances found in DT
>   * @notifier:		group notifier for CSI-2 async connections
> + * @info:		Platform dependent information about the VIN instances
>   * @vin:		VIN instances which are part of the group
>   * @link_setup:		Callback to create all links for the media graph
>   * @remotes:		array of pairs of async connection and subdev pointers
> @@ -255,6 +256,7 @@ struct rvin_group {
>  	struct mutex lock;
>  	unsigned int count;
>  	struct v4l2_async_notifier notifier;
> +	const struct rvin_info *info;
>  	struct rvin_dev *vin[RCAR_VIN_NUM];
>  
>  	int (*link_setup)(struct rvin_dev *vin);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 06/12] media: rcar-vin: Improve error paths for parallel devices

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:26:00PM +0200, Niklas Söderlund wrote:
> Use the __free(fwnode_handle) hooks to free the endpoints when the
> function exits to simplify the error paths and make the intent more
> clear.
> 
> While at it correct the error message when failing to parse an endpoint
> to report the correct node.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v4
> - New in v5, improvement suggested by Sakari and Laurent in review of v4.
> ---
>  .../platform/renesas/rcar-vin/rcar-core.c     | 28 +++++++------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index df3f15bd95a4..100432080ad7 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -340,24 +340,20 @@ static void rvin_group_notifier_cleanup(struct rvin_dev *vin)
>  
>  static int rvin_parallel_parse_of(struct rvin_dev *vin)
>  {
> -	struct fwnode_handle *ep, *fwnode;
> +	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
> +	struct fwnode_handle *ep __free(fwnode_handle) = NULL;
>  	struct v4l2_fwnode_endpoint vep = {
>  		.bus_type = V4L2_MBUS_UNKNOWN,
>  	};
>  	struct v4l2_async_connection *asc;
> -	int ret;
>  
>  	ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(vin->dev), 0, 0, 0);
>  	if (!ep)
>  		return 0;
>  
> -	fwnode = fwnode_graph_get_remote_endpoint(ep);
> -	ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> -	fwnode_handle_put(ep);
> -	if (ret) {
> -		vin_err(vin, "Failed to parse %pOF\n", to_of_node(fwnode));
> -		ret = -EINVAL;
> -		goto out;
> +	if (v4l2_fwnode_endpoint_parse(ep, &vep)) {
> +		vin_err(vin, "Failed to parse %pOF\n", to_of_node(ep));
> +		return -EINVAL;
>  	}
>  
>  	switch (vep.bus_type) {
> @@ -371,24 +367,20 @@ static int rvin_parallel_parse_of(struct rvin_dev *vin)
>  		break;
>  	default:
>  		vin_err(vin, "Unknown media bus type\n");
> -		ret = -EINVAL;
> -		goto out;
> +		return -EINVAL;
>  	}
>  
> +	fwnode = fwnode_graph_get_remote_endpoint(ep);
>  	asc = v4l2_async_nf_add_fwnode(&vin->notifier, fwnode,
>  				       struct v4l2_async_connection);
> -	if (IS_ERR(asc)) {
> -		ret = PTR_ERR(asc);
> -		goto out;
> -	}
> +	if (IS_ERR(asc))
> +		return PTR_ERR(asc);
>  
>  	vin->parallel.asc = asc;
>  
>  	vin_dbg(vin, "Add parallel OF device %pOF\n", to_of_node(fwnode));
> -out:
> -	fwnode_handle_put(fwnode);
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 08/12] media: rcar-vin: Always create a media pad

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:26:02PM +0200, Niklas Söderlund wrote:
> Prepare for Gen2 media graph support by always initializing a media pad
> for the VIN device.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v4
> - New in v5, broken out from a larger patch in v4.
> ---
>  drivers/media/platform/renesas/rcar-vin/rcar-core.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index c5510e3b258f..8cb3d0a3520f 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -895,10 +895,6 @@ static int rvin_csi2_init(struct rvin_dev *vin)
>  {
>  	int ret;
>  
> -	vin->pad.flags = MEDIA_PAD_FL_SINK;
> -	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
> -	if (ret)
> -		return ret;
>  
>  	ret = rvin_create_controls(vin, NULL);
>  	if (ret < 0)
> @@ -980,10 +976,6 @@ static int rvin_isp_init(struct rvin_dev *vin)
>  {
>  	int ret;
>  
> -	vin->pad.flags = MEDIA_PAD_FL_SINK;
> -	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
> -	if (ret)
> -		return ret;
>  
>  	ret = rvin_create_controls(vin, NULL);
>  	if (ret < 0)
> @@ -1375,6 +1367,11 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  	if (rvin_id_get(vin))
>  		return -EINVAL;
>  
> +	vin->pad.flags = MEDIA_PAD_FL_SINK;
> +	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
> +	if (ret)
> +		return ret;
> +
>  	if (vin->info->use_isp) {
>  		ret = rvin_isp_init(vin);
>  	} else if (vin->info->use_mc) {

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 12/12] media: rcar-vin: Fold event notifier into only user

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:26:06PM +0200, Niklas Söderlund wrote:
> With Gen2 converted to use the common media device there is only one
> caller left for the helper to notify a video device of an event, fold it
> in.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v4
> - Broken out from larger patch.
> ---
>  .../platform/renesas/rcar-vin/rcar-v4l2.c     | 20 +++++++------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> index 2bf94bd77c24..59b01cb0628a 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> @@ -656,18 +656,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
>  	video_unregister_device(&vin->vdev);
>  }
>  
> -static void rvin_notify_video_device(struct rvin_dev *vin,
> -				     unsigned int notification, void *arg)
> -{
> -	switch (notification) {
> -	case V4L2_DEVICE_NOTIFY_EVENT:
> -		v4l2_event_queue(&vin->vdev, arg);
> -		break;
> -	default:
> -		break;
> -	}
> -}
> -
>  static void rvin_notify(struct v4l2_subdev *sd,
>  			unsigned int notification, void *arg)
>  {
> @@ -693,7 +681,13 @@ static void rvin_notify(struct v4l2_subdev *sd,
>  		if (remote != sd)
>  			continue;
>  
> -		rvin_notify_video_device(vin, notification, arg);
> +		switch (notification) {
> +		case V4L2_DEVICE_NOTIFY_EVENT:
> +			v4l2_event_queue(&vin->vdev, arg);
> +			break;
> +		default:
> +			break;
> +		}

How about

		if (notification == V4L2_DEVICE_NOTIFY_EVENT)
			v4l2_event_queue(&vin->vdev, arg);

Unless you expect more notifications to be handled later ?

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

>  	}
>  }
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 03/12] media: rcar-vin: Change link setup argument

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:25:57PM +0200, Niklas Söderlund wrote:
> The link setup callback once acted on each VIN instance, and expected to
> be called once for each VIN instance. This have changed as the driver
> grew support for later hardware generations and the callback is now
> expected to setup links for all VIN in the group.
> 
> The argument to the callback have however remained a pointer to a single

s/have/has/

> VIN instance. This pointer was then used to get the group structure. Fix
> this and pass the group as the single argument to the link setup
> callback making the expectation of the function clear.
> 
> There is no intentional change in behavior.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v4
> - Use the group->info structure added in previous patch instead of
>   iterating over all VIN to find one that is instantiated to get the
>   same information.
> - Condense variable declaration in rvin_isp_setup_links().
> ---
>  .../platform/renesas/rcar-vin/rcar-core.c     | 37 ++++++++++---------
>  .../platform/renesas/rcar-vin/rcar-vin.h      |  2 +-
>  2 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index 66efe075adae..73d713868391 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -65,7 +65,7 @@ static void rvin_group_cleanup(struct rvin_group *group)
>  }
>  
>  static int rvin_group_init(struct rvin_group *group, struct rvin_dev *vin,
> -			   int (*link_setup)(struct rvin_dev *),
> +			   int (*link_setup)(struct rvin_group *),
>  			   const struct media_device_ops *ops)
>  {
>  	struct media_device *mdev = &group->mdev;
> @@ -115,7 +115,7 @@ static void rvin_group_release(struct kref *kref)
>  }
>  
>  static int rvin_group_get(struct rvin_dev *vin,
> -			  int (*link_setup)(struct rvin_dev *),
> +			  int (*link_setup)(struct rvin_group *),
>  			  const struct media_device_ops *ops)
>  {
>  	struct rvin_group *group;
> @@ -247,7 +247,7 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
>  		}
>  	}
>  
> -	return vin->group->link_setup(vin);
> +	return vin->group->link_setup(vin->group);
>  }
>  
>  static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
> @@ -910,35 +910,35 @@ static int rvin_csi2_create_link(struct rvin_group *group, unsigned int id,
>  	return 0;
>  }
>  
> -static int rvin_csi2_setup_links(struct rvin_dev *vin)
> +static int rvin_csi2_setup_links(struct rvin_group *group)
>  {
>  	const struct rvin_group_route *route;
>  	unsigned int id;
>  	int ret = -EINVAL;
>  
>  	/* Create all media device links between VINs and CSI-2's. */
> -	mutex_lock(&vin->group->lock);
> -	for (route = vin->info->routes; route->chsel; route++) {
> +	mutex_lock(&group->lock);
> +	for (route = group->info->routes; route->chsel; route++) {
>  		/* Check that VIN' master is part of the group. */
> -		if (!vin->group->vin[route->master])
> +		if (!group->vin[route->master])
>  			continue;
>  
>  		/* Check that CSI-2 is part of the group. */
> -		if (!vin->group->remotes[route->csi].subdev)
> +		if (!group->remotes[route->csi].subdev)
>  			continue;
>  
>  		for (id = route->master; id < route->master + 4; id++) {
>  			/* Check that VIN is part of the group. */
> -			if (!vin->group->vin[id])
> +			if (!group->vin[id])
>  				continue;
>  
> -			ret = rvin_csi2_create_link(vin->group, id, route);
> +			ret = rvin_csi2_create_link(group, id, route);
>  			if (ret)
>  				goto out;
>  		}
>  	}
>  out:
> -	mutex_unlock(&vin->group->lock);
> +	mutex_unlock(&group->lock);
>  
>  	return ret;
>  }
> @@ -992,30 +992,31 @@ static int rvin_csi2_init(struct rvin_dev *vin)
>   * ISP
>   */
>  
> -static int rvin_isp_setup_links(struct rvin_dev *vin)
> +static int rvin_isp_setup_links(struct rvin_group *group)
>  {
>  	unsigned int i;
>  	int ret = -EINVAL;
>  
>  	/* Create all media device links between VINs and ISP's. */
> -	mutex_lock(&vin->group->lock);
> +	mutex_lock(&group->lock);
>  	for (i = 0; i < RCAR_VIN_NUM; i++) {
>  		struct media_pad *source_pad, *sink_pad;
>  		struct media_entity *source, *sink;
>  		unsigned int source_slot = i / 8;
>  		unsigned int source_idx = i % 8 + 1;
> +		struct rvin_dev *vin = group->vin[i];
>  
> -		if (!vin->group->vin[i])
> +		if (!vin)
>  			continue;
>  
>  		/* Check that ISP is part of the group. */
> -		if (!vin->group->remotes[source_slot].subdev)
> +		if (!group->remotes[source_slot].subdev)
>  			continue;
>  
> -		source = &vin->group->remotes[source_slot].subdev->entity;
> +		source = &group->remotes[source_slot].subdev->entity;
>  		source_pad = &source->pads[source_idx];
>  
> -		sink = &vin->group->vin[i]->vdev.entity;
> +		sink = &vin->vdev.entity;
>  		sink_pad = &sink->pads[0];
>  
>  		/* Skip if link already exists. */
> @@ -1031,7 +1032,7 @@ static int rvin_isp_setup_links(struct rvin_dev *vin)
>  			break;
>  		}
>  	}
> -	mutex_unlock(&vin->group->lock);
> +	mutex_unlock(&group->lock);
>  
>  	return ret;
>  }
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> index 313703cd1103..cb8e8fa54f96 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> @@ -259,7 +259,7 @@ struct rvin_group {
>  	const struct rvin_info *info;
>  	struct rvin_dev *vin[RCAR_VIN_NUM];
>  
> -	int (*link_setup)(struct rvin_dev *vin);
> +	int (*link_setup)(struct rvin_group *group);
>  
>  	struct {
>  		struct v4l2_async_connection *asc;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 12/12] media: rcar-vin: Fold event notifier into only user

[Niklas Söderlund]

Hi Laurent,

Thanks for your review.

On 2025-06-12 03:28:16 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Fri, Jun 06, 2025 at 08:26:06PM +0200, Niklas Söderlund wrote:
> > With Gen2 converted to use the common media device there is only one
> > caller left for the helper to notify a video device of an event, fold it
> > in.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > * Changes since v4
> > - Broken out from larger patch.
> > ---
> >  .../platform/renesas/rcar-vin/rcar-v4l2.c     | 20 +++++++------------
> >  1 file changed, 7 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > index 2bf94bd77c24..59b01cb0628a 100644
> > --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > @@ -656,18 +656,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
> >  	video_unregister_device(&vin->vdev);
> >  }
> >  
> > -static void rvin_notify_video_device(struct rvin_dev *vin,
> > -				     unsigned int notification, void *arg)
> > -{
> > -	switch (notification) {
> > -	case V4L2_DEVICE_NOTIFY_EVENT:
> > -		v4l2_event_queue(&vin->vdev, arg);
> > -		break;
> > -	default:
> > -		break;
> > -	}
> > -}
> > -
> >  static void rvin_notify(struct v4l2_subdev *sd,
> >  			unsigned int notification, void *arg)
> >  {
> > @@ -693,7 +681,13 @@ static void rvin_notify(struct v4l2_subdev *sd,
> >  		if (remote != sd)
> >  			continue;
> >  
> > -		rvin_notify_video_device(vin, notification, arg);
> > +		switch (notification) {
> > +		case V4L2_DEVICE_NOTIFY_EVENT:
> > +			v4l2_event_queue(&vin->vdev, arg);
> > +			break;
> > +		default:
> > +			break;
> > +		}
> 
> How about
> 
> 		if (notification == V4L2_DEVICE_NOTIFY_EVENT)
> 			v4l2_event_queue(&vin->vdev, arg);
> 
> Unless you expect more notifications to be handled later ?

I do, I have a different series that tries to notify CSI-2 errors to 
user-space with events. The series was posted in 2021 and I need to get 
back to it at some point ;-)

As this just moves the existing structure around I would prefere to keep 
it as-is for now.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> >  	}
> >  }
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v5 12/12] media: rcar-vin: Fold event notifier into only user

[Laurent Pinchart]

On Thu, Jun 12, 2025 at 09:22:40AM +0200, Niklas Söderlund wrote:
> On 2025-06-12 03:28:16 +0300, Laurent Pinchart wrote:
> > On Fri, Jun 06, 2025 at 08:26:06PM +0200, Niklas Söderlund wrote:
> > > With Gen2 converted to use the common media device there is only one
> > > caller left for the helper to notify a video device of an event, fold it
> > > in.
> > > 
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > ---
> > > * Changes since v4
> > > - Broken out from larger patch.
> > > ---
> > >  .../platform/renesas/rcar-vin/rcar-v4l2.c     | 20 +++++++------------
> > >  1 file changed, 7 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > > index 2bf94bd77c24..59b01cb0628a 100644
> > > --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > > @@ -656,18 +656,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
> > >  	video_unregister_device(&vin->vdev);
> > >  }
> > >  
> > > -static void rvin_notify_video_device(struct rvin_dev *vin,
> > > -				     unsigned int notification, void *arg)
> > > -{
> > > -	switch (notification) {
> > > -	case V4L2_DEVICE_NOTIFY_EVENT:
> > > -		v4l2_event_queue(&vin->vdev, arg);
> > > -		break;
> > > -	default:
> > > -		break;
> > > -	}
> > > -}
> > > -
> > >  static void rvin_notify(struct v4l2_subdev *sd,
> > >  			unsigned int notification, void *arg)
> > >  {
> > > @@ -693,7 +681,13 @@ static void rvin_notify(struct v4l2_subdev *sd,
> > >  		if (remote != sd)
> > >  			continue;
> > >  
> > > -		rvin_notify_video_device(vin, notification, arg);
> > > +		switch (notification) {
> > > +		case V4L2_DEVICE_NOTIFY_EVENT:
> > > +			v4l2_event_queue(&vin->vdev, arg);
> > > +			break;
> > > +		default:
> > > +			break;
> > > +		}
> > 
> > How about
> > 
> > 		if (notification == V4L2_DEVICE_NOTIFY_EVENT)
> > 			v4l2_event_queue(&vin->vdev, arg);
> > 
> > Unless you expect more notifications to be handled later ?
> 
> I do, I have a different series that tries to notify CSI-2 errors to 
> user-space with events. The series was posted in 2021 and I need to get 
> back to it at some point ;-)

Could the event be sent on the subdev instead of the video device ?

> As this just moves the existing structure around I would prefere to keep 
> it as-is for now.

OK.

> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > 
> > >  	}
> > >  }
> > >  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 12/12] media: rcar-vin: Fold event notifier into only user

[Niklas Söderlund]

On 2025-06-12 12:44:08 +0300, Laurent Pinchart wrote:
> On Thu, Jun 12, 2025 at 09:22:40AM +0200, Niklas Söderlund wrote:
> > On 2025-06-12 03:28:16 +0300, Laurent Pinchart wrote:
> > > On Fri, Jun 06, 2025 at 08:26:06PM +0200, Niklas Söderlund wrote:
> > > > With Gen2 converted to use the common media device there is only one
> > > > caller left for the helper to notify a video device of an event, fold it
> > > > in.
> > > > 
> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > > ---
> > > > * Changes since v4
> > > > - Broken out from larger patch.
> > > > ---
> > > >  .../platform/renesas/rcar-vin/rcar-v4l2.c     | 20 +++++++------------
> > > >  1 file changed, 7 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > > > index 2bf94bd77c24..59b01cb0628a 100644
> > > > --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > > > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> > > > @@ -656,18 +656,6 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
> > > >  	video_unregister_device(&vin->vdev);
> > > >  }
> > > >  
> > > > -static void rvin_notify_video_device(struct rvin_dev *vin,
> > > > -				     unsigned int notification, void *arg)
> > > > -{
> > > > -	switch (notification) {
> > > > -	case V4L2_DEVICE_NOTIFY_EVENT:
> > > > -		v4l2_event_queue(&vin->vdev, arg);
> > > > -		break;
> > > > -	default:
> > > > -		break;
> > > > -	}
> > > > -}
> > > > -
> > > >  static void rvin_notify(struct v4l2_subdev *sd,
> > > >  			unsigned int notification, void *arg)
> > > >  {
> > > > @@ -693,7 +681,13 @@ static void rvin_notify(struct v4l2_subdev *sd,
> > > >  		if (remote != sd)
> > > >  			continue;
> > > >  
> > > > -		rvin_notify_video_device(vin, notification, arg);
> > > > +		switch (notification) {
> > > > +		case V4L2_DEVICE_NOTIFY_EVENT:
> > > > +			v4l2_event_queue(&vin->vdev, arg);
> > > > +			break;
> > > > +		default:
> > > > +			break;
> > > > +		}
> > > 
> > > How about
> > > 
> > > 		if (notification == V4L2_DEVICE_NOTIFY_EVENT)
> > > 			v4l2_event_queue(&vin->vdev, arg);
> > > 
> > > Unless you expect more notifications to be handled later ?
> > 
> > I do, I have a different series that tries to notify CSI-2 errors to 
> > user-space with events. The series was posted in 2021 and I need to get 
> > back to it at some point ;-)
> 
> Could the event be sent on the subdev instead of the video device ?

That is a good point, the event should be sent to user-space thru the 
subdev. The current work I have (that might change) however acts on the 
event to stop the capture process on error, but indeed maybe the call to 
stop capturing should also come from user-space as a reaction to the 
event.

I have captured these thoughts in my TODO file, thanks for the 
suggestion.

> 
> > As this just moves the existing structure around I would prefere to keep 
> > it as-is for now.
> 
> OK.
> 
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > 
> > > >  	}
> > > >  }
> > > >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v5 07/12] media: rcar-vin: Merge all notifiers

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:26:01PM +0200, Niklas Söderlund wrote:
> The VIN usage of v4l-async is complex and stems from organic growth of
> the driver of supporting both private local subdevices (Gen2, Gen3) and
> subdevices shared between all VIN instances (Gen3 and Gen4).
> 
> The driver used a separate notifier for each VIN for the private local
> ones, and a shared group notifier for the shared ones. This was complex
> and lead to subtle bugs when unbinding and later rebinding subdevices in
> one of the notifiers having to handle different edge cases depending on
> if it also had subdevices in the other notifiers etc.
> 
> To simplify this have the Gen2 devices allocate and form a VIN group
> too. This way all subdevices on all models can be collect in a
> single group notifier. Then there is only a single complete callback for
> all where the video devices and subdevice nodes can be registered etc.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> ---
> * Changes since v4
> - Fix spelling in commit message and comments.
> - Use mutex guard to simplify error path.
> - Fix code style, add braces and blank lines.
> ---
>  .../platform/renesas/rcar-vin/rcar-core.c     | 256 ++++++++----------
>  .../platform/renesas/rcar-vin/rcar-vin.h      |   2 -
>  2 files changed, 106 insertions(+), 152 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index 100432080ad7..c5510e3b258f 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -43,6 +43,9 @@
>  
>  #define v4l2_dev_to_vin(d)	container_of(d, struct rvin_dev, v4l2_dev)
>  
> +static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
> +					  struct v4l2_subdev *subdev);
> +
>  /* -----------------------------------------------------------------------------
>   * Gen3 Group Allocator
>   */
> @@ -233,7 +236,10 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
>  		}
>  	}
>  
> -	return vin->group->link_setup(vin->group);
> +	if (vin->group->link_setup)
> +		return vin->group->link_setup(vin->group);
> +
> +	return  0;
>  }
>  
>  static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
> @@ -241,20 +247,32 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  				     struct v4l2_async_connection *asc)
>  {
>  	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
> -	unsigned int i;
> +	struct rvin_group *group = vin->group;
>  
> -	for (i = 0; i < RCAR_VIN_NUM; i++)
> -		if (vin->group->vin[i])
> -			rvin_v4l2_unregister(vin->group->vin[i]);
> +	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
> +		if (group->vin[i])
> +			rvin_v4l2_unregister(group->vin[i]);
> +	}
>  
>  	mutex_lock(&vin->group->lock);
>  
> -	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
> -		if (vin->group->remotes[i].asc != asc)
> +	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
> +		if (!group->vin[i] || group->vin[i]->parallel.asc != asc)
>  			continue;
> -		vin->group->remotes[i].subdev = NULL;
> +
> +		group->vin[i]->parallel.subdev = NULL;
> +
> +		vin_dbg(group->vin[i], "Unbind parallel subdev %s\n",
> +			subdev->name);
> +	}
> +
> +	for (unsigned int i = 0; i < ARRAY_SIZE(group->remotes); i++) {
> +		if (group->remotes[i].asc != asc)
> +			continue;
> +
> +		group->remotes[i].subdev = NULL;
> +
>  		vin_dbg(vin, "Unbind %s from slot %u\n", subdev->name, i);
> -		break;
>  	}
>  
>  	mutex_unlock(&vin->group->lock);
> @@ -267,21 +285,38 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
>  				   struct v4l2_async_connection *asc)
>  {
>  	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
> -	unsigned int i;
> +	struct rvin_group *group = vin->group;
>  
> -	mutex_lock(&vin->group->lock);
> +	guard(mutex)(&group->lock);
>  
> -	for (i = 0; i < ARRAY_SIZE(vin->group->remotes); i++) {
> +	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
> +		int ret;
> +
> +		if (!group->vin[i] || group->vin[i]->parallel.asc != asc)
> +			continue;
> +
> +		ret = rvin_parallel_subdevice_attach(group->vin[i], subdev);
> +		if (ret)
> +			return ret;
> +
> +		v4l2_set_subdev_hostdata(subdev, group->vin[i]);
> +
> +		vin_dbg(group->vin[i], "Bound subdev %s\n", subdev->name);
> +
> +		return 0;
> +	}
> +
> +	for (unsigned int i = 0; i < ARRAY_SIZE(group->remotes); i++) {
>  		if (vin->group->remotes[i].asc != asc)
>  			continue;
> +
>  		vin->group->remotes[i].subdev = subdev;
>  		vin_dbg(vin, "Bound %s to slot %u\n", subdev->name, i);
> -		break;
> +
> +		return 0;
>  	}
>  
> -	mutex_unlock(&vin->group->lock);
> -
> -	return 0;
> +	return -ENODEV;
>  }
>  
>  static const struct v4l2_async_notifier_operations rvin_group_notify_ops = {
> @@ -371,7 +406,7 @@ static int rvin_parallel_parse_of(struct rvin_dev *vin)
>  	}
>  
>  	fwnode = fwnode_graph_get_remote_endpoint(ep);
> -	asc = v4l2_async_nf_add_fwnode(&vin->notifier, fwnode,
> +	asc = v4l2_async_nf_add_fwnode(&vin->group->notifier, fwnode,
>  				       struct v4l2_async_connection);
>  	if (IS_ERR(asc))
>  		return PTR_ERR(asc);
> @@ -417,6 +452,12 @@ static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
>  		if (!(vin_mask & BIT(i)))
>  			continue;
>  
> +		/* Parse local subdevice. */
> +		ret = rvin_parallel_parse_of(vin->group->vin[i]);
> +		if (ret)
> +			return ret;
> +
> +		/* Prase shared subdevices. */

s/Prase/Parse/

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

>  		for (id = 0; id < max_id; id++) {
>  			if (vin->group->remotes[id].asc)
>  				continue;
> @@ -596,124 +637,6 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  	return 0;
>  }
>  
> -static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
> -{
> -	rvin_v4l2_unregister(vin);
> -	vin->parallel.subdev = NULL;
> -
> -	if (!vin->info->use_mc)
> -		rvin_free_controls(vin);
> -}
> -
> -static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
> -{
> -	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
> -	struct media_entity *source;
> -	struct media_entity *sink;
> -	int ret;
> -
> -	ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
> -	if (ret < 0) {
> -		vin_err(vin, "Failed to register subdev nodes\n");
> -		return ret;
> -	}
> -
> -	if (!video_is_registered(&vin->vdev)) {
> -		ret = rvin_v4l2_register(vin);
> -		if (ret < 0)
> -			return ret;
> -	}
> -
> -	if (!vin->info->use_mc)
> -		return 0;
> -
> -	/* If we're running with media-controller, link the subdevs. */
> -	source = &vin->parallel.subdev->entity;
> -	sink = &vin->vdev.entity;
> -
> -	ret = media_create_pad_link(source, vin->parallel.source_pad,
> -				    sink, vin->parallel.sink_pad, 0);
> -	if (ret)
> -		vin_err(vin, "Error adding link from %s to %s: %d\n",
> -			source->name, sink->name, ret);
> -
> -	return ret;
> -}
> -
> -static void rvin_parallel_notify_unbind(struct v4l2_async_notifier *notifier,
> -					struct v4l2_subdev *subdev,
> -					struct v4l2_async_connection *asc)
> -{
> -	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
> -
> -	vin_dbg(vin, "unbind parallel subdev %s\n", subdev->name);
> -
> -	mutex_lock(&vin->lock);
> -	rvin_parallel_subdevice_detach(vin);
> -	mutex_unlock(&vin->lock);
> -}
> -
> -static int rvin_parallel_notify_bound(struct v4l2_async_notifier *notifier,
> -				      struct v4l2_subdev *subdev,
> -				      struct v4l2_async_connection *asc)
> -{
> -	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
> -	int ret;
> -
> -	mutex_lock(&vin->lock);
> -	ret = rvin_parallel_subdevice_attach(vin, subdev);
> -	mutex_unlock(&vin->lock);
> -	if (ret)
> -		return ret;
> -
> -	v4l2_set_subdev_hostdata(subdev, vin);
> -
> -	vin_dbg(vin, "bound subdev %s source pad: %u sink pad: %u\n",
> -		subdev->name, vin->parallel.source_pad,
> -		vin->parallel.sink_pad);
> -
> -	return 0;
> -}
> -
> -static const struct v4l2_async_notifier_operations rvin_parallel_notify_ops = {
> -	.bound = rvin_parallel_notify_bound,
> -	.unbind = rvin_parallel_notify_unbind,
> -	.complete = rvin_parallel_notify_complete,
> -};
> -
> -static void rvin_parallel_cleanup(struct rvin_dev *vin)
> -{
> -	v4l2_async_nf_unregister(&vin->notifier);
> -	v4l2_async_nf_cleanup(&vin->notifier);
> -}
> -
> -static int rvin_parallel_init(struct rvin_dev *vin)
> -{
> -	int ret;
> -
> -	v4l2_async_nf_init(&vin->notifier, &vin->v4l2_dev);
> -
> -	ret = rvin_parallel_parse_of(vin);
> -	if (ret)
> -		return ret;
> -
> -	if (!vin->parallel.asc)
> -		return -ENODEV;
> -
> -	vin_dbg(vin, "Found parallel subdevice %pOF\n",
> -		to_of_node(vin->parallel.asc->match.fwnode));
> -
> -	vin->notifier.ops = &rvin_parallel_notify_ops;
> -	ret = v4l2_async_nf_register(&vin->notifier);
> -	if (ret < 0) {
> -		vin_err(vin, "Notifier registration failed\n");
> -		v4l2_async_nf_cleanup(&vin->notifier);
> -		return ret;
> -	}
> -
> -	return 0;
> -}
> -
>  /* -----------------------------------------------------------------------------
>   * CSI-2
>   */
> @@ -888,11 +811,52 @@ static int rvin_csi2_create_link(struct rvin_group *group, unsigned int id,
>  	return 0;
>  }
>  
> +static int rvin_parallel_setup_links(struct rvin_group *group)
> +{
> +	u32 flags = MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE;
> +
> +	guard(mutex)(&group->lock);
> +
> +	/* If the group also has links don't enable the link. */
> +	for (unsigned int i = 0; i < ARRAY_SIZE(group->remotes); i++) {
> +		if (group->remotes[i].subdev) {
> +			flags = 0;
> +			break;
> +		}
> +	}
> +
> +	/* Create links. */
> +	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
> +		struct rvin_dev *vin = group->vin[i];
> +		struct media_entity *source;
> +		struct media_entity *sink;
> +		int ret;
> +
> +		/* Nothing to do if there is no VIN or parallel subdev. */
> +		if (!vin || !vin->parallel.subdev)
> +			continue;
> +
> +		source = &vin->parallel.subdev->entity;
> +		sink = &vin->vdev.entity;
> +
> +		ret = media_create_pad_link(source, vin->parallel.source_pad,
> +					    sink, 0, flags);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int rvin_csi2_setup_links(struct rvin_group *group)
>  {
>  	const struct rvin_group_route *route;
>  	unsigned int id;
> -	int ret = -EINVAL;
> +	int ret;
> +
> +	ret = rvin_parallel_setup_links(group);
> +	if (ret)
> +		return ret;
>  
>  	/* Create all media device links between VINs and CSI-2's. */
>  	mutex_lock(&group->lock);
> @@ -923,9 +887,7 @@ static int rvin_csi2_setup_links(struct rvin_group *group)
>  
>  static void rvin_csi2_cleanup(struct rvin_dev *vin)
>  {
> -	rvin_parallel_cleanup(vin);
>  	rvin_group_notifier_cleanup(vin);
> -	rvin_group_put(vin);
>  	rvin_free_controls(vin);
>  }
>  
> @@ -946,18 +908,11 @@ static int rvin_csi2_init(struct rvin_dev *vin)
>  	if (ret)
>  		goto err_controls;
>  
> -	/* It's OK to not have a parallel subdevice. */
> -	ret = rvin_parallel_init(vin);
> -	if (ret && ret != -ENODEV)
> -		goto err_group;
> -
>  	ret = rvin_group_notifier_init(vin, 1, RVIN_CSI_MAX);
>  	if (ret)
> -		goto err_parallel;
> +		goto err_group;
>  
>  	return 0;
> -err_parallel:
> -	rvin_parallel_cleanup(vin);
>  err_group:
>  	rvin_group_put(vin);
>  err_controls:
> @@ -1018,7 +973,6 @@ static int rvin_isp_setup_links(struct rvin_group *group)
>  static void rvin_isp_cleanup(struct rvin_dev *vin)
>  {
>  	rvin_group_notifier_cleanup(vin);
> -	rvin_group_put(vin);
>  	rvin_free_controls(vin);
>  }
>  
> @@ -1430,7 +1384,9 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  		    rvin_group_id_to_master(vin->id) == vin->id)
>  			vin->scaler = vin->info->scaler;
>  	} else {
> -		ret = rvin_parallel_init(vin);
> +		ret = rvin_group_get(vin, NULL, NULL);
> +		if (!ret)
> +			ret = rvin_group_notifier_init(vin, 0, 0);
>  
>  		if (vin->info->scaler)
>  			vin->scaler = vin->info->scaler;
> @@ -1460,8 +1416,8 @@ static void rcar_vin_remove(struct platform_device *pdev)
>  		rvin_isp_cleanup(vin);
>  	else if (vin->info->use_mc)
>  		rvin_csi2_cleanup(vin);
> -	else
> -		rvin_parallel_cleanup(vin);
> +
> +	rvin_group_put(vin);
>  
>  	rvin_id_put(vin);
>  
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> index cb8e8fa54f96..38ae2bd20b72 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> @@ -149,7 +149,6 @@ struct rvin_info {
>   * @vdev:		V4L2 video device associated with VIN
>   * @v4l2_dev:		V4L2 device
>   * @ctrl_handler:	V4L2 control handler
> - * @notifier:		V4L2 asynchronous subdevs notifier
>   *
>   * @parallel:		parallel input subdevice descriptor
>   *
> @@ -189,7 +188,6 @@ struct rvin_dev {
>  	struct video_device vdev;
>  	struct v4l2_device v4l2_dev;
>  	struct v4l2_ctrl_handler ctrl_handler;
> -	struct v4l2_async_notifier notifier;
>  
>  	struct rvin_parallel_entity parallel;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 09/12] media: rcar-vin: Remove NTSC workaround

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:26:03PM +0200, Niklas Söderlund wrote:
> On Gen2 where sub-devices where not exposed to user-space the filed

s/filed/field/

> TB/BT ordering was controlled by a hack in the VIN driver. Before
> converting it to media device model where the subdevice is exposed
> remove that hack.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

> ---
> * Changes since v4
> - Broken out from larger patch.
> ---
>  drivers/media/platform/renesas/rcar-vin/rcar-dma.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> index 5c08ee2c9807..4fb33359bb0f 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> @@ -700,9 +700,6 @@ static int rvin_setup(struct rvin_dev *vin)
>  	case V4L2_FIELD_INTERLACED:
>  		/* Default to TB */
>  		vnmc = VNMC_IM_FULL;
> -		/* Use BT if video standard can be read and is 60 Hz format */
> -		if (!vin->info->use_mc && vin->std & V4L2_STD_525_60)
> -			vnmc = VNMC_IM_FULL | VNMC_FOC;
>  		break;
>  	case V4L2_FIELD_INTERLACED_TB:
>  		vnmc = VNMC_IM_FULL;

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 10/12] media: rcar-vin: Only expose VIN controls

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:26:04PM +0200, Niklas Söderlund wrote:
> Before moving Gen2 to media controller simplify the creation of controls
> by not exposing the sub-device controls on the video device. This could
> be done while enabling media controller but doing it separately reduces
> the changes needed to do so.
> 
> The rework also allows the cleanup and remove paths to be simplified by
> folding all special cases into the only remaining call site.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v4
> - Broken out from a larger patch.
> ---
>  .../platform/renesas/rcar-vin/rcar-core.c     | 84 ++++---------------
>  1 file changed, 18 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index 8cb3d0a3520f..597e868a6bc5 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -365,14 +365,6 @@ static int rvin_group_parse_of(struct rvin_dev *vin, unsigned int port,
>  	return ret;
>  }
>  
> -static void rvin_group_notifier_cleanup(struct rvin_dev *vin)
> -{
> -	if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> -		v4l2_async_nf_unregister(&vin->group->notifier);
> -		v4l2_async_nf_cleanup(&vin->group->notifier);
> -	}
> -}
> -
>  static int rvin_parallel_parse_of(struct rvin_dev *vin)
>  {
>  	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
> @@ -510,7 +502,7 @@ static void rvin_free_controls(struct rvin_dev *vin)
>  	vin->vdev.ctrl_handler = NULL;
>  }
>  
> -static int rvin_create_controls(struct rvin_dev *vin, struct v4l2_subdev *subdev)
> +static int rvin_create_controls(struct rvin_dev *vin)
>  {
>  	int ret;
>  
> @@ -528,16 +520,6 @@ static int rvin_create_controls(struct rvin_dev *vin, struct v4l2_subdev *subdev
>  		return ret;
>  	}
>  

Now that the control handler for the video device only has a single
control, you can reduce the number of buckets:

-	ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 16);
+	ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 1);

> -	/* For the non-MC mode add controls from the subdevice. */
> -	if (subdev) {
> -		ret = v4l2_ctrl_add_handler(&vin->ctrl_handler,
> -					    subdev->ctrl_handler, NULL, true);
> -		if (ret < 0) {
> -			rvin_free_controls(vin);
> -			return ret;
> -		}
> -	}
> -
>  	vin->vdev.ctrl_handler = &vin->ctrl_handler;
>  
>  	return 0;
> @@ -627,11 +609,6 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  	if (ret < 0 && ret != -ENOIOCTLCMD)
>  		return ret;
>  
> -	/* Add the controls */
> -	ret = rvin_create_controls(vin, subdev);
> -	if (ret < 0)
> -		return ret;
> -
>  	vin->parallel.subdev = subdev;
>  
>  	return 0;
> @@ -885,34 +862,17 @@ static int rvin_csi2_setup_links(struct rvin_group *group)
>  	return ret;
>  }
>  
> -static void rvin_csi2_cleanup(struct rvin_dev *vin)
> -{
> -	rvin_group_notifier_cleanup(vin);
> -	rvin_free_controls(vin);
> -}
> -
>  static int rvin_csi2_init(struct rvin_dev *vin)
>  {
>  	int ret;
>  
> -
> -	ret = rvin_create_controls(vin, NULL);
> -	if (ret < 0)
> -		return ret;
> -
>  	ret = rvin_group_get(vin, rvin_csi2_setup_links, &rvin_csi2_media_ops);
>  	if (ret)
> -		goto err_controls;
> +		return ret;
>  
>  	ret = rvin_group_notifier_init(vin, 1, RVIN_CSI_MAX);
>  	if (ret)
> -		goto err_group;
> -
> -	return 0;
> -err_group:
> -	rvin_group_put(vin);
> -err_controls:
> -	rvin_free_controls(vin);
> +		rvin_group_put(vin);
>  
>  	return ret;
>  }
> @@ -966,34 +926,17 @@ static int rvin_isp_setup_links(struct rvin_group *group)
>  	return ret;
>  }
>  
> -static void rvin_isp_cleanup(struct rvin_dev *vin)
> -{
> -	rvin_group_notifier_cleanup(vin);
> -	rvin_free_controls(vin);
> -}
> -
>  static int rvin_isp_init(struct rvin_dev *vin)
>  {
>  	int ret;
>  
> -
> -	ret = rvin_create_controls(vin, NULL);
> -	if (ret < 0)
> -		return ret;
> -
>  	ret = rvin_group_get(vin, rvin_isp_setup_links, NULL);
>  	if (ret)
> -		goto err_controls;
> +		return ret;
>  
>  	ret = rvin_group_notifier_init(vin, 2, RVIN_ISP_MAX);
>  	if (ret)
> -		goto err_group;
> -
> -	return 0;
> -err_group:
> -	rvin_group_put(vin);
> -err_controls:
> -	rvin_free_controls(vin);
> +		rvin_group_put(vin);
>  
>  	return ret;
>  }
> @@ -1372,6 +1315,10 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	ret = rvin_create_controls(vin);
> +	if (ret < 0)
> +		return ret;

Don't you need to call rvin_dma_unregister() here ?

> +
>  	if (vin->info->use_isp) {
>  		ret = rvin_isp_init(vin);
>  	} else if (vin->info->use_mc) {
> @@ -1390,6 +1337,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  	}
>  
>  	if (ret) {
> +		rvin_free_controls(vin);
>  		rvin_dma_unregister(vin);
>  		rvin_id_put(vin);
>  		return ret;

Error labels at the end of the function will make this more manageable.

> @@ -1409,13 +1357,17 @@ static void rcar_vin_remove(struct platform_device *pdev)
>  
>  	rvin_v4l2_unregister(vin);
>  
> -	if (vin->info->use_isp)
> -		rvin_isp_cleanup(vin);
> -	else if (vin->info->use_mc)
> -		rvin_csi2_cleanup(vin);
> +	if (vin->info->use_isp || vin->info->use_mc) {
> +		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> +			v4l2_async_nf_unregister(&vin->group->notifier);
> +			v4l2_async_nf_cleanup(&vin->group->notifier);
> +		}
> +	}
>  
>  	rvin_group_put(vin);
>  
> +	rvin_free_controls(vin);
> +
>  	rvin_id_put(vin);
>  
>  	rvin_dma_unregister(vin);

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 11/12] media: rcar-vin: Enable media-graph on Gen2

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Jun 06, 2025 at 08:26:05PM +0200, Niklas Söderlund wrote:
> Complete the conversion from soc_camera to a full fledge media
> controller enabled devices for all supported generations of the device.
> All work is already done as this is already supported on Gen3, and
> later.
> 
> All that is missing is removing all special cases for the non
> media-graph call paths and use the common ones in their place.
> 
> The one change that stands out is dropping the doubling of the height in
> the Gen2 scaler setup, rvin_scaler_gen2(). In the Gen2 non-MC world the
> VIN size was set to match the video source subdevices, and if that was a
> TOP/BOTTOM video source it needed to be doubled for the scaler to
> function properly. In the MC world this is now handled by user-space
> configuration of the pipeline and the adjustment is not needed.
> 
> Mark the completion of converting from soc_camera by injecting an
> attribution of myself in the header.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> * Changes since v3
> - Break out small things to ease review.
> - Add rationale of odd looking change in rvin_scaler_gen2().
> 
> * Changes since v3
> - Resolve conflicts with other VIN work merged a head of this series.
> ---
>  .../platform/renesas/rcar-vin/rcar-core.c     | 158 +-----
>  .../platform/renesas/rcar-vin/rcar-dma.c      |  20 +-
>  .../platform/renesas/rcar-vin/rcar-v4l2.c     | 468 +-----------------
>  .../platform/renesas/rcar-vin/rcar-vin.h      |  10 +-
>  4 files changed, 41 insertions(+), 615 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> index 597e868a6bc5..6c653a8a8c27 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> @@ -2,12 +2,11 @@
>  /*
>   * Driver for Renesas R-Car VIN
>   *
> + * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
>   * Copyright (C) 2016 Renesas Electronics Corp.
>   * Copyright (C) 2011-2013 Renesas Solutions Corp.
>   * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
>   * Copyright (C) 2008 Magnus Damm
> - *
> - * Based on the soc-camera rcar_vin driver
>   */
>  
>  #include <linux/idr.h>
> @@ -43,9 +42,6 @@
>  
>  #define v4l2_dev_to_vin(d)	container_of(d, struct rvin_dev, v4l2_dev)
>  
> -static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
> -					  struct v4l2_subdev *subdev);
> -
>  /* -----------------------------------------------------------------------------
>   * Gen3 Group Allocator
>   */
> @@ -236,10 +232,7 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
>  		}
>  	}
>  
> -	if (vin->group->link_setup)
> -		return vin->group->link_setup(vin->group);
> -
> -	return  0;
> +	return vin->group->link_setup(vin->group);
>  }
>  
>  static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
> @@ -290,17 +283,15 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
>  	guard(mutex)(&group->lock);
>  
>  	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
		struct rvin_dev *vin = &group->vin[i];

will simplify the code blow.

> -		int ret;
> -
>  		if (!group->vin[i] || group->vin[i]->parallel.asc != asc)
>  			continue;
>  
> -		ret = rvin_parallel_subdevice_attach(group->vin[i], subdev);
> -		if (ret)
> -			return ret;
> -
> -		v4l2_set_subdev_hostdata(subdev, group->vin[i]);
> +		group->vin[i]->parallel.source_pad = 0;
> +		for (unsigned int pad = 0; pad < subdev->entity.num_pads; pad++)
> +			if (subdev->entity.pads[pad].flags & MEDIA_PAD_FL_SOURCE)
> +				group->vin[i]->parallel.source_pad = pad;
>  
> +		group->vin[i]->parallel.subdev = subdev;
>  		vin_dbg(group->vin[i], "Bound subdev %s\n", subdev->name);
>  
>  		return 0;
> @@ -525,95 +516,6 @@ static int rvin_create_controls(struct rvin_dev *vin)
>  	return 0;
>  }
>  
> -/* -----------------------------------------------------------------------------
> - * Async notifier
> - */
> -
> -static int rvin_find_pad(struct v4l2_subdev *sd, int direction)
> -{
> -	unsigned int pad;
> -
> -	if (sd->entity.num_pads <= 1)
> -		return 0;
> -
> -	for (pad = 0; pad < sd->entity.num_pads; pad++)
> -		if (sd->entity.pads[pad].flags & direction)
> -			return pad;
> -
> -	return -EINVAL;
> -}
> -
> -/* -----------------------------------------------------------------------------
> - * Parallel async notifier
> - */
> -
> -/* The vin lock should be held when calling the subdevice attach and detach */
> -static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
> -					  struct v4l2_subdev *subdev)
> -{
> -	struct v4l2_subdev_mbus_code_enum code = {
> -		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> -	};
> -	int ret;
> -
> -	/* Find source and sink pad of remote subdevice */
> -	ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
> -	if (ret < 0)
> -		return ret;
> -	vin->parallel.source_pad = ret;
> -
> -	ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
> -	vin->parallel.sink_pad = ret < 0 ? 0 : ret;
> -
> -	if (vin->info->use_mc) {
> -		vin->parallel.subdev = subdev;
> -		return 0;
> -	}
> -
> -	/* Find compatible subdevices mbus format */
> -	vin->mbus_code = 0;
> -	code.index = 0;
> -	code.pad = vin->parallel.source_pad;
> -	while (!vin->mbus_code &&
> -	       !v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
> -		code.index++;
> -		switch (code.code) {
> -		case MEDIA_BUS_FMT_YUYV8_1X16:
> -		case MEDIA_BUS_FMT_UYVY8_1X16:
> -		case MEDIA_BUS_FMT_UYVY8_2X8:
> -		case MEDIA_BUS_FMT_UYVY10_2X10:
> -		case MEDIA_BUS_FMT_RGB888_1X24:
> -			vin->mbus_code = code.code;
> -			vin_dbg(vin, "Found media bus format for %s: %d\n",
> -				subdev->name, vin->mbus_code);
> -			break;
> -		default:
> -			break;
> -		}
> -	}
> -
> -	if (!vin->mbus_code) {
> -		vin_err(vin, "Unsupported media bus format for %s\n",
> -			subdev->name);
> -		return -EINVAL;
> -	}
> -
> -	/* Read tvnorms */
> -	ret = v4l2_subdev_call(subdev, video, g_tvnorms, &vin->vdev.tvnorms);
> -	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> -		return ret;
> -
> -	/* Read standard */
> -	vin->std = V4L2_STD_UNKNOWN;
> -	ret = v4l2_subdev_call(subdev, video, g_std, &vin->std);
> -	if (ret < 0 && ret != -ENOIOCTLCMD)
> -		return ret;
> -
> -	vin->parallel.subdev = subdev;
> -
> -	return 0;
> -}
> -
>  /* -----------------------------------------------------------------------------
>   * CSI-2
>   */
> @@ -971,7 +873,7 @@ static int __maybe_unused rvin_resume(struct device *dev)
>  	 * as we don't know if and in which order the master VINs will
>  	 * be resumed.
>  	 */
> -	if (vin->info->use_mc) {
> +	if (vin->info->model == RCAR_GEN3) {

No need to do this for Gen4 ?

>  		unsigned int master_id = rvin_group_id_to_master(vin->id);
>  		struct rvin_dev *master = vin->group->vin[master_id];
>  		int ret;
> @@ -993,7 +895,6 @@ static int __maybe_unused rvin_resume(struct device *dev)
>  
>  static const struct rvin_info rcar_info_h1 = {
>  	.model = RCAR_H1,
> -	.use_mc = false,
>  	.max_width = 2048,
>  	.max_height = 2048,
>  	.scaler = rvin_scaler_gen2,
> @@ -1001,7 +902,6 @@ static const struct rvin_info rcar_info_h1 = {
>  
>  static const struct rvin_info rcar_info_m1 = {
>  	.model = RCAR_M1,
> -	.use_mc = false,
>  	.max_width = 2048,
>  	.max_height = 2048,
>  	.scaler = rvin_scaler_gen2,
> @@ -1009,7 +909,6 @@ static const struct rvin_info rcar_info_m1 = {
>  
>  static const struct rvin_info rcar_info_gen2 = {
>  	.model = RCAR_GEN2,
> -	.use_mc = false,
>  	.max_width = 2048,
>  	.max_height = 2048,
>  	.scaler = rvin_scaler_gen2,
> @@ -1024,7 +923,6 @@ static const struct rvin_group_route rcar_info_r8a774e1_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a774e1 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a774e1_routes,
> @@ -1040,7 +938,6 @@ static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a7795 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.nv12 = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
> @@ -1058,7 +955,6 @@ static const struct rvin_group_route rcar_info_r8a7796_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a7796 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.nv12 = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
> @@ -1076,7 +972,6 @@ static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a77965 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.nv12 = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
> @@ -1091,7 +986,6 @@ static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a77970 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a77970_routes,
> @@ -1105,7 +999,6 @@ static const struct rvin_group_route rcar_info_r8a77980_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a77980 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.nv12 = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
> @@ -1119,7 +1012,6 @@ static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a77990 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.nv12 = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
> @@ -1133,7 +1025,6 @@ static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
>  
>  static const struct rvin_info rcar_info_r8a77995 = {
>  	.model = RCAR_GEN3,
> -	.use_mc = true,
>  	.nv12 = true,
>  	.max_width = 4096,
>  	.max_height = 4096,
> @@ -1143,7 +1034,6 @@ static const struct rvin_info rcar_info_r8a77995 = {
>  
>  static const struct rvin_info rcar_info_gen4 = {
>  	.model = RCAR_GEN4,
> -	.use_mc = true,
>  	.use_isp = true,
>  	.nv12 = true,
>  	.raw10 = true,
> @@ -1319,21 +1209,27 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		return ret;
>  
> -	if (vin->info->use_isp) {
> -		ret = rvin_isp_init(vin);
> -	} else if (vin->info->use_mc) {
> -		ret = rvin_csi2_init(vin);
> +	switch (vin->info->model) {
> +	case RCAR_GEN3:
> +	case RCAR_GEN4:
> +		if (vin->info->use_isp) {
> +			ret = rvin_isp_init(vin);
> +		} else {
> +			ret = rvin_csi2_init(vin);
>  
> -		if (vin->info->scaler &&
> -		    rvin_group_id_to_master(vin->id) == vin->id)
> -			vin->scaler = vin->info->scaler;
> -	} else {
> -		ret = rvin_group_get(vin, NULL, NULL);
> +			if (vin->info->scaler &&
> +			    rvin_group_id_to_master(vin->id) == vin->id)
> +				vin->scaler = vin->info->scaler;
> +		}
> +		break;
> +	default:
> +		ret = rvin_group_get(vin, rvin_parallel_setup_links, NULL);
>  		if (!ret)
>  			ret = rvin_group_notifier_init(vin, 0, 0);
>  
>  		if (vin->info->scaler)
>  			vin->scaler = vin->info->scaler;

If I'm not mistaken there's one group per VIN on Gen2, right ? If so, I
think you could move the

		if (vin->info->scaler &&
		    rvin_group_id_to_master(vin->id) == vin->id)
			vin->scaler = vin->info->scaler;

code after the switch.

> +		break;
>  	}
>  
>  	if (ret) {
> @@ -1357,11 +1253,9 @@ static void rcar_vin_remove(struct platform_device *pdev)
>  
>  	rvin_v4l2_unregister(vin);
>  
> -	if (vin->info->use_isp || vin->info->use_mc) {
> -		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> -			v4l2_async_nf_unregister(&vin->group->notifier);
> -			v4l2_async_nf_cleanup(&vin->group->notifier);
> -		}
> +	if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> +		v4l2_async_nf_unregister(&vin->group->notifier);
> +		v4l2_async_nf_cleanup(&vin->group->notifier);
>  	}
>  
>  	rvin_group_put(vin);
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> index 4fb33359bb0f..d4faa5a4e757 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> @@ -2,12 +2,11 @@
>  /*
>   * Driver for Renesas R-Car VIN
>   *
> + * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
>   * Copyright (C) 2016 Renesas Electronics Corp.
>   * Copyright (C) 2011-2013 Renesas Solutions Corp.
>   * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
>   * Copyright (C) 2008 Magnus Damm
> - *
> - * Based on the soc-camera rcar_vin driver
>   */
>  
>  #include <linux/delay.h>
> @@ -555,17 +554,12 @@ static void rvin_set_coeff(struct rvin_dev *vin, unsigned short xs)
>  
>  void rvin_scaler_gen2(struct rvin_dev *vin)
>  {
> -	unsigned int crop_height;
>  	u32 xs, ys;
>  
>  	/* Set scaling coefficient */
> -	crop_height = vin->crop.height;
> -	if (V4L2_FIELD_HAS_BOTH(vin->format.field))
> -		crop_height *= 2;
> -
>  	ys = 0;
> -	if (crop_height != vin->compose.height)
> -		ys = (4096 * crop_height) / vin->compose.height;
> +	if (vin->crop.height != vin->compose.height)
> +		ys = (4096 * vin->crop.height) / vin->compose.height;
>  	rvin_write(vin, ys, VNYS_REG);
>  
>  	xs = 0;
> @@ -1294,14 +1288,6 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
>  	struct media_pad *pad;
>  	int ret;
>  
> -	/* No media controller used, simply pass operation to subdevice. */
> -	if (!vin->info->use_mc) {
> -		ret = v4l2_subdev_call(vin->parallel.subdev, video, s_stream,
> -				       on);
> -
> -		return ret == -ENOIOCTLCMD ? 0 : ret;
> -	}
> -
>  	pad = media_pad_remote_pad_first(&vin->pad);
>  	if (!pad)
>  		return -EPIPE;
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> index db091af57c19..2bf94bd77c24 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
> @@ -2,12 +2,11 @@
>  /*
>   * Driver for Renesas R-Car VIN
>   *
> + * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
>   * Copyright (C) 2016 Renesas Electronics Corp.
>   * Copyright (C) 2011-2013 Renesas Solutions Corp.
>   * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
>   * Copyright (C) 2008 Magnus Damm
> - *
> - * Based on the soc-camera rcar_vin driver
>   */
>  
>  #include <linux/pm_runtime.h>
> @@ -230,101 +229,6 @@ static void rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format *pix)
>   * V4L2
>   */
>  
> -static int rvin_reset_format(struct rvin_dev *vin)
> -{
> -	struct v4l2_subdev_format fmt = {
> -		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> -		.pad = vin->parallel.source_pad,
> -	};
> -	int ret;
> -
> -	ret = v4l2_subdev_call(vin_to_source(vin), pad, get_fmt, NULL, &fmt);
> -	if (ret)
> -		return ret;
> -
> -	v4l2_fill_pix_format(&vin->format, &fmt.format);
> -
> -	vin->crop.top = 0;
> -	vin->crop.left = 0;
> -	vin->crop.width = vin->format.width;
> -	vin->crop.height = vin->format.height;
> -
> -	/*  Make use of the hardware interlacer by default. */
> -	if (vin->format.field == V4L2_FIELD_ALTERNATE) {
> -		vin->format.field = V4L2_FIELD_INTERLACED;
> -		vin->format.height *= 2;
> -	}
> -
> -	rvin_format_align(vin, &vin->format);
> -
> -	vin->compose.top = 0;
> -	vin->compose.left = 0;
> -	vin->compose.width = vin->format.width;
> -	vin->compose.height = vin->format.height;
> -
> -	return 0;
> -}
> -
> -static int rvin_try_format(struct rvin_dev *vin, u32 which,
> -			   struct v4l2_pix_format *pix,
> -			   struct v4l2_rect *src_rect)
> -{
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	struct v4l2_subdev_state *sd_state;
> -	static struct lock_class_key key;
> -	struct v4l2_subdev_format format = {
> -		.which = which,
> -		.pad = vin->parallel.source_pad,
> -	};
> -	enum v4l2_field field;
> -	u32 width, height;
> -	int ret;
> -
> -	/*
> -	 * FIXME: Drop this call, drivers are not supposed to use
> -	 * __v4l2_subdev_state_alloc().
> -	 */
> -	sd_state = __v4l2_subdev_state_alloc(sd, "rvin:state->lock", &key);
> -	if (IS_ERR(sd_state))
> -		return PTR_ERR(sd_state);
> -
> -	if (!rvin_format_from_pixel(vin, pix->pixelformat))
> -		pix->pixelformat = RVIN_DEFAULT_FORMAT;
> -
> -	v4l2_fill_mbus_format(&format.format, pix, vin->mbus_code);
> -
> -	/* Allow the video device to override field and to scale */
> -	field = pix->field;
> -	width = pix->width;
> -	height = pix->height;
> -
> -	ret = v4l2_subdev_call(sd, pad, set_fmt, sd_state, &format);
> -	if (ret < 0 && ret != -ENOIOCTLCMD)
> -		goto done;
> -	ret = 0;
> -
> -	v4l2_fill_pix_format(pix, &format.format);
> -
> -	if (src_rect) {
> -		src_rect->top = 0;
> -		src_rect->left = 0;
> -		src_rect->width = pix->width;
> -		src_rect->height = pix->height;
> -	}
> -
> -	if (field != V4L2_FIELD_ANY)
> -		pix->field = field;
> -
> -	pix->width = width;
> -	pix->height = height;
> -
> -	rvin_format_align(vin, pix);
> -done:
> -	__v4l2_subdev_state_free(sd_state);
> -
> -	return ret;
> -}
> -
>  static int rvin_querycap(struct file *file, void *priv,
>  			 struct v4l2_capability *cap)
>  {
> @@ -333,42 +237,6 @@ static int rvin_querycap(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int rvin_try_fmt_vid_cap(struct file *file, void *priv,
> -				struct v4l2_format *f)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -
> -	return rvin_try_format(vin, V4L2_SUBDEV_FORMAT_TRY, &f->fmt.pix, NULL);
> -}
> -
> -static int rvin_s_fmt_vid_cap(struct file *file, void *priv,
> -			      struct v4l2_format *f)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_rect fmt_rect, src_rect;
> -	int ret;
> -
> -	if (vb2_is_busy(&vin->queue))
> -		return -EBUSY;
> -
> -	ret = rvin_try_format(vin, V4L2_SUBDEV_FORMAT_ACTIVE, &f->fmt.pix,
> -			      &src_rect);
> -	if (ret)
> -		return ret;
> -
> -	vin->format = f->fmt.pix;
> -
> -	fmt_rect.top = 0;
> -	fmt_rect.left = 0;
> -	fmt_rect.width = vin->format.width;
> -	fmt_rect.height = vin->format.height;
> -
> -	v4l2_rect_map_inside(&vin->crop, &src_rect);
> -	v4l2_rect_map_inside(&vin->compose, &fmt_rect);
> -
> -	return 0;
> -}
> -
>  static int rvin_g_fmt_vid_cap(struct file *file, void *priv,
>  			      struct v4l2_format *f)
>  {
> @@ -465,6 +333,7 @@ static int rvin_enum_fmt_vid_cap(struct file *file, void *priv,
>  
>  static int rvin_remote_rectangle(struct rvin_dev *vin, struct v4l2_rect *rect)
>  {
> +	struct media_pad *pad = media_pad_remote_pad_first(&vin->pad);
>  	struct v4l2_subdev_format fmt = {
>  		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
>  	};
> @@ -472,18 +341,11 @@ static int rvin_remote_rectangle(struct rvin_dev *vin, struct v4l2_rect *rect)
>  	unsigned int index;
>  	int ret;
>  
> -	if (vin->info->use_mc) {
> -		struct media_pad *pad = media_pad_remote_pad_first(&vin->pad);
> +	if (!pad)
> +		return -EINVAL;
>  
> -		if (!pad)
> -			return -EINVAL;
> -
> -		sd = media_entity_to_v4l2_subdev(pad->entity);
> -		index = pad->index;
> -	} else {
> -		sd = vin_to_source(vin);
> -		index = vin->parallel.source_pad;
> -	}
> +	sd = media_entity_to_v4l2_subdev(pad->entity);
> +	index = pad->index;
>  
>  	fmt.pad = index;
>  	ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> @@ -623,113 +485,6 @@ static int rvin_s_selection(struct file *file, void *fh,
>  	return 0;
>  }
>  
> -static int rvin_g_parm(struct file *file, void *priv,
> -		       struct v4l2_streamparm *parm)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -
> -	return v4l2_g_parm_cap(&vin->vdev, sd, parm);
> -}
> -
> -static int rvin_s_parm(struct file *file, void *priv,
> -		       struct v4l2_streamparm *parm)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -
> -	return v4l2_s_parm_cap(&vin->vdev, sd, parm);
> -}
> -
> -static int rvin_g_pixelaspect(struct file *file, void *priv,
> -			      int type, struct v4l2_fract *f)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -
> -	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -		return -EINVAL;
> -
> -	return v4l2_subdev_call(sd, video, g_pixelaspect, f);
> -}
> -
> -static int rvin_enum_input(struct file *file, void *priv,
> -			   struct v4l2_input *i)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	int ret;
> -
> -	if (i->index != 0)
> -		return -EINVAL;
> -
> -	ret = v4l2_subdev_call(sd, video, g_input_status, &i->status);
> -	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> -		return ret;
> -
> -	i->type = V4L2_INPUT_TYPE_CAMERA;
> -
> -	if (v4l2_subdev_has_op(sd, pad, dv_timings_cap)) {
> -		i->capabilities = V4L2_IN_CAP_DV_TIMINGS;
> -		i->std = 0;
> -	} else {
> -		i->capabilities = V4L2_IN_CAP_STD;
> -		i->std = vin->vdev.tvnorms;
> -	}
> -
> -	strscpy(i->name, "Camera", sizeof(i->name));
> -
> -	return 0;
> -}
> -
> -static int rvin_g_input(struct file *file, void *priv, unsigned int *i)
> -{
> -	*i = 0;
> -	return 0;
> -}
> -
> -static int rvin_s_input(struct file *file, void *priv, unsigned int i)
> -{
> -	if (i > 0)
> -		return -EINVAL;
> -	return 0;
> -}
> -
> -static int rvin_querystd(struct file *file, void *priv, v4l2_std_id *a)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -
> -	return v4l2_subdev_call(sd, video, querystd, a);
> -}
> -
> -static int rvin_s_std(struct file *file, void *priv, v4l2_std_id a)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	int ret;
> -
> -	ret = v4l2_subdev_call(vin_to_source(vin), video, s_std, a);
> -	if (ret < 0)
> -		return ret;
> -
> -	vin->std = a;
> -
> -	/* Changing the standard will change the width/height */
> -	return rvin_reset_format(vin);
> -}
> -
> -static int rvin_g_std(struct file *file, void *priv, v4l2_std_id *a)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -
> -	if (v4l2_subdev_has_op(vin_to_source(vin), pad, dv_timings_cap))
> -		return -ENOIOCTLCMD;
> -
> -	*a = vin->std;
> -
> -	return 0;
> -}
> -
>  static int rvin_subscribe_event(struct v4l2_fh *fh,
>  				const struct v4l2_event_subscription *sub)
>  {
> @@ -740,167 +495,6 @@ static int rvin_subscribe_event(struct v4l2_fh *fh,
>  	return v4l2_ctrl_subscribe_event(fh, sub);
>  }
>  
> -static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
> -				struct v4l2_enum_dv_timings *timings)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	int ret;
> -
> -	if (timings->pad)
> -		return -EINVAL;
> -
> -	timings->pad = vin->parallel.sink_pad;
> -
> -	ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
> -
> -	timings->pad = 0;
> -
> -	return ret;
> -}
> -
> -static int rvin_s_dv_timings(struct file *file, void *priv_fh,
> -			     struct v4l2_dv_timings *timings)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	int ret;
> -
> -	ret = v4l2_subdev_call(sd, pad, s_dv_timings,
> -			       vin->parallel.sink_pad, timings);
> -	if (ret)
> -		return ret;
> -
> -	/* Changing the timings will change the width/height */
> -	return rvin_reset_format(vin);
> -}
> -
> -static int rvin_g_dv_timings(struct file *file, void *priv_fh,
> -			     struct v4l2_dv_timings *timings)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -
> -	return v4l2_subdev_call(sd, pad, g_dv_timings,
> -				vin->parallel.sink_pad, timings);
> -}
> -
> -static int rvin_query_dv_timings(struct file *file, void *priv_fh,
> -				 struct v4l2_dv_timings *timings)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -
> -	return v4l2_subdev_call(sd, pad, query_dv_timings,
> -				vin->parallel.sink_pad, timings);
> -}
> -
> -static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
> -			       struct v4l2_dv_timings_cap *cap)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	int ret;
> -
> -	if (cap->pad)
> -		return -EINVAL;
> -
> -	cap->pad = vin->parallel.sink_pad;
> -
> -	ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
> -
> -	cap->pad = 0;
> -
> -	return ret;
> -}
> -
> -static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	int ret;
> -
> -	if (edid->pad)
> -		return -EINVAL;
> -
> -	edid->pad = vin->parallel.sink_pad;
> -
> -	ret = v4l2_subdev_call(sd, pad, get_edid, edid);
> -
> -	edid->pad = 0;
> -
> -	return ret;
> -}
> -
> -static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> -{
> -	struct rvin_dev *vin = video_drvdata(file);
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	int ret;
> -
> -	if (edid->pad)
> -		return -EINVAL;
> -
> -	edid->pad = vin->parallel.sink_pad;
> -
> -	ret = v4l2_subdev_call(sd, pad, set_edid, edid);
> -
> -	edid->pad = 0;
> -
> -	return ret;
> -}
> -
> -static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> -	.vidioc_querycap		= rvin_querycap,
> -	.vidioc_try_fmt_vid_cap		= rvin_try_fmt_vid_cap,
> -	.vidioc_g_fmt_vid_cap		= rvin_g_fmt_vid_cap,
> -	.vidioc_s_fmt_vid_cap		= rvin_s_fmt_vid_cap,
> -	.vidioc_enum_fmt_vid_cap	= rvin_enum_fmt_vid_cap,
> -
> -	.vidioc_g_selection		= rvin_g_selection,
> -	.vidioc_s_selection		= rvin_s_selection,
> -
> -	.vidioc_g_parm			= rvin_g_parm,
> -	.vidioc_s_parm			= rvin_s_parm,
> -
> -	.vidioc_g_pixelaspect		= rvin_g_pixelaspect,
> -
> -	.vidioc_enum_input		= rvin_enum_input,
> -	.vidioc_g_input			= rvin_g_input,
> -	.vidioc_s_input			= rvin_s_input,
> -
> -	.vidioc_dv_timings_cap		= rvin_dv_timings_cap,
> -	.vidioc_enum_dv_timings		= rvin_enum_dv_timings,
> -	.vidioc_g_dv_timings		= rvin_g_dv_timings,
> -	.vidioc_s_dv_timings		= rvin_s_dv_timings,
> -	.vidioc_query_dv_timings	= rvin_query_dv_timings,
> -
> -	.vidioc_g_edid			= rvin_g_edid,
> -	.vidioc_s_edid			= rvin_s_edid,
> -
> -	.vidioc_querystd		= rvin_querystd,
> -	.vidioc_g_std			= rvin_g_std,
> -	.vidioc_s_std			= rvin_s_std,
> -
> -	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
> -	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
> -	.vidioc_querybuf		= vb2_ioctl_querybuf,
> -	.vidioc_qbuf			= vb2_ioctl_qbuf,
> -	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
> -	.vidioc_expbuf			= vb2_ioctl_expbuf,
> -	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
> -	.vidioc_streamon		= vb2_ioctl_streamon,
> -	.vidioc_streamoff		= vb2_ioctl_streamoff,
> -
> -	.vidioc_log_status		= v4l2_ctrl_log_status,
> -	.vidioc_subscribe_event		= rvin_subscribe_event,
> -	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
> -};
> -
> -/* -----------------------------------------------------------------------------
> - * V4L2 Media Controller
> - */
> -
>  static void rvin_mc_try_format(struct rvin_dev *vin,
>  			       struct v4l2_pix_format *pix)
>  {
> @@ -979,19 +573,6 @@ static const struct v4l2_ioctl_ops rvin_mc_ioctl_ops = {
>   * File Operations
>   */
>  
> -static int rvin_power_parallel(struct rvin_dev *vin, bool on)
> -{
> -	struct v4l2_subdev *sd = vin_to_source(vin);
> -	int power = on ? 1 : 0;
> -	int ret;
> -
> -	ret = v4l2_subdev_call(sd, core, s_power, power);
> -	if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
> -		return ret;
> -
> -	return 0;
> -}
> -
>  static int rvin_open(struct file *file)
>  {
>  	struct rvin_dev *vin = video_drvdata(file);
> @@ -1011,11 +592,7 @@ static int rvin_open(struct file *file)
>  	if (ret)
>  		goto err_unlock;
>  
> -	if (vin->info->use_mc)
> -		ret = v4l2_pipeline_pm_get(&vin->vdev.entity);
> -	else if (v4l2_fh_is_singular_file(file))
> -		ret = rvin_power_parallel(vin, true);
> -
> +	ret = v4l2_pipeline_pm_get(&vin->vdev.entity);

Another item for your todo list, this is deprecated.

>  	if (ret < 0)
>  		goto err_open;
>  
> @@ -1027,10 +604,7 @@ static int rvin_open(struct file *file)
>  
>  	return 0;
>  err_power:
> -	if (vin->info->use_mc)
> -		v4l2_pipeline_pm_put(&vin->vdev.entity);
> -	else if (v4l2_fh_is_singular_file(file))
> -		rvin_power_parallel(vin, false);
> +	v4l2_pipeline_pm_put(&vin->vdev.entity);
>  err_open:
>  	v4l2_fh_release(file);
>  err_unlock:
> @@ -1044,23 +618,14 @@ static int rvin_open(struct file *file)
>  static int rvin_release(struct file *file)
>  {
>  	struct rvin_dev *vin = video_drvdata(file);
> -	bool fh_singular;
>  	int ret;
>  
>  	mutex_lock(&vin->lock);
>  
> -	/* Save the singular status before we call the clean-up helper */
> -	fh_singular = v4l2_fh_is_singular_file(file);
> -
>  	/* the release helper will cleanup any on-going streaming */
>  	ret = _vb2_fop_release(file, NULL);
>  
> -	if (vin->info->use_mc) {
> -		v4l2_pipeline_pm_put(&vin->vdev.entity);
> -	} else {
> -		if (fh_singular)
> -			rvin_power_parallel(vin, false);
> -	}
> +	v4l2_pipeline_pm_put(&vin->vdev.entity);
>  
>  	mutex_unlock(&vin->lock);
>  
> @@ -1113,12 +678,6 @@ static void rvin_notify(struct v4l2_subdev *sd,
>  		container_of(sd->v4l2_dev, struct rvin_dev, v4l2_dev);
>  	unsigned int i;
>  
> -	/* If no media controller, no need to route the event. */
> -	if (!vin->info->use_mc) {
> -		rvin_notify_video_device(vin, notification, arg);
> -		return;
> -	}
> -
>  	group = vin->group;
>  
>  	for (i = 0; i < RCAR_VIN_NUM; i++) {
> @@ -1162,13 +721,8 @@ int rvin_v4l2_register(struct rvin_dev *vin)
>  	vin->format.field = RVIN_DEFAULT_FIELD;
>  	vin->format.colorspace = RVIN_DEFAULT_COLORSPACE;
>  
> -	if (vin->info->use_mc) {
> -		vdev->device_caps |= V4L2_CAP_IO_MC;
> -		vdev->ioctl_ops = &rvin_mc_ioctl_ops;
> -	} else {
> -		vdev->ioctl_ops = &rvin_ioctl_ops;
> -		rvin_reset_format(vin);
> -	}
> +	vdev->device_caps |= V4L2_CAP_IO_MC;

You can combine this with the line that sets device_caps.

> +	vdev->ioctl_ops = &rvin_mc_ioctl_ops;

I would also move this above with the rest of the code that sets the
vdev fields.

>  
>  	rvin_format_align(vin, &vin->format);
>  
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> index 38ae2bd20b72..74bef5b8adad 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
> @@ -2,12 +2,11 @@
>  /*
>   * Driver for Renesas R-Car VIN
>   *
> + * Copyright (C) 2025 Niklas Söderlund <niklas.soderlund@ragnatech.se>
>   * Copyright (C) 2016 Renesas Electronics Corp.
>   * Copyright (C) 2011-2013 Renesas Solutions Corp.
>   * Copyright (C) 2013 Cogent Embedded, Inc., <source@cogentembedded.com>
>   * Copyright (C) 2008 Magnus Damm
> - *
> - * Based on the soc-camera rcar_vin driver
>   */
>  
>  #ifndef __RCAR_VIN__
> @@ -79,8 +78,6 @@ struct rvin_video_format {
>   * @mbus_type:	media bus type
>   * @bus:	media bus parallel configuration
>   * @source_pad:	source pad of remote subdevice
> - * @sink_pad:	sink pad of remote subdevice
> - *
>   */
>  struct rvin_parallel_entity {
>  	struct v4l2_async_connection *asc;
> @@ -90,7 +87,6 @@ struct rvin_parallel_entity {
>  	struct v4l2_mbus_config_parallel bus;
>  
>  	unsigned int source_pad;
> -	unsigned int sink_pad;
>  };
>  
>  /**
> @@ -117,7 +113,6 @@ struct rvin_group_route {
>  /**
>   * struct rvin_info - Information about the particular VIN implementation
>   * @model:		VIN model
> - * @use_mc:		use media controller instead of controlling subdevice
>   * @use_isp:		the VIN is connected to the ISP and not to the CSI-2
>   * @nv12:		support outputting NV12 pixel format
>   * @raw10:		support outputting RAW10 pixel format
> @@ -129,7 +124,6 @@ struct rvin_group_route {
>   */
>  struct rvin_info {
>  	enum model_id model;
> -	bool use_mc;
>  	bool use_isp;
>  	bool nv12;
>  	bool raw10;
> @@ -176,7 +170,6 @@ struct rvin_info {
>   * @crop:		active cropping
>   * @compose:		active composing
>   * @scaler:		Optional scaler
> - * @std:		active video standard of the video source
>   *
>   * @alpha:		Alpha component to fill in for supported pixel formats
>   */
> @@ -218,7 +211,6 @@ struct rvin_dev {
>  	struct v4l2_rect crop;
>  	struct v4l2_rect compose;
>  	void (*scaler)(struct rvin_dev *vin);
> -	v4l2_std_id std;
>  
>  	unsigned int alpha;
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 11/12] media: rcar-vin: Enable media-graph on Gen2

[Niklas Söderlund]

Hi Laurent,

Thanks for your review. I know this patch must not have been fun to 
review, much appreciated.

On 2025-06-13 02:53:19 +0300, Laurent Pinchart wrote:

[snip]

> > @@ -290,17 +283,15 @@ static int rvin_group_notify_bound(struct 
> > v4l2_async_notifier *notifier,
> >  	guard(mutex)(&group->lock);
> >  
> >  	for (unsigned int i = 0; i < RCAR_VIN_NUM; i++) {
> 		struct rvin_dev *vin = &group->vin[i];
> 
> will simplify the code blow.

Create idea.

[snip]

> > @@ -971,7 +873,7 @@ static int __maybe_unused rvin_resume(struct 
> > device *dev)
> >  	 * as we don't know if and in which order the master VINs will
> >  	 * be resumed.
> >  	 */
> > -	if (vin->info->use_mc) {
> > +	if (vin->info->model == RCAR_GEN3) {
> 
> No need to do this for Gen4 ?

No. This section restores the routing for a set of VIN instances, a 
feature only available on Gen3. On Gen4 this functionality is moved to 
the Channel Selector ISP. That it previously was done on Gen4 was not 
intentional.

I think this is a good example on how organic grow of this driver have 
created a lot of unneeded complexity that needs to be resolved, see next 
comment.

[snip]

> > @@ -1319,21 +1209,27 @@ static int rcar_vin_probe(struct 
> > platform_device *pdev)
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	if (vin->info->use_isp) {
> > -		ret = rvin_isp_init(vin);
> > -	} else if (vin->info->use_mc) {
> > -		ret = rvin_csi2_init(vin);
> > +	switch (vin->info->model) {
> > +	case RCAR_GEN3:
> > +	case RCAR_GEN4:
> > +		if (vin->info->use_isp) {
> > +			ret = rvin_isp_init(vin);
> > +		} else {
> > +			ret = rvin_csi2_init(vin);
> >  
> > -		if (vin->info->scaler &&
> > -		    rvin_group_id_to_master(vin->id) == vin->id)
> > -			vin->scaler = vin->info->scaler;
> > -	} else {
> > -		ret = rvin_group_get(vin, NULL, NULL);
> > +			if (vin->info->scaler &&
> > +			    rvin_group_id_to_master(vin->id) == vin->id)
> > +				vin->scaler = vin->info->scaler;
> > +		}
> > +		break;
> > +	default:
> > +		ret = rvin_group_get(vin, rvin_parallel_setup_links, NULL);
> >  		if (!ret)
> >  			ret = rvin_group_notifier_init(vin, 0, 0);
> >  
> >  		if (vin->info->scaler)
> >  			vin->scaler = vin->info->scaler;
> 
> If I'm not mistaken there's one group per VIN on Gen2, right ? If so, I
> think you could move the
> 
> 		if (vin->info->scaler &&
> 		    rvin_group_id_to_master(vin->id) == vin->id)
> 			vin->scaler = vin->info->scaler;
> 
> code after the switch.

Yes and no. The function rvin_group_id_to_master() is purely a Gen3 
thing. To move the check outside the switch it would need to be made 
Gen2 aware. It can be kept in the Gen3+Gen4 code path as we know we 
don't have any scalers on Gen4 so vin->info->scaler will always be 
false.

As the previous comment hints at, this is a bit of an organic growth 
mess. In my ever growing todo file to clean up this driver I hope to 
rework the info structure to remove the remove the 
vin->info->{model,use_isp,nv12,raw10} flags and replace them with 
function pointers that do the right thing for each platform without 
these special cases sprinkled all over. Too much small issues have come 
up over the years using the current design.

For this reason I would like to keep this split in each case of the 
switch over extending the special case handling more in the helper 
functions.

[snip]

> > @@ -1011,11 +592,7 @@ static int rvin_open(struct file *file)
> >  	if (ret)
> >  		goto err_unlock;
> >  
> > -	if (vin->info->use_mc)
> > -		ret = v4l2_pipeline_pm_get(&vin->vdev.entity);
> > -	else if (v4l2_fh_is_singular_file(file))
> > -		ret = rvin_power_parallel(vin, true);
> > -
> > +	ret = v4l2_pipeline_pm_get(&vin->vdev.entity);
> 
> Another item for your todo list, this is deprecated.

Ack, added to list. I hope I can get that list down to zero at some 
point! Getting this series done would be a good step as it's blocking 
many other smaller improvements. Hopefully we won't need another large 
series like this to move forward.

[snip]

> > @@ -1162,13 +721,8 @@ int rvin_v4l2_register(struct rvin_dev *vin)
> >  	vin->format.field = RVIN_DEFAULT_FIELD;
> >  	vin->format.colorspace = RVIN_DEFAULT_COLORSPACE;
> >  
> > -	if (vin->info->use_mc) {
> > -		vdev->device_caps |= V4L2_CAP_IO_MC;
> > -		vdev->ioctl_ops = &rvin_mc_ioctl_ops;
> > -	} else {
> > -		vdev->ioctl_ops = &rvin_ioctl_ops;
> > -		rvin_reset_format(vin);
> > -	}
> > +	vdev->device_caps |= V4L2_CAP_IO_MC;
> 
> You can combine this with the line that sets device_caps.
> 
> > +	vdev->ioctl_ops = &rvin_mc_ioctl_ops;
> 
> I would also move this above with the rest of the code that sets the
> vdev fields.

Great catch, thanks!

[snip]

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v5 10/12] media: rcar-vin: Only expose VIN controls

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2025-06-13 02:28:20 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Fri, Jun 06, 2025 at 08:26:04PM +0200, Niklas Söderlund wrote:
> > Before moving Gen2 to media controller simplify the creation of controls
> > by not exposing the sub-device controls on the video device. This could
> > be done while enabling media controller but doing it separately reduces
> > the changes needed to do so.
> > 
> > The rework also allows the cleanup and remove paths to be simplified by
> > folding all special cases into the only remaining call site.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> > * Changes since v4
> > - Broken out from a larger patch.
> > ---
> >  .../platform/renesas/rcar-vin/rcar-core.c     | 84 ++++---------------
> >  1 file changed, 18 insertions(+), 66 deletions(-)
> > 
> > diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-core.c b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> > index 8cb3d0a3520f..597e868a6bc5 100644
> > --- a/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/renesas/rcar-vin/rcar-core.c
> > @@ -365,14 +365,6 @@ static int rvin_group_parse_of(struct rvin_dev *vin, unsigned int port,
> >  	return ret;
> >  }
> >  
> > -static void rvin_group_notifier_cleanup(struct rvin_dev *vin)
> > -{
> > -	if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> > -		v4l2_async_nf_unregister(&vin->group->notifier);
> > -		v4l2_async_nf_cleanup(&vin->group->notifier);
> > -	}
> > -}
> > -
> >  static int rvin_parallel_parse_of(struct rvin_dev *vin)
> >  {
> >  	struct fwnode_handle *fwnode __free(fwnode_handle) = NULL;
> > @@ -510,7 +502,7 @@ static void rvin_free_controls(struct rvin_dev *vin)
> >  	vin->vdev.ctrl_handler = NULL;
> >  }
> >  
> > -static int rvin_create_controls(struct rvin_dev *vin, struct v4l2_subdev *subdev)
> > +static int rvin_create_controls(struct rvin_dev *vin)
> >  {
> >  	int ret;
> >  
> > @@ -528,16 +520,6 @@ static int rvin_create_controls(struct rvin_dev *vin, struct v4l2_subdev *subdev
> >  		return ret;
> >  	}
> >  
> 
> Now that the control handler for the video device only has a single
> control, you can reduce the number of buckets:
> 
> -	ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 16);
> +	ret = v4l2_ctrl_handler_init(&vin->ctrl_handler, 1);

Agreed.

> 
> > -	/* For the non-MC mode add controls from the subdevice. */
> > -	if (subdev) {
> > -		ret = v4l2_ctrl_add_handler(&vin->ctrl_handler,
> > -					    subdev->ctrl_handler, NULL, true);
> > -		if (ret < 0) {
> > -			rvin_free_controls(vin);
> > -			return ret;
> > -		}
> > -	}
> > -
> >  	vin->vdev.ctrl_handler = &vin->ctrl_handler;
> >  
> >  	return 0;
> > @@ -627,11 +609,6 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
> >  	if (ret < 0 && ret != -ENOIOCTLCMD)
> >  		return ret;
> >  
> > -	/* Add the controls */
> > -	ret = rvin_create_controls(vin, subdev);
> > -	if (ret < 0)
> > -		return ret;
> > -
> >  	vin->parallel.subdev = subdev;
> >  
> >  	return 0;
> > @@ -885,34 +862,17 @@ static int rvin_csi2_setup_links(struct rvin_group *group)
> >  	return ret;
> >  }
> >  
> > -static void rvin_csi2_cleanup(struct rvin_dev *vin)
> > -{
> > -	rvin_group_notifier_cleanup(vin);
> > -	rvin_free_controls(vin);
> > -}
> > -
> >  static int rvin_csi2_init(struct rvin_dev *vin)
> >  {
> >  	int ret;
> >  
> > -
> > -	ret = rvin_create_controls(vin, NULL);
> > -	if (ret < 0)
> > -		return ret;
> > -
> >  	ret = rvin_group_get(vin, rvin_csi2_setup_links, &rvin_csi2_media_ops);
> >  	if (ret)
> > -		goto err_controls;
> > +		return ret;
> >  
> >  	ret = rvin_group_notifier_init(vin, 1, RVIN_CSI_MAX);
> >  	if (ret)
> > -		goto err_group;
> > -
> > -	return 0;
> > -err_group:
> > -	rvin_group_put(vin);
> > -err_controls:
> > -	rvin_free_controls(vin);
> > +		rvin_group_put(vin);
> >  
> >  	return ret;
> >  }
> > @@ -966,34 +926,17 @@ static int rvin_isp_setup_links(struct rvin_group *group)
> >  	return ret;
> >  }
> >  
> > -static void rvin_isp_cleanup(struct rvin_dev *vin)
> > -{
> > -	rvin_group_notifier_cleanup(vin);
> > -	rvin_free_controls(vin);
> > -}
> > -
> >  static int rvin_isp_init(struct rvin_dev *vin)
> >  {
> >  	int ret;
> >  
> > -
> > -	ret = rvin_create_controls(vin, NULL);
> > -	if (ret < 0)
> > -		return ret;
> > -
> >  	ret = rvin_group_get(vin, rvin_isp_setup_links, NULL);
> >  	if (ret)
> > -		goto err_controls;
> > +		return ret;
> >  
> >  	ret = rvin_group_notifier_init(vin, 2, RVIN_ISP_MAX);
> >  	if (ret)
> > -		goto err_group;
> > -
> > -	return 0;
> > -err_group:
> > -	rvin_group_put(vin);
> > -err_controls:
> > -	rvin_free_controls(vin);
> > +		rvin_group_put(vin);
> >  
> >  	return ret;
> >  }
> > @@ -1372,6 +1315,10 @@ static int rcar_vin_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		return ret;
> >  
> > +	ret = rvin_create_controls(vin);
> > +	if (ret < 0)
> > +		return ret;
> 
> Don't you need to call rvin_dma_unregister() here ?

Indeed, there is a similar issue where rvin_id_put() is not called where 
it should introduced by this series.

> 
> > +
> >  	if (vin->info->use_isp) {
> >  		ret = rvin_isp_init(vin);
> >  	} else if (vin->info->use_mc) {
> > @@ -1390,6 +1337,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	if (ret) {
> > +		rvin_free_controls(vin);
> >  		rvin_dma_unregister(vin);
> >  		rvin_id_put(vin);
> >  		return ret;
> 
> Error labels at the end of the function will make this more manageable.

I agree, I have added a small patch early in this series that converts 
this error path to use labels and fixed the missing call to 
rvin_id_put() and rvin_dma_unregister() in the error paths using labels 
as you suggest it turns out nicer.

> 
> > @@ -1409,13 +1357,17 @@ static void rcar_vin_remove(struct platform_device *pdev)
> >  
> >  	rvin_v4l2_unregister(vin);
> >  
> > -	if (vin->info->use_isp)
> > -		rvin_isp_cleanup(vin);
> > -	else if (vin->info->use_mc)
> > -		rvin_csi2_cleanup(vin);
> > +	if (vin->info->use_isp || vin->info->use_mc) {
> > +		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> > +			v4l2_async_nf_unregister(&vin->group->notifier);
> > +			v4l2_async_nf_cleanup(&vin->group->notifier);
> > +		}
> > +	}
> >  
> >  	rvin_group_put(vin);
> >  
> > +	rvin_free_controls(vin);
> > +
> >  	rvin_id_put(vin);
> >  
> >  	rvin_dma_unregister(vin);
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH v5 07/12] media: rcar-vin: Merge all notifiers

[Geert Uytterhoeven]

Hi Laurent,

On Tue, 17 Jun 2025 at 21:48, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Fri, Jun 06, 2025 at 08:26:01PM +0200, Niklas Söderlund wrote:
> > The VIN usage of v4l-async is complex and stems from organic growth of
> > the driver of supporting both private local subdevices (Gen2, Gen3) and
> > subdevices shared between all VIN instances (Gen3 and Gen4).
> >
> > The driver used a separate notifier for each VIN for the private local
> > ones, and a shared group notifier for the shared ones. This was complex
> > and lead to subtle bugs when unbinding and later rebinding subdevices in
> > one of the notifiers having to handle different edge cases depending on
> > if it also had subdevices in the other notifiers etc.
> >
> > To simplify this have the Gen2 devices allocate and form a VIN group
> > too. This way all subdevices on all models can be collect in a
> > single group notifier. Then there is only a single complete callback for
> > all where the video devices and subdevice nodes can be registered etc.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>

[ deleted 132 lines of quoted patch ]

> > @@ -417,6 +452,12 @@ static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
> >               if (!(vin_mask & BIT(i)))
> >                       continue;
> >
> > +             /* Parse local subdevice. */
> > +             ret = rvin_parallel_parse_of(vin->group->vin[i]);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             /* Prase shared subdevices. */
>
> s/Prase/Parse/
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Thanks, but please trim your replies, I had to scroll three times
through your email to find this ;-)

[ deleted 262 lines of quoted patch ]

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v5 07/12] media: rcar-vin: Merge all notifiers

[Laurent Pinchart]

On Wed, Jun 18, 2025 at 09:44:02AM +0200, Geert Uytterhoeven wrote:
> Hi Laurent,
> 
> On Tue, 17 Jun 2025 at 21:48, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > On Fri, Jun 06, 2025 at 08:26:01PM +0200, Niklas Söderlund wrote:
> > > The VIN usage of v4l-async is complex and stems from organic growth of
> > > the driver of supporting both private local subdevices (Gen2, Gen3) and
> > > subdevices shared between all VIN instances (Gen3 and Gen4).
> > >
> > > The driver used a separate notifier for each VIN for the private local
> > > ones, and a shared group notifier for the shared ones. This was complex
> > > and lead to subtle bugs when unbinding and later rebinding subdevices in
> > > one of the notifiers having to handle different edge cases depending on
> > > if it also had subdevices in the other notifiers etc.
> > >
> > > To simplify this have the Gen2 devices allocate and form a VIN group
> > > too. This way all subdevices on all models can be collect in a
> > > single group notifier. Then there is only a single complete callback for
> > > all where the video devices and subdevice nodes can be registered etc.
> > >
> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > Tested-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> 
> [ deleted 132 lines of quoted patch ]
> 
> > > @@ -417,6 +452,12 @@ static int rvin_group_notifier_init(struct rvin_dev *vin, unsigned int port,
> > >               if (!(vin_mask & BIT(i)))
> > >                       continue;
> > >
> > > +             /* Parse local subdevice. */
> > > +             ret = rvin_parallel_parse_of(vin->group->vin[i]);
> > > +             if (ret)
> > > +                     return ret;
> > > +
> > > +             /* Prase shared subdevices. */
> >
> > s/Prase/Parse/
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> Thanks, but please trim your replies, I had to scroll three times
> through your email to find this ;-)

I intentionally don't, as I find it annoying when people do :-)

> [ deleted 262 lines of quoted patch ]

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 07/12] media: rcar-vin: Merge all notifiers

[Geert Uytterhoeven]

Hi Laurent,

On Wed, 18 Jun 2025 at 10:58, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Wed, Jun 18, 2025 at 09:44:02AM +0200, Geert Uytterhoeven wrote:
> > On Tue, 17 Jun 2025 at 21:48, Laurent Pinchart

> > [ deleted 132 lines of quoted patch ]

> > Thanks, but please trim your replies, I had to scroll three times
> > through your email to find this ;-)
>
> I intentionally don't, as I find it annoying when people do :-)

I find it annoying to have to search for the needle in the
haystack, and I'm not alone:
https://elixir.bootlin.com/linux/v6.15.2/source/Documentation/process/submitting-patches.rst#L352

Thanks!

> > [ deleted 262 lines of quoted patch ]

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v5 07/12] media: rcar-vin: Merge all notifiers

[Wolfram Sang]

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


> I find it annoying to have to search for the needle in the
> haystack, and I'm not alone:
> https://elixir.bootlin.com/linux/v6.15.2/source/Documentation/process/submitting-patches.rst#L352

FTR +1 ;)

Still, love you all!


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