| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20250911-csi-bgr-rgb-v2-3-e6c6b10c1040@kernel.org>
Date: Thu, 11 Sep 2025 11:15:34 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Mauro Carvalho Chehab <mchehab@...nel.org>,
Mats Randgaard <matrandg@...co.com>,
Alain Volmat <alain.volmat@...s.st.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Hans Verkuil <hverkuil@...nel.org>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
Hans Verkuil <hans.verkuil@...co.com>,
Dave Stevenson <dave.stevenson@...pberrypi.com>,
Maxime Ripard <mripard@...nel.org>
Subject: [PATCH v2 3/4] media: tc358743: Fix the RGB MBUS format
The tc358743 is an HDMI to MIPI-CSI2 bridge. It supports two of the
three HDMI 1.4 video formats: RGB 4:4:4 and YCbCr 422.
RGB 4:4:4 is converted to the MIPI-CSI2 RGB888 video format, and listed
in the driver as MEDIA_BUS_FMT_RGB888_1X24.
Most CSI2 receiver drivers then map MEDIA_BUS_FMT_RGB888_1X24 to
V4L2_PIX_FMT_RGB24.
However, V4L2_PIX_FMT_RGB24 is defined as having its color components in
the R, G and B order, from left to right. MIPI-CSI2 however defines the
RGB888 format with blue first.
This essentially means that the R and B will be swapped compared to what
V4L2_PIX_FMT_RGB24 defines.
The proper MBUS format would be BGR888, so let's use that.
Fixes: d32d98642de6 ("[media] Driver for Toshiba TC358743 HDMI to CSI-2 bridge")
Signed-off-by: Maxime Ripard <mripard@...nel.org>
---
drivers/media/i2c/tc358743.c | 51 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 42 insertions(+), 9 deletions(-)
diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index 1cc7636e446d77d7c6266ca86517496261d2b039..a3054cd823da4cf3db24c6b2bfbf0fcaa43f2814 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -693,11 +693,21 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd)
i2c_wr16_and_or(sd, CONFCTL, ~MASK_YCBCRFMT,
MASK_YCBCRFMT_422_8_BIT);
mutex_unlock(&state->confctl_mutex);
break;
case MEDIA_BUS_FMT_RGB888_1X24:
- v4l2_dbg(2, debug, sd, "%s: RGB 888 24-bit\n", __func__);
+ /*
+ * The driver was initially introduced with RGB888
+ * support, but CSI really means BGR.
+ *
+ * Since we might have applications that would have
+ * hard-coded the RGB888, let's support both.
+ */
+ v4l2_warn(sd, "RGB format isn't actually supported by the hardware. The application should be fixed to use BGR.");
+ fallthrough;
+ case MEDIA_BUS_FMT_BGR888_1X24:
+ v4l2_dbg(2, debug, sd, "%s: BGR 888 24-bit\n", __func__);
i2c_wr8_and_or(sd, VOUT_SET2,
~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff,
0x00);
i2c_wr8_and_or(sd, VI_REP, ~MASK_VOUT_COLOR_SEL & 0xff,
MASK_VOUT_COLOR_RGB_FULL);
@@ -1354,15 +1364,28 @@ static int tc358743_log_status(struct v4l2_subdev *sd)
(i2c_rd16(sd, CSI_STATUS) & MASK_S_RXACT) ?
"yes" : "no");
v4l2_info(sd, "Stopped: %s\n",
(i2c_rd16(sd, CSI_STATUS) & MASK_S_HLT) ?
"yes" : "no");
- v4l2_info(sd, "Color space: %s\n",
- state->mbus_fmt_code == MEDIA_BUS_FMT_UYVY8_1X16 ?
- "YCbCr 422 16-bit" :
- state->mbus_fmt_code == MEDIA_BUS_FMT_RGB888_1X24 ?
- "RGB 888 24-bit" : "Unsupported");
+
+ switch (state->mbus_fmt_code) {
+ case MEDIA_BUS_FMT_BGR888_1X24:
+ /*
+ * The driver was initially introduced with RGB888
+ * support, but CSI really means BGR.
+ *
+ * Since we might have applications that would have
+ * hard-coded the RGB888, let's support both.
+ */
+ fallthrough;
+ 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;
+ }
v4l2_info(sd, "-----%s status-----\n", is_hdmi(sd) ? "HDMI" : "DVI-D");
v4l2_info(sd, "HDCP encrypted content: %s\n",
hdmi_sys_status & MASK_S_HDCP ? "yes" : "no");
v4l2_info(sd, "Input color space: %s %s range\n",
@@ -1695,24 +1718,33 @@ static int tc358743_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_state *sd_state,
struct v4l2_subdev_mbus_code_enum *code)
{
switch (code->index) {
case 0:
- code->code = MEDIA_BUS_FMT_RGB888_1X24;
+ code->code = MEDIA_BUS_FMT_BGR888_1X24;
break;
case 1:
code->code = MEDIA_BUS_FMT_UYVY8_1X16;
break;
+ case 2:
+ /*
+ * We need to keep RGB888 for backward compatibility,
+ * but we should list it last for userspace to pick BGR.
+ */
+ code->code = MEDIA_BUS_FMT_RGB888_1X24;
+ break;
default:
return -EINVAL;
}
return 0;
}
static u32 tc358743_g_colorspace(u32 code)
{
switch (code) {
+ case MEDIA_BUS_FMT_BGR888_1X24:
+ fallthrough;
case MEDIA_BUS_FMT_RGB888_1X24:
return V4L2_COLORSPACE_SRGB;
case MEDIA_BUS_FMT_UYVY8_1X16:
return V4L2_COLORSPACE_SMPTE170M;
default:
@@ -1746,11 +1778,12 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd,
struct tc358743_state *state = to_state(sd);
u32 code = format->format.code; /* is overwritten by get_fmt */
int ret = tc358743_get_fmt(sd, sd_state, format);
- if (code == MEDIA_BUS_FMT_RGB888_1X24 ||
+ if (code == MEDIA_BUS_FMT_BGR888_1X24 ||
+ code == MEDIA_BUS_FMT_RGB888_1X24 ||
code == MEDIA_BUS_FMT_UYVY8_1X16)
format->format.code = code;
format->format.colorspace = tc358743_g_colorspace(format->format.code);
if (ret)
@@ -2166,11 +2199,11 @@ static int tc358743_probe(struct i2c_client *client)
sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
err = media_entity_pads_init(&sd->entity, 1, &state->pad);
if (err < 0)
goto err_hdl;
- state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24;
+ state->mbus_fmt_code = MEDIA_BUS_FMT_BGR888_1X24;
sd->dev = &client->dev;
mutex_init(&state->confctl_mutex);
--
2.50.1
Powered by blists - more mailing lists