media: i2c: ov13858: report the right bayer order when mirrored The driver always advertises MEDIA_BUS_FMT_SGRBG10_1X10, but the horizontal mirror moves the Bayer phase by one column, so the sensor actually delivers RGGB while HFLIP is set. The Surface Pro 12in has the rear ov13858 mounted upside down (rotation = <180> in DT), so both flips end up enabled and every frame is debayered with the wrong pattern: the two greens land in the red and blue output channels and the picture comes out desaturated, close to monochrome. Measured on a Surface Pro 12in by capturing raw frames off the CAMSS RDI node and comparing the mean absolute difference of the two diagonal pairs of each 2x2 quad (the two green samples correlate far better than red against blue): hflip=0 vflip=0 diagonal 4.29 antidiagonal 6.84 -> greens diagonal hflip=1 vflip=0 diagonal 7.03 antidiagonal 4.31 -> greens antidiagonal hflip=0 vflip=1 diagonal 4.35 antidiagonal 6.84 -> greens diagonal hflip=1 vflip=1 diagonal 6.93 antidiagonal 4.34 -> greens antidiagonal So only the horizontal mirror moves the phase; the ISP Y window offset written for VFLIP compensates the vertical one. Derive the media bus code from HFLIP and tag the control with V4L2_CTRL_FLAG_MODIFY_LAYOUT so userspace knows to renegotiate the format after toggling it. --- a/drivers/media/i2c/ov13858.c +++ b/drivers/media/i2c/ov13858.c @@ -1032,6 +1032,7 @@ struct v4l2_ctrl *vblank; struct v4l2_ctrl *hblank; struct v4l2_ctrl *exposure; + struct v4l2_ctrl *hflip; struct clk *img_clk; struct gpio_desc *reset_gpio; @@ -1222,8 +1223,10 @@ OV13858_DGTL_GAIN_MIN, OV13858_DGTL_GAIN_MAX, OV13858_DGTL_GAIN_STEP, OV13858_DGTL_GAIN_DEFAULT); - v4l2_ctrl_new_std(ctrl_hdlr, &ov13858_ctrl_ops, V4L2_CID_HFLIP, - 0, 1, 1, 0); + ov13858->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &ov13858_ctrl_ops, + V4L2_CID_HFLIP, 0, 1, 1, 0); + if (ov13858->hflip) + ov13858->hflip->flags |= V4L2_CTRL_FLAG_MODIFY_LAYOUT; v4l2_ctrl_new_std(ctrl_hdlr, &ov13858_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0); @@ -1258,12 +1261,24 @@ return ret; } -static void ov13858_update_pad_format(const struct ov13858_mode *mode, +/* + * Mirroring shifts the Bayer phase by one column, so the native GRBG order + * becomes RGGB. The vertical flip is compensated by the ISP Y window offset + * and leaves the order alone. + */ +static u32 ov13858_get_format_code(struct ov13858 *ov13858) +{ + return ov13858->hflip->val ? MEDIA_BUS_FMT_SRGGB10_1X10 + : MEDIA_BUS_FMT_SGRBG10_1X10; +} + +static void ov13858_update_pad_format(struct ov13858 *ov13858, + const struct ov13858_mode *mode, struct v4l2_mbus_framefmt *fmt) { fmt->width = mode->width; fmt->height = mode->height; - fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10; + fmt->code = ov13858_get_format_code(ov13858); fmt->field = V4L2_FIELD_NONE; } @@ -1418,7 +1433,7 @@ ARRAY_SIZE(supported_modes), width, height, fmt->format.width, fmt->format.height); - ov13858_update_pad_format(mode, &fmt->format); + ov13858_update_pad_format(ov13858, mode, &fmt->format); *v4l2_subdev_state_get_format(sd_state, fmt->pad) = fmt->format; if (fmt->which == V4L2_SUBDEV_FORMAT_TRY) @@ -1453,11 +1468,11 @@ struct v4l2_subdev_state *sd_state, struct v4l2_subdev_mbus_code_enum *code) { - /* Only one bayer order(GRBG) is supported */ + /* Only one bayer order is supported, which one depends on the mirror */ if (code->index > 0) return -EINVAL; - code->code = MEDIA_BUS_FMT_SGRBG10_1X10; + code->code = ov13858_get_format_code(to_ov13858(sd)); return 0; } @@ -1469,7 +1484,7 @@ if (fse->index >= ARRAY_SIZE(supported_modes)) return -EINVAL; - if (fse->code != MEDIA_BUS_FMT_SGRBG10_1X10) + if (fse->code != ov13858_get_format_code(to_ov13858(sd))) return -EINVAL; fse->min_width = supported_modes[fse->index].width; @@ -1520,7 +1535,7 @@ { struct ov13858 *ov13858 = to_ov13858(sd); - ov13858_update_pad_format(ov13858->cur_mode, + ov13858_update_pad_format(ov13858, ov13858->cur_mode, v4l2_subdev_state_get_format(sd_state, 0)); return 0;