[PATCH] media: i2c: ov13858: add horizontal and vertical flip controls

[Sergey Lebedev]

The driver programs OV13858_REG_FORMAT1 (0x3820) from its mode tables and
never exposes the readout direction, so a module mounted rotated cannot be
corrected.

The Microsoft Surface Pro 11 for Business (Intel) mounts this sensor upside
down, and ipu-bridge now says so - b238116ccd4b ("media: ipu-bridge: Add
upside-down quirk for Surface Pro 11"). libcamera reads that rotation and
tries to compensate with sensor flips, finds neither control, and falls
back to Rot0, so the quirk on its own names a rotation nothing can undo.

ov13b10 has the same two controls, but it is a different part and its bit
assignments do not carry over. There is no public datasheet for this one,
so these were found by experiment on a single sample: single bits written
over i2c mid-stream, each captured frame correlated against the flipped
baseline. Of every bit in 0x3820 through 0x3823 exactly two move the
image: 0x3820 BIT(4) set flips vertically, and BIT(3) cleared mirrors
horizontally. 0x3821, where the mirror sits on several other OmniVision
parts, has no effect here.

Verified through the controls against a static scene, as correlation with
the flipped reference and, as a control, with the unflipped one:

  vflip  +0.994 / +0.629    hflip  +0.973 / -0.141    both  +0.975 / -0.178

The mirror bit is active low and every mode table already sets it, so the
defaults write back what the mode list just wrote.
__v4l2_ctrl_handler_setup() runs after that list and before MODE_SELECT,
so the read-modify-write here sees the value the mode just programmed.

The Bayer order at the output does not change with either flip: per-channel
means over the four states agree to 0.2 counts in 70, and all four frames
demosaic correctly against one fixed pattern. Unlike imx219 and imx258,
whose flips select a different media bus code, these controls therefore do
not need V4L2_CTRL_FLAG_MODIFY_LAYOUT.

Signed-off-by: Sergey Lebedev <lsa.uz@xxxxx>
---
 drivers/media/i2c/ov13858.c | 44 +++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index de2b79a9a0..1bf21ffbc7 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -76,6 +76,15 @@
 #define OV13858_DGTL_GAIN_DEFAULT	1024	/* Default gain = 1 X */
 #define OV13858_DGTL_GAIN_STEP		1	/* Each step = 1/1024 */
 
+/*
+ * Readout direction. Neither flip changes the Bayer order at the output, so
+ * no window offset compensation is needed. The mirror bit is active low: the
+ * value the mode tables program already has it set.
+ */
+#define OV13858_REG_FORMAT1		0x3820
+#define OV13858_FORMAT1_VFLIP		BIT(4)
+#define OV13858_FORMAT1_HFLIP_N		BIT(3)
+
 /* Test Pattern Control */
 #define OV13858_REG_TEST_PATTERN	0x4503
 #define OV13858_TEST_PATTERN_ENABLE	BIT(7)
@@ -1042,6 +1051,8 @@ struct ov13858 {
 	struct v4l2_ctrl *vblank;
 	struct v4l2_ctrl *hblank;
 	struct v4l2_ctrl *exposure;
+	struct v4l2_ctrl *hflip;
+	struct v4l2_ctrl *vflip;
 
 	/* Current mode */
 	const struct ov13858_mode *cur_mode;
@@ -1208,6 +1219,30 @@ static int ov13858_enable_test_pattern(struct ov13858 *ov13858, u32 pattern)
 				 OV13858_REG_VALUE_08BIT, val);
 }
 
+static int ov13858_update_flips(struct ov13858 *ov13858)
+{
+	u32 val;
+	int ret;
+
+	ret = ov13858_read_reg(ov13858, OV13858_REG_FORMAT1,
+			       OV13858_REG_VALUE_08BIT, &val);
+	if (ret)
+		return ret;
+
+	if (ov13858->vflip->val)
+		val |= OV13858_FORMAT1_VFLIP;
+	else
+		val &= ~OV13858_FORMAT1_VFLIP;
+
+	if (ov13858->hflip->val)
+		val &= ~OV13858_FORMAT1_HFLIP_N;
+	else
+		val |= OV13858_FORMAT1_HFLIP_N;
+
+	return ov13858_write_reg(ov13858, OV13858_REG_FORMAT1,
+				 OV13858_REG_VALUE_08BIT, val);
+}
+
 static int ov13858_set_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct ov13858 *ov13858 = container_of(ctrl->handler,
@@ -1254,6 +1289,10 @@ static int ov13858_set_ctrl(struct v4l2_ctrl *ctrl)
 					ov13858->cur_mode->height
 					  + ctrl->val);
 		break;
+	case V4L2_CID_HFLIP:
+	case V4L2_CID_VFLIP:
+		ret = ov13858_update_flips(ov13858);
+		break;
 	case V4L2_CID_TEST_PATTERN:
 		ret = ov13858_enable_test_pattern(ov13858, ctrl->val);
 		break;
@@ -1619,6 +1658,11 @@ static int ov13858_init_controls(struct ov13858 *ov13858)
 			  OV13858_DGTL_GAIN_MIN, OV13858_DGTL_GAIN_MAX,
 			  OV13858_DGTL_GAIN_STEP, OV13858_DGTL_GAIN_DEFAULT);
 
+	ov13858->hflip = v4l2_ctrl_new_std(ctrl_hdlr, &ov13858_ctrl_ops,
+					   V4L2_CID_HFLIP, 0, 1, 1, 0);
+	ov13858->vflip = v4l2_ctrl_new_std(ctrl_hdlr, &ov13858_ctrl_ops,
+					   V4L2_CID_VFLIP, 0, 1, 1, 0);
+
 	v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &ov13858_ctrl_ops,
 				     V4L2_CID_TEST_PATTERN,
 				     ARRAY_SIZE(ov13858_test_pattern_menu) - 1,
-- 
2.54.0 (Apple Git-157)