linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format
@ 2020-04-15 10:19 Lad Prabhakar
  2020-04-15 10:19 ` [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats Lad Prabhakar
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Lad Prabhakar @ 2020-04-15 10:19 UTC (permalink / raw)
  To: Niklas
  Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
	linux-kernel, Prabhakar, Lad Prabhakar

Hi,

This patch series adds support for MEDIA_BUS_FMT_SRGGB8_1X8 format for vin

Cheers,
--Prabhakar

Changed for v4:
* patch 1/3 is new patch which adds a check for conversion from input to
  output.
* patch 2/3 added a comment while setting VNIS_REG for RAW format as
  suggested by Niklas

Changes for v3:
* Dropped patch 1/1 from v2 as this handled neatly by patches
  https://patchwork.linuxtv.org/project/linux-media/list/?series=1974
* Included Ack from Niklas for patch 2/2
* Updated commit message for patch 1/1.

Changes for v2:
* Added support for matching fwnode against endpoints/nodes.
* Separated patch for rcar-vin and rcar-csi2.c which added
  MEDIA_BUS_FMT_SRGGB8_1X8.

Lad Prabhakar (3):
  media: rcar-vin: Invalidate pipeline if conversion is not possible on
    input formats
  media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format
  media: rcar-csi2: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format

 drivers/media/platform/rcar-vin/rcar-csi2.c |  1 +
 drivers/media/platform/rcar-vin/rcar-dma.c  | 21 +++++++++++++++++++--
 drivers/media/platform/rcar-vin/rcar-v4l2.c |  4 ++++
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats
  2020-04-15 10:19 [PATCH v4 0/3] media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
@ 2020-04-15 10:19 ` Lad Prabhakar
  2020-05-12 22:26   ` Niklas
  2020-04-15 10:19 ` [PATCH v4 2/3] media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Lad Prabhakar @ 2020-04-15 10:19 UTC (permalink / raw)
  To: Niklas
  Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
	linux-kernel, Prabhakar, Lad Prabhakar

Up until now the VIN was capable to convert any of its supported input mbus
formats to any of it's supported output pixel formats. With the addition of
RAW formats this is no longer true.
This patch invalidates the pipeline by adding a check if given vin input
format can be converted to supported output pixel format.

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

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 1a30cd036371..48bd9bfc3948 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1109,13 +1109,17 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
 	case MEDIA_BUS_FMT_UYVY8_1X16:
 	case MEDIA_BUS_FMT_UYVY8_2X8:
 	case MEDIA_BUS_FMT_UYVY10_2X10:
+		break;
 	case MEDIA_BUS_FMT_RGB888_1X24:
-		vin->mbus_code = fmt.format.code;
+		if (vin->format.pixelformat != V4L2_PIX_FMT_SRGGB8)
+			return -EPIPE;
 		break;
 	default:
 		return -EPIPE;
 	}
 
+	vin->mbus_code = fmt.format.code;
+
 	switch (fmt.format.field) {
 	case V4L2_FIELD_TOP:
 	case V4L2_FIELD_BOTTOM:
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 2/3] media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format
  2020-04-15 10:19 [PATCH v4 0/3] media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
  2020-04-15 10:19 ` [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats Lad Prabhakar
@ 2020-04-15 10:19 ` Lad Prabhakar
  2020-05-12 22:28   ` Niklas
  2020-04-15 10:19 ` [PATCH v4 3/3] media: rcar-csi2: " Lad Prabhakar
  2020-04-29 22:08 ` [PATCH v4 0/3] media: rcar-vin: Enable " Lad, Prabhakar
  3 siblings, 1 reply; 9+ messages in thread
From: Lad Prabhakar @ 2020-04-15 10:19 UTC (permalink / raw)
  To: Niklas
  Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
	linux-kernel, Prabhakar, Lad Prabhakar

Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format in rcar-vin by setting
format type to RAW8 in VNMC register and appropriately setting the bpp
and bytesperline to enable V4L2_PIX_FMT_SRGGB8.

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

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 48bd9bfc3948..3e8e29573e6d 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,19 @@ 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;
+
+	/* For RAW8 format bpp is 1, but the hardware process RAW8
+	 * format in 2 pixel unit hence configure VNIS_REG as stride / 2.
+	 */
+	if (vin->format.pixelformat == V4L2_PIX_FMT_SRGGB8)
+		stride /= 2;
+
 	rvin_write(vin, stride, VNIS_REG);
 }
 
@@ -676,6 +683,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 +747,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);
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 5151a3cd8a6e..ca542219e8ae 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v4 3/3] media: rcar-csi2: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format
  2020-04-15 10:19 [PATCH v4 0/3] media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
  2020-04-15 10:19 ` [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats Lad Prabhakar
  2020-04-15 10:19 ` [PATCH v4 2/3] media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
@ 2020-04-15 10:19 ` Lad Prabhakar
  2020-04-29 22:08 ` [PATCH v4 0/3] media: rcar-vin: Enable " Lad, Prabhakar
  3 siblings, 0 replies; 9+ messages in thread
From: Lad Prabhakar @ 2020-04-15 10:19 UTC (permalink / raw)
  To: Niklas
  Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
	linux-kernel, Prabhakar, Lad Prabhakar

This patch adds support for MEDIA_BUS_FMT_SRGGB8_1X8 format for CSI2
input.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index faa9fb23a2e9..3d1945d7996c 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)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 0/3] media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format
  2020-04-15 10:19 [PATCH v4 0/3] media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
                   ` (2 preceding siblings ...)
  2020-04-15 10:19 ` [PATCH v4 3/3] media: rcar-csi2: " Lad Prabhakar
@ 2020-04-29 22:08 ` Lad, Prabhakar
  3 siblings, 0 replies; 9+ messages in thread
From: Lad, Prabhakar @ 2020-04-29 22:08 UTC (permalink / raw)
  To: Niklas
  Cc: Mauro Carvalho Chehab, linux-media, Linux-Renesas, LKML, Lad Prabhakar

Hi Niklas,

On Wed, Apr 15, 2020 at 11:19 AM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
>
> Hi,
>
> This patch series adds support for MEDIA_BUS_FMT_SRGGB8_1X8 format for vin
>
> Cheers,
> --Prabhakar
>
> Changed for v4:
> * patch 1/3 is new patch which adds a check for conversion from input to
>   output.
> * patch 2/3 added a comment while setting VNIS_REG for RAW format as
>   suggested by Niklas
>
> Changes for v3:
> * Dropped patch 1/1 from v2 as this handled neatly by patches
>   https://patchwork.linuxtv.org/project/linux-media/list/?series=1974
> * Included Ack from Niklas for patch 2/2
> * Updated commit message for patch 1/1.
>
> Changes for v2:
> * Added support for matching fwnode against endpoints/nodes.
> * Separated patch for rcar-vin and rcar-csi2.c which added
>   MEDIA_BUS_FMT_SRGGB8_1X8.
>
> Lad Prabhakar (3):
>   media: rcar-vin: Invalidate pipeline if conversion is not possible on
>     input formats
>   media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format
>   media: rcar-csi2: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format
>
Gentle ping.

Cheers,
--Prabhakar

>  drivers/media/platform/rcar-vin/rcar-csi2.c |  1 +
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 21 +++++++++++++++++++--
>  drivers/media/platform/rcar-vin/rcar-v4l2.c |  4 ++++
>  3 files changed, 24 insertions(+), 2 deletions(-)
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats
  2020-04-15 10:19 ` [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats Lad Prabhakar
@ 2020-05-12 22:26   ` Niklas
  2020-05-13 11:16     ` Lad, Prabhakar
  0 siblings, 1 reply; 9+ messages in thread
From: Niklas @ 2020-05-12 22:26 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
	linux-kernel, Prabhakar

Hi Lad,

Thanks for your work.

On 2020-04-15 11:19:06 +0100, Lad Prabhakar wrote:
> Up until now the VIN was capable to convert any of its supported input mbus
> formats to any of it's supported output pixel formats. With the addition of
> RAW formats this is no longer true.

Add blank line.

> This patch invalidates the pipeline by adding a check if given vin input
> format can be converted to supported output pixel format.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

I like this patch I think there is a typo bellow and patch [1] have been 
merged in the media-tree which unfortunately addes one more thing to do 
in this patch. In rvin_enum_fmt_vid_cap() there is a TODO noted for what 
needs to be done. In imagine the fix is simple and the end result would 
look something like this.

    switch (f->mbus_code) {
    case 0:
    case MEDIA_BUS_FMT_YUYV8_1X16:
    case MEDIA_BUS_FMT_UYVY8_1X16:
    case MEDIA_BUS_FMT_UYVY8_2X8:
    case MEDIA_BUS_FMT_UYVY10_2X10:
    case MEDIA_BUS_FMT_RGB888_1X24:
        break;
    case MEDIA_BUS_FMT_SRGGB8_1X8:
        if (f->index)
            return -EINVAL;

        f->pixelformat = V4L2_PIX_FMT_SRGGB8;
        return 0;
    case default:
        return -EINVAL;
    }

1. d5f74a1eff9aef3b ("media: rcar-vin: Make use of V4L2_CAP_IO_MC")

> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 1a30cd036371..48bd9bfc3948 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1109,13 +1109,17 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
>  	case MEDIA_BUS_FMT_UYVY8_1X16:
>  	case MEDIA_BUS_FMT_UYVY8_2X8:
>  	case MEDIA_BUS_FMT_UYVY10_2X10:
> +		break;
>  	case MEDIA_BUS_FMT_RGB888_1X24:
> -		vin->mbus_code = fmt.format.code;

This is not right is it?

Should you not add a case for MEDIA_BUS_FMT_SRGGB8_1X8 instead of taking 
over MEDIA_BUS_FMT_RGB888_1X24?

> +		if (vin->format.pixelformat != V4L2_PIX_FMT_SRGGB8)
> +			return -EPIPE;
>  		break;
>  	default:
>  		return -EPIPE;
>  	}
>  
> +	vin->mbus_code = fmt.format.code;
> +
>  	switch (fmt.format.field) {
>  	case V4L2_FIELD_TOP:
>  	case V4L2_FIELD_BOTTOM:
> -- 
> 2.20.1
> 

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 2/3] media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format
  2020-04-15 10:19 ` [PATCH v4 2/3] media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
@ 2020-05-12 22:28   ` Niklas
  0 siblings, 0 replies; 9+ messages in thread
From: Niklas @ 2020-05-12 22:28 UTC (permalink / raw)
  To: Lad Prabhakar
  Cc: Mauro Carvalho Chehab, linux-media, linux-renesas-soc,
	linux-kernel, Prabhakar

Hi Lad,

Thanks for your work.

On 2020-04-15 11:19:07 +0100, Lad Prabhakar wrote:
> Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format in rcar-vin by setting
> format type to RAW8 in VNMC register and appropriately setting the bpp
> and bytesperline to enable V4L2_PIX_FMT_SRGGB8.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

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

> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c  | 15 ++++++++++++++-
>  drivers/media/platform/rcar-vin/rcar-v4l2.c |  4 ++++
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index 48bd9bfc3948..3e8e29573e6d 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,19 @@ 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;
> +
> +	/* For RAW8 format bpp is 1, but the hardware process RAW8
> +	 * format in 2 pixel unit hence configure VNIS_REG as stride / 2.
> +	 */
> +	if (vin->format.pixelformat == V4L2_PIX_FMT_SRGGB8)
> +		stride /= 2;
> +
>  	rvin_write(vin, stride, VNIS_REG);
>  }
>  
> @@ -676,6 +683,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 +747,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);
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 5151a3cd8a6e..ca542219e8ae 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats
  2020-05-12 22:26   ` Niklas
@ 2020-05-13 11:16     ` Lad, Prabhakar
  2020-05-19 13:12       ` Niklas
  0 siblings, 1 reply; 9+ messages in thread
From: Lad, Prabhakar @ 2020-05-13 11:16 UTC (permalink / raw)
  To: Niklas
  Cc: Lad Prabhakar, Mauro Carvalho Chehab, linux-media, Linux-Renesas, LKML

Hi Niklas,

Thank you for the review.

On Tue, May 12, 2020 at 11:26 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
>
> Hi Lad,
>
> Thanks for your work.
>
> On 2020-04-15 11:19:06 +0100, Lad Prabhakar wrote:
> > Up until now the VIN was capable to convert any of its supported input mbus
> > formats to any of it's supported output pixel formats. With the addition of
> > RAW formats this is no longer true.
>
> Add blank line.
>
> > This patch invalidates the pipeline by adding a check if given vin input
> > format can be converted to supported output pixel format.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> I like this patch I think there is a typo bellow and patch [1] have been
> merged in the media-tree which unfortunately addes one more thing to do
> in this patch. In rvin_enum_fmt_vid_cap() there is a TODO noted for what
> needs to be done. In imagine the fix is simple and the end result would
> look something like this.
>
>     switch (f->mbus_code) {
>     case 0:
>     case MEDIA_BUS_FMT_YUYV8_1X16:
>     case MEDIA_BUS_FMT_UYVY8_1X16:
>     case MEDIA_BUS_FMT_UYVY8_2X8:
>     case MEDIA_BUS_FMT_UYVY10_2X10:
>     case MEDIA_BUS_FMT_RGB888_1X24:
>         break;
>     case MEDIA_BUS_FMT_SRGGB8_1X8:
>         if (f->index)
>             return -EINVAL;
>
>         f->pixelformat = V4L2_PIX_FMT_SRGGB8;
>         return 0;
>     case default:
>         return -EINVAL;
>     }
>
> 1. d5f74a1eff9aef3b ("media: rcar-vin: Make use of V4L2_CAP_IO_MC")
>
Sure Ill take of care of this and just repost this patch is that OK with you ?

> > ---
> >  drivers/media/platform/rcar-vin/rcar-dma.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index 1a30cd036371..48bd9bfc3948 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -1109,13 +1109,17 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
> >       case MEDIA_BUS_FMT_UYVY8_1X16:
> >       case MEDIA_BUS_FMT_UYVY8_2X8:
> >       case MEDIA_BUS_FMT_UYVY10_2X10:
> > +             break;
> >       case MEDIA_BUS_FMT_RGB888_1X24:
> > -             vin->mbus_code = fmt.format.code;
>
> This is not right is it?
>
> Should you not add a case for MEDIA_BUS_FMT_SRGGB8_1X8 instead of taking
> over MEDIA_BUS_FMT_RGB888_1X24?
>
Agreed, I blindly took this suggestion from your previous comments [1].

[1] https://lkml.org/lkml/2020/3/19/858

Cheers,
--Prabhakar Lad

> > +             if (vin->format.pixelformat != V4L2_PIX_FMT_SRGGB8)
> > +                     return -EPIPE;
> >               break;
> >       default:
> >               return -EPIPE;
> >       }
> >
> > +     vin->mbus_code = fmt.format.code;
> > +
> >       switch (fmt.format.field) {
> >       case V4L2_FIELD_TOP:
> >       case V4L2_FIELD_BOTTOM:
> > --
> > 2.20.1
> >
>
> --
> Regards,
> Niklas Söderlund

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats
  2020-05-13 11:16     ` Lad, Prabhakar
@ 2020-05-19 13:12       ` Niklas
  0 siblings, 0 replies; 9+ messages in thread
From: Niklas @ 2020-05-19 13:12 UTC (permalink / raw)
  To: Lad, Prabhakar
  Cc: Lad Prabhakar, Mauro Carvalho Chehab, linux-media, Linux-Renesas, LKML

Hi Lad,

On 2020-05-13 12:16:00 +0100, Lad, Prabhakar wrote:
> Hi Niklas,
> 
> Thank you for the review.
> 
> On Tue, May 12, 2020 at 11:26 PM Niklas <niklas.soderlund@ragnatech.se> wrote:
> >
> > Hi Lad,
> >
> > Thanks for your work.
> >
> > On 2020-04-15 11:19:06 +0100, Lad Prabhakar wrote:
> > > Up until now the VIN was capable to convert any of its supported input mbus
> > > formats to any of it's supported output pixel formats. With the addition of
> > > RAW formats this is no longer true.
> >
> > Add blank line.
> >
> > > This patch invalidates the pipeline by adding a check if given vin input
> > > format can be converted to supported output pixel format.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > I like this patch I think there is a typo bellow and patch [1] have been
> > merged in the media-tree which unfortunately addes one more thing to do
> > in this patch. In rvin_enum_fmt_vid_cap() there is a TODO noted for what
> > needs to be done. In imagine the fix is simple and the end result would
> > look something like this.
> >
> >     switch (f->mbus_code) {
> >     case 0:
> >     case MEDIA_BUS_FMT_YUYV8_1X16:
> >     case MEDIA_BUS_FMT_UYVY8_1X16:
> >     case MEDIA_BUS_FMT_UYVY8_2X8:
> >     case MEDIA_BUS_FMT_UYVY10_2X10:
> >     case MEDIA_BUS_FMT_RGB888_1X24:
> >         break;
> >     case MEDIA_BUS_FMT_SRGGB8_1X8:
> >         if (f->index)
> >             return -EINVAL;
> >
> >         f->pixelformat = V4L2_PIX_FMT_SRGGB8;
> >         return 0;
> >     case default:
> >         return -EINVAL;
> >     }
> >
> > 1. d5f74a1eff9aef3b ("media: rcar-vin: Make use of V4L2_CAP_IO_MC")
> >
> Sure Ill take of care of this and just repost this patch is that OK with you ?

Yes, also please test it as you point out bellow my last suggesting had 
a typo which would break it ;-)

> 
> > > ---
> > >  drivers/media/platform/rcar-vin/rcar-dma.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > index 1a30cd036371..48bd9bfc3948 100644
> > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > > @@ -1109,13 +1109,17 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
> > >       case MEDIA_BUS_FMT_UYVY8_1X16:
> > >       case MEDIA_BUS_FMT_UYVY8_2X8:
> > >       case MEDIA_BUS_FMT_UYVY10_2X10:
> > > +             break;
> > >       case MEDIA_BUS_FMT_RGB888_1X24:
> > > -             vin->mbus_code = fmt.format.code;
> >
> > This is not right is it?
> >
> > Should you not add a case for MEDIA_BUS_FMT_SRGGB8_1X8 instead of taking
> > over MEDIA_BUS_FMT_RGB888_1X24?
> >
> Agreed, I blindly took this suggestion from your previous comments [1].
> 
> [1] https://lkml.org/lkml/2020/3/19/858
> 
> Cheers,
> --Prabhakar Lad
> 
> > > +             if (vin->format.pixelformat != V4L2_PIX_FMT_SRGGB8)
> > > +                     return -EPIPE;
> > >               break;
> > >       default:
> > >               return -EPIPE;
> > >       }
> > >
> > > +     vin->mbus_code = fmt.format.code;
> > > +
> > >       switch (fmt.format.field) {
> > >       case V4L2_FIELD_TOP:
> > >       case V4L2_FIELD_BOTTOM:
> > > --
> > > 2.20.1
> > >
> >
> > --
> > Regards,
> > Niklas Söderlund

-- 
Regards,
Niklas Söderlund

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-05-19 13:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-15 10:19 [PATCH v4 0/3] media: rcar-vin: Enable MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
2020-04-15 10:19 ` [PATCH v4 1/3] media: rcar-vin: Invalidate pipeline if conversion is not possible on input formats Lad Prabhakar
2020-05-12 22:26   ` Niklas
2020-05-13 11:16     ` Lad, Prabhakar
2020-05-19 13:12       ` Niklas
2020-04-15 10:19 ` [PATCH v4 2/3] media: rcar-vin: Add support for MEDIA_BUS_FMT_SRGGB8_1X8 format Lad Prabhakar
2020-05-12 22:28   ` Niklas
2020-04-15 10:19 ` [PATCH v4 3/3] media: rcar-csi2: " Lad Prabhakar
2020-04-29 22:08 ` [PATCH v4 0/3] media: rcar-vin: Enable " Lad, Prabhakar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).