linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] media: atomisp-ov2722: use v4l2_find_nearest_size()
@ 2021-11-04 19:02 Mauro Carvalho Chehab
  2021-11-04 19:02 ` [PATCH 2/4] media: atomisp-gc0310: " Mauro Carvalho Chehab
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-04 19:02 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Filip Kolev,
	Greg Kroah-Hartman, Hans Verkuil, Martiros Shakhzadyan,
	Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
	linux-kernel, linux-media, linux-staging

Instead of reinventing the wheel, use v4l2_find_nearest_size()
in order to get the closest resolution.

This should address a bug where the wrong resolution was
selected.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../media/atomisp/i2c/atomisp-ov2722.c        | 118 ++++--------------
 drivers/staging/media/atomisp/i2c/ov2722.h    |   3 +-
 2 files changed, 22 insertions(+), 99 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index 90d0871a78a3..da98094d7094 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -557,7 +557,7 @@ static int ov2722_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 		ret = ov2722_g_fnumber_range(&dev->sd, &ctrl->val);
 		break;
 	case V4L2_CID_LINK_FREQ:
-		val = ov2722_res[dev->fmt_idx].mipi_freq;
+		val = dev->res->mipi_freq;
 		if (val == 0)
 			return -EINVAL;
 
@@ -782,76 +782,6 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on)
 	return ret;
 }
 
-/*
- * distance - calculate the distance
- * @res: resolution
- * @w: width
- * @h: height
- *
- * Get the gap between resolution and w/h.
- * res->width/height smaller than w/h wouldn't be considered.
- * Returns the value of gap or -1 if fail.
- */
-#define LARGEST_ALLOWED_RATIO_MISMATCH 800
-static int distance(struct ov2722_resolution *res, u32 w, u32 h)
-{
-	unsigned int w_ratio = (res->width << 13) / w;
-	unsigned int h_ratio;
-	int match;
-
-	if (h == 0)
-		return -1;
-	h_ratio = (res->height << 13) / h;
-	if (h_ratio == 0)
-		return -1;
-	match   = abs(((w_ratio << 13) / h_ratio) - 8192);
-
-	if ((w_ratio < 8192) || (h_ratio < 8192) ||
-	    (match > LARGEST_ALLOWED_RATIO_MISMATCH))
-		return -1;
-
-	return w_ratio + h_ratio;
-}
-
-/* Return the nearest higher resolution index */
-static int nearest_resolution_index(int w, int h)
-{
-	int i;
-	int idx = -1;
-	int dist;
-	int min_dist = INT_MAX;
-	struct ov2722_resolution *tmp_res = NULL;
-
-	for (i = 0; i < N_RES; i++) {
-		tmp_res = &ov2722_res[i];
-		dist = distance(tmp_res, w, h);
-		if (dist == -1)
-			continue;
-		if (dist < min_dist) {
-			min_dist = dist;
-			idx = i;
-		}
-	}
-
-	return idx;
-}
-
-static int get_resolution_index(int w, int h)
-{
-	int i;
-
-	for (i = 0; i < N_RES; i++) {
-		if (w != ov2722_res[i].width)
-			continue;
-		if (h != ov2722_res[i].height)
-			continue;
-
-		return i;
-	}
-
-	return -1;
-}
-
 /* TODO: remove it. */
 static int startup(struct v4l2_subdev *sd)
 {
@@ -866,7 +796,7 @@ static int startup(struct v4l2_subdev *sd)
 		return ret;
 	}
 
-	ret = ov2722_write_reg_array(client, ov2722_res[dev->fmt_idx].regs);
+	ret = ov2722_write_reg_array(client, dev->res->regs);
 	if (ret) {
 		dev_err(&client->dev, "ov2722 write register err.\n");
 		return ret;
@@ -882,9 +812,9 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
 	struct v4l2_mbus_framefmt *fmt = &format->format;
 	struct ov2722_device *dev = to_ov2722_sensor(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct ov2722_resolution *res;
 	struct camera_mipi_info *ov2722_info = NULL;
 	int ret = 0;
-	int idx;
 
 	if (format->pad)
 		return -EINVAL;
@@ -895,15 +825,16 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	mutex_lock(&dev->input_lock);
-	idx = nearest_resolution_index(fmt->width, fmt->height);
-	if (idx == -1) {
-		/* return the largest resolution */
-		fmt->width = ov2722_res[N_RES - 1].width;
-		fmt->height = ov2722_res[N_RES - 1].height;
-	} else {
-		fmt->width = ov2722_res[idx].width;
-		fmt->height = ov2722_res[idx].height;
-	}
+	res = v4l2_find_nearest_size(ov2722_res_preview,
+				     ARRAY_SIZE(ov2722_res_preview), width,
+				     height, fmt->width, fmt->height);
+	if (!res)
+		res = &ov2722_res_preview[N_RES - 1];
+
+	fmt->width = res->width;
+	fmt->height = res->height;
+	dev->res = res;
+
 	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
 	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
 		sd_state->pads->try_fmt = *fmt;
@@ -911,15 +842,9 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
 		return 0;
 	}
 
-	dev->fmt_idx = get_resolution_index(fmt->width, fmt->height);
-	if (dev->fmt_idx == -1) {
-		dev_err(&client->dev, "get resolution fail\n");
-		mutex_unlock(&dev->input_lock);
-		return -EINVAL;
-	}
 
-	dev->pixels_per_line = ov2722_res[dev->fmt_idx].pixels_per_line;
-	dev->lines_per_frame = ov2722_res[dev->fmt_idx].lines_per_frame;
+	dev->pixels_per_line = dev->res->pixels_per_line;
+	dev->lines_per_frame = dev->res->lines_per_frame;
 
 	ret = startup(sd);
 	if (ret) {
@@ -950,8 +875,7 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
 		}
 	}
 
-	ret = ov2722_get_intg_factor(client, ov2722_info,
-				     &ov2722_res[dev->fmt_idx]);
+	ret = ov2722_get_intg_factor(client, ov2722_info, dev->res);
 	if (ret)
 		dev_err(&client->dev, "failed to get integration_factor\n");
 
@@ -972,8 +896,8 @@ static int ov2722_get_fmt(struct v4l2_subdev *sd,
 	if (!fmt)
 		return -EINVAL;
 
-	fmt->width = ov2722_res[dev->fmt_idx].width;
-	fmt->height = ov2722_res[dev->fmt_idx].height;
+	fmt->width = dev->res->width;
+	fmt->height = dev->res->height;
 	fmt->code = MEDIA_BUS_FMT_SBGGR10_1X10;
 
 	return 0;
@@ -1098,7 +1022,7 @@ static int ov2722_g_frame_interval(struct v4l2_subdev *sd,
 	struct ov2722_device *dev = to_ov2722_sensor(sd);
 
 	interval->interval.numerator = 1;
-	interval->interval.denominator = ov2722_res[dev->fmt_idx].fps;
+	interval->interval.denominator = dev->res->fps;
 
 	return 0;
 }
@@ -1136,7 +1060,7 @@ static int ov2722_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
 	struct ov2722_device *dev = to_ov2722_sensor(sd);
 
 	mutex_lock(&dev->input_lock);
-	*frames = ov2722_res[dev->fmt_idx].skip_frames;
+	*frames = dev->res->skip_frames;
 	mutex_unlock(&dev->input_lock);
 
 	return 0;
@@ -1220,7 +1144,7 @@ static int ov2722_probe(struct i2c_client *client)
 
 	mutex_init(&dev->input_lock);
 
-	dev->fmt_idx = 0;
+	dev->res = &ov2722_res_preview[0];
 	v4l2_i2c_subdev_init(&dev->sd, client, &ov2722_ops);
 
 	ovpdev = gmin_camera_platform_data(&dev->sd,
diff --git a/drivers/staging/media/atomisp/i2c/ov2722.h b/drivers/staging/media/atomisp/i2c/ov2722.h
index 7b0debb6c53d..d6e2510bc01c 100644
--- a/drivers/staging/media/atomisp/i2c/ov2722.h
+++ b/drivers/staging/media/atomisp/i2c/ov2722.h
@@ -201,14 +201,13 @@ struct ov2722_device {
 	struct media_pad pad;
 	struct v4l2_mbus_framefmt format;
 	struct mutex input_lock;
+	struct ov2722_resolution *res;
 
 	struct camera_sensor_platform_data *platform_data;
 	int vt_pix_clk_freq_mhz;
-	int fmt_idx;
 	int run_mode;
 	u16 pixels_per_line;
 	u16 lines_per_frame;
-	u8 res;
 	u8 type;
 
 	struct v4l2_ctrl_handler ctrl_handler;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/4] media: atomisp-gc0310: use v4l2_find_nearest_size()
  2021-11-04 19:02 [PATCH 1/4] media: atomisp-ov2722: use v4l2_find_nearest_size() Mauro Carvalho Chehab
@ 2021-11-04 19:02 ` Mauro Carvalho Chehab
  2021-11-04 19:02 ` [PATCH 3/4] media: atomisp-gc2235: " Mauro Carvalho Chehab
  2021-11-04 19:02 ` [PATCH 4/4] media: atomisp-mt9m114: " Mauro Carvalho Chehab
  2 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-04 19:02 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Deepak R Varma,
	Greg Kroah-Hartman, Hans Verkuil, Leonid Kushnir,
	Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen,
	linux-kernel, linux-media, linux-staging

Instead of reinventing the wheel, use v4l2_find_nearest_size()
in order to get the closest resolution.

This should address a bug where the wrong resolution was
selected.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../media/atomisp/i2c/atomisp-gc0310.c        | 119 ++++--------------
 drivers/staging/media/atomisp/i2c/gc0310.h    |   3 +-
 2 files changed, 22 insertions(+), 100 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
index 687888d643df..cbc8b1d91995 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c
@@ -266,7 +266,7 @@ static int gc0310_g_bin_factor_x(struct v4l2_subdev *sd, s32 *val)
 {
 	struct gc0310_device *dev = to_gc0310_sensor(sd);
 
-	*val = gc0310_res[dev->fmt_idx].bin_factor_x;
+	*val = dev->res->bin_factor_x;
 
 	return 0;
 }
@@ -275,7 +275,7 @@ static int gc0310_g_bin_factor_y(struct v4l2_subdev *sd, s32 *val)
 {
 	struct gc0310_device *dev = to_gc0310_sensor(sd);
 
-	*val = gc0310_res[dev->fmt_idx].bin_factor_y;
+	*val = dev->res->bin_factor_y;
 
 	return 0;
 }
@@ -878,76 +878,6 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on)
 	return gc0310_init(sd);
 }
 
-/*
- * distance - calculate the distance
- * @res: resolution
- * @w: width
- * @h: height
- *
- * Get the gap between resolution and w/h.
- * res->width/height smaller than w/h wouldn't be considered.
- * Returns the value of gap or -1 if fail.
- */
-#define LARGEST_ALLOWED_RATIO_MISMATCH 800
-static int distance(struct gc0310_resolution *res, u32 w, u32 h)
-{
-	unsigned int w_ratio = (res->width << 13) / w;
-	unsigned int h_ratio;
-	int match;
-
-	if (h == 0)
-		return -1;
-	h_ratio = (res->height << 13) / h;
-	if (h_ratio == 0)
-		return -1;
-	match   = abs(((w_ratio << 13) / h_ratio) - 8192);
-
-	if ((w_ratio < 8192) || (h_ratio < 8192)  ||
-	    (match > LARGEST_ALLOWED_RATIO_MISMATCH))
-		return -1;
-
-	return w_ratio + h_ratio;
-}
-
-/* Return the nearest higher resolution index */
-static int nearest_resolution_index(int w, int h)
-{
-	int i;
-	int idx = -1;
-	int dist;
-	int min_dist = INT_MAX;
-	struct gc0310_resolution *tmp_res = NULL;
-
-	for (i = 0; i < N_RES; i++) {
-		tmp_res = &gc0310_res[i];
-		dist = distance(tmp_res, w, h);
-		if (dist == -1)
-			continue;
-		if (dist < min_dist) {
-			min_dist = dist;
-			idx = i;
-		}
-	}
-
-	return idx;
-}
-
-static int get_resolution_index(int w, int h)
-{
-	int i;
-
-	for (i = 0; i < N_RES; i++) {
-		if (w != gc0310_res[i].width)
-			continue;
-		if (h != gc0310_res[i].height)
-			continue;
-
-		return i;
-	}
-
-	return -1;
-}
-
 /* TODO: remove it. */
 static int startup(struct v4l2_subdev *sd)
 {
@@ -955,7 +885,7 @@ static int startup(struct v4l2_subdev *sd)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	ret = gc0310_write_reg_array(client, gc0310_res[dev->fmt_idx].regs);
+	ret = gc0310_write_reg_array(client, dev->res->regs);
 	if (ret) {
 		dev_err(&client->dev, "gc0310 write register err.\n");
 		return ret;
@@ -972,8 +902,8 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
 	struct gc0310_device *dev = to_gc0310_sensor(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct camera_mipi_info *gc0310_info = NULL;
+	struct gc0310_resolution *res;
 	int ret = 0;
-	int idx = 0;
 
 	if (format->pad)
 		return -EINVAL;
@@ -987,15 +917,16 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
 
 	mutex_lock(&dev->input_lock);
 
-	idx = nearest_resolution_index(fmt->width, fmt->height);
-	if (idx == -1) {
-		/* return the largest resolution */
-		fmt->width = gc0310_res[N_RES - 1].width;
-		fmt->height = gc0310_res[N_RES - 1].height;
-	} else {
-		fmt->width = gc0310_res[idx].width;
-		fmt->height = gc0310_res[idx].height;
-	}
+	res = v4l2_find_nearest_size(gc0310_res_preview,
+				     ARRAY_SIZE(gc0310_res_preview), width,
+				     height, fmt->width, fmt->height);
+	if (!res)
+		res = &gc0310_res_preview[N_RES - 1];
+
+	fmt->width = res->width;
+	fmt->height = res->height;
+	dev->res = res;
+
 	fmt->code = MEDIA_BUS_FMT_SGRBG8_1X8;
 
 	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
@@ -1004,23 +935,15 @@ static int gc0310_set_fmt(struct v4l2_subdev *sd,
 		return 0;
 	}
 
-	dev->fmt_idx = get_resolution_index(fmt->width, fmt->height);
-	if (dev->fmt_idx == -1) {
-		dev_err(&client->dev, "get resolution fail\n");
-		mutex_unlock(&dev->input_lock);
-		return -EINVAL;
-	}
-
 	dev_dbg(&client->dev, "%s: before gc0310_write_reg_array %s\n",
-		__func__, gc0310_res[dev->fmt_idx].desc);
+		__func__, dev->res->desc);
 	ret = startup(sd);
 	if (ret) {
 		dev_err(&client->dev, "gc0310 startup err\n");
 		goto err;
 	}
 
-	ret = gc0310_get_intg_factor(client, gc0310_info,
-				     &gc0310_res[dev->fmt_idx]);
+	ret = gc0310_get_intg_factor(client, gc0310_info, dev->res);
 	if (ret) {
 		dev_err(&client->dev, "failed to get integration_factor\n");
 		goto err;
@@ -1044,8 +967,8 @@ static int gc0310_get_fmt(struct v4l2_subdev *sd,
 	if (!fmt)
 		return -EINVAL;
 
-	fmt->width = gc0310_res[dev->fmt_idx].width;
-	fmt->height = gc0310_res[dev->fmt_idx].height;
+	fmt->width = dev->res->width;
+	fmt->height = dev->res->height;
 	fmt->code = MEDIA_BUS_FMT_SGRBG8_1X8;
 
 	return 0;
@@ -1199,7 +1122,7 @@ static int gc0310_g_frame_interval(struct v4l2_subdev *sd,
 	struct gc0310_device *dev = to_gc0310_sensor(sd);
 
 	interval->interval.numerator = 1;
-	interval->interval.denominator = gc0310_res[dev->fmt_idx].fps;
+	interval->interval.denominator = dev->res->fps;
 
 	return 0;
 }
@@ -1237,7 +1160,7 @@ static int gc0310_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
 	struct gc0310_device *dev = to_gc0310_sensor(sd);
 
 	mutex_lock(&dev->input_lock);
-	*frames = gc0310_res[dev->fmt_idx].skip_frames;
+	*frames = dev->res->skip_frames;
 	mutex_unlock(&dev->input_lock);
 
 	return 0;
@@ -1301,7 +1224,7 @@ static int gc0310_probe(struct i2c_client *client)
 
 	mutex_init(&dev->input_lock);
 
-	dev->fmt_idx = 0;
+	dev->res = &gc0310_res_preview[0];
 	v4l2_i2c_subdev_init(&dev->sd, client, &gc0310_ops);
 
 	pdata = gmin_camera_platform_data(&dev->sd,
diff --git a/drivers/staging/media/atomisp/i2c/gc0310.h b/drivers/staging/media/atomisp/i2c/gc0310.h
index 2fe3de115083..db643ebc3909 100644
--- a/drivers/staging/media/atomisp/i2c/gc0310.h
+++ b/drivers/staging/media/atomisp/i2c/gc0310.h
@@ -150,8 +150,7 @@ struct gc0310_device {
 
 	struct camera_sensor_platform_data *platform_data;
 	int vt_pix_clk_freq_mhz;
-	int fmt_idx;
-	u8 res;
+	struct gc0310_resolution *res;
 	u8 type;
 };
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/4] media: atomisp-gc2235: use v4l2_find_nearest_size()
  2021-11-04 19:02 [PATCH 1/4] media: atomisp-ov2722: use v4l2_find_nearest_size() Mauro Carvalho Chehab
  2021-11-04 19:02 ` [PATCH 2/4] media: atomisp-gc0310: " Mauro Carvalho Chehab
@ 2021-11-04 19:02 ` Mauro Carvalho Chehab
  2021-11-04 19:02 ` [PATCH 4/4] media: atomisp-mt9m114: " Mauro Carvalho Chehab
  2 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-04 19:02 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab,
	Beatriz Martins de Carvalho, Bhaskar Chowdhury, Deepak R Varma,
	Greg Kroah-Hartman, Hans Verkuil, Mauro Carvalho Chehab,
	Randy Dunlap, Sakari Ailus, Tom Rix, Tomi Valkeinen,
	linux-kernel, linux-media, linux-staging

Instead of reinventing the wheel, use v4l2_find_nearest_size()
in order to get the closest resolution.

This should address a bug where the wrong resolution was
selected.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../media/atomisp/i2c/atomisp-gc2235.c        | 113 +++---------------
 drivers/staging/media/atomisp/i2c/gc2235.h    |   3 +-
 2 files changed, 20 insertions(+), 96 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
index 4d769590f2d3..5e7085264189 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
@@ -670,76 +670,6 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on)
 	return ret;
 }
 
-/*
- * distance - calculate the distance
- * @res: resolution
- * @w: width
- * @h: height
- *
- * Get the gap between resolution and w/h.
- * res->width/height smaller than w/h wouldn't be considered.
- * Returns the value of gap or -1 if fail.
- */
-#define LARGEST_ALLOWED_RATIO_MISMATCH 800
-static int distance(struct gc2235_resolution *res, u32 w, u32 h)
-{
-	unsigned int w_ratio = (res->width << 13) / w;
-	unsigned int h_ratio;
-	int match;
-
-	if (h == 0)
-		return -1;
-	h_ratio = (res->height << 13) / h;
-	if (h_ratio == 0)
-		return -1;
-	match   = abs(((w_ratio << 13) / h_ratio) - 8192);
-
-	if ((w_ratio < 8192) || (h_ratio < 8192) ||
-	    (match > LARGEST_ALLOWED_RATIO_MISMATCH))
-		return -1;
-
-	return w_ratio + h_ratio;
-}
-
-/* Return the nearest higher resolution index */
-static int nearest_resolution_index(int w, int h)
-{
-	int i;
-	int idx = -1;
-	int dist;
-	int min_dist = INT_MAX;
-	struct gc2235_resolution *tmp_res = NULL;
-
-	for (i = 0; i < N_RES; i++) {
-		tmp_res = &gc2235_res[i];
-		dist = distance(tmp_res, w, h);
-		if (dist == -1)
-			continue;
-		if (dist < min_dist) {
-			min_dist = dist;
-			idx = i;
-		}
-	}
-
-	return idx;
-}
-
-static int get_resolution_index(int w, int h)
-{
-	int i;
-
-	for (i = 0; i < N_RES; i++) {
-		if (w != gc2235_res[i].width)
-			continue;
-		if (h != gc2235_res[i].height)
-			continue;
-
-		return i;
-	}
-
-	return -1;
-}
-
 static int startup(struct v4l2_subdev *sd)
 {
 	struct gc2235_device *dev = to_gc2235_sensor(sd);
@@ -758,7 +688,7 @@ static int startup(struct v4l2_subdev *sd)
 		gc2235_write_reg_array(client, gc2235_init_settings);
 	}
 
-	ret = gc2235_write_reg_array(client, gc2235_res[dev->fmt_idx].regs);
+	ret = gc2235_write_reg_array(client, dev->res->regs);
 	if (ret) {
 		dev_err(&client->dev, "gc2235 write register err.\n");
 		return ret;
@@ -776,8 +706,8 @@ static int gc2235_set_fmt(struct v4l2_subdev *sd,
 	struct gc2235_device *dev = to_gc2235_sensor(sd);
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct camera_mipi_info *gc2235_info = NULL;
+	struct gc2235_resolution *res;
 	int ret = 0;
-	int idx;
 
 	gc2235_info = v4l2_get_subdev_hostdata(sd);
 	if (!gc2235_info)
@@ -786,16 +716,18 @@ static int gc2235_set_fmt(struct v4l2_subdev *sd,
 		return -EINVAL;
 	if (!fmt)
 		return -EINVAL;
+
 	mutex_lock(&dev->input_lock);
-	idx = nearest_resolution_index(fmt->width, fmt->height);
-	if (idx == -1) {
-		/* return the largest resolution */
-		fmt->width = gc2235_res[N_RES - 1].width;
-		fmt->height = gc2235_res[N_RES - 1].height;
-	} else {
-		fmt->width = gc2235_res[idx].width;
-		fmt->height = gc2235_res[idx].height;
-	}
+	res = v4l2_find_nearest_size(gc2235_res_preview,
+				     ARRAY_SIZE(gc2235_res_preview), width,
+				     height, fmt->width, fmt->height);
+	if (!res)
+		res = &gc2235_res_preview[N_RES - 1];
+
+	fmt->width = res->width;
+	fmt->height = res->height;
+	dev->res = res;
+
 	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
 	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
 		sd_state->pads->try_fmt = *fmt;
@@ -803,13 +735,6 @@ static int gc2235_set_fmt(struct v4l2_subdev *sd,
 		return 0;
 	}
 
-	dev->fmt_idx = get_resolution_index(fmt->width, fmt->height);
-	if (dev->fmt_idx == -1) {
-		dev_err(&client->dev, "get resolution fail\n");
-		mutex_unlock(&dev->input_lock);
-		return -EINVAL;
-	}
-
 	ret = startup(sd);
 	if (ret) {
 		dev_err(&client->dev, "gc2235 startup err\n");
@@ -817,7 +742,7 @@ static int gc2235_set_fmt(struct v4l2_subdev *sd,
 	}
 
 	ret = gc2235_get_intg_factor(client, gc2235_info,
-				     &gc2235_res[dev->fmt_idx]);
+				     dev->res);
 	if (ret)
 		dev_err(&client->dev, "failed to get integration_factor\n");
 
@@ -839,8 +764,8 @@ static int gc2235_get_fmt(struct v4l2_subdev *sd,
 	if (!fmt)
 		return -EINVAL;
 
-	fmt->width = gc2235_res[dev->fmt_idx].width;
-	fmt->height = gc2235_res[dev->fmt_idx].height;
+	fmt->width = dev->res->width;
+	fmt->height = dev->res->height;
 	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
 
 	return 0;
@@ -953,7 +878,7 @@ static int gc2235_g_frame_interval(struct v4l2_subdev *sd,
 	struct gc2235_device *dev = to_gc2235_sensor(sd);
 
 	interval->interval.numerator = 1;
-	interval->interval.denominator = gc2235_res[dev->fmt_idx].fps;
+	interval->interval.denominator = dev->res->fps;
 
 	return 0;
 }
@@ -991,7 +916,7 @@ static int gc2235_g_skip_frames(struct v4l2_subdev *sd, u32 *frames)
 	struct gc2235_device *dev = to_gc2235_sensor(sd);
 
 	mutex_lock(&dev->input_lock);
-	*frames = gc2235_res[dev->fmt_idx].skip_frames;
+	*frames = dev->res->skip_frames;
 	mutex_unlock(&dev->input_lock);
 
 	return 0;
@@ -1055,7 +980,7 @@ static int gc2235_probe(struct i2c_client *client)
 
 	mutex_init(&dev->input_lock);
 
-	dev->fmt_idx = 0;
+	dev->res = &gc2235_res_preview[0];
 	v4l2_i2c_subdev_init(&dev->sd, client, &gc2235_ops);
 
 	gcpdev = gmin_camera_platform_data(&dev->sd,
diff --git a/drivers/staging/media/atomisp/i2c/gc2235.h b/drivers/staging/media/atomisp/i2c/gc2235.h
index ba5db1230033..806be5dff7a5 100644
--- a/drivers/staging/media/atomisp/i2c/gc2235.h
+++ b/drivers/staging/media/atomisp/i2c/gc2235.h
@@ -158,11 +158,10 @@ struct gc2235_device {
 	struct v4l2_mbus_framefmt format;
 	struct mutex input_lock;
 	struct v4l2_ctrl_handler ctrl_handler;
+	struct gc2235_resolution *res;
 
 	struct camera_sensor_platform_data *platform_data;
 	int vt_pix_clk_freq_mhz;
-	int fmt_idx;
-	u8 res;
 	u8 type;
 };
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 4/4] media: atomisp-mt9m114: use v4l2_find_nearest_size()
  2021-11-04 19:02 [PATCH 1/4] media: atomisp-ov2722: use v4l2_find_nearest_size() Mauro Carvalho Chehab
  2021-11-04 19:02 ` [PATCH 2/4] media: atomisp-gc0310: " Mauro Carvalho Chehab
  2021-11-04 19:02 ` [PATCH 3/4] media: atomisp-gc2235: " Mauro Carvalho Chehab
@ 2021-11-04 19:02 ` Mauro Carvalho Chehab
  2 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2021-11-04 19:02 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Deepak R Varma,
	Greg Kroah-Hartman, Hans Verkuil, Martiros Shakhzadyan,
	Mauro Carvalho Chehab, Sakari Ailus, Tomi Valkeinen, Yizhuo,
	linux-kernel, linux-media, linux-staging

Instead of reinventing the wheel, use v4l2_find_nearest_size()
in order to get the closest resolution.

This should address a bug where the wrong resolution was
selected.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../media/atomisp/i2c/atomisp-mt9m114.c       | 130 ++----------------
 1 file changed, 15 insertions(+), 115 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
index 49f4090856d3..00d6842c07d6 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
@@ -579,107 +579,6 @@ static int mt9m114_s_power(struct v4l2_subdev *sd, int power)
 	return mt9m114_init_common(sd);
 }
 
-/*
- * distance - calculate the distance
- * @res: resolution
- * @w: width
- * @h: height
- *
- * Get the gap between resolution and w/h.
- * res->width/height smaller than w/h wouldn't be considered.
- * Returns the value of gap or -1 if fail.
- */
-#define LARGEST_ALLOWED_RATIO_MISMATCH 600
-static int distance(struct mt9m114_res_struct const *res, u32 w, u32 h)
-{
-	unsigned int w_ratio;
-	unsigned int h_ratio;
-	int match;
-
-	if (w == 0)
-		return -1;
-	w_ratio = (res->width << 13) / w;
-	if (h == 0)
-		return -1;
-	h_ratio = (res->height << 13) / h;
-	if (h_ratio == 0)
-		return -1;
-	match   = abs(((w_ratio << 13) / h_ratio) - 8192);
-
-	if ((w_ratio < 8192) || (h_ratio < 8192) ||
-	    (match > LARGEST_ALLOWED_RATIO_MISMATCH))
-		return -1;
-
-	return w_ratio + h_ratio;
-}
-
-/* Return the nearest higher resolution index */
-static int nearest_resolution_index(int w, int h)
-{
-	int i;
-	int idx = -1;
-	int dist;
-	int min_dist = INT_MAX;
-	const struct mt9m114_res_struct *tmp_res = NULL;
-
-	for (i = 0; i < ARRAY_SIZE(mt9m114_res); i++) {
-		tmp_res = &mt9m114_res[i];
-		dist = distance(tmp_res, w, h);
-		if (dist == -1)
-			continue;
-		if (dist < min_dist) {
-			min_dist = dist;
-			idx = i;
-		}
-	}
-
-	return idx;
-}
-
-static int mt9m114_try_res(u32 *w, u32 *h)
-{
-	int idx = 0;
-
-	if ((*w > MT9M114_RES_960P_SIZE_H)
-	    || (*h > MT9M114_RES_960P_SIZE_V)) {
-		*w = MT9M114_RES_960P_SIZE_H;
-		*h = MT9M114_RES_960P_SIZE_V;
-	} else {
-		idx = nearest_resolution_index(*w, *h);
-
-		/*
-		 * nearest_resolution_index() doesn't return smaller
-		 *  resolutions. If it fails, it means the requested
-		 *  resolution is higher than wecan support. Fallback
-		 *  to highest possible resolution in this case.
-		 */
-		if (idx == -1)
-			idx = ARRAY_SIZE(mt9m114_res) - 1;
-
-		*w = mt9m114_res[idx].width;
-		*h = mt9m114_res[idx].height;
-	}
-
-	return 0;
-}
-
-static struct mt9m114_res_struct *mt9m114_to_res(u32 w, u32 h)
-{
-	int  index;
-
-	for (index = 0; index < N_RES; index++) {
-		if ((mt9m114_res[index].width == w) &&
-		    (mt9m114_res[index].height == h))
-			break;
-	}
-
-	/* No mode found */
-	if (index >= N_RES)
-		return NULL;
-
-	return &mt9m114_res[index];
-}
-
 static int mt9m114_res2size(struct v4l2_subdev *sd, int *h_size, int *v_size)
 {
 	struct mt9m114_device *dev = to_mt9m114_sensor(sd);
@@ -829,7 +728,7 @@ static int mt9m114_set_fmt(struct v4l2_subdev *sd,
 	struct v4l2_mbus_framefmt *fmt = &format->format;
 	struct i2c_client *c = v4l2_get_subdevdata(sd);
 	struct mt9m114_device *dev = to_mt9m114_sensor(sd);
-	struct mt9m114_res_struct *res_index;
+	struct mt9m114_res_struct *res;
 	u32 width = fmt->width;
 	u32 height = fmt->height;
 	struct camera_mipi_info *mt9m114_info = NULL;
@@ -845,20 +744,21 @@ static int mt9m114_set_fmt(struct v4l2_subdev *sd,
 	if (!mt9m114_info)
 		return -EINVAL;
 
-	mt9m114_try_res(&width, &height);
+	res = v4l2_find_nearest_size(mt9m114_res,
+				     ARRAY_SIZE(mt9m114_res), width,
+				     height, fmt->width, fmt->height);
+	if (!res)
+		res = &mt9m114_res[N_RES - 1];
+
+	fmt->width = res->width;
+	fmt->height = res->height;
+
 	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
 		sd_state->pads->try_fmt = *fmt;
 		return 0;
 	}
-	res_index = mt9m114_to_res(width, height);
 
-	/* Sanity check */
-	if (unlikely(!res_index)) {
-		WARN_ON(1);
-		return -EINVAL;
-	}
-
-	switch (res_index->res) {
+	switch (res->res) {
 	case MT9M114_RES_736P:
 		ret = mt9m114_write_reg_array(c, mt9m114_736P_init, NO_POLLING);
 		ret += misensor_rmw_reg(c, MISENSOR_16BIT, MISENSOR_READ_MODE,
@@ -876,7 +776,7 @@ static int mt9m114_set_fmt(struct v4l2_subdev *sd,
 					MISENSOR_R_MODE_MASK, MISENSOR_NORMAL_SET);
 		break;
 	default:
-		v4l2_err(sd, "set resolution: %d failed!\n", res_index->res);
+		v4l2_err(sd, "set resolution: %d failed!\n", res->res);
 		return -EINVAL;
 	}
 
@@ -890,7 +790,7 @@ static int mt9m114_set_fmt(struct v4l2_subdev *sd,
 	if (mt9m114_set_suspend(sd))
 		return -EINVAL;
 
-	if (dev->res != res_index->res) {
+	if (dev->res != res->res) {
 		int index;
 
 		/* Switch to different size */
@@ -922,7 +822,7 @@ static int mt9m114_set_fmt(struct v4l2_subdev *sd,
 		}
 	}
 	ret = mt9m114_get_intg_factor(c, mt9m114_info,
-				      &mt9m114_res[res_index->res]);
+				      &mt9m114_res[res->res]);
 	if (ret) {
 		dev_err(&c->dev, "failed to get integration_factor\n");
 		return -EINVAL;
@@ -931,7 +831,7 @@ static int mt9m114_set_fmt(struct v4l2_subdev *sd,
 	 * mt9m114 - we don't poll for context switch
 	 * because it does not happen with streaming disabled.
 	 */
-	dev->res = res_index->res;
+	dev->res = res->res;
 
 	fmt->width = width;
 	fmt->height = height;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-04 19:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 19:02 [PATCH 1/4] media: atomisp-ov2722: use v4l2_find_nearest_size() Mauro Carvalho Chehab
2021-11-04 19:02 ` [PATCH 2/4] media: atomisp-gc0310: " Mauro Carvalho Chehab
2021-11-04 19:02 ` [PATCH 3/4] media: atomisp-gc2235: " Mauro Carvalho Chehab
2021-11-04 19:02 ` [PATCH 4/4] media: atomisp-mt9m114: " Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).