1 From 3ad8e28669e0058e3cec482a47215e50e33f2574 Mon Sep 17 00:00:00 2001
2 From: Vinay Varma <varmavinaym@gmail.com>
3 Date: Sun, 11 Jun 2023 23:45:03 +0800
4 Subject: [PATCH] media: i2c: imx219: fix binning and rate_factor for 480p and
7 At a high FPS with RAW10, there is frame corruption for 480p because the
8 rate_factor of 2 is used with the normal 2x2 bining [1]. This commit
9 ties the rate_factor to the selected binning mode. For the 480p mode,
10 analog 2x2 binning mode with a rate_factor of 2 is always used. For the
11 1232p mode the normal 2x2 binning mode is used for RAW10 while analog
12 2x2 binning mode is used for RAW8.
14 [1] https://github.com/raspberrypi/linux/issues/5493
16 Signed-off-by: Vinay Varma <varmavinaym@gmail.com>
18 drivers/media/i2c/imx219.c | 143 ++++++++++++++++++++++++++-----------
19 1 file changed, 100 insertions(+), 43 deletions(-)
21 --- a/drivers/media/i2c/imx219.c
22 +++ b/drivers/media/i2c/imx219.c
23 @@ -136,6 +136,18 @@ enum pad_types {
29 + BINNING_DIGITAL_2x2,
33 +enum binning_bit_depths {
42 @@ -162,11 +174,8 @@ struct imx219_mode {
43 /* Default register values */
44 struct imx219_reg_list reg_list;
46 - /* 2x2 binning is used */
49 - /* Relative pixel clock rate factor for the mode. */
50 - unsigned int rate_factor;
51 + /* binning mode based on format code */
52 + enum binning_mode binning[BINNING_IDX_MAX];
55 static const struct imx219_reg imx219_common_regs[] = {
56 @@ -404,8 +413,10 @@ static const struct imx219_mode supporte
57 .num_of_regs = ARRAY_SIZE(mode_3280x2464_regs),
58 .regs = mode_3280x2464_regs,
63 + [BINNING_IDX_8_BIT] = BINNING_NONE,
64 + [BINNING_IDX_10_BIT] = BINNING_NONE,
68 /* 1080P 30fps cropped */
69 @@ -422,8 +433,10 @@ static const struct imx219_mode supporte
70 .num_of_regs = ARRAY_SIZE(mode_1920_1080_regs),
71 .regs = mode_1920_1080_regs,
76 + [BINNING_IDX_8_BIT] = BINNING_NONE,
77 + [BINNING_IDX_10_BIT] = BINNING_NONE,
81 /* 2x2 binned 30fps mode */
82 @@ -440,8 +453,10 @@ static const struct imx219_mode supporte
83 .num_of_regs = ARRAY_SIZE(mode_1640_1232_regs),
84 .regs = mode_1640_1232_regs,
89 + [BINNING_IDX_8_BIT] = BINNING_ANALOG_2x2,
90 + [BINNING_IDX_10_BIT] = BINNING_DIGITAL_2x2,
94 /* 640x480 30fps mode */
95 @@ -458,12 +473,10 @@ static const struct imx219_mode supporte
96 .num_of_regs = ARRAY_SIZE(mode_640_480_regs),
97 .regs = mode_640_480_regs,
101 - * This mode uses a special 2x2 binning that doubles the
102 - * internal pixel clock rate.
106 + [BINNING_IDX_8_BIT] = BINNING_ANALOG_2x2,
107 + [BINNING_IDX_10_BIT] = BINNING_ANALOG_2x2,
112 @@ -652,12 +665,51 @@ static int imx219_open(struct v4l2_subde
116 +static int imx219_resolve_binning(struct imx219 *imx219,
117 + enum binning_mode *binning)
119 + switch (imx219->fmt.code) {
120 + case MEDIA_BUS_FMT_SRGGB8_1X8:
121 + case MEDIA_BUS_FMT_SGRBG8_1X8:
122 + case MEDIA_BUS_FMT_SGBRG8_1X8:
123 + case MEDIA_BUS_FMT_SBGGR8_1X8:
124 + *binning = imx219->mode->binning[BINNING_IDX_8_BIT];
127 + case MEDIA_BUS_FMT_SRGGB10_1X10:
128 + case MEDIA_BUS_FMT_SGRBG10_1X10:
129 + case MEDIA_BUS_FMT_SGBRG10_1X10:
130 + case MEDIA_BUS_FMT_SBGGR10_1X10:
131 + *binning = imx219->mode->binning[BINNING_IDX_10_BIT];
137 +static int imx219_get_rate_factor(struct imx219 *imx219)
139 + enum binning_mode binning = BINNING_NONE;
140 + int ret = imx219_resolve_binning(imx219, &binning);
146 + case BINNING_DIGITAL_2x2:
148 + case BINNING_ANALOG_2x2:
154 static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
156 struct imx219 *imx219 =
157 container_of(ctrl->handler, struct imx219, ctrl_handler);
158 struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
162 if (ctrl->id == V4L2_CID_VBLANK) {
163 int exposure_max, exposure_def;
164 @@ -679,6 +731,10 @@ static int imx219_set_ctrl(struct v4l2_c
165 if (pm_runtime_get_if_in_use(&client->dev) == 0)
168 + rate_factor = imx219_get_rate_factor(imx219);
169 + if (rate_factor < 0)
170 + return rate_factor;
173 case V4L2_CID_ANALOGUE_GAIN:
174 ret = imx219_write_reg(imx219, IMX219_REG_ANALOG_GAIN,
175 @@ -687,7 +743,7 @@ static int imx219_set_ctrl(struct v4l2_c
176 case V4L2_CID_EXPOSURE:
177 ret = imx219_write_reg(imx219, IMX219_REG_EXPOSURE,
178 IMX219_REG_VALUE_16BIT,
179 - ctrl->val / imx219->mode->rate_factor);
180 + ctrl->val / rate_factor);
182 case V4L2_CID_DIGITAL_GAIN:
183 ret = imx219_write_reg(imx219, IMX219_REG_DIGITAL_GAIN,
184 @@ -708,7 +764,7 @@ static int imx219_set_ctrl(struct v4l2_c
185 ret = imx219_write_reg(imx219, IMX219_REG_VTS,
186 IMX219_REG_VALUE_16BIT,
187 (imx219->mode->height + ctrl->val) /
188 - imx219->mode->rate_factor);
191 case V4L2_CID_HBLANK:
192 ret = imx219_write_reg(imx219, IMX219_REG_HTS,
193 @@ -890,7 +946,7 @@ static int imx219_set_pad_format(struct
194 struct imx219 *imx219 = to_imx219(sd);
195 const struct imx219_mode *mode;
196 struct v4l2_mbus_framefmt *framefmt;
197 - int exposure_max, exposure_def, hblank, pixel_rate;
198 + int exposure_max, exposure_def, hblank, pixel_rate, rate_factor;
201 if (fmt->pad >= NUM_PADS)
202 @@ -924,6 +980,9 @@ static int imx219_set_pad_format(struct
204 imx219->fmt = fmt->format;
206 + rate_factor = imx219_get_rate_factor(imx219);
207 + if (rate_factor < 0)
208 + return rate_factor;
209 /* Update limits and set FPS to default */
210 __v4l2_ctrl_modify_range(imx219->vblank,
212 @@ -957,8 +1016,7 @@ static int imx219_set_pad_format(struct
213 __v4l2_ctrl_s_ctrl(imx219->hblank, hblank);
215 /* Scale the pixel rate based on the mode specific factor */
217 - IMX219_PIXEL_RATE * imx219->mode->rate_factor;
218 + pixel_rate = IMX219_PIXEL_RATE * rate_factor;
219 __v4l2_ctrl_modify_range(imx219->pixel_rate, pixel_rate,
220 pixel_rate, 1, pixel_rate);
222 @@ -1001,30 +1059,25 @@ static int imx219_set_framefmt(struct im
224 static int imx219_set_binning(struct imx219 *imx219)
226 - if (!imx219->mode->binning) {
227 + enum binning_mode binning = BINNING_NONE;
228 + int ret = imx219_resolve_binning(imx219, &binning);
234 return imx219_write_reg(imx219, IMX219_REG_BINNING_MODE,
235 IMX219_REG_VALUE_16BIT,
236 IMX219_BINNING_NONE);
239 - switch (imx219->fmt.code) {
240 - case MEDIA_BUS_FMT_SRGGB8_1X8:
241 - case MEDIA_BUS_FMT_SGRBG8_1X8:
242 - case MEDIA_BUS_FMT_SGBRG8_1X8:
243 - case MEDIA_BUS_FMT_SBGGR8_1X8:
244 + case BINNING_DIGITAL_2x2:
245 return imx219_write_reg(imx219, IMX219_REG_BINNING_MODE,
246 IMX219_REG_VALUE_16BIT,
247 - IMX219_BINNING_2X2_ANALOG);
249 - case MEDIA_BUS_FMT_SRGGB10_1X10:
250 - case MEDIA_BUS_FMT_SGRBG10_1X10:
251 - case MEDIA_BUS_FMT_SGBRG10_1X10:
252 - case MEDIA_BUS_FMT_SBGGR10_1X10:
253 + IMX219_BINNING_2X2);
254 + case BINNING_ANALOG_2x2:
255 return imx219_write_reg(imx219, IMX219_REG_BINNING_MODE,
256 IMX219_REG_VALUE_16BIT,
257 - IMX219_BINNING_2X2);
258 + IMX219_BINNING_2X2_ANALOG);
264 @@ -1342,7 +1395,7 @@ static int imx219_init_controls(struct i
265 struct v4l2_ctrl_handler *ctrl_hdlr;
266 unsigned int height = imx219->mode->height;
267 struct v4l2_fwnode_device_properties props;
268 - int exposure_max, exposure_def, hblank, pixel_rate;
269 + int exposure_max, exposure_def, hblank, pixel_rate, rate_factor;
272 ctrl_hdlr = &imx219->ctrl_handler;
273 @@ -1353,8 +1406,12 @@ static int imx219_init_controls(struct i
274 mutex_init(&imx219->mutex);
275 ctrl_hdlr->lock = &imx219->mutex;
277 + rate_factor = imx219_get_rate_factor(imx219);
278 + if (rate_factor < 0)
279 + return rate_factor;
281 /* By default, PIXEL_RATE is read only */
282 - pixel_rate = IMX219_PIXEL_RATE * imx219->mode->rate_factor;
283 + pixel_rate = IMX219_PIXEL_RATE * rate_factor;
284 imx219->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, &imx219_ctrl_ops,
286 pixel_rate, pixel_rate,
287 @@ -1576,6 +1633,9 @@ static int imx219_probe(struct i2c_clien
288 goto error_power_off;
289 usleep_range(100, 110);
291 + /* Initialize default format */
292 + imx219_set_default_format(imx219);
294 ret = imx219_init_controls(imx219);
296 goto error_power_off;
297 @@ -1590,9 +1650,6 @@ static int imx219_probe(struct i2c_clien
298 imx219->pad[IMAGE_PAD].flags = MEDIA_PAD_FL_SOURCE;
299 imx219->pad[METADATA_PAD].flags = MEDIA_PAD_FL_SOURCE;
301 - /* Initialize default format */
302 - imx219_set_default_format(imx219);
304 ret = media_entity_pads_init(&imx219->sd.entity, NUM_PADS, imx219->pad);
306 dev_err(dev, "failed to init entity pads: %d\n", ret);