mbox series

[v3,0/7] v4l: subdev active state

Message ID 20220207161107.1166376-1-tomi.valkeinen@ideasonboard.com
Headers show
Series v4l: subdev active state | expand

Message

Tomi Valkeinen Feb. 7, 2022, 4:11 p.m. UTC 
Hi,

This is v3 of the subdev active state series. Changes since v2:

- Doc improvements
- Allow state->lock to be set by the driver (similarly to v4l2_ctrl_handler)
- Rename fields in 'struct v4l2_subdev_pad_config' and drop the try_ prefix.
- Add v4l2_subdev_get_locked_active_state(), which calls lockdep_assert_locked() and returns the state.
- Changed v4l2_subdev_get_active_state() to call lockdep_assert_not_locked()

The idea with the v4l2_subdev_get_active_state /
v4l2_subdev_get_locked_active_state change is to have a lockdep_assert
called. Roughly I think there are two cases where the
v4l2_subdev_get_active_state could be called:

- With the intention of just passing it forward to another subdev, in
  which case the state must _not_ be locked. Here
  v4l2_subdev_get_active_state() can be called.

- With the intention of using the state in a case where the state is
  known to be already locked. Here v4l2_subdev_get_locked_active_state()
  can be called.

The state->lock change hopefully solves Sakari's concerns about the
locking between controls and state.

 Tomi

Tomi Valkeinen (7):
  media: subdev: rename subdev-state alloc & free
  media: subdev: add active state to struct v4l2_subdev
  media: subdev: pass also the active state to subdevs from ioctls
  media: subdev: add subdev state locking
  media: subdev: Add v4l2_subdev_lock_and_return_state()
  media: Documentation: add documentation about subdev state
  media: subdev: rename v4l2_subdev_pad_config.try_* fields

 .../driver-api/media/v4l2-subdev.rst          |  60 ++++++
 drivers/media/i2c/adv7183.c                   |   2 +-
 drivers/media/i2c/imx274.c                    |  12 +-
 drivers/media/i2c/mt9m001.c                   |   2 +-
 drivers/media/i2c/mt9m111.c                   |   2 +-
 drivers/media/i2c/mt9t112.c                   |   2 +-
 drivers/media/i2c/mt9v011.c                   |   2 +-
 drivers/media/i2c/mt9v111.c                   |   4 +-
 drivers/media/i2c/ov2640.c                    |   2 +-
 drivers/media/i2c/ov6650.c                    |  18 +-
 drivers/media/i2c/ov772x.c                    |   2 +-
 drivers/media/i2c/ov9640.c                    |   2 +-
 drivers/media/i2c/rj54n1cb0c.c                |   2 +-
 drivers/media/i2c/saa6752hs.c                 |   2 +-
 drivers/media/i2c/sr030pc30.c                 |   2 +-
 drivers/media/i2c/tw9910.c                    |   2 +-
 drivers/media/i2c/vs6624.c                    |   2 +-
 drivers/media/platform/atmel/atmel-isc-base.c |   8 +-
 drivers/media/platform/atmel/atmel-isi.c      |   8 +-
 drivers/media/platform/rcar-vin/rcar-v4l2.c   |   9 +-
 drivers/media/platform/vsp1/vsp1_entity.c     |  10 +-
 drivers/media/v4l2-core/v4l2-subdev.c         | 126 +++++++++--
 drivers/staging/media/tegra-video/vi.c        |  10 +-
 include/media/v4l2-subdev.h                   | 201 ++++++++++++++++--
 24 files changed, 415 insertions(+), 77 deletions(-)

Comments

Hans Verkuil Feb. 8, 2022, 1:59 p.m. UTC | #1 
Hi Tomi,

On 2/7/22 17:11, Tomi Valkeinen wrote:
> struct v4l2_subdev_pad_config used to be relevant only for
> V4L2_SUBDEV_FORMAT_TRY configuration, and thus the fields of the struct
> were named, e.g. try_fmt.
> 
> This struct is now used in struct v4l2_subdev_state, and thus can be
> used in both try and active cases. Thus rename the fields and drop the
> "try_" prefix.

This is fine and necessary, but...

> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  drivers/media/i2c/adv7183.c                   |  2 +-
>  drivers/media/i2c/imx274.c                    | 12 ++++-----
>  drivers/media/i2c/mt9m001.c                   |  2 +-
>  drivers/media/i2c/mt9m111.c                   |  2 +-
>  drivers/media/i2c/mt9t112.c                   |  2 +-
>  drivers/media/i2c/mt9v011.c                   |  2 +-
>  drivers/media/i2c/mt9v111.c                   |  4 +--
>  drivers/media/i2c/ov2640.c                    |  2 +-
>  drivers/media/i2c/ov6650.c                    | 18 ++++++-------
>  drivers/media/i2c/ov772x.c                    |  2 +-
>  drivers/media/i2c/ov9640.c                    |  2 +-
>  drivers/media/i2c/rj54n1cb0c.c                |  2 +-
>  drivers/media/i2c/saa6752hs.c                 |  2 +-
>  drivers/media/i2c/sr030pc30.c                 |  2 +-
>  drivers/media/i2c/tw9910.c                    |  2 +-
>  drivers/media/i2c/vs6624.c                    |  2 +-
>  drivers/media/platform/atmel/atmel-isc-base.c |  8 +++---
>  drivers/media/platform/atmel/atmel-isi.c      |  8 +++---
>  include/media/v4l2-subdev.h                   | 26 +++++++++----------
>  19 files changed, 50 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/media/i2c/adv7183.c b/drivers/media/i2c/adv7183.c
> index 92cafdea3f1f..9e590abf88e1 100644
> --- a/drivers/media/i2c/adv7183.c
> +++ b/drivers/media/i2c/adv7183.c
> @@ -443,7 +443,7 @@ static int adv7183_set_fmt(struct v4l2_subdev *sd,
>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>  		decoder->fmt = *fmt;
>  	else
> -		sd_state->pads->try_fmt = *fmt;
> +		sd_state->pads->fmt = *fmt;

...now this isn't as clear as it used to be, since there is no longer any
indication that sd_state->pads refers to a 'try' state.

It would help if sd_state would be renamed to try_state, then it is clear
again.

The same would have to be done in the subdev.h callback prototypes
(i.e. rename state to try_state).

Calling it try_state is also consistent with the new sd->active_state, so
I think that would make a lot of sense.

Changing these names in the drivers is of course more work, but it would
make it a lot more readable.

Regards,

	Hans

>  	return 0;
>  }
>  
> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
> index 2aa15b9c23cc..c94c24358931 100644
> --- a/drivers/media/i2c/imx274.c
> +++ b/drivers/media/i2c/imx274.c
> @@ -1020,8 +1020,8 @@ static int __imx274_change_compose(struct stimx274 *imx274,
>  	int best_goodness = INT_MIN;
>  
>  	if (which == V4L2_SUBDEV_FORMAT_TRY) {
> -		cur_crop = &sd_state->pads->try_crop;
> -		tgt_fmt = &sd_state->pads->try_fmt;
> +		cur_crop = &sd_state->pads->crop;
> +		tgt_fmt = &sd_state->pads->fmt;
>  	} else {
>  		cur_crop = &imx274->crop;
>  		tgt_fmt = &imx274->format;
> @@ -1114,7 +1114,7 @@ static int imx274_set_fmt(struct v4l2_subdev *sd,
>  	 */
>  	fmt->field = V4L2_FIELD_NONE;
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY)
> -		sd_state->pads->try_fmt = *fmt;
> +		sd_state->pads->fmt = *fmt;
>  	else
>  		imx274->format = *fmt;
>  
> @@ -1145,8 +1145,8 @@ static int imx274_get_selection(struct v4l2_subdev *sd,
>  	}
>  
>  	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		src_crop = &sd_state->pads->try_crop;
> -		src_fmt = &sd_state->pads->try_fmt;
> +		src_crop = &sd_state->pads->crop;
> +		src_fmt = &sd_state->pads->fmt;
>  	} else {
>  		src_crop = &imx274->crop;
>  		src_fmt = &imx274->format;
> @@ -1217,7 +1217,7 @@ static int imx274_set_selection_crop(struct stimx274 *imx274,
>  	sel->r = new_crop;
>  
>  	if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
> -		tgt_crop = &sd_state->pads->try_crop;
> +		tgt_crop = &sd_state->pads->crop;
>  	else
>  		tgt_crop = &imx274->crop;
>  
> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
> index c9f0bd997ea7..4cf1334efdfe 100644
> --- a/drivers/media/i2c/mt9m001.c
> +++ b/drivers/media/i2c/mt9m001.c
> @@ -411,7 +411,7 @@ static int mt9m001_set_fmt(struct v4l2_subdev *sd,
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>  		return mt9m001_s_fmt(sd, fmt, mf);
> -	sd_state->pads->try_fmt = *mf;
> +	sd_state->pads->fmt = *mf;
>  	return 0;
>  }
>  
> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
> index 91a44359bcd3..495731f11006 100644
> --- a/drivers/media/i2c/mt9m111.c
> +++ b/drivers/media/i2c/mt9m111.c
> @@ -678,7 +678,7 @@ static int mt9m111_set_fmt(struct v4l2_subdev *sd,
>  	mf->xfer_func	= V4L2_XFER_FUNC_DEFAULT;
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		sd_state->pads->try_fmt = *mf;
> +		sd_state->pads->fmt = *mf;
>  		return 0;
>  	}
>  
> diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
> index 8d2e3caa9b28..4457b030d1a8 100644
> --- a/drivers/media/i2c/mt9t112.c
> +++ b/drivers/media/i2c/mt9t112.c
> @@ -982,7 +982,7 @@ static int mt9t112_set_fmt(struct v4l2_subdev *sd,
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>  		return mt9t112_s_fmt(sd, mf);
> -	sd_state->pads->try_fmt = *mf;
> +	sd_state->pads->fmt = *mf;
>  
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
> index 7699e64e1127..c02a51f2a327 100644
> --- a/drivers/media/i2c/mt9v011.c
> +++ b/drivers/media/i2c/mt9v011.c
> @@ -358,7 +358,7 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
>  
>  		set_res(sd);
>  	} else {
> -		sd_state->pads->try_fmt = *fmt;
> +		sd_state->pads->fmt = *fmt;
>  	}
>  
>  	return 0;
> diff --git a/drivers/media/i2c/mt9v111.c b/drivers/media/i2c/mt9v111.c
> index 2dc4a0f24ce8..eed8e5a8dcdd 100644
> --- a/drivers/media/i2c/mt9v111.c
> +++ b/drivers/media/i2c/mt9v111.c
> @@ -800,7 +800,7 @@ static struct v4l2_mbus_framefmt *__mt9v111_get_pad_format(
>  #if IS_ENABLED(CONFIG_VIDEO_V4L2_SUBDEV_API)
>  		return v4l2_subdev_get_try_format(&mt9v111->sd, sd_state, pad);
>  #else
> -		return &sd_state->pads->try_fmt;
> +		return &sd_state->pads->fmt;
>  #endif
>  	case V4L2_SUBDEV_FORMAT_ACTIVE:
>  		return &mt9v111->fmt;
> @@ -957,7 +957,7 @@ static int mt9v111_set_format(struct v4l2_subdev *subdev,
>  static int mt9v111_init_cfg(struct v4l2_subdev *subdev,
>  			    struct v4l2_subdev_state *sd_state)
>  {
> -	sd_state->pads->try_fmt = mt9v111_def_fmt;
> +	sd_state->pads->fmt = mt9v111_def_fmt;
>  
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
> index 4b75da55b260..8c576fe1e203 100644
> --- a/drivers/media/i2c/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -996,7 +996,7 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>  		/* select format */
>  		priv->cfmt_code = mf->code;
>  	} else {
> -		sd_state->pads->try_fmt = *mf;
> +		sd_state->pads->fmt = *mf;
>  	}
>  out:
>  	mutex_unlock(&priv->lock);
> diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
> index f67412150b16..2ba4402c5e96 100644
> --- a/drivers/media/i2c/ov6650.c
> +++ b/drivers/media/i2c/ov6650.c
> @@ -550,9 +550,9 @@ static int ov6650_get_fmt(struct v4l2_subdev *sd,
>  
>  	/* update media bus format code and frame size */
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		mf->width = sd_state->pads->try_fmt.width;
> -		mf->height = sd_state->pads->try_fmt.height;
> -		mf->code = sd_state->pads->try_fmt.code;
> +		mf->width = sd_state->pads->fmt.width;
> +		mf->height = sd_state->pads->fmt.height;
> +		mf->code = sd_state->pads->fmt.code;
>  
>  	} else {
>  		mf->width = priv->rect.width >> priv->half_scale;
> @@ -701,15 +701,15 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd,
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>  		/* store media bus format code and frame size in pad config */
> -		sd_state->pads->try_fmt.width = mf->width;
> -		sd_state->pads->try_fmt.height = mf->height;
> -		sd_state->pads->try_fmt.code = mf->code;
> +		sd_state->pads->fmt.width = mf->width;
> +		sd_state->pads->fmt.height = mf->height;
> +		sd_state->pads->fmt.code = mf->code;
>  
>  		/* return default mbus frame format updated with pad config */
>  		*mf = ov6650_def_fmt;
> -		mf->width = sd_state->pads->try_fmt.width;
> -		mf->height = sd_state->pads->try_fmt.height;
> -		mf->code = sd_state->pads->try_fmt.code;
> +		mf->width = sd_state->pads->fmt.width;
> +		mf->height = sd_state->pads->fmt.height;
> +		mf->code = sd_state->pads->fmt.code;
>  
>  	} else {
>  		/* apply new media bus format code and frame size */
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 78602a2f70b0..6ff5961b87b2 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -1222,7 +1222,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
>  	mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		sd_state->pads->try_fmt = *mf;
> +		sd_state->pads->fmt = *mf;
>  		return 0;
>  	}
>  
> diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
> index 0bab8c2cf160..f4b23183b8b1 100644
> --- a/drivers/media/i2c/ov9640.c
> +++ b/drivers/media/i2c/ov9640.c
> @@ -547,7 +547,7 @@ static int ov9640_set_fmt(struct v4l2_subdev *sd,
>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>  		return ov9640_s_fmt(sd, mf);
>  
> -	sd_state->pads->try_fmt = *mf;
> +	sd_state->pads->fmt = *mf;
>  
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/rj54n1cb0c.c b/drivers/media/i2c/rj54n1cb0c.c
> index 2e4018c26912..867e4d7339f2 100644
> --- a/drivers/media/i2c/rj54n1cb0c.c
> +++ b/drivers/media/i2c/rj54n1cb0c.c
> @@ -1009,7 +1009,7 @@ static int rj54n1_set_fmt(struct v4l2_subdev *sd,
>  			      &mf->height, 84, RJ54N1_MAX_HEIGHT, align, 0);
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		sd_state->pads->try_fmt = *mf;
> +		sd_state->pads->fmt = *mf;
>  		return 0;
>  	}
>  
> diff --git a/drivers/media/i2c/saa6752hs.c b/drivers/media/i2c/saa6752hs.c
> index a7f043cad149..74c10fba2af0 100644
> --- a/drivers/media/i2c/saa6752hs.c
> +++ b/drivers/media/i2c/saa6752hs.c
> @@ -595,7 +595,7 @@ static int saa6752hs_set_fmt(struct v4l2_subdev *sd,
>  	f->colorspace = V4L2_COLORSPACE_SMPTE170M;
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		sd_state->pads->try_fmt = *f;
> +		sd_state->pads->fmt = *f;
>  		return 0;
>  	}
>  
> diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
> index 19c0252df2f1..f0ccdf1f887d 100644
> --- a/drivers/media/i2c/sr030pc30.c
> +++ b/drivers/media/i2c/sr030pc30.c
> @@ -541,7 +541,7 @@ static int sr030pc30_set_fmt(struct v4l2_subdev *sd,
>  
>  	fmt = try_fmt(sd, mf);
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		sd_state->pads->try_fmt = *mf;
> +		sd_state->pads->fmt = *mf;
>  		return 0;
>  	}
>  
> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> index 09f5b3986928..96f6cbc663f0 100644
> --- a/drivers/media/i2c/tw9910.c
> +++ b/drivers/media/i2c/tw9910.c
> @@ -829,7 +829,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>  		return tw9910_s_fmt(sd, mf);
>  
> -	sd_state->pads->try_fmt = *mf;
> +	sd_state->pads->fmt = *mf;
>  
>  	return 0;
>  }
> diff --git a/drivers/media/i2c/vs6624.c b/drivers/media/i2c/vs6624.c
> index 29003dec6f2d..72a422693bc0 100644
> --- a/drivers/media/i2c/vs6624.c
> +++ b/drivers/media/i2c/vs6624.c
> @@ -587,7 +587,7 @@ static int vs6624_set_fmt(struct v4l2_subdev *sd,
>  	fmt->colorspace = vs6624_formats[index].colorspace;
>  
>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
> -		sd_state->pads->try_fmt = *fmt;
> +		sd_state->pads->fmt = *fmt;
>  		return 0;
>  	}
>  
> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
> index db15770d5b88..aa90aa55a128 100644
> --- a/drivers/media/platform/atmel/atmel-isc-base.c
> +++ b/drivers/media/platform/atmel/atmel-isc-base.c
> @@ -835,11 +835,11 @@ static void isc_try_fse(struct isc_device *isc,
>  	 * just use the maximum ISC can receive.
>  	 */
>  	if (ret) {
> -		sd_state->pads->try_crop.width = isc->max_width;
> -		sd_state->pads->try_crop.height = isc->max_height;
> +		sd_state->pads->crop.width = isc->max_width;
> +		sd_state->pads->crop.height = isc->max_height;
>  	} else {
> -		sd_state->pads->try_crop.width = fse.max_width;
> -		sd_state->pads->try_crop.height = fse.max_height;
> +		sd_state->pads->crop.width = fse.max_width;
> +		sd_state->pads->crop.height = fse.max_height;
>  	}
>  }
>  
> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
> index 4d15814e4481..5806de277bdc 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -572,11 +572,11 @@ static void isi_try_fse(struct atmel_isi *isi, const struct isi_format *isi_fmt,
>  	 * just use the maximum ISI can receive.
>  	 */
>  	if (ret) {
> -		sd_state->pads->try_crop.width = MAX_SUPPORT_WIDTH;
> -		sd_state->pads->try_crop.height = MAX_SUPPORT_HEIGHT;
> +		sd_state->pads->crop.width = MAX_SUPPORT_WIDTH;
> +		sd_state->pads->crop.height = MAX_SUPPORT_HEIGHT;
>  	} else {
> -		sd_state->pads->try_crop.width = fse.max_width;
> -		sd_state->pads->try_crop.height = fse.max_height;
> +		sd_state->pads->crop.width = fse.max_width;
> +		sd_state->pads->crop.height = fse.max_height;
>  	}
>  }
>  
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index 16497b8fc875..e7c5617820ab 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -638,21 +638,19 @@ struct v4l2_subdev_ir_ops {
>  /**
>   * struct v4l2_subdev_pad_config - Used for storing subdev pad information.
>   *
> - * @try_fmt: &struct v4l2_mbus_framefmt
> - * @try_crop: &struct v4l2_rect to be used for crop
> - * @try_compose: &struct v4l2_rect to be used for compose
> + * @fmt: &struct v4l2_mbus_framefmt
> + * @crop: &struct v4l2_rect to be used for crop
> + * @compose: &struct v4l2_rect to be used for compose
>   *
>   * This structure only needs to be passed to the pad op if the 'which' field
>   * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For
>   * %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL.
>   *
> - * Note: This struct is also used in active state, and the try_ prefix is
> - * historical and to be removed.
>   */
>  struct v4l2_subdev_pad_config {
> -	struct v4l2_mbus_framefmt try_fmt;
> -	struct v4l2_rect try_crop;
> -	struct v4l2_rect try_compose;
> +	struct v4l2_mbus_framefmt fmt;
> +	struct v4l2_rect crop;
> +	struct v4l2_rect compose;
>  };
>  
>  /**
> @@ -1008,7 +1006,7 @@ struct v4l2_subdev_fh {
>  
>  /**
>   * v4l2_subdev_get_try_format - ancillary routine to call
> - *	&struct v4l2_subdev_pad_config->try_fmt
> + *	&struct v4l2_subdev_pad_config->fmt
>   *
>   * @sd: pointer to &struct v4l2_subdev
>   * @state: pointer to &struct v4l2_subdev_state
> @@ -1021,12 +1019,12 @@ v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
>  {
>  	if (WARN_ON(pad >= sd->entity.num_pads))
>  		pad = 0;
> -	return &state->pads[pad].try_fmt;
> +	return &state->pads[pad].fmt;
>  }
>  
>  /**
>   * v4l2_subdev_get_try_crop - ancillary routine to call
> - *	&struct v4l2_subdev_pad_config->try_crop
> + *	&struct v4l2_subdev_pad_config->crop
>   *
>   * @sd: pointer to &struct v4l2_subdev
>   * @state: pointer to &struct v4l2_subdev_state.
> @@ -1039,12 +1037,12 @@ v4l2_subdev_get_try_crop(struct v4l2_subdev *sd,
>  {
>  	if (WARN_ON(pad >= sd->entity.num_pads))
>  		pad = 0;
> -	return &state->pads[pad].try_crop;
> +	return &state->pads[pad].crop;
>  }
>  
>  /**
>   * v4l2_subdev_get_try_compose - ancillary routine to call
> - *	&struct v4l2_subdev_pad_config->try_compose
> + *	&struct v4l2_subdev_pad_config->compose
>   *
>   * @sd: pointer to &struct v4l2_subdev
>   * @state: pointer to &struct v4l2_subdev_state.
> @@ -1057,7 +1055,7 @@ v4l2_subdev_get_try_compose(struct v4l2_subdev *sd,
>  {
>  	if (WARN_ON(pad >= sd->entity.num_pads))
>  		pad = 0;
> -	return &state->pads[pad].try_compose;
> +	return &state->pads[pad].compose;
>  }
>  
>  #endif
Hans Verkuil Feb. 8, 2022, 2:16 p.m. UTC | #2 
Hi Laurent,

On 2/8/22 15:05, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Tue, Feb 08, 2022 at 02:59:31PM +0100, Hans Verkuil wrote:
>> On 2/7/22 17:11, Tomi Valkeinen wrote:
>>> struct v4l2_subdev_pad_config used to be relevant only for
>>> V4L2_SUBDEV_FORMAT_TRY configuration, and thus the fields of the struct
>>> were named, e.g. try_fmt.
>>>
>>> This struct is now used in struct v4l2_subdev_state, and thus can be
>>> used in both try and active cases. Thus rename the fields and drop the
>>> "try_" prefix.
>>
>> This is fine and necessary, but...
>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>>  drivers/media/i2c/adv7183.c                   |  2 +-
>>>  drivers/media/i2c/imx274.c                    | 12 ++++-----
>>>  drivers/media/i2c/mt9m001.c                   |  2 +-
>>>  drivers/media/i2c/mt9m111.c                   |  2 +-
>>>  drivers/media/i2c/mt9t112.c                   |  2 +-
>>>  drivers/media/i2c/mt9v011.c                   |  2 +-
>>>  drivers/media/i2c/mt9v111.c                   |  4 +--
>>>  drivers/media/i2c/ov2640.c                    |  2 +-
>>>  drivers/media/i2c/ov6650.c                    | 18 ++++++-------
>>>  drivers/media/i2c/ov772x.c                    |  2 +-
>>>  drivers/media/i2c/ov9640.c                    |  2 +-
>>>  drivers/media/i2c/rj54n1cb0c.c                |  2 +-
>>>  drivers/media/i2c/saa6752hs.c                 |  2 +-
>>>  drivers/media/i2c/sr030pc30.c                 |  2 +-
>>>  drivers/media/i2c/tw9910.c                    |  2 +-
>>>  drivers/media/i2c/vs6624.c                    |  2 +-
>>>  drivers/media/platform/atmel/atmel-isc-base.c |  8 +++---
>>>  drivers/media/platform/atmel/atmel-isi.c      |  8 +++---
>>>  include/media/v4l2-subdev.h                   | 26 +++++++++----------
>>>  19 files changed, 50 insertions(+), 52 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/adv7183.c b/drivers/media/i2c/adv7183.c
>>> index 92cafdea3f1f..9e590abf88e1 100644
>>> --- a/drivers/media/i2c/adv7183.c
>>> +++ b/drivers/media/i2c/adv7183.c
>>> @@ -443,7 +443,7 @@ static int adv7183_set_fmt(struct v4l2_subdev *sd,
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>>>  		decoder->fmt = *fmt;
>>>  	else
>>> -		sd_state->pads->try_fmt = *fmt;
>>> +		sd_state->pads->fmt = *fmt;
>>
>> ...now this isn't as clear as it used to be, since there is no longer any
>> indication that sd_state->pads refers to a 'try' state.
>>
>> It would help if sd_state would be renamed to try_state, then it is clear
>> again.
>>
>> The same would have to be done in the subdev.h callback prototypes
>> (i.e. rename state to try_state).
>>
>> Calling it try_state is also consistent with the new sd->active_state, so
>> I think that would make a lot of sense.
> 
> With the active state series, the state passed to .set_fmt() can be the
> try or active state, depending on the which value. Existing drivers will
> continue to operate as expected, and will receive a NULL state when
> which is ACTIVE. Drivers that call v4l2_subdev_init_finalize() will get
> the active state in that case.
> 
> We could rename the arguments in the driver touched by this patch, but
> I'd rather modify them to use the v4l2_subdev_get_try_format() helper
> instead of accessing the try_fmt field directly. We'll then have to
> discuss this again when we'll rename v4l2_subdev_get_try_format() to
> v4l2_subdev_state_get_format() :-)

You are absolutely correct. I should have refeshed my memory of this series
a bit better.

Ignore my comments, and just add:

Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

And BTW, I *do* think v4l2_subdev_get_try_format() et al should be renamed to
v4l2_subdev_state_get_format() et al. It makes a lot of sense. But that's a
separate patch, I think.

Regards,

	Hans

> 
>> Changing these names in the drivers is of course more work, but it would
>> make it a lot more readable.
>>
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
>>> index 2aa15b9c23cc..c94c24358931 100644
>>> --- a/drivers/media/i2c/imx274.c
>>> +++ b/drivers/media/i2c/imx274.c
>>> @@ -1020,8 +1020,8 @@ static int __imx274_change_compose(struct stimx274 *imx274,
>>>  	int best_goodness = INT_MIN;
>>>  
>>>  	if (which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		cur_crop = &sd_state->pads->try_crop;
>>> -		tgt_fmt = &sd_state->pads->try_fmt;
>>> +		cur_crop = &sd_state->pads->crop;
>>> +		tgt_fmt = &sd_state->pads->fmt;
>>>  	} else {
>>>  		cur_crop = &imx274->crop;
>>>  		tgt_fmt = &imx274->format;
>>> @@ -1114,7 +1114,7 @@ static int imx274_set_fmt(struct v4l2_subdev *sd,
>>>  	 */
>>>  	fmt->field = V4L2_FIELD_NONE;
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY)
>>> -		sd_state->pads->try_fmt = *fmt;
>>> +		sd_state->pads->fmt = *fmt;
>>>  	else
>>>  		imx274->format = *fmt;
>>>  
>>> @@ -1145,8 +1145,8 @@ static int imx274_get_selection(struct v4l2_subdev *sd,
>>>  	}
>>>  
>>>  	if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		src_crop = &sd_state->pads->try_crop;
>>> -		src_fmt = &sd_state->pads->try_fmt;
>>> +		src_crop = &sd_state->pads->crop;
>>> +		src_fmt = &sd_state->pads->fmt;
>>>  	} else {
>>>  		src_crop = &imx274->crop;
>>>  		src_fmt = &imx274->format;
>>> @@ -1217,7 +1217,7 @@ static int imx274_set_selection_crop(struct stimx274 *imx274,
>>>  	sel->r = new_crop;
>>>  
>>>  	if (sel->which == V4L2_SUBDEV_FORMAT_TRY)
>>> -		tgt_crop = &sd_state->pads->try_crop;
>>> +		tgt_crop = &sd_state->pads->crop;
>>>  	else
>>>  		tgt_crop = &imx274->crop;
>>>  
>>> diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
>>> index c9f0bd997ea7..4cf1334efdfe 100644
>>> --- a/drivers/media/i2c/mt9m001.c
>>> +++ b/drivers/media/i2c/mt9m001.c
>>> @@ -411,7 +411,7 @@ static int mt9m001_set_fmt(struct v4l2_subdev *sd,
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>>>  		return mt9m001_s_fmt(sd, fmt, mf);
>>> -	sd_state->pads->try_fmt = *mf;
>>> +	sd_state->pads->fmt = *mf;
>>>  	return 0;
>>>  }
>>>  
>>> diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
>>> index 91a44359bcd3..495731f11006 100644
>>> --- a/drivers/media/i2c/mt9m111.c
>>> +++ b/drivers/media/i2c/mt9m111.c
>>> @@ -678,7 +678,7 @@ static int mt9m111_set_fmt(struct v4l2_subdev *sd,
>>>  	mf->xfer_func	= V4L2_XFER_FUNC_DEFAULT;
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		sd_state->pads->try_fmt = *mf;
>>> +		sd_state->pads->fmt = *mf;
>>>  		return 0;
>>>  	}
>>>  
>>> diff --git a/drivers/media/i2c/mt9t112.c b/drivers/media/i2c/mt9t112.c
>>> index 8d2e3caa9b28..4457b030d1a8 100644
>>> --- a/drivers/media/i2c/mt9t112.c
>>> +++ b/drivers/media/i2c/mt9t112.c
>>> @@ -982,7 +982,7 @@ static int mt9t112_set_fmt(struct v4l2_subdev *sd,
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>>>  		return mt9t112_s_fmt(sd, mf);
>>> -	sd_state->pads->try_fmt = *mf;
>>> +	sd_state->pads->fmt = *mf;
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/drivers/media/i2c/mt9v011.c b/drivers/media/i2c/mt9v011.c
>>> index 7699e64e1127..c02a51f2a327 100644
>>> --- a/drivers/media/i2c/mt9v011.c
>>> +++ b/drivers/media/i2c/mt9v011.c
>>> @@ -358,7 +358,7 @@ static int mt9v011_set_fmt(struct v4l2_subdev *sd,
>>>  
>>>  		set_res(sd);
>>>  	} else {
>>> -		sd_state->pads->try_fmt = *fmt;
>>> +		sd_state->pads->fmt = *fmt;
>>>  	}
>>>  
>>>  	return 0;
>>> diff --git a/drivers/media/i2c/mt9v111.c b/drivers/media/i2c/mt9v111.c
>>> index 2dc4a0f24ce8..eed8e5a8dcdd 100644
>>> --- a/drivers/media/i2c/mt9v111.c
>>> +++ b/drivers/media/i2c/mt9v111.c
>>> @@ -800,7 +800,7 @@ static struct v4l2_mbus_framefmt *__mt9v111_get_pad_format(
>>>  #if IS_ENABLED(CONFIG_VIDEO_V4L2_SUBDEV_API)
>>>  		return v4l2_subdev_get_try_format(&mt9v111->sd, sd_state, pad);
>>>  #else
>>> -		return &sd_state->pads->try_fmt;
>>> +		return &sd_state->pads->fmt;
>>>  #endif
>>>  	case V4L2_SUBDEV_FORMAT_ACTIVE:
>>>  		return &mt9v111->fmt;
>>> @@ -957,7 +957,7 @@ static int mt9v111_set_format(struct v4l2_subdev *subdev,
>>>  static int mt9v111_init_cfg(struct v4l2_subdev *subdev,
>>>  			    struct v4l2_subdev_state *sd_state)
>>>  {
>>> -	sd_state->pads->try_fmt = mt9v111_def_fmt;
>>> +	sd_state->pads->fmt = mt9v111_def_fmt;
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
>>> index 4b75da55b260..8c576fe1e203 100644
>>> --- a/drivers/media/i2c/ov2640.c
>>> +++ b/drivers/media/i2c/ov2640.c
>>> @@ -996,7 +996,7 @@ static int ov2640_set_fmt(struct v4l2_subdev *sd,
>>>  		/* select format */
>>>  		priv->cfmt_code = mf->code;
>>>  	} else {
>>> -		sd_state->pads->try_fmt = *mf;
>>> +		sd_state->pads->fmt = *mf;
>>>  	}
>>>  out:
>>>  	mutex_unlock(&priv->lock);
>>> diff --git a/drivers/media/i2c/ov6650.c b/drivers/media/i2c/ov6650.c
>>> index f67412150b16..2ba4402c5e96 100644
>>> --- a/drivers/media/i2c/ov6650.c
>>> +++ b/drivers/media/i2c/ov6650.c
>>> @@ -550,9 +550,9 @@ static int ov6650_get_fmt(struct v4l2_subdev *sd,
>>>  
>>>  	/* update media bus format code and frame size */
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		mf->width = sd_state->pads->try_fmt.width;
>>> -		mf->height = sd_state->pads->try_fmt.height;
>>> -		mf->code = sd_state->pads->try_fmt.code;
>>> +		mf->width = sd_state->pads->fmt.width;
>>> +		mf->height = sd_state->pads->fmt.height;
>>> +		mf->code = sd_state->pads->fmt.code;
>>>  
>>>  	} else {
>>>  		mf->width = priv->rect.width >> priv->half_scale;
>>> @@ -701,15 +701,15 @@ static int ov6650_set_fmt(struct v4l2_subdev *sd,
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>>  		/* store media bus format code and frame size in pad config */
>>> -		sd_state->pads->try_fmt.width = mf->width;
>>> -		sd_state->pads->try_fmt.height = mf->height;
>>> -		sd_state->pads->try_fmt.code = mf->code;
>>> +		sd_state->pads->fmt.width = mf->width;
>>> +		sd_state->pads->fmt.height = mf->height;
>>> +		sd_state->pads->fmt.code = mf->code;
>>>  
>>>  		/* return default mbus frame format updated with pad config */
>>>  		*mf = ov6650_def_fmt;
>>> -		mf->width = sd_state->pads->try_fmt.width;
>>> -		mf->height = sd_state->pads->try_fmt.height;
>>> -		mf->code = sd_state->pads->try_fmt.code;
>>> +		mf->width = sd_state->pads->fmt.width;
>>> +		mf->height = sd_state->pads->fmt.height;
>>> +		mf->code = sd_state->pads->fmt.code;
>>>  
>>>  	} else {
>>>  		/* apply new media bus format code and frame size */
>>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>>> index 78602a2f70b0..6ff5961b87b2 100644
>>> --- a/drivers/media/i2c/ov772x.c
>>> +++ b/drivers/media/i2c/ov772x.c
>>> @@ -1222,7 +1222,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
>>>  	mf->xfer_func = V4L2_XFER_FUNC_DEFAULT;
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		sd_state->pads->try_fmt = *mf;
>>> +		sd_state->pads->fmt = *mf;
>>>  		return 0;
>>>  	}
>>>  
>>> diff --git a/drivers/media/i2c/ov9640.c b/drivers/media/i2c/ov9640.c
>>> index 0bab8c2cf160..f4b23183b8b1 100644
>>> --- a/drivers/media/i2c/ov9640.c
>>> +++ b/drivers/media/i2c/ov9640.c
>>> @@ -547,7 +547,7 @@ static int ov9640_set_fmt(struct v4l2_subdev *sd,
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>>>  		return ov9640_s_fmt(sd, mf);
>>>  
>>> -	sd_state->pads->try_fmt = *mf;
>>> +	sd_state->pads->fmt = *mf;
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/drivers/media/i2c/rj54n1cb0c.c b/drivers/media/i2c/rj54n1cb0c.c
>>> index 2e4018c26912..867e4d7339f2 100644
>>> --- a/drivers/media/i2c/rj54n1cb0c.c
>>> +++ b/drivers/media/i2c/rj54n1cb0c.c
>>> @@ -1009,7 +1009,7 @@ static int rj54n1_set_fmt(struct v4l2_subdev *sd,
>>>  			      &mf->height, 84, RJ54N1_MAX_HEIGHT, align, 0);
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		sd_state->pads->try_fmt = *mf;
>>> +		sd_state->pads->fmt = *mf;
>>>  		return 0;
>>>  	}
>>>  
>>> diff --git a/drivers/media/i2c/saa6752hs.c b/drivers/media/i2c/saa6752hs.c
>>> index a7f043cad149..74c10fba2af0 100644
>>> --- a/drivers/media/i2c/saa6752hs.c
>>> +++ b/drivers/media/i2c/saa6752hs.c
>>> @@ -595,7 +595,7 @@ static int saa6752hs_set_fmt(struct v4l2_subdev *sd,
>>>  	f->colorspace = V4L2_COLORSPACE_SMPTE170M;
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		sd_state->pads->try_fmt = *f;
>>> +		sd_state->pads->fmt = *f;
>>>  		return 0;
>>>  	}
>>>  
>>> diff --git a/drivers/media/i2c/sr030pc30.c b/drivers/media/i2c/sr030pc30.c
>>> index 19c0252df2f1..f0ccdf1f887d 100644
>>> --- a/drivers/media/i2c/sr030pc30.c
>>> +++ b/drivers/media/i2c/sr030pc30.c
>>> @@ -541,7 +541,7 @@ static int sr030pc30_set_fmt(struct v4l2_subdev *sd,
>>>  
>>>  	fmt = try_fmt(sd, mf);
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		sd_state->pads->try_fmt = *mf;
>>> +		sd_state->pads->fmt = *mf;
>>>  		return 0;
>>>  	}
>>>  
>>> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
>>> index 09f5b3986928..96f6cbc663f0 100644
>>> --- a/drivers/media/i2c/tw9910.c
>>> +++ b/drivers/media/i2c/tw9910.c
>>> @@ -829,7 +829,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
>>>  		return tw9910_s_fmt(sd, mf);
>>>  
>>> -	sd_state->pads->try_fmt = *mf;
>>> +	sd_state->pads->fmt = *mf;
>>>  
>>>  	return 0;
>>>  }
>>> diff --git a/drivers/media/i2c/vs6624.c b/drivers/media/i2c/vs6624.c
>>> index 29003dec6f2d..72a422693bc0 100644
>>> --- a/drivers/media/i2c/vs6624.c
>>> +++ b/drivers/media/i2c/vs6624.c
>>> @@ -587,7 +587,7 @@ static int vs6624_set_fmt(struct v4l2_subdev *sd,
>>>  	fmt->colorspace = vs6624_formats[index].colorspace;
>>>  
>>>  	if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
>>> -		sd_state->pads->try_fmt = *fmt;
>>> +		sd_state->pads->fmt = *fmt;
>>>  		return 0;
>>>  	}
>>>  
>>> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c
>>> index db15770d5b88..aa90aa55a128 100644
>>> --- a/drivers/media/platform/atmel/atmel-isc-base.c
>>> +++ b/drivers/media/platform/atmel/atmel-isc-base.c
>>> @@ -835,11 +835,11 @@ static void isc_try_fse(struct isc_device *isc,
>>>  	 * just use the maximum ISC can receive.
>>>  	 */
>>>  	if (ret) {
>>> -		sd_state->pads->try_crop.width = isc->max_width;
>>> -		sd_state->pads->try_crop.height = isc->max_height;
>>> +		sd_state->pads->crop.width = isc->max_width;
>>> +		sd_state->pads->crop.height = isc->max_height;
>>>  	} else {
>>> -		sd_state->pads->try_crop.width = fse.max_width;
>>> -		sd_state->pads->try_crop.height = fse.max_height;
>>> +		sd_state->pads->crop.width = fse.max_width;
>>> +		sd_state->pads->crop.height = fse.max_height;
>>>  	}
>>>  }
>>>  
>>> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
>>> index 4d15814e4481..5806de277bdc 100644
>>> --- a/drivers/media/platform/atmel/atmel-isi.c
>>> +++ b/drivers/media/platform/atmel/atmel-isi.c
>>> @@ -572,11 +572,11 @@ static void isi_try_fse(struct atmel_isi *isi, const struct isi_format *isi_fmt,
>>>  	 * just use the maximum ISI can receive.
>>>  	 */
>>>  	if (ret) {
>>> -		sd_state->pads->try_crop.width = MAX_SUPPORT_WIDTH;
>>> -		sd_state->pads->try_crop.height = MAX_SUPPORT_HEIGHT;
>>> +		sd_state->pads->crop.width = MAX_SUPPORT_WIDTH;
>>> +		sd_state->pads->crop.height = MAX_SUPPORT_HEIGHT;
>>>  	} else {
>>> -		sd_state->pads->try_crop.width = fse.max_width;
>>> -		sd_state->pads->try_crop.height = fse.max_height;
>>> +		sd_state->pads->crop.width = fse.max_width;
>>> +		sd_state->pads->crop.height = fse.max_height;
>>>  	}
>>>  }
>>>  
>>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>>> index 16497b8fc875..e7c5617820ab 100644
>>> --- a/include/media/v4l2-subdev.h
>>> +++ b/include/media/v4l2-subdev.h
>>> @@ -638,21 +638,19 @@ struct v4l2_subdev_ir_ops {
>>>  /**
>>>   * struct v4l2_subdev_pad_config - Used for storing subdev pad information.
>>>   *
>>> - * @try_fmt: &struct v4l2_mbus_framefmt
>>> - * @try_crop: &struct v4l2_rect to be used for crop
>>> - * @try_compose: &struct v4l2_rect to be used for compose
>>> + * @fmt: &struct v4l2_mbus_framefmt
>>> + * @crop: &struct v4l2_rect to be used for crop
>>> + * @compose: &struct v4l2_rect to be used for compose
>>>   *
>>>   * This structure only needs to be passed to the pad op if the 'which' field
>>>   * of the main argument is set to %V4L2_SUBDEV_FORMAT_TRY. For
>>>   * %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL.
>>>   *
>>> - * Note: This struct is also used in active state, and the try_ prefix is
>>> - * historical and to be removed.
>>>   */
>>>  struct v4l2_subdev_pad_config {
>>> -	struct v4l2_mbus_framefmt try_fmt;
>>> -	struct v4l2_rect try_crop;
>>> -	struct v4l2_rect try_compose;
>>> +	struct v4l2_mbus_framefmt fmt;
>>> +	struct v4l2_rect crop;
>>> +	struct v4l2_rect compose;
>>>  };
>>>  
>>>  /**
>>> @@ -1008,7 +1006,7 @@ struct v4l2_subdev_fh {
>>>  
>>>  /**
>>>   * v4l2_subdev_get_try_format - ancillary routine to call
>>> - *	&struct v4l2_subdev_pad_config->try_fmt
>>> + *	&struct v4l2_subdev_pad_config->fmt
>>>   *
>>>   * @sd: pointer to &struct v4l2_subdev
>>>   * @state: pointer to &struct v4l2_subdev_state
>>> @@ -1021,12 +1019,12 @@ v4l2_subdev_get_try_format(struct v4l2_subdev *sd,
>>>  {
>>>  	if (WARN_ON(pad >= sd->entity.num_pads))
>>>  		pad = 0;
>>> -	return &state->pads[pad].try_fmt;
>>> +	return &state->pads[pad].fmt;
>>>  }
>>>  
>>>  /**
>>>   * v4l2_subdev_get_try_crop - ancillary routine to call
>>> - *	&struct v4l2_subdev_pad_config->try_crop
>>> + *	&struct v4l2_subdev_pad_config->crop
>>>   *
>>>   * @sd: pointer to &struct v4l2_subdev
>>>   * @state: pointer to &struct v4l2_subdev_state.
>>> @@ -1039,12 +1037,12 @@ v4l2_subdev_get_try_crop(struct v4l2_subdev *sd,
>>>  {
>>>  	if (WARN_ON(pad >= sd->entity.num_pads))
>>>  		pad = 0;
>>> -	return &state->pads[pad].try_crop;
>>> +	return &state->pads[pad].crop;
>>>  }
>>>  
>>>  /**
>>>   * v4l2_subdev_get_try_compose - ancillary routine to call
>>> - *	&struct v4l2_subdev_pad_config->try_compose
>>> + *	&struct v4l2_subdev_pad_config->compose
>>>   *
>>>   * @sd: pointer to &struct v4l2_subdev
>>>   * @state: pointer to &struct v4l2_subdev_state.
>>> @@ -1057,7 +1055,7 @@ v4l2_subdev_get_try_compose(struct v4l2_subdev *sd,
>>>  {
>>>  	if (WARN_ON(pad >= sd->entity.num_pads))
>>>  		pad = 0;
>>> -	return &state->pads[pad].try_compose;
>>> +	return &state->pads[pad].compose;
>>>  }
>>>  
>>>  #endif
>