Re: [PATCH v2 17/23] media: ov5640: Implement get_selection

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jacopo,

Thank you for the patch.

On Thu, Feb 10, 2022 at 12:10:02PM +0100, Jacopo Mondi wrote:
> Implement the get_selection pad operation for the OV5640 sensor driver.
> 
> The supported targets report the sensor's native size, the active pixel
> array size and the analog crop rectangle from which the image is
> produced.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/i2c/ov5640.c | 51 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 8b90fab26d16..4ebb76d95849 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -35,6 +35,13 @@
>  #define OV5640_MIN_VBLANK	24
>  #define OV5640_MAX_VTS		1968
>  
> +#define OV5640_NATIVE_WIDTH		2624
> +#define OV5640_NATIVE_HEIGHT		1964
> +#define OV5640_PIXEL_ARRAY_TOP		14
> +#define OV5640_PIXEL_ARRAY_LEFT		16
> +#define OV5640_PIXEL_ARRAY_WIDTH	2592
> +#define OV5640_PIXEL_ARRAY_HEIGHT	1944
> +
>  #define OV5640_DEFAULT_SLAVE_ID 0x3c
>  
>  #define OV5640_LINK_RATE_MAX		490000000U
> @@ -2693,6 +2700,43 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
>  	return ret;
>  }
>  
> +static int ov5640_get_selection(struct v4l2_subdev *sd,
> +				struct v4l2_subdev_state *sd_state,
> +				struct v4l2_subdev_selection *sel)
> +{
> +	struct ov5640_dev *sensor = to_ov5640_dev(sd);
> +	const struct ov5640_mode_info *mode = sensor->current_mode;
> +
> +	switch (sel->target) {
> +	case V4L2_SEL_TGT_CROP: {
> +		mutex_lock(&sensor->lock);
> +		sel->r = mode->analog_crop;
> +		mutex_unlock(&sensor->lock);
> +
> +		return 0;
> +	}
> +
> +	case V4L2_SEL_TGT_NATIVE_SIZE:
> +	case V4L2_SEL_TGT_CROP_BOUNDS:
> +		sel->r.top = 0;
> +		sel->r.left = 0;
> +		sel->r.width = OV5640_NATIVE_WIDTH;
> +		sel->r.height = OV5640_NATIVE_HEIGHT;
> +
> +		return 0;
> +
> +	case V4L2_SEL_TGT_CROP_DEFAULT:
> +		sel->r.top = OV5640_PIXEL_ARRAY_TOP;
> +		sel->r.left = OV5640_PIXEL_ARRAY_LEFT;
> +		sel->r.width = OV5640_PIXEL_ARRAY_WIDTH;
> +		sel->r.height = OV5640_PIXEL_ARRAY_HEIGHT;
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int ov5640_set_framefmt(struct ov5640_dev *sensor,
>  			       struct v4l2_mbus_framefmt *format)
>  {
> @@ -3383,9 +3427,15 @@ static int ov5640_init_cfg(struct v4l2_subdev *sd,
>  			   struct v4l2_subdev_state *state)
>  {
>  	struct v4l2_mbus_framefmt *fmt = v4l2_subdev_get_try_format(sd, state, 0);
> +	struct v4l2_rect *crop = v4l2_subdev_get_try_crop(sd, state, 0);
>  
>  	*fmt = ov5640_default_fmt;
>  
> +	crop->left = OV5640_PIXEL_ARRAY_LEFT;
> +	crop->top = OV5640_PIXEL_ARRAY_TOP;
> +	crop->width = OV5640_PIXEL_ARRAY_WIDTH;
> +	crop->height = OV5640_PIXEL_ARRAY_HEIGHT;
> +
>  	return 0;
>  }
>  
> @@ -3407,6 +3457,7 @@ static const struct v4l2_subdev_pad_ops ov5640_pad_ops = {
>  	.enum_mbus_code = ov5640_enum_mbus_code,
>  	.get_fmt = ov5640_get_fmt,
>  	.set_fmt = ov5640_set_fmt,
> +	.get_selection = ov5640_get_selection,
>  	.enum_frame_size = ov5640_enum_frame_size,
>  	.enum_frame_interval = ov5640_enum_frame_interval,
>  };

-- 
Regards,

Laurent Pinchart