tegra-vi4 output format

danielellxz · February 25, 2019, 3:32pm

Hi,

We are using NVIDIA jetsons TX2 eval board with the ov5693 camera and we have
noticed the BG10 output configured by the vi4 is not actually the real output.

So, that’s our analysis:

ov5693 pixel format output is BG10 (V4L2_PIX_FMT_SBGGR10).
vi4 input is BG10 (camera input) but output (conversion) is BYR2.

file:

drivers/media/platform/tegra/camera/vi/vi4_formats.h

snippet:

static const struct tegra_video_format vi4_video_formats[] = {
...
        TEGRA_VIDEO_FORMAT(RAW10, 10, SBGGR10_1X10, 2, 1, T_R16_I,
                                RAW10, SBGGR10, "BGBG.. GRGR.."),
...

qv4l2 & BG10:
Performing the conversion in qv4l2 for the expected BG10 input (with OpenGL
disabled) produces wrong image output.

qv4l2 & BYR2:
But if we do the conversion by shifting 8 bits to the right and assuming the vi4
ouptut is BYR2 (16 bits-word, little endian) then, we are able to capture
images properly.

L4T version:
Linux kernel version: 4.4.38 provided by latest L4T (28.2.1).

Questions:

Are you sure the vi4 output is BG10?
Why if we use qv4l2 - NVIDIA OpenGL converts properly the image? Might be possible
that we have also a bug here?

v4l formats and links:
BG10: V4L2_PIX_FMT_SBGGR10
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-srggb10.html

BYR2: V4L2_PIX_FMT_SBGGR16
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-srggb16.html

Regards

ShaneCCC · February 26, 2019, 4:25am

Please have a check TRM for the memory layout for vi output for the detail.

External Media

ricardo.ribalda · February 26, 2019, 7:40am

Hi @ShaneCCC

According to your diagram, the data is MSB aligned, therefore it is not V4L2_PIX_FMT_SBGGR10.

Userspace applications will not be able to decode properly the images, and is probably chaining bugs in other libraries from nvidia like libgl, which in fact is rendering V4L2_PIX_FMT_SBGGR10 wrongly.

I believe that the driver should say that image is formated as V4L2_PIX_FMT_SBGGR16, as @danielellxz is proposing.

IMHO: This is kind of a serious bug

danielellxz · February 26, 2019, 1:32pm

Hi ShaneCCC,

Thanks for your quick reply.

That confirms our guess so, the pixel format is not SBGGR10. But we fixed the tegra vi4 driver to return the proper format when T_R16_I function is used and it worked. Here the patch:

diff --git a/drivers/media/platform/tegra/camera/camera_common.c b/drivers/media/platform/tegra/camera/camera_common.c
index 625fa069e1ec..876a7829958a 100644
--- a/drivers/media/platform/tegra/camera/camera_common.c
+++ b/drivers/media/platform/tegra/camera/camera_common.c
@@ -44,12 +44,12 @@ static const struct camera_common_colorfmt camera_common_color_fmts[] = {
 	{
 		MEDIA_BUS_FMT_SRGGB10_1X10,
 		V4L2_COLORSPACE_SRGB,
-		V4L2_PIX_FMT_SRGGB10,
+		V4L2_PIX_FMT_SRGGB16,
 	},
 	{
 		MEDIA_BUS_FMT_SBGGR10_1X10,
 		V4L2_COLORSPACE_SRGB,
-		V4L2_PIX_FMT_SBGGR10,
+		V4L2_PIX_FMT_SBGGR16,
 	},
 	{
 		MEDIA_BUS_FMT_SRGGB8_1X8,
@@ -417,6 +417,9 @@ static const struct camera_common_colorfmt *find_matching_color_fmt(
 break_loops:
 	if (match_num < index)
 		return NULL;
+
+	if (match_index < 0)
+		return NULL;
 	return &camera_common_color_fmts[match_index];
 }
 
diff --git a/drivers/media/platform/tegra/camera/sensor_common.c b/drivers/media/platform/tegra/camera/sensor_common.c
index 4e8063f0c023..ed4a9374edb2 100644
--- a/drivers/media/platform/tegra/camera/sensor_common.c
+++ b/drivers/media/platform/tegra/camera/sensor_common.c
@@ -120,17 +120,17 @@ static int extract_pixel_format(
 	size_t size = strnlen(pixel_t, OF_MAX_STR_LEN);
 
 	if (strncmp(pixel_t, "bayer_bggr10", size) == 0)
-		*format = V4L2_PIX_FMT_SBGGR10;
+		*format = V4L2_PIX_FMT_SBGGR16;
 	else if (strncmp(pixel_t, "bayer_rggb10", size) == 0)
-		*format = V4L2_PIX_FMT_SRGGB10;
+		*format = V4L2_PIX_FMT_SRGGB16;
 	else if (strncmp(pixel_t, "bayer_bggr12", size) == 0)
-		*format = V4L2_PIX_FMT_SBGGR12;
+		*format = V4L2_PIX_FMT_SBGGR16;
 	else if (strncmp(pixel_t, "bayer_rggb12", size) == 0)
-		*format = V4L2_PIX_FMT_SRGGB12;
+		*format = V4L2_PIX_FMT_SRGGB16;
 	else if (strncmp(pixel_t, "bayer_wdr_pwl_rggb12", size) == 0)
-		*format = V4L2_PIX_FMT_SRGGB12;
+		*format = V4L2_PIX_FMT_SRGGB16;
 	else if (strncmp(pixel_t, "bayer_wdr_dol_rggb10", size) == 0)
-		*format = V4L2_PIX_FMT_SRGGB10;
+		*format = V4L2_PIX_FMT_SRGGB16;
 	else if (strncmp(pixel_t, "bayer_xbggr10p", size) == 0)
 		*format = V4L2_PIX_FMT_XBGGR10P;
 	else if (strncmp(pixel_t, "bayer_xrggb10p", size) == 0)
diff --git a/drivers/media/platform/tegra/camera/vi/vi2_formats.h b/drivers/media/platform/tegra/camera/vi/vi2_formats.h
index c272eac3e964..fa1ec2be27f8 100644
--- a/drivers/media/platform/tegra/camera/vi/vi2_formats.h
+++ b/drivers/media/platform/tegra/camera/vi/vi2_formats.h
@@ -84,13 +84,13 @@ static const struct tegra_video_format vi2_video_formats[] = {
 
 	/* RAW 10 */
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SRGGB10_1X10, 2, 1, T_R16_I,
-				RAW10, SRGGB10, "RGRG.. GBGB.."),
+				RAW10, SRGGB16, "RGRG.. GBGB.."),
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SGRBG10_1X10, 2, 1, T_R16_I,
-				RAW10, SGRBG10, "GRGR.. BGBG.."),
+				RAW10, SGRBG16, "GRGR.. BGBG.."),
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SGBRG10_1X10, 2, 1, T_R16_I,
-				RAW10, SGBRG10, "GBGB.. RGRG.."),
+				RAW10, SGBRG16, "GBGB.. RGRG.."),
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SBGGR10_1X10, 2, 1, T_R16_I,
-				RAW10, SBGGR10, "BGBG.. GRGR.."),
+				RAW10, SBGGR16, "BGBG.. GRGR.."),
 
 	/* RAW 10 Packed format */
 	TEGRA_VIDEO_FORMAT(RAW10, 10, XBGGR10P_3X10, 4, 3, T_X2Lc10Lb10La10,
diff --git a/drivers/media/platform/tegra/camera/vi/vi4_formats.h b/drivers/media/platform/tegra/camera/vi/vi4_formats.h
index bbdbbab71569..5a235d2b9077 100644
--- a/drivers/media/platform/tegra/camera/vi/vi4_formats.h
+++ b/drivers/media/platform/tegra/camera/vi/vi4_formats.h
@@ -98,23 +98,23 @@ static const struct tegra_video_format vi4_video_formats[] = {
 
 	/* RAW 10 */
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SRGGB10_1X10, 2, 1, T_R16_I,
-				RAW10, SRGGB10, "RGRG.. GBGB.."),
+				RAW10, SRGGB16, "RGRG.. GBGB.."),
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SGRBG10_1X10, 2, 1, T_R16_I,
-				RAW10, SGRBG10, "GRGR.. BGBG.."),
+				RAW10, SGRBG16, "GRGR.. BGBG.."),
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SGBRG10_1X10, 2, 1, T_R16_I,
-				RAW10, SGBRG10, "GBGB.. RGRG.."),
+				RAW10, SGBRG16, "GBGB.. RGRG.."),
 	TEGRA_VIDEO_FORMAT(RAW10, 10, SBGGR10_1X10, 2, 1, T_R16_I,
-				RAW10, SBGGR10, "BGBG.. GRGR.."),
+				RAW10, SBGGR16, "BGBG.. GRGR.."),
 
 	/* RAW 12 */
 	TEGRA_VIDEO_FORMAT(RAW12, 12, SRGGB12_1X12, 2, 1, T_R16_I,
-				RAW12, SRGGB12, "RGRG.. GBGB.."),
+				RAW12, SRGGB16, "RGRG.. GBGB.."),
 	TEGRA_VIDEO_FORMAT(RAW12, 12, SGRBG12_1X12, 2, 1, T_R16_I,
-				RAW12, SGRBG12, "GRGR.. BGBG.."),
+				RAW12, SGRBG16, "GRGR.. BGBG.."),
 	TEGRA_VIDEO_FORMAT(RAW12, 12, SGBRG12_1X12, 2, 1, T_R16_I,
-				RAW12, SGBRG12, "GBGB.. RGRG.."),
+				RAW12, SGBRG16, "GBGB.. RGRG.."),
 	TEGRA_VIDEO_FORMAT(RAW12, 12, SBGGR12_1X12, 2, 1, T_R16_I,
-				RAW12, SBGGR12, "BGBG.. GRGR.."),
+				RAW12, SBGGR16, "BGBG.. GRGR.."),
 
 	/* RGB888 */
 	TEGRA_VIDEO_FORMAT(RGB888, 24, RGB888_1X24, 4, 1, T_A8R8G8B8,

Now qv4l2 is able to decode the image: OV5693: V4L2_PIX_FMT_SBGGR10 → tegra-vi4: V4L2_PIX_FMT_SBGGR16 → qv4l2 bayer raw 16 conversion.

But now the NVIDIA OpenGL conversion doesn’t work as before so it confirms our guess that the bug was spread over the stack from the vi4 output to the NVIDIA OpenGL library.

ShaneCCC · March 4, 2019, 9:37am

Have you consider to use argus instead of used vi mode to capture.
You can reference to the sample code …/tegra_multimedia_api/argus/samples/openglBox

danielellxz · March 7, 2019, 3:20pm

Hi ShaneCCC,

At this time, we plan to use v4l since it’s the same for every platform we have.