[PATCH v5 0/10] rcar-vin: Add support for parallel input on Gen3

[PATCH v5 0/10] rcar-vin: Add support for parallel input on Gen3

[Jacopo Mondi]

Hello,
   this series adds support for parallel video input to the Gen3 version of
rcar-vin driver.

Since last version one patch has been added (as anticipated by email) to clean
up the memory reserved for notifier's async subdevices if the notifier
registration fails ([4/10]), and two comments from Niklas has been addressed
in [6/10], where the group notifier is now cleaned up in the error path if
it's necessary to do that.

Patches 4,5 and 6 have not yet been Acked-by/Reviewed-by, all the other ones
have at least one tag, so things looks quite good.

Additional changelogs in single patches.

Thanks
    j

Jacopo Mondi (10):
  media: rcar-vin: Rename 'digital' to 'parallel'
  media: rcar-vin: Remove two empty lines
  media: rcar-vin: Create a group notifier
  media: rcar-vin: Cleanup notifier in error path
  media: rcar-vin: Cache the mbus configuration flags
  media: rcar-vin: Parse parallel input on Gen3
  media: rcar-vin: Link parallel input media entities
  media: rcar-vin: Handle parallel subdev in link_notify
  media: rcar-vin: Rename _rcar_info to rcar_info
  media: rcar-vin: Add support for R-Car R8A77995 SoC

 drivers/media/platform/rcar-vin/rcar-core.c | 269 +++++++++++++++++++---------
 drivers/media/platform/rcar-vin/rcar-dma.c  |  36 ++--
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  12 +-
 drivers/media/platform/rcar-vin/rcar-vin.h  |  29 +--
 4 files changed, 230 insertions(+), 116 deletions(-)

--
2.7.4

[PATCH v5 01/10] media: rcar-vin: Rename 'digital' to 'parallel'

[Jacopo Mondi]

As the term 'digital' is used all over the rcar-vin code in place of
'parallel', rename all the occurrencies.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 72 ++++++++++++++---------------
 drivers/media/platform/rcar-vin/rcar-dma.c  |  4 +-
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 12 ++---
 drivers/media/platform/rcar-vin/rcar-vin.h  |  6 +--
 4 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index d3072e1..16d3e01 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -376,12 +376,12 @@ static int rvin_find_pad(struct v4l2_subdev *sd, int direction)
 }
 
 /* -----------------------------------------------------------------------------
- * Digital async notifier
+ * Parallel async notifier
  */
 
 /* The vin lock should be held when calling the subdevice attach and detach */
-static int rvin_digital_subdevice_attach(struct rvin_dev *vin,
-					 struct v4l2_subdev *subdev)
+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,
@@ -392,15 +392,15 @@ static int rvin_digital_subdevice_attach(struct rvin_dev *vin,
 	ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SOURCE);
 	if (ret < 0)
 		return ret;
-	vin->digital->source_pad = ret;
+	vin->parallel->source_pad = ret;
 
 	ret = rvin_find_pad(subdev, MEDIA_PAD_FL_SINK);
-	vin->digital->sink_pad = ret < 0 ? 0 : ret;
+	vin->parallel->sink_pad = ret < 0 ? 0 : ret;
 
 	/* Find compatible subdevices mbus format */
 	vin->mbus_code = 0;
 	code.index = 0;
-	code.pad = vin->digital->source_pad;
+	code.pad = vin->parallel->source_pad;
 	while (!vin->mbus_code &&
 	       !v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
 		code.index++;
@@ -450,21 +450,21 @@ static int rvin_digital_subdevice_attach(struct rvin_dev *vin,
 
 	vin->vdev.ctrl_handler = &vin->ctrl_handler;
 
-	vin->digital->subdev = subdev;
+	vin->parallel->subdev = subdev;
 
 	return 0;
 }
 
-static void rvin_digital_subdevice_detach(struct rvin_dev *vin)
+static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
 {
 	rvin_v4l2_unregister(vin);
 	v4l2_ctrl_handler_free(&vin->ctrl_handler);
 
 	vin->vdev.ctrl_handler = NULL;
-	vin->digital->subdev = NULL;
+	vin->parallel->subdev = NULL;
 }
 
-static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
+static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 	int ret;
@@ -478,28 +478,28 @@ static int rvin_digital_notify_complete(struct v4l2_async_notifier *notifier)
 	return rvin_v4l2_register(vin);
 }
 
-static void rvin_digital_notify_unbind(struct v4l2_async_notifier *notifier,
-				       struct v4l2_subdev *subdev,
-				       struct v4l2_async_subdev *asd)
+static void rvin_parallel_notify_unbind(struct v4l2_async_notifier *notifier,
+					struct v4l2_subdev *subdev,
+					struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 
-	vin_dbg(vin, "unbind digital subdev %s\n", subdev->name);
+	vin_dbg(vin, "unbind parallel subdev %s\n", subdev->name);
 
 	mutex_lock(&vin->lock);
-	rvin_digital_subdevice_detach(vin);
+	rvin_parallel_subdevice_detach(vin);
 	mutex_unlock(&vin->lock);
 }
 
-static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
-				     struct v4l2_subdev *subdev,
-				     struct v4l2_async_subdev *asd)
+static int rvin_parallel_notify_bound(struct v4l2_async_notifier *notifier,
+				      struct v4l2_subdev *subdev,
+				      struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
 	int ret;
 
 	mutex_lock(&vin->lock);
-	ret = rvin_digital_subdevice_attach(vin, subdev);
+	ret = rvin_parallel_subdevice_attach(vin, subdev);
 	mutex_unlock(&vin->lock);
 	if (ret)
 		return ret;
@@ -507,21 +507,21 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
 	v4l2_set_subdev_hostdata(subdev, vin);
 
 	vin_dbg(vin, "bound subdev %s source pad: %u sink pad: %u\n",
-		subdev->name, vin->digital->source_pad,
-		vin->digital->sink_pad);
+		subdev->name, vin->parallel->source_pad,
+		vin->parallel->sink_pad);
 
 	return 0;
 }
 
-static const struct v4l2_async_notifier_operations rvin_digital_notify_ops = {
-	.bound = rvin_digital_notify_bound,
-	.unbind = rvin_digital_notify_unbind,
-	.complete = rvin_digital_notify_complete,
+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 int rvin_digital_parse_v4l2(struct device *dev,
-				   struct v4l2_fwnode_endpoint *vep,
-				   struct v4l2_async_subdev *asd)
+static int rvin_parallel_parse_v4l2(struct device *dev,
+				    struct v4l2_fwnode_endpoint *vep,
+				    struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = dev_get_drvdata(dev);
 	struct rvin_graph_entity *rvge =
@@ -546,28 +546,28 @@ static int rvin_digital_parse_v4l2(struct device *dev,
 		return -EINVAL;
 	}
 
-	vin->digital = rvge;
+	vin->parallel = rvge;
 
 	return 0;
 }
 
-static int rvin_digital_graph_init(struct rvin_dev *vin)
+static int rvin_parallel_graph_init(struct rvin_dev *vin)
 {
 	int ret;
 
 	ret = v4l2_async_notifier_parse_fwnode_endpoints(
 		vin->dev, &vin->notifier,
-		sizeof(struct rvin_graph_entity), rvin_digital_parse_v4l2);
+		sizeof(struct rvin_graph_entity), rvin_parallel_parse_v4l2);
 	if (ret)
 		return ret;
 
-	if (!vin->digital)
+	if (!vin->parallel)
 		return -ENODEV;
 
-	vin_dbg(vin, "Found digital subdevice %pOF\n",
-		to_of_node(vin->digital->asd.match.fwnode));
+	vin_dbg(vin, "Found parallel subdevice %pOF\n",
+		to_of_node(vin->parallel->asd.match.fwnode));
 
-	vin->notifier.ops = &rvin_digital_notify_ops;
+	vin->notifier.ops = &rvin_parallel_notify_ops;
 	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
 	if (ret < 0) {
 		vin_err(vin, "Notifier registration failed\n");
@@ -1088,7 +1088,7 @@ static int rcar_vin_probe(struct platform_device *pdev)
 	if (vin->info->use_mc)
 		ret = rvin_mc_init(vin);
 	else
-		ret = rvin_digital_graph_init(vin);
+		ret = rvin_parallel_graph_init(vin);
 	if (ret < 0)
 		goto error;
 
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index ac07f99..f1c3585 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -733,7 +733,7 @@ static int rvin_setup(struct rvin_dev *vin)
 		vnmc |= VNMC_BPS;
 
 	if (vin->info->model == RCAR_GEN3) {
-		/* Select between CSI-2 and Digital input */
+		/* Select between CSI-2 and parallel input */
 		if (vin->mbus_cfg.type == V4L2_MBUS_CSI2)
 			vnmc &= ~VNMC_DPINE;
 		else
@@ -1074,7 +1074,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
 
 	/* No media controller used, simply pass operation to subdevice. */
 	if (!vin->info->use_mc) {
-		ret = v4l2_subdev_call(vin->digital->subdev, video, s_stream,
+		ret = v4l2_subdev_call(vin->parallel->subdev, video, s_stream,
 				       on);
 
 		return ret == -ENOIOCTLCMD ? 0 : ret;
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index e78fba8..87a718b 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -144,7 +144,7 @@ static int rvin_reset_format(struct rvin_dev *vin)
 {
 	struct v4l2_subdev_format fmt = {
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
-		.pad = vin->digital->source_pad,
+		.pad = vin->parallel->source_pad,
 	};
 	int ret;
 
@@ -175,7 +175,7 @@ static int rvin_try_format(struct rvin_dev *vin, u32 which,
 	struct v4l2_subdev_pad_config *pad_cfg;
 	struct v4l2_subdev_format format = {
 		.which = which,
-		.pad = vin->digital->source_pad,
+		.pad = vin->parallel->source_pad,
 	};
 	enum v4l2_field field;
 	u32 width, height;
@@ -517,7 +517,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
 	if (timings->pad)
 		return -EINVAL;
 
-	timings->pad = vin->digital->sink_pad;
+	timings->pad = vin->parallel->sink_pad;
 
 	ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
 
@@ -569,7 +569,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
 	if (cap->pad)
 		return -EINVAL;
 
-	cap->pad = vin->digital->sink_pad;
+	cap->pad = vin->parallel->sink_pad;
 
 	ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
 
@@ -587,7 +587,7 @@ static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
 	if (edid->pad)
 		return -EINVAL;
 
-	edid->pad = vin->digital->sink_pad;
+	edid->pad = vin->parallel->sink_pad;
 
 	ret = v4l2_subdev_call(sd, pad, get_edid, edid);
 
@@ -605,7 +605,7 @@ static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
 	if (edid->pad)
 		return -EINVAL;
 
-	edid->pad = vin->digital->sink_pad;
+	edid->pad = vin->parallel->sink_pad;
 
 	ret = v4l2_subdev_call(sd, pad, set_edid, edid);
 
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index c2aef78..755ac3c 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -146,7 +146,7 @@ struct rvin_info {
  * @v4l2_dev:		V4L2 device
  * @ctrl_handler:	V4L2 control handler
  * @notifier:		V4L2 asynchronous subdevs notifier
- * @digital:		entity in the DT for local digital subdevice
+ * @parallel:		entity in the DT for local parallel subdevice
  *
  * @group:		Gen3 CSI group
  * @id:			Gen3 group id for this VIN
@@ -182,7 +182,7 @@ struct rvin_dev {
 	struct v4l2_device v4l2_dev;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_async_notifier notifier;
-	struct rvin_graph_entity *digital;
+	struct rvin_graph_entity *parallel;
 
 	struct rvin_group *group;
 	unsigned int id;
@@ -209,7 +209,7 @@ struct rvin_dev {
 	v4l2_std_id std;
 };
 
-#define vin_to_source(vin)		((vin)->digital->subdev)
+#define vin_to_source(vin)		((vin)->parallel->subdev)
 
 /* Debug */
 #define vin_dbg(d, fmt, arg...)		dev_dbg(d->dev, fmt, ##arg)
-- 
2.7.4

[PATCH v5 02/10] media: rcar-vin: Remove two empty lines

[Jacopo Mondi]

Remove un-necessary empty lines.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 16d3e01..bcf02de 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -707,11 +707,9 @@ static int rvin_mc_parse_of_endpoint(struct device *dev,
 		return -EINVAL;
 
 	if (!of_device_is_available(to_of_node(asd->match.fwnode))) {
-
 		vin_dbg(vin, "OF device %pOF disabled, ignoring\n",
 			to_of_node(asd->match.fwnode));
 		return -ENOTCONN;
-
 	}
 
 	if (vin->group->csi[vep->base.id].fwnode) {
-- 
2.7.4

[PATCH v5 03/10] media: rcar-vin: Create a group notifier

[Jacopo Mondi]

As CSI-2 subdevices are shared between several VIN instances, a shared
notifier to collect the CSI-2 async subdevices is required. So far, the
rcar-vin driver used the notifier of the last VIN instance to probe but
with the forth-coming introduction of parallel input subdevices support
in mc-compliant code path, each VIN may register its own notifier if any
parallel subdevice is connected there.

To avoid registering a notifier twice (once for parallel subdev and one
for the CSI-2 subdevs) create a group notifier, shared by all the VIN
instances.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
v3 -> v4:
- Unregister and cleanup group notifier when un-registering the VIN
  instance whose v4l2_dev the notifier is associated to.
---
 drivers/media/platform/rcar-vin/rcar-core.c | 38 ++++++++++++++---------------
 drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++--
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index bcf02de..d3aadf3 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -46,6 +46,8 @@
  */
 #define rvin_group_id_to_master(vin) ((vin) < 4 ? 0 : 4)
 
+#define v4l2_dev_to_vin(d)	container_of(d, struct rvin_dev, v4l2_dev)
+
 /* -----------------------------------------------------------------------------
  * Media Controller link notification
  */
@@ -583,7 +585,7 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin)
 
 static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
 {
-	struct rvin_dev *vin = notifier_to_vin(notifier);
+	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
 	const struct rvin_group_route *route;
 	unsigned int i;
 	int ret;
@@ -649,7 +651,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 				     struct v4l2_subdev *subdev,
 				     struct v4l2_async_subdev *asd)
 {
-	struct rvin_dev *vin = notifier_to_vin(notifier);
+	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
 	unsigned int i;
 
 	for (i = 0; i < RCAR_VIN_NUM; i++)
@@ -673,7 +675,7 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
 				   struct v4l2_subdev *subdev,
 				   struct v4l2_async_subdev *asd)
 {
-	struct rvin_dev *vin = notifier_to_vin(notifier);
+	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
 	unsigned int i;
 
 	mutex_lock(&vin->group->lock);
@@ -734,12 +736,6 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
 
 	mutex_lock(&vin->group->lock);
 
-	/* If there already is a notifier something has gone wrong, bail out. */
-	if (WARN_ON(vin->group->notifier)) {
-		mutex_unlock(&vin->group->lock);
-		return -EINVAL;
-	}
-
 	/* If not all VIN's are registered don't register the notifier. */
 	for (i = 0; i < RCAR_VIN_NUM; i++)
 		if (vin->group->vin[i])
@@ -751,19 +747,16 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
 	}
 
 	/*
-	 * Have all VIN's look for subdevices. Some subdevices will overlap
-	 * but the parser function can handle it, so each subdevice will
-	 * only be registered once with the notifier.
+	 * Have all VIN's look for CSI-2 subdevices. Some subdevices will
+	 * overlap but the parser function can handle it, so each subdevice
+	 * will only be registered once with the group notifier.
 	 */
-
-	vin->group->notifier = &vin->notifier;
-
 	for (i = 0; i < RCAR_VIN_NUM; i++) {
 		if (!vin->group->vin[i])
 			continue;
 
 		ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
-				vin->group->vin[i]->dev, vin->group->notifier,
+				vin->group->vin[i]->dev, &vin->group->notifier,
 				sizeof(struct v4l2_async_subdev), 1,
 				rvin_mc_parse_of_endpoint);
 		if (ret) {
@@ -774,9 +767,12 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
 
 	mutex_unlock(&vin->group->lock);
 
-	vin->group->notifier->ops = &rvin_group_notify_ops;
+	if (!vin->group->notifier.num_subdevs)
+		return 0;
 
-	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
+	vin->group->notifier.ops = &rvin_group_notify_ops;
+	ret = v4l2_async_notifier_register(&vin->v4l2_dev,
+					   &vin->group->notifier);
 	if (ret < 0) {
 		vin_err(vin, "Notifier registration failed\n");
 		return ret;
@@ -1114,8 +1110,10 @@ static int rcar_vin_remove(struct platform_device *pdev)
 
 	if (vin->info->use_mc) {
 		mutex_lock(&vin->group->lock);
-		if (vin->group->notifier == &vin->notifier)
-			vin->group->notifier = NULL;
+		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
+			v4l2_async_notifier_unregister(&vin->group->notifier);
+			v4l2_async_notifier_cleanup(&vin->group->notifier);
+		}
 		mutex_unlock(&vin->group->lock);
 		rvin_group_put(vin);
 	} else {
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 755ac3c..ebb480f7 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -225,8 +225,7 @@ struct rvin_dev {
  *
  * @lock:		protects the count, notifier, vin and csi members
  * @count:		number of enabled VIN instances found in DT
- * @notifier:		pointer to the notifier of a VIN which handles the
- *			groups async sub-devices.
+ * @notifier:		group notifier for CSI-2 async subdevices
  * @vin:		VIN instances which are part of the group
  * @csi:		array of pairs of fwnode and subdev pointers
  *			to all CSI-2 subdevices.
@@ -238,7 +237,7 @@ struct rvin_group {
 
 	struct mutex lock;
 	unsigned int count;
-	struct v4l2_async_notifier *notifier;
+	struct v4l2_async_notifier notifier;
 	struct rvin_dev *vin[RCAR_VIN_NUM];
 
 	struct {
-- 
2.7.4

Re: [PATCH v5 03/10] media: rcar-vin: Create a group notifier

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 10:48:01 +0200, Jacopo Mondi wrote:
> As CSI-2 subdevices are shared between several VIN instances, a shared
> notifier to collect the CSI-2 async subdevices is required. So far, the
> rcar-vin driver used the notifier of the last VIN instance to probe but
> with the forth-coming introduction of parallel input subdevices support
> in mc-compliant code path, each VIN may register its own notifier if any
> parallel subdevice is connected there.
> 
> To avoid registering a notifier twice (once for parallel subdev and one
> for the CSI-2 subdevs) create a group notifier, shared by all the VIN
> instances.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> ---
> v3 -> v4:
> - Unregister and cleanup group notifier when un-registering the VIN
>   instance whose v4l2_dev the notifier is associated to.
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 38 ++++++++++++++---------------
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++--
>  2 files changed, 20 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index bcf02de..d3aadf3 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -46,6 +46,8 @@
>   */
>  #define rvin_group_id_to_master(vin) ((vin) < 4 ? 0 : 4)
>  
> +#define v4l2_dev_to_vin(d)	container_of(d, struct rvin_dev, v4l2_dev)

I'm fine with this new macro but could you update the last 3 remaining 
call sites which after this patch still use the old macro 
notifier_to_vin()?  The old macro is with this patch misleading as it 
can't handle the group notifier. Or perhaps you could update the old 
macro and move it here?

#define notifier_to_vin(n) container_of((n)->v4l2_dev, struct rvin_dev, v4l2_dev)

I don't really care which macro you pick but I do think that there 
should only be one macro to go from a notifier to a rvin_dev instance 
and that it should be able to handle both the vin local and the group 
notifier.

The rest of this patch looks really nice.

> +
>  /* -----------------------------------------------------------------------------
>   * Media Controller link notification
>   */
> @@ -583,7 +585,7 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin)
>  
>  static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
>  {
> -	struct rvin_dev *vin = notifier_to_vin(notifier);
> +	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
>  	const struct rvin_group_route *route;
>  	unsigned int i;
>  	int ret;
> @@ -649,7 +651,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  				     struct v4l2_subdev *subdev,
>  				     struct v4l2_async_subdev *asd)
>  {
> -	struct rvin_dev *vin = notifier_to_vin(notifier);
> +	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
>  	unsigned int i;
>  
>  	for (i = 0; i < RCAR_VIN_NUM; i++)
> @@ -673,7 +675,7 @@ static int rvin_group_notify_bound(struct v4l2_async_notifier *notifier,
>  				   struct v4l2_subdev *subdev,
>  				   struct v4l2_async_subdev *asd)
>  {
> -	struct rvin_dev *vin = notifier_to_vin(notifier);
> +	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
>  	unsigned int i;
>  
>  	mutex_lock(&vin->group->lock);
> @@ -734,12 +736,6 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>  
>  	mutex_lock(&vin->group->lock);
>  
> -	/* If there already is a notifier something has gone wrong, bail out. */
> -	if (WARN_ON(vin->group->notifier)) {
> -		mutex_unlock(&vin->group->lock);
> -		return -EINVAL;
> -	}
> -
>  	/* If not all VIN's are registered don't register the notifier. */
>  	for (i = 0; i < RCAR_VIN_NUM; i++)
>  		if (vin->group->vin[i])
> @@ -751,19 +747,16 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>  	}
>  
>  	/*
> -	 * Have all VIN's look for subdevices. Some subdevices will overlap
> -	 * but the parser function can handle it, so each subdevice will
> -	 * only be registered once with the notifier.
> +	 * Have all VIN's look for CSI-2 subdevices. Some subdevices will
> +	 * overlap but the parser function can handle it, so each subdevice
> +	 * will only be registered once with the group notifier.
>  	 */
> -
> -	vin->group->notifier = &vin->notifier;
> -
>  	for (i = 0; i < RCAR_VIN_NUM; i++) {
>  		if (!vin->group->vin[i])
>  			continue;
>  
>  		ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> -				vin->group->vin[i]->dev, vin->group->notifier,
> +				vin->group->vin[i]->dev, &vin->group->notifier,
>  				sizeof(struct v4l2_async_subdev), 1,
>  				rvin_mc_parse_of_endpoint);
>  		if (ret) {
> @@ -774,9 +767,12 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>  
>  	mutex_unlock(&vin->group->lock);
>  
> -	vin->group->notifier->ops = &rvin_group_notify_ops;
> +	if (!vin->group->notifier.num_subdevs)
> +		return 0;
>  
> -	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
> +	vin->group->notifier.ops = &rvin_group_notify_ops;
> +	ret = v4l2_async_notifier_register(&vin->v4l2_dev,
> +					   &vin->group->notifier);
>  	if (ret < 0) {
>  		vin_err(vin, "Notifier registration failed\n");
>  		return ret;
> @@ -1114,8 +1110,10 @@ static int rcar_vin_remove(struct platform_device *pdev)
>  
>  	if (vin->info->use_mc) {
>  		mutex_lock(&vin->group->lock);
> -		if (vin->group->notifier == &vin->notifier)
> -			vin->group->notifier = NULL;
> +		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> +			v4l2_async_notifier_unregister(&vin->group->notifier);
> +			v4l2_async_notifier_cleanup(&vin->group->notifier);
> +		}
>  		mutex_unlock(&vin->group->lock);
>  		rvin_group_put(vin);
>  	} else {
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index 755ac3c..ebb480f7 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -225,8 +225,7 @@ struct rvin_dev {
>   *
>   * @lock:		protects the count, notifier, vin and csi members
>   * @count:		number of enabled VIN instances found in DT
> - * @notifier:		pointer to the notifier of a VIN which handles the
> - *			groups async sub-devices.
> + * @notifier:		group notifier for CSI-2 async subdevices
>   * @vin:		VIN instances which are part of the group
>   * @csi:		array of pairs of fwnode and subdev pointers
>   *			to all CSI-2 subdevices.
> @@ -238,7 +237,7 @@ struct rvin_group {
>  
>  	struct mutex lock;
>  	unsigned int count;
> -	struct v4l2_async_notifier *notifier;
> +	struct v4l2_async_notifier notifier;
>  	struct rvin_dev *vin[RCAR_VIN_NUM];
>  
>  	struct {
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v5 04/10] media: rcar-vin: Cleanup notifier in error path

[Jacopo Mondi]

During the notifier initialization, memory for the list of associated async
subdevices is reserved during the fwnode endpoint parsing from the v4l2-async
framework. If the notifier registration fails, that memory should be released
and the notifier 'cleaned up'.

Catch the notifier registration error and perform the cleanup both for the
group and the parallel notifiers.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
v5:
- new patch

---
 drivers/media/platform/rcar-vin/rcar-core.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index d3aadf3..f7a28e9 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -573,10 +573,15 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin)
 	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
 	if (ret < 0) {
 		vin_err(vin, "Notifier registration failed\n");
-		return ret;
+		goto error_notifier_cleanup;
 	}

 	return 0;
+
+error_notifier_cleanup:
+	v4l2_async_notifier_cleanup(&vin->group->notifier);
+
+	return ret;
 }

 /* -----------------------------------------------------------------------------
@@ -775,10 +780,15 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
 					   &vin->group->notifier);
 	if (ret < 0) {
 		vin_err(vin, "Notifier registration failed\n");
-		return ret;
+		goto error_notifier_cleanup;
 	}

 	return 0;
+
+error_notifier_cleanup:
+	v4l2_async_notifier_cleanup(&vin->group->notifier);
+
+	return ret;
 }

 static int rvin_mc_init(struct rvin_dev *vin)
--
2.7.4

Re: [PATCH v5 04/10] media: rcar-vin: Cleanup notifier in error path

[Kieran Bingham]

Hi Jacopo,

Thankyou for the patch,

On 29/05/18 09:48, Jacopo Mondi wrote:
> During the notifier initialization, memory for the list of associated async
> subdevices is reserved during the fwnode endpoint parsing from the v4l2-async
> framework. If the notifier registration fails, that memory should be released
> and the notifier 'cleaned up'.
> 
> Catch the notifier registration error and perform the cleanup both for the
> group and the parallel notifiers.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> ---
> v5:
> - new patch
> 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3aadf3..f7a28e9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -573,10 +573,15 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin)
>  	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
>  	if (ret < 0) {
>  		vin_err(vin, "Notifier registration failed\n");
> -		return ret;
> +		goto error_notifier_cleanup;
>  	}
> 
>  	return 0;
> +
> +error_notifier_cleanup:
> +	v4l2_async_notifier_cleanup(&vin->group->notifier);

Wouldn't it be less lines to just call the cleanup before the return? Or do you
anticipate multiple paths needing to call through the clean up here ?

> +
> +	return ret;
>  }
> 
>  /* -----------------------------------------------------------------------------
> @@ -775,10 +780,15 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>  					   &vin->group->notifier);
>  	if (ret < 0) {
>  		vin_err(vin, "Notifier registration failed\n");
> -		return ret;
> +		goto error_notifier_cleanup;
>  	}
> 
>  	return 0;
> +
> +error_notifier_cleanup:
> +	v4l2_async_notifier_cleanup(&vin->group->notifier);
> +

Same here...

> +	return ret;
>  }
> 
>  static int rvin_mc_init(struct rvin_dev *vin)
> --
> 2.7.4
> 

Re: [PATCH v5 04/10] media: rcar-vin: Cleanup notifier in error path

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2018-05-29 10:48:02 +0200, Jacopo Mondi wrote:
> During the notifier initialization, memory for the list of associated async
> subdevices is reserved during the fwnode endpoint parsing from the v4l2-async
> framework. If the notifier registration fails, that memory should be released
> and the notifier 'cleaned up'.
> 
> Catch the notifier registration error and perform the cleanup both for the
> group and the parallel notifiers.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

I agree with Kieran's review comment that it's better to call 
v4l2_async_notifier_cleanup() directly instead of adding a goto. With 
that fixed feel free to add

Acked-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>

> 
> ---
> v5:
> - new patch
> 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index d3aadf3..f7a28e9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -573,10 +573,15 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin)
>  	ret = v4l2_async_notifier_register(&vin->v4l2_dev, &vin->notifier);
>  	if (ret < 0) {
>  		vin_err(vin, "Notifier registration failed\n");
> -		return ret;
> +		goto error_notifier_cleanup;
>  	}
> 
>  	return 0;
> +
> +error_notifier_cleanup:
> +	v4l2_async_notifier_cleanup(&vin->group->notifier);
> +
> +	return ret;
>  }
> 
>  /* -----------------------------------------------------------------------------
> @@ -775,10 +780,15 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>  					   &vin->group->notifier);
>  	if (ret < 0) {
>  		vin_err(vin, "Notifier registration failed\n");
> -		return ret;
> +		goto error_notifier_cleanup;
>  	}
> 
>  	return 0;
> +
> +error_notifier_cleanup:
> +	v4l2_async_notifier_cleanup(&vin->group->notifier);
> +
> +	return ret;
>  }
> 
>  static int rvin_mc_init(struct rvin_dev *vin)
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v5 05/10] media: rcar-vin: Cache the mbus configuration flags

[Jacopo Mondi]

Media bus configuration flags and media bus type were so far a property
of each VIN instance, as the subdevice they were connected to was
immutable during the whole system life time.

With the forth-coming introduction of parallel input devices support,
a VIN instance can have the subdevice it is connected to switched at
runtime, from a CSI-2 subdevice to a parallel one and viceversa, through
the modification of links between media entities in the media controller
graph. To avoid discarding the per-subdevice configuration flags retrieved by
v4l2_fwnode parsing facilities, cache them in the 'rvin_graph_entity'
member of each VIN instance, opportunely renamed to 'rvin_parallel_entity'.

Also modify the register configuration function to take mbus flags into
account when running on a bus type that supports them.

The media bus type currently in use will be updated in a follow-up patch
to the link state change notification function.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 21 ++++++++-----------
 drivers/media/platform/rcar-vin/rcar-dma.c  | 32 +++++++++++++++++++----------
 drivers/media/platform/rcar-vin/rcar-vin.h  | 22 ++++++++++++++------
 3 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index f7a28e9..fc98986 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -526,30 +526,29 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 				    struct v4l2_async_subdev *asd)
 {
 	struct rvin_dev *vin = dev_get_drvdata(dev);
-	struct rvin_graph_entity *rvge =
-		container_of(asd, struct rvin_graph_entity, asd);
+	struct rvin_parallel_entity *rvpe =
+		container_of(asd, struct rvin_parallel_entity, asd);
 
 	if (vep->base.port || vep->base.id)
 		return -ENOTCONN;
 
-	vin->mbus_cfg.type = vep->bus_type;
+	vin->parallel = rvpe;
+	vin->parallel->mbus_type = vep->bus_type;
 
-	switch (vin->mbus_cfg.type) {
+	switch (vin->parallel->mbus_type) {
 	case V4L2_MBUS_PARALLEL:
 		vin_dbg(vin, "Found PARALLEL media bus\n");
-		vin->mbus_cfg.flags = vep->bus.parallel.flags;
+		vin->parallel->mbus_flags = vep->bus.parallel.flags;
 		break;
 	case V4L2_MBUS_BT656:
 		vin_dbg(vin, "Found BT656 media bus\n");
-		vin->mbus_cfg.flags = 0;
+		vin->parallel->mbus_flags = 0;
 		break;
 	default:
 		vin_err(vin, "Unknown media bus type\n");
 		return -EINVAL;
 	}
 
-	vin->parallel = rvge;
-
 	return 0;
 }
 
@@ -559,7 +558,7 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin)
 
 	ret = v4l2_async_notifier_parse_fwnode_endpoints(
 		vin->dev, &vin->notifier,
-		sizeof(struct rvin_graph_entity), rvin_parallel_parse_v4l2);
+		sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
 	if (ret)
 		return ret;
 
@@ -795,10 +794,6 @@ static int rvin_mc_init(struct rvin_dev *vin)
 {
 	int ret;
 
-	/* All our sources are CSI-2 */
-	vin->mbus_cfg.type = V4L2_MBUS_CSI2;
-	vin->mbus_cfg.flags = 0;
-
 	vin->pad.flags = MEDIA_PAD_FL_SINK;
 	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
 	if (ret)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index f1c3585..d2b7002 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -659,8 +659,12 @@ static int rvin_setup(struct rvin_dev *vin)
 		break;
 	case MEDIA_BUS_FMT_UYVY8_2X8:
 		/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
-		vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
-			VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
+		if (!vin->is_csi &&
+		    vin->parallel->mbus_type == V4L2_MBUS_BT656)
+			vnmc |= VNMC_INF_YUV8_BT656;
+		else
+			vnmc |= VNMC_INF_YUV8_BT601;
+
 		input_is_yuv = true;
 		break;
 	case MEDIA_BUS_FMT_RGB888_1X24:
@@ -668,8 +672,12 @@ static int rvin_setup(struct rvin_dev *vin)
 		break;
 	case MEDIA_BUS_FMT_UYVY10_2X10:
 		/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
-		vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
-			VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
+		if (!vin->is_csi &&
+		    vin->parallel->mbus_type == V4L2_MBUS_BT656)
+			vnmc |= VNMC_INF_YUV10_BT656;
+		else
+			vnmc |= VNMC_INF_YUV10_BT601;
+
 		input_is_yuv = true;
 		break;
 	default:
@@ -682,13 +690,15 @@ static int rvin_setup(struct rvin_dev *vin)
 	else
 		dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
 
-	/* Hsync Signal Polarity Select */
-	if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
-		dmr2 |= VNDMR2_HPS;
+	if (!vin->is_csi) {
+		/* Hsync Signal Polarity Select */
+		if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
+			dmr2 |= VNDMR2_HPS;
 
-	/* Vsync Signal Polarity Select */
-	if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
-		dmr2 |= VNDMR2_VPS;
+		/* Vsync Signal Polarity Select */
+		if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
+			dmr2 |= VNDMR2_VPS;
+	}
 
 	/*
 	 * Output format
@@ -734,7 +744,7 @@ static int rvin_setup(struct rvin_dev *vin)
 
 	if (vin->info->model == RCAR_GEN3) {
 		/* Select between CSI-2 and parallel input */
-		if (vin->mbus_cfg.type == V4L2_MBUS_CSI2)
+		if (vin->is_csi)
 			vnmc &= ~VNMC_DPINE;
 		else
 			vnmc |= VNMC_DPINE;
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index ebb480f7..8bc3704 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -73,16 +73,22 @@ struct rvin_video_format {
 };
 
 /**
- * struct rvin_graph_entity - Video endpoint from async framework
+ * struct rvin_parallel_entity - Parallel video input endpoint descriptor
  * @asd:	sub-device descriptor for async framework
  * @subdev:	subdevice matched using async framework
+ * @mbus_type:	media bus type
+ * @mbus_flags:	media bus configuration flags
  * @source_pad:	source pad of remote subdevice
  * @sink_pad:	sink pad of remote subdevice
+ *
  */
-struct rvin_graph_entity {
+struct rvin_parallel_entity {
 	struct v4l2_async_subdev asd;
 	struct v4l2_subdev *subdev;
 
+	enum v4l2_mbus_type mbus_type;
+	unsigned int mbus_flags;
+
 	unsigned int source_pad;
 	unsigned int sink_pad;
 };
@@ -146,7 +152,8 @@ struct rvin_info {
  * @v4l2_dev:		V4L2 device
  * @ctrl_handler:	V4L2 control handler
  * @notifier:		V4L2 asynchronous subdevs notifier
- * @parallel:		entity in the DT for local parallel subdevice
+ *
+ * @parallel:		parallel input subdevice descriptor
  *
  * @group:		Gen3 CSI group
  * @id:			Gen3 group id for this VIN
@@ -164,7 +171,8 @@ struct rvin_info {
  * @sequence:		V4L2 buffers sequence number
  * @state:		keeps track of operation state
  *
- * @mbus_cfg:		media bus configuration from DT
+ * @is_csi:		flag to mark the VIN as using a CSI-2 subdevice
+ *
  * @mbus_code:		media bus format code
  * @format:		active V4L2 pixel format
  *
@@ -182,7 +190,8 @@ struct rvin_dev {
 	struct v4l2_device v4l2_dev;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_async_notifier notifier;
-	struct rvin_graph_entity *parallel;
+
+	struct rvin_parallel_entity *parallel;
 
 	struct rvin_group *group;
 	unsigned int id;
@@ -199,7 +208,8 @@ struct rvin_dev {
 	unsigned int sequence;
 	enum rvin_dma_state state;
 
-	struct v4l2_mbus_config mbus_cfg;
+	bool is_csi;
+
 	u32 mbus_code;
 	struct v4l2_pix_format format;
 
-- 
2.7.4

Re: [PATCH v5 05/10] media: rcar-vin: Cache the mbus configuration flags

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 10:48:03 +0200, Jacopo Mondi wrote:
> Media bus configuration flags and media bus type were so far a property
> of each VIN instance, as the subdevice they were connected to was
> immutable during the whole system life time.
> 
> With the forth-coming introduction of parallel input devices support,
> a VIN instance can have the subdevice it is connected to switched at
> runtime, from a CSI-2 subdevice to a parallel one and viceversa, through
> the modification of links between media entities in the media controller
> graph. To avoid discarding the per-subdevice configuration flags retrieved by
> v4l2_fwnode parsing facilities, cache them in the 'rvin_graph_entity'
> member of each VIN instance, opportunely renamed to 'rvin_parallel_entity'.
> 
> Also modify the register configuration function to take mbus flags into
> account when running on a bus type that supports them.
> 
> The media bus type currently in use will be updated in a follow-up patch
> to the link state change notification function.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Acked-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 21 ++++++++-----------
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 32 +++++++++++++++++++----------
>  drivers/media/platform/rcar-vin/rcar-vin.h  | 22 ++++++++++++++------
>  3 files changed, 45 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index f7a28e9..fc98986 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -526,30 +526,29 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
>  				    struct v4l2_async_subdev *asd)
>  {
>  	struct rvin_dev *vin = dev_get_drvdata(dev);
> -	struct rvin_graph_entity *rvge =
> -		container_of(asd, struct rvin_graph_entity, asd);
> +	struct rvin_parallel_entity *rvpe =
> +		container_of(asd, struct rvin_parallel_entity, asd);
>  
>  	if (vep->base.port || vep->base.id)
>  		return -ENOTCONN;
>  
> -	vin->mbus_cfg.type = vep->bus_type;
> +	vin->parallel = rvpe;
> +	vin->parallel->mbus_type = vep->bus_type;
>  
> -	switch (vin->mbus_cfg.type) {
> +	switch (vin->parallel->mbus_type) {
>  	case V4L2_MBUS_PARALLEL:
>  		vin_dbg(vin, "Found PARALLEL media bus\n");
> -		vin->mbus_cfg.flags = vep->bus.parallel.flags;
> +		vin->parallel->mbus_flags = vep->bus.parallel.flags;
>  		break;
>  	case V4L2_MBUS_BT656:
>  		vin_dbg(vin, "Found BT656 media bus\n");
> -		vin->mbus_cfg.flags = 0;
> +		vin->parallel->mbus_flags = 0;
>  		break;
>  	default:
>  		vin_err(vin, "Unknown media bus type\n");
>  		return -EINVAL;
>  	}
>  
> -	vin->parallel = rvge;
> -
>  	return 0;
>  }
>  
> @@ -559,7 +558,7 @@ static int rvin_parallel_graph_init(struct rvin_dev *vin)
>  
>  	ret = v4l2_async_notifier_parse_fwnode_endpoints(
>  		vin->dev, &vin->notifier,
> -		sizeof(struct rvin_graph_entity), rvin_parallel_parse_v4l2);
> +		sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
>  	if (ret)
>  		return ret;
>  
> @@ -795,10 +794,6 @@ static int rvin_mc_init(struct rvin_dev *vin)
>  {
>  	int ret;
>  
> -	/* All our sources are CSI-2 */
> -	vin->mbus_cfg.type = V4L2_MBUS_CSI2;
> -	vin->mbus_cfg.flags = 0;
> -
>  	vin->pad.flags = MEDIA_PAD_FL_SINK;
>  	ret = media_entity_pads_init(&vin->vdev.entity, 1, &vin->pad);
>  	if (ret)
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index f1c3585..d2b7002 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -659,8 +659,12 @@ static int rvin_setup(struct rvin_dev *vin)
>  		break;
>  	case MEDIA_BUS_FMT_UYVY8_2X8:
>  		/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */
> -		vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
> -			VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601;
> +		if (!vin->is_csi &&
> +		    vin->parallel->mbus_type == V4L2_MBUS_BT656)
> +			vnmc |= VNMC_INF_YUV8_BT656;
> +		else
> +			vnmc |= VNMC_INF_YUV8_BT601;
> +
>  		input_is_yuv = true;
>  		break;
>  	case MEDIA_BUS_FMT_RGB888_1X24:
> @@ -668,8 +672,12 @@ static int rvin_setup(struct rvin_dev *vin)
>  		break;
>  	case MEDIA_BUS_FMT_UYVY10_2X10:
>  		/* BT.656 10bit YCbCr422 or BT.601 10bit YCbCr422 */
> -		vnmc |= vin->mbus_cfg.type == V4L2_MBUS_BT656 ?
> -			VNMC_INF_YUV10_BT656 : VNMC_INF_YUV10_BT601;
> +		if (!vin->is_csi &&
> +		    vin->parallel->mbus_type == V4L2_MBUS_BT656)
> +			vnmc |= VNMC_INF_YUV10_BT656;
> +		else
> +			vnmc |= VNMC_INF_YUV10_BT601;
> +
>  		input_is_yuv = true;
>  		break;
>  	default:
> @@ -682,13 +690,15 @@ static int rvin_setup(struct rvin_dev *vin)
>  	else
>  		dmr2 = VNDMR2_FTEV | VNDMR2_VLV(1);
>  
> -	/* Hsync Signal Polarity Select */
> -	if (!(vin->mbus_cfg.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> -		dmr2 |= VNDMR2_HPS;
> +	if (!vin->is_csi) {
> +		/* Hsync Signal Polarity Select */
> +		if (!(vin->parallel->mbus_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW))
> +			dmr2 |= VNDMR2_HPS;
>  
> -	/* Vsync Signal Polarity Select */
> -	if (!(vin->mbus_cfg.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> -		dmr2 |= VNDMR2_VPS;
> +		/* Vsync Signal Polarity Select */
> +		if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> +			dmr2 |= VNDMR2_VPS;
> +	}
>  
>  	/*
>  	 * Output format
> @@ -734,7 +744,7 @@ static int rvin_setup(struct rvin_dev *vin)
>  
>  	if (vin->info->model == RCAR_GEN3) {
>  		/* Select between CSI-2 and parallel input */
> -		if (vin->mbus_cfg.type == V4L2_MBUS_CSI2)
> +		if (vin->is_csi)
>  			vnmc &= ~VNMC_DPINE;
>  		else
>  			vnmc |= VNMC_DPINE;
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index ebb480f7..8bc3704 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -73,16 +73,22 @@ struct rvin_video_format {
>  };
>  
>  /**
> - * struct rvin_graph_entity - Video endpoint from async framework
> + * struct rvin_parallel_entity - Parallel video input endpoint descriptor
>   * @asd:	sub-device descriptor for async framework
>   * @subdev:	subdevice matched using async framework
> + * @mbus_type:	media bus type
> + * @mbus_flags:	media bus configuration flags
>   * @source_pad:	source pad of remote subdevice
>   * @sink_pad:	sink pad of remote subdevice
> + *
>   */
> -struct rvin_graph_entity {
> +struct rvin_parallel_entity {
>  	struct v4l2_async_subdev asd;
>  	struct v4l2_subdev *subdev;
>  
> +	enum v4l2_mbus_type mbus_type;
> +	unsigned int mbus_flags;
> +
>  	unsigned int source_pad;
>  	unsigned int sink_pad;
>  };
> @@ -146,7 +152,8 @@ struct rvin_info {
>   * @v4l2_dev:		V4L2 device
>   * @ctrl_handler:	V4L2 control handler
>   * @notifier:		V4L2 asynchronous subdevs notifier
> - * @parallel:		entity in the DT for local parallel subdevice
> + *
> + * @parallel:		parallel input subdevice descriptor
>   *
>   * @group:		Gen3 CSI group
>   * @id:			Gen3 group id for this VIN
> @@ -164,7 +171,8 @@ struct rvin_info {
>   * @sequence:		V4L2 buffers sequence number
>   * @state:		keeps track of operation state
>   *
> - * @mbus_cfg:		media bus configuration from DT
> + * @is_csi:		flag to mark the VIN as using a CSI-2 subdevice
> + *
>   * @mbus_code:		media bus format code
>   * @format:		active V4L2 pixel format
>   *
> @@ -182,7 +190,8 @@ struct rvin_dev {
>  	struct v4l2_device v4l2_dev;
>  	struct v4l2_ctrl_handler ctrl_handler;
>  	struct v4l2_async_notifier notifier;
> -	struct rvin_graph_entity *parallel;
> +
> +	struct rvin_parallel_entity *parallel;
>  
>  	struct rvin_group *group;
>  	unsigned int id;
> @@ -199,7 +208,8 @@ struct rvin_dev {
>  	unsigned int sequence;
>  	enum rvin_dma_state state;
>  
> -	struct v4l2_mbus_config mbus_cfg;
> +	bool is_csi;
> +
>  	u32 mbus_code;
>  	struct v4l2_pix_format format;
>  
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v5 06/10] media: rcar-vin: Parse parallel input on Gen3

[Jacopo Mondi]

The rcar-vin driver so far had a mutually exclusive code path for
handling parallel and CSI-2 video input subdevices, with only the CSI-2
use case supporting media-controller. As we add support for parallel
inputs to Gen3 media-controller compliant code path now parse both port@0
and port@1, handling the media-controller use case in the parallel
bound/unbind notifier operations.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

---
v4 -> v5:
- Re-group rvin_mc_init() function
- Add error_group_unreg error path to clean up group registration
- Change rvin_parallel_init() return type to make sure Gen2 works as before

v3 -> v4:
- Change the mc/parallel initialization order. Initialize mc first, then
  parallel
- As a consequence no need to delay parallel notifiers registration, the
  media controller is set up already when parallel input got parsed,
  this greatly simplify the group notifier complete callback.

---
 drivers/media/platform/rcar-vin/rcar-core.c | 53 +++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index fc98986..c1d6feb 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -399,6 +399,11 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
 	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;
@@ -460,10 +465,12 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
 static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
 {
 	rvin_v4l2_unregister(vin);
-	v4l2_ctrl_handler_free(&vin->ctrl_handler);
-
-	vin->vdev.ctrl_handler = NULL;
 	vin->parallel->subdev = NULL;
+
+	if (!vin->info->use_mc) {
+		v4l2_ctrl_handler_free(&vin->ctrl_handler);
+		vin->vdev.ctrl_handler = NULL;
+	}
 }

 static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
@@ -552,18 +559,19 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
 	return 0;
 }

-static int rvin_parallel_graph_init(struct rvin_dev *vin)
+static int rvin_parallel_init(struct rvin_dev *vin)
 {
 	int ret;

-	ret = v4l2_async_notifier_parse_fwnode_endpoints(
-		vin->dev, &vin->notifier,
-		sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
+	ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
+		vin->dev, &vin->notifier, sizeof(struct rvin_parallel_entity),
+		0, rvin_parallel_parse_v4l2);
 	if (ret)
 		return ret;

+	/* If using mc, it's fine not to have any input registered. */
 	if (!vin->parallel)
-		return -ENODEV;
+		return vin->info->use_mc ? 0 : -ENODEV;

 	vin_dbg(vin, "Found parallel subdevice %pOF\n",
 		to_of_node(vin->parallel->asd.match.fwnode));
@@ -1084,20 +1092,35 @@ static int rcar_vin_probe(struct platform_device *pdev)
 		return ret;

 	platform_set_drvdata(pdev, vin);
-	if (vin->info->use_mc)
+
+	if (vin->info->use_mc) {
 		ret = rvin_mc_init(vin);
-	else
-		ret = rvin_parallel_graph_init(vin);
-	if (ret < 0)
-		goto error;
+		if (ret)
+			goto error_dma_unregister;
+	}
+
+	ret = rvin_parallel_init(vin);
+	if (ret)
+		goto error_group_unregister;

 	pm_suspend_ignore_children(&pdev->dev, true);
 	pm_runtime_enable(&pdev->dev);

 	return 0;
-error:
+
+error_group_unreg:
+	if (vin->info->use_mc) {
+		mutex_lock(&vin->group->lock);
+		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
+			v4l2_async_notifier_unregister(&vin->group->notifier);
+			v4l2_async_notifier_cleanup(&vin->group->notifier);
+		}
+		mutex_unlock(&vin->group->lock);
+		rvin_group_put(vin);
+	}
+
+error_dma_unreg:
 	rvin_dma_unregister(vin);
-	v4l2_async_notifier_cleanup(&vin->notifier);

 	return ret;
 }
--
2.7.4

Re: [PATCH v5 06/10] media: rcar-vin: Parse parallel input on Gen3

[jacopo mondi]

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

Hi,

On Tue, May 29, 2018 at 10:48:04AM +0200, Jacopo Mondi wrote:
> The rcar-vin driver so far had a mutually exclusive code path for
> handling parallel and CSI-2 video input subdevices, with only the CSI-2
> use case supporting media-controller. As we add support for parallel
> inputs to Gen3 media-controller compliant code path now parse both port@0
> and port@1, handling the media-controller use case in the parallel
> bound/unbind notifier operations.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> ---
> v4 -> v5:
> - Re-group rvin_mc_init() function
> - Add error_group_unreg error path to clean up group registration
> - Change rvin_parallel_init() return type to make sure Gen2 works as before
>
> v3 -> v4:
> - Change the mc/parallel initialization order. Initialize mc first, then
>   parallel
> - As a consequence no need to delay parallel notifiers registration, the
>   media controller is set up already when parallel input got parsed,
>   this greatly simplify the group notifier complete callback.
>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 53 +++++++++++++++++++++--------
>  1 file changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index fc98986..c1d6feb 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -399,6 +399,11 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  	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;
> @@ -460,10 +465,12 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
>  {
>  	rvin_v4l2_unregister(vin);
> -	v4l2_ctrl_handler_free(&vin->ctrl_handler);
> -
> -	vin->vdev.ctrl_handler = NULL;
>  	vin->parallel->subdev = NULL;
> +
> +	if (!vin->info->use_mc) {
> +		v4l2_ctrl_handler_free(&vin->ctrl_handler);
> +		vin->vdev.ctrl_handler = NULL;
> +	}
>  }
>
>  static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
> @@ -552,18 +559,19 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
>  	return 0;
>  }
>
> -static int rvin_parallel_graph_init(struct rvin_dev *vin)
> +static int rvin_parallel_init(struct rvin_dev *vin)
>  {
>  	int ret;
>
> -	ret = v4l2_async_notifier_parse_fwnode_endpoints(
> -		vin->dev, &vin->notifier,
> -		sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
> +	ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> +		vin->dev, &vin->notifier, sizeof(struct rvin_parallel_entity),
> +		0, rvin_parallel_parse_v4l2);
>  	if (ret)
>  		return ret;
>
> +	/* If using mc, it's fine not to have any input registered. */
>  	if (!vin->parallel)
> -		return -ENODEV;
> +		return vin->info->use_mc ? 0 : -ENODEV;
>
>  	vin_dbg(vin, "Found parallel subdevice %pOF\n",
>  		to_of_node(vin->parallel->asd.match.fwnode));
> @@ -1084,20 +1092,35 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  		return ret;
>
>  	platform_set_drvdata(pdev, vin);
> -	if (vin->info->use_mc)
> +
> +	if (vin->info->use_mc) {
>  		ret = rvin_mc_init(vin);
> -	else
> -		ret = rvin_parallel_graph_init(vin);
> -	if (ret < 0)
> -		goto error;
> +		if (ret)
> +			goto error_dma_unregister;
> +	}
> +
> +	ret = rvin_parallel_init(vin);
> +	if (ret)
> +		goto error_group_unregister;
>
>  	pm_suspend_ignore_children(&pdev->dev, true);
>  	pm_runtime_enable(&pdev->dev);
>
>  	return 0;
> -error:
> +
> +error_group_unreg:

I just noticed I forgot to add a change before formatting out patches.
This label name is wrong.

I'll wait for other comments, and will send v6 with this fixed. Sorry
about that.

Thanks
   j

> +	if (vin->info->use_mc) {
> +		mutex_lock(&vin->group->lock);
> +		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> +			v4l2_async_notifier_unregister(&vin->group->notifier);
> +			v4l2_async_notifier_cleanup(&vin->group->notifier);
> +		}
> +		mutex_unlock(&vin->group->lock);
> +		rvin_group_put(vin);
> +	}
> +
> +error_dma_unreg:
>  	rvin_dma_unregister(vin);
> -	v4l2_async_notifier_cleanup(&vin->notifier);
>
>  	return ret;
>  }
> --
> 2.7.4
>

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

Re: [PATCH v5 06/10] media: rcar-vin: Parse parallel input on Gen3

[kbuild test robot]

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

Hi Jacopo,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20180530]
[cannot apply to v4.17-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/rcar-vin-Add-support-for-parallel-input-on-Gen3/20180531-182328
base:   git://linuxtv.org/media_tree.git master
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media//platform/rcar-vin/rcar-core.c: In function 'rcar_vin_probe':
   drivers/media//platform/rcar-vin/rcar-core.c:1122:1: warning: label 'error_dma_unreg' defined but not used [-Wunused-label]
    error_dma_unreg:
    ^~~~~~~~~~~~~~~
   drivers/media//platform/rcar-vin/rcar-core.c:1111:1: warning: label 'error_group_unreg' defined but not used [-Wunused-label]
    error_group_unreg:
    ^~~~~~~~~~~~~~~~~
>> drivers/media//platform/rcar-vin/rcar-core.c:1104:3: error: label 'error_group_unregister' used but not defined
      goto error_group_unregister;
      ^~~~
>> drivers/media//platform/rcar-vin/rcar-core.c:1099:4: error: label 'error_dma_unregister' used but not defined
       goto error_dma_unregister;
       ^~~~

sparse warnings: (new ones prefixed by >>)

>> drivers/media/platform/rcar-vin/rcar-core.c:1099:25: sparse: label 'error_dma_unregister' was not declared
>> drivers/media/platform/rcar-vin/rcar-core.c:1104:17: sparse: label 'error_group_unregister' was not declared
   drivers/media/platform/rcar-vin/rcar-core.c: In function 'rcar_vin_probe':
   drivers/media/platform/rcar-vin/rcar-core.c:1122:1: warning: label 'error_dma_unreg' defined but not used [-Wunused-label]
    error_dma_unreg:
    ^~~~~~~~~~~~~~~
   drivers/media/platform/rcar-vin/rcar-core.c:1111:1: warning: label 'error_group_unreg' defined but not used [-Wunused-label]
    error_group_unreg:
    ^~~~~~~~~~~~~~~~~
   drivers/media/platform/rcar-vin/rcar-core.c:1104:3: error: label 'error_group_unregister' used but not defined
      goto error_group_unregister;
      ^~~~
   drivers/media/platform/rcar-vin/rcar-core.c:1099:4: error: label 'error_dma_unregister' used but not defined
       goto error_dma_unregister;
       ^~~~

vim +/error_group_unregister +1104 drivers/media//platform/rcar-vin/rcar-core.c

  1055	
  1056	static int rcar_vin_probe(struct platform_device *pdev)
  1057	{
  1058		const struct soc_device_attribute *attr;
  1059		struct rvin_dev *vin;
  1060		struct resource *mem;
  1061		int irq, ret;
  1062	
  1063		vin = devm_kzalloc(&pdev->dev, sizeof(*vin), GFP_KERNEL);
  1064		if (!vin)
  1065			return -ENOMEM;
  1066	
  1067		vin->dev = &pdev->dev;
  1068		vin->info = of_device_get_match_data(&pdev->dev);
  1069	
  1070		/*
  1071		 * Special care is needed on r8a7795 ES1.x since it
  1072		 * uses different routing than r8a7795 ES2.0.
  1073		 */
  1074		attr = soc_device_match(r8a7795es1);
  1075		if (attr)
  1076			vin->info = attr->data;
  1077	
  1078		mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1079		if (mem == NULL)
  1080			return -EINVAL;
  1081	
  1082		vin->base = devm_ioremap_resource(vin->dev, mem);
  1083		if (IS_ERR(vin->base))
  1084			return PTR_ERR(vin->base);
  1085	
  1086		irq = platform_get_irq(pdev, 0);
  1087		if (irq < 0)
  1088			return irq;
  1089	
  1090		ret = rvin_dma_register(vin, irq);
  1091		if (ret)
  1092			return ret;
  1093	
  1094		platform_set_drvdata(pdev, vin);
  1095	
  1096		if (vin->info->use_mc) {
  1097			ret = rvin_mc_init(vin);
  1098			if (ret)
> 1099				goto error_dma_unregister;
  1100		}
  1101	
  1102		ret = rvin_parallel_init(vin);
  1103		if (ret)
> 1104			goto error_group_unregister;
  1105	
  1106		pm_suspend_ignore_children(&pdev->dev, true);
  1107		pm_runtime_enable(&pdev->dev);
  1108	
  1109		return 0;
  1110	
  1111	error_group_unreg:
  1112		if (vin->info->use_mc) {
  1113			mutex_lock(&vin->group->lock);
  1114			if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
  1115				v4l2_async_notifier_unregister(&vin->group->notifier);
  1116				v4l2_async_notifier_cleanup(&vin->group->notifier);
  1117			}
  1118			mutex_unlock(&vin->group->lock);
  1119			rvin_group_put(vin);
  1120		}
  1121	
> 1122	error_dma_unreg:
  1123		rvin_dma_unregister(vin);
  1124	
  1125		return ret;
  1126	}
  1127	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63754 bytes --]

Re: [PATCH v5 06/10] media: rcar-vin: Parse parallel input on Gen3

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2018-05-29 10:48:04 +0200, Jacopo Mondi wrote:
> The rcar-vin driver so far had a mutually exclusive code path for
> handling parallel and CSI-2 video input subdevices, with only the CSI-2
> use case supporting media-controller. As we add support for parallel
> inputs to Gen3 media-controller compliant code path now parse both port@0
> and port@1, handling the media-controller use case in the parallel
> bound/unbind notifier operations.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

I think this patch looks fine except the label name mismatch. I think 
it's ready but will hold off with my tag until I can review a complete 
version of the patch :-)

> 
> ---
> v4 -> v5:
> - Re-group rvin_mc_init() function
> - Add error_group_unreg error path to clean up group registration
> - Change rvin_parallel_init() return type to make sure Gen2 works as before
> 
> v3 -> v4:
> - Change the mc/parallel initialization order. Initialize mc first, then
>   parallel
> - As a consequence no need to delay parallel notifiers registration, the
>   media controller is set up already when parallel input got parsed,
>   this greatly simplify the group notifier complete callback.
> 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 53 +++++++++++++++++++++--------
>  1 file changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index fc98986..c1d6feb 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -399,6 +399,11 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  	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;
> @@ -460,10 +465,12 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
>  {
>  	rvin_v4l2_unregister(vin);
> -	v4l2_ctrl_handler_free(&vin->ctrl_handler);
> -
> -	vin->vdev.ctrl_handler = NULL;
>  	vin->parallel->subdev = NULL;
> +
> +	if (!vin->info->use_mc) {
> +		v4l2_ctrl_handler_free(&vin->ctrl_handler);
> +		vin->vdev.ctrl_handler = NULL;
> +	}
>  }
> 
>  static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
> @@ -552,18 +559,19 @@ static int rvin_parallel_parse_v4l2(struct device *dev,
>  	return 0;
>  }
> 
> -static int rvin_parallel_graph_init(struct rvin_dev *vin)
> +static int rvin_parallel_init(struct rvin_dev *vin)
>  {
>  	int ret;
> 
> -	ret = v4l2_async_notifier_parse_fwnode_endpoints(
> -		vin->dev, &vin->notifier,
> -		sizeof(struct rvin_parallel_entity), rvin_parallel_parse_v4l2);
> +	ret = v4l2_async_notifier_parse_fwnode_endpoints_by_port(
> +		vin->dev, &vin->notifier, sizeof(struct rvin_parallel_entity),
> +		0, rvin_parallel_parse_v4l2);
>  	if (ret)
>  		return ret;
> 
> +	/* If using mc, it's fine not to have any input registered. */
>  	if (!vin->parallel)
> -		return -ENODEV;
> +		return vin->info->use_mc ? 0 : -ENODEV;
> 
>  	vin_dbg(vin, "Found parallel subdevice %pOF\n",
>  		to_of_node(vin->parallel->asd.match.fwnode));
> @@ -1084,20 +1092,35 @@ static int rcar_vin_probe(struct platform_device *pdev)
>  		return ret;
> 
>  	platform_set_drvdata(pdev, vin);
> -	if (vin->info->use_mc)
> +
> +	if (vin->info->use_mc) {
>  		ret = rvin_mc_init(vin);
> -	else
> -		ret = rvin_parallel_graph_init(vin);
> -	if (ret < 0)
> -		goto error;
> +		if (ret)
> +			goto error_dma_unregister;
> +	}
> +
> +	ret = rvin_parallel_init(vin);
> +	if (ret)
> +		goto error_group_unregister;
> 
>  	pm_suspend_ignore_children(&pdev->dev, true);
>  	pm_runtime_enable(&pdev->dev);
> 
>  	return 0;
> -error:
> +
> +error_group_unreg:
> +	if (vin->info->use_mc) {
> +		mutex_lock(&vin->group->lock);
> +		if (&vin->v4l2_dev == vin->group->notifier.v4l2_dev) {
> +			v4l2_async_notifier_unregister(&vin->group->notifier);
> +			v4l2_async_notifier_cleanup(&vin->group->notifier);
> +		}
> +		mutex_unlock(&vin->group->lock);
> +		rvin_group_put(vin);
> +	}
> +
> +error_dma_unreg:
>  	rvin_dma_unregister(vin);
> -	v4l2_async_notifier_cleanup(&vin->notifier);
> 
>  	return ret;
>  }
> --
> 2.7.4
> 

-- 
Regards,
Niklas S�derlund

[PATCH v5 07/10] media: rcar-vin: Link parallel input media entities

[Jacopo Mondi]

When running with media-controller link the parallel input
media entities with the VIN entities at 'complete' callback time.

To create media links the v4l2_device should be registered first.
Check if the device is already registered, to avoid double registrations.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index c1d6feb..66066a0 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -476,6 +476,8 @@ static void rvin_parallel_subdevice_detach(struct rvin_dev *vin)
 static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
 {
 	struct rvin_dev *vin = notifier_to_vin(notifier);
+	struct media_entity *source;
+	struct media_entity *sink;
 	int ret;
 
 	ret = v4l2_device_register_subdev_nodes(&vin->v4l2_dev);
@@ -484,7 +486,26 @@ static int rvin_parallel_notify_complete(struct v4l2_async_notifier *notifier)
 		return ret;
 	}
 
-	return rvin_v4l2_register(vin);
+	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,
@@ -610,7 +631,8 @@ static int rvin_group_notify_complete(struct v4l2_async_notifier *notifier)
 
 	/* Register all video nodes for the group. */
 	for (i = 0; i < RCAR_VIN_NUM; i++) {
-		if (vin->group->vin[i]) {
+		if (vin->group->vin[i] &&
+		    !video_is_registered(&vin->group->vin[i]->vdev)) {
 			ret = rvin_v4l2_register(vin->group->vin[i]);
 			if (ret)
 				return ret;
-- 
2.7.4

[PATCH v5 08/10] media: rcar-vin: Handle parallel subdev in link_notify

[Jacopo Mondi]

Handle parallel subdevices in link_notify callback. If the notified link
involves a parallel subdevice, do not change routing of the VIN-CSI-2
devices and mark the VIN instance as using a parallel input. If the
CSI-2 link setup succeeds instead, mark the VIN instance as using CSI-2.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 35 ++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 66066a0..e353ba9 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -171,9 +171,37 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
 
 	/* Add the new link to the existing mask and check if it works. */
 	csi_id = rvin_group_entity_to_csi_id(group, link->source->entity);
+
+	if (csi_id == -ENODEV) {
+		struct v4l2_subdev *sd;
+		unsigned int i;
+
+		/*
+		 * Make sure the source entity subdevice is registered as
+		 * a parallel input of one of the enabled VINs if it is not
+		 * one of the CSI-2 subdevices.
+		 *
+		 * No hardware configuration required for parallel inputs,
+		 * we can return here.
+		 */
+		sd = media_entity_to_v4l2_subdev(link->source->entity);
+		for (i = 0; i < RCAR_VIN_NUM; i++) {
+			if (group->vin[i] && group->vin[i]->parallel &&
+			    group->vin[i]->parallel->subdev == sd) {
+				group->vin[i]->is_csi = false;
+				ret = 0;
+				goto out;
+			}
+		}
+
+		vin_err(vin, "Subdevice %s not registered to any VIN\n",
+			link->source->entity->name);
+		ret = -ENODEV;
+		goto out;
+	}
+
 	channel = rvin_group_csi_pad_to_channel(link->source->index);
 	mask_new = mask & rvin_group_get_mask(vin, csi_id, channel);
-
 	vin_dbg(vin, "Try link change mask: 0x%x new: 0x%x\n", mask, mask_new);
 
 	if (!mask_new) {
@@ -183,6 +211,11 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
 
 	/* New valid CHSEL found, set the new value. */
 	ret = rvin_set_channel_routing(group->vin[master_id], __ffs(mask_new));
+	if (ret)
+		goto out;
+
+	vin->is_csi = true;
+
 out:
 	mutex_unlock(&group->lock);
 
-- 
2.7.4

[PATCH v5 09/10] media: rcar-vin: Rename _rcar_info to rcar_info

[Jacopo Mondi]

Remove leading underscore to align all rcar_group_route structure
declarations.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index e353ba9..7869308 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1036,7 +1036,7 @@ static const struct rvin_info rcar_info_r8a7796 = {
 	.routes = rcar_info_r8a7796_routes,
 };
 
-static const struct rvin_group_route _rcar_info_r8a77970_routes[] = {
+static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
 	{ .csi = RVIN_CSI40, .channel = 0, .vin = 0, .mask = BIT(0) | BIT(3) },
 	{ .csi = RVIN_CSI40, .channel = 0, .vin = 1, .mask = BIT(2) },
 	{ .csi = RVIN_CSI40, .channel = 1, .vin = 1, .mask = BIT(3) },
@@ -1052,7 +1052,7 @@ static const struct rvin_info rcar_info_r8a77970 = {
 	.use_mc = true,
 	.max_width = 4096,
 	.max_height = 4096,
-	.routes = _rcar_info_r8a77970_routes,
+	.routes = rcar_info_r8a77970_routes,
 };
 
 static const struct of_device_id rvin_of_id_table[] = {
-- 
2.7.4

[PATCH v5 10/10] media: rcar-vin: Add support for R-Car R8A77995 SoC

[Jacopo Mondi]

Add R-Car R8A77995 SoC to the rcar-vin supported ones.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 7869308..3062171 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -1055,6 +1055,18 @@ static const struct rvin_info rcar_info_r8a77970 = {
 	.routes = rcar_info_r8a77970_routes,
 };
 
+static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
+	{ /* Sentinel */ }
+};
+
+static const struct rvin_info rcar_info_r8a77995 = {
+	.model = RCAR_GEN3,
+	.use_mc = true,
+	.max_width = 4096,
+	.max_height = 4096,
+	.routes = rcar_info_r8a77995_routes,
+};
+
 static const struct of_device_id rvin_of_id_table[] = {
 	{
 		.compatible = "renesas,vin-r8a7778",
@@ -1096,6 +1108,10 @@ static const struct of_device_id rvin_of_id_table[] = {
 		.compatible = "renesas,vin-r8a77970",
 		.data = &rcar_info_r8a77970,
 	},
+	{
+		.compatible = "renesas,vin-r8a77995",
+		.data = &rcar_info_r8a77995,
+	},
 	{ /* Sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, rvin_of_id_table);
-- 
2.7.4