[PATCH 0/2] media: rcar-vin: Add "renesas,id" to V3H

[PATCH 0/2] media: rcar-vin: Add "renesas,id" to V3H

[Jacopo Mondi]

These two patches add the mandatory "renesas,id" property to
V3H device tree.

As the property is mandatory and the driver checks for its presence,
this might actually fix VIN probe on V3H boards.

In order to support V3H, which has 16 VIN instances, modify the way the
VIN driver handles the maximum number of VIN, making it a per-SoC property.

As explained in the commit message of [2/2] if anyone with an V3H board could
provide a Tested-by tag, these two patches might actually be tagged as fixes.

Tested on Salvator-x M3-W and Ebisu E3.

Thanks
  j

Jacopo Mondi (2):
  media: rcar-vin: Make the number of VIN SoC-dependent
  arm64: dts: renesas: r8a77980: Add "renesas,id" to VIN

 arch/arm64/boot/dts/renesas/r8a77980.dtsi   | 16 ++++++++++++++
 drivers/media/platform/rcar-vin/rcar-core.c | 23 +++++++++++++++------
 drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++++-
 3 files changed, 37 insertions(+), 7 deletions(-)

--
2.21.0

[PATCH 1/2] media: rcar-vin: Make the number of VIN SoC-dependent

[Jacopo Mondi]

In preparation to add ids to VIN's device tree nodes for the V3H SoC
which features up to 16 VIN instances, remove the hardcoded number of
VINs and make it a per-SoC property.

Enlarge the existing RCAR_VIN_NUM definition, which is used to
statically allocate the number of vin devices to 16, to accommodate
SoCs with that many instances as V3H.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/platform/rcar-vin/rcar-core.c | 23 +++++++++++++++------
 drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++++-
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 64519b3097f7..6e9c6c8471e9 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -188,7 +188,7 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
 		 * we can return here.
 		 */
 		sd = media_entity_to_v4l2_subdev(link->source->entity);
-		for (i = 0; i < RCAR_VIN_NUM; i++) {
+		for (i = 0; i <= vin->info->vin_max_id ; i++) {
 			if (group->vin[i] && group->vin[i]->parallel &&
 			    group->vin[i]->parallel->subdev == sd) {
 				group->vin[i]->is_csi = false;
@@ -319,7 +319,7 @@ static int rvin_group_get(struct rvin_dev *vin)
 		return -EINVAL;
 	}

-	if (id >= RCAR_VIN_NUM) {
+	if (id > vin->info->vin_max_id) {
 		vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
 			vin->dev->of_node, id);
 		return -EINVAL;
@@ -662,7 +662,7 @@ 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++) {
+	for (i = 0; i <= vin->info->vin_max_id; i++) {
 		if (vin->group->vin[i] &&
 		    !video_is_registered(&vin->group->vin[i]->vdev)) {
 			ret = rvin_v4l2_register(vin->group->vin[i]);
@@ -720,7 +720,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
 	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
 	unsigned int i;

-	for (i = 0; i < RCAR_VIN_NUM; i++)
+	for (i = 0; i <= vin->info->vin_max_id; i++)
 		if (vin->group->vin[i])
 			rvin_v4l2_unregister(vin->group->vin[i]);

@@ -809,7 +809,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
 	mutex_lock(&vin->group->lock);

 	/* If not all VIN's are registered don't register the notifier. */
-	for (i = 0; i < RCAR_VIN_NUM; i++) {
+	for (i = 0; i <= vin->info->vin_max_id; i++) {
 		if (vin->group->vin[i]) {
 			count++;
 			vin_mask |= BIT(i);
@@ -830,7 +830,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
 	 * overlap but the parser function can handle it, so each subdevice
 	 * will only be registered once with the group notifier.
 	 */
-	for (i = 0; i < RCAR_VIN_NUM; i++) {
+	for (i = 0; i <= vin->info->vin_max_id ; i++) {
 		if (!(vin_mask & BIT(i)))
 			continue;

@@ -884,6 +884,7 @@ static int rvin_mc_init(struct rvin_dev *vin)
 static const struct rvin_info rcar_info_h1 = {
 	.model = RCAR_H1,
 	.use_mc = false,
+	.vin_max_id = 3,
 	.max_width = 2048,
 	.max_height = 2048,
 };
@@ -891,6 +892,7 @@ static const struct rvin_info rcar_info_h1 = {
 static const struct rvin_info rcar_info_m1 = {
 	.model = RCAR_M1,
 	.use_mc = false,
+	.vin_max_id = 1,
 	.max_width = 2048,
 	.max_height = 2048,
 };
@@ -898,6 +900,7 @@ static const struct rvin_info rcar_info_m1 = {
 static const struct rvin_info rcar_info_gen2 = {
 	.model = RCAR_GEN2,
 	.use_mc = false,
+	.vin_max_id = 5,
 	.max_width = 2048,
 	.max_height = 2048,
 };
@@ -941,6 +944,7 @@ static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
 static const struct rvin_info rcar_info_r8a7795 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 7,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a7795_routes,
@@ -995,6 +999,7 @@ static const struct rvin_group_route rcar_info_r8a7795es1_routes[] = {
 static const struct rvin_info rcar_info_r8a7795es1 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 7,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a7795es1_routes,
@@ -1035,6 +1040,7 @@ static const struct rvin_group_route rcar_info_r8a7796_routes[] = {
 static const struct rvin_info rcar_info_r8a7796 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 7,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a7796_routes,
@@ -1079,6 +1085,7 @@ static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
 static const struct rvin_info rcar_info_r8a77965 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 7,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77965_routes,
@@ -1098,6 +1105,7 @@ static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
 static const struct rvin_info rcar_info_r8a77970 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 3,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77970_routes,
@@ -1126,6 +1134,7 @@ static const struct rvin_group_route rcar_info_r8a77980_routes[] = {
 static const struct rvin_info rcar_info_r8a77980 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 15,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77980_routes,
@@ -1142,6 +1151,7 @@ static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
 static const struct rvin_info rcar_info_r8a77990 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 5,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77990_routes,
@@ -1154,6 +1164,7 @@ static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
 static const struct rvin_info rcar_info_r8a77995 = {
 	.model = RCAR_GEN3,
 	.use_mc = true,
+	.vin_max_id = 4,
 	.max_width = 4096,
 	.max_height = 4096,
 	.routes = rcar_info_r8a77995_routes,
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 0b13b34d03e3..2726c56935e4 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -28,7 +28,8 @@
 #define HW_BUFFER_MASK 0x7f

 /* Max number on VIN instances that can be in a system */
-#define RCAR_VIN_NUM 8
+#define RCAR_VIN_NUM 16
+

 struct rvin_group;

@@ -126,6 +127,7 @@ 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
+ * @vin_max_id:		id of the last VIN instance
  * @max_width:		max input width the VIN supports
  * @max_height:		max input height the VIN supports
  * @routes:		list of possible routes from the CSI-2 recivers to
@@ -135,6 +137,7 @@ struct rvin_info {
 	enum model_id model;
 	bool use_mc;

+	unsigned int vin_max_id;
 	unsigned int max_width;
 	unsigned int max_height;
 	const struct rvin_group_route *routes;
--
2.21.0

[PATCH 2/2] arm64: dts: renesas: r8a77980: Add "renesas,id" to VIN

[Jacopo Mondi]

Add the "renesas,id" property to VIN nodes in the R-Car V3H R8A77980
device tree.

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

I do not have a V3H board to test on, but I suspect VIN failed to probe
on V3H before these 2 patches.

If that's the case, these patches would merit
Fixes: 3182aa4e0bf4 ("arm64: dts: renesas: r8a77980: add CSI2/VIN support")

Sergei, do you still have a V3H board around? Could I have your Tested-by
tag if things are fine now?

Thanks
  j
---
 arch/arm64/boot/dts/renesas/r8a77980.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a77980.dtsi b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
index 4081622d548a..a901a341dcf7 100644
--- a/arch/arm64/boot/dts/renesas/r8a77980.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
@@ -865,6 +865,7 @@
 			clocks = <&cpg CPG_MOD 811>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 811>;
+			renesas,id = <0>;
 			status = "disabled";

 			ports {
@@ -892,6 +893,7 @@
 			clocks = <&cpg CPG_MOD 810>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			status = "disabled";
+			renesas,id = <1>;
 			resets = <&cpg 810>;

 			ports {
@@ -919,6 +921,7 @@
 			clocks = <&cpg CPG_MOD 809>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 809>;
+			renesas,id = <2>;
 			status = "disabled";

 			ports {
@@ -946,6 +949,7 @@
 			clocks = <&cpg CPG_MOD 808>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 808>;
+			renesas,id = <3>;
 			status = "disabled";

 			ports {
@@ -973,6 +977,7 @@
 			clocks = <&cpg CPG_MOD 807>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 807>;
+			renesas,id = <4>;
 			status = "disabled";

 			ports {
@@ -1000,6 +1005,7 @@
 			clocks = <&cpg CPG_MOD 806>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 806>;
+			renesas,id = <5>;
 			status = "disabled";

 			ports {
@@ -1027,6 +1033,7 @@
 			clocks = <&cpg CPG_MOD 805>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 805>;
+			renesas,id = <6>;
 			status = "disabled";

 			ports {
@@ -1054,6 +1061,7 @@
 			clocks = <&cpg CPG_MOD 804>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 804>;
+			renesas,id = <7>;
 			status = "disabled";

 			ports {
@@ -1081,6 +1089,7 @@
 			clocks = <&cpg CPG_MOD 628>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 628>;
+			renesas,id = <8>;
 			status = "disabled";
 		};

@@ -1091,6 +1100,7 @@
 			clocks = <&cpg CPG_MOD 627>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 627>;
+			renesas,id = <9>;
 			status = "disabled";
 		};

@@ -1101,6 +1111,7 @@
 			clocks = <&cpg CPG_MOD 625>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 625>;
+			renesas,id = <10>;
 			status = "disabled";
 		};

@@ -1111,6 +1122,7 @@
 			clocks = <&cpg CPG_MOD 618>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 618>;
+			renesas,id = <11>;
 			status = "disabled";
 		};

@@ -1121,6 +1133,7 @@
 			clocks = <&cpg CPG_MOD 612>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 612>;
+			renesas,id = <12>;
 			status = "disabled";
 		};

@@ -1131,6 +1144,7 @@
 			clocks = <&cpg CPG_MOD 608>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 608>;
+			renesas,id = <13>;
 			status = "disabled";
 		};

@@ -1141,6 +1155,7 @@
 			clocks = <&cpg CPG_MOD 605>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 605>;
+			renesas,id = <14>;
 			status = "disabled";
 		};

@@ -1151,6 +1166,7 @@
 			clocks = <&cpg CPG_MOD 604>;
 			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
 			resets = <&cpg 604>;
+			renesas,id = <15>;
 			status = "disabled";
 		};

--
2.21.0

Re: [PATCH 1/2] media: rcar-vin: Make the number of VIN SoC-dependent

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-04-12 13:38:31 +0200, Jacopo Mondi wrote:
> In preparation to add ids to VIN's device tree nodes for the V3H SoC
> which features up to 16 VIN instances, remove the hardcoded number of
> VINs and make it a per-SoC property.
> 
> Enlarge the existing RCAR_VIN_NUM definition, which is used to
> statically allocate the number of vin devices to 16, to accommodate
> SoCs with that many instances as V3H.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

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

This is wrong for two reasons, as explained on irc.

- The RCAR_VIN_NUM checks are bounds check for the array and not to 
  validate that the renesas,id property in the DT is correct. Making it 
  dynamic only serves to complicate the driver for no gain.

- It's true that the V3H have 16 VINs, but VIN8-15 are only connected to 
  the ISP for which there exists no upstream driver and no support in 
  the rcar-vin parsing code to handle it. With this change the rcar-vin 
  would probe for such VINs but the result would be use-less as it can 
  not get hold of the ISP, even if a driver would exist for it.

  I might be convinced to accept a patch which would allow DT to 
  describe all 16 VIN's of the V3H but where the rcar-vin would still 
  fail to probe if the renesas,id is above 7 but with a different 
  dev_err() informing the user why it failed.

  I would of course also be happy about a driver for the ISP and added 
  support for it in rcar-vin ;-)

> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 23 +++++++++++++++------
>  drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++++-
>  2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 64519b3097f7..6e9c6c8471e9 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -188,7 +188,7 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
>  		 * we can return here.
>  		 */
>  		sd = media_entity_to_v4l2_subdev(link->source->entity);
> -		for (i = 0; i < RCAR_VIN_NUM; i++) {
> +		for (i = 0; i <= vin->info->vin_max_id ; i++) {
>  			if (group->vin[i] && group->vin[i]->parallel &&
>  			    group->vin[i]->parallel->subdev == sd) {
>  				group->vin[i]->is_csi = false;
> @@ -319,7 +319,7 @@ static int rvin_group_get(struct rvin_dev *vin)
>  		return -EINVAL;
>  	}
> 
> -	if (id >= RCAR_VIN_NUM) {
> +	if (id > vin->info->vin_max_id) {
>  		vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
>  			vin->dev->of_node, id);
>  		return -EINVAL;
> @@ -662,7 +662,7 @@ 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++) {
> +	for (i = 0; i <= vin->info->vin_max_id; i++) {
>  		if (vin->group->vin[i] &&
>  		    !video_is_registered(&vin->group->vin[i]->vdev)) {
>  			ret = rvin_v4l2_register(vin->group->vin[i]);
> @@ -720,7 +720,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
>  	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
>  	unsigned int i;
> 
> -	for (i = 0; i < RCAR_VIN_NUM; i++)
> +	for (i = 0; i <= vin->info->vin_max_id; i++)
>  		if (vin->group->vin[i])
>  			rvin_v4l2_unregister(vin->group->vin[i]);
> 
> @@ -809,7 +809,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>  	mutex_lock(&vin->group->lock);
> 
>  	/* If not all VIN's are registered don't register the notifier. */
> -	for (i = 0; i < RCAR_VIN_NUM; i++) {
> +	for (i = 0; i <= vin->info->vin_max_id; i++) {
>  		if (vin->group->vin[i]) {
>  			count++;
>  			vin_mask |= BIT(i);
> @@ -830,7 +830,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
>  	 * overlap but the parser function can handle it, so each subdevice
>  	 * will only be registered once with the group notifier.
>  	 */
> -	for (i = 0; i < RCAR_VIN_NUM; i++) {
> +	for (i = 0; i <= vin->info->vin_max_id ; i++) {
>  		if (!(vin_mask & BIT(i)))
>  			continue;
> 
> @@ -884,6 +884,7 @@ static int rvin_mc_init(struct rvin_dev *vin)
>  static const struct rvin_info rcar_info_h1 = {
>  	.model = RCAR_H1,
>  	.use_mc = false,
> +	.vin_max_id = 3,
>  	.max_width = 2048,
>  	.max_height = 2048,
>  };
> @@ -891,6 +892,7 @@ static const struct rvin_info rcar_info_h1 = {
>  static const struct rvin_info rcar_info_m1 = {
>  	.model = RCAR_M1,
>  	.use_mc = false,
> +	.vin_max_id = 1,
>  	.max_width = 2048,
>  	.max_height = 2048,
>  };
> @@ -898,6 +900,7 @@ static const struct rvin_info rcar_info_m1 = {
>  static const struct rvin_info rcar_info_gen2 = {
>  	.model = RCAR_GEN2,
>  	.use_mc = false,
> +	.vin_max_id = 5,
>  	.max_width = 2048,
>  	.max_height = 2048,
>  };
> @@ -941,6 +944,7 @@ static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
>  static const struct rvin_info rcar_info_r8a7795 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 7,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a7795_routes,
> @@ -995,6 +999,7 @@ static const struct rvin_group_route rcar_info_r8a7795es1_routes[] = {
>  static const struct rvin_info rcar_info_r8a7795es1 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 7,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a7795es1_routes,
> @@ -1035,6 +1040,7 @@ static const struct rvin_group_route rcar_info_r8a7796_routes[] = {
>  static const struct rvin_info rcar_info_r8a7796 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 7,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a7796_routes,
> @@ -1079,6 +1085,7 @@ static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
>  static const struct rvin_info rcar_info_r8a77965 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 7,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a77965_routes,
> @@ -1098,6 +1105,7 @@ static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
>  static const struct rvin_info rcar_info_r8a77970 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 3,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a77970_routes,
> @@ -1126,6 +1134,7 @@ static const struct rvin_group_route rcar_info_r8a77980_routes[] = {
>  static const struct rvin_info rcar_info_r8a77980 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 15,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a77980_routes,
> @@ -1142,6 +1151,7 @@ static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
>  static const struct rvin_info rcar_info_r8a77990 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 5,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a77990_routes,
> @@ -1154,6 +1164,7 @@ static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
>  static const struct rvin_info rcar_info_r8a77995 = {
>  	.model = RCAR_GEN3,
>  	.use_mc = true,
> +	.vin_max_id = 4,
>  	.max_width = 4096,
>  	.max_height = 4096,
>  	.routes = rcar_info_r8a77995_routes,
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index 0b13b34d03e3..2726c56935e4 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -28,7 +28,8 @@
>  #define HW_BUFFER_MASK 0x7f
> 
>  /* Max number on VIN instances that can be in a system */
> -#define RCAR_VIN_NUM 8
> +#define RCAR_VIN_NUM 16
> +
> 
>  struct rvin_group;
> 
> @@ -126,6 +127,7 @@ 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
> + * @vin_max_id:		id of the last VIN instance
>   * @max_width:		max input width the VIN supports
>   * @max_height:		max input height the VIN supports
>   * @routes:		list of possible routes from the CSI-2 recivers to
> @@ -135,6 +137,7 @@ struct rvin_info {
>  	enum model_id model;
>  	bool use_mc;
> 
> +	unsigned int vin_max_id;
>  	unsigned int max_width;
>  	unsigned int max_height;
>  	const struct rvin_group_route *routes;
> --
> 2.21.0
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] arm64: dts: renesas: r8a77980: Add "renesas,id" to VIN

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-04-12 13:38:32 +0200, Jacopo Mondi wrote:
> Add the "renesas,id" property to VIN nodes in the R-Car V3H R8A77980
> device tree.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
> 
> I do not have a V3H board to test on, but I suspect VIN failed to probe
> on V3H before these 2 patches.
> 
> If that's the case, these patches would merit
> Fixes: 3182aa4e0bf4 ("arm64: dts: renesas: r8a77980: add CSI2/VIN support")
> 
> Sergei, do you still have a V3H board around? Could I have your Tested-by
> tag if things are fine now?
> 
> Thanks
>   j
> ---
>  arch/arm64/boot/dts/renesas/r8a77980.dtsi | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a77980.dtsi b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> index 4081622d548a..a901a341dcf7 100644
> --- a/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77980.dtsi
> @@ -865,6 +865,7 @@
>  			clocks = <&cpg CPG_MOD 811>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 811>;
> +			renesas,id = <0>;
>  			status = "disabled";
> 
>  			ports {
> @@ -892,6 +893,7 @@
>  			clocks = <&cpg CPG_MOD 810>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			status = "disabled";
> +			renesas,id = <1>;
>  			resets = <&cpg 810>;
> 
>  			ports {
> @@ -919,6 +921,7 @@
>  			clocks = <&cpg CPG_MOD 809>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 809>;
> +			renesas,id = <2>;
>  			status = "disabled";
> 
>  			ports {
> @@ -946,6 +949,7 @@
>  			clocks = <&cpg CPG_MOD 808>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 808>;
> +			renesas,id = <3>;
>  			status = "disabled";
> 
>  			ports {
> @@ -973,6 +977,7 @@
>  			clocks = <&cpg CPG_MOD 807>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 807>;
> +			renesas,id = <4>;
>  			status = "disabled";
> 
>  			ports {
> @@ -1000,6 +1005,7 @@
>  			clocks = <&cpg CPG_MOD 806>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 806>;
> +			renesas,id = <5>;
>  			status = "disabled";
> 
>  			ports {
> @@ -1027,6 +1033,7 @@
>  			clocks = <&cpg CPG_MOD 805>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 805>;
> +			renesas,id = <6>;
>  			status = "disabled";
> 
>  			ports {
> @@ -1054,6 +1061,7 @@
>  			clocks = <&cpg CPG_MOD 804>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 804>;
> +			renesas,id = <7>;
>  			status = "disabled";
> 
>  			ports {
> @@ -1081,6 +1089,7 @@
>  			clocks = <&cpg CPG_MOD 628>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 628>;
> +			renesas,id = <8>;
>  			status = "disabled";
>  		};
> 
> @@ -1091,6 +1100,7 @@
>  			clocks = <&cpg CPG_MOD 627>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 627>;
> +			renesas,id = <9>;
>  			status = "disabled";
>  		};
> 
> @@ -1101,6 +1111,7 @@
>  			clocks = <&cpg CPG_MOD 625>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 625>;
> +			renesas,id = <10>;
>  			status = "disabled";
>  		};
> 
> @@ -1111,6 +1122,7 @@
>  			clocks = <&cpg CPG_MOD 618>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 618>;
> +			renesas,id = <11>;
>  			status = "disabled";
>  		};
> 
> @@ -1121,6 +1133,7 @@
>  			clocks = <&cpg CPG_MOD 612>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 612>;
> +			renesas,id = <12>;
>  			status = "disabled";
>  		};
> 
> @@ -1131,6 +1144,7 @@
>  			clocks = <&cpg CPG_MOD 608>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 608>;
> +			renesas,id = <13>;
>  			status = "disabled";
>  		};
> 
> @@ -1141,6 +1155,7 @@
>  			clocks = <&cpg CPG_MOD 605>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 605>;
> +			renesas,id = <14>;
>  			status = "disabled";
>  		};
> 
> @@ -1151,6 +1166,7 @@
>  			clocks = <&cpg CPG_MOD 604>;
>  			power-domains = <&sysc R8A77980_PD_ALWAYS_ON>;
>  			resets = <&cpg 604>;
> +			renesas,id = <15>;
>  			status = "disabled";
>  		};
> 
> --
> 2.21.0
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] media: rcar-vin: Make the number of VIN SoC-dependent

[Jacopo Mondi]

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

Hi Niklas,

On Fri, Apr 12, 2019 at 01:58:36PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2019-04-12 13:38:31 +0200, Jacopo Mondi wrote:
> > In preparation to add ids to VIN's device tree nodes for the V3H SoC
> > which features up to 16 VIN instances, remove the hardcoded number of
> > VINs and make it a per-SoC property.
> >
> > Enlarge the existing RCAR_VIN_NUM definition, which is used to
> > statically allocate the number of vin devices to 16, to accommodate
> > SoCs with that many instances as V3H.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
>
> Nacked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> This is wrong for two reasons, as explained on irc.
>
> - The RCAR_VIN_NUM checks are bounds check for the array and not to
>   validate that the renesas,id property in the DT is correct. Making it
>   dynamic only serves to complicate the driver for no gain.

Regardless of V3H support, validating that the renesas,id property is
valid for the current SoC is worth the "complication" imho.
Why is bound checking different now? (we could discuss if allocating 8
vin devices more for all SoC is worth, that I agree)
Not to mention having that value fixed it means we alway cycle on all
8 vin devices instances, even if the SoC supports less than that.
>
> - It's true that the V3H have 16 VINs, but VIN8-15 are only connected to
>   the ISP for which there exists no upstream driver and no support in
>   the rcar-vin parsing code to handle it. With this change the rcar-vin
>   would probe for such VINs but the result would be use-less as it can
>   not get hold of the ISP, even if a driver would exist for it.
>

Let's remove them from DT then?

>   I might be convinced to accept a patch which would allow DT to
>   describe all 16 VIN's of the V3H but where the rcar-vin would still
>   fail to probe if the renesas,id is above 7 but with a different
>   dev_err() informing the user why it failed.

I cannot test on V3H but I think all VINs (included 0-7, which are
connected to a CSI-2 receiver) fail to probe without a "renesas,id"
property, and this should be fixed. I like less the idea of having a special
failure path for the VIN[8-15] instances than removing them completely
tbh.

>
>   I would of course also be happy about a driver for the ISP and added
>   support for it in rcar-vin ;-)
>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c | 23 +++++++++++++++------
> >  drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++++-
> >  2 files changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > index 64519b3097f7..6e9c6c8471e9 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -188,7 +188,7 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
> >  		 * we can return here.
> >  		 */
> >  		sd = media_entity_to_v4l2_subdev(link->source->entity);
> > -		for (i = 0; i < RCAR_VIN_NUM; i++) {
> > +		for (i = 0; i <= vin->info->vin_max_id ; i++) {
> >  			if (group->vin[i] && group->vin[i]->parallel &&
> >  			    group->vin[i]->parallel->subdev == sd) {
> >  				group->vin[i]->is_csi = false;
> > @@ -319,7 +319,7 @@ static int rvin_group_get(struct rvin_dev *vin)
> >  		return -EINVAL;
> >  	}
> >
> > -	if (id >= RCAR_VIN_NUM) {
> > +	if (id > vin->info->vin_max_id) {
> >  		vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
> >  			vin->dev->of_node, id);
> >  		return -EINVAL;
> > @@ -662,7 +662,7 @@ 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++) {
> > +	for (i = 0; i <= vin->info->vin_max_id; i++) {
> >  		if (vin->group->vin[i] &&
> >  		    !video_is_registered(&vin->group->vin[i]->vdev)) {
> >  			ret = rvin_v4l2_register(vin->group->vin[i]);
> > @@ -720,7 +720,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
> >  	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
> >  	unsigned int i;
> >
> > -	for (i = 0; i < RCAR_VIN_NUM; i++)
> > +	for (i = 0; i <= vin->info->vin_max_id; i++)
> >  		if (vin->group->vin[i])
> >  			rvin_v4l2_unregister(vin->group->vin[i]);
> >
> > @@ -809,7 +809,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
> >  	mutex_lock(&vin->group->lock);
> >
> >  	/* If not all VIN's are registered don't register the notifier. */
> > -	for (i = 0; i < RCAR_VIN_NUM; i++) {
> > +	for (i = 0; i <= vin->info->vin_max_id; i++) {
> >  		if (vin->group->vin[i]) {
> >  			count++;
> >  			vin_mask |= BIT(i);
> > @@ -830,7 +830,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
> >  	 * overlap but the parser function can handle it, so each subdevice
> >  	 * will only be registered once with the group notifier.
> >  	 */
> > -	for (i = 0; i < RCAR_VIN_NUM; i++) {
> > +	for (i = 0; i <= vin->info->vin_max_id ; i++) {
> >  		if (!(vin_mask & BIT(i)))
> >  			continue;
> >
> > @@ -884,6 +884,7 @@ static int rvin_mc_init(struct rvin_dev *vin)
> >  static const struct rvin_info rcar_info_h1 = {
> >  	.model = RCAR_H1,
> >  	.use_mc = false,
> > +	.vin_max_id = 3,
> >  	.max_width = 2048,
> >  	.max_height = 2048,
> >  };
> > @@ -891,6 +892,7 @@ static const struct rvin_info rcar_info_h1 = {
> >  static const struct rvin_info rcar_info_m1 = {
> >  	.model = RCAR_M1,
> >  	.use_mc = false,
> > +	.vin_max_id = 1,
> >  	.max_width = 2048,
> >  	.max_height = 2048,
> >  };
> > @@ -898,6 +900,7 @@ static const struct rvin_info rcar_info_m1 = {
> >  static const struct rvin_info rcar_info_gen2 = {
> >  	.model = RCAR_GEN2,
> >  	.use_mc = false,
> > +	.vin_max_id = 5,
> >  	.max_width = 2048,
> >  	.max_height = 2048,
> >  };
> > @@ -941,6 +944,7 @@ static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
> >  static const struct rvin_info rcar_info_r8a7795 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 7,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a7795_routes,
> > @@ -995,6 +999,7 @@ static const struct rvin_group_route rcar_info_r8a7795es1_routes[] = {
> >  static const struct rvin_info rcar_info_r8a7795es1 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 7,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a7795es1_routes,
> > @@ -1035,6 +1040,7 @@ static const struct rvin_group_route rcar_info_r8a7796_routes[] = {
> >  static const struct rvin_info rcar_info_r8a7796 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 7,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a7796_routes,
> > @@ -1079,6 +1085,7 @@ static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
> >  static const struct rvin_info rcar_info_r8a77965 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 7,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a77965_routes,
> > @@ -1098,6 +1105,7 @@ static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
> >  static const struct rvin_info rcar_info_r8a77970 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 3,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a77970_routes,
> > @@ -1126,6 +1134,7 @@ static const struct rvin_group_route rcar_info_r8a77980_routes[] = {
> >  static const struct rvin_info rcar_info_r8a77980 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 15,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a77980_routes,
> > @@ -1142,6 +1151,7 @@ static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
> >  static const struct rvin_info rcar_info_r8a77990 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 5,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a77990_routes,
> > @@ -1154,6 +1164,7 @@ static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
> >  static const struct rvin_info rcar_info_r8a77995 = {
> >  	.model = RCAR_GEN3,
> >  	.use_mc = true,
> > +	.vin_max_id = 4,
> >  	.max_width = 4096,
> >  	.max_height = 4096,
> >  	.routes = rcar_info_r8a77995_routes,
> > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> > index 0b13b34d03e3..2726c56935e4 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> > @@ -28,7 +28,8 @@
> >  #define HW_BUFFER_MASK 0x7f
> >
> >  /* Max number on VIN instances that can be in a system */
> > -#define RCAR_VIN_NUM 8
> > +#define RCAR_VIN_NUM 16
> > +
> >
> >  struct rvin_group;
> >
> > @@ -126,6 +127,7 @@ 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
> > + * @vin_max_id:		id of the last VIN instance
> >   * @max_width:		max input width the VIN supports
> >   * @max_height:		max input height the VIN supports
> >   * @routes:		list of possible routes from the CSI-2 recivers to
> > @@ -135,6 +137,7 @@ struct rvin_info {
> >  	enum model_id model;
> >  	bool use_mc;
> >
> > +	unsigned int vin_max_id;
> >  	unsigned int max_width;
> >  	unsigned int max_height;
> >  	const struct rvin_group_route *routes;
> > --
> > 2.21.0
> >
>
> --
> Regards,
> Niklas Söderlund

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

Re: [PATCH 1/2] media: rcar-vin: Make the number of VIN SoC-dependent

[Niklas Söderlund]

Hi Jacopo,

On 2019-04-12 14:13:52 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Fri, Apr 12, 2019 at 01:58:36PM +0200, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your work.
> >
> > On 2019-04-12 13:38:31 +0200, Jacopo Mondi wrote:
> > > In preparation to add ids to VIN's device tree nodes for the V3H SoC
> > > which features up to 16 VIN instances, remove the hardcoded number of
> > > VINs and make it a per-SoC property.
> > >
> > > Enlarge the existing RCAR_VIN_NUM definition, which is used to
> > > statically allocate the number of vin devices to 16, to accommodate
> > > SoCs with that many instances as V3H.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> >
> > Nacked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >
> > This is wrong for two reasons, as explained on irc.
> >
> > - The RCAR_VIN_NUM checks are bounds check for the array and not to
> >   validate that the renesas,id property in the DT is correct. Making it
> >   dynamic only serves to complicate the driver for no gain.
> 
> Regardless of V3H support, validating that the renesas,id property is
> valid for the current SoC is worth the "complication" imho.
> Why is bound checking different now? (we could discuss if allocating 8
> vin devices more for all SoC is worth, that I agree)
> Not to mention having that value fixed it means we alway cycle on all
> 8 vin devices instances, even if the SoC supports less than that.

I don't think the driver should validate the renesas,id property. We 
must assume that DT is correct. Only reason we do it now is that we have 
an array of RCAR_VIN_NUM size and it's more nasty to crash then then to 
check it if the DT happens to be incorrect. If we where to validate it 
with SoC knowledge we could drop the property completely and derive the 
information for the base address of the VIN.

I hope to one day remove most of the group hack and for that reason I 
wish to keep this as simple as possible. Iterating 4 more times V3M, 7 
more times on D3 and 6 more times on E3 in the small loops in the probe 
path is acceptable for me to keep it simple.

> >
> > - It's true that the V3H have 16 VINs, but VIN8-15 are only connected to
> >   the ISP for which there exists no upstream driver and no support in
> >   the rcar-vin parsing code to handle it. With this change the rcar-vin
> >   would probe for such VINs but the result would be use-less as it can
> >   not get hold of the ISP, even if a driver would exist for it.
> >
> 
> Let's remove them from DT then?

DT should describe hardware, this is a driver limitation I think all 
VINs should be kept in DT.

> 
> >   I might be convinced to accept a patch which would allow DT to
> >   describe all 16 VIN's of the V3H but where the rcar-vin would still
> >   fail to probe if the renesas,id is above 7 but with a different
> >   dev_err() informing the user why it failed.
> 
> I cannot test on V3H but I think all VINs (included 0-7, which are
> connected to a CSI-2 receiver) fail to probe without a "renesas,id"
> property, and this should be fixed. I like less the idea of having a special
> failure path for the VIN[8-15] instances than removing them completely
> tbh.

My preferred option would be to:

- Add the renesas,id to all 16 VIN in the V3H DT.
- Keep all VIN in status = "disabled" as they are in r8a77980.dtsi.
- None of the users of r8a77980.dtsi (r8a77980-condor.dts, 
  r8a77980-v3hsk.dts) enables any of the VINs so there is no regression 
  upstream. If a new board is added it should only enable VIN0-7 as it's 
  the only ones they can use with rcar-vin.
- Keep the error message and logic as it is today in rcar-vin as a id 
  larger then 7 is invalid for the driver.

This would allow the DT to be correct and the driver to correctly report 
what it supports. If/When we add ISP support we can revisit this topic.

> 
> >
> >   I would of course also be happy about a driver for the ISP and added
> >   support for it in rcar-vin ;-)
> >
> > > ---
> > >  drivers/media/platform/rcar-vin/rcar-core.c | 23 +++++++++++++++------
> > >  drivers/media/platform/rcar-vin/rcar-vin.h  |  5 ++++-
> > >  2 files changed, 21 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > > index 64519b3097f7..6e9c6c8471e9 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > > @@ -188,7 +188,7 @@ static int rvin_group_link_notify(struct media_link *link, u32 flags,
> > >  		 * we can return here.
> > >  		 */
> > >  		sd = media_entity_to_v4l2_subdev(link->source->entity);
> > > -		for (i = 0; i < RCAR_VIN_NUM; i++) {
> > > +		for (i = 0; i <= vin->info->vin_max_id ; i++) {
> > >  			if (group->vin[i] && group->vin[i]->parallel &&
> > >  			    group->vin[i]->parallel->subdev == sd) {
> > >  				group->vin[i]->is_csi = false;
> > > @@ -319,7 +319,7 @@ static int rvin_group_get(struct rvin_dev *vin)
> > >  		return -EINVAL;
> > >  	}
> > >
> > > -	if (id >= RCAR_VIN_NUM) {
> > > +	if (id > vin->info->vin_max_id) {
> > >  		vin_err(vin, "%pOF: Invalid renesas,id '%u'\n",
> > >  			vin->dev->of_node, id);
> > >  		return -EINVAL;
> > > @@ -662,7 +662,7 @@ 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++) {
> > > +	for (i = 0; i <= vin->info->vin_max_id; i++) {
> > >  		if (vin->group->vin[i] &&
> > >  		    !video_is_registered(&vin->group->vin[i]->vdev)) {
> > >  			ret = rvin_v4l2_register(vin->group->vin[i]);
> > > @@ -720,7 +720,7 @@ static void rvin_group_notify_unbind(struct v4l2_async_notifier *notifier,
> > >  	struct rvin_dev *vin = v4l2_dev_to_vin(notifier->v4l2_dev);
> > >  	unsigned int i;
> > >
> > > -	for (i = 0; i < RCAR_VIN_NUM; i++)
> > > +	for (i = 0; i <= vin->info->vin_max_id; i++)
> > >  		if (vin->group->vin[i])
> > >  			rvin_v4l2_unregister(vin->group->vin[i]);
> > >
> > > @@ -809,7 +809,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
> > >  	mutex_lock(&vin->group->lock);
> > >
> > >  	/* If not all VIN's are registered don't register the notifier. */
> > > -	for (i = 0; i < RCAR_VIN_NUM; i++) {
> > > +	for (i = 0; i <= vin->info->vin_max_id; i++) {
> > >  		if (vin->group->vin[i]) {
> > >  			count++;
> > >  			vin_mask |= BIT(i);
> > > @@ -830,7 +830,7 @@ static int rvin_mc_parse_of_graph(struct rvin_dev *vin)
> > >  	 * overlap but the parser function can handle it, so each subdevice
> > >  	 * will only be registered once with the group notifier.
> > >  	 */
> > > -	for (i = 0; i < RCAR_VIN_NUM; i++) {
> > > +	for (i = 0; i <= vin->info->vin_max_id ; i++) {
> > >  		if (!(vin_mask & BIT(i)))
> > >  			continue;
> > >
> > > @@ -884,6 +884,7 @@ static int rvin_mc_init(struct rvin_dev *vin)
> > >  static const struct rvin_info rcar_info_h1 = {
> > >  	.model = RCAR_H1,
> > >  	.use_mc = false,
> > > +	.vin_max_id = 3,
> > >  	.max_width = 2048,
> > >  	.max_height = 2048,
> > >  };
> > > @@ -891,6 +892,7 @@ static const struct rvin_info rcar_info_h1 = {
> > >  static const struct rvin_info rcar_info_m1 = {
> > >  	.model = RCAR_M1,
> > >  	.use_mc = false,
> > > +	.vin_max_id = 1,
> > >  	.max_width = 2048,
> > >  	.max_height = 2048,
> > >  };
> > > @@ -898,6 +900,7 @@ static const struct rvin_info rcar_info_m1 = {
> > >  static const struct rvin_info rcar_info_gen2 = {
> > >  	.model = RCAR_GEN2,
> > >  	.use_mc = false,
> > > +	.vin_max_id = 5,
> > >  	.max_width = 2048,
> > >  	.max_height = 2048,
> > >  };
> > > @@ -941,6 +944,7 @@ static const struct rvin_group_route rcar_info_r8a7795_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a7795 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 7,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a7795_routes,
> > > @@ -995,6 +999,7 @@ static const struct rvin_group_route rcar_info_r8a7795es1_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a7795es1 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 7,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a7795es1_routes,
> > > @@ -1035,6 +1040,7 @@ static const struct rvin_group_route rcar_info_r8a7796_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a7796 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 7,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a7796_routes,
> > > @@ -1079,6 +1085,7 @@ static const struct rvin_group_route rcar_info_r8a77965_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a77965 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 7,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a77965_routes,
> > > @@ -1098,6 +1105,7 @@ static const struct rvin_group_route rcar_info_r8a77970_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a77970 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 3,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a77970_routes,
> > > @@ -1126,6 +1134,7 @@ static const struct rvin_group_route rcar_info_r8a77980_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a77980 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 15,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a77980_routes,
> > > @@ -1142,6 +1151,7 @@ static const struct rvin_group_route rcar_info_r8a77990_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a77990 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 5,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a77990_routes,
> > > @@ -1154,6 +1164,7 @@ static const struct rvin_group_route rcar_info_r8a77995_routes[] = {
> > >  static const struct rvin_info rcar_info_r8a77995 = {
> > >  	.model = RCAR_GEN3,
> > >  	.use_mc = true,
> > > +	.vin_max_id = 4,
> > >  	.max_width = 4096,
> > >  	.max_height = 4096,
> > >  	.routes = rcar_info_r8a77995_routes,
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> > > index 0b13b34d03e3..2726c56935e4 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> > > +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> > > @@ -28,7 +28,8 @@
> > >  #define HW_BUFFER_MASK 0x7f
> > >
> > >  /* Max number on VIN instances that can be in a system */
> > > -#define RCAR_VIN_NUM 8
> > > +#define RCAR_VIN_NUM 16
> > > +
> > >
> > >  struct rvin_group;
> > >
> > > @@ -126,6 +127,7 @@ 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
> > > + * @vin_max_id:		id of the last VIN instance
> > >   * @max_width:		max input width the VIN supports
> > >   * @max_height:		max input height the VIN supports
> > >   * @routes:		list of possible routes from the CSI-2 recivers to
> > > @@ -135,6 +137,7 @@ struct rvin_info {
> > >  	enum model_id model;
> > >  	bool use_mc;
> > >
> > > +	unsigned int vin_max_id;
> > >  	unsigned int max_width;
> > >  	unsigned int max_height;
> > >  	const struct rvin_group_route *routes;
> > > --
> > > 2.21.0
> > >
> >
> > --
> > Regards,
> > Niklas Söderlund



-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] arm64: dts: renesas: r8a77980: Add "renesas,id" to VIN

[Sergei Shtylyov]

On 04/12/2019 02:38 PM, Jacopo Mondi wrote:

> Add the "renesas,id" property to VIN nodes in the R-Car V3H R8A77980
> device tree.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> 
> I do not have a V3H board to test on, but I suspect VIN failed to probe
> on V3H before these 2 patches.

   Yes, probably. The upstream patch wasn't ever runtime tested, so this
went unnoticed. The BSPs use the old soc_camera driver.

> If that's the case, these patches would merit
> Fixes: 3182aa4e0bf4 ("arm64: dts: renesas: r8a77980: add CSI2/VIN support")

   I'm OK with that.

> Sergei, do you still have a V3H board around? Could I have your Tested-by
> tag if things are fine now?

   I have V3H Stater Kit on my desk but I don't think it has VINs wired to
anything...

> Thanks
>   j

MBR, Sergei

Re: [PATCH 2/2] arm64: dts: renesas: r8a77980: Add "renesas,id" to VIN

[Simon Horman]

On Fri, Apr 12, 2019 at 02:00:53PM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
> 
> Thanks for your patch.
> 
> On 2019-04-12 13:38:32 +0200, Jacopo Mondi wrote:
> > Add the "renesas,id" property to VIN nodes in the R-Car V3H R8A77980
> > device tree.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

...

On Sat, Apr 13, 2019 at 09:25:08PM +0300, Sergei Shtylyov wrote:
> On 04/12/2019 02:38 PM, Jacopo Mondi wrote:
> 
> > Add the "renesas,id" property to VIN nodes in the R-Car V3H R8A77980
> > device tree.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> > 
> > I do not have a V3H board to test on, but I suspect VIN failed to probe
> > on V3H before these 2 patches.
> 
>    Yes, probably. The upstream patch wasn't ever runtime tested, so this
> went unnoticed. The BSPs use the old soc_camera driver.
> 
> > If that's the case, these patches would merit
> > Fixes: 3182aa4e0bf4 ("arm64: dts: renesas: r8a77980: add CSI2/VIN support")
> 
>    I'm OK with that.
> 
> > Sergei, do you still have a V3H board around? Could I have your Tested-by
> > tag if things are fine now?
> 
>    I have V3H Stater Kit on my desk but I don't think it has VINs wired to
> anything...

...

Thanks everyone,

I have applied this for inclusion in v5.2 with the fixes tag.