[PATCH v4 0/4] media: Fix CSI2 RGB vs BGR pixel order

[PATCH v4 0/4] 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 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 (4):
      media: uapi: Clarify MBUS color component order for serial buses
      media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16
      media: tc358743: Fix the RGB MBUS format
      media: gc2145: Fix the RGB MBUS format

 .../userspace-api/media/v4l/subdev-formats.rst     | 51 ++++++++++++++++++---
 drivers/media/i2c/gc2145.c                         | 24 +++++++++-
 drivers/media/i2c/tc358743.c                       | 53 ++++++++++++++++++----
 include/uapi/linux/media-bus-format.h              |  3 +-
 4 files changed, 113 insertions(+), 18 deletions(-)
---
base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
change-id: 20250612-csi-bgr-rgb-b837980c00b3

Best regards,
-- 
Maxime Ripard <mripard@kernel.org>

[PATCH v4 1/4] 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>
---
 Documentation/userspace-api/media/v4l/subdev-formats.rst | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index 2a94371448dc07e5c7097421bd82f42dcd7e21aa..8e92f784abd8123f9ea950f954a60af56ee76dbe 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -158,16 +158,18 @@ 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.
+parallel format that transfers a sample on a single clock cycle is used. The
+color component order used is the same used on the serial bus. 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,
+with BGR meaning that the blue component is transmitted first, then green, then
+red. This is because there is effectively only a single way to transport that
+format on the serial busses.
 
 Packed RGB Formats
 ^^^^^^^^^^^^^^^^^^
 
 Those formats transfer pixel data as red, green and blue components. The

-- 
2.51.0

Re: [PATCH v4 1/4] media: uapi: Clarify MBUS color component order for serial buses

[Laurent Pinchart]

Hi Maxime,

Thank you for the patch.

On Mon, Oct 13, 2025 at 01:01:33PM +0200, Maxime Ripard wrote:
> 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.

A long due improvement, thanks for working on it.

> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  Documentation/userspace-api/media/v4l/subdev-formats.rst | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> index 2a94371448dc07e5c7097421bd82f42dcd7e21aa..8e92f784abd8123f9ea950f954a60af56ee76dbe 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> @@ -158,16 +158,18 @@ 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.
> +parallel format that transfers a sample on a single clock cycle is used. The
> +color component order used is the same used on the serial bus. 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,
> +with BGR meaning that the blue component is transmitted first, then green, then
> +red. This is because there is effectively only a single way to transport that
> +format on the serial busses.

I find this confusing. The text says that on serial buses
MEDIA_BUS_FMT_BGR888_1X24 is used, without mentioning this is an example
specific to CSI-2. Here's an attempt at improving the paragraph:


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_BGR888_1X24 media bus code because CSI-2 transmits the blue colour
component first, followed by green and red. While used for 24-bit RGB data on
parallel buses, the MEDIA_BUS_FMT_BGR888_3X8 or MEDIA_BUS_FMT_RGB888_1X24 codes
must not be used for CSI-2.

>  
>  Packed RGB Formats
>  ^^^^^^^^^^^^^^^^^^
>  
>  Those formats transfer pixel data as red, green and blue components. The
> 

-- 
Regards,

Laurent Pinchart

[PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Maxime Ripard]

MIPI-CSI2 sends its RGB format on the wire with the blue component
first, then green, then red. MIPI calls that format "RGB", but by v4l2
conventions it would be BGR.

MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.

We already have BGR666 and BGR888 media bus formats, we don't have any
CSI transceivers using the 444 and 555 variants, but some transceivers
use the CSI RGB565 format, while using the RGB565 media bus code.

That's a mistake, but since we don't have a BGR565 media bus code we
need to introduce one before fixing it.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
 include/uapi/linux/media-bus-format.h              |  3 +-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
--- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
+++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
@@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
       - b\ :sub:`4`
       - b\ :sub:`3`
       - b\ :sub:`2`
       - b\ :sub:`1`
       - b\ :sub:`0`
+    * .. _MEDIA-BUS-FMT-BGR565-1X16:
+
+      - MEDIA_BUS_FMT_BGR565_1X16
+      - 0x1028
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      -
+      - b\ :sub:`4`
+      - b\ :sub:`3`
+      - b\ :sub:`2`
+      - b\ :sub:`1`
+      - b\ :sub:`0`
+      - g\ :sub:`5`
+      - g\ :sub:`4`
+      - g\ :sub:`3`
+      - g\ :sub:`2`
+      - g\ :sub:`1`
+      - g\ :sub:`0`
+      - r\ :sub:`4`
+      - r\ :sub:`3`
+      - r\ :sub:`2`
+      - r\ :sub:`1`
+      - r\ :sub:`0`
     * .. _MEDIA-BUS-FMT-BGR565-2X8-BE:
 
       - MEDIA_BUS_FMT_BGR565_2X8_BE
       - 0x1005
       -
diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
index ff62056feed5b6588bfcfdff178f5b68eecd3a26..a73d91876d31844bf8c2da91ddea541181840bd2 100644
--- a/include/uapi/linux/media-bus-format.h
+++ b/include/uapi/linux/media-bus-format.h
@@ -32,17 +32,18 @@
  * new pixel codes.
  */
 
 #define MEDIA_BUS_FMT_FIXED			0x0001
 
-/* RGB - next is	0x1028 */
+/* RGB - next is	0x1029 */
 #define MEDIA_BUS_FMT_RGB444_1X12		0x1016
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE	0x1001
 #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE	0x1002
 #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE	0x1003
 #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE	0x1004
 #define MEDIA_BUS_FMT_RGB565_1X16		0x1017
+#define MEDIA_BUS_FMT_BGR565_1X16		0x1028
 #define MEDIA_BUS_FMT_BGR565_2X8_BE		0x1005
 #define MEDIA_BUS_FMT_BGR565_2X8_LE		0x1006
 #define MEDIA_BUS_FMT_RGB565_2X8_BE		0x1007
 #define MEDIA_BUS_FMT_RGB565_2X8_LE		0x1008
 #define MEDIA_BUS_FMT_RGB666_1X18		0x1009

-- 
2.51.0

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> MIPI-CSI2 sends its RGB format on the wire with the blue component
> first, then green, then red. MIPI calls that format "RGB", but by v4l2
> conventions it would be BGR.
> 
> MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> 
> We already have BGR666 and BGR888 media bus formats, we don't have any
> CSI transceivers using the 444 and 555 variants, but some transceivers
> use the CSI RGB565 format, while using the RGB565 media bus code.
> 
> That's a mistake, but since we don't have a BGR565 media bus code we
> need to introduce one before fixing it.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
>  include/uapi/linux/media-bus-format.h              |  3 +-
>  2 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
>        - b\ :sub:`4`
>        - b\ :sub:`3`
>        - b\ :sub:`2`
>        - b\ :sub:`1`
>        - b\ :sub:`0`
> +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> +
> +      - MEDIA_BUS_FMT_BGR565_1X16
> +      - 0x1028
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      -
> +      - b\ :sub:`4`
> +      - b\ :sub:`3`
> +      - b\ :sub:`2`
> +      - b\ :sub:`1`
> +      - b\ :sub:`0`
> +      - g\ :sub:`5`
> +      - g\ :sub:`4`
> +      - g\ :sub:`3`
> +      - g\ :sub:`2`
> +      - g\ :sub:`1`
> +      - g\ :sub:`0`
> +      - r\ :sub:`4`
> +      - r\ :sub:`3`
> +      - r\ :sub:`2`
> +      - r\ :sub:`1`
> +      - r\ :sub:`0`

We're definitely in convention territory, because this is not how 16-bit
RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
starts with bit 0, not bit 4.

Have you explored the alternative of picking the parallel bus code that
matches the serial order when transmitted with the least significant bit
first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.

>      * .. _MEDIA-BUS-FMT-BGR565-2X8-BE:
>  
>        - MEDIA_BUS_FMT_BGR565_2X8_BE
>        - 0x1005
>        -
> diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
> index ff62056feed5b6588bfcfdff178f5b68eecd3a26..a73d91876d31844bf8c2da91ddea541181840bd2 100644
> --- a/include/uapi/linux/media-bus-format.h
> +++ b/include/uapi/linux/media-bus-format.h
> @@ -32,17 +32,18 @@
>   * new pixel codes.
>   */
>  
>  #define MEDIA_BUS_FMT_FIXED			0x0001
>  
> -/* RGB - next is	0x1028 */
> +/* RGB - next is	0x1029 */
>  #define MEDIA_BUS_FMT_RGB444_1X12		0x1016
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE	0x1001
>  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE	0x1002
>  #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE	0x1003
>  #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE	0x1004
>  #define MEDIA_BUS_FMT_RGB565_1X16		0x1017
> +#define MEDIA_BUS_FMT_BGR565_1X16		0x1028
>  #define MEDIA_BUS_FMT_BGR565_2X8_BE		0x1005
>  #define MEDIA_BUS_FMT_BGR565_2X8_LE		0x1006
>  #define MEDIA_BUS_FMT_RGB565_2X8_BE		0x1007
>  #define MEDIA_BUS_FMT_RGB565_2X8_LE		0x1008
>  #define MEDIA_BUS_FMT_RGB666_1X18		0x1009

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > conventions it would be BGR.
> > 
> > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > 
> > We already have BGR666 and BGR888 media bus formats, we don't have any
> > CSI transceivers using the 444 and 555 variants, but some transceivers
> > use the CSI RGB565 format, while using the RGB565 media bus code.
> > 
> > That's a mistake, but since we don't have a BGR565 media bus code we
> > need to introduce one before fixing it.
> > 
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > ---
> >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> >  include/uapi/linux/media-bus-format.h              |  3 +-
> >  2 files changed, 39 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> >        - b\ :sub:`4`
> >        - b\ :sub:`3`
> >        - b\ :sub:`2`
> >        - b\ :sub:`1`
> >        - b\ :sub:`0`
> > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > +
> > +      - MEDIA_BUS_FMT_BGR565_1X16
> > +      - 0x1028
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      -
> > +      - b\ :sub:`4`
> > +      - b\ :sub:`3`
> > +      - b\ :sub:`2`
> > +      - b\ :sub:`1`
> > +      - b\ :sub:`0`
> > +      - g\ :sub:`5`
> > +      - g\ :sub:`4`
> > +      - g\ :sub:`3`
> > +      - g\ :sub:`2`
> > +      - g\ :sub:`1`
> > +      - g\ :sub:`0`
> > +      - r\ :sub:`4`
> > +      - r\ :sub:`3`
> > +      - r\ :sub:`2`
> > +      - r\ :sub:`1`
> > +      - r\ :sub:`0`
> 
> We're definitely in convention territory, because this is not how 16-bit
> RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> starts with bit 0, not bit 4.
> 
> Have you explored the alternative of picking the parallel bus code that
> matches the serial order when transmitted with the least significant bit
> first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.

To be clear, media bus codes are a matter of conventions. Some
conventions would be easier to explain that others, and can also be more
consistent with pixel format namings, but at the end of the day they're
all conventions. While saying "pick the media bus code that transmits a
pixel in one clock sample, with the bit order matching LSB-first
transmission" could be the simplest to document, there will be a
mismatch in component orders between the media bus code and the pixel
format in some cases. There may also be more drivers implementing other
conventions, making the transition more difficult.

I'll be very busy the upcoming week and will likely not be able to
participate in this discussion in the near future.

> >      * .. _MEDIA-BUS-FMT-BGR565-2X8-BE:
> >  
> >        - MEDIA_BUS_FMT_BGR565_2X8_BE
> >        - 0x1005
> >        -
> > diff --git a/include/uapi/linux/media-bus-format.h b/include/uapi/linux/media-bus-format.h
> > index ff62056feed5b6588bfcfdff178f5b68eecd3a26..a73d91876d31844bf8c2da91ddea541181840bd2 100644
> > --- a/include/uapi/linux/media-bus-format.h
> > +++ b/include/uapi/linux/media-bus-format.h
> > @@ -32,17 +32,18 @@
> >   * new pixel codes.
> >   */
> >  
> >  #define MEDIA_BUS_FMT_FIXED			0x0001
> >  
> > -/* RGB - next is	0x1028 */
> > +/* RGB - next is	0x1029 */
> >  #define MEDIA_BUS_FMT_RGB444_1X12		0x1016
> >  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_BE	0x1001
> >  #define MEDIA_BUS_FMT_RGB444_2X8_PADHI_LE	0x1002
> >  #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_BE	0x1003
> >  #define MEDIA_BUS_FMT_RGB555_2X8_PADHI_LE	0x1004
> >  #define MEDIA_BUS_FMT_RGB565_1X16		0x1017
> > +#define MEDIA_BUS_FMT_BGR565_1X16		0x1028
> >  #define MEDIA_BUS_FMT_BGR565_2X8_BE		0x1005
> >  #define MEDIA_BUS_FMT_BGR565_2X8_LE		0x1006
> >  #define MEDIA_BUS_FMT_RGB565_2X8_BE		0x1007
> >  #define MEDIA_BUS_FMT_RGB565_2X8_LE		0x1008
> >  #define MEDIA_BUS_FMT_RGB666_1X18		0x1009

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Maxime Ripard]

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

Hi Laurent,

On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > conventions it would be BGR.
> > > 
> > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > 
> > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > 
> > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > need to introduce one before fixing it.
> > > 
> > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > ---
> > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > >        - b\ :sub:`4`
> > >        - b\ :sub:`3`
> > >        - b\ :sub:`2`
> > >        - b\ :sub:`1`
> > >        - b\ :sub:`0`
> > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > +
> > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > +      - 0x1028
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      -
> > > +      - b\ :sub:`4`
> > > +      - b\ :sub:`3`
> > > +      - b\ :sub:`2`
> > > +      - b\ :sub:`1`
> > > +      - b\ :sub:`0`
> > > +      - g\ :sub:`5`
> > > +      - g\ :sub:`4`
> > > +      - g\ :sub:`3`
> > > +      - g\ :sub:`2`
> > > +      - g\ :sub:`1`
> > > +      - g\ :sub:`0`
> > > +      - r\ :sub:`4`
> > > +      - r\ :sub:`3`
> > > +      - r\ :sub:`2`
> > > +      - r\ :sub:`1`
> > > +      - r\ :sub:`0`
> > 
> > We're definitely in convention territory, because this is not how 16-bit
> > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > starts with bit 0, not bit 4.
> > 
> > Have you explored the alternative of picking the parallel bus code that
> > matches the serial order when transmitted with the least significant bit
> > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> 
> To be clear, media bus codes are a matter of conventions. Some
> conventions would be easier to explain that others, and can also be more
> consistent with pixel format namings, but at the end of the day they're
> all conventions. While saying "pick the media bus code that transmits a
> pixel in one clock sample, with the bit order matching LSB-first
> transmission" could be the simplest to document, there will be a
> mismatch in component orders between the media bus code and the pixel
> format in some cases. There may also be more drivers implementing other
> conventions, making the transition more difficult.
> 
> I'll be very busy the upcoming week and will likely not be able to
> participate in this discussion in the near future.

For the record, we've discussed it on IRC recently.

The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
variants make sense to me. And we can easily document it, because we
could match the first bit transmitted with the least significant bit
of a media bus code indeed.

Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
That's indeed the case right now with tc358743:
https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775

Unicam however hardcodes (and validates) that the v4l2 format codes
matches the media bus code of the other end:

https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333

That alone makes total sense, but it has an association between
V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24

https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343

Using the convention you suggested, this association is wrong, and
V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
the red and blue color components are mixed up.

I initially tried to fix it in my v1 by removing the RGB24 support
https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/

This was shot down (rightfully) because it would still be broken.

The second version changed the media bus tc358743 reported:
https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/

Dave was against it because it would potentially break userspace, citing
Linus that we shouldn't break userspace ever. I understand and somewhat
agree with his point, but having two drivers reporting the same data
format but with a different meaning is also a way of breaking userspace.

Anyway. It was then suggested to support both in the tc358743. That's
what the second, third and fourth that you commented on worked towards.

https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/

In order to implement your suggestion, I wouldn't to modify tc358743,
but would need to modify the association between the v4l2 format and
media bus code that unicam has. In a way, it's very similar to my first
version that got shot down, and suffers from the same flaws: we could
have a userspace application out there hardcoding formats and codes that
will get an error.

So I'm not sure your suggestion really works, unless we reevaluate what
we mean by breaking userspace. Either way, I don't care, I just want to
get pixels in the expected (and documented!) order when using unicam.

Maxime

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

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > conventions it would be BGR.
> > > > 
> > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > 
> > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > 
> > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > need to introduce one before fixing it.
> > > > 
> > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > ---
> > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > >        - b\ :sub:`4`
> > > >        - b\ :sub:`3`
> > > >        - b\ :sub:`2`
> > > >        - b\ :sub:`1`
> > > >        - b\ :sub:`0`
> > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > +
> > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > +      - 0x1028
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      -
> > > > +      - b\ :sub:`4`
> > > > +      - b\ :sub:`3`
> > > > +      - b\ :sub:`2`
> > > > +      - b\ :sub:`1`
> > > > +      - b\ :sub:`0`
> > > > +      - g\ :sub:`5`
> > > > +      - g\ :sub:`4`
> > > > +      - g\ :sub:`3`
> > > > +      - g\ :sub:`2`
> > > > +      - g\ :sub:`1`
> > > > +      - g\ :sub:`0`
> > > > +      - r\ :sub:`4`
> > > > +      - r\ :sub:`3`
> > > > +      - r\ :sub:`2`
> > > > +      - r\ :sub:`1`
> > > > +      - r\ :sub:`0`
> > > 
> > > We're definitely in convention territory, because this is not how 16-bit
> > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > starts with bit 0, not bit 4.
> > > 
> > > Have you explored the alternative of picking the parallel bus code that
> > > matches the serial order when transmitted with the least significant bit
> > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > 
> > To be clear, media bus codes are a matter of conventions. Some
> > conventions would be easier to explain that others, and can also be more
> > consistent with pixel format namings, but at the end of the day they're
> > all conventions. While saying "pick the media bus code that transmits a
> > pixel in one clock sample, with the bit order matching LSB-first
> > transmission" could be the simplest to document, there will be a
> > mismatch in component orders between the media bus code and the pixel
> > format in some cases. There may also be more drivers implementing other
> > conventions, making the transition more difficult.
> > 
> > I'll be very busy the upcoming week and will likely not be able to
> > participate in this discussion in the near future.
> 
> For the record, we've discussed it on IRC recently.
> 
> The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> variants make sense to me. And we can easily document it, because we
> could match the first bit transmitted with the least significant bit
> of a media bus code indeed.

That's one of the things I like about it, it's consistent and easy to
document. Glad we agree :-)

> Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> That's indeed the case right now with tc358743:
> https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> 
> Unicam however hardcodes (and validates) that the v4l2 format codes
> matches the media bus code of the other end:
> 
> https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> 
> That alone makes total sense, but it has an association between
> V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> 
> https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> 
> Using the convention you suggested, this association is wrong, and
> V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> the red and blue color components are mixed up.

Correct.

> I initially tried to fix it in my v1 by removing the RGB24 support
> https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> 
> This was shot down (rightfully) because it would still be broken.
> 
> The second version changed the media bus tc358743 reported:
> https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> 
> Dave was against it because it would potentially break userspace, citing
> Linus that we shouldn't break userspace ever. I understand and somewhat
> agree with his point, but having two drivers reporting the same data
> format but with a different meaning is also a way of breaking userspace.

Yes, I would find that pretty bad, possibly even worse.

> Anyway. It was then suggested to support both in the tc358743. That's
> what the second, third and fourth that you commented on worked towards.
> 
> https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> 
> In order to implement your suggestion, I wouldn't to modify tc358743,
> but would need to modify the association between the v4l2 format and
> media bus code that unicam has. In a way, it's very similar to my first
> version that got shot down, and suffers from the same flaws: we could
> have a userspace application out there hardcoding formats and codes that
> will get an error.
> 
> So I'm not sure your suggestion really works, unless we reevaluate what
> we mean by breaking userspace. Either way, I don't care, I just want to
> get pixels in the expected (and documented!) order when using unicam.

I've lost track of the status of this series and what your current
suggestion is. Can we standardize on

- Using MEDIA_BUS_FMT_RGB*
- Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
- Possibly implement backward compatibility somewhere (where ?) to avoid
  regressions, but with a big warning

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Maxime Ripard]

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

On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > conventions it would be BGR.
> > > > > 
> > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > 
> > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > 
> > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > need to introduce one before fixing it.
> > > > > 
> > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > ---
> > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > >        - b\ :sub:`4`
> > > > >        - b\ :sub:`3`
> > > > >        - b\ :sub:`2`
> > > > >        - b\ :sub:`1`
> > > > >        - b\ :sub:`0`
> > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > +
> > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > +      - 0x1028
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      -
> > > > > +      - b\ :sub:`4`
> > > > > +      - b\ :sub:`3`
> > > > > +      - b\ :sub:`2`
> > > > > +      - b\ :sub:`1`
> > > > > +      - b\ :sub:`0`
> > > > > +      - g\ :sub:`5`
> > > > > +      - g\ :sub:`4`
> > > > > +      - g\ :sub:`3`
> > > > > +      - g\ :sub:`2`
> > > > > +      - g\ :sub:`1`
> > > > > +      - g\ :sub:`0`
> > > > > +      - r\ :sub:`4`
> > > > > +      - r\ :sub:`3`
> > > > > +      - r\ :sub:`2`
> > > > > +      - r\ :sub:`1`
> > > > > +      - r\ :sub:`0`
> > > > 
> > > > We're definitely in convention territory, because this is not how 16-bit
> > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > starts with bit 0, not bit 4.
> > > > 
> > > > Have you explored the alternative of picking the parallel bus code that
> > > > matches the serial order when transmitted with the least significant bit
> > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > 
> > > To be clear, media bus codes are a matter of conventions. Some
> > > conventions would be easier to explain that others, and can also be more
> > > consistent with pixel format namings, but at the end of the day they're
> > > all conventions. While saying "pick the media bus code that transmits a
> > > pixel in one clock sample, with the bit order matching LSB-first
> > > transmission" could be the simplest to document, there will be a
> > > mismatch in component orders between the media bus code and the pixel
> > > format in some cases. There may also be more drivers implementing other
> > > conventions, making the transition more difficult.
> > > 
> > > I'll be very busy the upcoming week and will likely not be able to
> > > participate in this discussion in the near future.
> > 
> > For the record, we've discussed it on IRC recently.
> > 
> > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > variants make sense to me. And we can easily document it, because we
> > could match the first bit transmitted with the least significant bit
> > of a media bus code indeed.
> 
> That's one of the things I like about it, it's consistent and easy to
> document. Glad we agree :-)
> 
> > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > That's indeed the case right now with tc358743:
> > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > 
> > Unicam however hardcodes (and validates) that the v4l2 format codes
> > matches the media bus code of the other end:
> > 
> > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > 
> > That alone makes total sense, but it has an association between
> > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > 
> > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > 
> > Using the convention you suggested, this association is wrong, and
> > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > the red and blue color components are mixed up.
> 
> Correct.
> 
> > I initially tried to fix it in my v1 by removing the RGB24 support
> > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > 
> > This was shot down (rightfully) because it would still be broken.
> > 
> > The second version changed the media bus tc358743 reported:
> > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > 
> > Dave was against it because it would potentially break userspace, citing
> > Linus that we shouldn't break userspace ever. I understand and somewhat
> > agree with his point, but having two drivers reporting the same data
> > format but with a different meaning is also a way of breaking userspace.
> 
> Yes, I would find that pretty bad, possibly even worse.
> 
> > Anyway. It was then suggested to support both in the tc358743. That's
> > what the second, third and fourth that you commented on worked towards.
> > 
> > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > 
> > In order to implement your suggestion, I wouldn't to modify tc358743,
> > but would need to modify the association between the v4l2 format and
> > media bus code that unicam has. In a way, it's very similar to my first
> > version that got shot down, and suffers from the same flaws: we could
> > have a userspace application out there hardcoding formats and codes that
> > will get an error.
> > 
> > So I'm not sure your suggestion really works, unless we reevaluate what
> > we mean by breaking userspace. Either way, I don't care, I just want to
> > get pixels in the expected (and documented!) order when using unicam.
> 
> I've lost track of the status of this series and what your current
> suggestion is. Can we standardize on
> 
> - Using MEDIA_BUS_FMT_RGB*

I guess we can do that.

> - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam

You called "pretty bad, possibly even worse" to do the exact opposite
(ie, change the bridge media bus to match unicam) because it would break
userspace. Changing the unicam media bus to match the bridge creates the
exact same situation.

The alternative would still be to report both for the bridge, and invert
the current assocation for the v4l2 formats and mbus codes.

> - Possibly implement backward compatibility somewhere (where ?) to avoid
>   regressions, but with a big warning

What would you improve there exactly? It's very clearly in the patches
already, so unless you have some specific comments I'm not really sure
what you want me to do.

Maxime

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

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > conventions it would be BGR.
> > > > > > 
> > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > 
> > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > 
> > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > need to introduce one before fixing it.
> > > > > > 
> > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > ---
> > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > >        - b\ :sub:`4`
> > > > > >        - b\ :sub:`3`
> > > > > >        - b\ :sub:`2`
> > > > > >        - b\ :sub:`1`
> > > > > >        - b\ :sub:`0`
> > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > +
> > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > +      - 0x1028
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      -
> > > > > > +      - b\ :sub:`4`
> > > > > > +      - b\ :sub:`3`
> > > > > > +      - b\ :sub:`2`
> > > > > > +      - b\ :sub:`1`
> > > > > > +      - b\ :sub:`0`
> > > > > > +      - g\ :sub:`5`
> > > > > > +      - g\ :sub:`4`
> > > > > > +      - g\ :sub:`3`
> > > > > > +      - g\ :sub:`2`
> > > > > > +      - g\ :sub:`1`
> > > > > > +      - g\ :sub:`0`
> > > > > > +      - r\ :sub:`4`
> > > > > > +      - r\ :sub:`3`
> > > > > > +      - r\ :sub:`2`
> > > > > > +      - r\ :sub:`1`
> > > > > > +      - r\ :sub:`0`
> > > > > 
> > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > starts with bit 0, not bit 4.
> > > > > 
> > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > matches the serial order when transmitted with the least significant bit
> > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > 
> > > > To be clear, media bus codes are a matter of conventions. Some
> > > > conventions would be easier to explain that others, and can also be more
> > > > consistent with pixel format namings, but at the end of the day they're
> > > > all conventions. While saying "pick the media bus code that transmits a
> > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > transmission" could be the simplest to document, there will be a
> > > > mismatch in component orders between the media bus code and the pixel
> > > > format in some cases. There may also be more drivers implementing other
> > > > conventions, making the transition more difficult.
> > > > 
> > > > I'll be very busy the upcoming week and will likely not be able to
> > > > participate in this discussion in the near future.
> > > 
> > > For the record, we've discussed it on IRC recently.
> > > 
> > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > variants make sense to me. And we can easily document it, because we
> > > could match the first bit transmitted with the least significant bit
> > > of a media bus code indeed.
> > 
> > That's one of the things I like about it, it's consistent and easy to
> > document. Glad we agree :-)
> > 
> > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > That's indeed the case right now with tc358743:
> > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > 
> > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > matches the media bus code of the other end:
> > > 
> > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > 
> > > That alone makes total sense, but it has an association between
> > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > 
> > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > 
> > > Using the convention you suggested, this association is wrong, and
> > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > the red and blue color components are mixed up.
> > 
> > Correct.
> > 
> > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > 
> > > This was shot down (rightfully) because it would still be broken.
> > > 
> > > The second version changed the media bus tc358743 reported:
> > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > 
> > > Dave was against it because it would potentially break userspace, citing
> > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > agree with his point, but having two drivers reporting the same data
> > > format but with a different meaning is also a way of breaking userspace.
> > 
> > Yes, I would find that pretty bad, possibly even worse.
> > 
> > > Anyway. It was then suggested to support both in the tc358743. That's
> > > what the second, third and fourth that you commented on worked towards.
> > > 
> > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > 
> > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > but would need to modify the association between the v4l2 format and
> > > media bus code that unicam has. In a way, it's very similar to my first
> > > version that got shot down, and suffers from the same flaws: we could
> > > have a userspace application out there hardcoding formats and codes that
> > > will get an error.
> > > 
> > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > get pixels in the expected (and documented!) order when using unicam.
> > 
> > I've lost track of the status of this series and what your current
> > suggestion is. Can we standardize on
> > 
> > - Using MEDIA_BUS_FMT_RGB*
> 
> I guess we can do that.
> 
> > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> 
> You called "pretty bad, possibly even worse" to do the exact opposite
> (ie, change the bridge media bus to match unicam) because it would break
> userspace. Changing the unicam media bus to match the bridge creates the
> exact same situation.
> 
> The alternative would still be to report both for the bridge, and invert
> the current assocation for the v4l2 formats and mbus codes.
> 
> > - Possibly implement backward compatibility somewhere (where ?) to avoid
> >   regressions, but with a big warning
> 
> What would you improve there exactly? It's very clearly in the patches
> already, so unless you have some specific comments I'm not really sure
> what you want me to do.

If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
driver, not in the tc358743 driver. Is it possible to implement the
backward compatibility (with a warning) in unicam instead of tc358743 ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Jai Luthra]

Hi Laurent, Maxime,

Quoting Laurent Pinchart (2026-01-28 18:49:03)
> On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > conventions it would be BGR.
> > > > > > > 
> > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > 
> > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > 
> > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > need to introduce one before fixing it.
> > > > > > > 
> > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > ---
> > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > >        - b\ :sub:`4`
> > > > > > >        - b\ :sub:`3`
> > > > > > >        - b\ :sub:`2`
> > > > > > >        - b\ :sub:`1`
> > > > > > >        - b\ :sub:`0`
> > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > +
> > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > +      - 0x1028
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      - b\ :sub:`4`
> > > > > > > +      - b\ :sub:`3`
> > > > > > > +      - b\ :sub:`2`
> > > > > > > +      - b\ :sub:`1`
> > > > > > > +      - b\ :sub:`0`
> > > > > > > +      - g\ :sub:`5`
> > > > > > > +      - g\ :sub:`4`
> > > > > > > +      - g\ :sub:`3`
> > > > > > > +      - g\ :sub:`2`
> > > > > > > +      - g\ :sub:`1`
> > > > > > > +      - g\ :sub:`0`
> > > > > > > +      - r\ :sub:`4`
> > > > > > > +      - r\ :sub:`3`
> > > > > > > +      - r\ :sub:`2`
> > > > > > > +      - r\ :sub:`1`
> > > > > > > +      - r\ :sub:`0`
> > > > > > 
> > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > starts with bit 0, not bit 4.
> > > > > > 
> > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > 
> > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > conventions would be easier to explain that others, and can also be more
> > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > transmission" could be the simplest to document, there will be a
> > > > > mismatch in component orders between the media bus code and the pixel
> > > > > format in some cases. There may also be more drivers implementing other
> > > > > conventions, making the transition more difficult.
> > > > > 
> > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > participate in this discussion in the near future.
> > > > 
> > > > For the record, we've discussed it on IRC recently.
> > > > 
> > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > variants make sense to me. And we can easily document it, because we
> > > > could match the first bit transmitted with the least significant bit
> > > > of a media bus code indeed.
> > > 
> > > That's one of the things I like about it, it's consistent and easy to
> > > document. Glad we agree :-)
> > > 
> > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > That's indeed the case right now with tc358743:
> > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > 
> > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > matches the media bus code of the other end:
> > > > 
> > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > 
> > > > That alone makes total sense, but it has an association between
> > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > 
> > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > 
> > > > Using the convention you suggested, this association is wrong, and
> > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > the red and blue color components are mixed up.
> > > 
> > > Correct.
> > > 
> > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > 
> > > > This was shot down (rightfully) because it would still be broken.
> > > > 
> > > > The second version changed the media bus tc358743 reported:
> > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > 
> > > > Dave was against it because it would potentially break userspace, citing
> > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > agree with his point, but having two drivers reporting the same data
> > > > format but with a different meaning is also a way of breaking userspace.
> > > 
> > > Yes, I would find that pretty bad, possibly even worse.
> > > 
> > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > what the second, third and fourth that you commented on worked towards.
> > > > 
> > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > 
> > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > but would need to modify the association between the v4l2 format and
> > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > version that got shot down, and suffers from the same flaws: we could
> > > > have a userspace application out there hardcoding formats and codes that
> > > > will get an error.
> > > > 
> > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > get pixels in the expected (and documented!) order when using unicam.
> > > 
> > > I've lost track of the status of this series and what your current
> > > suggestion is. Can we standardize on
> > > 
> > > - Using MEDIA_BUS_FMT_RGB*
> > 
> > I guess we can do that.
> > 
> > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > 
> > You called "pretty bad, possibly even worse" to do the exact opposite
> > (ie, change the bridge media bus to match unicam) because it would break
> > userspace. Changing the unicam media bus to match the bridge creates the
> > exact same situation.
> > 
> > The alternative would still be to report both for the bridge, and invert
> > the current assocation for the v4l2 formats and mbus codes.
> > 
> > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > >   regressions, but with a big warning
> > 
> > What would you improve there exactly? It's very clearly in the patches
> > already, so unless you have some specific comments I'm not really sure
> > what you want me to do.
> 
> If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> driver, not in the tc358743 driver. Is it possible to implement the
> backward compatibility (with a warning) in unicam instead of tc358743 ?

Not just unicam. TI's CSI driver (j721e-csi2rx.c) also uses:

V4L2_PIX_FMT_XBGR32 for MEDIA_BUS_FMT_RGB888_1X24 (and vice versa)

(my 2 cents) It's better to have backward compatibility in drivers that
currently don't follow the Media documentation. I agree the docs are
confusing, they tripped me up too, but there are already userspace
scripts/applications that TI / beagle users use that follow those confusing
docs :)

Thanks,
    Jai

> 
> -- 
> Regards,
> 
> Laurent Pinchart
>

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Mon, Feb 02, 2026 at 12:53:15PM +0530, Jai Luthra wrote:
> Hi Laurent, Maxime,
> 
> Quoting Laurent Pinchart (2026-01-28 18:49:03)
> > On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > > conventions it would be BGR.
> > > > > > > > 
> > > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > > 
> > > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > > 
> > > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > > need to introduce one before fixing it.
> > > > > > > > 
> > > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > > ---
> > > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > > 
> > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > > >        - b\ :sub:`4`
> > > > > > > >        - b\ :sub:`3`
> > > > > > > >        - b\ :sub:`2`
> > > > > > > >        - b\ :sub:`1`
> > > > > > > >        - b\ :sub:`0`
> > > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > > +
> > > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > > +      - 0x1028
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      - b\ :sub:`4`
> > > > > > > > +      - b\ :sub:`3`
> > > > > > > > +      - b\ :sub:`2`
> > > > > > > > +      - b\ :sub:`1`
> > > > > > > > +      - b\ :sub:`0`
> > > > > > > > +      - g\ :sub:`5`
> > > > > > > > +      - g\ :sub:`4`
> > > > > > > > +      - g\ :sub:`3`
> > > > > > > > +      - g\ :sub:`2`
> > > > > > > > +      - g\ :sub:`1`
> > > > > > > > +      - g\ :sub:`0`
> > > > > > > > +      - r\ :sub:`4`
> > > > > > > > +      - r\ :sub:`3`
> > > > > > > > +      - r\ :sub:`2`
> > > > > > > > +      - r\ :sub:`1`
> > > > > > > > +      - r\ :sub:`0`
> > > > > > > 
> > > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > > starts with bit 0, not bit 4.
> > > > > > > 
> > > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > > 
> > > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > > conventions would be easier to explain that others, and can also be more
> > > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > > transmission" could be the simplest to document, there will be a
> > > > > > mismatch in component orders between the media bus code and the pixel
> > > > > > format in some cases. There may also be more drivers implementing other
> > > > > > conventions, making the transition more difficult.
> > > > > > 
> > > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > > participate in this discussion in the near future.
> > > > > 
> > > > > For the record, we've discussed it on IRC recently.
> > > > > 
> > > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > > variants make sense to me. And we can easily document it, because we
> > > > > could match the first bit transmitted with the least significant bit
> > > > > of a media bus code indeed.
> > > > 
> > > > That's one of the things I like about it, it's consistent and easy to
> > > > document. Glad we agree :-)
> > > > 
> > > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > > That's indeed the case right now with tc358743:
> > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > > 
> > > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > > matches the media bus code of the other end:
> > > > > 
> > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > > 
> > > > > That alone makes total sense, but it has an association between
> > > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > > 
> > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > > 
> > > > > Using the convention you suggested, this association is wrong, and
> > > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > > the red and blue color components are mixed up.
> > > > 
> > > > Correct.
> > > > 
> > > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > > 
> > > > > This was shot down (rightfully) because it would still be broken.
> > > > > 
> > > > > The second version changed the media bus tc358743 reported:
> > > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > > 
> > > > > Dave was against it because it would potentially break userspace, citing
> > > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > > agree with his point, but having two drivers reporting the same data
> > > > > format but with a different meaning is also a way of breaking userspace.
> > > > 
> > > > Yes, I would find that pretty bad, possibly even worse.
> > > > 
> > > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > > what the second, third and fourth that you commented on worked towards.
> > > > > 
> > > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > > 
> > > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > > but would need to modify the association between the v4l2 format and
> > > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > > version that got shot down, and suffers from the same flaws: we could
> > > > > have a userspace application out there hardcoding formats and codes that
> > > > > will get an error.
> > > > > 
> > > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > > get pixels in the expected (and documented!) order when using unicam.
> > > > 
> > > > I've lost track of the status of this series and what your current
> > > > suggestion is. Can we standardize on
> > > > 
> > > > - Using MEDIA_BUS_FMT_RGB*
> > > 
> > > I guess we can do that.
> > > 
> > > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > > 
> > > You called "pretty bad, possibly even worse" to do the exact opposite
> > > (ie, change the bridge media bus to match unicam) because it would break
> > > userspace. Changing the unicam media bus to match the bridge creates the
> > > exact same situation.
> > > 
> > > The alternative would still be to report both for the bridge, and invert
> > > the current assocation for the v4l2 formats and mbus codes.
> > > 
> > > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > > >   regressions, but with a big warning
> > > 
> > > What would you improve there exactly? It's very clearly in the patches
> > > already, so unless you have some specific comments I'm not really sure
> > > what you want me to do.
> > 
> > If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> > driver, not in the tc358743 driver. Is it possible to implement the
> > backward compatibility (with a warning) in unicam instead of tc358743 ?
> 
> Not just unicam. TI's CSI driver (j721e-csi2rx.c) also uses:
> 
> V4L2_PIX_FMT_XBGR32 for MEDIA_BUS_FMT_RGB888_1X24 (and vice versa)

That is not necessarily wrong. The mapping between media bus codes and
pixel formats is device-dependent. Does the upstream j721e-csi2rx driver
map MEDIA_BUS_FMT_RGB888_1X24 to V4L2_PIX_FMT_XBGR32 but actually writes
a different pixel format to memory ?

> (my 2 cents) It's better to have backward compatibility in drivers that
> currently don't follow the Media documentation. I agree the docs are
> confusing, they tripped me up too, but there are already userspace
> scripts/applications that TI / beagle users use that follow those confusing
> docs :)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Jai Luthra]

Quoting Laurent Pinchart (2026-02-02 15:01:05)
> On Mon, Feb 02, 2026 at 12:53:15PM +0530, Jai Luthra wrote:
> > Hi Laurent, Maxime,
> > 
> > Quoting Laurent Pinchart (2026-01-28 18:49:03)
> > > On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > > > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > > > conventions it would be BGR.
> > > > > > > > > 
> > > > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > > > 
> > > > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > > > 
> > > > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > > > need to introduce one before fixing it.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > > > ---
> > > > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > > > >        - b\ :sub:`4`
> > > > > > > > >        - b\ :sub:`3`
> > > > > > > > >        - b\ :sub:`2`
> > > > > > > > >        - b\ :sub:`1`
> > > > > > > > >        - b\ :sub:`0`
> > > > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > > > +
> > > > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > > > +      - 0x1028
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      - b\ :sub:`4`
> > > > > > > > > +      - b\ :sub:`3`
> > > > > > > > > +      - b\ :sub:`2`
> > > > > > > > > +      - b\ :sub:`1`
> > > > > > > > > +      - b\ :sub:`0`
> > > > > > > > > +      - g\ :sub:`5`
> > > > > > > > > +      - g\ :sub:`4`
> > > > > > > > > +      - g\ :sub:`3`
> > > > > > > > > +      - g\ :sub:`2`
> > > > > > > > > +      - g\ :sub:`1`
> > > > > > > > > +      - g\ :sub:`0`
> > > > > > > > > +      - r\ :sub:`4`
> > > > > > > > > +      - r\ :sub:`3`
> > > > > > > > > +      - r\ :sub:`2`
> > > > > > > > > +      - r\ :sub:`1`
> > > > > > > > > +      - r\ :sub:`0`
> > > > > > > > 
> > > > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > > > starts with bit 0, not bit 4.
> > > > > > > > 
> > > > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > > > 
> > > > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > > > conventions would be easier to explain that others, and can also be more
> > > > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > > > transmission" could be the simplest to document, there will be a
> > > > > > > mismatch in component orders between the media bus code and the pixel
> > > > > > > format in some cases. There may also be more drivers implementing other
> > > > > > > conventions, making the transition more difficult.
> > > > > > > 
> > > > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > > > participate in this discussion in the near future.
> > > > > > 
> > > > > > For the record, we've discussed it on IRC recently.
> > > > > > 
> > > > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > > > variants make sense to me. And we can easily document it, because we
> > > > > > could match the first bit transmitted with the least significant bit
> > > > > > of a media bus code indeed.
> > > > > 
> > > > > That's one of the things I like about it, it's consistent and easy to
> > > > > document. Glad we agree :-)
> > > > > 
> > > > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > > > That's indeed the case right now with tc358743:
> > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > > > 
> > > > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > > > matches the media bus code of the other end:
> > > > > > 
> > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > > > 
> > > > > > That alone makes total sense, but it has an association between
> > > > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > > > 
> > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > > > 
> > > > > > Using the convention you suggested, this association is wrong, and
> > > > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > > > the red and blue color components are mixed up.
> > > > > 
> > > > > Correct.
> > > > > 
> > > > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > > > 
> > > > > > This was shot down (rightfully) because it would still be broken.
> > > > > > 
> > > > > > The second version changed the media bus tc358743 reported:
> > > > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > > > 
> > > > > > Dave was against it because it would potentially break userspace, citing
> > > > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > > > agree with his point, but having two drivers reporting the same data
> > > > > > format but with a different meaning is also a way of breaking userspace.
> > > > > 
> > > > > Yes, I would find that pretty bad, possibly even worse.
> > > > > 
> > > > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > > > what the second, third and fourth that you commented on worked towards.
> > > > > > 
> > > > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > > > 
> > > > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > > > but would need to modify the association between the v4l2 format and
> > > > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > > > version that got shot down, and suffers from the same flaws: we could
> > > > > > have a userspace application out there hardcoding formats and codes that
> > > > > > will get an error.
> > > > > > 
> > > > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > > > get pixels in the expected (and documented!) order when using unicam.
> > > > > 
> > > > > I've lost track of the status of this series and what your current
> > > > > suggestion is. Can we standardize on
> > > > > 
> > > > > - Using MEDIA_BUS_FMT_RGB*
> > > > 
> > > > I guess we can do that.
> > > > 
> > > > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > > > 
> > > > You called "pretty bad, possibly even worse" to do the exact opposite
> > > > (ie, change the bridge media bus to match unicam) because it would break
> > > > userspace. Changing the unicam media bus to match the bridge creates the
> > > > exact same situation.
> > > > 
> > > > The alternative would still be to report both for the bridge, and invert
> > > > the current assocation for the v4l2 formats and mbus codes.
> > > > 
> > > > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > > > >   regressions, but with a big warning
> > > > 
> > > > What would you improve there exactly? It's very clearly in the patches
> > > > already, so unless you have some specific comments I'm not really sure
> > > > what you want me to do.
> > > 
> > > If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> > > driver, not in the tc358743 driver. Is it possible to implement the
> > > backward compatibility (with a warning) in unicam instead of tc358743 ?
> > 
> > Not just unicam. TI's CSI driver (j721e-csi2rx.c) also uses:
> > 
> > V4L2_PIX_FMT_XBGR32 for MEDIA_BUS_FMT_RGB888_1X24 (and vice versa)
> 
> That is not necessarily wrong. The mapping between media bus codes and
> pixel formats is device-dependent. Does the upstream j721e-csi2rx driver
> map MEDIA_BUS_FMT_RGB888_1X24 to V4L2_PIX_FMT_XBGR32 but actually writes
> a different pixel format to memory ?

From the TRM, MIPI RGB888 unpacks to:
BYTE0		BYTE1		BYTE2		BYTE3		
BBBBBBBB	GGGGGGGG	RRRRRRRR	00000000	

Which is V4L2_PIX_FMT_XBGR32 I believe.

> 
> > (my 2 cents) It's better to have backward compatibility in drivers that
> > currently don't follow the Media documentation. I agree the docs are
> > confusing, they tripped me up too, but there are already userspace
> > scripts/applications that TI / beagle users use that follow those confusing
> > docs :)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Thanks,
    Jai

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Mon, Feb 02, 2026 at 04:21:00PM +0530, Jai Luthra wrote:
> Quoting Laurent Pinchart (2026-02-02 15:01:05)
> > On Mon, Feb 02, 2026 at 12:53:15PM +0530, Jai Luthra wrote:
> > > Quoting Laurent Pinchart (2026-01-28 18:49:03)
> > > > On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > > > > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > > > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > > > > conventions it would be BGR.
> > > > > > > > > > 
> > > > > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > > > > 
> > > > > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > > > > 
> > > > > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > > > > need to introduce one before fixing it.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > > > > ---
> > > > > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > > > > >        - b\ :sub:`4`
> > > > > > > > > >        - b\ :sub:`3`
> > > > > > > > > >        - b\ :sub:`2`
> > > > > > > > > >        - b\ :sub:`1`
> > > > > > > > > >        - b\ :sub:`0`
> > > > > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > > > > +
> > > > > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > > > > +      - 0x1028
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      - b\ :sub:`4`
> > > > > > > > > > +      - b\ :sub:`3`
> > > > > > > > > > +      - b\ :sub:`2`
> > > > > > > > > > +      - b\ :sub:`1`
> > > > > > > > > > +      - b\ :sub:`0`
> > > > > > > > > > +      - g\ :sub:`5`
> > > > > > > > > > +      - g\ :sub:`4`
> > > > > > > > > > +      - g\ :sub:`3`
> > > > > > > > > > +      - g\ :sub:`2`
> > > > > > > > > > +      - g\ :sub:`1`
> > > > > > > > > > +      - g\ :sub:`0`
> > > > > > > > > > +      - r\ :sub:`4`
> > > > > > > > > > +      - r\ :sub:`3`
> > > > > > > > > > +      - r\ :sub:`2`
> > > > > > > > > > +      - r\ :sub:`1`
> > > > > > > > > > +      - r\ :sub:`0`
> > > > > > > > > 
> > > > > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > > > > starts with bit 0, not bit 4.
> > > > > > > > > 
> > > > > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > > > > 
> > > > > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > > > > conventions would be easier to explain that others, and can also be more
> > > > > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > > > > transmission" could be the simplest to document, there will be a
> > > > > > > > mismatch in component orders between the media bus code and the pixel
> > > > > > > > format in some cases. There may also be more drivers implementing other
> > > > > > > > conventions, making the transition more difficult.
> > > > > > > > 
> > > > > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > > > > participate in this discussion in the near future.
> > > > > > > 
> > > > > > > For the record, we've discussed it on IRC recently.
> > > > > > > 
> > > > > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > > > > variants make sense to me. And we can easily document it, because we
> > > > > > > could match the first bit transmitted with the least significant bit
> > > > > > > of a media bus code indeed.
> > > > > > 
> > > > > > That's one of the things I like about it, it's consistent and easy to
> > > > > > document. Glad we agree :-)
> > > > > > 
> > > > > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > > > > That's indeed the case right now with tc358743:
> > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > > > > 
> > > > > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > > > > matches the media bus code of the other end:
> > > > > > > 
> > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > > > > 
> > > > > > > That alone makes total sense, but it has an association between
> > > > > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > > > > 
> > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > > > > 
> > > > > > > Using the convention you suggested, this association is wrong, and
> > > > > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > > > > the red and blue color components are mixed up.
> > > > > > 
> > > > > > Correct.
> > > > > > 
> > > > > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > > > > 
> > > > > > > This was shot down (rightfully) because it would still be broken.
> > > > > > > 
> > > > > > > The second version changed the media bus tc358743 reported:
> > > > > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > > > > 
> > > > > > > Dave was against it because it would potentially break userspace, citing
> > > > > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > > > > agree with his point, but having two drivers reporting the same data
> > > > > > > format but with a different meaning is also a way of breaking userspace.
> > > > > > 
> > > > > > Yes, I would find that pretty bad, possibly even worse.
> > > > > > 
> > > > > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > > > > what the second, third and fourth that you commented on worked towards.
> > > > > > > 
> > > > > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > > > > 
> > > > > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > > > > but would need to modify the association between the v4l2 format and
> > > > > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > > > > version that got shot down, and suffers from the same flaws: we could
> > > > > > > have a userspace application out there hardcoding formats and codes that
> > > > > > > will get an error.
> > > > > > > 
> > > > > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > > > > get pixels in the expected (and documented!) order when using unicam.
> > > > > > 
> > > > > > I've lost track of the status of this series and what your current
> > > > > > suggestion is. Can we standardize on
> > > > > > 
> > > > > > - Using MEDIA_BUS_FMT_RGB*
> > > > > 
> > > > > I guess we can do that.
> > > > > 
> > > > > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > > > > 
> > > > > You called "pretty bad, possibly even worse" to do the exact opposite
> > > > > (ie, change the bridge media bus to match unicam) because it would break
> > > > > userspace. Changing the unicam media bus to match the bridge creates the
> > > > > exact same situation.
> > > > > 
> > > > > The alternative would still be to report both for the bridge, and invert
> > > > > the current assocation for the v4l2 formats and mbus codes.
> > > > > 
> > > > > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > > > > >   regressions, but with a big warning
> > > > > 
> > > > > What would you improve there exactly? It's very clearly in the patches
> > > > > already, so unless you have some specific comments I'm not really sure
> > > > > what you want me to do.
> > > > 
> > > > If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> > > > driver, not in the tc358743 driver. Is it possible to implement the
> > > > backward compatibility (with a warning) in unicam instead of tc358743 ?
> > > 
> > > Not just unicam. TI's CSI driver (j721e-csi2rx.c) also uses:
> > > 
> > > V4L2_PIX_FMT_XBGR32 for MEDIA_BUS_FMT_RGB888_1X24 (and vice versa)
> > 
> > That is not necessarily wrong. The mapping between media bus codes and
> > pixel formats is device-dependent. Does the upstream j721e-csi2rx driver
> > map MEDIA_BUS_FMT_RGB888_1X24 to V4L2_PIX_FMT_XBGR32 but actually writes
> > a different pixel format to memory ?
> 
> From the TRM, MIPI RGB888 unpacks to:
> BYTE0		BYTE1		BYTE2		BYTE3		
> BBBBBBBB	GGGGGGGG	RRRRRRRR	00000000	
> 
> Which is V4L2_PIX_FMT_XBGR32 I believe.

Correct, it's V4L2_PIX_FMT_XBGR32. Where's the problem ? The mapping
between a media bus code and a pixel format is device-dependent,
j721e-csi2rx and unicam don't have to use the same mapping.

> > > (my 2 cents) It's better to have backward compatibility in drivers that
> > > currently don't follow the Media documentation. I agree the docs are
> > > confusing, they tripped me up too, but there are already userspace
> > > scripts/applications that TI / beagle users use that follow those confusing
> > > docs :)

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Jai Luthra]

Quoting Laurent Pinchart (2026-02-04 05:13:54)
> On Mon, Feb 02, 2026 at 04:21:00PM +0530, Jai Luthra wrote:
> > Quoting Laurent Pinchart (2026-02-02 15:01:05)
> > > On Mon, Feb 02, 2026 at 12:53:15PM +0530, Jai Luthra wrote:
> > > > Quoting Laurent Pinchart (2026-01-28 18:49:03)
> > > > > On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > > > > > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > > > > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > > > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > > > > > conventions it would be BGR.
> > > > > > > > > > > 
> > > > > > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > > > > > 
> > > > > > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > > > > > 
> > > > > > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > > > > > need to introduce one before fixing it.
> > > > > > > > > > > 
> > > > > > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > > > > > ---
> > > > > > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > > > > > 
> > > > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > > > > > >        - b\ :sub:`4`
> > > > > > > > > > >        - b\ :sub:`3`
> > > > > > > > > > >        - b\ :sub:`2`
> > > > > > > > > > >        - b\ :sub:`1`
> > > > > > > > > > >        - b\ :sub:`0`
> > > > > > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > > > > > +
> > > > > > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > > > > > +      - 0x1028
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      -
> > > > > > > > > > > +      - b\ :sub:`4`
> > > > > > > > > > > +      - b\ :sub:`3`
> > > > > > > > > > > +      - b\ :sub:`2`
> > > > > > > > > > > +      - b\ :sub:`1`
> > > > > > > > > > > +      - b\ :sub:`0`
> > > > > > > > > > > +      - g\ :sub:`5`
> > > > > > > > > > > +      - g\ :sub:`4`
> > > > > > > > > > > +      - g\ :sub:`3`
> > > > > > > > > > > +      - g\ :sub:`2`
> > > > > > > > > > > +      - g\ :sub:`1`
> > > > > > > > > > > +      - g\ :sub:`0`
> > > > > > > > > > > +      - r\ :sub:`4`
> > > > > > > > > > > +      - r\ :sub:`3`
> > > > > > > > > > > +      - r\ :sub:`2`
> > > > > > > > > > > +      - r\ :sub:`1`
> > > > > > > > > > > +      - r\ :sub:`0`
> > > > > > > > > > 
> > > > > > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > > > > > starts with bit 0, not bit 4.
> > > > > > > > > > 
> > > > > > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > > > > > 
> > > > > > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > > > > > conventions would be easier to explain that others, and can also be more
> > > > > > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > > > > > transmission" could be the simplest to document, there will be a
> > > > > > > > > mismatch in component orders between the media bus code and the pixel
> > > > > > > > > format in some cases. There may also be more drivers implementing other
> > > > > > > > > conventions, making the transition more difficult.
> > > > > > > > > 
> > > > > > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > > > > > participate in this discussion in the near future.
> > > > > > > > 
> > > > > > > > For the record, we've discussed it on IRC recently.
> > > > > > > > 
> > > > > > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > > > > > variants make sense to me. And we can easily document it, because we
> > > > > > > > could match the first bit transmitted with the least significant bit
> > > > > > > > of a media bus code indeed.
> > > > > > > 
> > > > > > > That's one of the things I like about it, it's consistent and easy to
> > > > > > > document. Glad we agree :-)
> > > > > > > 
> > > > > > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > > > > > That's indeed the case right now with tc358743:
> > > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > > > > > 
> > > > > > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > > > > > matches the media bus code of the other end:
> > > > > > > > 
> > > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > > > > > 
> > > > > > > > That alone makes total sense, but it has an association between
> > > > > > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > > > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > > > > > 
> > > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > > > > > 
> > > > > > > > Using the convention you suggested, this association is wrong, and
> > > > > > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > > > > > the red and blue color components are mixed up.
> > > > > > > 
> > > > > > > Correct.
> > > > > > > 
> > > > > > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > > > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > > > > > 
> > > > > > > > This was shot down (rightfully) because it would still be broken.
> > > > > > > > 
> > > > > > > > The second version changed the media bus tc358743 reported:
> > > > > > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > > > > > 
> > > > > > > > Dave was against it because it would potentially break userspace, citing
> > > > > > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > > > > > agree with his point, but having two drivers reporting the same data
> > > > > > > > format but with a different meaning is also a way of breaking userspace.
> > > > > > > 
> > > > > > > Yes, I would find that pretty bad, possibly even worse.
> > > > > > > 
> > > > > > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > > > > > what the second, third and fourth that you commented on worked towards.
> > > > > > > > 
> > > > > > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > > > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > > > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > > > > > 
> > > > > > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > > > > > but would need to modify the association between the v4l2 format and
> > > > > > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > > > > > version that got shot down, and suffers from the same flaws: we could
> > > > > > > > have a userspace application out there hardcoding formats and codes that
> > > > > > > > will get an error.
> > > > > > > > 
> > > > > > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > > > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > > > > > get pixels in the expected (and documented!) order when using unicam.
> > > > > > > 
> > > > > > > I've lost track of the status of this series and what your current
> > > > > > > suggestion is. Can we standardize on
> > > > > > > 
> > > > > > > - Using MEDIA_BUS_FMT_RGB*
> > > > > > 
> > > > > > I guess we can do that.
> > > > > > 
> > > > > > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > > > > > 
> > > > > > You called "pretty bad, possibly even worse" to do the exact opposite
> > > > > > (ie, change the bridge media bus to match unicam) because it would break
> > > > > > userspace. Changing the unicam media bus to match the bridge creates the
> > > > > > exact same situation.
> > > > > > 
> > > > > > The alternative would still be to report both for the bridge, and invert
> > > > > > the current assocation for the v4l2 formats and mbus codes.
> > > > > > 
> > > > > > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > > > > > >   regressions, but with a big warning
> > > > > > 
> > > > > > What would you improve there exactly? It's very clearly in the patches
> > > > > > already, so unless you have some specific comments I'm not really sure
> > > > > > what you want me to do.
> > > > > 
> > > > > If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> > > > > driver, not in the tc358743 driver. Is it possible to implement the
> > > > > backward compatibility (with a warning) in unicam instead of tc358743 ?
> > > > 
> > > > Not just unicam. TI's CSI driver (j721e-csi2rx.c) also uses:
> > > > 
> > > > V4L2_PIX_FMT_XBGR32 for MEDIA_BUS_FMT_RGB888_1X24 (and vice versa)
> > > 
> > > That is not necessarily wrong. The mapping between media bus codes and
> > > pixel formats is device-dependent. Does the upstream j721e-csi2rx driver
> > > map MEDIA_BUS_FMT_RGB888_1X24 to V4L2_PIX_FMT_XBGR32 but actually writes
> > > a different pixel format to memory ?
> > 
> > From the TRM, MIPI RGB888 unpacks to:
> > BYTE0         BYTE1           BYTE2           BYTE3           
> > BBBBBBBB      GGGGGGGG        RRRRRRRR        00000000        
> > 
> > Which is V4L2_PIX_FMT_XBGR32 I believe.
> 
> Correct, it's V4L2_PIX_FMT_XBGR32. Where's the problem ? The mapping
> between a media bus code and a pixel format is device-dependent,
> j721e-csi2rx and unicam don't have to use the same mapping.

Ah my bad, I misread the unicam driver, and thought you were suggesting
changing MEDIA_BUS_FMT_RGB888_1X24 to no longer map to the default bit
order of MIPI CSI2 RGB888 datatype.

Sorry for the confusion.

> 
> > > > (my 2 cents) It's better to have backward compatibility in drivers that
> > > > currently don't follow the Media documentation. I agree the docs are
> > > > confusing, they tripped me up too, but there are already userspace
> > > > scripts/applications that TI / beagle users use that follow those confusing
> > > > docs :)
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Maxime Ripard]

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

On Wed, Jan 28, 2026 at 03:19:03PM +0200, Laurent Pinchart wrote:
> On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > conventions it would be BGR.
> > > > > > > 
> > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > 
> > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > 
> > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > need to introduce one before fixing it.
> > > > > > > 
> > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > ---
> > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > >        - b\ :sub:`4`
> > > > > > >        - b\ :sub:`3`
> > > > > > >        - b\ :sub:`2`
> > > > > > >        - b\ :sub:`1`
> > > > > > >        - b\ :sub:`0`
> > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > +
> > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > +      - 0x1028
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      -
> > > > > > > +      - b\ :sub:`4`
> > > > > > > +      - b\ :sub:`3`
> > > > > > > +      - b\ :sub:`2`
> > > > > > > +      - b\ :sub:`1`
> > > > > > > +      - b\ :sub:`0`
> > > > > > > +      - g\ :sub:`5`
> > > > > > > +      - g\ :sub:`4`
> > > > > > > +      - g\ :sub:`3`
> > > > > > > +      - g\ :sub:`2`
> > > > > > > +      - g\ :sub:`1`
> > > > > > > +      - g\ :sub:`0`
> > > > > > > +      - r\ :sub:`4`
> > > > > > > +      - r\ :sub:`3`
> > > > > > > +      - r\ :sub:`2`
> > > > > > > +      - r\ :sub:`1`
> > > > > > > +      - r\ :sub:`0`
> > > > > > 
> > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > starts with bit 0, not bit 4.
> > > > > > 
> > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > 
> > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > conventions would be easier to explain that others, and can also be more
> > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > transmission" could be the simplest to document, there will be a
> > > > > mismatch in component orders between the media bus code and the pixel
> > > > > format in some cases. There may also be more drivers implementing other
> > > > > conventions, making the transition more difficult.
> > > > > 
> > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > participate in this discussion in the near future.
> > > > 
> > > > For the record, we've discussed it on IRC recently.
> > > > 
> > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > variants make sense to me. And we can easily document it, because we
> > > > could match the first bit transmitted with the least significant bit
> > > > of a media bus code indeed.
> > > 
> > > That's one of the things I like about it, it's consistent and easy to
> > > document. Glad we agree :-)
> > > 
> > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > That's indeed the case right now with tc358743:
> > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > 
> > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > matches the media bus code of the other end:
> > > > 
> > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > 
> > > > That alone makes total sense, but it has an association between
> > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > 
> > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > 
> > > > Using the convention you suggested, this association is wrong, and
> > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > the red and blue color components are mixed up.
> > > 
> > > Correct.
> > > 
> > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > 
> > > > This was shot down (rightfully) because it would still be broken.
> > > > 
> > > > The second version changed the media bus tc358743 reported:
> > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > 
> > > > Dave was against it because it would potentially break userspace, citing
> > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > agree with his point, but having two drivers reporting the same data
> > > > format but with a different meaning is also a way of breaking userspace.
> > > 
> > > Yes, I would find that pretty bad, possibly even worse.
> > > 
> > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > what the second, third and fourth that you commented on worked towards.
> > > > 
> > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > 
> > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > but would need to modify the association between the v4l2 format and
> > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > version that got shot down, and suffers from the same flaws: we could
> > > > have a userspace application out there hardcoding formats and codes that
> > > > will get an error.
> > > > 
> > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > get pixels in the expected (and documented!) order when using unicam.
> > > 
> > > I've lost track of the status of this series and what your current
> > > suggestion is. Can we standardize on
> > > 
> > > - Using MEDIA_BUS_FMT_RGB*
> > 
> > I guess we can do that.
> > 
> > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > 
> > You called "pretty bad, possibly even worse" to do the exact opposite
> > (ie, change the bridge media bus to match unicam) because it would break
> > userspace. Changing the unicam media bus to match the bridge creates the
> > exact same situation.
> > 
> > The alternative would still be to report both for the bridge, and invert
> > the current assocation for the v4l2 formats and mbus codes.
> > 
> > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > >   regressions, but with a big warning
> > 
> > What would you improve there exactly? It's very clearly in the patches
> > already, so unless you have some specific comments I'm not really sure
> > what you want me to do.
> 
> If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> driver, not in the tc358743 driver. Is it possible to implement the
> backward compatibility (with a warning) in unicam instead of tc358743 ?

I don't think we can, because the media bus code is exposed on the
links, and we would change the media bus code we require on that link.

Maxime

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

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Tue, Feb 03, 2026 at 09:52:16AM +0100, Maxime Ripard wrote:
> On Wed, Jan 28, 2026 at 03:19:03PM +0200, Laurent Pinchart wrote:
> > On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > > conventions it would be BGR.
> > > > > > > > 
> > > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > > 
> > > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > > 
> > > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > > need to introduce one before fixing it.
> > > > > > > > 
> > > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > > ---
> > > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > > 
> > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > > >        - b\ :sub:`4`
> > > > > > > >        - b\ :sub:`3`
> > > > > > > >        - b\ :sub:`2`
> > > > > > > >        - b\ :sub:`1`
> > > > > > > >        - b\ :sub:`0`
> > > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > > +
> > > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > > +      - 0x1028
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      -
> > > > > > > > +      - b\ :sub:`4`
> > > > > > > > +      - b\ :sub:`3`
> > > > > > > > +      - b\ :sub:`2`
> > > > > > > > +      - b\ :sub:`1`
> > > > > > > > +      - b\ :sub:`0`
> > > > > > > > +      - g\ :sub:`5`
> > > > > > > > +      - g\ :sub:`4`
> > > > > > > > +      - g\ :sub:`3`
> > > > > > > > +      - g\ :sub:`2`
> > > > > > > > +      - g\ :sub:`1`
> > > > > > > > +      - g\ :sub:`0`
> > > > > > > > +      - r\ :sub:`4`
> > > > > > > > +      - r\ :sub:`3`
> > > > > > > > +      - r\ :sub:`2`
> > > > > > > > +      - r\ :sub:`1`
> > > > > > > > +      - r\ :sub:`0`
> > > > > > > 
> > > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > > starts with bit 0, not bit 4.
> > > > > > > 
> > > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > > 
> > > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > > conventions would be easier to explain that others, and can also be more
> > > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > > transmission" could be the simplest to document, there will be a
> > > > > > mismatch in component orders between the media bus code and the pixel
> > > > > > format in some cases. There may also be more drivers implementing other
> > > > > > conventions, making the transition more difficult.
> > > > > > 
> > > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > > participate in this discussion in the near future.
> > > > > 
> > > > > For the record, we've discussed it on IRC recently.
> > > > > 
> > > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > > variants make sense to me. And we can easily document it, because we
> > > > > could match the first bit transmitted with the least significant bit
> > > > > of a media bus code indeed.
> > > > 
> > > > That's one of the things I like about it, it's consistent and easy to
> > > > document. Glad we agree :-)
> > > > 
> > > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > > That's indeed the case right now with tc358743:
> > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > > 
> > > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > > matches the media bus code of the other end:
> > > > > 
> > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > > 
> > > > > That alone makes total sense, but it has an association between
> > > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > > 
> > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > > 
> > > > > Using the convention you suggested, this association is wrong, and
> > > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > > the red and blue color components are mixed up.
> > > > 
> > > > Correct.
> > > > 
> > > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > > 
> > > > > This was shot down (rightfully) because it would still be broken.
> > > > > 
> > > > > The second version changed the media bus tc358743 reported:
> > > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > > 
> > > > > Dave was against it because it would potentially break userspace, citing
> > > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > > agree with his point, but having two drivers reporting the same data
> > > > > format but with a different meaning is also a way of breaking userspace.
> > > > 
> > > > Yes, I would find that pretty bad, possibly even worse.
> > > > 
> > > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > > what the second, third and fourth that you commented on worked towards.
> > > > > 
> > > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > > 
> > > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > > but would need to modify the association between the v4l2 format and
> > > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > > version that got shot down, and suffers from the same flaws: we could
> > > > > have a userspace application out there hardcoding formats and codes that
> > > > > will get an error.
> > > > > 
> > > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > > get pixels in the expected (and documented!) order when using unicam.
> > > > 
> > > > I've lost track of the status of this series and what your current
> > > > suggestion is. Can we standardize on
> > > > 
> > > > - Using MEDIA_BUS_FMT_RGB*
> > > 
> > > I guess we can do that.
> > > 
> > > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > > 
> > > You called "pretty bad, possibly even worse" to do the exact opposite
> > > (ie, change the bridge media bus to match unicam) because it would break
> > > userspace. Changing the unicam media bus to match the bridge creates the
> > > exact same situation.
> > > 
> > > The alternative would still be to report both for the bridge, and invert
> > > the current assocation for the v4l2 formats and mbus codes.
> > > 
> > > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > > >   regressions, but with a big warning
> > > 
> > > What would you improve there exactly? It's very clearly in the patches
> > > already, so unless you have some specific comments I'm not really sure
> > > what you want me to do.
> > 
> > If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> > driver, not in the tc358743 driver. Is it possible to implement the
> > backward compatibility (with a warning) in unicam instead of tc358743 ?
> 
> I don't think we can, because the media bus code is exposed on the
> links, and we would change the media bus code we require on that link.

I'm sorry but I don't get you. The tc358743 driver uses
MEDIA_BUS_FMT_RGB888_1X24. If we standardize on RGB* media bus formats,
the link between the tc358743 and unicam will keep using
MEDIA_BUS_FMT_RGB888_1X24. What link would use a different media bus
code ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Maxime Ripard]

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

On Wed, Feb 04, 2026 at 01:40:36AM +0200, Laurent Pinchart wrote:
> On Tue, Feb 03, 2026 at 09:52:16AM +0100, Maxime Ripard wrote:
> > On Wed, Jan 28, 2026 at 03:19:03PM +0200, Laurent Pinchart wrote:
> > > On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > > > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > > > conventions it would be BGR.
> > > > > > > > > 
> > > > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > > > 
> > > > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > > > 
> > > > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > > > need to introduce one before fixing it.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > > > ---
> > > > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > > > >        - b\ :sub:`4`
> > > > > > > > >        - b\ :sub:`3`
> > > > > > > > >        - b\ :sub:`2`
> > > > > > > > >        - b\ :sub:`1`
> > > > > > > > >        - b\ :sub:`0`
> > > > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > > > +
> > > > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > > > +      - 0x1028
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      -
> > > > > > > > > +      - b\ :sub:`4`
> > > > > > > > > +      - b\ :sub:`3`
> > > > > > > > > +      - b\ :sub:`2`
> > > > > > > > > +      - b\ :sub:`1`
> > > > > > > > > +      - b\ :sub:`0`
> > > > > > > > > +      - g\ :sub:`5`
> > > > > > > > > +      - g\ :sub:`4`
> > > > > > > > > +      - g\ :sub:`3`
> > > > > > > > > +      - g\ :sub:`2`
> > > > > > > > > +      - g\ :sub:`1`
> > > > > > > > > +      - g\ :sub:`0`
> > > > > > > > > +      - r\ :sub:`4`
> > > > > > > > > +      - r\ :sub:`3`
> > > > > > > > > +      - r\ :sub:`2`
> > > > > > > > > +      - r\ :sub:`1`
> > > > > > > > > +      - r\ :sub:`0`
> > > > > > > > 
> > > > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > > > starts with bit 0, not bit 4.
> > > > > > > > 
> > > > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > > > 
> > > > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > > > conventions would be easier to explain that others, and can also be more
> > > > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > > > transmission" could be the simplest to document, there will be a
> > > > > > > mismatch in component orders between the media bus code and the pixel
> > > > > > > format in some cases. There may also be more drivers implementing other
> > > > > > > conventions, making the transition more difficult.
> > > > > > > 
> > > > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > > > participate in this discussion in the near future.
> > > > > > 
> > > > > > For the record, we've discussed it on IRC recently.
> > > > > > 
> > > > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > > > variants make sense to me. And we can easily document it, because we
> > > > > > could match the first bit transmitted with the least significant bit
> > > > > > of a media bus code indeed.
> > > > > 
> > > > > That's one of the things I like about it, it's consistent and easy to
> > > > > document. Glad we agree :-)
> > > > > 
> > > > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > > > That's indeed the case right now with tc358743:
> > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > > > 
> > > > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > > > matches the media bus code of the other end:
> > > > > > 
> > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > > > 
> > > > > > That alone makes total sense, but it has an association between
> > > > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > > > 
> > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > > > 
> > > > > > Using the convention you suggested, this association is wrong, and
> > > > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > > > the red and blue color components are mixed up.
> > > > > 
> > > > > Correct.
> > > > > 
> > > > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > > > 
> > > > > > This was shot down (rightfully) because it would still be broken.
> > > > > > 
> > > > > > The second version changed the media bus tc358743 reported:
> > > > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > > > 
> > > > > > Dave was against it because it would potentially break userspace, citing
> > > > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > > > agree with his point, but having two drivers reporting the same data
> > > > > > format but with a different meaning is also a way of breaking userspace.
> > > > > 
> > > > > Yes, I would find that pretty bad, possibly even worse.
> > > > > 
> > > > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > > > what the second, third and fourth that you commented on worked towards.
> > > > > > 
> > > > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > > > 
> > > > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > > > but would need to modify the association between the v4l2 format and
> > > > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > > > version that got shot down, and suffers from the same flaws: we could
> > > > > > have a userspace application out there hardcoding formats and codes that
> > > > > > will get an error.
> > > > > > 
> > > > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > > > get pixels in the expected (and documented!) order when using unicam.
> > > > > 
> > > > > I've lost track of the status of this series and what your current
> > > > > suggestion is. Can we standardize on
> > > > > 
> > > > > - Using MEDIA_BUS_FMT_RGB*
> > > > 
> > > > I guess we can do that.
> > > > 
> > > > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > > > 
> > > > You called "pretty bad, possibly even worse" to do the exact opposite
> > > > (ie, change the bridge media bus to match unicam) because it would break
> > > > userspace. Changing the unicam media bus to match the bridge creates the
> > > > exact same situation.
> > > > 
> > > > The alternative would still be to report both for the bridge, and invert
> > > > the current assocation for the v4l2 formats and mbus codes.
> > > > 
> > > > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > > > >   regressions, but with a big warning
> > > > 
> > > > What would you improve there exactly? It's very clearly in the patches
> > > > already, so unless you have some specific comments I'm not really sure
> > > > what you want me to do.
> > > 
> > > If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> > > driver, not in the tc358743 driver. Is it possible to implement the
> > > backward compatibility (with a warning) in unicam instead of tc358743 ?
> > 
> > I don't think we can, because the media bus code is exposed on the
> > links, and we would change the media bus code we require on that link.
> 
> I'm sorry but I don't get you. The tc358743 driver uses
> MEDIA_BUS_FMT_RGB888_1X24. If we standardize on RGB* media bus formats,
> the link between the tc358743 and unicam will keep using
> MEDIA_BUS_FMT_RGB888_1X24. What link would use a different media bus
> code ?

From the summary you asked for:

> Unicam however hardcodes (and validates) that the v4l2 format codes
> matches the media bus code of the other end:
>
> https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
>
> That alone makes total sense, but it has an association between
> V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
>
> https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343

If *only* standardize on MEDIA_BUS_FMT_RGB888_1X24 for
V4L2_PIX_FMT_BGR24, then that means breaking the media link code to v4l2
format unicam has enforced for years now, and both are exposed to
userspace.

If that's not what you suggest, then please state what you actually
suggest.

Maxime

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

Re: [PATCH v4 2/4] media: uapi: Introduce MEDIA_BUS_FMT_BGR565_1X16

[Laurent Pinchart]

On Wed, Feb 04, 2026 at 10:31:03AM +0100, Maxime Ripard wrote:
> On Wed, Feb 04, 2026 at 01:40:36AM +0200, Laurent Pinchart wrote:
> > On Tue, Feb 03, 2026 at 09:52:16AM +0100, Maxime Ripard wrote:
> > > On Wed, Jan 28, 2026 at 03:19:03PM +0200, Laurent Pinchart wrote:
> > > > On Wed, Jan 28, 2026 at 01:32:15PM +0100, Maxime Ripard wrote:
> > > > > On Fri, Jan 23, 2026 at 05:34:32PM +0200, Laurent Pinchart wrote:
> > > > > > On Mon, Dec 08, 2025 at 04:32:33PM +0100, Maxime Ripard wrote:
> > > > > > > On Mon, Oct 27, 2025 at 01:33:08AM +0200, Laurent Pinchart wrote:
> > > > > > > > On Mon, Oct 27, 2025 at 01:15:54AM +0200, Laurent Pinchart wrote:
> > > > > > > > > On Mon, Oct 13, 2025 at 01:01:34PM +0200, Maxime Ripard wrote:
> > > > > > > > > > MIPI-CSI2 sends its RGB format on the wire with the blue component
> > > > > > > > > > first, then green, then red. MIPI calls that format "RGB", but by v4l2
> > > > > > > > > > conventions it would be BGR.
> > > > > > > > > > 
> > > > > > > > > > MIPI-CSI2 supports three RGB variants: 444, 555, 565, 666 and 888.
> > > > > > > > > > 
> > > > > > > > > > We already have BGR666 and BGR888 media bus formats, we don't have any
> > > > > > > > > > CSI transceivers using the 444 and 555 variants, but some transceivers
> > > > > > > > > > use the CSI RGB565 format, while using the RGB565 media bus code.
> > > > > > > > > > 
> > > > > > > > > > That's a mistake, but since we don't have a BGR565 media bus code we
> > > > > > > > > > need to introduce one before fixing it.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > > > > > > > > ---
> > > > > > > > > >  .../userspace-api/media/v4l/subdev-formats.rst     | 37 ++++++++++++++++++++++
> > > > > > > > > >  include/uapi/linux/media-bus-format.h              |  3 +-
> > > > > > > > > >  2 files changed, 39 insertions(+), 1 deletion(-)
> > > > > > > > > > 
> > > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > index 8e92f784abd8123f9ea950f954a60af56ee76dbe..def0d24ef6cdb1a2ec9395af1468f56adf31a8de 100644
> > > > > > > > > > --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst
> > > > > > > > > > @@ -625,10 +625,47 @@ The following tables list existing packed RGB formats.
> > > > > > > > > >        - b\ :sub:`4`
> > > > > > > > > >        - b\ :sub:`3`
> > > > > > > > > >        - b\ :sub:`2`
> > > > > > > > > >        - b\ :sub:`1`
> > > > > > > > > >        - b\ :sub:`0`
> > > > > > > > > > +    * .. _MEDIA-BUS-FMT-BGR565-1X16:
> > > > > > > > > > +
> > > > > > > > > > +      - MEDIA_BUS_FMT_BGR565_1X16
> > > > > > > > > > +      - 0x1028
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      -
> > > > > > > > > > +      - b\ :sub:`4`
> > > > > > > > > > +      - b\ :sub:`3`
> > > > > > > > > > +      - b\ :sub:`2`
> > > > > > > > > > +      - b\ :sub:`1`
> > > > > > > > > > +      - b\ :sub:`0`
> > > > > > > > > > +      - g\ :sub:`5`
> > > > > > > > > > +      - g\ :sub:`4`
> > > > > > > > > > +      - g\ :sub:`3`
> > > > > > > > > > +      - g\ :sub:`2`
> > > > > > > > > > +      - g\ :sub:`1`
> > > > > > > > > > +      - g\ :sub:`0`
> > > > > > > > > > +      - r\ :sub:`4`
> > > > > > > > > > +      - r\ :sub:`3`
> > > > > > > > > > +      - r\ :sub:`2`
> > > > > > > > > > +      - r\ :sub:`1`
> > > > > > > > > > +      - r\ :sub:`0`
> > > > > > > > > 
> > > > > > > > > We're definitely in convention territory, because this is not how 16-bit
> > > > > > > > > RGB data is transmitted over CSI-2. CSI-2 transmits blue first, but
> > > > > > > > > starts with bit 0, not bit 4.
> > > > > > > > > 
> > > > > > > > > Have you explored the alternative of picking the parallel bus code that
> > > > > > > > > matches the serial order when transmitted with the least significant bit
> > > > > > > > > first ? That would be MEDIA_BUS_FMT_RGB565_1X16 here, and
> > > > > > > > > MEDIA_BUS_FMT_RGB888_1X24 for 24-bit RGB.
> > > > > > > > 
> > > > > > > > To be clear, media bus codes are a matter of conventions. Some
> > > > > > > > conventions would be easier to explain that others, and can also be more
> > > > > > > > consistent with pixel format namings, but at the end of the day they're
> > > > > > > > all conventions. While saying "pick the media bus code that transmits a
> > > > > > > > pixel in one clock sample, with the bit order matching LSB-first
> > > > > > > > transmission" could be the simplest to document, there will be a
> > > > > > > > mismatch in component orders between the media bus code and the pixel
> > > > > > > > format in some cases. There may also be more drivers implementing other
> > > > > > > > conventions, making the transition more difficult.
> > > > > > > > 
> > > > > > > > I'll be very busy the upcoming week and will likely not be able to
> > > > > > > > participate in this discussion in the near future.
> > > > > > > 
> > > > > > > For the record, we've discussed it on IRC recently.
> > > > > > > 
> > > > > > > The suggestion to have all CSI Data Formats as MEDIA_BUS_FMT_RGB*_1X*
> > > > > > > variants make sense to me. And we can easily document it, because we
> > > > > > > could match the first bit transmitted with the least significant bit
> > > > > > > of a media bus code indeed.
> > > > > > 
> > > > > > That's one of the things I like about it, it's consistent and easy to
> > > > > > document. Glad we agree :-)
> > > > > > 
> > > > > > > Thus a sensor using RGB888 would register MEDIA_BUS_FMT_RGB888_1X24.
> > > > > > > That's indeed the case right now with tc358743:
> > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/i2c/tc358743.c#L1775
> > > > > > > 
> > > > > > > Unicam however hardcodes (and validates) that the v4l2 format codes
> > > > > > > matches the media bus code of the other end:
> > > > > > > 
> > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> > > > > > > 
> > > > > > > That alone makes total sense, but it has an association between
> > > > > > > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > > > > > > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> > > > > > > 
> > > > > > > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> > > > > > > 
> > > > > > > Using the convention you suggested, this association is wrong, and
> > > > > > > V4L2_PIX_FMT_BGR24 should be associated MEDIA_BUS_FMT_RGB888_1X24. Thus,
> > > > > > > the red and blue color components are mixed up.
> > > > > > 
> > > > > > Correct.
> > > > > > 
> > > > > > > I initially tried to fix it in my v1 by removing the RGB24 support
> > > > > > > https://lore.kernel.org/all/20250606-rpi-unicam-rgb-bgr-fix-v1-1-9930b963f3eb@kernel.org/
> > > > > > > 
> > > > > > > This was shot down (rightfully) because it would still be broken.
> > > > > > > 
> > > > > > > The second version changed the media bus tc358743 reported:
> > > > > > > https://lore.kernel.org/all/20250612-csi-bgr-rgb-v1-0-dc8a309118f8@kernel.org/
> > > > > > > 
> > > > > > > Dave was against it because it would potentially break userspace, citing
> > > > > > > Linus that we shouldn't break userspace ever. I understand and somewhat
> > > > > > > agree with his point, but having two drivers reporting the same data
> > > > > > > format but with a different meaning is also a way of breaking userspace.
> > > > > > 
> > > > > > Yes, I would find that pretty bad, possibly even worse.
> > > > > > 
> > > > > > > Anyway. It was then suggested to support both in the tc358743. That's
> > > > > > > what the second, third and fourth that you commented on worked towards.
> > > > > > > 
> > > > > > > https://lore.kernel.org/all/20250911-csi-bgr-rgb-v2-0-e6c6b10c1040@kernel.org/
> > > > > > > https://lore.kernel.org/all/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/
> > > > > > > https://lore.kernel.org/all/20251013-csi-bgr-rgb-v4-0-55eab2caa69f@kernel.org/
> > > > > > > 
> > > > > > > In order to implement your suggestion, I wouldn't to modify tc358743,
> > > > > > > but would need to modify the association between the v4l2 format and
> > > > > > > media bus code that unicam has. In a way, it's very similar to my first
> > > > > > > version that got shot down, and suffers from the same flaws: we could
> > > > > > > have a userspace application out there hardcoding formats and codes that
> > > > > > > will get an error.
> > > > > > > 
> > > > > > > So I'm not sure your suggestion really works, unless we reevaluate what
> > > > > > > we mean by breaking userspace. Either way, I don't care, I just want to
> > > > > > > get pixels in the expected (and documented!) order when using unicam.
> > > > > > 
> > > > > > I've lost track of the status of this series and what your current
> > > > > > suggestion is. Can we standardize on
> > > > > > 
> > > > > > - Using MEDIA_BUS_FMT_RGB*
> > > > > 
> > > > > I guess we can do that.
> > > > > 
> > > > > > - Produce V4L2_PIX_FMT_BGR24 from MEDIA_BUS_FMT_RGB888_1X24 in unicam
> > > > > 
> > > > > You called "pretty bad, possibly even worse" to do the exact opposite
> > > > > (ie, change the bridge media bus to match unicam) because it would break
> > > > > userspace. Changing the unicam media bus to match the bridge creates the
> > > > > exact same situation.
> > > > > 
> > > > > The alternative would still be to report both for the bridge, and invert
> > > > > the current assocation for the v4l2 formats and mbus codes.
> > > > > 
> > > > > > - Possibly implement backward compatibility somewhere (where ?) to avoid
> > > > > >   regressions, but with a big warning
> > > > > 
> > > > > What would you improve there exactly? It's very clearly in the patches
> > > > > already, so unless you have some specific comments I'm not really sure
> > > > > what you want me to do.
> > > > 
> > > > If we standardize on MEDIA_BUS_FMT_RGB*, then the issue is in the unicam
> > > > driver, not in the tc358743 driver. Is it possible to implement the
> > > > backward compatibility (with a warning) in unicam instead of tc358743 ?
> > > 
> > > I don't think we can, because the media bus code is exposed on the
> > > links, and we would change the media bus code we require on that link.
> > 
> > I'm sorry but I don't get you. The tc358743 driver uses
> > MEDIA_BUS_FMT_RGB888_1X24. If we standardize on RGB* media bus formats,
> > the link between the tc358743 and unicam will keep using
> > MEDIA_BUS_FMT_RGB888_1X24. What link would use a different media bus
> > code ?
> 
> From the summary you asked for:
> 
> > Unicam however hardcodes (and validates) that the v4l2 format codes
> > matches the media bus code of the other end:
> >
> > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L1333
> >
> > That alone makes total sense, but it has an association between
> > V4l2_PIX_FMT_RGB24 and MEDIA_BUS_FMT_RGB888_1X24, and between
> > V4L2_PIX_FMT_BGR24 and MEDIA_BUS_FMT_BGR888_1X24
> >
> > https://elixir.bootlin.com/linux/v6.18/source/drivers/media/platform/broadcom/bcm2835-unicam.c#L343
> 
> If *only* standardize on MEDIA_BUS_FMT_RGB888_1X24 for
> V4L2_PIX_FMT_BGR24, then that means breaking the media link code to v4l2
> format unicam has enforced for years now, and both are exposed to
> userspace.
> 
> If that's not what you suggest, then please state what you actually
> suggest.

What I was thinking is accepting any combination of
{ V4l2_PIX_FMT_RGB24, V4L2_PIX_FMT_BGR24 } and
{ MEDIA_BUS_FMT_RGB888_1X24, MEDIA_BUS_FMT_BGR888_1X24 } in unicam. The
tc358743 driver would continue using MEDIA_BUS_FMT_RGB888_1X24, and
unicam will accept both V4l2_PIX_FMT_RGB24 (currently used, incorrect)
and V4L2_PIX_FMT_BGR24 (newly supported, correct). The incorrect pixel
format would generate a warning but would still work.

-- 
Regards,

Laurent Pinchart

[PATCH v4 3/4] media: tc358743: Fix the RGB MBUS format

[Maxime Ripard]

The tc358743 is an HDMI to MIPI-CSI2 bridge. It can output all three
HDMI 1.4 video formats: RGB 4:4:4, YCbCr 4:2:2, and YCbCr 4:4:4.

RGB 4:4:4 is converted to the MIPI-CSI2 RGB888 video format, and listed
in the driver as MEDIA_BUS_FMT_RGB888_1X24.

Most CSI2 receiver drivers then map MEDIA_BUS_FMT_RGB888_1X24 to
V4L2_PIX_FMT_RGB24.

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.

This essentially means that the R and B will be swapped compared to what
V4L2_PIX_FMT_RGB24 defines.

The proper MBUS format would be BGR888, so let's use that.

Fixes: d32d98642de6 ("[media] Driver for Toshiba TC358743 HDMI to CSI-2 bridge")
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/media/i2c/tc358743.c | 53 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a0ca19359c43145988535d7816012ef607555b87..feb089d8de724dd25801a0fb621c768542e05254 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -754,10 +754,11 @@ static void tc358743_set_ref_clk(struct v4l2_subdev *sd)
 }
 
 static void tc358743_set_csi_color_space(struct v4l2_subdev *sd)
 {
 	struct tc358743_state *state = to_state(sd);
+	struct device *dev = &state->i2c_client->dev;
 
 	switch (state->mbus_fmt_code) {
 	case MEDIA_BUS_FMT_UYVY8_1X16:
 		v4l2_dbg(2, debug, sd, "%s: YCbCr 422 16-bit\n", __func__);
 		i2c_wr8_and_or(sd, VOUT_SET2,
@@ -769,11 +770,21 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd)
 		i2c_wr16_and_or(sd, CONFCTL, ~MASK_YCBCRFMT,
 				MASK_YCBCRFMT_422_8_BIT);
 		mutex_unlock(&state->confctl_mutex);
 		break;
 	case MEDIA_BUS_FMT_RGB888_1X24:
-		v4l2_dbg(2, debug, sd, "%s: RGB 888 24-bit\n", __func__);
+		/*
+		 * The driver was initially introduced with RGB888
+		 * support, but CSI really means BGR.
+		 *
+		 * Since we might have applications that would have
+		 * hard-coded the RGB888, let's support both.
+		 */
+		dev_warn_once(dev, "RGB format isn't actually supported by the hardware. The application should be fixed to use BGR.");
+		fallthrough;
+	case MEDIA_BUS_FMT_BGR888_1X24:
+		v4l2_dbg(2, debug, sd, "%s: BGR 888 24-bit\n", __func__);
 		i2c_wr8_and_or(sd, VOUT_SET2,
 				~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff,
 				0x00);
 		i2c_wr8_and_or(sd, VI_REP, ~MASK_VOUT_COLOR_SEL & 0xff,
 				MASK_VOUT_COLOR_RGB_FULL);
@@ -1430,15 +1441,30 @@ static int tc358743_log_status(struct v4l2_subdev *sd)
 			(i2c_rd16(sd, CSI_STATUS) & MASK_S_RXACT) ?
 			"yes" : "no");
 	v4l2_info(sd, "Stopped: %s\n",
 			(i2c_rd16(sd, CSI_STATUS) & MASK_S_HLT) ?
 			"yes" : "no");
-	v4l2_info(sd, "Color space: %s\n",
-			state->mbus_fmt_code == MEDIA_BUS_FMT_UYVY8_1X16 ?
-			"YCbCr 422 16-bit" :
-			state->mbus_fmt_code == MEDIA_BUS_FMT_RGB888_1X24 ?
-			"RGB 888 24-bit" : "Unsupported");
+
+	switch (state->mbus_fmt_code) {
+	case MEDIA_BUS_FMT_BGR888_1X24:
+		/*
+		 * The driver was initially introduced with RGB888
+		 * support, but CSI really means BGR.
+		 *
+		 * Since we might have applications that would have
+		 * hard-coded the RGB888, let's support both.
+		 */
+	case MEDIA_BUS_FMT_RGB888_1X24:
+		v4l2_info(sd, "Color space: BGR 888 24-bit\n");
+		break;
+	case MEDIA_BUS_FMT_UYVY8_1X16:
+		v4l2_info(sd, "Color space: YCbCr 422 16-bit\n");
+		break;
+	default:
+		v4l2_info(sd, "Color space: Unsupported\n");
+		break;
+	}
 
 	v4l2_info(sd, "-----%s status-----\n", is_hdmi(sd) ? "HDMI" : "DVI-D");
 	v4l2_info(sd, "HDCP encrypted content: %s\n",
 			hdmi_sys_status & MASK_S_HDCP ? "yes" : "no");
 	v4l2_info(sd, "Input color space: %s %s range\n",
@@ -1771,24 +1797,32 @@ static int tc358743_enum_mbus_code(struct v4l2_subdev *sd,
 		struct v4l2_subdev_state *sd_state,
 		struct v4l2_subdev_mbus_code_enum *code)
 {
 	switch (code->index) {
 	case 0:
-		code->code = MEDIA_BUS_FMT_RGB888_1X24;
+		code->code = MEDIA_BUS_FMT_BGR888_1X24;
 		break;
 	case 1:
 		code->code = MEDIA_BUS_FMT_UYVY8_1X16;
 		break;
+	case 2:
+		/*
+		 * We need to keep RGB888 for backward compatibility,
+		 * but we should list it last for userspace to pick BGR.
+		 */
+		code->code = MEDIA_BUS_FMT_RGB888_1X24;
+		break;
 	default:
 		return -EINVAL;
 	}
 	return 0;
 }
 
 static u32 tc358743_g_colorspace(u32 code)
 {
 	switch (code) {
+	case MEDIA_BUS_FMT_BGR888_1X24:
 	case MEDIA_BUS_FMT_RGB888_1X24:
 		return V4L2_COLORSPACE_SRGB;
 	case MEDIA_BUS_FMT_UYVY8_1X16:
 		return V4L2_COLORSPACE_SMPTE170M;
 	default:
@@ -1822,11 +1856,12 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd,
 	struct tc358743_state *state = to_state(sd);
 
 	u32 code = format->format.code; /* is overwritten by get_fmt */
 	int ret = tc358743_get_fmt(sd, sd_state, format);
 
-	if (code == MEDIA_BUS_FMT_RGB888_1X24 ||
+	if (code == MEDIA_BUS_FMT_BGR888_1X24 ||
+	    code == MEDIA_BUS_FMT_RGB888_1X24 ||
 	    code == MEDIA_BUS_FMT_UYVY8_1X16)
 		format->format.code = code;
 	format->format.colorspace = tc358743_g_colorspace(format->format.code);
 
 	if (ret)
@@ -2242,11 +2277,11 @@ static int tc358743_probe(struct i2c_client *client)
 	sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
 	err = media_entity_pads_init(&sd->entity, 1, &state->pad);
 	if (err < 0)
 		goto err_hdl;
 
-	state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24;
+	state->mbus_fmt_code = MEDIA_BUS_FMT_BGR888_1X24;
 
 	sd->dev = &client->dev;
 
 	mutex_init(&state->confctl_mutex);
 

-- 
2.51.0

Re: [PATCH v4 3/4] media: tc358743: Fix the RGB MBUS format

[Dave Stevenson]

On Mon, 13 Oct 2025 at 12:01, Maxime Ripard <mripard@kernel.org> wrote:
>
> The tc358743 is an HDMI to MIPI-CSI2 bridge. It can output all three
> HDMI 1.4 video formats: RGB 4:4:4, YCbCr 4:2:2, and YCbCr 4:4:4.
>
> RGB 4:4:4 is converted to the MIPI-CSI2 RGB888 video format, and listed
> in the driver as MEDIA_BUS_FMT_RGB888_1X24.
>
> Most CSI2 receiver drivers then map MEDIA_BUS_FMT_RGB888_1X24 to
> V4L2_PIX_FMT_RGB24.
>
> 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.
>
> This essentially means that the R and B will be swapped compared to what
> V4L2_PIX_FMT_RGB24 defines.
>
> The proper MBUS format would be BGR888, so let's use that.
>
> Fixes: d32d98642de6 ("[media] Driver for Toshiba TC358743 HDMI to CSI-2 bridge")
> Signed-off-by: Maxime Ripard <mripard@kernel.org>

Apologies, I thought I'd already sent:

Tested-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

Patches 1 and 2 look fine, but I'm not sure it's in my scope to give
an R-b. I can do if it's worth it.
Patch 4 also looks good as it copies this pattern, but I have no
knowledge of that sensor.

  Dave

> ---
>  drivers/media/i2c/tc358743.c | 53 ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index a0ca19359c43145988535d7816012ef607555b87..feb089d8de724dd25801a0fb621c768542e05254 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -754,10 +754,11 @@ static void tc358743_set_ref_clk(struct v4l2_subdev *sd)
>  }
>
>  static void tc358743_set_csi_color_space(struct v4l2_subdev *sd)
>  {
>         struct tc358743_state *state = to_state(sd);
> +       struct device *dev = &state->i2c_client->dev;
>
>         switch (state->mbus_fmt_code) {
>         case MEDIA_BUS_FMT_UYVY8_1X16:
>                 v4l2_dbg(2, debug, sd, "%s: YCbCr 422 16-bit\n", __func__);
>                 i2c_wr8_and_or(sd, VOUT_SET2,
> @@ -769,11 +770,21 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd)
>                 i2c_wr16_and_or(sd, CONFCTL, ~MASK_YCBCRFMT,
>                                 MASK_YCBCRFMT_422_8_BIT);
>                 mutex_unlock(&state->confctl_mutex);
>                 break;
>         case MEDIA_BUS_FMT_RGB888_1X24:
> -               v4l2_dbg(2, debug, sd, "%s: RGB 888 24-bit\n", __func__);
> +               /*
> +                * The driver was initially introduced with RGB888
> +                * support, but CSI really means BGR.
> +                *
> +                * Since we might have applications that would have
> +                * hard-coded the RGB888, let's support both.
> +                */
> +               dev_warn_once(dev, "RGB format isn't actually supported by the hardware. The application should be fixed to use BGR.");
> +               fallthrough;
> +       case MEDIA_BUS_FMT_BGR888_1X24:
> +               v4l2_dbg(2, debug, sd, "%s: BGR 888 24-bit\n", __func__);
>                 i2c_wr8_and_or(sd, VOUT_SET2,
>                                 ~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff,
>                                 0x00);
>                 i2c_wr8_and_or(sd, VI_REP, ~MASK_VOUT_COLOR_SEL & 0xff,
>                                 MASK_VOUT_COLOR_RGB_FULL);
> @@ -1430,15 +1441,30 @@ static int tc358743_log_status(struct v4l2_subdev *sd)
>                         (i2c_rd16(sd, CSI_STATUS) & MASK_S_RXACT) ?
>                         "yes" : "no");
>         v4l2_info(sd, "Stopped: %s\n",
>                         (i2c_rd16(sd, CSI_STATUS) & MASK_S_HLT) ?
>                         "yes" : "no");
> -       v4l2_info(sd, "Color space: %s\n",
> -                       state->mbus_fmt_code == MEDIA_BUS_FMT_UYVY8_1X16 ?
> -                       "YCbCr 422 16-bit" :
> -                       state->mbus_fmt_code == MEDIA_BUS_FMT_RGB888_1X24 ?
> -                       "RGB 888 24-bit" : "Unsupported");
> +
> +       switch (state->mbus_fmt_code) {
> +       case MEDIA_BUS_FMT_BGR888_1X24:
> +               /*
> +                * The driver was initially introduced with RGB888
> +                * support, but CSI really means BGR.
> +                *
> +                * Since we might have applications that would have
> +                * hard-coded the RGB888, let's support both.
> +                */
> +       case MEDIA_BUS_FMT_RGB888_1X24:
> +               v4l2_info(sd, "Color space: BGR 888 24-bit\n");
> +               break;
> +       case MEDIA_BUS_FMT_UYVY8_1X16:
> +               v4l2_info(sd, "Color space: YCbCr 422 16-bit\n");
> +               break;
> +       default:
> +               v4l2_info(sd, "Color space: Unsupported\n");
> +               break;
> +       }
>
>         v4l2_info(sd, "-----%s status-----\n", is_hdmi(sd) ? "HDMI" : "DVI-D");
>         v4l2_info(sd, "HDCP encrypted content: %s\n",
>                         hdmi_sys_status & MASK_S_HDCP ? "yes" : "no");
>         v4l2_info(sd, "Input color space: %s %s range\n",
> @@ -1771,24 +1797,32 @@ static int tc358743_enum_mbus_code(struct v4l2_subdev *sd,
>                 struct v4l2_subdev_state *sd_state,
>                 struct v4l2_subdev_mbus_code_enum *code)
>  {
>         switch (code->index) {
>         case 0:
> -               code->code = MEDIA_BUS_FMT_RGB888_1X24;
> +               code->code = MEDIA_BUS_FMT_BGR888_1X24;
>                 break;
>         case 1:
>                 code->code = MEDIA_BUS_FMT_UYVY8_1X16;
>                 break;
> +       case 2:
> +               /*
> +                * We need to keep RGB888 for backward compatibility,
> +                * but we should list it last for userspace to pick BGR.
> +                */
> +               code->code = MEDIA_BUS_FMT_RGB888_1X24;
> +               break;
>         default:
>                 return -EINVAL;
>         }
>         return 0;
>  }
>
>  static u32 tc358743_g_colorspace(u32 code)
>  {
>         switch (code) {
> +       case MEDIA_BUS_FMT_BGR888_1X24:
>         case MEDIA_BUS_FMT_RGB888_1X24:
>                 return V4L2_COLORSPACE_SRGB;
>         case MEDIA_BUS_FMT_UYVY8_1X16:
>                 return V4L2_COLORSPACE_SMPTE170M;
>         default:
> @@ -1822,11 +1856,12 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd,
>         struct tc358743_state *state = to_state(sd);
>
>         u32 code = format->format.code; /* is overwritten by get_fmt */
>         int ret = tc358743_get_fmt(sd, sd_state, format);
>
> -       if (code == MEDIA_BUS_FMT_RGB888_1X24 ||
> +       if (code == MEDIA_BUS_FMT_BGR888_1X24 ||
> +           code == MEDIA_BUS_FMT_RGB888_1X24 ||
>             code == MEDIA_BUS_FMT_UYVY8_1X16)
>                 format->format.code = code;
>         format->format.colorspace = tc358743_g_colorspace(format->format.code);
>
>         if (ret)
> @@ -2242,11 +2277,11 @@ static int tc358743_probe(struct i2c_client *client)
>         sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
>         err = media_entity_pads_init(&sd->entity, 1, &state->pad);
>         if (err < 0)
>                 goto err_hdl;
>
> -       state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24;
> +       state->mbus_fmt_code = MEDIA_BUS_FMT_BGR888_1X24;
>
>         sd->dev = &client->dev;
>
>         mutex_init(&state->confctl_mutex);
>
>
> --
> 2.51.0
>

[PATCH v4 4/4] media: gc2145: Fix the RGB MBUS format

[Maxime Ripard]

The GalaxyCore GC2145 is an MIPI-CSI2 sensor. Among others, it support
the MIPI-CSI2 RGB565 format, listed in the driver as
MEDIA_BUS_FMT_RGB565_1X16.

Most CSI2 receiver drivers then map MEDIA_BUS_FMT_RGB565_1X16 to
V4L2_PIX_FMT_RGB565.

However, V4L2_PIX_FMT_RGB565 is defined as having its color components
in the R, G and B order, from left to right. MIPI-CSI2 however defines
the RGB565 format with blue first.

This essentially means that the R and B will be swapped compared to what
V4L2_PIX_FMT_RGB565 defines.

The proper MBUS format would be BGR565, so let's use that.

Fixes: 03cc7fefbb09 ("media: i2c: gc2145: Galaxy Core GC2145 sensor support")
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/media/i2c/gc2145.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/gc2145.c b/drivers/media/i2c/gc2145.c
index b215963a2648b7423fc0ca42b300b6c586d2b994..3c179ccf19cd7ed016699d4de4eb81271c1e11ab 100644
--- a/drivers/media/i2c/gc2145.c
+++ b/drivers/media/i2c/gc2145.c
@@ -579,11 +579,11 @@ static const struct gc2145_format supported_formats[] = {
 		.colorspace	= V4L2_COLORSPACE_SRGB,
 		.datatype	= MIPI_CSI2_DT_YUV422_8B,
 		.output_fmt	= 0x03,
 	},
 	{
-		.code		= MEDIA_BUS_FMT_RGB565_1X16,
+		.code		= MEDIA_BUS_FMT_BGR565_1X16,
 		.colorspace	= V4L2_COLORSPACE_SRGB,
 		.datatype	= MIPI_CSI2_DT_RGB565,
 		.output_fmt	= 0x06,
 		.switch_bit	= true,
 	},
@@ -613,10 +613,25 @@ static const struct gc2145_format supported_formats[] = {
 		.colorspace     = V4L2_COLORSPACE_RAW,
 		.datatype       = MIPI_CSI2_DT_RAW8,
 		.output_fmt     = 0x17,
 		.row_col_switch = GC2145_SYNC_MODE_ROW_SWITCH,
 	},
+	{
+	/*
+	 * The driver was initially introduced with RGB565 support, but
+	 * CSI really means BGR.
+	 *
+	 * Since we might have applications that would have hard-coded
+	 * the RGB565, let's support both, with RGB being last to make
+	 * sure it's only a last resort.
+	 */
+		.code		= MEDIA_BUS_FMT_RGB565_1X16,
+		.colorspace	= V4L2_COLORSPACE_SRGB,
+		.datatype	= MIPI_CSI2_DT_RGB565,
+		.output_fmt	= 0x06,
+		.switch_bit	= true,
+	},
 };
 
 struct gc2145_ctrls {
 	struct v4l2_ctrl_handler handler;
 	struct v4l2_ctrl *pixel_rate;
@@ -658,12 +673,17 @@ static inline struct v4l2_subdev *gc2145_ctrl_to_sd(struct v4l2_ctrl *ctrl)
 }
 
 static const struct gc2145_format *
 gc2145_get_format_code(struct gc2145 *gc2145, u32 code)
 {
+	struct i2c_client *client = v4l2_get_subdevdata(&gc2145->sd);
 	unsigned int i;
 
+	if (code == MEDIA_BUS_FMT_RGB565_1X16)
+		dev_warn_once(&client->dev,
+			      "RGB format isn't actually supported by the hardware. The application should be fixed to use BGR.");
+
 	for (i = 0; i < ARRAY_SIZE(supported_formats); i++) {
 		if (supported_formats[i].code == code)
 			break;
 	}
 
@@ -696,11 +716,11 @@ static int gc2145_init_state(struct v4l2_subdev *sd,
 	struct v4l2_rect *crop;
 
 	/* Initialize pad format */
 	format = v4l2_subdev_state_get_format(state, 0);
 	gc2145_update_pad_format(gc2145, &supported_modes[0], format,
-				 MEDIA_BUS_FMT_RGB565_1X16,
+				 MEDIA_BUS_FMT_BGR565_1X16,
 				 V4L2_COLORSPACE_SRGB);
 
 	/* Initialize crop rectangle. */
 	crop = v4l2_subdev_state_get_crop(state, 0);
 	*crop = supported_modes[0].crop;

-- 
2.51.0