[PATCH v5 2/2] media: bcm2835-unicam: Fix RGB format / mbus code association

[Maxime Ripard <mripard@kernel.org>]

From: Maxime Ripard <mripard@redhat.com>

The Unicam driver is a MIPI-CSI2 Receiver, that can capture RGB 4:4:4,
YCbCr 4:2:2, and raw formats.

RGB 4:4:4 is converted to the MIPI-CSI2 RGB888 video format, and
associated to the MEDIA_BUS_FMT_RGB888_1X24 media bus code.

However, V4L2_PIX_FMT_RGB24 is defined as having its color components in
the R, G and B order, from left to right. MIPI-CSI2 however defines the
RGB888 format with blue first, and that's what MEDIA_BUS_FMT_RGB888_1X24
defines too.

This essentially means that the R and B will be swapped compared to what
V4L2_PIX_FMT_RGB24 defines. The same situation occurs with
V4L2_PIX_FMT_BGR24 being associated to MEDIA_BUS_FMT_BGR888_1X24.

In order to fix the swapped components, we need to change the
association of V4L2_PIX_FMT_BGR24 to MEDIA_BUS_FMT_RGB888_1X24, and of
V4L2_PIX_FMT_RGB24 to MEDIA_BUS_FMT_BGR888_1X24.

Since the media bus code is exposed to userspace, and validated by
unicam's link_validate implementation, we need to explicitly accept (and
warn) the old association still to preserve backward compatibility.

Signed-off-by: Maxime Ripard <mripard@redhat.com>
---
 drivers/media/platform/broadcom/bcm2835-unicam.c | 36 +++++++++++++++++++++---
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index f10064107d543caf867249d0566a0f42d6d8c4c6..5e4850831c931d346146aa8e22c53f0655e462c9 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -340,16 +340,16 @@ static const struct unicam_format_info unicam_image_formats[] = {
 		.code		= MEDIA_BUS_FMT_RGB565_1X16,
 		.depth		= 16,
 		.csi_dt		= MIPI_CSI2_DT_RGB565,
 	}, {
 		.fourcc		= V4L2_PIX_FMT_RGB24, /* rgb */
-		.code		= MEDIA_BUS_FMT_RGB888_1X24,
+		.code		= MEDIA_BUS_FMT_BGR888_1X24,
 		.depth		= 24,
 		.csi_dt		= MIPI_CSI2_DT_RGB888,
 	}, {
 		.fourcc		= V4L2_PIX_FMT_BGR24, /* bgr */
-		.code		= MEDIA_BUS_FMT_BGR888_1X24,
+		.code		= MEDIA_BUS_FMT_RGB888_1X24,
 		.depth		= 24,
 		.csi_dt		= MIPI_CSI2_DT_RGB888,
 	}, {
 	/* Bayer Formats */
 		.fourcc		= V4L2_PIX_FMT_SBGGR8,
@@ -2153,12 +2153,40 @@ static int unicam_video_link_validate(struct media_link *link)
 		if (WARN_ON(!fmtinfo)) {
 			ret = -EPIPE;
 			goto out;
 		}
 
-		if (fmtinfo->code != format->code ||
-		    fmt->height != format->height ||
+		/*
+		 * Unicam initially associated BGR24 to BGR888_1X24 and
+		 * RGB24 to RGB888_1X24.
+		 *
+		 * In order to allow the applications using the old
+		 * behaviour to run, let's accept the old combination,
+		 * but warn about it.
+		 */
+		if (fmtinfo->code != format->code) {
+			if (fmtinfo->fourcc == V4L2_PIX_FMT_BGR24 &&
+			    format->code == MEDIA_BUS_FMT_BGR888_1X24) {
+				dev_warn_once(node->dev->dev,
+					      "MIPI-CSI media bus code for RGB88 is RGB888_1X24. The application must be fixed.");
+			} else if (fmtinfo->fourcc == V4L2_PIX_FMT_RGB24 &&
+				   format->code == MEDIA_BUS_FMT_RGB888_1X24) {
+				dev_warn_once(node->dev->dev,
+					      "MIPI-CSI media bus code for BGR888 is BGR888_1X24. The application must be fixed.");
+			} else {
+				dev_dbg(node->dev->dev,
+					"image: (%u x %u) 0x%08x %s != (%u x %u) 0x%08x %s\n",
+					fmt->width, fmt->height, fmtinfo->code,
+					v4l2_field_names[fmt->field],
+					format->width, format->height, format->code,
+					v4l2_field_names[format->field]);
+				ret = -EPIPE;
+				goto out;
+			}
+		}
+
+		if (fmt->height != format->height ||
 		    fmt->width != format->width ||
 		    fmt->field != format->field) {
 			dev_dbg(node->dev->dev,
 				"image: (%u x %u) 0x%08x %s != (%u x %u) 0x%08x %s\n",
 				fmt->width, fmt->height, fmtinfo->code,

-- 
2.52.0