aboutsummaryrefslogtreecommitdiffci
path: root/patches/camera/0027-media-i2c-ov13858-fix-bayer-order-when-mirrored.patch
blob: d465f8d2ea59638e9ffe282f48e8c4da750cc1a2 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
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;