Diffstat (limited to 'patches/camera/0027-media-i2c-ov13858-fix-bayer-order-when-mirrored.patch')
| -rw-r--r-- | patches/camera/0027-media-i2c-ov13858-fix-bayer-order-when-mirrored.patch | 142 |
1 files changed, 142 insertions, 0 deletions
diff --git a/patches/camera/0027-media-i2c-ov13858-fix-bayer-order-when-mirrored.patch b/patches/camera/0027-media-i2c-ov13858-fix-bayer-order-when-mirrored.patch new file mode 100644 index 0000000..d465f8d --- /dev/null +++ b/patches/camera/0027-media-i2c-ov13858-fix-bayer-order-when-mirrored.patch @@ -0,0 +1,142 @@ +media: i2c: ov13858: report the right bayer order + +The driver advertises MEDIA_BUS_FMT_SGRBG10_1X10 unconditionally, but that is +not what the sensor delivers in any flip state on the Surface Pro 12in. The +horizontal mirror moves the Bayer phase by one column, and the native order is +GBRG rather than GRBG, so the two correct codes are GBRG unmirrored and BGGR +mirrored. + +The rear ov13858 is mounted upside down (rotation = <180> in DT), so userspace +enables both flips to compensate and every frame is then debayered as RGGB +while the sensor emits BGGR. Red and blue are exchanged: an orange object comes +out cyan, warm wood comes out blue, while neutrals are unaffected, which is +what makes it easy to mistake for a white balance problem. + +Two measurements on a Surface Pro 12in, from raw frames captured off the CAMSS +RDI video node. The first locates the green pair by comparing the mean absolute +difference of the diagonal against the antidiagonal samples of each 2x2 quad, +the two greens correlating far better than red against blue: + + hflip=0 vflip=0 diagonal 8.65 antidiagonal 56.82 -> greens diagonal + hflip=1 vflip=0 diagonal 56.25 antidiagonal 8.34 -> greens antidiagonal + hflip=0 vflip=1 diagonal 13.21 antidiagonal 55.94 -> greens diagonal + hflip=1 vflip=1 diagonal 56.04 antidiagonal 13.16 -> greens antidiagonal + +So only the horizontal mirror moves the phase; the ISP Y window offset written +for VFLIP compensates the vertical one. + +That test cannot tell red from blue, because GBRG and GRBG share a green +diagonal just as BGGR and RGGB share a green antidiagonal. Resolving the +remaining ambiguity needs colour: a scene of objects with known colour was +captured in all four states, and the red channel identified as the one whose +ratio to blue matches the scene. The rejected assignment is the reciprocal, so +the margin is large: + + hflip=0 vflip=0 R/B 1.651 (reciprocal 0.606) -> GBRG + hflip=0 vflip=1 R/B 1.622 (reciprocal 0.616) -> GBRG + hflip=1 vflip=0 R/B 1.651 (reciprocal 0.606) -> BGGR + hflip=1 vflip=1 R/B 1.625 (reciprocal 0.615) -> BGGR + +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. + +The native order disagreeing with upstream is suspicious in itself. The removal +of the per-mode {0x3811, 0x04} and {0x3813, 0x05} ISP window offsets in +"media: i2c: ov13858: add rotate control support", which now writes 1 or 2 into +those registers from the flip handlers, changes the X parity, and that patch +also sets BIT(3) of ROTATE_CONTROL when HFLIP is clear while VFLIP sets BIT(4) +when set. Whether the phase should instead be corrected there is left alone +here: this patch only reports what the sensor is measured to emit. + +--- 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 GBRG order ++ * becomes BGGR. 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_SBGGR10_1X10 ++ : MEDIA_BUS_FMT_SGBRG10_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; |