From 7c49e176ef1bbf70f20f7e3497d63f4c700e4758 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Wed, 17 Sep 2025 13:16:11 +0200 Subject: [PATCH 1/2] media: uapi: Clarify MBUS color component order for serial buses Upstream series https://lore.kernel.org/linux-media/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/ The subdev format documentation has a subsection describing how to use the media bus pixel codes for serial buses. While it describes the sampling part well, it doesn't really describe the current convention used for the components order. Let's improve that. Signed-off-by: Maxime Ripard --- .../userspace-api/media/v4l/subdev-formats.rst | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Documentation/userspace-api/media/v4l/subdev-formats.rst b/Documentation/userspace-api/media/v4l/subdev-formats.rst index 7ae8f4526e5e92..a179b428ebbbc4 100644 --- a/Documentation/userspace-api/media/v4l/subdev-formats.rst +++ b/Documentation/userspace-api/media/v4l/subdev-formats.rst @@ -160,12 +160,14 @@ correspondence between them. The media bus pixel codes document parallel formats. Should the pixel data be transported over a serial bus, the media bus pixel code that describes a -parallel format that transfers a sample on a single clock cycle is used. For -instance, both MEDIA_BUS_FMT_BGR888_1X24 and MEDIA_BUS_FMT_BGR888_3X8 are used -on parallel busses for transferring an 8 bits per sample BGR data, whereas on -serial busses the data in this format is only referred to using -MEDIA_BUS_FMT_BGR888_1X24. This is because there is effectively only a single -way to transport that format on the serial busses. +parallel format that transfers a sample on a single clock cycle is used. The +color component order used is the same used on the serial bus. For instance, +both MEDIA_BUS_FMT_BGR888_1X24 and MEDIA_BUS_FMT_BGR888_3X8 are used on parallel +busses for transferring an 8 bits per sample BGR data, whereas on serial busses +the data in this format is only referred to using MEDIA_BUS_FMT_BGR888_1X24, +with BGR meaning that the blue component is transmitted first, then green, then +red. This is because there is effectively only a single way to transport that +format on the serial busses. Packed RGB Formats ^^^^^^^^^^^^^^^^^^ From 26797815fd747fc7427fac24ba4e2cb7f113a055 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Wed, 17 Sep 2025 13:16:13 +0200 Subject: [PATCH 2/2] media: tc358743: Fix the RGB MBUS format Upstream series https://lore.kernel.org/linux-media/20250917-csi-bgr-rgb-v3-0-0145571b3aa4@kernel.org/ The tc358743 is an HDMI to MIPI-CSI2 bridge. It can output all three HDMI 1.4 video formats: RGB 4:4:4, YCbCr 4:2:2, and YCbCr 4:4:4. RGB 4:4:4 is converted to the MIPI-CSI2 RGB888 video format, and listed in the driver as MEDIA_BUS_FMT_RGB888_1X24. Most CSI2 receiver drivers then map MEDIA_BUS_FMT_RGB888_1X24 to V4L2_PIX_FMT_RGB24. However, V4L2_PIX_FMT_RGB24 is defined as having its color components in the R, G and B order, from left to right. MIPI-CSI2 however defines the RGB888 format with blue first. This essentially means that the R and B will be swapped compared to what V4L2_PIX_FMT_RGB24 defines. The proper MBUS format would be BGR888, so let's use that. Fixes: d32d98642de6 ("[media] Driver for Toshiba TC358743 HDMI to CSI-2 bridge") Signed-off-by: Maxime Ripard --- drivers/media/i2c/tc358743.c | 53 ++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index 1c73b00d82b6b8..d429917a95e8c6 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -649,6 +649,7 @@ static void tc358743_set_ref_clk(struct v4l2_subdev *sd) static void tc358743_set_csi_color_space(struct v4l2_subdev *sd) { struct tc358743_state *state = to_state(sd); + struct device *dev = &state->i2c_client->dev; switch (state->mbus_fmt_code) { case MEDIA_BUS_FMT_UYVY8_1X16: @@ -664,7 +665,17 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd) mutex_unlock(&state->confctl_mutex); break; case MEDIA_BUS_FMT_RGB888_1X24: - v4l2_dbg(2, debug, sd, "%s: RGB 888 24-bit\n", __func__); + /* + * The driver was initially introduced with RGB888 + * support, but CSI really means BGR. + * + * Since we might have applications that would have + * hard-coded the RGB888, let's support both. + */ + dev_warn_once(dev, "RGB format isn't actually supported by the hardware. The application should be fixed to use BGR."); + fallthrough; + case MEDIA_BUS_FMT_BGR888_1X24: + v4l2_dbg(2, debug, sd, "%s: BGR 888 24-bit\n", __func__); i2c_wr8_and_or(sd, VOUT_SET2, ~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff, 0x00); @@ -1325,11 +1336,26 @@ static int tc358743_log_status(struct v4l2_subdev *sd) v4l2_info(sd, "Stopped: %s\n", (i2c_rd16(sd, CSI_STATUS) & MASK_S_HLT) ? "yes" : "no"); - v4l2_info(sd, "Color space: %s\n", - state->mbus_fmt_code == MEDIA_BUS_FMT_UYVY8_1X16 ? - "YCbCr 422 16-bit" : - state->mbus_fmt_code == MEDIA_BUS_FMT_RGB888_1X24 ? - "RGB 888 24-bit" : "Unsupported"); + + switch (state->mbus_fmt_code) { + case MEDIA_BUS_FMT_BGR888_1X24: + /* + * The driver was initially introduced with RGB888 + * support, but CSI really means BGR. + * + * Since we might have applications that would have + * hard-coded the RGB888, let's support both. + */ + case MEDIA_BUS_FMT_RGB888_1X24: + v4l2_info(sd, "Color space: BGR 888 24-bit\n"); + break; + case MEDIA_BUS_FMT_UYVY8_1X16: + v4l2_info(sd, "Color space: YCbCr 422 16-bit\n"); + break; + default: + v4l2_info(sd, "Color space: Unsupported\n"); + break; + } v4l2_info(sd, "-----%s status-----\n", is_hdmi(sd) ? "HDMI" : "DVI-D"); v4l2_info(sd, "HDCP encrypted content: %s\n", @@ -1666,11 +1692,18 @@ static int tc358743_enum_mbus_code(struct v4l2_subdev *sd, { switch (code->index) { case 0: - code->code = MEDIA_BUS_FMT_RGB888_1X24; + code->code = MEDIA_BUS_FMT_BGR888_1X24; break; case 1: code->code = MEDIA_BUS_FMT_UYVY8_1X16; break; + case 2: + /* + * We need to keep RGB888 for backward compatibility, + * but we should list it last for userspace to pick BGR. + */ + code->code = MEDIA_BUS_FMT_RGB888_1X24; + break; default: return -EINVAL; } @@ -1680,6 +1713,7 @@ static int tc358743_enum_mbus_code(struct v4l2_subdev *sd, static u32 tc358743_g_colorspace(u32 code) { switch (code) { + case MEDIA_BUS_FMT_BGR888_1X24: case MEDIA_BUS_FMT_RGB888_1X24: return V4L2_COLORSPACE_SRGB; case MEDIA_BUS_FMT_UYVY8_1X16: @@ -1717,7 +1751,8 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd, u32 code = format->format.code; /* is overwritten by get_fmt */ int ret = tc358743_get_fmt(sd, sd_state, format); - if (code == MEDIA_BUS_FMT_RGB888_1X24 || + if (code == MEDIA_BUS_FMT_BGR888_1X24 || + code == MEDIA_BUS_FMT_RGB888_1X24 || code == MEDIA_BUS_FMT_UYVY8_1X16) format->format.code = code; format->format.colorspace = tc358743_g_colorspace(format->format.code); @@ -2137,7 +2172,7 @@ static int tc358743_probe(struct i2c_client *client) if (err < 0) goto err_hdl; - state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24; + state->mbus_fmt_code = MEDIA_BUS_FMT_BGR888_1X24; sd->dev = &client->dev;