[PATCH] imx274: remove composition support, add V4L2_SEL_TGT_CROP_DEFAULT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This driver does not support composition, only cropping.
Composition means that the sensor can output e.g. 1920x1080,
but can compose a cropped 1280x720 image in the middle of the
1920x1080 canvas, filling in the unused area with a background
color.

That's not supported at all. So drop the bogus composition support.

Support for V4L2_SEL_TGT_CROP_DEFAULT was missing in imx274_get_selection,
which meant that VIDIOC_CROPCAP couldn't be emulated and that caused a
v4l2-compliance failure. Add support for this target to fix this.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
---
This patch has been in my queue for almost half a year. I thought I had
posted it before, but apparently not. Apologies for that.
---
 drivers/media/i2c/imx274.c | 125 +++++++++++--------------------------
 1 file changed, 36 insertions(+), 89 deletions(-)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index 54642d5f2d5b..abb2f8d4895f 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -980,11 +980,11 @@ static int imx274_binning_goodness(struct stimx274 *imx274,
 }

 /**
- * Helper function to change binning and set both compose and format.
+ * Helper function to change binning and set both crop and format.
  *
  * We have two entry points to change binning: set_fmt and
- * set_selection(COMPOSE). Both have to compute the new output size
- * and set it in both the compose rect and the frame format size. We
+ * set_selection(CROP). Both have to compute the new output size
+ * and set it in both the crop rect and the frame format size. We
  * also need to do the same things after setting cropping to restore
  * 1:1 binning.
  *
@@ -1003,12 +1003,12 @@ static int imx274_binning_goodness(struct stimx274 *imx274,
  * @flags:  Selection flags from struct v4l2_subdev_selection, or 0 if not
  *          available (when called from set_fmt)
  */
-static int __imx274_change_compose(struct stimx274 *imx274,
-				   struct v4l2_subdev_pad_config *cfg,
-				   u32 which,
-				   u32 *width,
-				   u32 *height,
-				   u32 flags)
+static int __imx274_change_crop_fmt(struct stimx274 *imx274,
+				    struct v4l2_subdev_pad_config *cfg,
+				    u32 which,
+				    u32 *width,
+				    u32 *height,
+				    u32 flags)
 {
 	struct device *dev = &imx274->client->dev;
 	const struct v4l2_rect *cur_crop;
@@ -1099,14 +1099,14 @@ static int imx274_set_fmt(struct v4l2_subdev *sd,

 	mutex_lock(&imx274->lock);

-	err = __imx274_change_compose(imx274, cfg, format->which,
-				      &fmt->width, &fmt->height, 0);
+	err = __imx274_change_crop_fmt(imx274, cfg, format->which,
+				       &fmt->width, &fmt->height, 0);

 	if (err)
 		goto out;

 	/*
-	 * __imx274_change_compose already set width and height in the
+	 * __imx274_change_crop_fmt already set width and height in the
 	 * applicable format, but we need to keep all other format
 	 * values, so do a full copy here
 	 */
@@ -1127,14 +1127,12 @@ static int imx274_get_selection(struct v4l2_subdev *sd,
 				struct v4l2_subdev_selection *sel)
 {
 	struct stimx274 *imx274 = to_imx274(sd);
-	const struct v4l2_rect *src_crop;
-	const struct v4l2_mbus_framefmt *src_fmt;
-	int ret = 0;

 	if (sel->pad != 0)
 		return -EINVAL;

-	if (sel->target == V4L2_SEL_TGT_CROP_BOUNDS) {
+	if (sel->target == V4L2_SEL_TGT_CROP_BOUNDS ||
+	    sel->target == V4L2_SEL_TGT_CROP_DEFAULT) {
 		sel->r.left = 0;
 		sel->r.top = 0;
 		sel->r.width = IMX274_MAX_WIDTH;
@@ -1142,57 +1140,42 @@ static int imx274_get_selection(struct v4l2_subdev *sd,
 		return 0;
 	}

-	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
-		src_crop = &cfg->try_crop;
-		src_fmt = &cfg->try_fmt;
-	} else {
-		src_crop = &imx274->crop;
-		src_fmt = &imx274->format;
-	}
+	if (sel->target != V4L2_SEL_TGT_CROP)
+		return -EINVAL;

 	mutex_lock(&imx274->lock);
-
-	switch (sel->target) {
-	case V4L2_SEL_TGT_CROP:
-		sel->r = *src_crop;
-		break;
-	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
-		sel->r.top = 0;
-		sel->r.left = 0;
-		sel->r.width = src_crop->width;
-		sel->r.height = src_crop->height;
-		break;
-	case V4L2_SEL_TGT_COMPOSE:
-		sel->r.top = 0;
-		sel->r.left = 0;
-		sel->r.width = src_fmt->width;
-		sel->r.height = src_fmt->height;
-		break;
-	default:
-		ret = -EINVAL;
-	}
+	if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
+		sel->r = cfg->try_crop;
+	else
+		sel->r = imx274->crop;

 	mutex_unlock(&imx274->lock);

-	return ret;
+	return 0;
 }

-static int imx274_set_selection_crop(struct stimx274 *imx274,
-				     struct v4l2_subdev_pad_config *cfg,
-				     struct v4l2_subdev_selection *sel)
+static int imx274_set_selection(struct v4l2_subdev *sd,
+				struct v4l2_subdev_pad_config *cfg,
+				struct v4l2_subdev_selection *sel)
 {
+	struct stimx274 *imx274 = to_imx274(sd);
 	struct v4l2_rect *tgt_crop;
 	struct v4l2_rect new_crop;
 	bool size_changed;
-
 	/*
 	 * h_step could be 12 or 24 depending on the binning. But we
 	 * won't know the binning until we choose the mode later in
-	 * __imx274_change_compose(). Thus let's be safe and use the
+	 * __imx274_change_crop_fmt(). Thus let's be safe and use the
 	 * most conservative value in all cases.
 	 */
 	const u32 h_step = 24;

+	if (sel->pad != 0)
+		return -EINVAL;
+
+	if (sel->target != V4L2_SEL_TGT_CROP)
+		return -EINVAL;
+
 	new_crop.width = min_t(u32,
 			       IMX274_ROUND(sel->r.width, h_step, sel->flags),
 			       IMX274_MAX_WIDTH);
@@ -1224,56 +1207,20 @@ static int imx274_set_selection_crop(struct stimx274 *imx274,
 	size_changed = (new_crop.width != tgt_crop->width ||
 			new_crop.height != tgt_crop->height);

-	/* __imx274_change_compose needs the new size in *tgt_crop */
+	/* __imx274_change_crop_fmt needs the new size in *tgt_crop */
 	*tgt_crop = new_crop;

 	/* if crop size changed then reset the output image size */
 	if (size_changed)
-		__imx274_change_compose(imx274, cfg, sel->which,
-					&new_crop.width, &new_crop.height,
-					sel->flags);
+		__imx274_change_crop_fmt(imx274, cfg, sel->which,
+					 &new_crop.width, &new_crop.height,
+					 sel->flags);

 	mutex_unlock(&imx274->lock);

 	return 0;
 }

-static int imx274_set_selection(struct v4l2_subdev *sd,
-				struct v4l2_subdev_pad_config *cfg,
-				struct v4l2_subdev_selection *sel)
-{
-	struct stimx274 *imx274 = to_imx274(sd);
-
-	if (sel->pad != 0)
-		return -EINVAL;
-
-	if (sel->target == V4L2_SEL_TGT_CROP)
-		return imx274_set_selection_crop(imx274, cfg, sel);
-
-	if (sel->target == V4L2_SEL_TGT_COMPOSE) {
-		int err;
-
-		mutex_lock(&imx274->lock);
-		err =  __imx274_change_compose(imx274, cfg, sel->which,
-					       &sel->r.width, &sel->r.height,
-					       sel->flags);
-		mutex_unlock(&imx274->lock);
-
-		/*
-		 * __imx274_change_compose already set width and
-		 * height in set->r, we still need to set top-left
-		 */
-		if (!err) {
-			sel->r.top = 0;
-			sel->r.left = 0;
-		}
-
-		return err;
-	}
-
-	return -EINVAL;
-}
-
 static int imx274_apply_trimming(struct stimx274 *imx274)
 {
 	u32 h_start;
-- 
2.29.2




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux