Commit a2514b9a authored by Laurent Pinchart's avatar Laurent Pinchart Committed by Mauro Carvalho Chehab
Browse files

media: i2c: imx290: Use V4L2 subdev active state 



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: default avatarLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: default avatarAlexander Stein <alexander.stein@ew.tq-group.com>
Signed-off-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
parent e5d363ca

drivers/media/i2c/imx290.c

+65 −89
Original line number Diff line number Diff line
@@ -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 imx290 *imx290, 
			    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_ctrl *ctrl) 
					     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_ctrl *ctrl) 
					       &ret);

		}

		break;



	default:

		ret = -EINVAL;

		break;
@@ -625,13 +634,6 @@ static void imx290_ctrl_update(struct imx290 *imx290, 
	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 imx290 *imx290) 
		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 imx290 *imx290) 
		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 imx290 *imx290) 
 */



/* 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 imx290 *imx290) 
	}



	/* 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 imx290 *imx290) 
		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 imx290 *imx290) 
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 v4l2_subdev *sd, 
	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_subdev *sd, 
	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_subdev *sd, 
	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_subdev *sd, 


	*format = fmt->format;



	mutex_unlock(&imx290->lock);



	return 0;

}
@@ -925,14 +897,11 @@ static int imx290_get_selection(struct v4l2_subdev *sd, 
				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 v4l2_subdev *sd, 
		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 v4l2_subdev *sd, 
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 imx290_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 imx290 *imx290) 
	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 imx290 *imx290) 
	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_client *client) 
	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_client *client) 


err_subdev:

	imx290_subdev_cleanup(imx290);

err_mutex:

	mutex_destroy(&imx290->lock);



	return ret;

}
@@ -1312,8 +1290,6 @@ static void imx290_remove(struct i2c_client *client) 
	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);