[PATCH 0/3] media: vimc: Add support for GBR and BGR formats on source pad of debayer

[PATCH 0/3] media: vimc: Add support for GBR and BGR formats on source pad of debayer

[Nícolas F. R. A. Prado]

The objective of this series is to add support for GBR and BGR media bus formats
for the source pad of debayer subdevices of the vimc driver.

Since the GBR media bus code doesn't have a corresponding pixelformat, it needed
to use the pixelformat of another bus code.

The first patch makes it possible to have multiple media bus codes mapping to
the same pixelformat.

The second patch adds the GBR media bus code, using the RGB pixelformat.

The third patch adds support for GBR and BGR media bus formats on the source
pad of the debayer subdevice.

This patch series passed all tests of v4l2-compliance:
$ compliance_git -m /dev/media0
v4l2-compliance SHA: c4a62f26c5c3ecd856ca10cf2f0d35d100283d7f, 64 bits, 64-bit time_t

Grand Total for vimc device /dev/media0: 461, Succeeded: 461, Failed: 0, Warnings: 0

Nícolas F. R. A. Prado (3):
  media: vimc: Support multiple buscodes for each pixelformat
  media: vimc: Add GBR media bus code
  media: vimc: deb: Add support for GBR and BGR bus formats on source
    pad

 drivers/media/platform/vimc/vimc-common.c  | 68 +++++++++++++---------
 drivers/media/platform/vimc/vimc-common.h  |  9 ++-
 drivers/media/platform/vimc/vimc-debayer.c | 53 +++++++++++++----
 drivers/media/platform/vimc/vimc-scaler.c  | 10 +++-
 drivers/media/platform/vimc/vimc-sensor.c  |  6 +-
 5 files changed, 102 insertions(+), 44 deletions(-)

-- 
2.25.0

[PATCH 1/3] media: vimc: Support multiple buscodes for each pixelformat

[Nícolas F. R. A. Prado]

Change vimc_pix_map_list to allow multiple media bus codes to map to the
same pixelformat, making it possible to add media bus codes for which
there are no pixelformat.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>
---
 drivers/media/platform/vimc/vimc-common.c | 68 ++++++++++++++---------
 drivers/media/platform/vimc/vimc-common.h |  9 ++-
 drivers/media/platform/vimc/vimc-scaler.c | 10 +++-
 drivers/media/platform/vimc/vimc-sensor.c |  6 +-
 4 files changed, 61 insertions(+), 32 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
index 16ce9f3b7c75..55797e5ab2b1 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -19,19 +19,19 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
 
 	/* RGB formats */
 	{
-		.code = MEDIA_BUS_FMT_BGR888_1X24,
+		.code = {MEDIA_BUS_FMT_BGR888_1X24},
 		.pixelformat = V4L2_PIX_FMT_BGR24,
 		.bpp = 3,
 		.bayer = false,
 	},
 	{
-		.code = MEDIA_BUS_FMT_RGB888_1X24,
+		.code = {MEDIA_BUS_FMT_RGB888_1X24},
 		.pixelformat = V4L2_PIX_FMT_RGB24,
 		.bpp = 3,
 		.bayer = false,
 	},
 	{
-		.code = MEDIA_BUS_FMT_ARGB8888_1X32,
+		.code = {MEDIA_BUS_FMT_ARGB8888_1X32},
 		.pixelformat = V4L2_PIX_FMT_ARGB32,
 		.bpp = 4,
 		.bayer = false,
@@ -39,49 +39,49 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
 
 	/* Bayer formats */
 	{
-		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
+		.code = {MEDIA_BUS_FMT_SBGGR8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SBGGR8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
+		.code = {MEDIA_BUS_FMT_SGBRG8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SGBRG8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
+		.code = {MEDIA_BUS_FMT_SGRBG8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SGRBG8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
+		.code = {MEDIA_BUS_FMT_SRGGB8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SRGGB8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
+		.code = {MEDIA_BUS_FMT_SBGGR10_1X10},
 		.pixelformat = V4L2_PIX_FMT_SBGGR10,
 		.bpp = 2,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
+		.code = {MEDIA_BUS_FMT_SGBRG10_1X10},
 		.pixelformat = V4L2_PIX_FMT_SGBRG10,
 		.bpp = 2,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
+		.code = {MEDIA_BUS_FMT_SGRBG10_1X10},
 		.pixelformat = V4L2_PIX_FMT_SGRBG10,
 		.bpp = 2,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
+		.code = {MEDIA_BUS_FMT_SRGGB10_1X10},
 		.pixelformat = V4L2_PIX_FMT_SRGGB10,
 		.bpp = 2,
 		.bayer = true,
@@ -89,25 +89,25 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
 
 	/* 10bit raw bayer a-law compressed to 8 bits */
 	{
-		.code = MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8,
+		.code = {MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SBGGR10ALAW8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8,
+		.code = {MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SGBRG10ALAW8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8,
+		.code = {MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SGRBG10ALAW8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8,
+		.code = {MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SRGGB10ALAW8,
 		.bpp = 1,
 		.bayer = true,
@@ -115,49 +115,49 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
 
 	/* 10bit raw bayer DPCM compressed to 8 bits */
 	{
-		.code = MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8,
+		.code = {MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SBGGR10DPCM8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8,
+		.code = {MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SGBRG10DPCM8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
+		.code = {MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SGRBG10DPCM8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8,
+		.code = {MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8},
 		.pixelformat = V4L2_PIX_FMT_SRGGB10DPCM8,
 		.bpp = 1,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
+		.code = {MEDIA_BUS_FMT_SBGGR12_1X12},
 		.pixelformat = V4L2_PIX_FMT_SBGGR12,
 		.bpp = 2,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
+		.code = {MEDIA_BUS_FMT_SGBRG12_1X12},
 		.pixelformat = V4L2_PIX_FMT_SGBRG12,
 		.bpp = 2,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
+		.code = {MEDIA_BUS_FMT_SGRBG12_1X12},
 		.pixelformat = V4L2_PIX_FMT_SGRBG12,
 		.bpp = 2,
 		.bayer = true,
 	},
 	{
-		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
+		.code = {MEDIA_BUS_FMT_SRGGB12_1X12},
 		.pixelformat = V4L2_PIX_FMT_SRGGB12,
 		.bpp = 2,
 		.bayer = true,
@@ -182,13 +182,29 @@ const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i)
 	return &vimc_pix_map_list[i];
 }
 
+const u32 vimc_mbus_code_by_index(unsigned int i)
+{
+	unsigned int j, k;
+
+	for (j = 0; j < ARRAY_SIZE(vimc_pix_map_list); j++) {
+		for (k = 0; vimc_pix_map_list[j].code[k]; k++) {
+			if (!i)
+				return vimc_pix_map_list[j].code[k];
+			i--;
+		}
+	}
+	return 0;
+}
+
 const struct vimc_pix_map *vimc_pix_map_by_code(u32 code)
 {
-	unsigned int i;
+	unsigned int i, j;
 
 	for (i = 0; i < ARRAY_SIZE(vimc_pix_map_list); i++) {
-		if (vimc_pix_map_list[i].code == code)
-			return &vimc_pix_map_list[i];
+		for (j = 0; j < ARRAY_SIZE(vimc_pix_map_list[i].code); j++) {
+			if (vimc_pix_map_list[i].code[j] == code)
+				return &vimc_pix_map_list[i];
+		}
 	}
 	return NULL;
 }
diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
index 87eb8259c2a8..132a5889f1ea 100644
--- a/drivers/media/platform/vimc/vimc-common.h
+++ b/drivers/media/platform/vimc/vimc-common.h
@@ -69,7 +69,7 @@ do {									\
  * V4L2_PIX_FMT_* fourcc pixelformat and its bytes per pixel (bpp)
  */
 struct vimc_pix_map {
-	unsigned int code;
+	unsigned int code[3];
 	unsigned int bpp;
 	u32 pixelformat;
 	bool bayer;
@@ -171,6 +171,13 @@ void vimc_sen_rm(struct vimc_device *vimc, struct vimc_ent_device *ved);
  */
 const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i);
 
+/**
+ * vimc_mbus_code_by_index - get mbus code by its index
+ *
+ * @i:			index of the mbus code in vimc_pix_map_list
+ */
+const u32 vimc_mbus_code_by_index(unsigned int i);
+
 /**
  * vimc_pix_map_by_code - get vimc_pix_map struct by media bus code
  *
diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
index e2e551bc3ded..d2ba0a2cfcc4 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -110,13 +110,19 @@ static int vimc_sca_enum_mbus_code(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_pad_config *cfg,
 				   struct v4l2_subdev_mbus_code_enum *code)
 {
-	const struct vimc_pix_map *vpix = vimc_pix_map_by_index(code->index);
+	const u32 mbus_code = vimc_mbus_code_by_index(code->index);
+	const struct vimc_pix_map *vpix;
+
+	if (!mbus_code)
+		return -EINVAL;
+
+	vpix = vimc_pix_map_by_code(mbus_code);
 
 	/* We don't support bayer format */
 	if (!vpix || vpix->bayer)
 		return -EINVAL;
 
-	code->code = vpix->code;
+	code->code = mbus_code;
 
 	return 0;
 }
diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
index 32380f504591..9f4dd7fee9ab 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -52,12 +52,12 @@ static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_pad_config *cfg,
 				   struct v4l2_subdev_mbus_code_enum *code)
 {
-	const struct vimc_pix_map *vpix = vimc_pix_map_by_index(code->index);
+	const u32 mbus_code = vimc_mbus_code_by_index(code->index);
 
-	if (!vpix)
+	if (!mbus_code)
 		return -EINVAL;
 
-	code->code = vpix->code;
+	code->code = mbus_code;
 
 	return 0;
 }
-- 
2.25.0

[PATCH 2/3] media: vimc: Add GBR media bus code

[Nícolas F. R. A. Prado]

Add missing GBR888_1X24 media bus code in the vimc_pix_map_list. Since
there is no pixel format for it, use the pixelformat for RGB.

Co-developed-by: Vitor Massaru Iha <vitor@massaru.org>
Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>
---
 drivers/media/platform/vimc/vimc-common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
index 55797e5ab2b1..591a50f63766 100644
--- a/drivers/media/platform/vimc/vimc-common.c
+++ b/drivers/media/platform/vimc/vimc-common.c
@@ -25,7 +25,7 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
 		.bayer = false,
 	},
 	{
-		.code = {MEDIA_BUS_FMT_RGB888_1X24},
+		.code = {MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_GBR888_1X24},
 		.pixelformat = V4L2_PIX_FMT_RGB24,
 		.bpp = 3,
 		.bayer = false,
-- 
2.25.0

[PATCH 3/3] media: vimc: deb: Add support for GBR and BGR bus formats on source pad

[Nícolas F. R. A. Prado]

Add support for GBR and BGR media bus formats for the source pad of
debayer subdevices.

Co-developed-by: Vitor Massaru Iha <vitor@massaru.org>
Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>
---
 drivers/media/platform/vimc/vimc-debayer.c | 53 +++++++++++++++++-----
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
index 5d1b67d684bb..463cafbe107e 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -51,6 +51,11 @@ static const struct v4l2_mbus_framefmt sink_fmt_default = {
 	.colorspace = V4L2_COLORSPACE_DEFAULT,
 };
 
+static const u32 src_rgb_codes[] = {
+	MEDIA_BUS_FMT_BGR888_1X24,
+	MEDIA_BUS_FMT_RGB888_1X24,
+	MEDIA_BUS_FMT_GBR888_1X24};
+
 static const struct vimc_deb_pix_map vimc_deb_pix_map_list[] = {
 	{
 		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
@@ -148,14 +153,11 @@ static int vimc_deb_enum_mbus_code(struct v4l2_subdev *sd,
 				   struct v4l2_subdev_pad_config *cfg,
 				   struct v4l2_subdev_mbus_code_enum *code)
 {
-	/* We only support one format for source pads */
 	if (VIMC_IS_SRC(code->pad)) {
-		struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
-
-		if (code->index)
+		if (code->index >= ARRAY_SIZE(src_rgb_codes))
 			return -EINVAL;
 
-		code->code = vdeb->src_code;
+		code->code = src_rgb_codes[code->index];
 	} else {
 		if (code->index >= ARRAY_SIZE(vimc_deb_pix_map_list))
 			return -EINVAL;
@@ -170,7 +172,7 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd,
 				    struct v4l2_subdev_pad_config *cfg,
 				    struct v4l2_subdev_frame_size_enum *fse)
 {
-	struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
+	int i;
 
 	if (fse->index)
 		return -EINVAL;
@@ -181,8 +183,13 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd,
 
 		if (!vpix)
 			return -EINVAL;
-	} else if (fse->code != vdeb->src_code) {
-		return -EINVAL;
+	} else {
+		for (i = 0; i < ARRAY_SIZE(src_rgb_codes); i++) {
+			if (src_rgb_codes[i] == fse->code)
+				break;
+		}
+		if (i == ARRAY_SIZE(src_rgb_codes))
+			return -EINVAL;
 	}
 
 	fse->min_width = VIMC_FRAME_MIN_WIDTH;
@@ -237,6 +244,8 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd,
 {
 	struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
 	struct v4l2_mbus_framefmt *sink_fmt;
+	unsigned int i;
+	u32 *src_code;
 
 	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
 		/* Do not change the format while stream is on */
@@ -244,8 +253,10 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd,
 			return -EBUSY;
 
 		sink_fmt = &vdeb->sink_fmt;
+		src_code = &vdeb->src_code;
 	} else {
 		sink_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+		src_code = &v4l2_subdev_get_try_format(sd, cfg, 1)->code;
 	}
 
 	/*
@@ -253,9 +264,17 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd,
 	 * it is propagated from the sink
 	 */
 	if (VIMC_IS_SRC(fmt->pad)) {
+		u32 code = fmt->format.code;
+
 		fmt->format = *sink_fmt;
-		/* TODO: Add support for other formats */
-		fmt->format.code = vdeb->src_code;
+
+		for (i = 0; i < ARRAY_SIZE(src_rgb_codes); i++) {
+			if (src_rgb_codes[i] == code) {
+				*src_code = src_rgb_codes[i];
+				break;
+			}
+		}
+		fmt->format.code = *src_code;
 	} else {
 		/* Set the new format in the sink pad */
 		vimc_deb_adjust_sink_fmt(&fmt->format);
@@ -291,11 +310,21 @@ static void vimc_deb_set_rgb_mbus_fmt_rgb888_1x24(struct vimc_deb_device *vdeb,
 						  unsigned int col,
 						  unsigned int rgb[3])
 {
+	const struct vimc_pix_map *vpix;
 	unsigned int i, index;
 
+	vpix = vimc_pix_map_by_code(vdeb->src_code);
 	index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 3);
-	for (i = 0; i < 3; i++)
-		vdeb->src_frame[index + i] = rgb[i];
+	for (i = 0; i < 3; i++) {
+		switch (vpix->pixelformat) {
+		case V4L2_PIX_FMT_RGB24:
+			vdeb->src_frame[index + i] = rgb[i];
+			break;
+		case V4L2_PIX_FMT_BGR24:
+			vdeb->src_frame[index + i] = rgb[2-i];
+			break;
+		}
+	}
 }
 
 static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
-- 
2.25.0

Re: [PATCH 1/3] media: vimc: Support multiple buscodes for each pixelformat

[Helen Koike]

Hi Nícolas,

Thanks for the patch, just some minor nits:

On 2/2/20 1:50 PM, Nícolas F. R. A. Prado wrote:
> Change vimc_pix_map_list to allow multiple media bus codes to map to the
> same pixelformat, making it possible to add media bus codes for which
> there are no pixelformat.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>
> ---
>  drivers/media/platform/vimc/vimc-common.c | 68 ++++++++++++++---------
>  drivers/media/platform/vimc/vimc-common.h |  9 ++-
>  drivers/media/platform/vimc/vimc-scaler.c | 10 +++-
>  drivers/media/platform/vimc/vimc-sensor.c |  6 +-
>  4 files changed, 61 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
> index 16ce9f3b7c75..55797e5ab2b1 100644
> --- a/drivers/media/platform/vimc/vimc-common.c
> +++ b/drivers/media/platform/vimc/vimc-common.c
> @@ -19,19 +19,19 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
>  
>  	/* RGB formats */
>  	{
> -		.code = MEDIA_BUS_FMT_BGR888_1X24,
> +		.code = {MEDIA_BUS_FMT_BGR888_1X24},

Could you add spaces around the curly brackets?
Example: https://git.linuxtv.org/media_tree.git/tree/drivers/media/common/v4l2-tpg/v4l2-tpg-colors.c#n35

>  		.pixelformat = V4L2_PIX_FMT_BGR24,
>  		.bpp = 3,
>  		.bayer = false,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_RGB888_1X24,
> +		.code = {MEDIA_BUS_FMT_RGB888_1X24},
>  		.pixelformat = V4L2_PIX_FMT_RGB24,
>  		.bpp = 3,
>  		.bayer = false,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_ARGB8888_1X32,
> +		.code = {MEDIA_BUS_FMT_ARGB8888_1X32},
>  		.pixelformat = V4L2_PIX_FMT_ARGB32,
>  		.bpp = 4,
>  		.bayer = false,
> @@ -39,49 +39,49 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
>  
>  	/* Bayer formats */
>  	{
> -		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
> +		.code = {MEDIA_BUS_FMT_SBGGR8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SBGGR8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGBRG8_1X8,
> +		.code = {MEDIA_BUS_FMT_SGBRG8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SGBRG8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGRBG8_1X8,
> +		.code = {MEDIA_BUS_FMT_SGRBG8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SGRBG8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SRGGB8_1X8,
> +		.code = {MEDIA_BUS_FMT_SRGGB8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SRGGB8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SBGGR10_1X10,
> +		.code = {MEDIA_BUS_FMT_SBGGR10_1X10},
>  		.pixelformat = V4L2_PIX_FMT_SBGGR10,
>  		.bpp = 2,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGBRG10_1X10,
> +		.code = {MEDIA_BUS_FMT_SGBRG10_1X10},
>  		.pixelformat = V4L2_PIX_FMT_SGBRG10,
>  		.bpp = 2,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGRBG10_1X10,
> +		.code = {MEDIA_BUS_FMT_SGRBG10_1X10},
>  		.pixelformat = V4L2_PIX_FMT_SGRBG10,
>  		.bpp = 2,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SRGGB10_1X10,
> +		.code = {MEDIA_BUS_FMT_SRGGB10_1X10},
>  		.pixelformat = V4L2_PIX_FMT_SRGGB10,
>  		.bpp = 2,
>  		.bayer = true,
> @@ -89,25 +89,25 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
>  
>  	/* 10bit raw bayer a-law compressed to 8 bits */
>  	{
> -		.code = MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8,
> +		.code = {MEDIA_BUS_FMT_SBGGR10_ALAW8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SBGGR10ALAW8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8,
> +		.code = {MEDIA_BUS_FMT_SGBRG10_ALAW8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SGBRG10ALAW8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8,
> +		.code = {MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SGRBG10ALAW8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8,
> +		.code = {MEDIA_BUS_FMT_SRGGB10_ALAW8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SRGGB10ALAW8,
>  		.bpp = 1,
>  		.bayer = true,
> @@ -115,49 +115,49 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
>  
>  	/* 10bit raw bayer DPCM compressed to 8 bits */
>  	{
> -		.code = MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8,
> +		.code = {MEDIA_BUS_FMT_SBGGR10_DPCM8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SBGGR10DPCM8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8,
> +		.code = {MEDIA_BUS_FMT_SGBRG10_DPCM8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SGBRG10DPCM8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
> +		.code = {MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SGRBG10DPCM8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8,
> +		.code = {MEDIA_BUS_FMT_SRGGB10_DPCM8_1X8},
>  		.pixelformat = V4L2_PIX_FMT_SRGGB10DPCM8,
>  		.bpp = 1,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SBGGR12_1X12,
> +		.code = {MEDIA_BUS_FMT_SBGGR12_1X12},
>  		.pixelformat = V4L2_PIX_FMT_SBGGR12,
>  		.bpp = 2,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGBRG12_1X12,
> +		.code = {MEDIA_BUS_FMT_SGBRG12_1X12},
>  		.pixelformat = V4L2_PIX_FMT_SGBRG12,
>  		.bpp = 2,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SGRBG12_1X12,
> +		.code = {MEDIA_BUS_FMT_SGRBG12_1X12},
>  		.pixelformat = V4L2_PIX_FMT_SGRBG12,
>  		.bpp = 2,
>  		.bayer = true,
>  	},
>  	{
> -		.code = MEDIA_BUS_FMT_SRGGB12_1X12,
> +		.code = {MEDIA_BUS_FMT_SRGGB12_1X12},
>  		.pixelformat = V4L2_PIX_FMT_SRGGB12,
>  		.bpp = 2,
>  		.bayer = true,
> @@ -182,13 +182,29 @@ const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i)
>  	return &vimc_pix_map_list[i];
>  }
>  
> +const u32 vimc_mbus_code_by_index(unsigned int i)> +{
> +	unsigned int j, k;

I would rename

s/i/index
s/j/i
s/k/j

> +
> +	for (j = 0; j < ARRAY_SIZE(vimc_pix_map_list); j++) {
> +		for (k = 0; vimc_pix_map_list[j].code[k]; k++) {
> +			if (!i)
> +				return vimc_pix_map_list[j].code[k];
> +			i--;
> +		}
> +	}
> +	return 0;
> +}
> +
>  const struct vimc_pix_map *vimc_pix_map_by_code(u32 code)
>  {
> -	unsigned int i;
> +	unsigned int i, j;
>  
>  	for (i = 0; i < ARRAY_SIZE(vimc_pix_map_list); i++) {
> -		if (vimc_pix_map_list[i].code == code)
> -			return &vimc_pix_map_list[i];
> +		for (j = 0; j < ARRAY_SIZE(vimc_pix_map_list[i].code); j++) {
> +			if (vimc_pix_map_list[i].code[j] == code)
> +				return &vimc_pix_map_list[i];
> +		}
>  	}
>  	return NULL;
>  }
> diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> index 87eb8259c2a8..132a5889f1ea 100644
> --- a/drivers/media/platform/vimc/vimc-common.h
> +++ b/drivers/media/platform/vimc/vimc-common.h
> @@ -69,7 +69,7 @@ do {									\
>   * V4L2_PIX_FMT_* fourcc pixelformat and its bytes per pixel (bpp)
>   */
>  struct vimc_pix_map {
> -	unsigned int code;
> +	unsigned int code[3];

why 3?

>  	unsigned int bpp;
>  	u32 pixelformat;
>  	bool bayer;
> @@ -171,6 +171,13 @@ void vimc_sen_rm(struct vimc_device *vimc, struct vimc_ent_device *ved);
>   */
>  const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i);
>  
> +/**
> + * vimc_mbus_code_by_index - get mbus code by its index
> + *
> + * @i:			index of the mbus code in vimc_pix_map_list
> + */

Could you add a comment saying it returns 0 if no mbus code is found?

Regards,
Helen

> +const u32 vimc_mbus_code_by_index(unsigned int i);
> +
>  /**
>   * vimc_pix_map_by_code - get vimc_pix_map struct by media bus code
>   *
> diff --git a/drivers/media/platform/vimc/vimc-scaler.c b/drivers/media/platform/vimc/vimc-scaler.c
> index e2e551bc3ded..d2ba0a2cfcc4 100644
> --- a/drivers/media/platform/vimc/vimc-scaler.c
> +++ b/drivers/media/platform/vimc/vimc-scaler.c
> @@ -110,13 +110,19 @@ static int vimc_sca_enum_mbus_code(struct v4l2_subdev *sd,
>  				   struct v4l2_subdev_pad_config *cfg,
>  				   struct v4l2_subdev_mbus_code_enum *code)
>  {
> -	const struct vimc_pix_map *vpix = vimc_pix_map_by_index(code->index);
> +	const u32 mbus_code = vimc_mbus_code_by_index(code->index);
> +	const struct vimc_pix_map *vpix;
> +
> +	if (!mbus_code)
> +		return -EINVAL;
> +
> +	vpix = vimc_pix_map_by_code(mbus_code);
>  
>  	/* We don't support bayer format */
>  	if (!vpix || vpix->bayer)
>  		return -EINVAL;
>  
> -	code->code = vpix->code;
> +	code->code = mbus_code;
>  
>  	return 0;
>  }
> diff --git a/drivers/media/platform/vimc/vimc-sensor.c b/drivers/media/platform/vimc/vimc-sensor.c
> index 32380f504591..9f4dd7fee9ab 100644
> --- a/drivers/media/platform/vimc/vimc-sensor.c
> +++ b/drivers/media/platform/vimc/vimc-sensor.c
> @@ -52,12 +52,12 @@ static int vimc_sen_enum_mbus_code(struct v4l2_subdev *sd,
>  				   struct v4l2_subdev_pad_config *cfg,
>  				   struct v4l2_subdev_mbus_code_enum *code)
>  {
> -	const struct vimc_pix_map *vpix = vimc_pix_map_by_index(code->index);
> +	const u32 mbus_code = vimc_mbus_code_by_index(code->index);
>  
> -	if (!vpix)
> +	if (!mbus_code)
>  		return -EINVAL;
>  
> -	code->code = vpix->code;
> +	code->code = mbus_code;
>  
>  	return 0;
>  }
> 

Re: [PATCH 2/3] media: vimc: Add GBR media bus code

[Helen Koike]

Hi Nícolas,

Thank you for the patch.

On 2/2/20 1:50 PM, Nícolas F. R. A. Prado wrote:
> Add missing GBR888_1X24 media bus code in the vimc_pix_map_list. Since
> there is no pixel format for it, use the pixelformat for RGB.
> 
> Co-developed-by: Vitor Massaru Iha <vitor@massaru.org>
> Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>
> ---
>  drivers/media/platform/vimc/vimc-common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
> index 55797e5ab2b1..591a50f63766 100644
> --- a/drivers/media/platform/vimc/vimc-common.c
> +++ b/drivers/media/platform/vimc/vimc-common.c
> @@ -25,7 +25,7 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
>  		.bayer = false,
>  	},
>  	{
> -		.code = {MEDIA_BUS_FMT_RGB888_1X24},
> +		.code = {MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_GBR888_1X24},

Could you add spaces around the curly brackets here too?

I was also thinking that all the MEDIA_BUS_FMT_RGB888_* and MEDIA_BUS_FMT_GBR888_* variants
could be added here (to be verified).

Thanks
Helen

>  		.pixelformat = V4L2_PIX_FMT_RGB24,
>  		.bpp = 3,
>  		.bayer = false,
> 

Re: [PATCH 3/3] media: vimc: deb: Add support for GBR and BGR bus formats on source pad

[Helen Koike]

Hi Nicolas,

Thanks for the patch.

On 2/2/20 1:50 PM, Nícolas F. R. A. Prado wrote:
> Add support for GBR and BGR media bus formats for the source pad of
> debayer subdevices.
> 
> Co-developed-by: Vitor Massaru Iha <vitor@massaru.org>
> Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>
> ---
>  drivers/media/platform/vimc/vimc-debayer.c | 53 +++++++++++++++++-----
>  1 file changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/vimc/vimc-debayer.c b/drivers/media/platform/vimc/vimc-debayer.c
> index 5d1b67d684bb..463cafbe107e 100644
> --- a/drivers/media/platform/vimc/vimc-debayer.c
> +++ b/drivers/media/platform/vimc/vimc-debayer.c
> @@ -51,6 +51,11 @@ static const struct v4l2_mbus_framefmt sink_fmt_default = {
>  	.colorspace = V4L2_COLORSPACE_DEFAULT,
>  };
>  
> +static const u32 src_rgb_codes[] = {

s/src_rgb_codes/vimc_deb_src_mbus_codes

> +	MEDIA_BUS_FMT_BGR888_1X24,
> +	MEDIA_BUS_FMT_RGB888_1X24,
> +	MEDIA_BUS_FMT_GBR888_1X24};

The closing bracket should be in the next line.

I was also wondering if all the MEDIA_BUS_FMT_BGR888_* MEDIA_BUS_FMT_RGB888_*
MEDIA_BUS_FMT_GBR888_* variants could be added here as well.

> +
>  static const struct vimc_deb_pix_map vimc_deb_pix_map_list[] = {
>  	{
>  		.code = MEDIA_BUS_FMT_SBGGR8_1X8,
> @@ -148,14 +153,11 @@ static int vimc_deb_enum_mbus_code(struct v4l2_subdev *sd,
>  				   struct v4l2_subdev_pad_config *cfg,
>  				   struct v4l2_subdev_mbus_code_enum *code)
>  {
> -	/* We only support one format for source pads */
>  	if (VIMC_IS_SRC(code->pad)) {
> -		struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
> -
> -		if (code->index)
> +		if (code->index >= ARRAY_SIZE(src_rgb_codes))
>  			return -EINVAL;
>  
> -		code->code = vdeb->src_code;
> +		code->code = src_rgb_codes[code->index];
>  	} else {
>  		if (code->index >= ARRAY_SIZE(vimc_deb_pix_map_list))
>  			return -EINVAL;
> @@ -170,7 +172,7 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd,
>  				    struct v4l2_subdev_pad_config *cfg,
>  				    struct v4l2_subdev_frame_size_enum *fse)
>  {
> -	struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
> +	int i;

unsigned

>  
>  	if (fse->index)
>  		return -EINVAL;
> @@ -181,8 +183,13 @@ static int vimc_deb_enum_frame_size(struct v4l2_subdev *sd,
>  
>  		if (!vpix)
>  			return -EINVAL;
> -	} else if (fse->code != vdeb->src_code) {
> -		return -EINVAL;
> +	} else {

You can declare i inside this else statement.

> +		for (i = 0; i < ARRAY_SIZE(src_rgb_codes); i++) {
> +			if (src_rgb_codes[i] == fse->code)
> +				break;
> +		}
> +		if (i == ARRAY_SIZE(src_rgb_codes))
> +			return -EINVAL;
>  	}
>  
>  	fse->min_width = VIMC_FRAME_MIN_WIDTH;
> @@ -237,6 +244,8 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd,
>  {
>  	struct vimc_deb_device *vdeb = v4l2_get_subdevdata(sd);
>  	struct v4l2_mbus_framefmt *sink_fmt;
> +	unsigned int i;
> +	u32 *src_code;
>  
>  	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
>  		/* Do not change the format while stream is on */
> @@ -244,8 +253,10 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd,
>  			return -EBUSY;
>  
>  		sink_fmt = &vdeb->sink_fmt;
> +		src_code = &vdeb->src_code;
>  	} else {
>  		sink_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
> +		src_code = &v4l2_subdev_get_try_format(sd, cfg, 1)->code;
>  	}
>  
>  	/*
> @@ -253,9 +264,17 @@ static int vimc_deb_set_fmt(struct v4l2_subdev *sd,
>  	 * it is propagated from the sink
>  	 */
>  	if (VIMC_IS_SRC(fmt->pad)) {
> +		u32 code = fmt->format.code;
> +
>  		fmt->format = *sink_fmt;
> -		/* TODO: Add support for other formats */
> -		fmt->format.code = vdeb->src_code;
> +
> +		for (i = 0; i < ARRAY_SIZE(src_rgb_codes); i++) {
> +			if (src_rgb_codes[i] == code) {
> +				*src_code = src_rgb_codes[i];
> +				break;
> +			}
> +		}

Maybe you can add a function for this, since you also repeat this loop above.

Regards,
Helen

> +		fmt->format.code = *src_code;
>  	} else {
>  		/* Set the new format in the sink pad */
>  		vimc_deb_adjust_sink_fmt(&fmt->format);
> @@ -291,11 +310,21 @@ static void vimc_deb_set_rgb_mbus_fmt_rgb888_1x24(struct vimc_deb_device *vdeb,
>  						  unsigned int col,
>  						  unsigned int rgb[3])
>  {
> +	const struct vimc_pix_map *vpix;
>  	unsigned int i, index;
>  
> +	vpix = vimc_pix_map_by_code(vdeb->src_code);
>  	index = VIMC_FRAME_INDEX(lin, col, vdeb->sink_fmt.width, 3);
> -	for (i = 0; i < 3; i++)
> -		vdeb->src_frame[index + i] = rgb[i];
> +	for (i = 0; i < 3; i++) {
> +		switch (vpix->pixelformat) {
> +		case V4L2_PIX_FMT_RGB24:
> +			vdeb->src_frame[index + i] = rgb[i];
> +			break;
> +		case V4L2_PIX_FMT_BGR24:
> +			vdeb->src_frame[index + i] = rgb[2-i];
> +			break;
> +		}
> +	}
>  }
>  
>  static int vimc_deb_s_stream(struct v4l2_subdev *sd, int enable)
> 

Re: [PATCH 1/3] media: vimc: Support multiple buscodes for each pixelformat

[Nícolas F. R. A. Prado]

Hi Helen,
thanks for the review.

Just one comment below.

On Wed, Feb 05, 2020 at 11:17:31AM -0300, Helen Koike wrote:
>
> Hi Nícolas,
>
> Thanks for the patch, just some minor nits:
>
> On 2/2/20 1:50 PM, Nícolas F. R. A. Prado wrote:
> > diff --git a/drivers/media/platform/vimc/vimc-common.h b/drivers/media/platform/vimc/vimc-common.h
> > index 87eb8259c2a8..132a5889f1ea 100644
> > --- a/drivers/media/platform/vimc/vimc-common.h
> > +++ b/drivers/media/platform/vimc/vimc-common.h
> > @@ -69,7 +69,7 @@ do {                                                                      \
> >   * V4L2_PIX_FMT_* fourcc pixelformat and its bytes per pixel (bpp)
> >   */
> >  struct vimc_pix_map {
> > -   unsigned int code;
> > +   unsigned int code[3];
>
> why 3?

On my patch I'm using at most 2.
I chose 3 so that it would be easier to add more formats, but I guess it doesn't
really make sense, since it's wasting memory and that value can be increased
when it's necessary.
So I should change it to 2, right?

> Regards,
> Helen

Thanks,
Nícolas

Re: [PATCH 2/3] media: vimc: Add GBR media bus code

[Nícolas F. R. A. Prado]

Hi Helen,

thank you for the review.

Please see my comment below.

On Wed, Feb 05, 2020 at 11:17:35AM -0300, Helen Koike wrote:
> 
> Hi Nícolas,
> 
> Thank you for the patch.
> 
> On 2/2/20 1:50 PM, Nícolas F. R. A. Prado wrote:
> > Add missing GBR888_1X24 media bus code in the vimc_pix_map_list. Since
> > there is no pixel format for it, use the pixelformat for RGB.
> >
> > Co-developed-by: Vitor Massaru Iha <vitor@massaru.org>
> > Signed-off-by: Vitor Massaru Iha <vitor@massaru.org>
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@protonmail.com>
> > ---
> >  drivers/media/platform/vimc/vimc-common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/vimc/vimc-common.c b/drivers/media/platform/vimc/vimc-common.c
> > index 55797e5ab2b1..591a50f63766 100644
> > --- a/drivers/media/platform/vimc/vimc-common.c
> > +++ b/drivers/media/platform/vimc/vimc-common.c
> > @@ -25,7 +25,7 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {
> >  		.bayer = false,
> >  	},
> >  	{
> > -		.code = {MEDIA_BUS_FMT_RGB888_1X24},
> > +		.code = {MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_GBR888_1X24},
> 
> Could you add spaces around the curly brackets here too?
> 
> I was also thinking that all the MEDIA_BUS_FMT_RGB888_* and MEDIA_BUS_FMT_GBR888_* variants
> could be added here (to be verified).

Now that you said it, it does seem that this could be done.

From the pixelformat definitions on [1], there is a single format definition for
RGB-8-8-8 and for BGR-8-8-8:

#define V4L2_PIX_FMT_BGR24   v4l2_fourcc('B', 'G', 'R', '3') /* 24  BGR-8-8-8     */
#define V4L2_PIX_FMT_RGB24   v4l2_fourcc('R', 'G', 'B', '3') /* 24  RGB-8-8-8     */

This probably means that all media bus code variants of RGB888 should map to the
same RGB888 pixelformat. Same for BGR888.

So, should I go on and send a v2 adding these other formats or should we wait
for others to comment on this first?

Thanks,
Nícolas

[1] https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/videodev2.h#L549

> 
> Thanks
> Helen
> 
> >  		.pixelformat = V4L2_PIX_FMT_RGB24,
> >  		.bpp = 3,
> >  		.bayer = false,
> >

Re: [PATCH 0/3] media: vimc: Add support for GBR and BGR formats on source pad of debayer

[Ezequiel Garcia]

On Sun, 2 Feb 2020 at 12:50, Nícolas F. R. A. Prado
<nfraprado@protonmail.com> wrote:
>
> The objective of this series is to add support for GBR and BGR media bus formats
> for the source pad of debayer subdevices of the vimc driver.
>

Can you elaborate why is this needed, e.g. what use-case is this enabling,
or what is this fixing?

Thanks,
Ezequiel

> Since the GBR media bus code doesn't have a corresponding pixelformat, it needed
> to use the pixelformat of another bus code.
>
> The first patch makes it possible to have multiple media bus codes mapping to
> the same pixelformat.
>
> The second patch adds the GBR media bus code, using the RGB pixelformat.
>
> The third patch adds support for GBR and BGR media bus formats on the source
> pad of the debayer subdevice.
>
> This patch series passed all tests of v4l2-compliance:
> $ compliance_git -m /dev/media0
> v4l2-compliance SHA: c4a62f26c5c3ecd856ca10cf2f0d35d100283d7f, 64 bits, 64-bit time_t
>
> Grand Total for vimc device /dev/media0: 461, Succeeded: 461, Failed: 0, Warnings: 0
>
> Nícolas F. R. A. Prado (3):
>   media: vimc: Support multiple buscodes for each pixelformat
>   media: vimc: Add GBR media bus code
>   media: vimc: deb: Add support for GBR and BGR bus formats on source
>     pad
>
>  drivers/media/platform/vimc/vimc-common.c  | 68 +++++++++++++---------
>  drivers/media/platform/vimc/vimc-common.h  |  9 ++-
>  drivers/media/platform/vimc/vimc-debayer.c | 53 +++++++++++++----
>  drivers/media/platform/vimc/vimc-scaler.c  | 10 +++-
>  drivers/media/platform/vimc/vimc-sensor.c  |  6 +-
>  5 files changed, 102 insertions(+), 44 deletions(-)
>
> --
> 2.25.0
>
>

Re: [PATCH 0/3] media: vimc: Add support for GBR and BGR formats on source pad of debayer

[Nícolas F. R. A. Prado]

Hi,
Ezequiel.

On Sun, Feb 09, 2020 at 02:09:17PM -0300, Ezequiel Garcia wrote:
> 
> On Sun, 2 Feb 2020 at 12:50, Nícolas F. R. A. Prado
> <nfraprado@protonmail.com> wrote:
> >
> > The objective of this series is to add support for GBR and BGR media bus formats
> > for the source pad of debayer subdevices of the vimc driver.
> >
> 
> Can you elaborate why is this needed, e.g. what use-case is this enabling,
> or what is this fixing?

Sure.

At the moment, the only supported media bus format on the source pad of
the debayer subdevice is the RGB888_1X24.

The mbus format of the source pad of the debayer ultimately determines the
pixelformat that is streamed on /dev/video3, since:
    * The mbus formats of the links on the topology have to match for the
        streaming to be possible, and [1] shows that the source pad of the
        debayer links to the sink of the scaler.
    * The scaler uses the same mbus format on sink and source.
    * The source pad of the scaler is linked to the /dev/video3 Capture.

That said, there isn't a GBR pixelformat, so it uses the RGB pixelformat.

So what this patch series enables:
    * Setting debayer and scaler subdevices to use GBR and BGR mbus formats.
    * Stream video using the BGR pixelformat from /dev/video3.

By enabling these, it makes it possible to use vimc to emulate hardware that
uses GBR or BGR mbus formats internally or that streams using the BGR
pixelformat.

Regards,
Nícolas

[1] https://linuxtv.org/downloads/v4l-dvb-apis-new/v4l-drivers/vimc.html#topology

> 
> Thanks,
> Ezequiel