[PATCH v5 0/2] media: Fix CSI2 RGB vs BGR pixel order

[PATCH v5 0/2] media: Fix CSI2 RGB vs BGR pixel order

[Maxime Ripard]

Hi,

Here's an(other [1]) attempt at fixing the current mess due to the
opposite meaning of what v4l2 and the MIPI-CSI2 spec call "RGB". By v4l2
nomenclature, the format CSI calls RGB is actually BGR.

Unfortunately, a handful of CSI transceivers report through RGB media
bus pixel code, which is then understood as V4L2_PIX_FMT_RGB24 by CSI
receivers.

This is made somewhat worse the fact that media bus codes have been made
mostly with parallel busses in mind, and thus the order of pixels wasn't
clearly defined anywhere.

So the v4l2 vs CSI mismatch was confusing (but there's nothing we can do
about it), but the doc didn't really make an attempt at clearing it up
either.

We did have a convention so far though, that about half the affected
drivers were following. 

This series improves the doc, adds the missing media bus codes, and
converts the transceiver drivers to the rightful media bus format.

We'll also need that series [2] from Laurent to fix all the affected
transceivers. 

Let me know what you think,
Maxime

1: https://lore.kernel.org/r/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org
2: https://lore.kernel.org/r/20250611181528.19542-1-laurent.pinchart@ideasonboard.com

---
Changes in v5:
- Standardize on using RGB888_1X24 for RGB888 instead of BGR888
- Drop the BGR patch
- Drop the csi driver patches in favor of unicam
- Link to v4: https://lore.kernel.org/r/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org

Changes in v4:
- Rebased on 6.18-rc1
- Link to v3: https://lore.kernel.org/r/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org

Changes in v3:
- Fix typos in commit messages
- use dev_warn_once for deprecation warnings
- Reintroduce dropped unsupported colorspace handling
- Remove unneeded fallthroughs
- Link to v2: https://lore.kernel.org/r/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org

Changes in v2:
- Don't drop RGB, but treat it as deprecated instead.
- Rebase on 6.17-rc5
- Link to v1: https://lore.kernel.org/r/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org

---
Maxime Ripard (2):
      media: uapi: Clarify MBUS color component order for serial buses
      media: bcm2835-unicam: Fix RGB format / mbus code association

 .../userspace-api/media/v4l/subdev-formats.rst     | 19 +++++++-----
 drivers/media/platform/broadcom/bcm2835-unicam.c   | 36 +++++++++++++++++++---
 2 files changed, 43 insertions(+), 12 deletions(-)
---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20250612-csi-bgr-rgb-b837980c00b3

Best regards,
-- 
Maxime Ripard <mripard@redhat.com>

[PATCH v5 1/2] media: uapi: Clarify MBUS color component order for serial buses

[Maxime Ripard]

The subdev format documentation has a subsection describing how to use
the media bus pixel codes for serial buses. While it describes the
sampling part well, it doesn't really describe the current convention
used for the components order.

Let's improve that.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 .../userspace-api/media/v4l/subdev-formats.rst        | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index cf970750dd4c6ab32274f75453390eb8148ef3c6..6d57c325ffa506fd57dad0845c9a742fd199a6f0 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -157,18 +157,21 @@ memory.
 While there is a relationship between image formats on buses and image
 formats in memory (a raw Bayer image won't be magically converted to
 JPEG just by storing it to memory), there is no one-to-one
 correspondence between them.
 
-The media bus pixel codes document parallel formats. Should the pixel data be
-transported over a serial bus, the media bus pixel code that describes a
-parallel format that transfers a sample on a single clock cycle is used. For
-instance, both MEDIA_BUS_FMT_BGR888_1X24 and MEDIA_BUS_FMT_BGR888_3X8 are used
-on parallel busses for transferring an 8 bits per sample BGR data, whereas on
-serial busses the data in this format is only referred to using
-MEDIA_BUS_FMT_BGR888_1X24. This is because there is effectively only a single
-way to transport that format on the serial busses.
+While the media bus pixel codes are named based on how pixels are
+transmitted on parallel buses, serial buses do not define separate
+codes. By convention, they use the codes that transfer a sample on a
+single clock cycle. and whose names correspond to the order in which
+colour components are transmitted on the serial bus. For instance, the
+MIPI CSI-2 24-bit RGB (RGB888) format uses the MEDIA_BUS_FMT_RGB888_1X24
+media bus code because CSI-2 transmits the blue colour component first,
+followed by green and red, and MEDIA_BUS_FMT_RGB888_1X24 defines the
+first bit of blue at index 0. While used for 24-bit RGB data on parallel
+buses, the MEDIA_BUS_FMT_RGB888_3X8 or MEDIA_BUS_FMT_BGR888_1X24 codes
+must not be used for CSI-2.
 
 Packed RGB Formats
 ^^^^^^^^^^^^^^^^^^
 
 Those formats transfer pixel data as red, green and blue components. The

-- 
2.52.0

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

[Maxime Ripard]

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

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

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

On Mon, Feb 09, 2026 at 04:03:17PM +0100, Maxime Ripard wrote:
> 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.");

s/RGB88/RGB888/

But I find this confusing, the message doesn't make it clear if RGB888 +
RGB888_1X24 is expected or is an error. I think the following would be
clearer.

				dev_warn_once(node->dev->dev,
					      "Incorrect pixel format BGR24 for BGR888_1X24. Fix your application to use RGB24.");

> +			} 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.");

				dev_warn_once(node->dev->dev,
					      "Incorrect pixel format RGB24 for RGB888_1X24. Fix your application to use BGR24.");

Or you could combine those two (untested):

diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index f10064107d54..e9f191536765 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -2148,22 +2148,38 @@ static int unicam_video_link_validate(struct media_link *link)
 		const struct v4l2_pix_format *fmt = &node->fmt.fmt.pix;
 		const struct unicam_format_info *fmtinfo;

-		fmtinfo = unicam_find_format_by_fourcc(fmt->pixelformat,
-						       UNICAM_SD_PAD_SOURCE_IMAGE);
+		fmtinfo = unicam_find_format_by_code(format->code,
+						     UNICAM_SD_PAD_SOURCE_IMAGE);
 		if (WARN_ON(!fmtinfo)) {
 			ret = -EPIPE;
 			goto out;
 		}

-		if (fmtinfo->code != format->code ||
-		    fmt->height != format->height ||
+		if (fmtinfo->fourcc != fmt->pixelformat) {
+			if ((fmtinfo->fourcc == V4L2_PIX_FMT_BGR24 &&
+			     format->code == MEDIA_BUS_FMT_BGR888_1X24) ||
+			    (fmtinfo->fourcc == V4L2_PIX_FMT_RGB24 &&
+			     format->code == MEDIA_BUS_FMT_RGB888_1X24)) {
+                                dev_warn_once(node->dev->dev,
+                                              "Incorrect pixel format %p4CC for 0x%04x. Fix your application to use %p4CC.\n",
+                                              &fmt->pixelformat, format->code, &fmtinfo->fourcc);
+			} else {
+				dev_dbg(node->dev->dev,
+					"image: format mismatch: 0x%04x <=> %p4CCx\n",
+					format->code, &fmt->pixelformat);
+				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,
+				"image: (%u x %u) %s != (%u x %u) %s\n",
+				fmt->width, fmt->height,
 				v4l2_field_names[fmt->field],
-				format->width, format->height, format->code,
+				format->width, format->height,
 				v4l2_field_names[format->field]);
 			ret = -EPIPE;
 		}

> +			} 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]);

As this message is printed specifically due to a format mismatch, I
would drop the size and field information:

				dev_dbg(node->dev->dev,
					"image: format mismatch: 0x%04x <=> %p4CC\n",
					format->code, &fmt->pixelformat);

> +				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,

And here you can drop the format.

I like the approach in this v5. As far as I can see, it won't break
userspace, will warn of incorrect format usage, and implements the
backward compatibility in the driver that initially got it wrong,
unicam. I'm happy with it.

-- 
Regards,

Laurent Pinchart

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 8045 bytes --]

Hi Laurent,

On Tue, Feb 10, 2026 at 12:44:36AM +0200, Laurent Pinchart wrote:
> On Mon, Feb 09, 2026 at 04:03:17PM +0100, Maxime Ripard wrote:
> > 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.");
> 
> s/RGB88/RGB888/
> 
> But I find this confusing, the message doesn't make it clear if RGB888 +
> RGB888_1X24 is expected or is an error. I think the following would be
> clearer.
> 
> 				dev_warn_once(node->dev->dev,
> 					      "Incorrect pixel format BGR24 for BGR888_1X24. Fix your application to use RGB24.");
> 
> > +			} 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.");
> 
> 				dev_warn_once(node->dev->dev,
> 					      "Incorrect pixel format RGB24 for RGB888_1X24. Fix your application to use BGR24.");
> 
> Or you could combine those two (untested):
> 
> diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> index f10064107d54..e9f191536765 100644
> --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> @@ -2148,22 +2148,38 @@ static int unicam_video_link_validate(struct media_link *link)
>  		const struct v4l2_pix_format *fmt = &node->fmt.fmt.pix;
>  		const struct unicam_format_info *fmtinfo;
> 
> -		fmtinfo = unicam_find_format_by_fourcc(fmt->pixelformat,
> -						       UNICAM_SD_PAD_SOURCE_IMAGE);
> +		fmtinfo = unicam_find_format_by_code(format->code,
> +						     UNICAM_SD_PAD_SOURCE_IMAGE);
>  		if (WARN_ON(!fmtinfo)) {
>  			ret = -EPIPE;
>  			goto out;
>  		}
> 
> -		if (fmtinfo->code != format->code ||
> -		    fmt->height != format->height ||
> +		if (fmtinfo->fourcc != fmt->pixelformat) {

I gave it a try, and fmtinfo->fourcc would always be equal to
fmt->pixelformat, so we never enter that branch and don't warn.

I kept the old lookup and condition...

> +			if ((fmtinfo->fourcc == V4L2_PIX_FMT_BGR24 &&
> +			     format->code == MEDIA_BUS_FMT_BGR888_1X24) ||
> +			    (fmtinfo->fourcc == V4L2_PIX_FMT_RGB24 &&
> +			     format->code == MEDIA_BUS_FMT_RGB888_1X24)) {
> +                                dev_warn_once(node->dev->dev,
> +                                              "Incorrect pixel format %p4CC for 0x%04x. Fix your application to use %p4CC.\n",
> +                                              &fmt->pixelformat, format->code, &fmtinfo->fourcc);

... Added a call to unicam_find_format_by_code() here to print what we should do ...

> +			} else {
> +				dev_dbg(node->dev->dev,
> +					"image: format mismatch: 0x%04x <=> %p4CCx\n",
> +					format->code, &fmt->pixelformat);
> +				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,
> +				"image: (%u x %u) %s != (%u x %u) %s\n",
> +				fmt->width, fmt->height,
>  				v4l2_field_names[fmt->field],
> -				format->width, format->height, format->code,
> +				format->width, format->height,
>  				v4l2_field_names[format->field]);
>  			ret = -EPIPE;
>  		}
> 
> > +			} 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]);
> 
> As this message is printed specifically due to a format mismatch, I
> would drop the size and field information:
> 
> 				dev_dbg(node->dev->dev,
> 					"image: format mismatch: 0x%04x <=> %p4CC\n",
> 					format->code, &fmt->pixelformat);
> 
> > +				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,
> 
> And here you can drop the format.
> 
> I like the approach in this v5. As far as I can see, it won't break
> userspace, will warn of incorrect format usage, and implements the
> backward compatibility in the driver that initially got it wrong,
> unicam. I'm happy with it.

And the rest works fine. I'll send a new version by the end of the week
unless some other review comes up. Thanks!

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

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

[Laurent Pinchart]

On Wed, Feb 11, 2026 at 11:35:32AM +0100, Maxime Ripard wrote:
> On Tue, Feb 10, 2026 at 12:44:36AM +0200, Laurent Pinchart wrote:
> > On Mon, Feb 09, 2026 at 04:03:17PM +0100, Maxime Ripard wrote:
> > > 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.");
> > 
> > s/RGB88/RGB888/
> > 
> > But I find this confusing, the message doesn't make it clear if RGB888 +
> > RGB888_1X24 is expected or is an error. I think the following would be
> > clearer.
> > 
> > 				dev_warn_once(node->dev->dev,
> > 					      "Incorrect pixel format BGR24 for BGR888_1X24. Fix your application to use RGB24.");
> > 
> > > +			} 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.");
> > 
> > 				dev_warn_once(node->dev->dev,
> > 					      "Incorrect pixel format RGB24 for RGB888_1X24. Fix your application to use BGR24.");
> > 
> > Or you could combine those two (untested):
> > 
> > diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > index f10064107d54..e9f191536765 100644
> > --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> > +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > @@ -2148,22 +2148,38 @@ static int unicam_video_link_validate(struct media_link *link)
> >  		const struct v4l2_pix_format *fmt = &node->fmt.fmt.pix;
> >  		const struct unicam_format_info *fmtinfo;
> > 
> > -		fmtinfo = unicam_find_format_by_fourcc(fmt->pixelformat,
> > -						       UNICAM_SD_PAD_SOURCE_IMAGE);
> > +		fmtinfo = unicam_find_format_by_code(format->code,
> > +						     UNICAM_SD_PAD_SOURCE_IMAGE);
> >  		if (WARN_ON(!fmtinfo)) {
> >  			ret = -EPIPE;
> >  			goto out;
> >  		}
> > 
> > -		if (fmtinfo->code != format->code ||
> > -		    fmt->height != format->height ||
> > +		if (fmtinfo->fourcc != fmt->pixelformat) {
> 
> I gave it a try, and fmtinfo->fourcc would always be equal to
> fmt->pixelformat, so we never enter that branch and don't warn.

I'm puzzled by this. Let's consider the incorrect case where the media
bus format on the subdev pad (format->code) is BGR888_1X24 and the pixel
format (fmt->pixelformat) BGR24.

The unicam_find_format_by_code() function iterates over
unicam_image_formats and finds the entry that has been patched to

 	}, {
 		.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,
 	}, {

fmtinfo->fourcc is V4L2_PIX_FMT_RGB24, which is not equal to
fmt->pixelformat (BGR24). What am I missing ?

> I kept the old lookup and condition...
> 
> > +			if ((fmtinfo->fourcc == V4L2_PIX_FMT_BGR24 &&
> > +			     format->code == MEDIA_BUS_FMT_BGR888_1X24) ||
> > +			    (fmtinfo->fourcc == V4L2_PIX_FMT_RGB24 &&
> > +			     format->code == MEDIA_BUS_FMT_RGB888_1X24)) {
> > +                                dev_warn_once(node->dev->dev,
> > +                                              "Incorrect pixel format %p4CC for 0x%04x. Fix your application to use %p4CC.\n",
> > +                                              &fmt->pixelformat, format->code, &fmtinfo->fourcc);
> 
> ... Added a call to unicam_find_format_by_code() here to print what we should do ...
> 
> > +			} else {
> > +				dev_dbg(node->dev->dev,
> > +					"image: format mismatch: 0x%04x <=> %p4CCx\n",
> > +					format->code, &fmt->pixelformat);
> > +				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,
> > +				"image: (%u x %u) %s != (%u x %u) %s\n",
> > +				fmt->width, fmt->height,
> >  				v4l2_field_names[fmt->field],
> > -				format->width, format->height, format->code,
> > +				format->width, format->height,
> >  				v4l2_field_names[format->field]);
> >  			ret = -EPIPE;
> >  		}
> > 
> > > +			} 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]);
> > 
> > As this message is printed specifically due to a format mismatch, I
> > would drop the size and field information:
> > 
> > 				dev_dbg(node->dev->dev,
> > 					"image: format mismatch: 0x%04x <=> %p4CC\n",
> > 					format->code, &fmt->pixelformat);
> > 
> > > +				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,
> > 
> > And here you can drop the format.
> > 
> > I like the approach in this v5. As far as I can see, it won't break
> > userspace, will warn of incorrect format usage, and implements the
> > backward compatibility in the driver that initially got it wrong,
> > unicam. I'm happy with it.
> 
> And the rest works fine. I'll send a new version by the end of the week
> unless some other review comes up. Thanks!

-- 
Regards,

Laurent Pinchart

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 7307 bytes --]

On Thu, Feb 12, 2026 at 11:18:43AM +0200, Laurent Pinchart wrote:
> On Wed, Feb 11, 2026 at 11:35:32AM +0100, Maxime Ripard wrote:
> > On Tue, Feb 10, 2026 at 12:44:36AM +0200, Laurent Pinchart wrote:
> > > On Mon, Feb 09, 2026 at 04:03:17PM +0100, Maxime Ripard wrote:
> > > > 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.");
> > > 
> > > s/RGB88/RGB888/
> > > 
> > > But I find this confusing, the message doesn't make it clear if RGB888 +
> > > RGB888_1X24 is expected or is an error. I think the following would be
> > > clearer.
> > > 
> > > 				dev_warn_once(node->dev->dev,
> > > 					      "Incorrect pixel format BGR24 for BGR888_1X24. Fix your application to use RGB24.");
> > > 
> > > > +			} 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.");
> > > 
> > > 				dev_warn_once(node->dev->dev,
> > > 					      "Incorrect pixel format RGB24 for RGB888_1X24. Fix your application to use BGR24.");
> > > 
> > > Or you could combine those two (untested):
> > > 
> > > diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > > index f10064107d54..e9f191536765 100644
> > > --- a/drivers/media/platform/broadcom/bcm2835-unicam.c
> > > +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > > @@ -2148,22 +2148,38 @@ static int unicam_video_link_validate(struct media_link *link)
> > >  		const struct v4l2_pix_format *fmt = &node->fmt.fmt.pix;
> > >  		const struct unicam_format_info *fmtinfo;
> > > 
> > > -		fmtinfo = unicam_find_format_by_fourcc(fmt->pixelformat,
> > > -						       UNICAM_SD_PAD_SOURCE_IMAGE);
> > > +		fmtinfo = unicam_find_format_by_code(format->code,
> > > +						     UNICAM_SD_PAD_SOURCE_IMAGE);
> > >  		if (WARN_ON(!fmtinfo)) {
> > >  			ret = -EPIPE;
> > >  			goto out;
> > >  		}
> > > 
> > > -		if (fmtinfo->code != format->code ||
> > > -		    fmt->height != format->height ||
> > > +		if (fmtinfo->fourcc != fmt->pixelformat) {
> > 
> > I gave it a try, and fmtinfo->fourcc would always be equal to
> > fmt->pixelformat, so we never enter that branch and don't warn.
> 
> I'm puzzled by this.

Sorry, I was thinking about an earlier case where I missed a condition.
It's still not working though, see below

> Let's consider the incorrect case where the media
> bus format on the subdev pad (format->code) is BGR888_1X24 and the pixel
> format (fmt->pixelformat) BGR24.
> 
> The unicam_find_format_by_code() function iterates over
> unicam_image_formats and finds the entry that has been patched to
> 
>  	}, {
>  		.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,
>  	}, {
> 
> fmtinfo->fourcc is V4L2_PIX_FMT_RGB24, which is not equal to
> fmt->pixelformat (BGR24). What am I missing ?
>
> > I kept the old lookup and condition...
> > 
> > > +			if ((fmtinfo->fourcc == V4L2_PIX_FMT_BGR24 &&
> > > +			     format->code == MEDIA_BUS_FMT_BGR888_1X24) ||
> > > +			    (fmtinfo->fourcc == V4L2_PIX_FMT_RGB24 &&
> > > +			     format->code == MEDIA_BUS_FMT_RGB888_1X24)) {

The application I tested this with was using RGB24 /
MEDIA_BUS_FMT_RGB888_1X24 to get the right pixel order.

So, with the changes in those patches, the lookup by mbus code will now
return the BGR24 fmtinfo, but fmt->pixelformat is RGB24.

I've changed it to test fmt->pixelformat instead of fmtinfo->fourcc, and
it works fine now.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]