path: root/target/linux/bcm27xx/patches-6.1/950-0641-media-i2c-imx290-Use-V4L2-subdev-active-state.patch

From 94caa6139edcfebd8934f362f562eac56b85b610 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Mon, 17 Oct 2022 12:44:27 +0200
Subject: [PATCH] media: i2c: imx290: Use V4L2 subdev active state

Upstream commit a2514b9a634a

Use the V4L2 subdev active state API to store the active format. This
simplifies the driver not only by dropping the imx290 current_format
field, but it also allows dropping the imx290 lock, replaced with the
state lock.

The lock check in imx290_ctrl_update() can be dropped as
imx290_set_fmt() can't be called anywmore with which set to ACTIVE
before controls are initialized.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 drivers/media/i2c/imx290.c | 154 ++++++++++++++++---------------------
 1 file changed, 65 insertions(+), 89 deletions(-)

--- a/drivers/media/i2c/imx290.c
+++ b/drivers/media/i2c/imx290.c
@@ -177,12 +177,12 @@ struct imx290 {
 	struct clk *xclk;
 	struct regmap *regmap;
 	u8 nlanes;
-	u8 bpp;
 
 	struct v4l2_subdev sd;
 	struct media_pad pad;
-	struct v4l2_mbus_framefmt current_format;
+
 	const struct imx290_mode *current_mode;
+	u8 bpp;
 
 	struct regulator_bulk_data supplies[IMX290_NUM_SUPPLIES];
 	struct gpio_desc *rst_gpio;
@@ -192,8 +192,6 @@ struct imx290 {
 	struct v4l2_ctrl *pixel_rate;
 	struct v4l2_ctrl *hblank;
 	struct v4l2_ctrl *vblank;
-
-	struct mutex lock;
 };
 
 static inline struct imx290 *to_imx290(struct v4l2_subdev *_sd)
@@ -524,13 +522,14 @@ static int imx290_set_black_level(struct
 			    black_level >> (16 - imx290->bpp), err);
 }
 
-static int imx290_write_current_format(struct imx290 *imx290)
+static int imx290_setup_format(struct imx290 *imx290,
+			       const struct v4l2_mbus_framefmt *format)
 {
 	const struct imx290_regval *regs;
 	unsigned int num_regs;
 	int ret;
 
-	switch (imx290->current_format.code) {
+	switch (format->code) {
 	case MEDIA_BUS_FMT_SRGGB10_1X10:
 		regs = imx290_10bit_settings;
 		num_regs = ARRAY_SIZE(imx290_10bit_settings);
@@ -563,6 +562,15 @@ static int imx290_set_ctrl(struct v4l2_c
 					     struct imx290, ctrls);
 	int ret = 0;
 
+	/*
+	 * Return immediately for controls that don't need to be applied to the
+	 * device. Those controls are modified in imx290_ctrl_update(), which
+	 * is called at probe time before runtime PM is initialized, so we
+	 * can't proceed to the pm_runtime_get_if_in_use() call below.
+	 */
+	if (ctrl->flags & V4L2_CTRL_FLAG_READ_ONLY)
+		return 0;
+
 	/* V4L2 controls values will be applied only when power is already up */
 	if (!pm_runtime_get_if_in_use(imx290->dev))
 		return 0;
@@ -592,6 +600,7 @@ static int imx290_set_ctrl(struct v4l2_c
 					       &ret);
 		}
 		break;
+
 	default:
 		ret = -EINVAL;
 		break;
@@ -625,13 +634,6 @@ static void imx290_ctrl_update(struct im
 	s64 link_freq = imx290_link_freqs_ptr(imx290)[mode->link_freq_index];
 	u64 pixel_rate;
 
-	/*
-	 * This function may be called from imx290_set_fmt() before controls
-	 * get created by imx290_ctrl_init(). Return immediately in that case.
-	 */
-	if (!imx290->ctrls.lock)
-		return;
-
 	/* pixel rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
 	pixel_rate = link_freq * 2 * imx290->nlanes;
 	do_div(pixel_rate, imx290->bpp);
@@ -653,7 +655,6 @@ static int imx290_ctrl_init(struct imx29
 		return ret;
 
 	v4l2_ctrl_handler_init(&imx290->ctrls, 9);
-	imx290->ctrls.lock = &imx290->lock;
 
 	/*
 	 * The sensor has an analog gain and a digital gain, both controlled
@@ -718,10 +719,6 @@ static int imx290_ctrl_init(struct imx29
 		return ret;
 	}
 
-	mutex_lock(imx290->ctrls.lock);
-	imx290_ctrl_update(imx290, imx290->current_mode);
-	mutex_unlock(imx290->ctrls.lock);
-
 	return 0;
 }
 
@@ -730,8 +727,10 @@ static int imx290_ctrl_init(struct imx29
  */
 
 /* Start streaming */
-static int imx290_start_streaming(struct imx290 *imx290)
+static int imx290_start_streaming(struct imx290 *imx290,
+				  struct v4l2_subdev_state *state)
 {
+	const struct v4l2_mbus_framefmt *format;
 	int ret;
 
 	/* Set init register settings */
@@ -744,7 +743,8 @@ static int imx290_start_streaming(struct
 	}
 
 	/* Apply the register values related to current frame format */
-	ret = imx290_write_current_format(imx290);
+	format = v4l2_subdev_get_pad_format(&imx290->sd, state, 0);
+	ret = imx290_setup_format(imx290, format);
 	if (ret < 0) {
 		dev_err(imx290->dev, "Could not set frame format\n");
 		return ret;
@@ -764,7 +764,7 @@ static int imx290_start_streaming(struct
 		return ret;
 
 	/* Apply customized values from user */
-	ret = v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
+	ret = __v4l2_ctrl_handler_setup(imx290->sd.ctrl_handler);
 	if (ret) {
 		dev_err(imx290->dev, "Could not sync v4l2 controls\n");
 		return ret;
@@ -793,39 +793,32 @@ static int imx290_stop_streaming(struct
 static int imx290_set_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct imx290 *imx290 = to_imx290(sd);
+	struct v4l2_subdev_state *state;
 	int ret = 0;
 
+	state = v4l2_subdev_lock_and_get_active_state(sd);
+
 	if (enable) {
 		ret = pm_runtime_resume_and_get(imx290->dev);
 		if (ret < 0)
-			goto unlock_and_return;
+			goto unlock;
 
-		ret = imx290_start_streaming(imx290);
+		ret = imx290_start_streaming(imx290, state);
 		if (ret) {
 			dev_err(imx290->dev, "Start stream failed\n");
 			pm_runtime_put(imx290->dev);
-			goto unlock_and_return;
+			goto unlock;
 		}
 	} else {
 		imx290_stop_streaming(imx290);
 		pm_runtime_put(imx290->dev);
 	}
 
-unlock_and_return:
-
+unlock:
+	v4l2_subdev_unlock_state(state);
 	return ret;
 }
 
-static struct v4l2_mbus_framefmt *
-imx290_get_pad_format(struct imx290 *imx290, struct v4l2_subdev_state *state,
-		      u32 which)
-{
-	if (which == V4L2_SUBDEV_FORMAT_ACTIVE)
-		return &imx290->current_format;
-	else
-		return v4l2_subdev_get_try_format(&imx290->sd, state, 0);
-}
-
 static int imx290_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_state *sd_state,
 				 struct v4l2_subdev_mbus_code_enum *code)
@@ -860,23 +853,6 @@ static int imx290_enum_frame_size(struct
 	return 0;
 }
 
-static int imx290_get_fmt(struct v4l2_subdev *sd,
-			  struct v4l2_subdev_state *sd_state,
-			  struct v4l2_subdev_format *fmt)
-{
-	struct imx290 *imx290 = to_imx290(sd);
-	struct v4l2_mbus_framefmt *framefmt;
-
-	mutex_lock(&imx290->lock);
-
-	framefmt = imx290_get_pad_format(imx290, sd_state, fmt->which);
-	fmt->format = *framefmt;
-
-	mutex_unlock(&imx290->lock);
-
-	return 0;
-}
-
 static int imx290_set_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_state *sd_state,
 			  struct v4l2_subdev_format *fmt)
@@ -886,8 +862,6 @@ static int imx290_set_fmt(struct v4l2_su
 	struct v4l2_mbus_framefmt *format;
 	unsigned int i;
 
-	mutex_lock(&imx290->lock);
-
 	mode = v4l2_find_nearest_size(imx290_modes_ptr(imx290),
 				      imx290_modes_num(imx290), width, height,
 				      fmt->format.width, fmt->format.height);
@@ -905,7 +879,7 @@ static int imx290_set_fmt(struct v4l2_su
 	fmt->format.code = imx290_formats[i].code;
 	fmt->format.field = V4L2_FIELD_NONE;
 
-	format = imx290_get_pad_format(imx290, sd_state, fmt->which);
+	format = v4l2_subdev_get_pad_format(sd, sd_state, 0);
 
 	if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
 		imx290->current_mode = mode;
@@ -916,8 +890,6 @@ static int imx290_set_fmt(struct v4l2_su
 
 	*format = fmt->format;
 
-	mutex_unlock(&imx290->lock);
-
 	return 0;
 }
 
@@ -925,14 +897,11 @@ static int imx290_get_selection(struct v
 				struct v4l2_subdev_state *sd_state,
 				struct v4l2_subdev_selection *sel)
 {
-	struct imx290 *imx290 = to_imx290(sd);
 	struct v4l2_mbus_framefmt *format;
 
 	switch (sel->target) {
 	case V4L2_SEL_TGT_CROP: {
-		format = imx290_get_pad_format(imx290, sd_state, sel->which);
-
-		mutex_lock(&imx290->lock);
+		format = v4l2_subdev_get_pad_format(sd, sd_state, 0);
 
 		sel->r.top = IMX920_PIXEL_ARRAY_MARGIN_TOP
 			   + (IMX290_PIXEL_ARRAY_RECORDING_HEIGHT - format->height) / 2;
@@ -941,7 +910,6 @@ static int imx290_get_selection(struct v
 		sel->r.width = format->width;
 		sel->r.height = format->height;
 
-		mutex_unlock(&imx290->lock);
 		return 0;
 	}
 
@@ -970,11 +938,13 @@ static int imx290_get_selection(struct v
 static int imx290_entity_init_cfg(struct v4l2_subdev *subdev,
 				  struct v4l2_subdev_state *sd_state)
 {
-	struct v4l2_subdev_format fmt = { 0 };
-
-	fmt.which = sd_state ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
-	fmt.format.width = 1920;
-	fmt.format.height = 1080;
+	struct v4l2_subdev_format fmt = {
+		.which = V4L2_SUBDEV_FORMAT_TRY,
+		.format = {
+			.width = 1920,
+			.height = 1080,
+		},
+	};
 
 	imx290_set_fmt(subdev, sd_state, &fmt);
 
@@ -989,7 +959,7 @@ static const struct v4l2_subdev_pad_ops
 	.init_cfg = imx290_entity_init_cfg,
 	.enum_mbus_code = imx290_enum_mbus_code,
 	.enum_frame_size = imx290_enum_frame_size,
-	.get_fmt = imx290_get_fmt,
+	.get_fmt = v4l2_subdev_get_fmt,
 	.set_fmt = imx290_set_fmt,
 	.get_selection = imx290_get_selection,
 };
@@ -1008,18 +978,8 @@ static int imx290_subdev_init(struct imx
 	struct i2c_client *client = to_i2c_client(imx290->dev);
 	int ret;
 
-	/*
-	 * Initialize the frame format. In particular, imx290->current_mode
-	 * and imx290->bpp are set to defaults: imx290_calc_pixel_rate() call
-	 * below relies on these fields.
-	 */
-	imx290_entity_init_cfg(&imx290->sd, NULL);
-
-	ret = imx290_ctrl_init(imx290);
-	if (ret < 0) {
-		dev_err(imx290->dev, "Control initialization error %d\n", ret);
-		return ret;
-	}
+	imx290->current_mode = &imx290_modes_ptr(imx290)[0];
+	imx290->bpp = imx290_formats[0].bpp;
 
 	v4l2_i2c_subdev_init(&imx290->sd, client, &imx290_subdev_ops);
 	imx290->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
@@ -1031,15 +991,37 @@ static int imx290_subdev_init(struct imx
 	ret = media_entity_pads_init(&imx290->sd.entity, 1, &imx290->pad);
 	if (ret < 0) {
 		dev_err(imx290->dev, "Could not register media entity\n");
-		v4l2_ctrl_handler_free(&imx290->ctrls);
 		return ret;
 	}
 
+	ret = imx290_ctrl_init(imx290);
+	if (ret < 0) {
+		dev_err(imx290->dev, "Control initialization error %d\n", ret);
+		goto err_media;
+	}
+
+	imx290->sd.state_lock = imx290->ctrls.lock;
+
+	ret = v4l2_subdev_init_finalize(&imx290->sd);
+	if (ret < 0) {
+		dev_err(imx290->dev, "subdev initialization error %d\n", ret);
+		goto err_ctrls;
+	}
+
+	imx290_ctrl_update(imx290, imx290->current_mode);
+
 	return 0;
+
+err_ctrls:
+	v4l2_ctrl_handler_free(&imx290->ctrls);
+err_media:
+	media_entity_cleanup(&imx290->sd.entity);
+	return ret;
 }
 
 static void imx290_subdev_cleanup(struct imx290 *imx290)
 {
+	v4l2_subdev_cleanup(&imx290->sd);
 	media_entity_cleanup(&imx290->sd.entity);
 	v4l2_ctrl_handler_free(&imx290->ctrls);
 }
@@ -1270,12 +1252,10 @@ static int imx290_probe(struct i2c_clien
 	if (ret)
 		return ret;
 
-	mutex_init(&imx290->lock);
-
 	/* Initialize and register subdev. */
 	ret = imx290_subdev_init(imx290);
 	if (ret)
-		goto err_mutex;
+		return ret;
 
 	ret = v4l2_async_register_subdev(&imx290->sd);
 	if (ret < 0) {
@@ -1298,8 +1278,6 @@ static int imx290_probe(struct i2c_clien
 
 err_subdev:
 	imx290_subdev_cleanup(imx290);
-err_mutex:
-	mutex_destroy(&imx290->lock);
 
 	return ret;
 }
@@ -1312,8 +1290,6 @@ static void imx290_remove(struct i2c_cli
 	v4l2_async_unregister_subdev(sd);
 	imx290_subdev_cleanup(imx290);
 
-	mutex_destroy(&imx290->lock);
-
 	pm_runtime_disable(imx290->dev);
 	if (!pm_runtime_status_suspended(imx290->dev))
 		imx290_power_off(imx290->dev);