2020-05-01 13:13:10

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v4 0/3] media: vimc: Add support for {RGB,BGR,GBR}888 bus formats on debayer source pad

This patch series adds support for the missing RGB888_*, BGR888_* and GBR888_*
media bus formats on the source pad of debayer subdevices of the vimc driver.
These additional bus formats enable a wider range of formats to be configured
on the topologies, making it possible to test more real use cases.
It also enables video to be streamed in the BGR pixelformat on /dev/video3.

The first patch makes it possible to have multiple media bus codes mapping to
the same pixelformat, so that, for example, all RGB888_* media bus formats use
the same RGB24 pixelformat.

The second patch maps the missing RGB888_*, BGR888_* and GBR888_* media bus
codes to the RGB24 and BGR24 pixelformats.
Since there isn't a GBR24 pixelformat, the GBR888_1X24 bus code maps to the
RGB24 pixelformat.

The third patch enables the source pad of the debayer subdevice to use the
added media bus formats.

This patch series passed all tests of v4l2-compliance:
$ compliance_git -m /dev/media0
v4l2-compliance SHA: 81e45d957c4db39397f893100b3d2729ef39b052, 64 bits, 64-bit time_t
Grand Total for vimc device /dev/media0: 461, Succeeded: 461, Failed: 0, Warnings: 0

As a side note, when listing the pads containing the new formats added, I
noticed that MEDIA_BUS_FMT_RGB888_3X8 doesn't have its name displayed by
v4l2-ctl, but from my understanding that should be a bug in v4l-utils.

$ v4l2-ctl -d /dev/v4l-subdev2 --list-subdev-mbus-codes 1
ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=1)
0x1014: MEDIA_BUS_FMT_GBR888_1X24
0x1013: MEDIA_BUS_FMT_BGR888_1X24
0x101b: MEDIA_BUS_FMT_BGR888_3X8
0x100a: MEDIA_BUS_FMT_RGB888_1X24
0x100b: MEDIA_BUS_FMT_RGB888_2X12_BE
0x100c: MEDIA_BUS_FMT_RGB888_2X12_LE
0x101c
0x1011: MEDIA_BUS_FMT_RGB888_1X7X4_SPWG
0x1012: MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA
0x100f: MEDIA_BUS_FMT_RGB888_1X32_PADHI

Changes in v4:
- Suggested by Dafna:
- Rename vimc_deb_set_rgb_mbus_fmt_rgb888_1x24() to
vimc_deb_process_rgb_frame()

Changes in v3:
- Make loop in vimc_mbus_code_by_index() clearer by using break instead of if
- Suggested by Helen:
- Rename vimc_deb_is_src_code_invalid() to vimc_deb_src_code_is_valid()
- Change vimc_deb_src_code_is_valid() to return bool
- Suggested by Shuah:
- Use VIMC_PIX_FMT_MAX_CODES define instead of hardcoded value for the
size of code array in struct vimc_pix_map

Changes in v2:
- Fix vimc_mbus_code_by_index not checking code array bounds
- Change commit messages to reflect v2 changes
- Suggested by Helen:
- Rename variables
- Fix array formatting
- Change code array size
- Add comment about vimc_mbus_code_by_index return value
- Add vimc_deb_is_src_code_valid function
- Add other BGR888 and RGB888 formats to BGR24 and RGB24 pixelformats
- Add other BGR888 and RGB888 formats to debayer source pad supported
formats
- Suggested by Ezequiel:
- Change cover letter to better explain this patch series

You can find v1 here: https://patchwork.linuxtv.org/cover/61391/

Nícolas F. R. A. Prado (3):
media: vimc: Support multiple media bus codes for each pixelformat
media: vimc: Add missing {RGB,BGR,GBR}888 media bus codes
media: vimc: deb: Add support for {RGB,BGR,GBR}888 bus formats on
source pad

drivers/media/test-drivers/vimc/vimc-common.c | 83 +++++++++++++------
drivers/media/test-drivers/vimc/vimc-common.h | 13 ++-
.../media/test-drivers/vimc/vimc-debayer.c | 71 ++++++++++++----
drivers/media/test-drivers/vimc/vimc-scaler.c | 10 ++-
drivers/media/test-drivers/vimc/vimc-sensor.c | 6 +-
5 files changed, 134 insertions(+), 49 deletions(-)

--
2.26.2



2020-05-01 13:14:07

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v4 2/3] media: vimc: Add missing {RGB,BGR,GBR}888 media bus codes

Add missing RGB888_*, BGR888_* and GBR888_* media bus codes in the
vimc_pix_map_list. Since there is no GBR24 pixelformat, use the RGB24
pixelformat for MEDIA_BUS_FMT_GBR888_1X24.

Acked-by: Helen Koike <[email protected]>
Co-developed-by: Vitor Massaru Iha <[email protected]>
Signed-off-by: Vitor Massaru Iha <[email protected]>
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---

Changes in v4:
None

Changes in v3:
- Use VIMC_PIX_FMT_MAX_CODES define instead of hardcoded value for the size of
code array in struct vimc_pix_map

Changes in v2:
- Fix array formatting
- Change commit message to reflect v2 changes
- Change code array size
- Add other BGR888 and RGB888 formats to BGR24 and RGB24 pixelformats

drivers/media/test-drivers/vimc/vimc-common.c | 16 ++++++++++++++--
drivers/media/test-drivers/vimc/vimc-common.h | 2 +-
2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/test-drivers/vimc/vimc-common.c b/drivers/media/test-drivers/vimc/vimc-common.c
index e11107e4796c..45b5312d6271 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.c
+++ b/drivers/media/test-drivers/vimc/vimc-common.c
@@ -19,13 +19,25 @@ static const struct vimc_pix_map vimc_pix_map_list[] = {

/* RGB formats */
{
- .code = { MEDIA_BUS_FMT_BGR888_1X24 },
+ .code = {
+ MEDIA_BUS_FMT_BGR888_1X24,
+ MEDIA_BUS_FMT_BGR888_3X8
+ },
.pixelformat = V4L2_PIX_FMT_BGR24,
.bpp = 3,
.bayer = false,
},
{
- .code = { MEDIA_BUS_FMT_RGB888_1X24 },
+ .code = {
+ MEDIA_BUS_FMT_RGB888_1X24,
+ MEDIA_BUS_FMT_RGB888_2X12_BE,
+ MEDIA_BUS_FMT_RGB888_2X12_LE,
+ MEDIA_BUS_FMT_RGB888_3X8,
+ MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
+ MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA,
+ MEDIA_BUS_FMT_RGB888_1X32_PADHI,
+ MEDIA_BUS_FMT_GBR888_1X24
+ },
.pixelformat = V4L2_PIX_FMT_RGB24,
.bpp = 3,
.bayer = false,
diff --git a/drivers/media/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
index dfebf6f75cfc..aa67cfebeb26 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.h
+++ b/drivers/media/test-drivers/vimc/vimc-common.h
@@ -32,7 +32,7 @@
#define VIMC_IS_SRC(pad) (pad)
#define VIMC_IS_SINK(pad) (!(pad))

-#define VIMC_PIX_FMT_MAX_CODES 1
+#define VIMC_PIX_FMT_MAX_CODES 8

/**
* vimc_colorimetry_clamp - Adjust colorimetry parameters
--
2.26.2


2020-05-01 13:15:24

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [PATCH v4 1/3] media: vimc: Support multiple media bus codes for each pixelformat

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.

Acked-by: Helen Koike <[email protected]>
Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---

Changes in v4:
None

Changes in v3:
- Make loop in vimc_mbus_code_by_index() clearer by using break instead of if
- Use VIMC_PIX_FMT_MAX_CODES define instead of hardcoded value for the size of
code array in struct vimc_pix_map

Changes in v2:
- Fix vimc_mbus_code_by_index not checking code array bounds
- Fix array formatting
- Rename variables
- Change code array size
- Add comment about vimc_mbus_code_by_index return value

drivers/media/test-drivers/vimc/vimc-common.c | 71 ++++++++++++-------
drivers/media/test-drivers/vimc/vimc-common.h | 13 +++-
drivers/media/test-drivers/vimc/vimc-scaler.c | 10 ++-
drivers/media/test-drivers/vimc/vimc-sensor.c | 6 +-
4 files changed, 68 insertions(+), 32 deletions(-)

diff --git a/drivers/media/test-drivers/vimc/vimc-common.c b/drivers/media/test-drivers/vimc/vimc-common.c
index c95c17c048f2..e11107e4796c 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.c
+++ b/drivers/media/test-drivers/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,32 @@ 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 index)
+{
+ unsigned int i, j;
+
+ for (i = 0; i < ARRAY_SIZE(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])
+ break;
+
+ if (!index)
+ return vimc_pix_map_list[i].code[j];
+ index--;
+ }
+ }
+ 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/test-drivers/vimc/vimc-common.h b/drivers/media/test-drivers/vimc/vimc-common.h
index 487bd020f85c..dfebf6f75cfc 100644
--- a/drivers/media/test-drivers/vimc/vimc-common.h
+++ b/drivers/media/test-drivers/vimc/vimc-common.h
@@ -32,6 +32,8 @@
#define VIMC_IS_SRC(pad) (pad)
#define VIMC_IS_SINK(pad) (!(pad))

+#define VIMC_PIX_FMT_MAX_CODES 1
+
/**
* vimc_colorimetry_clamp - Adjust colorimetry parameters
*
@@ -70,7 +72,7 @@ do { \
* V4L2_PIX_FMT_* fourcc pixelformat and its bytes per pixel (bpp)
*/
struct vimc_pix_map {
- unsigned int code;
+ unsigned int code[VIMC_PIX_FMT_MAX_CODES];
unsigned int bpp;
u32 pixelformat;
bool bayer;
@@ -169,6 +171,15 @@ extern struct vimc_ent_type vimc_cap_type;
*/
const struct vimc_pix_map *vimc_pix_map_by_index(unsigned int i);

+/**
+ * vimc_mbus_code_by_index - get mbus code by its index
+ *
+ * @index: index of the mbus code in vimc_pix_map_list
+ *
+ * Returns 0 if no mbus code is found for the given index.
+ */
+const u32 vimc_mbus_code_by_index(unsigned int index);
+
/**
* vimc_pix_map_by_code - get vimc_pix_map struct by media bus code
*
diff --git a/drivers/media/test-drivers/vimc/vimc-scaler.c b/drivers/media/test-drivers/vimc/vimc-scaler.c
index 465b906b7497..02615ded1c93 100644
--- a/drivers/media/test-drivers/vimc/vimc-scaler.c
+++ b/drivers/media/test-drivers/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/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
index 228120b3a6dd..9391f5b5c58a 100644
--- a/drivers/media/test-drivers/vimc/vimc-sensor.c
+++ b/drivers/media/test-drivers/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.26.2