[PATCH 0/2] media: rcar-vin: feature enhancement and fixes

[PATCH 0/2] media: rcar-vin: feature enhancement and fixes

[Lad Prabhakar]

First patch of the series fixes registering endpoints to the vin module and
the second patch adds supports for SRGGB8_1X8 format.

Lad Prabhakar (2):
  media: rcar-csi2: Fix registering camera endpoint to VIN
  media: rcar-vin: Add support for SRGGB8_1X8

 drivers/media/platform/rcar-vin/rcar-core.c |  1 +
 drivers/media/platform/rcar-vin/rcar-csi2.c | 15 +++++++--------
 drivers/media/platform/rcar-vin/rcar-dma.c  | 14 ++++++++++++--
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  4 ++++
 4 files changed, 24 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH 1/2] media: rcar-csi2: Fix registering camera endpoint to VIN

[Lad Prabhakar]

CSI2 registers camera/sensor as v4l2 async sub device with fwnode is
remote endpoint and the camera/sensor register itself as v4l2 sub device
with fwnode is remote device as a result the match.fwnode should be
fwnode_graph_get_remote_port_parent and not
fwnode_graph_get_remote_endpoint.

This patch makes use of v4l2 helper function
v4l2_async_notifier_add_fwnode_remote_subdev() which uses
fwnode_graph_get_remote_port_parent as match.fwnode fixing the issue
of registering camera endpoint to the driver.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index faa9fb23a2e9..5b04e4768eb1 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -833,20 +833,18 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv)
 		return ret;
 	}
 
-	priv->asd.match.fwnode =
-		fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
-	priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
-
-	of_node_put(ep);
-
 	v4l2_async_notifier_init(&priv->notifier);
 
-	ret = v4l2_async_notifier_add_subdev(&priv->notifier, &priv->asd);
+	ret = v4l2_async_notifier_add_fwnode_remote_subdev(&priv->notifier,
+							   of_fwnode_handle(ep),
+							   &priv->asd);
 	if (ret) {
-		fwnode_handle_put(priv->asd.match.fwnode);
+		of_node_put(ep);
 		return ret;
 	}
 
+	of_node_put(ep);
+
 	priv->notifier.ops = &rcar_csi2_notify_ops;
 
 	dev_dbg(priv->dev, "Found '%pOF'\n",
-- 
2.20.1

[PATCH 2/2] media: rcar-vin: Add support for SRGGB8_1X8

[Lad Prabhakar]

Most of the RZ/G2x boards can support capturing frames in SRGGB8_1X8
format, add support for this with a runtime check that the running
hardware supports it.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 drivers/media/platform/rcar-vin/rcar-core.c |  1 +
 drivers/media/platform/rcar-vin/rcar-csi2.c |  1 +
 drivers/media/platform/rcar-vin/rcar-dma.c  | 14 ++++++++++++--
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  4 ++++
 4 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 7440c8965d27..76daf2fe5bcd 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -469,6 +469,7 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
 		case MEDIA_BUS_FMT_UYVY8_2X8:
 		case MEDIA_BUS_FMT_UYVY10_2X10:
 		case MEDIA_BUS_FMT_RGB888_1X24:
+		case MEDIA_BUS_FMT_SRGGB8_1X8:
 			vin->mbus_code = code.code;
 			vin_dbg(vin, "Found media bus format for %s: %d\n",
 				subdev->name, vin->mbus_code);
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 5b04e4768eb1..97e4886f63f0 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -320,6 +320,7 @@ static const struct rcar_csi2_format rcar_csi2_formats[] = {
 	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .bpp = 16 },
 	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
 	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 20 },
+	{ .code = MEDIA_BUS_FMT_SRGGB8_1X8,     .datatype = 0x2a, .bpp = 8 },
 };
 
 static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 1a30cd036371..63709b4bbf7a 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -85,6 +85,7 @@
 #define VNMC_INF_YUV8_BT601	(1 << 16)
 #define VNMC_INF_YUV10_BT656	(2 << 16)
 #define VNMC_INF_YUV10_BT601	(3 << 16)
+#define VNMC_INF_RAW8		(4 << 16)
 #define VNMC_INF_YUV16		(5 << 16)
 #define VNMC_INF_RGB888		(6 << 16)
 #define VNMC_VUP		(1 << 10)
@@ -587,13 +588,15 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
 	rvin_write(vin, vin->crop.top, VNSLPRC_REG);
 	rvin_write(vin, vin->crop.top + vin->crop.height - 1, VNELPRC_REG);
 
-
 	/* TODO: Add support for the UDS scaler. */
 	if (vin->info->model != RCAR_GEN3)
 		rvin_crop_scale_comp_gen2(vin);
 
 	fmt = rvin_format_from_pixel(vin, vin->format.pixelformat);
-	stride = vin->format.bytesperline / fmt->bpp;
+	if (vin->format.pixelformat == V4L2_PIX_FMT_SRGGB8)
+		stride = ALIGN(vin->format.bytesperline / 2, 0x10);
+	else
+		stride = vin->format.bytesperline / fmt->bpp;
 	rvin_write(vin, stride, VNIS_REG);
 }
 
@@ -676,6 +679,9 @@ static int rvin_setup(struct rvin_dev *vin)
 
 		input_is_yuv = true;
 		break;
+	case MEDIA_BUS_FMT_SRGGB8_1X8:
+		vnmc |= VNMC_INF_RAW8;
+		break;
 	default:
 		break;
 	}
@@ -737,6 +743,9 @@ static int rvin_setup(struct rvin_dev *vin)
 	case V4L2_PIX_FMT_ABGR32:
 		dmr = VNDMR_A8BIT(vin->alpha) | VNDMR_EXRGB | VNDMR_DTMD_ARGB;
 		break;
+	case V4L2_PIX_FMT_SRGGB8:
+		dmr = 0;
+		break;
 	default:
 		vin_err(vin, "Invalid pixelformat (0x%x)\n",
 			vin->format.pixelformat);
@@ -1110,6 +1119,7 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
 	case MEDIA_BUS_FMT_UYVY8_2X8:
 	case MEDIA_BUS_FMT_UYVY10_2X10:
 	case MEDIA_BUS_FMT_RGB888_1X24:
+	case MEDIA_BUS_FMT_SRGGB8_1X8:
 		vin->mbus_code = fmt.format.code;
 		break;
 	default:
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index b12b3f88836a..d56cf85ba065 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -66,6 +66,10 @@ static const struct rvin_video_format rvin_formats[] = {
 		.fourcc			= V4L2_PIX_FMT_ABGR32,
 		.bpp			= 4,
 	},
+	{
+		.fourcc			= V4L2_PIX_FMT_SRGGB8,
+		.bpp			= 1,
+	},
 };
 
 const struct rvin_video_format *rvin_format_from_pixel(struct rvin_dev *vin,
-- 
2.20.1

Re: [PATCH 1/2] media: rcar-csi2: Fix registering camera endpoint to VIN

[Niklas]

Hi Lad,

Thanks for your work.

On 2020-02-28 16:50:10 +0000, Lad Prabhakar wrote:
> CSI2 registers camera/sensor as v4l2 async sub device with fwnode is
> remote endpoint and the camera/sensor register itself as v4l2 sub device
> with fwnode is remote device as a result the match.fwnode should be
> fwnode_graph_get_remote_port_parent and not
> fwnode_graph_get_remote_endpoint.
> 
> This patch makes use of v4l2 helper function
> v4l2_async_notifier_add_fwnode_remote_subdev() which uses
> fwnode_graph_get_remote_port_parent as match.fwnode fixing the issue
> of registering camera endpoint to the driver.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

I'm afraid this is not the right solution. The rcar-csi2 driver uses the 
v4l2-async framework to do endpoint matching instead of node matching.  
This is needed as it needs to work with the adv748x driver which 
register it self in v4l2-async using endpoints instead of nodes. The 
reason for this is that from a single DT node it creates multiple 
subdevices, one for each endpoint IIRC.

Without this patch the two CSI-2 receivers on R-Car M3-n registers the 
two following 'paths' in v4l2 to be able to find the two subdevice CSI-2 
transmitters created by the ADV748x.

rcar-csi2 fea80000.csi2: '/soc/i2c@e66d8000/video-receiver@70/port@b/endpoint'
rcar-csi2 feaa0000.csi2: '/soc/i2c@e66d8000/video-receiver@70/port@a/endpoint'

With this patch applied it registers the following which can't be found 
as they are not present in the v4l2-async list of subdevices (as they 
are registerd as above).

rcar-csi2 fea80000.csi2: '/soc/i2c@e66d8000/video-receiver@70'
rcar-csi2 feaa0000.csi2: '/soc/i2c@e66d8000/video-receiver@70'
rcar-csi2: probe of feaa0000.csi2 failed with error -17

This patch may unlock your use-case as it's a known problem that 
endpoint and node matching do not mix. But it will break the already 
upstream use-case and for that reason, I'm really sorry about this.

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

The real solution to this problem IMHO is to make all of v4l2-async 
operate using endpoint matching or possibly some kind of fallback to 
node matching if no endpoint can be found. Never the less some work is 
required in the v4l2-async core to sort out node and endpoint matching 
coexistence.

> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index faa9fb23a2e9..5b04e4768eb1 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -833,20 +833,18 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv)
>  		return ret;
>  	}
>  
> -	priv->asd.match.fwnode =
> -		fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
> -	priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> -
> -	of_node_put(ep);
> -
>  	v4l2_async_notifier_init(&priv->notifier);
>  
> -	ret = v4l2_async_notifier_add_subdev(&priv->notifier, &priv->asd);
> +	ret = v4l2_async_notifier_add_fwnode_remote_subdev(&priv->notifier,
> +							   of_fwnode_handle(ep),
> +							   &priv->asd);
>  	if (ret) {
> -		fwnode_handle_put(priv->asd.match.fwnode);
> +		of_node_put(ep);
>  		return ret;
>  	}
>  
> +	of_node_put(ep);
> +
>  	priv->notifier.ops = &rcar_csi2_notify_ops;
>  
>  	dev_dbg(priv->dev, "Found '%pOF'\n",
> -- 
> 2.20.1
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 2/2] media: rcar-vin: Add support for SRGGB8_1X8

[Niklas]

Hi Lad,

Thanks for your work.

On 2020-02-28 16:50:11 +0000, Lad Prabhakar wrote:
> Most of the RZ/G2x boards can support capturing frames in SRGGB8_1X8
> format, add support for this with a runtime check that the running
> hardware supports it.

Where is the runtime check for supported hardware? Also out of curiosity 
which boars can't support SRGGB and what setup are you using to test 
SRGGB capture?

> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
>  drivers/media/platform/rcar-vin/rcar-csi2.c |  1 +
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 14 ++++++++++++--
>  drivers/media/platform/rcar-vin/rcar-v4l2.c |  4 ++++

This patch shall be split in two. One which touches rcar-csi2.c and one 
for the other files as they are two different drivers.

>  4 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 7440c8965d27..76daf2fe5bcd 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -469,6 +469,7 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
>  		case MEDIA_BUS_FMT_UYVY8_2X8:
>  		case MEDIA_BUS_FMT_UYVY10_2X10:
>  		case MEDIA_BUS_FMT_RGB888_1X24:
> +		case MEDIA_BUS_FMT_SRGGB8_1X8:
>  			vin->mbus_code = code.code;
>  			vin_dbg(vin, "Found media bus format for %s: %d\n",
>  				subdev->name, vin->mbus_code);
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 5b04e4768eb1..97e4886f63f0 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -320,6 +320,7 @@ static const struct rcar_csi2_format rcar_csi2_formats[] = {
>  	{ .code = MEDIA_BUS_FMT_YUYV8_1X16,	.datatype = 0x1e, .bpp = 16 },
>  	{ .code = MEDIA_BUS_FMT_UYVY8_2X8,	.datatype = 0x1e, .bpp = 16 },
>  	{ .code = MEDIA_BUS_FMT_YUYV10_2X10,	.datatype = 0x1e, .bpp = 20 },
> +	{ .code = MEDIA_BUS_FMT_SRGGB8_1X8,     .datatype = 0x2a, .bpp = 8 },
>  };
>  
>  static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 1a30cd036371..63709b4bbf7a 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -85,6 +85,7 @@
>  #define VNMC_INF_YUV8_BT601	(1 << 16)
>  #define VNMC_INF_YUV10_BT656	(2 << 16)
>  #define VNMC_INF_YUV10_BT601	(3 << 16)
> +#define VNMC_INF_RAW8		(4 << 16)
>  #define VNMC_INF_YUV16		(5 << 16)
>  #define VNMC_INF_RGB888		(6 << 16)
>  #define VNMC_VUP		(1 << 10)
> @@ -587,13 +588,15 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
>  	rvin_write(vin, vin->crop.top, VNSLPRC_REG);
>  	rvin_write(vin, vin->crop.top + vin->crop.height - 1, VNELPRC_REG);
>  
> -
>  	/* TODO: Add support for the UDS scaler. */
>  	if (vin->info->model != RCAR_GEN3)
>  		rvin_crop_scale_comp_gen2(vin);
>  
>  	fmt = rvin_format_from_pixel(vin, vin->format.pixelformat);
> -	stride = vin->format.bytesperline / fmt->bpp;
> +	if (vin->format.pixelformat == V4L2_PIX_FMT_SRGGB8)
> +		stride = ALIGN(vin->format.bytesperline / 2, 0x10);
> +	else
> +		stride = vin->format.bytesperline / fmt->bpp;

This does not look right, you should to move this logic into 
rvin_format_bytesperline().

>  	rvin_write(vin, stride, VNIS_REG);
>  }
>  
> @@ -676,6 +679,9 @@ static int rvin_setup(struct rvin_dev *vin)
>  
>  		input_is_yuv = true;
>  		break;
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
> +		vnmc |= VNMC_INF_RAW8;
> +		break;
>  	default:
>  		break;
>  	}
> @@ -737,6 +743,9 @@ static int rvin_setup(struct rvin_dev *vin)
>  	case V4L2_PIX_FMT_ABGR32:
>  		dmr = VNDMR_A8BIT(vin->alpha) | VNDMR_EXRGB | VNDMR_DTMD_ARGB;
>  		break;
> +	case V4L2_PIX_FMT_SRGGB8:
> +		dmr = 0;
> +		break;
>  	default:
>  		vin_err(vin, "Invalid pixelformat (0x%x)\n",
>  			vin->format.pixelformat);
> @@ -1110,6 +1119,7 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
>  	case MEDIA_BUS_FMT_UYVY8_2X8:
>  	case MEDIA_BUS_FMT_UYVY10_2X10:
>  	case MEDIA_BUS_FMT_RGB888_1X24:
> +	case MEDIA_BUS_FMT_SRGGB8_1X8:
>  		vin->mbus_code = fmt.format.code;
>  		break;
>  	default:
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index b12b3f88836a..d56cf85ba065 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -66,6 +66,10 @@ static const struct rvin_video_format rvin_formats[] = {
>  		.fourcc			= V4L2_PIX_FMT_ABGR32,
>  		.bpp			= 4,
>  	},
> +	{
> +		.fourcc			= V4L2_PIX_FMT_SRGGB8,
> +		.bpp			= 1,
> +	},
>  };
>  
>  const struct rvin_video_format *rvin_format_from_pixel(struct rvin_dev *vin,
> -- 
> 2.20.1
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] media: rcar-csi2: Fix registering camera endpoint to VIN

[Lad, Prabhakar]

Hi Niklas,

Thank you for the review.

On Wed, Mar 4, 2020 at 8:47 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
>
> Hi Lad,
>
> Thanks for your work.
>
> On 2020-02-28 16:50:10 +0000, Lad Prabhakar wrote:
> > CSI2 registers camera/sensor as v4l2 async sub device with fwnode is
> > remote endpoint and the camera/sensor register itself as v4l2 sub device
> > with fwnode is remote device as a result the match.fwnode should be
> > fwnode_graph_get_remote_port_parent and not
> > fwnode_graph_get_remote_endpoint.
> >
> > This patch makes use of v4l2 helper function
> > v4l2_async_notifier_add_fwnode_remote_subdev() which uses
> > fwnode_graph_get_remote_port_parent as match.fwnode fixing the issue
> > of registering camera endpoint to the driver.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> I'm afraid this is not the right solution. The rcar-csi2 driver uses the
> v4l2-async framework to do endpoint matching instead of node matching.
> This is needed as it needs to work with the adv748x driver which
> register it self in v4l2-async using endpoints instead of nodes. The
> reason for this is that from a single DT node it creates multiple
> subdevices, one for each endpoint IIRC.
>
> Without this patch the two CSI-2 receivers on R-Car M3-n registers the
> two following 'paths' in v4l2 to be able to find the two subdevice CSI-2
> transmitters created by the ADV748x.
>
> rcar-csi2 fea80000.csi2: '/soc/i2c@e66d8000/video-receiver@70/port@b/endpoint'
> rcar-csi2 feaa0000.csi2: '/soc/i2c@e66d8000/video-receiver@70/port@a/endpoint'
>
> With this patch applied it registers the following which can't be found
> as they are not present in the v4l2-async list of subdevices (as they
> are registerd as above).
>
> rcar-csi2 fea80000.csi2: '/soc/i2c@e66d8000/video-receiver@70'
> rcar-csi2 feaa0000.csi2: '/soc/i2c@e66d8000/video-receiver@70'
> rcar-csi2: probe of feaa0000.csi2 failed with error -17
>
> This patch may unlock your use-case as it's a known problem that
> endpoint and node matching do not mix. But it will break the already
> upstream use-case and for that reason, I'm really sorry about this.
>
Completely missed it, thank you for pointing out.

> Nacked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
> The real solution to this problem IMHO is to make all of v4l2-async
> operate using endpoint matching or possibly some kind of fallback to
> node matching if no endpoint can be found. Never the less some work is
> required in the v4l2-async core to sort out node and endpoint matching
> coexistence.
>
Thank you for the pointers, I shall do some digging in v4l2-asyn.

Cheers,
--Prabhakar

> > ---
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index faa9fb23a2e9..5b04e4768eb1 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -833,20 +833,18 @@ static int rcsi2_parse_dt(struct rcar_csi2 *priv)
> >               return ret;
> >       }
> >
> > -     priv->asd.match.fwnode =
> > -             fwnode_graph_get_remote_endpoint(of_fwnode_handle(ep));
> > -     priv->asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> > -
> > -     of_node_put(ep);
> > -
> >       v4l2_async_notifier_init(&priv->notifier);
> >
> > -     ret = v4l2_async_notifier_add_subdev(&priv->notifier, &priv->asd);
> > +     ret = v4l2_async_notifier_add_fwnode_remote_subdev(&priv->notifier,
> > +                                                        of_fwnode_handle(ep),
> > +                                                        &priv->asd);
> >       if (ret) {
> > -             fwnode_handle_put(priv->asd.match.fwnode);
> > +             of_node_put(ep);
> >               return ret;
> >       }
> >
> > +     of_node_put(ep);
> > +
> >       priv->notifier.ops = &rcar_csi2_notify_ops;
> >
> >       dev_dbg(priv->dev, "Found '%pOF'\n",
> > --
> > 2.20.1
> >
>
> --
> Regards,
> Niklas Söderlund

Re: [PATCH 2/2] media: rcar-vin: Add support for SRGGB8_1X8

[Lad, Prabhakar]

Hi Niklas,

Thank you for the review.

On Wed, Mar 4, 2020 at 8:55 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
>
> Hi Lad,
>
> Thanks for your work.
>
> On 2020-02-28 16:50:11 +0000, Lad Prabhakar wrote:
> > Most of the RZ/G2x boards can support capturing frames in SRGGB8_1X8
> > format, add support for this with a runtime check that the running
> > hardware supports it.
>
> Where is the runtime check for supported hardware? Also out of curiosity
> which boars can't support SRGGB and what setup are you using to test
> SRGGB capture?
>
My bad l'll drop the statement "runtime check for supported hardware".
All the G2x
boards support SRGGB8. I am currently testing this on a G2E board connected with
a mipi adapter (https://www.96boards.org/product/mipiadapter/) and
raspberry pi camera
module (https://www.raspberrypi.org/products/camera-module-v2/).

> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-core.c |  1 +
> >  drivers/media/platform/rcar-vin/rcar-csi2.c |  1 +
> >  drivers/media/platform/rcar-vin/rcar-dma.c  | 14 ++++++++++++--
> >  drivers/media/platform/rcar-vin/rcar-v4l2.c |  4 ++++
>
> This patch shall be split in two. One which touches rcar-csi2.c and one
> for the other files as they are two different drivers.
>
sure will split this patch into two.

> >  4 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> > index 7440c8965d27..76daf2fe5bcd 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-core.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> > @@ -469,6 +469,7 @@ static int rvin_parallel_subdevice_attach(struct rvin_dev *vin,
> >               case MEDIA_BUS_FMT_UYVY8_2X8:
> >               case MEDIA_BUS_FMT_UYVY10_2X10:
> >               case MEDIA_BUS_FMT_RGB888_1X24:
> > +             case MEDIA_BUS_FMT_SRGGB8_1X8:
> >                       vin->mbus_code = code.code;
> >                       vin_dbg(vin, "Found media bus format for %s: %d\n",
> >                               subdev->name, vin->mbus_code);
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index 5b04e4768eb1..97e4886f63f0 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -320,6 +320,7 @@ static const struct rcar_csi2_format rcar_csi2_formats[] = {
> >       { .code = MEDIA_BUS_FMT_YUYV8_1X16,     .datatype = 0x1e, .bpp = 16 },
> >       { .code = MEDIA_BUS_FMT_UYVY8_2X8,      .datatype = 0x1e, .bpp = 16 },
> >       { .code = MEDIA_BUS_FMT_YUYV10_2X10,    .datatype = 0x1e, .bpp = 20 },
> > +     { .code = MEDIA_BUS_FMT_SRGGB8_1X8,     .datatype = 0x2a, .bpp = 8 },
> >  };
> >
> >  static const struct rcar_csi2_format *rcsi2_code_to_fmt(unsigned int code)
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index 1a30cd036371..63709b4bbf7a 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -85,6 +85,7 @@
> >  #define VNMC_INF_YUV8_BT601  (1 << 16)
> >  #define VNMC_INF_YUV10_BT656 (2 << 16)
> >  #define VNMC_INF_YUV10_BT601 (3 << 16)
> > +#define VNMC_INF_RAW8                (4 << 16)
> >  #define VNMC_INF_YUV16               (5 << 16)
> >  #define VNMC_INF_RGB888              (6 << 16)
> >  #define VNMC_VUP             (1 << 10)
> > @@ -587,13 +588,15 @@ void rvin_crop_scale_comp(struct rvin_dev *vin)
> >       rvin_write(vin, vin->crop.top, VNSLPRC_REG);
> >       rvin_write(vin, vin->crop.top + vin->crop.height - 1, VNELPRC_REG);
> >
> > -
> >       /* TODO: Add support for the UDS scaler. */
> >       if (vin->info->model != RCAR_GEN3)
> >               rvin_crop_scale_comp_gen2(vin);
> >
> >       fmt = rvin_format_from_pixel(vin, vin->format.pixelformat);
> > -     stride = vin->format.bytesperline / fmt->bpp;
> > +     if (vin->format.pixelformat == V4L2_PIX_FMT_SRGGB8)
> > +             stride = ALIGN(vin->format.bytesperline / 2, 0x10);
> > +     else
> > +             stride = vin->format.bytesperline / fmt->bpp;
>
> This does not look right, you should to move this logic into
> rvin_format_bytesperline().
>
sure will move it.

Cheers,
--Prabhakar

> >       rvin_write(vin, stride, VNIS_REG);
> >  }
> >
> > @@ -676,6 +679,9 @@ static int rvin_setup(struct rvin_dev *vin)
> >
> >               input_is_yuv = true;
> >               break;
> > +     case MEDIA_BUS_FMT_SRGGB8_1X8:
> > +             vnmc |= VNMC_INF_RAW8;
> > +             break;
> >       default:
> >               break;
> >       }
> > @@ -737,6 +743,9 @@ static int rvin_setup(struct rvin_dev *vin)
> >       case V4L2_PIX_FMT_ABGR32:
> >               dmr = VNDMR_A8BIT(vin->alpha) | VNDMR_EXRGB | VNDMR_DTMD_ARGB;
> >               break;
> > +     case V4L2_PIX_FMT_SRGGB8:
> > +             dmr = 0;
> > +             break;
> >       default:
> >               vin_err(vin, "Invalid pixelformat (0x%x)\n",
> >                       vin->format.pixelformat);
> > @@ -1110,6 +1119,7 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
> >       case MEDIA_BUS_FMT_UYVY8_2X8:
> >       case MEDIA_BUS_FMT_UYVY10_2X10:
> >       case MEDIA_BUS_FMT_RGB888_1X24:
> > +     case MEDIA_BUS_FMT_SRGGB8_1X8:
> >               vin->mbus_code = fmt.format.code;
> >               break;
> >       default:
> > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > index b12b3f88836a..d56cf85ba065 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> > @@ -66,6 +66,10 @@ static const struct rvin_video_format rvin_formats[] = {
> >               .fourcc                 = V4L2_PIX_FMT_ABGR32,
> >               .bpp                    = 4,
> >       },
> > +     {
> > +             .fourcc                 = V4L2_PIX_FMT_SRGGB8,
> > +             .bpp                    = 1,
> > +     },
> >  };
> >
> >  const struct rvin_video_format *rvin_format_from_pixel(struct rvin_dev *vin,
> > --
> > 2.20.1
> >
>
> --
> Regards,
> Niklas Söderlund