diff mbox series

[v2,8/8] media: i2c: ov5670: Add .get_selection() support

Message ID 20220314162714.153970-9-jacopo@jmondi.org (mailing list archive)
State Superseded
Delegated to: Sakari Ailus
Headers
Series media: i2c: ov5670: OF support, runtime_pm, regulators |

Commit Message

Jacopo Mondi March 14, 2022, 4:27 p.m. UTC 
  From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>

Add support for the .get_selection() pad operation to the ov5670 sensor
driver.

Report the native sensor size (pixel array), the crop bounds (readable
pixel array area) and the current and default analog crop rectangles.

Currently all driver's mode use an analog crop rectangle of size
[12, 4, 2600, 1952]. Instead of hardcoding the value in the operation
implementation, ad an .analog_crop field to the sensor's mode
definitions, to make sure that if any mode gets added, its crop
rectangle will be defined as well.

While at it re-sort the mode's field definition order to match the
declaration order and initialize the crop rectangle in init_cfg().

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5670.c | 89 +++++++++++++++++++++++++++++++++++---
 1 file changed, 83 insertions(+), 6 deletions(-)

Comments

kernel test robot March 15, 2022, 4:18 a.m. UTC | #1 
Hi Jacopo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v5.17-rc8 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220315-003034
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-a012-20220314 (https://download.01.org/0day-ci/archive/20220315/202203151238.pCKNarov-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 3e4950d7fa78ac83f33bbf1658e2f49a73719236)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c619a8eee6477517dfaa05511344d0bddc4e1c55
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220315-003034
        git checkout c619a8eee6477517dfaa05511344d0bddc4e1c55
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/media/i2c/ov5670.c:1787:18: error: initializer element is not a compile-time constant
                   .analog_crop = ov5670_analog_crop,
                                  ^~~~~~~~~~~~~~~~~~
   1 error generated.


vim +1787 drivers/media/i2c/ov5670.c

  1774	
  1775	/*
  1776	 * OV5670 sensor supports following resolutions with full FOV:
  1777	 * 4:3  ==> {2592x1944, 1296x972, 648x486}
  1778	 * 16:9 ==> {2560x1440, 1280x720, 640x360}
  1779	 */
  1780	static const struct ov5670_mode supported_modes[] = {
  1781		{
  1782			.width = 2592,
  1783			.height = 1944,
  1784			.vts_def = OV5670_VTS_30FPS,
  1785			.vts_min = OV5670_VTS_30FPS,
  1786			.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
> 1787			.analog_crop = ov5670_analog_crop,
  1788			.reg_list = {
  1789				.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
  1790				.regs = mode_2592x1944_regs,
  1791			},
  1792		},
  1793		{
  1794			.width = 1296,
  1795			.height = 972,
  1796			.vts_def = OV5670_VTS_30FPS,
  1797			.vts_min = 996,
  1798			.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
  1799			.analog_crop = ov5670_analog_crop,
  1800			.reg_list = {
  1801				.num_of_regs = ARRAY_SIZE(mode_1296x972_regs),
  1802				.regs = mode_1296x972_regs,
  1803			},
  1804		},
  1805		{
  1806			.width = 648,
  1807			.height = 486,
  1808			.vts_def = OV5670_VTS_30FPS,
  1809			.vts_min = 516,
  1810			.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
  1811			.analog_crop = ov5670_analog_crop,
  1812			.reg_list = {
  1813				.num_of_regs = ARRAY_SIZE(mode_648x486_regs),
  1814				.regs = mode_648x486_regs,
  1815			},
  1816		},
  1817		{
  1818			.width = 2560,
  1819			.height = 1440,
  1820			.vts_def = OV5670_VTS_30FPS,
  1821			.vts_min = OV5670_VTS_30FPS,
  1822			.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
  1823			.analog_crop = ov5670_analog_crop,
  1824			.reg_list = {
  1825				.num_of_regs = ARRAY_SIZE(mode_2560x1440_regs),
  1826				.regs = mode_2560x1440_regs,
  1827			},
  1828		},
  1829		{
  1830			.width = 1280,
  1831			.height = 720,
  1832			.vts_def = OV5670_VTS_30FPS,
  1833			.vts_min = 1020,
  1834	
  1835			.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
  1836			.analog_crop = ov5670_analog_crop,
  1837			.reg_list = {
  1838				.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
  1839				.regs = mode_1280x720_regs,
  1840			},
  1841		},
  1842		{
  1843			.width = 640,
  1844			.height = 360,
  1845			.vts_def = OV5670_VTS_30FPS,
  1846			.vts_min = 510,
  1847			.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
  1848			.analog_crop = ov5670_analog_crop,
  1849			.reg_list = {
  1850				.num_of_regs = ARRAY_SIZE(mode_640x360_regs),
  1851				.regs = mode_640x360_regs,
  1852			},
  1853		}
  1854	};
  1855	

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Laurent Pinchart March 15, 2022, 8:19 a.m. UTC | #2 
Hi Jacopo,

Thank you for the patch.

On Mon, Mar 14, 2022 at 05:27:14PM +0100, Jacopo Mondi wrote:
> From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> 
> Add support for the .get_selection() pad operation to the ov5670 sensor
> driver.
> 
> Report the native sensor size (pixel array), the crop bounds (readable
> pixel array area) and the current and default analog crop rectangles.
> 
> Currently all driver's mode use an analog crop rectangle of size

s/mode/modes/

> [12, 4, 2600, 1952]. Instead of hardcoding the value in the operation
> implementation, ad an .analog_crop field to the sensor's mode
> definitions, to make sure that if any mode gets added, its crop
> rectangle will be defined as well.
> 
> While at it re-sort the mode's field definition order to match the
> declaration order and initialize the crop rectangle in init_cfg().
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  drivers/media/i2c/ov5670.c | 89 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 83 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
> index 9aa82774f8a6..67897dabb712 100644
> --- a/drivers/media/i2c/ov5670.c
> +++ b/drivers/media/i2c/ov5670.c
> @@ -71,6 +71,10 @@
>  #define OV5670_REG_VALUE_16BIT		2
>  #define OV5670_REG_VALUE_24BIT		3
>  
> +/* Pixel Array */
> +#define OV5670_NATIVE_WIDTH		2624
> +#define OV5670_NATIVE_HEIGHT		1980
> +
>  /* Initial number of frames to skip to avoid possible garbage */
>  #define OV5670_NUM_OF_SKIP_FRAMES	2
>  
> @@ -113,10 +117,25 @@ struct ov5670_mode {
>  	/* Link frequency needed for this resolution */
>  	u32 link_freq_index;
>  
> +	/* Analog crop rectangle */
> +	const struct v4l2_rect analog_crop;
> +
>  	/* Sensor register settings for this resolution */
>  	const struct ov5670_reg_list reg_list;
>  };
>  
> +/*
> + * All the modes supported by the driver are obtained by subsampling the
> + * full pixel array. The below values are reflected in registers from
> + * 03800-0x3807 in the modes register-value tables.
> + */
> +static const struct v4l2_rect ov5670_analog_crop = {
> +	.left	= 12,
> +	.top	= 4,
> +	.width	= 2600,
> +	.height	= 1952,
> +};
> +
>  static const struct ov5670_reg mipi_data_rate_840mbps[] = {
>  	{0x0300, 0x04},
>  	{0x0301, 0x00},
> @@ -1764,66 +1783,73 @@ static const struct ov5670_mode supported_modes[] = {
>  		.height = 1944,
>  		.vts_def = OV5670_VTS_30FPS,
>  		.vts_min = OV5670_VTS_30FPS,
> +		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
> +		.analog_crop = ov5670_analog_crop,

The buildbot reported issues here.

You could turn analog_crop into a pointer to fix this.

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

>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
>  			.regs = mode_2592x1944_regs,
>  		},
> -		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
>  	},
>  	{
>  		.width = 1296,
>  		.height = 972,
>  		.vts_def = OV5670_VTS_30FPS,
>  		.vts_min = 996,
> +		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
> +		.analog_crop = ov5670_analog_crop,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(mode_1296x972_regs),
>  			.regs = mode_1296x972_regs,
>  		},
> -		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
>  	},
>  	{
>  		.width = 648,
>  		.height = 486,
>  		.vts_def = OV5670_VTS_30FPS,
>  		.vts_min = 516,
> +		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
> +		.analog_crop = ov5670_analog_crop,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(mode_648x486_regs),
>  			.regs = mode_648x486_regs,
>  		},
> -		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
>  	},
>  	{
>  		.width = 2560,
>  		.height = 1440,
>  		.vts_def = OV5670_VTS_30FPS,
>  		.vts_min = OV5670_VTS_30FPS,
> +		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
> +		.analog_crop = ov5670_analog_crop,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(mode_2560x1440_regs),
>  			.regs = mode_2560x1440_regs,
>  		},
> -		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
>  	},
>  	{
>  		.width = 1280,
>  		.height = 720,
>  		.vts_def = OV5670_VTS_30FPS,
>  		.vts_min = 1020,
> +
> +		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
> +		.analog_crop = ov5670_analog_crop,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
>  			.regs = mode_1280x720_regs,
>  		},
> -		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
>  	},
>  	{
>  		.width = 640,
>  		.height = 360,
>  		.vts_def = OV5670_VTS_30FPS,
>  		.vts_min = 510,
> +		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
> +		.analog_crop = ov5670_analog_crop,
>  		.reg_list = {
>  			.num_of_regs = ARRAY_SIZE(mode_640x360_regs),
>  			.regs = mode_640x360_regs,
>  		},
> -		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
>  	}
>  };
>  
> @@ -2163,6 +2189,7 @@ static int ov5670_init_cfg(struct v4l2_subdev *sd,
>  	struct v4l2_mbus_framefmt *fmt =
>  				v4l2_subdev_get_try_format(sd, state, 0);
>  	const struct ov5670_mode *default_mode = &supported_modes[0];
> +	struct v4l2_rect *crop = v4l2_subdev_get_try_crop(sd, state, 0);
>  
>  	fmt->width = default_mode->width;
>  	fmt->height = default_mode->height;
> @@ -2173,6 +2200,8 @@ static int ov5670_init_cfg(struct v4l2_subdev *sd,
>  	fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
>  	fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB);
>  
> +	*crop = default_mode->analog_crop;
> +
>  	return 0;
>  }
>  
> @@ -2492,6 +2521,52 @@ static const struct v4l2_subdev_core_ops ov5670_core_ops = {
>  	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
>  };
>  
> +static const struct v4l2_rect *
> +__ov5670_get_pad_crop(struct ov5670 *sensor, struct v4l2_subdev_state *state,
> +		      unsigned int pad, enum v4l2_subdev_format_whence which)
> +{
> +	const struct ov5670_mode *mode = sensor->cur_mode;
> +
> +	switch (which) {
> +	case V4L2_SUBDEV_FORMAT_TRY:
> +		return v4l2_subdev_get_try_crop(&sensor->sd, state, pad);
> +	case V4L2_SUBDEV_FORMAT_ACTIVE:
> +		return &mode->analog_crop;
> +	}
> +
> +	return NULL;
> +}
> +
> +static int ov5670_get_selection(struct v4l2_subdev *subdev,
> +				struct v4l2_subdev_state *state,
> +				struct v4l2_subdev_selection *sel)
> +{
> +	struct ov5670 *sensor = to_ov5670(subdev);
> +
> +	switch (sel->target) {
> +	case V4L2_SEL_TGT_CROP:
> +		mutex_lock(&sensor->mutex);
> +		sel->r = *__ov5670_get_pad_crop(sensor, state, sel->pad,
> +						sel->which);
> +		mutex_unlock(&sensor->mutex);
> +		break;
> +	case V4L2_SEL_TGT_NATIVE_SIZE:
> +	case V4L2_SEL_TGT_CROP_BOUNDS:
> +		sel->r.top = 0;
> +		sel->r.left = 0;
> +		sel->r.width = OV5670_NATIVE_WIDTH;
> +		sel->r.height = OV5670_NATIVE_HEIGHT;
> +		break;
> +	case V4L2_SEL_TGT_CROP_DEFAULT:
> +		sel->r = ov5670_analog_crop;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static const struct v4l2_subdev_video_ops ov5670_video_ops = {
>  	.s_stream = ov5670_set_stream,
>  };
> @@ -2502,6 +2577,8 @@ static const struct v4l2_subdev_pad_ops ov5670_pad_ops = {
>  	.get_fmt = ov5670_get_pad_format,
>  	.set_fmt = ov5670_set_pad_format,
>  	.enum_frame_size = ov5670_enum_frame_size,
> +	.get_selection = ov5670_get_selection,
> +	.set_selection = ov5670_get_selection,
>  };
>  
>  static const struct v4l2_subdev_sensor_ops ov5670_sensor_ops = {
Nathan Chancellor March 15, 2022, 4:55 p.m. UTC | #3 
On Tue, Mar 15, 2022 at 12:18:48PM +0800, kernel test robot wrote:
> Hi Jacopo,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on media-tree/master]
> [also build test ERROR on linus/master v5.17-rc8 next-20220310]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220315-003034
> base:   git://linuxtv.org/media_tree.git master
> config: i386-randconfig-a012-20220314 (https://download.01.org/0day-ci/archive/20220315/202203151238.pCKNarov-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 3e4950d7fa78ac83f33bbf1658e2f49a73719236)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/c619a8eee6477517dfaa05511344d0bddc4e1c55
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Jacopo-Mondi/media-i2c-ov5670-OF-support-runtime_pm-regulators/20220315-003034
>         git checkout c619a8eee6477517dfaa05511344d0bddc4e1c55
>         # save the config file to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/media/i2c/ov5670.c:1787:18: error: initializer element is not a compile-time constant
>                    .analog_crop = ov5670_analog_crop,
>                                   ^~~~~~~~~~~~~~~~~~
>    1 error generated.

GCC versions prior to 8 will complain about this as well:

$ gcc --version | head -1
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

$ make -j"$(nproc)" mrproper allmodconfig drivers/media/i2c/ov5670.o
drivers/media/i2c/ov5670.c:1787:18: error: initializer element is not constant
   .analog_crop = ov5670_analog_crop,
                  ^~~~~~~~~~~~~~~~~~
drivers/media/i2c/ov5670.c:1787:18: note: (near initialization for ‘supported_modes[0].analog_crop’)
drivers/media/i2c/ov5670.c:1799:18: error: initializer element is not constant
   .analog_crop = ov5670_analog_crop,
                  ^~~~~~~~~~~~~~~~~~
drivers/media/i2c/ov5670.c:1799:18: note: (near initialization for ‘supported_modes[1].analog_crop’)
drivers/media/i2c/ov5670.c:1811:18: error: initializer element is not constant
   .analog_crop = ov5670_analog_crop,
                  ^~~~~~~~~~~~~~~~~~
drivers/media/i2c/ov5670.c:1811:18: note: (near initialization for ‘supported_modes[2].analog_crop’)
drivers/media/i2c/ov5670.c:1823:18: error: initializer element is not constant
   .analog_crop = ov5670_analog_crop,
                  ^~~~~~~~~~~~~~~~~~
drivers/media/i2c/ov5670.c:1823:18: note: (near initialization for ‘supported_modes[3].analog_crop’)
drivers/media/i2c/ov5670.c:1836:18: error: initializer element is not constant
   .analog_crop = ov5670_analog_crop,
                  ^~~~~~~~~~~~~~~~~~
drivers/media/i2c/ov5670.c:1836:18: note: (near initialization for ‘supported_modes[4].analog_crop’)
drivers/media/i2c/ov5670.c:1848:18: error: initializer element is not constant
   .analog_crop = ov5670_analog_crop,
                  ^~~~~~~~~~~~~~~~~~
drivers/media/i2c/ov5670.c:1848:18: note: (near initialization for ‘supported_modes[5].analog_crop’)
scripts/Makefile.build:288: recipe for target 'drivers/media/i2c/ov5670.o' failed

clang may eventually support this: https://reviews.llvm.org/D76096

Cheers,
Nathan
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 9aa82774f8a6..67897dabb712 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -71,6 +71,10 @@ 
 #define OV5670_REG_VALUE_16BIT		2
 #define OV5670_REG_VALUE_24BIT		3
 
+/* Pixel Array */
+#define OV5670_NATIVE_WIDTH		2624
+#define OV5670_NATIVE_HEIGHT		1980
+
 /* Initial number of frames to skip to avoid possible garbage */
 #define OV5670_NUM_OF_SKIP_FRAMES	2
 
@@ -113,10 +117,25 @@  struct ov5670_mode {
 	/* Link frequency needed for this resolution */
 	u32 link_freq_index;
 
+	/* Analog crop rectangle */
+	const struct v4l2_rect analog_crop;
+
 	/* Sensor register settings for this resolution */
 	const struct ov5670_reg_list reg_list;
 };
 
+/*
+ * All the modes supported by the driver are obtained by subsampling the
+ * full pixel array. The below values are reflected in registers from
+ * 03800-0x3807 in the modes register-value tables.
+ */
+static const struct v4l2_rect ov5670_analog_crop = {
+	.left	= 12,
+	.top	= 4,
+	.width	= 2600,
+	.height	= 1952,
+};
+
 static const struct ov5670_reg mipi_data_rate_840mbps[] = {
 	{0x0300, 0x04},
 	{0x0301, 0x00},
@@ -1764,66 +1783,73 @@  static const struct ov5670_mode supported_modes[] = {
 		.height = 1944,
 		.vts_def = OV5670_VTS_30FPS,
 		.vts_min = OV5670_VTS_30FPS,
+		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
+		.analog_crop = ov5670_analog_crop,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_2592x1944_regs),
 			.regs = mode_2592x1944_regs,
 		},
-		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
 	},
 	{
 		.width = 1296,
 		.height = 972,
 		.vts_def = OV5670_VTS_30FPS,
 		.vts_min = 996,
+		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
+		.analog_crop = ov5670_analog_crop,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_1296x972_regs),
 			.regs = mode_1296x972_regs,
 		},
-		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
 	},
 	{
 		.width = 648,
 		.height = 486,
 		.vts_def = OV5670_VTS_30FPS,
 		.vts_min = 516,
+		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
+		.analog_crop = ov5670_analog_crop,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_648x486_regs),
 			.regs = mode_648x486_regs,
 		},
-		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
 	},
 	{
 		.width = 2560,
 		.height = 1440,
 		.vts_def = OV5670_VTS_30FPS,
 		.vts_min = OV5670_VTS_30FPS,
+		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
+		.analog_crop = ov5670_analog_crop,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_2560x1440_regs),
 			.regs = mode_2560x1440_regs,
 		},
-		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
 	},
 	{
 		.width = 1280,
 		.height = 720,
 		.vts_def = OV5670_VTS_30FPS,
 		.vts_min = 1020,
+
+		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
+		.analog_crop = ov5670_analog_crop,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
 			.regs = mode_1280x720_regs,
 		},
-		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
 	},
 	{
 		.width = 640,
 		.height = 360,
 		.vts_def = OV5670_VTS_30FPS,
 		.vts_min = 510,
+		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
+		.analog_crop = ov5670_analog_crop,
 		.reg_list = {
 			.num_of_regs = ARRAY_SIZE(mode_640x360_regs),
 			.regs = mode_640x360_regs,
 		},
-		.link_freq_index = OV5670_LINK_FREQ_422MHZ_INDEX,
 	}
 };
 
@@ -2163,6 +2189,7 @@  static int ov5670_init_cfg(struct v4l2_subdev *sd,
 	struct v4l2_mbus_framefmt *fmt =
 				v4l2_subdev_get_try_format(sd, state, 0);
 	const struct ov5670_mode *default_mode = &supported_modes[0];
+	struct v4l2_rect *crop = v4l2_subdev_get_try_crop(sd, state, 0);
 
 	fmt->width = default_mode->width;
 	fmt->height = default_mode->height;
@@ -2173,6 +2200,8 @@  static int ov5670_init_cfg(struct v4l2_subdev *sd,
 	fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
 	fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(V4L2_COLORSPACE_SRGB);
 
+	*crop = default_mode->analog_crop;
+
 	return 0;
 }
 
@@ -2492,6 +2521,52 @@  static const struct v4l2_subdev_core_ops ov5670_core_ops = {
 	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
 };
 
+static const struct v4l2_rect *
+__ov5670_get_pad_crop(struct ov5670 *sensor, struct v4l2_subdev_state *state,
+		      unsigned int pad, enum v4l2_subdev_format_whence which)
+{
+	const struct ov5670_mode *mode = sensor->cur_mode;
+
+	switch (which) {
+	case V4L2_SUBDEV_FORMAT_TRY:
+		return v4l2_subdev_get_try_crop(&sensor->sd, state, pad);
+	case V4L2_SUBDEV_FORMAT_ACTIVE:
+		return &mode->analog_crop;
+	}
+
+	return NULL;
+}
+
+static int ov5670_get_selection(struct v4l2_subdev *subdev,
+				struct v4l2_subdev_state *state,
+				struct v4l2_subdev_selection *sel)
+{
+	struct ov5670 *sensor = to_ov5670(subdev);
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP:
+		mutex_lock(&sensor->mutex);
+		sel->r = *__ov5670_get_pad_crop(sensor, state, sel->pad,
+						sel->which);
+		mutex_unlock(&sensor->mutex);
+		break;
+	case V4L2_SEL_TGT_NATIVE_SIZE:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = OV5670_NATIVE_WIDTH;
+		sel->r.height = OV5670_NATIVE_HEIGHT;
+		break;
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		sel->r = ov5670_analog_crop;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static const struct v4l2_subdev_video_ops ov5670_video_ops = {
 	.s_stream = ov5670_set_stream,
 };
@@ -2502,6 +2577,8 @@  static const struct v4l2_subdev_pad_ops ov5670_pad_ops = {
 	.get_fmt = ov5670_get_pad_format,
 	.set_fmt = ov5670_set_pad_format,
 	.enum_frame_size = ov5670_enum_frame_size,
+	.get_selection = ov5670_get_selection,
+	.set_selection = ov5670_get_selection,
 };
 
 static const struct v4l2_subdev_sensor_ops ov5670_sensor_ops = {