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.0On 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
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
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
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
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
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
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
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
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
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
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
>
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
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
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
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
© 2016 - 2026 Red Hat, Inc.