Skip to content

Fixup CFE and Unicam RGB/BGR888 handling #7406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
6by9 wants to merge 4 commits into
base: rpi-6.18.y
Choose a base branch
from
Open

Fixup CFE and Unicam RGB/BGR888 handling #7406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
adb4f2d
Revert "media: tc358743: Fix the RGB MBUS format"
6by9 May 28, 2026
4c2b287
media: bcm2835-unicam: Fix RGB format / mbus code association
Mar 9, 2026
40009ce
media: rp1-cfe: Fix RGB format / mbus code association
6by9 May 28, 2026
9ff6804
media: rp1-cfe-downstream: Fix RGB format / mbus code association
6by9 May 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 9 additions & 44 deletions drivers/media/i2c/tc358743.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,6 @@ 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:
Expand All @@ -772,17 +771,7 @@ static void tc358743_set_csi_color_space(struct v4l2_subdev *sd)
mutex_unlock(&state->confctl_mutex);
break;
case MEDIA_BUS_FMT_RGB888_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.
*/
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__);
v4l2_dbg(2, debug, sd, "%s: RGB 888 24-bit\n", __func__);
i2c_wr8_and_or(sd, VOUT_SET2,
~(MASK_SEL422 | MASK_VOUT_422FIL_100) & 0xff,
0x00);
Expand Down Expand Up @@ -1443,26 +1432,11 @@ 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");

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, "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");

v4l2_info(sd, "-----%s status-----\n", is_hdmi(sd) ? "HDMI" : "DVI-D");
v4l2_info(sd, "HDCP encrypted content: %s\n",
Expand Down Expand Up @@ -1799,18 +1773,11 @@ static int tc358743_enum_mbus_code(struct v4l2_subdev *sd,
{
switch (code->index) {
case 0:
code->code = MEDIA_BUS_FMT_BGR888_1X24;
code->code = MEDIA_BUS_FMT_RGB888_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;
}
Expand All @@ -1820,7 +1787,6 @@ 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:
Expand Down Expand Up @@ -1858,8 +1824,7 @@ 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_BGR888_1X24 ||
code == MEDIA_BUS_FMT_RGB888_1X24 ||
if (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);
Expand Down Expand Up @@ -2279,7 +2244,7 @@ static int tc358743_probe(struct i2c_client *client)
if (err < 0)
goto err_hdl;

state->mbus_fmt_code = MEDIA_BUS_FMT_BGR888_1X24;
state->mbus_fmt_code = MEDIA_BUS_FMT_RGB888_1X24;

sd->dev = &client->dev;

Expand Down
41 changes: 32 additions & 9 deletions drivers/media/platform/broadcom/bcm2835-unicam.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,12 +342,12 @@ static const struct unicam_format_info unicam_image_formats[] = {
.csi_dt = MIPI_CSI2_DT_RGB565,
}, {
.fourcc = V4L2_PIX_FMT_RGB24, /* rgb */
.code = MEDIA_BUS_FMT_RGB888_1X24,
.code = MEDIA_BUS_FMT_BGR888_1X24,
.depth = 24,
.csi_dt = MIPI_CSI2_DT_RGB888,
}, {
.fourcc = V4L2_PIX_FMT_BGR24, /* bgr */
.code = MEDIA_BUS_FMT_BGR888_1X24,
.code = MEDIA_BUS_FMT_RGB888_1X24,
.depth = 24,
.csi_dt = MIPI_CSI2_DT_RGB888,
}, {
Expand Down Expand Up @@ -2148,22 +2148,45 @@ static int unicam_video_link_validate(struct media_link *link)
const struct v4l2_pix_format *fmt = &node->fmt.fmt.pix;
const struct unicam_format_info *fmtinfo;

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

if (fmtinfo->code != format->code ||
fmt->height != format->height ||
/*
* Unicam initially associated BGR24 to BGR888_1X24 and RGB24 to
* RGB888_1X24.
*
* In order to allow the applications using the old behaviour to
* run, let's accept the old combination, but warn about it.
*/
if (fmtinfo->fourcc != fmt->pixelformat) {
if ((fmt->pixelformat == V4L2_PIX_FMT_BGR24 &&
format->code == MEDIA_BUS_FMT_BGR888_1X24) ||
(fmt->pixelformat == V4L2_PIX_FMT_RGB24 &&
format->code == MEDIA_BUS_FMT_RGB888_1X24)) {
dev_warn_once(node->dev->dev,
"Incorrect pixel format %p4cc for 0x%04x. Fix your application to use %p4cc.\n",
&fmt->pixelformat, format->code, &fmtinfo->fourcc);
} else {
dev_dbg(node->dev->dev,
"image: format mismatch: 0x%04x <=> %p4cc\n",
format->code, &fmt->pixelformat);
ret = -EPIPE;
goto out;
}
}

if (fmt->height != format->height ||
fmt->width != format->width ||
fmt->field != format->field) {
dev_dbg(node->dev->dev,
"image: (%u x %u) 0x%08x %s != (%u x %u) 0x%08x %s\n",
fmt->width, fmt->height, fmtinfo->code,
"image: (%u x %u) %s != (%u x %u) %s\n",
fmt->width, fmt->height,
v4l2_field_names[fmt->field],
format->width, format->height, format->code,
format->width, format->height,
v4l2_field_names[format->field]);
ret = -EPIPE;
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/media/platform/raspberrypi/rp1-cfe/cfe-fmts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ static const struct cfe_fmt formats[] = {
},
{
.fourcc = V4L2_PIX_FMT_RGB24, /* rgb */
.code = MEDIA_BUS_FMT_RGB888_1X24,
.code = MEDIA_BUS_FMT_BGR888_1X24,
.depth = 24,
.csi_dt = MIPI_CSI2_DT_RGB888,
},
{
.fourcc = V4L2_PIX_FMT_BGR24, /* bgr */
.code = MEDIA_BUS_FMT_BGR888_1X24,
.code = MEDIA_BUS_FMT_RGB888_1X24,
.depth = 24,
.csi_dt = MIPI_CSI2_DT_RGB888,
},
Expand Down
23 changes: 20 additions & 3 deletions drivers/media/platform/raspberrypi/rp1-cfe/cfe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1807,13 +1807,30 @@ static int cfe_video_link_validate(struct media_link *link)
goto out;
}

fmt = find_format_by_code_and_fourcc(source_fmt->code,
pix_fmt->pixelformat);
fmt = find_format_by_code(source_fmt->code);

if (!fmt) {
cfe_err(cfe, "Format mismatch!\n");
cfe_err(cfe, "Format mismatch - unknown code!\n");
ret = -EINVAL;
goto out;
}

if (fmt->fourcc != pix_fmt->pixelformat) {
if ((pix_fmt->pixelformat == V4L2_PIX_FMT_BGR24 &&
source_fmt->code == MEDIA_BUS_FMT_BGR888_1X24) ||
(pix_fmt->pixelformat == V4L2_PIX_FMT_RGB24 &&
source_fmt->code == MEDIA_BUS_FMT_RGB888_1X24)) {
dev_warn_once(&cfe->pdev->dev,
"Incorrect pixel format %p4cc for 0x%04x. Fix your application to use %p4cc.\n",
&pix_fmt->pixelformat,
source_fmt->code, &fmt->fourcc);
} else {
cfe_err(cfe, "Format mismatch! Code %u fourcc %p4cc\n",
source_fmt->code, &pix_fmt->pixelformat);
ret = -EINVAL;
goto out;
}
}
} else if (is_csi2_node(node) && is_meta_output_node(node)) {
struct v4l2_meta_format *meta_fmt = &node->meta_fmt.fmt.meta;
const struct cfe_fmt *fmt;
Expand Down
29 changes: 20 additions & 9 deletions drivers/media/platform/raspberrypi/rp1_cfe/cfe.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1676,7 +1676,6 @@ static int cfe_video_link_validate(struct cfe_node *node,
if (is_image_output_node(node)) {
struct v4l2_pix_format *pix_fmt = &node->vid_fmt.fmt.pix;
const struct cfe_fmt *fmt = NULL;
unsigned int i;

if (remote_fmt->width != pix_fmt->width ||
remote_fmt->height != pix_fmt->height) {
Expand All @@ -1687,17 +1686,29 @@ static int cfe_video_link_validate(struct cfe_node *node,
return -EINVAL;
}

for (i = 0; i < ARRAY_SIZE(formats); i++) {
if (formats[i].code == remote_fmt->code &&
formats[i].fourcc == pix_fmt->pixelformat) {
fmt = &formats[i];
break;
}
}
fmt = find_format_by_code(remote_fmt->code);

if (!fmt) {
cfe_err("Format mismatch!\n");
cfe_err("Format not found. %08x %p4cc\n",
remote_fmt->code, &pix_fmt->pixelformat);
return -EINVAL;
}

if (fmt->fourcc != pix_fmt->pixelformat) {
if ((pix_fmt->pixelformat == V4L2_PIX_FMT_BGR24 &&
remote_fmt->code == MEDIA_BUS_FMT_BGR888_1X24) ||
(pix_fmt->pixelformat == V4L2_PIX_FMT_RGB24 &&
remote_fmt->code == MEDIA_BUS_FMT_RGB888_1X24)) {
dev_warn_once(&cfe->pdev->dev,
"Incorrect pixel format %p4cc for 0x%04x. Fix your application to use %p4cc.\n",
&pix_fmt->pixelformat,
remote_fmt->code, &fmt->fourcc);
} else {
cfe_err("Format mismatch! Code %u fourcc %p4cc\n",
remote_fmt->code, &pix_fmt->pixelformat);
return -EINVAL;
}
}
} else if (is_csi2_node(node) && is_meta_output_node(node)) {
struct v4l2_meta_format *meta_fmt = &node->meta_fmt.fmt.meta;
const struct cfe_fmt *fmt;
Expand Down
4 changes: 2 additions & 2 deletions drivers/media/platform/raspberrypi/rp1_cfe/cfe_fmts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ static const struct cfe_fmt formats[] = {
},
{
.fourcc = V4L2_PIX_FMT_RGB24, /* rgb */
.code = MEDIA_BUS_FMT_RGB888_1X24,
.code = MEDIA_BUS_FMT_BGR888_1X24,
.depth = 24,
.csi_dt = MIPI_CSI2_DT_RGB888,
},
{
.fourcc = V4L2_PIX_FMT_BGR24, /* bgr */
.code = MEDIA_BUS_FMT_BGR888_1X24,
.code = MEDIA_BUS_FMT_RGB888_1X24,
.depth = 24,
.csi_dt = MIPI_CSI2_DT_RGB888,
},
Expand Down
Loading