[RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch series converts the last remaining drivers that use g/s_crop and
cropcap to g/s_selection.

The first two patches do some minor code cleanup.

The third patch adds a new video_device flag to indicate that the driver
inverts the normal usage of g/s_crop/cropcap. This applies to the old
Samsung drivers that predate the Selection API and that abused the existing
crop API.

The next three patches do some code cleanup and prepare drivers for the
removal of g/s_crop and ensure that cropcap only returns the pixelaspect.

The next three patches convert the remaining Samsung drivers and set the
QUIRK flag for all three.

The final two patches remove vidioc_g/s_crop and rename vidioc_cropcap
to vidioc_g_pixelaspect.

I would really appreciate it if someone from Samsung can test these
three drivers or at the very least review the code.

Niklas, this series supersedes your 'v4l2-ioctl: fix CROPCAP type handling'
patch. Sorry about that :-)

Regards,

	Hans

Hans Verkuil (11):
  v4l2-ioctl: don't use CROP/COMPOSE_ACTIVE
  v4l2-common.h: put backwards compat defines under #ifndef __KERNEL__
  v4l2-ioctl: add QUIRK_INVERTED_CROP
  davinci/vpbe: drop unused g_cropcap
  cropcap/g_selection split
  exynos-gsc: replace v4l2_crop by v4l2_selection
  s5p_mfc_dec.c: convert g_crop to g_selection
  exynos4-is: convert g/s_crop to g/s_selection
  s5p-g2d: convert g/s_crop to g/s_selection
  v4l2-ioctl: remove unused vidioc_g/s_crop
  vidioc_cropcap -> vidioc_g_pixelaspect

 drivers/media/pci/bt8xx/bttv-driver.c         |  12 +-
 drivers/media/pci/cobalt/cobalt-v4l2.c        |  48 +++++--
 drivers/media/pci/cx18/cx18-ioctl.c           |  13 +-
 drivers/media/pci/cx23885/cx23885-video.c     |  40 ++++--
 drivers/media/pci/ivtv/ivtv-ioctl.c           |  17 +--
 drivers/media/pci/saa7134/saa7134-video.c     |  21 ++-
 drivers/media/platform/am437x/am437x-vpfe.c   |  31 ++---
 drivers/media/platform/davinci/vpbe.c         |  23 ----
 drivers/media/platform/davinci/vpbe_display.c |  10 +-
 drivers/media/platform/davinci/vpfe_capture.c |  12 +-
 drivers/media/platform/exynos-gsc/gsc-core.c  |  57 +++-----
 drivers/media/platform/exynos-gsc/gsc-core.h  |   3 +-
 drivers/media/platform/exynos-gsc/gsc-m2m.c   |  23 ++--
 drivers/media/platform/exynos4-is/fimc-core.h |   6 +-
 drivers/media/platform/exynos4-is/fimc-m2m.c  | 130 ++++++++++--------
 drivers/media/platform/rcar-vin/rcar-v4l2.c   |  10 +-
 drivers/media/platform/s5p-g2d/g2d.c          | 102 +++++++++-----
 drivers/media/platform/s5p-mfc/s5p_mfc.c      |   1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c  |  49 ++++---
 drivers/media/platform/vivid/vivid-core.c     |   9 +-
 drivers/media/platform/vivid/vivid-vid-cap.c  |  18 ++-
 drivers/media/platform/vivid/vivid-vid-cap.h  |   2 +-
 drivers/media/platform/vivid/vivid-vid-out.c  |  18 ++-
 drivers/media/platform/vivid/vivid-vid-out.h  |   2 +-
 drivers/media/usb/au0828/au0828-video.c       |  38 +++--
 drivers/media/usb/cpia2/cpia2_v4l.c           |  31 +++--
 drivers/media/usb/cx231xx/cx231xx-417.c       |  41 ++++--
 drivers/media/usb/cx231xx/cx231xx-video.c     |  41 ++++--
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c      |  13 +-
 drivers/media/v4l2-core/v4l2-dev.c            |   8 +-
 drivers/media/v4l2-core/v4l2-ioctl.c          |  44 ++++--
 include/media/davinci/vpbe.h                  |   4 -
 include/media/v4l2-dev.h                      |  13 +-
 include/media/v4l2-ioctl.h                    |  16 +--
 include/uapi/linux/v4l2-common.h              |  28 ++--
 35 files changed, 537 insertions(+), 397 deletions(-)

-- 
2.18.0

[RFC PATCH 01/11] v4l2-ioctl: don't use CROP/COMPOSE_ACTIVE

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Drop the deprecated _ACTIVE part.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 7de041bae84f..9c2370e4d05c 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2212,9 +2212,9 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
 
 	/* crop means compose for output devices */
 	if (V4L2_TYPE_IS_OUTPUT(p->type))
-		s.target = V4L2_SEL_TGT_COMPOSE_ACTIVE;
+		s.target = V4L2_SEL_TGT_COMPOSE;
 	else
-		s.target = V4L2_SEL_TGT_CROP_ACTIVE;
+		s.target = V4L2_SEL_TGT_CROP;
 
 	ret = v4l_g_selection(ops, file, fh, &s);
 
@@ -2239,9 +2239,9 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
 
 	/* crop means compose for output devices */
 	if (V4L2_TYPE_IS_OUTPUT(p->type))
-		s.target = V4L2_SEL_TGT_COMPOSE_ACTIVE;
+		s.target = V4L2_SEL_TGT_COMPOSE;
 	else
-		s.target = V4L2_SEL_TGT_CROP_ACTIVE;
+		s.target = V4L2_SEL_TGT_CROP;
 
 	return v4l_s_selection(ops, file, fh, &s);
 }
-- 
2.18.0

Re: [RFC PATCH 01/11] v4l2-ioctl: don't use CROP/COMPOSE_ACTIVE

[Niklas Söderlund]

Hi Hans,

Thanks for your work.

On 2018-10-05 09:49:01 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Drop the deprecated _ACTIVE part.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 7de041bae84f..9c2370e4d05c 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2212,9 +2212,9 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
>  
>  	/* crop means compose for output devices */
>  	if (V4L2_TYPE_IS_OUTPUT(p->type))
> -		s.target = V4L2_SEL_TGT_COMPOSE_ACTIVE;
> +		s.target = V4L2_SEL_TGT_COMPOSE;
>  	else
> -		s.target = V4L2_SEL_TGT_CROP_ACTIVE;
> +		s.target = V4L2_SEL_TGT_CROP;
>  
>  	ret = v4l_g_selection(ops, file, fh, &s);
>  
> @@ -2239,9 +2239,9 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
>  
>  	/* crop means compose for output devices */
>  	if (V4L2_TYPE_IS_OUTPUT(p->type))
> -		s.target = V4L2_SEL_TGT_COMPOSE_ACTIVE;
> +		s.target = V4L2_SEL_TGT_COMPOSE;
>  	else
> -		s.target = V4L2_SEL_TGT_CROP_ACTIVE;
> +		s.target = V4L2_SEL_TGT_CROP;
>  
>  	return v4l_s_selection(ops, file, fh, &s);
>  }
> -- 
> 2.18.0
> 

-- 
Regards,
Niklas Söderlund

Re: [RFC PATCH 01/11] v4l2-ioctl: don't use CROP/COMPOSE_ACTIVE

[Sylwester Nawrocki]

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Drop the deprecated _ACTIVE part.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[RFC PATCH 02/11] v4l2-common.h: put backwards compat defines under #ifndef __KERNEL__

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This ensures that they won't be used in kernel code.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 include/uapi/linux/v4l2-common.h | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
index 4f7b892377cd..7d21c1634b4d 100644
--- a/include/uapi/linux/v4l2-common.h
+++ b/include/uapi/linux/v4l2-common.h
@@ -79,24 +79,11 @@
 /* Current composing area plus all padding pixels */
 #define V4L2_SEL_TGT_COMPOSE_PADDED	0x0103
 
-/* Backward compatibility target definitions --- to be removed. */
-#define V4L2_SEL_TGT_CROP_ACTIVE	V4L2_SEL_TGT_CROP
-#define V4L2_SEL_TGT_COMPOSE_ACTIVE	V4L2_SEL_TGT_COMPOSE
-#define V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL	V4L2_SEL_TGT_CROP
-#define V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTUAL V4L2_SEL_TGT_COMPOSE
-#define V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS	V4L2_SEL_TGT_CROP_BOUNDS
-#define V4L2_SUBDEV_SEL_TGT_COMPOSE_BOUNDS V4L2_SEL_TGT_COMPOSE_BOUNDS
-
 /* Selection flags */
 #define V4L2_SEL_FLAG_GE		(1 << 0)
 #define V4L2_SEL_FLAG_LE		(1 << 1)
 #define V4L2_SEL_FLAG_KEEP_CONFIG	(1 << 2)
 
-/* Backward compatibility flag definitions --- to be removed. */
-#define V4L2_SUBDEV_SEL_FLAG_SIZE_GE	V4L2_SEL_FLAG_GE
-#define V4L2_SUBDEV_SEL_FLAG_SIZE_LE	V4L2_SEL_FLAG_LE
-#define V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG V4L2_SEL_FLAG_KEEP_CONFIG
-
 struct v4l2_edid {
 	__u32 pad;
 	__u32 start_block;
@@ -105,4 +92,19 @@ struct v4l2_edid {
 	__u8  *edid;
 };
 
+#ifndef __KERNEL__
+/* Backward compatibility target definitions --- to be removed. */
+#define V4L2_SEL_TGT_CROP_ACTIVE	V4L2_SEL_TGT_CROP
+#define V4L2_SEL_TGT_COMPOSE_ACTIVE	V4L2_SEL_TGT_COMPOSE
+#define V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL	V4L2_SEL_TGT_CROP
+#define V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTUAL V4L2_SEL_TGT_COMPOSE
+#define V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS	V4L2_SEL_TGT_CROP_BOUNDS
+#define V4L2_SUBDEV_SEL_TGT_COMPOSE_BOUNDS V4L2_SEL_TGT_COMPOSE_BOUNDS
+
+/* Backward compatibility flag definitions --- to be removed. */
+#define V4L2_SUBDEV_SEL_FLAG_SIZE_GE	V4L2_SEL_FLAG_GE
+#define V4L2_SUBDEV_SEL_FLAG_SIZE_LE	V4L2_SEL_FLAG_LE
+#define V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG V4L2_SEL_FLAG_KEEP_CONFIG
+#endif
+
 #endif /* __V4L2_COMMON__ */
-- 
2.18.0

Re: [RFC PATCH 02/11] v4l2-common.h: put backwards compat defines under #ifndef __KERNEL__

[Niklas Söderlund]

Hi Hans,

Thanks for your patch.

On 2018-10-05 09:49:02 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This ensures that they won't be used in kernel code.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  include/uapi/linux/v4l2-common.h | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
> index 4f7b892377cd..7d21c1634b4d 100644
> --- a/include/uapi/linux/v4l2-common.h
> +++ b/include/uapi/linux/v4l2-common.h
> @@ -79,24 +79,11 @@
>  /* Current composing area plus all padding pixels */
>  #define V4L2_SEL_TGT_COMPOSE_PADDED	0x0103
>  
> -/* Backward compatibility target definitions --- to be removed. */
> -#define V4L2_SEL_TGT_CROP_ACTIVE	V4L2_SEL_TGT_CROP
> -#define V4L2_SEL_TGT_COMPOSE_ACTIVE	V4L2_SEL_TGT_COMPOSE
> -#define V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL	V4L2_SEL_TGT_CROP
> -#define V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTUAL V4L2_SEL_TGT_COMPOSE
> -#define V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS	V4L2_SEL_TGT_CROP_BOUNDS
> -#define V4L2_SUBDEV_SEL_TGT_COMPOSE_BOUNDS V4L2_SEL_TGT_COMPOSE_BOUNDS
> -
>  /* Selection flags */
>  #define V4L2_SEL_FLAG_GE		(1 << 0)
>  #define V4L2_SEL_FLAG_LE		(1 << 1)
>  #define V4L2_SEL_FLAG_KEEP_CONFIG	(1 << 2)
>  
> -/* Backward compatibility flag definitions --- to be removed. */
> -#define V4L2_SUBDEV_SEL_FLAG_SIZE_GE	V4L2_SEL_FLAG_GE
> -#define V4L2_SUBDEV_SEL_FLAG_SIZE_LE	V4L2_SEL_FLAG_LE
> -#define V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG V4L2_SEL_FLAG_KEEP_CONFIG
> -
>  struct v4l2_edid {
>  	__u32 pad;
>  	__u32 start_block;
> @@ -105,4 +92,19 @@ struct v4l2_edid {
>  	__u8  *edid;
>  };
>  
> +#ifndef __KERNEL__
> +/* Backward compatibility target definitions --- to be removed. */
> +#define V4L2_SEL_TGT_CROP_ACTIVE	V4L2_SEL_TGT_CROP
> +#define V4L2_SEL_TGT_COMPOSE_ACTIVE	V4L2_SEL_TGT_COMPOSE
> +#define V4L2_SUBDEV_SEL_TGT_CROP_ACTUAL	V4L2_SEL_TGT_CROP
> +#define V4L2_SUBDEV_SEL_TGT_COMPOSE_ACTUAL V4L2_SEL_TGT_COMPOSE
> +#define V4L2_SUBDEV_SEL_TGT_CROP_BOUNDS	V4L2_SEL_TGT_CROP_BOUNDS
> +#define V4L2_SUBDEV_SEL_TGT_COMPOSE_BOUNDS V4L2_SEL_TGT_COMPOSE_BOUNDS
> +
> +/* Backward compatibility flag definitions --- to be removed. */
> +#define V4L2_SUBDEV_SEL_FLAG_SIZE_GE	V4L2_SEL_FLAG_GE
> +#define V4L2_SUBDEV_SEL_FLAG_SIZE_LE	V4L2_SEL_FLAG_LE
> +#define V4L2_SUBDEV_SEL_FLAG_KEEP_CONFIG V4L2_SEL_FLAG_KEEP_CONFIG
> +#endif
> +
>  #endif /* __V4L2_COMMON__ */
> -- 
> 2.18.0
> 

-- 
Regards,
Niklas Söderlund

[RFC PATCH 03/11] v4l2-ioctl: add QUIRK_INVERTED_CROP

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Some old Samsung drivers use the legacy crop API incorrectly:
the crop and compose targets are swapped. Normally VIDIOC_G_CROP
will return the CROP rectangle of a CAPTURE stream and the COMPOSE
rectangle of an OUTPUT stream.

The Samsung drivers do the opposite. Note that these drivers predate
the selection API.

If this 'QUIRK' flag is set, then the v4l2-ioctl core will swap
the CROP and COMPOSE targets as well.

That way backwards compatibility is ensured and we can convert the
Samsung drivers to the selection API.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 17 ++++++++++++++++-
 include/media/v4l2-dev.h             | 13 +++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 9c2370e4d05c..63a92285de39 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2200,6 +2200,7 @@ static int v4l_s_selection(const struct v4l2_ioctl_ops *ops,
 static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
+	struct video_device *vfd = video_devdata(file);
 	struct v4l2_crop *p = arg;
 	struct v4l2_selection s = {
 		.type = p->type,
@@ -2216,6 +2217,10 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
 	else
 		s.target = V4L2_SEL_TGT_CROP;
 
+	if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags))
+		s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
+			V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
+
 	ret = v4l_g_selection(ops, file, fh, &s);
 
 	/* copying results to old structure on success */
@@ -2227,6 +2232,7 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
 static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
+	struct video_device *vfd = video_devdata(file);
 	struct v4l2_crop *p = arg;
 	struct v4l2_selection s = {
 		.type = p->type,
@@ -2243,12 +2249,17 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
 	else
 		s.target = V4L2_SEL_TGT_CROP;
 
+	if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags))
+		s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
+			V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
+
 	return v4l_s_selection(ops, file, fh, &s);
 }
 
 static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
 				struct file *file, void *fh, void *arg)
 {
+	struct video_device *vfd = video_devdata(file);
 	struct v4l2_cropcap *p = arg;
 	struct v4l2_selection s = { .type = p->type };
 	int ret = 0;
@@ -2285,13 +2296,17 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
 	else
 		s.target = V4L2_SEL_TGT_CROP_BOUNDS;
 
+	if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags))
+		s.target = s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS ?
+			V4L2_SEL_TGT_CROP_BOUNDS : V4L2_SEL_TGT_COMPOSE_BOUNDS;
+
 	ret = v4l_g_selection(ops, file, fh, &s);
 	if (ret)
 		return ret;
 	p->bounds = s.r;
 
 	/* obtaining defrect */
-	if (V4L2_TYPE_IS_OUTPUT(p->type))
+	if (s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS)
 		s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
 	else
 		s.target = V4L2_SEL_TGT_CROP_DEFAULT;
diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
index 456ac13eca1d..48531e57cc5a 100644
--- a/include/media/v4l2-dev.h
+++ b/include/media/v4l2-dev.h
@@ -74,10 +74,19 @@ struct v4l2_ctrl_handler;
  *	indicates that file->private_data points to &struct v4l2_fh.
  *	This flag is set by the core when v4l2_fh_init() is called.
  *	All new drivers should use it.
+ * @V4L2_FL_QUIRK_INVERTED_CROP:
+ *	some old M2M drivers use g/s_crop/cropcap incorrectly: crop and
+ *	compose are swapped. If this flag is set, then the selection
+ *	targets are swapped in the g/s_crop/cropcap functions in v4l2-ioctl.c.
+ *	This allows those drivers to correctly implement the selection API,
+ *	but the old crop API will still work as expected in order to preserve
+ *	backwards compatibility.
+ *	Never set this flag for new drivers.
  */
 enum v4l2_video_device_flags {
-	V4L2_FL_REGISTERED	= 0,
-	V4L2_FL_USES_V4L2_FH	= 1,
+	V4L2_FL_REGISTERED		= 0,
+	V4L2_FL_USES_V4L2_FH		= 1,
+	V4L2_FL_QUIRK_INVERTED_CROP	= 2,
 };
 
 /* Priority helper functions */
-- 
2.18.0

Re: [RFC PATCH 03/11] v4l2-ioctl: add QUIRK_INVERTED_CROP

[Niklas Söderlund]

Hi Hans,

Thanks for your patch.

On 2018-10-05 09:49:03 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Some old Samsung drivers use the legacy crop API incorrectly:
> the crop and compose targets are swapped. Normally VIDIOC_G_CROP
> will return the CROP rectangle of a CAPTURE stream and the COMPOSE
> rectangle of an OUTPUT stream.
> 
> The Samsung drivers do the opposite. Note that these drivers predate
> the selection API.
> 
> If this 'QUIRK' flag is set, then the v4l2-ioctl core will swap
> the CROP and COMPOSE targets as well.
> 
> That way backwards compatibility is ensured and we can convert the
> Samsung drivers to the selection API.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

I understand the need for this quirk but it make my head hurt :-)

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 17 ++++++++++++++++-
>  include/media/v4l2-dev.h             | 13 +++++++++++--
>  2 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 9c2370e4d05c..63a92285de39 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2200,6 +2200,7 @@ static int v4l_s_selection(const struct v4l2_ioctl_ops *ops,
>  static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
>  				struct file *file, void *fh, void *arg)
>  {
> +	struct video_device *vfd = video_devdata(file);
>  	struct v4l2_crop *p = arg;
>  	struct v4l2_selection s = {
>  		.type = p->type,
> @@ -2216,6 +2217,10 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
>  	else
>  		s.target = V4L2_SEL_TGT_CROP;
>  
> +	if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags))
> +		s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
> +			V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
> +
>  	ret = v4l_g_selection(ops, file, fh, &s);
>  
>  	/* copying results to old structure on success */
> @@ -2227,6 +2232,7 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
>  static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
>  				struct file *file, void *fh, void *arg)
>  {
> +	struct video_device *vfd = video_devdata(file);
>  	struct v4l2_crop *p = arg;
>  	struct v4l2_selection s = {
>  		.type = p->type,
> @@ -2243,12 +2249,17 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
>  	else
>  		s.target = V4L2_SEL_TGT_CROP;
>  
> +	if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags))
> +		s.target = s.target == V4L2_SEL_TGT_COMPOSE ?
> +			V4L2_SEL_TGT_CROP : V4L2_SEL_TGT_COMPOSE;
> +
>  	return v4l_s_selection(ops, file, fh, &s);
>  }
>  
>  static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
>  				struct file *file, void *fh, void *arg)
>  {
> +	struct video_device *vfd = video_devdata(file);
>  	struct v4l2_cropcap *p = arg;
>  	struct v4l2_selection s = { .type = p->type };
>  	int ret = 0;
> @@ -2285,13 +2296,17 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
>  	else
>  		s.target = V4L2_SEL_TGT_CROP_BOUNDS;
>  
> +	if (test_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags))
> +		s.target = s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS ?
> +			V4L2_SEL_TGT_CROP_BOUNDS : V4L2_SEL_TGT_COMPOSE_BOUNDS;
> +
>  	ret = v4l_g_selection(ops, file, fh, &s);
>  	if (ret)
>  		return ret;
>  	p->bounds = s.r;
>  
>  	/* obtaining defrect */
> -	if (V4L2_TYPE_IS_OUTPUT(p->type))
> +	if (s.target == V4L2_SEL_TGT_COMPOSE_BOUNDS)
>  		s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
>  	else
>  		s.target = V4L2_SEL_TGT_CROP_DEFAULT;
> diff --git a/include/media/v4l2-dev.h b/include/media/v4l2-dev.h
> index 456ac13eca1d..48531e57cc5a 100644
> --- a/include/media/v4l2-dev.h
> +++ b/include/media/v4l2-dev.h
> @@ -74,10 +74,19 @@ struct v4l2_ctrl_handler;
>   *	indicates that file->private_data points to &struct v4l2_fh.
>   *	This flag is set by the core when v4l2_fh_init() is called.
>   *	All new drivers should use it.
> + * @V4L2_FL_QUIRK_INVERTED_CROP:
> + *	some old M2M drivers use g/s_crop/cropcap incorrectly: crop and
> + *	compose are swapped. If this flag is set, then the selection
> + *	targets are swapped in the g/s_crop/cropcap functions in v4l2-ioctl.c.
> + *	This allows those drivers to correctly implement the selection API,
> + *	but the old crop API will still work as expected in order to preserve
> + *	backwards compatibility.
> + *	Never set this flag for new drivers.
>   */
>  enum v4l2_video_device_flags {
> -	V4L2_FL_REGISTERED	= 0,
> -	V4L2_FL_USES_V4L2_FH	= 1,
> +	V4L2_FL_REGISTERED		= 0,
> +	V4L2_FL_USES_V4L2_FH		= 1,
> +	V4L2_FL_QUIRK_INVERTED_CROP	= 2,
>  };
>  
>  /* Priority helper functions */
> -- 
> 2.18.0
> 

-- 
Regards,
Niklas Söderlund

Re: [RFC PATCH 03/11] v4l2-ioctl: add QUIRK_INVERTED_CROP

[Sylwester Nawrocki]

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Some old Samsung drivers use the legacy crop API incorrectly:
> the crop and compose targets are swapped. Normally VIDIOC_G_CROP
> will return the CROP rectangle of a CAPTURE stream and the COMPOSE
> rectangle of an OUTPUT stream.
>
> The Samsung drivers do the opposite. Note that these drivers predate
> the selection API.
>
> If this 'QUIRK' flag is set, then the v4l2-ioctl core will swap
> the CROP and COMPOSE targets as well.
>
> That way backwards compatibility is ensured and we can convert the
> Samsung drivers to the selection API.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[RFC PATCH 04/11] davinci/vpbe: drop unused g_cropcap

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

This function/callback is never used. Drop it.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/davinci/vpbe.c | 23 -----------------------
 include/media/davinci/vpbe.h          |  4 ----
 2 files changed, 27 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
index 18c035ef84cf..e80d7806cc45 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -92,28 +92,6 @@ static int vpbe_find_encoder_sd_index(struct vpbe_config *cfg,
 	return -EINVAL;
 }
 
-/**
- * vpbe_g_cropcap - Get crop capabilities of the display
- * @vpbe_dev: vpbe device ptr
- * @cropcap: cropcap is a ptr to struct v4l2_cropcap
- *
- * Update the crop capabilities in crop cap for current
- * mode
- */
-static int vpbe_g_cropcap(struct vpbe_device *vpbe_dev,
-			  struct v4l2_cropcap *cropcap)
-{
-	if (!cropcap)
-		return -EINVAL;
-	cropcap->bounds.left = 0;
-	cropcap->bounds.top = 0;
-	cropcap->bounds.width = vpbe_dev->current_timings.xres;
-	cropcap->bounds.height = vpbe_dev->current_timings.yres;
-	cropcap->defrect = cropcap->bounds;
-
-	return 0;
-}
-
 /**
  * vpbe_enum_outputs - enumerate outputs
  * @vpbe_dev: vpbe device ptr
@@ -793,7 +771,6 @@ static void vpbe_deinitialize(struct device *dev, struct vpbe_device *vpbe_dev)
 }
 
 static const struct vpbe_device_ops vpbe_dev_ops = {
-	.g_cropcap = vpbe_g_cropcap,
 	.enum_outputs = vpbe_enum_outputs,
 	.set_output = vpbe_set_output,
 	.get_output = vpbe_get_output,
diff --git a/include/media/davinci/vpbe.h b/include/media/davinci/vpbe.h
index 79a566d7defd..5c31a7682492 100644
--- a/include/media/davinci/vpbe.h
+++ b/include/media/davinci/vpbe.h
@@ -100,10 +100,6 @@ struct vpbe_config {
 struct vpbe_device;
 
 struct vpbe_device_ops {
-	/* crop cap for the display */
-	int (*g_cropcap)(struct vpbe_device *vpbe_dev,
-			 struct v4l2_cropcap *cropcap);
-
 	/* Enumerate the outputs */
 	int (*enum_outputs)(struct vpbe_device *vpbe_dev,
 			    struct v4l2_output *output);
-- 
2.18.0

[RFC PATCH 05/11] cropcap/g_selection split

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

If g_selection is implemented, then the v4l2-ioctl cropcap code assumes
that cropcap just implements the pixelaspect part and that g_selection
provides the crop bounds and default rectangles.

There are still some drivers that only implement cropcap and not
g_selection. Split up cropcap into a cropcap and g_selection for those
drivers.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/cobalt/cobalt-v4l2.c      | 38 ++++++++++++++++++---
 drivers/media/pci/cx23885/cx23885-video.c   | 28 ++++++++++++---
 drivers/media/platform/am437x/am437x-vpfe.c | 18 +++++-----
 drivers/media/usb/au0828/au0828-video.c     | 30 ++++++++++++----
 drivers/media/usb/cpia2/cpia2_v4l.c         | 31 +++++++++--------
 drivers/media/usb/cx231xx/cx231xx-417.c     | 29 +++++++++++++---
 drivers/media/usb/cx231xx/cx231xx-video.c   | 29 +++++++++++++---
 7 files changed, 152 insertions(+), 51 deletions(-)

diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index 0525f5e1565b..4a0205aae4b4 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -1089,14 +1089,43 @@ static int cobalt_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cc)
 		timings = cea1080p60;
 	else
 		err = v4l2_subdev_call(s->sd, video, g_dv_timings, &timings);
-	if (!err) {
-		cc->bounds.width = cc->defrect.width = timings.bt.width;
-		cc->bounds.height = cc->defrect.height = timings.bt.height;
+	if (!err)
 		cc->pixelaspect = v4l2_dv_timings_aspect_ratio(&timings);
-	}
 	return err;
 }
 
+static int cobalt_g_selection(struct file *file, void *fh,
+			      struct v4l2_selection *sel)
+{
+	struct cobalt_stream *s = video_drvdata(file);
+	struct v4l2_dv_timings timings;
+	int err = 0;
+
+	if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	if (s->input == 1)
+		timings = cea1080p60;
+	else
+		err = v4l2_subdev_call(s->sd, video, g_dv_timings, &timings);
+
+	if (err)
+		return err;
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = timings.bt.width;
+		sel->r.height = timings.bt.height;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static const struct v4l2_ioctl_ops cobalt_ioctl_ops = {
 	.vidioc_querycap		= cobalt_querycap,
 	.vidioc_g_parm			= cobalt_g_parm,
@@ -1104,6 +1133,7 @@ static const struct v4l2_ioctl_ops cobalt_ioctl_ops = {
 	.vidioc_streamon		= vb2_ioctl_streamon,
 	.vidioc_streamoff		= vb2_ioctl_streamoff,
 	.vidioc_cropcap			= cobalt_cropcap,
+	.vidioc_g_selection		= cobalt_g_selection,
 	.vidioc_enum_input		= cobalt_enum_input,
 	.vidioc_g_input			= cobalt_g_input,
 	.vidioc_s_input			= cobalt_s_input,
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index 92d32a733f1b..a9844c4020ff 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -677,17 +677,34 @@ static int vidioc_cropcap(struct file *file, void *priv,
 	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	cc->bounds.left = 0;
-	cc->bounds.top = 0;
-	cc->bounds.width = 720;
-	cc->bounds.height = norm_maxh(dev->tvnorm);
-	cc->defrect = cc->bounds;
 	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
 	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
 
 	return 0;
 }
 
+static int vidioc_g_selection(struct file *file, void *fh,
+			      struct v4l2_selection *sel)
+{
+	struct cx23885_dev *dev = video_drvdata(file);
+
+	if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	switch (sel->target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		sel->r.top = 0;
+		sel->r.left = 0;
+		sel->r.width = 720;
+		sel->r.height = norm_maxh(dev->tvnorm);
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *id)
 {
 	struct cx23885_dev *dev = video_drvdata(file);
@@ -1123,6 +1140,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_streamon      = vb2_ioctl_streamon,
 	.vidioc_streamoff     = vb2_ioctl_streamoff,
 	.vidioc_cropcap       = vidioc_cropcap,
+	.vidioc_g_selection   = vidioc_g_selection,
 	.vidioc_s_std         = vidioc_s_std,
 	.vidioc_g_std         = vidioc_g_std,
 	.vidioc_enum_input    = vidioc_enum_input,
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index cac6aec0ffa7..6d44531092ec 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2091,13 +2091,6 @@ static int vpfe_cropcap(struct file *file, void *priv,
 	if (vpfe->std_index >= ARRAY_SIZE(vpfe_standards))
 		return -EINVAL;
 
-	memset(crop, 0, sizeof(struct v4l2_cropcap));
-
-	crop->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	crop->defrect.width = vpfe_standards[vpfe->std_index].width;
-	crop->bounds.width = crop->defrect.width;
-	crop->defrect.height = vpfe_standards[vpfe->std_index].height;
-	crop->bounds.height = crop->defrect.height;
 	crop->pixelaspect = vpfe_standards[vpfe->std_index].pixelaspect;
 
 	return 0;
@@ -2108,12 +2101,17 @@ vpfe_g_selection(struct file *file, void *fh, struct v4l2_selection *s)
 {
 	struct vpfe_device *vpfe = video_drvdata(file);
 
+	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
+	    vpfe->std_index >= ARRAY_SIZE(vpfe_standards))
+		return -EINVAL;
+
 	switch (s->target) {
 	case V4L2_SEL_TGT_CROP_BOUNDS:
 	case V4L2_SEL_TGT_CROP_DEFAULT:
-		s->r.left = s->r.top = 0;
-		s->r.width = vpfe->crop.width;
-		s->r.height = vpfe->crop.height;
+		s->r.left = 0;
+		s->r.top = 0;
+		s->r.width = vpfe_standards[vpfe->std_index].width;
+		s->r.height = vpfe_standards[vpfe->std_index].height;
 		break;
 
 	case V4L2_SEL_TGT_CROP:
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index efbf210147c7..d2250f594cf9 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1627,19 +1627,34 @@ static int vidioc_cropcap(struct file *file, void *priv,
 	dprintk(1, "%s called std_set %d dev_state %ld\n", __func__,
 		dev->std_set_in_tuner_core, dev->dev_state);
 
-	cc->bounds.left = 0;
-	cc->bounds.top = 0;
-	cc->bounds.width = dev->width;
-	cc->bounds.height = dev->height;
-
-	cc->defrect = cc->bounds;
-
 	cc->pixelaspect.numerator = 54;
 	cc->pixelaspect.denominator = 59;
 
 	return 0;
 }
 
+static int vidioc_g_selection(struct file *file, void *priv,
+			      struct v4l2_selection *s)
+{
+	struct au0828_dev *dev = video_drvdata(file);
+
+	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		s->r.left = 0;
+		s->r.top = 0;
+		s->r.width = dev->width;
+		s->r.height = dev->height;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 static int vidioc_g_register(struct file *file, void *priv,
 			     struct v4l2_dbg_register *reg)
@@ -1763,6 +1778,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_g_audio             = vidioc_g_audio,
 	.vidioc_s_audio             = vidioc_s_audio,
 	.vidioc_cropcap             = vidioc_cropcap,
+	.vidioc_g_selection         = vidioc_g_selection,
 
 	.vidioc_reqbufs             = vb2_ioctl_reqbufs,
 	.vidioc_create_bufs         = vb2_ioctl_create_bufs,
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c b/drivers/media/usb/cpia2/cpia2_v4l.c
index aa7f3c307b22..61f29c53e667 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -479,24 +479,25 @@ static int cpia2_g_fmt_vid_cap(struct file *file, void *fh,
  *
  *****************************************************************************/
 
-static int cpia2_cropcap(struct file *file, void *fh, struct v4l2_cropcap *c)
+static int cpia2_g_selection(struct file *file, void *fh,
+			     struct v4l2_selection *s)
 {
 	struct camera_data *cam = video_drvdata(file);
 
-	if (c->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
-	       return -EINVAL;
-
-	c->bounds.left = 0;
-	c->bounds.top = 0;
-	c->bounds.width = cam->width;
-	c->bounds.height = cam->height;
-	c->defrect.left = 0;
-	c->defrect.top = 0;
-	c->defrect.width = cam->width;
-	c->defrect.height = cam->height;
-	c->pixelaspect.numerator = 1;
-	c->pixelaspect.denominator = 1;
+	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
 
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		s->r.left = 0;
+		s->r.top = 0;
+		s->r.width = cam->width;
+		s->r.height = cam->height;
+		break;
+	default:
+		return -EINVAL;
+	}
 	return 0;
 }
 
@@ -1047,7 +1048,7 @@ static const struct v4l2_ioctl_ops cpia2_ioctl_ops = {
 	.vidioc_try_fmt_vid_cap		    = cpia2_try_fmt_vid_cap,
 	.vidioc_g_jpegcomp		    = cpia2_g_jpegcomp,
 	.vidioc_s_jpegcomp		    = cpia2_s_jpegcomp,
-	.vidioc_cropcap			    = cpia2_cropcap,
+	.vidioc_g_selection		    = cpia2_g_selection,
 	.vidioc_reqbufs			    = cpia2_reqbufs,
 	.vidioc_querybuf		    = cpia2_querybuf,
 	.vidioc_qbuf			    = cpia2_qbuf,
diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c
index f700ec35b7f3..f47d41de5ca7 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1510,17 +1510,35 @@ static int vidioc_cropcap(struct file *file, void *priv,
 	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	cc->bounds.left = 0;
-	cc->bounds.top = 0;
-	cc->bounds.width = dev->ts1.width;
-	cc->bounds.height = dev->ts1.height;
-	cc->defrect = cc->bounds;
 	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
 	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
 
 	return 0;
 }
 
+static int vidioc_g_selection(struct file *file, void *priv,
+			      struct v4l2_selection *s)
+{
+	struct cx231xx_fh *fh = priv;
+	struct cx231xx *dev = fh->dev;
+
+	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		s->r.left = 0;
+		s->r.top = 0;
+		s->r.width = dev->ts1.width;
+		s->r.height = dev->ts1.height;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int vidioc_g_std(struct file *file, void *fh0, v4l2_std_id *norm)
 {
 	struct cx231xx_fh  *fh  = file->private_data;
@@ -1866,6 +1884,7 @@ static const struct v4l2_ioctl_ops mpeg_ioctl_ops = {
 	.vidioc_s_input		 = cx231xx_s_input,
 	.vidioc_s_ctrl		 = vidioc_s_ctrl,
 	.vidioc_cropcap		 = vidioc_cropcap,
+	.vidioc_g_selection	 = vidioc_g_selection,
 	.vidioc_querycap	 = cx231xx_querycap,
 	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
 	.vidioc_g_fmt_vid_cap	 = vidioc_g_fmt_vid_cap,
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 29160df76cf7..a24a278bc586 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1492,17 +1492,35 @@ static int vidioc_cropcap(struct file *file, void *priv,
 	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	cc->bounds.left = 0;
-	cc->bounds.top = 0;
-	cc->bounds.width = dev->width;
-	cc->bounds.height = dev->height;
-	cc->defrect = cc->bounds;
 	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
 	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
 
 	return 0;
 }
 
+static int vidioc_g_selection(struct file *file, void *priv,
+			      struct v4l2_selection *s)
+{
+	struct cx231xx_fh *fh = priv;
+	struct cx231xx *dev = fh->dev;
+
+	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+		s->r.left = 0;
+		s->r.top = 0;
+		s->r.width = dev->width;
+		s->r.height = dev->height;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
 static int vidioc_streamon(struct file *file, void *priv,
 			   enum v4l2_buf_type type)
 {
@@ -2094,6 +2112,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_try_fmt_vbi_cap        = vidioc_try_fmt_vbi_cap,
 	.vidioc_s_fmt_vbi_cap          = vidioc_s_fmt_vbi_cap,
 	.vidioc_cropcap                = vidioc_cropcap,
+	.vidioc_g_selection            = vidioc_g_selection,
 	.vidioc_reqbufs                = vidioc_reqbufs,
 	.vidioc_querybuf               = vidioc_querybuf,
 	.vidioc_qbuf                   = vidioc_qbuf,
-- 
2.18.0

[RFC PATCH 06/11] exynos-gsc: replace v4l2_crop by v4l2_selection

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Replace the use of struct v4l2_crop by struct v4l2_selection.
Also drop the unused gsc_g_crop function.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 57 ++++++++------------
 drivers/media/platform/exynos-gsc/gsc-core.h |  3 +-
 drivers/media/platform/exynos-gsc/gsc-m2m.c  | 23 ++++----
 3 files changed, 33 insertions(+), 50 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 838c5c53de37..0fa3ec04ab7b 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -541,20 +541,7 @@ void gsc_check_crop_change(u32 tmp_w, u32 tmp_h, u32 *w, u32 *h)
 	}
 }
 
-int gsc_g_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr)
-{
-	struct gsc_frame *frame;
-
-	frame = ctx_get_frame(ctx, cr->type);
-	if (IS_ERR(frame))
-		return PTR_ERR(frame);
-
-	cr->c = frame->crop;
-
-	return 0;
-}
-
-int gsc_try_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr)
+int gsc_try_selection(struct gsc_ctx *ctx, struct v4l2_selection *s)
 {
 	struct gsc_frame *f;
 	struct gsc_dev *gsc = ctx->gsc_dev;
@@ -562,25 +549,25 @@ int gsc_try_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr)
 	u32 mod_x = 0, mod_y = 0, tmp_w, tmp_h;
 	u32 min_w, min_h, max_w, max_h;
 
-	if (cr->c.top < 0 || cr->c.left < 0) {
+	if (s->r.top < 0 || s->r.left < 0) {
 		pr_err("doesn't support negative values for top & left\n");
 		return -EINVAL;
 	}
-	pr_debug("user put w: %d, h: %d", cr->c.width, cr->c.height);
+	pr_debug("user put w: %d, h: %d", s->r.width, s->r.height);
 
-	if (cr->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		f = &ctx->d_frame;
-	else if (cr->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+	else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		f = &ctx->s_frame;
 	else
 		return -EINVAL;
 
 	max_w = f->f_width;
 	max_h = f->f_height;
-	tmp_w = cr->c.width;
-	tmp_h = cr->c.height;
+	tmp_w = s->r.width;
+	tmp_h = s->r.height;
 
-	if (V4L2_TYPE_IS_OUTPUT(cr->type)) {
+	if (V4L2_TYPE_IS_OUTPUT(s->type)) {
 		if ((is_yuv422(f->fmt->color) && f->fmt->num_comp == 1) ||
 		    is_rgb(f->fmt->color))
 			min_w = 32;
@@ -602,8 +589,8 @@ int gsc_try_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr)
 			max_h = f->f_width;
 			min_w = variant->pix_min->target_rot_en_w;
 			min_h = variant->pix_min->target_rot_en_h;
-			tmp_w = cr->c.height;
-			tmp_h = cr->c.width;
+			tmp_w = s->r.height;
+			tmp_h = s->r.width;
 		} else {
 			min_w = variant->pix_min->target_rot_dis_w;
 			min_h = variant->pix_min->target_rot_dis_h;
@@ -616,29 +603,29 @@ int gsc_try_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr)
 	v4l_bound_align_image(&tmp_w, min_w, max_w, mod_x,
 			      &tmp_h, min_h, max_h, mod_y, 0);
 
-	if (!V4L2_TYPE_IS_OUTPUT(cr->type) &&
-		(ctx->gsc_ctrls.rotate->val == 90 ||
-		ctx->gsc_ctrls.rotate->val == 270))
+	if (!V4L2_TYPE_IS_OUTPUT(s->type) &&
+	    (ctx->gsc_ctrls.rotate->val == 90 ||
+	     ctx->gsc_ctrls.rotate->val == 270))
 		gsc_check_crop_change(tmp_h, tmp_w,
-					&cr->c.width, &cr->c.height);
+					&s->r.width, &s->r.height);
 	else
 		gsc_check_crop_change(tmp_w, tmp_h,
-					&cr->c.width, &cr->c.height);
+					&s->r.width, &s->r.height);
 
 
 	/* adjust left/top if cropping rectangle is out of bounds */
 	/* Need to add code to algin left value with 2's multiple */
-	if (cr->c.left + tmp_w > max_w)
-		cr->c.left = max_w - tmp_w;
-	if (cr->c.top + tmp_h > max_h)
-		cr->c.top = max_h - tmp_h;
+	if (s->r.left + tmp_w > max_w)
+		s->r.left = max_w - tmp_w;
+	if (s->r.top + tmp_h > max_h)
+		s->r.top = max_h - tmp_h;
 
 	if ((is_yuv420(f->fmt->color) || is_yuv422(f->fmt->color)) &&
-		cr->c.left & 1)
-			cr->c.left -= 1;
+	    s->r.left & 1)
+		s->r.left -= 1;
 
 	pr_debug("Aligned l:%d, t:%d, w:%d, h:%d, f_w: %d, f_h: %d",
-	    cr->c.left, cr->c.top, cr->c.width, cr->c.height, max_w, max_h);
+		 s->r.left, s->r.top, s->r.width, s->r.height, max_w, max_h);
 
 	return 0;
 }
diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h
index 715d9c9d8d30..c81f0a17d286 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.h
+++ b/drivers/media/platform/exynos-gsc/gsc-core.h
@@ -392,8 +392,7 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f);
 void gsc_set_frame_size(struct gsc_frame *frame, int width, int height);
 int gsc_g_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f);
 void gsc_check_crop_change(u32 tmp_w, u32 tmp_h, u32 *w, u32 *h);
-int gsc_g_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr);
-int gsc_try_crop(struct gsc_ctx *ctx, struct v4l2_crop *cr);
+int gsc_try_selection(struct gsc_ctx *ctx, struct v4l2_selection *s);
 int gsc_cal_prescaler_ratio(struct gsc_variant *var, u32 src, u32 dst,
 							u32 *ratio);
 void gsc_get_prescaler_shfactor(u32 hratio, u32 vratio, u32 *sh);
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index cc5d690818e1..c757f5d98bcc 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -494,30 +494,27 @@ static int gsc_m2m_s_selection(struct file *file, void *fh,
 {
 	struct gsc_frame *frame;
 	struct gsc_ctx *ctx = fh_to_ctx(fh);
-	struct v4l2_crop cr;
 	struct gsc_variant *variant = ctx->gsc_dev->variant;
+	struct v4l2_selection sel = *s;
 	int ret;
 
-	cr.type = s->type;
-	cr.c = s->r;
-
 	if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
 	    (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
 		return -EINVAL;
 
-	ret = gsc_try_crop(ctx, &cr);
+	ret = gsc_try_selection(ctx, &sel);
 	if (ret)
 		return ret;
 
 	if (s->flags & V4L2_SEL_FLAG_LE &&
-	    !is_rectangle_enclosed(&cr.c, &s->r))
+	    !is_rectangle_enclosed(&sel.r, &s->r))
 		return -ERANGE;
 
 	if (s->flags & V4L2_SEL_FLAG_GE &&
-	    !is_rectangle_enclosed(&s->r, &cr.c))
+	    !is_rectangle_enclosed(&s->r, &sel.r))
 		return -ERANGE;
 
-	s->r = cr.c;
+	s->r = sel.r;
 
 	switch (s->target) {
 	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
@@ -539,15 +536,15 @@ static int gsc_m2m_s_selection(struct file *file, void *fh,
 	/* Check to see if scaling ratio is within supported range */
 	if (gsc_ctx_state_is_set(GSC_DST_FMT | GSC_SRC_FMT, ctx)) {
 		if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
-			ret = gsc_check_scaler_ratio(variant, cr.c.width,
-				cr.c.height, ctx->d_frame.crop.width,
+			ret = gsc_check_scaler_ratio(variant, sel.r.width,
+				sel.r.height, ctx->d_frame.crop.width,
 				ctx->d_frame.crop.height,
 				ctx->gsc_ctrls.rotate->val, ctx->out_path);
 		} else {
 			ret = gsc_check_scaler_ratio(variant,
 				ctx->s_frame.crop.width,
-				ctx->s_frame.crop.height, cr.c.width,
-				cr.c.height, ctx->gsc_ctrls.rotate->val,
+				ctx->s_frame.crop.height, sel.r.width,
+				sel.r.height, ctx->gsc_ctrls.rotate->val,
 				ctx->out_path);
 		}
 
@@ -557,7 +554,7 @@ static int gsc_m2m_s_selection(struct file *file, void *fh,
 		}
 	}
 
-	frame->crop = cr.c;
+	frame->crop = sel.r;
 
 	gsc_ctx_state_lock_set(GSC_PARAMS, ctx);
 	return 0;
-- 
2.18.0

Re: [RFC PATCH 06/11] exynos-gsc: replace v4l2_crop by v4l2_selection

[Sylwester Nawrocki]

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Replace the use of struct v4l2_crop by struct v4l2_selection.
> Also drop the unused gsc_g_crop function.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[RFC PATCH 07/11] s5p_mfc_dec.c: convert g_crop to g_selection

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

The g_crop really implemented composition for the CAPTURE stream.

Replace g_crop by g_selection and set the V4L2_FL_QUIRK_INVERTED_CROP
flag since this is one of the old drivers that predates the selection
API. Those old drivers allowed g_crop when it really shouldn't have
since g_crop returns a compose rectangle instead of a crop rectangle.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc.c     |  1 +
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c | 49 +++++++++++++-------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index 927a1235408d..8a5ba3bec3af 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1342,6 +1342,7 @@ static int s5p_mfc_probe(struct platform_device *pdev)
 	vfd->lock	= &dev->mfc_mutex;
 	vfd->v4l2_dev	= &dev->v4l2_dev;
 	vfd->vfl_dir	= VFL_DIR_M2M;
+	set_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags);
 	snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_DEC_NAME);
 	dev->vfd_dec	= vfd;
 	video_set_drvdata(vfd, dev);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 670ca869babb..eaaf1418b0fa 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -773,19 +773,23 @@ static const struct v4l2_ctrl_ops s5p_mfc_dec_ctrl_ops = {
 	.g_volatile_ctrl = s5p_mfc_dec_g_v_ctrl,
 };
 
-/* Get cropping information */
-static int vidioc_g_crop(struct file *file, void *priv,
-		struct v4l2_crop *cr)
+/* Get compose information */
+static int vidioc_g_selection(struct file *file, void *priv,
+			      struct v4l2_selection *s)
 {
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
 	struct s5p_mfc_dev *dev = ctx->dev;
 	u32 left, right, top, bottom;
+	u32 width, height;
+
+	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
 
 	if (ctx->state != MFCINST_HEAD_PARSED &&
 	    ctx->state != MFCINST_RUNNING &&
 	    ctx->state != MFCINST_FINISHING &&
 	    ctx->state != MFCINST_FINISHED) {
-		mfc_err("Can not get crop information\n");
+		mfc_err("Can not get compose information\n");
 		return -EINVAL;
 	}
 	if (ctx->src_fmt->fourcc == V4L2_PIX_FMT_H264) {
@@ -795,22 +799,33 @@ static int vidioc_g_crop(struct file *file, void *priv,
 		top = s5p_mfc_hw_call(dev->mfc_ops, get_crop_info_v, ctx);
 		bottom = top >> S5P_FIMV_SHARED_CROP_BOTTOM_SHIFT;
 		top = top & S5P_FIMV_SHARED_CROP_TOP_MASK;
-		cr->c.left = left;
-		cr->c.top = top;
-		cr->c.width = ctx->img_width - left - right;
-		cr->c.height = ctx->img_height - top - bottom;
-		mfc_debug(2, "Cropping info [h264]: l=%d t=%d w=%d h=%d (r=%d b=%d fw=%d fh=%d\n",
-			  left, top, cr->c.width, cr->c.height, right, bottom,
+		width = ctx->img_width - left - right;
+		height = ctx->img_height - top - bottom;
+		mfc_debug(2, "Composing info [h264]: l=%d t=%d w=%d h=%d (r=%d b=%d fw=%d fh=%d\n",
+			  left, top, s->r.width, s->r.height, right, bottom,
 			  ctx->buf_width, ctx->buf_height);
 	} else {
-		cr->c.left = 0;
-		cr->c.top = 0;
-		cr->c.width = ctx->img_width;
-		cr->c.height = ctx->img_height;
-		mfc_debug(2, "Cropping info: w=%d h=%d fw=%d fh=%d\n",
-			  cr->c.width,	cr->c.height, ctx->buf_width,
+		left = 0;
+		top = 0;
+		width = ctx->img_width;
+		height = ctx->img_height;
+		mfc_debug(2, "Composing info: w=%d h=%d fw=%d fh=%d\n",
+			  s->r.width, s->r.height, ctx->buf_width,
 			  ctx->buf_height);
 	}
+
+	switch (s->target) {
+	case V4L2_SEL_TGT_COMPOSE:
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		s->r.left = left;
+		s->r.top = top;
+		s->r.width = width;
+		s->r.height = height;
+		break;
+	default:
+		return -EINVAL;
+	}
 	return 0;
 }
 
@@ -887,7 +902,7 @@ static const struct v4l2_ioctl_ops s5p_mfc_dec_ioctl_ops = {
 	.vidioc_expbuf = vidioc_expbuf,
 	.vidioc_streamon = vidioc_streamon,
 	.vidioc_streamoff = vidioc_streamoff,
-	.vidioc_g_crop = vidioc_g_crop,
+	.vidioc_g_selection = vidioc_g_selection,
 	.vidioc_decoder_cmd = vidioc_decoder_cmd,
 	.vidioc_subscribe_event = vidioc_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
-- 
2.18.0

Re: [RFC PATCH 07/11] s5p_mfc_dec.c: convert g_crop to g_selection

[Sylwester Nawrocki]

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> The g_crop really implemented composition for the CAPTURE stream.
>
> Replace g_crop by g_selection and set the V4L2_FL_QUIRK_INVERTED_CROP
> flag since this is one of the old drivers that predates the selection
> API. Those old drivers allowed g_crop when it really shouldn't have
> since g_crop returns a compose rectangle instead of a crop rectangle.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[RFC PATCH 08/11] exynos4-is: convert g/s_crop to g/s_selection

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Replace g/s_crop by g/s_selection and set the V4L2_FL_QUIRK_INVERTED_CROP
flag since this is one of the old drivers that predates the selection
API. Those old drivers allowed g_crop when it really shouldn't have since
g_crop returns a compose rectangle instead of a crop rectangle for the
CAPTURE stream, and vice versa for the OUTPUT stream.

Also drop the now unused vidioc_cropcap.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/exynos4-is/fimc-core.h |   6 +-
 drivers/media/platform/exynos4-is/fimc-m2m.c  | 130 ++++++++++--------
 2 files changed, 79 insertions(+), 57 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-core.h b/drivers/media/platform/exynos4-is/fimc-core.h
index 82d514df97f0..9f751a5efd64 100644
--- a/drivers/media/platform/exynos4-is/fimc-core.h
+++ b/drivers/media/platform/exynos4-is/fimc-core.h
@@ -596,12 +596,14 @@ static inline struct fimc_frame *ctx_get_frame(struct fimc_ctx *ctx,
 {
 	struct fimc_frame *frame;
 
-	if (V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE == type) {
+	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ||
+	    type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
 		if (fimc_ctx_state_is_set(FIMC_CTX_M2M, ctx))
 			frame = &ctx->s_frame;
 		else
 			return ERR_PTR(-EINVAL);
-	} else if (V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE == type) {
+	} else if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
+		   type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
 		frame = &ctx->d_frame;
 	} else {
 		v4l2_err(ctx->fimc_dev->v4l2_dev,
diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
index a19f8b164a47..61c8177409cf 100644
--- a/drivers/media/platform/exynos4-is/fimc-m2m.c
+++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
@@ -383,60 +383,80 @@ static int fimc_m2m_s_fmt_mplane(struct file *file, void *fh,
 	return 0;
 }
 
-static int fimc_m2m_cropcap(struct file *file, void *fh,
-			    struct v4l2_cropcap *cr)
+static int fimc_m2m_g_selection(struct file *file, void *fh,
+				struct v4l2_selection *s)
 {
 	struct fimc_ctx *ctx = fh_to_ctx(fh);
 	struct fimc_frame *frame;
 
-	frame = ctx_get_frame(ctx, cr->type);
+	frame = ctx_get_frame(ctx, s->type);
 	if (IS_ERR(frame))
 		return PTR_ERR(frame);
 
-	cr->bounds.left = 0;
-	cr->bounds.top = 0;
-	cr->bounds.width = frame->o_width;
-	cr->bounds.height = frame->o_height;
-	cr->defrect = cr->bounds;
-
-	return 0;
-}
-
-static int fimc_m2m_g_crop(struct file *file, void *fh, struct v4l2_crop *cr)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	struct fimc_frame *frame;
-
-	frame = ctx_get_frame(ctx, cr->type);
-	if (IS_ERR(frame))
-		return PTR_ERR(frame);
-
-	cr->c.left = frame->offs_h;
-	cr->c.top = frame->offs_v;
-	cr->c.width = frame->width;
-	cr->c.height = frame->height;
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+			return -EINVAL;
+		break;
+	case V4L2_SEL_TGT_COMPOSE:
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
 
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP:
+	case V4L2_SEL_TGT_COMPOSE:
+		s->r.left = frame->offs_h;
+		s->r.top = frame->offs_v;
+		s->r.width = frame->width;
+		s->r.height = frame->height;
+		break;
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		s->r.left = 0;
+		s->r.top = 0;
+		s->r.width = frame->o_width;
+		s->r.height = frame->o_height;
+		break;
+	default:
+		return -EINVAL;
+	}
 	return 0;
 }
 
-static int fimc_m2m_try_crop(struct fimc_ctx *ctx, struct v4l2_crop *cr)
+static int fimc_m2m_try_selection(struct fimc_ctx *ctx,
+				  struct v4l2_selection *s)
 {
 	struct fimc_dev *fimc = ctx->fimc_dev;
 	struct fimc_frame *f;
 	u32 min_size, halign, depth = 0;
 	int i;
 
-	if (cr->c.top < 0 || cr->c.left < 0) {
+	if (s->r.top < 0 || s->r.left < 0) {
 		v4l2_err(&fimc->m2m.vfd,
 			"doesn't support negative values for top & left\n");
 		return -EINVAL;
 	}
-	if (cr->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+	if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
 		f = &ctx->d_frame;
-	else if (cr->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		if (s->target != V4L2_SEL_TGT_COMPOSE)
+			return -EINVAL;
+	} else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
 		f = &ctx->s_frame;
-	else
+		if (s->target != V4L2_SEL_TGT_CROP)
+			return -EINVAL;
+	} else {
 		return -EINVAL;
+	}
 
 	min_size = (f == &ctx->s_frame) ?
 		fimc->variant->min_inp_pixsize : fimc->variant->min_out_pixsize;
@@ -450,61 +470,61 @@ static int fimc_m2m_try_crop(struct fimc_ctx *ctx, struct v4l2_crop *cr)
 	for (i = 0; i < f->fmt->memplanes; i++)
 		depth += f->fmt->depth[i];
 
-	v4l_bound_align_image(&cr->c.width, min_size, f->o_width,
+	v4l_bound_align_image(&s->r.width, min_size, f->o_width,
 			      ffs(min_size) - 1,
-			      &cr->c.height, min_size, f->o_height,
+			      &s->r.height, min_size, f->o_height,
 			      halign, 64/(ALIGN(depth, 8)));
 
 	/* adjust left/top if cropping rectangle is out of bounds */
-	if (cr->c.left + cr->c.width > f->o_width)
-		cr->c.left = f->o_width - cr->c.width;
-	if (cr->c.top + cr->c.height > f->o_height)
-		cr->c.top = f->o_height - cr->c.height;
+	if (s->r.left + s->r.width > f->o_width)
+		s->r.left = f->o_width - s->r.width;
+	if (s->r.top + s->r.height > f->o_height)
+		s->r.top = f->o_height - s->r.height;
 
-	cr->c.left = round_down(cr->c.left, min_size);
-	cr->c.top  = round_down(cr->c.top, fimc->variant->hor_offs_align);
+	s->r.left = round_down(s->r.left, min_size);
+	s->r.top  = round_down(s->r.top, fimc->variant->hor_offs_align);
 
 	dbg("l:%d, t:%d, w:%d, h:%d, f_w: %d, f_h: %d",
-	    cr->c.left, cr->c.top, cr->c.width, cr->c.height,
+	    s->r.left, s->r.top, s->r.width, s->r.height,
 	    f->f_width, f->f_height);
 
 	return 0;
 }
 
-static int fimc_m2m_s_crop(struct file *file, void *fh, const struct v4l2_crop *crop)
+static int fimc_m2m_s_selection(struct file *file, void *fh,
+				struct v4l2_selection *s)
 {
 	struct fimc_ctx *ctx = fh_to_ctx(fh);
 	struct fimc_dev *fimc = ctx->fimc_dev;
-	struct v4l2_crop cr = *crop;
 	struct fimc_frame *f;
 	int ret;
 
-	ret = fimc_m2m_try_crop(ctx, &cr);
+	ret = fimc_m2m_try_selection(ctx, s);
 	if (ret)
 		return ret;
 
-	f = (cr.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) ?
+	f = (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) ?
 		&ctx->s_frame : &ctx->d_frame;
 
 	/* Check to see if scaling ratio is within supported range */
-	if (cr.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
-		ret = fimc_check_scaler_ratio(ctx, cr.c.width,
-				cr.c.height, ctx->d_frame.width,
+	if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+		ret = fimc_check_scaler_ratio(ctx, s->r.width,
+				s->r.height, ctx->d_frame.width,
 				ctx->d_frame.height, ctx->rotation);
 	} else {
 		ret = fimc_check_scaler_ratio(ctx, ctx->s_frame.width,
-				ctx->s_frame.height, cr.c.width,
-				cr.c.height, ctx->rotation);
+				ctx->s_frame.height, s->r.width,
+				s->r.height, ctx->rotation);
 	}
 	if (ret) {
 		v4l2_err(&fimc->m2m.vfd, "Out of scaler range\n");
 		return -EINVAL;
 	}
 
-	f->offs_h = cr.c.left;
-	f->offs_v = cr.c.top;
-	f->width  = cr.c.width;
-	f->height = cr.c.height;
+	f->offs_h = s->r.left;
+	f->offs_v = s->r.top;
+	f->width  = s->r.width;
+	f->height = s->r.height;
 
 	fimc_ctx_state_set(FIMC_PARAMS, ctx);
 
@@ -528,9 +548,8 @@ static const struct v4l2_ioctl_ops fimc_m2m_ioctl_ops = {
 	.vidioc_expbuf			= v4l2_m2m_ioctl_expbuf,
 	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
 	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
-	.vidioc_g_crop			= fimc_m2m_g_crop,
-	.vidioc_s_crop			= fimc_m2m_s_crop,
-	.vidioc_cropcap			= fimc_m2m_cropcap
+	.vidioc_g_selection		= fimc_m2m_g_selection,
+	.vidioc_s_selection		= fimc_m2m_s_selection,
 
 };
 
@@ -717,6 +736,7 @@ int fimc_register_m2m_device(struct fimc_dev *fimc,
 	vfd->release = video_device_release_empty;
 	vfd->lock = &fimc->lock;
 	vfd->vfl_dir = VFL_DIR_M2M;
+	set_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags);
 
 	snprintf(vfd->name, sizeof(vfd->name), "fimc.%d.m2m", fimc->id);
 	video_set_drvdata(vfd, fimc);
-- 
2.18.0

Re: [RFC PATCH 08/11] exynos4-is: convert g/s_crop to g/s_selection

[Sylwester Nawrocki]

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Replace g/s_crop by g/s_selection and set the V4L2_FL_QUIRK_INVERTED_CROP
> flag since this is one of the old drivers that predates the selection
> API. Those old drivers allowed g_crop when it really shouldn't have since
> g_crop returns a compose rectangle instead of a crop rectangle for the
> CAPTURE stream, and vice versa for the OUTPUT stream.
>
> Also drop the now unused vidioc_cropcap.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[RFC PATCH 09/11] s5p-g2d: convert g/s_crop to g/s_selection

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Replace g/s_crop by g/s_selection and set the V4L2_FL_QUIRK_INVERTED_CROP
flag since this is one of the old drivers that predates the selection
API. Those old drivers allowed g_crop when it really shouldn't have since
g_crop returns a compose rectangle instead of a crop rectangle for the
CAPTURE stream, and vice versa for the OUTPUT stream.

Also drop the now unused vidioc_cropcap.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/s5p-g2d/g2d.c | 102 +++++++++++++++++----------
 1 file changed, 64 insertions(+), 38 deletions(-)

diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
index e901201b6fcc..57ab1d1085d1 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -89,7 +89,7 @@ static struct g2d_fmt *find_fmt(struct v4l2_format *f)
 
 
 static struct g2d_frame *get_frame(struct g2d_ctx *ctx,
-							enum v4l2_buf_type type)
+				   enum v4l2_buf_type type)
 {
 	switch (type) {
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
@@ -408,51 +408,76 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	return 0;
 }
 
-static int vidioc_cropcap(struct file *file, void *priv,
-					struct v4l2_cropcap *cr)
-{
-	struct g2d_ctx *ctx = priv;
-	struct g2d_frame *f;
-
-	f = get_frame(ctx, cr->type);
-	if (IS_ERR(f))
-		return PTR_ERR(f);
-
-	cr->bounds.left		= 0;
-	cr->bounds.top		= 0;
-	cr->bounds.width	= f->width;
-	cr->bounds.height	= f->height;
-	cr->defrect		= cr->bounds;
-	return 0;
-}
-
-static int vidioc_g_crop(struct file *file, void *prv, struct v4l2_crop *cr)
+static int vidioc_g_selection(struct file *file, void *prv,
+			      struct v4l2_selection *s)
 {
 	struct g2d_ctx *ctx = prv;
 	struct g2d_frame *f;
 
-	f = get_frame(ctx, cr->type);
+	f = get_frame(ctx, s->type);
 	if (IS_ERR(f))
 		return PTR_ERR(f);
 
-	cr->c.left	= f->o_height;
-	cr->c.top	= f->o_width;
-	cr->c.width	= f->c_width;
-	cr->c.height	= f->c_height;
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+			return -EINVAL;
+		break;
+	case V4L2_SEL_TGT_COMPOSE:
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP:
+	case V4L2_SEL_TGT_COMPOSE:
+		s->r.left = f->o_height;
+		s->r.top = f->o_width;
+		s->r.width = f->c_width;
+		s->r.height = f->c_height;
+		break;
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+		s->r.left = 0;
+		s->r.top = 0;
+		s->r.width = f->width;
+		s->r.height = f->height;
+		break;
+	default:
+		return -EINVAL;
+	}
 	return 0;
 }
 
-static int vidioc_try_crop(struct file *file, void *prv, const struct v4l2_crop *cr)
+static int vidioc_try_selection(struct file *file, void *prv,
+				const struct v4l2_selection *s)
 {
 	struct g2d_ctx *ctx = prv;
 	struct g2d_dev *dev = ctx->dev;
 	struct g2d_frame *f;
 
-	f = get_frame(ctx, cr->type);
+	f = get_frame(ctx, s->type);
 	if (IS_ERR(f))
 		return PTR_ERR(f);
 
-	if (cr->c.top < 0 || cr->c.left < 0) {
+	if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+		if (s->target != V4L2_SEL_TGT_COMPOSE)
+			return -EINVAL;
+	} else if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+		if (s->target != V4L2_SEL_TGT_CROP)
+			return -EINVAL;
+	}
+
+	if (s->r.top < 0 || s->r.left < 0) {
 		v4l2_err(&dev->v4l2_dev,
 			"doesn't support negative values for top & left\n");
 		return -EINVAL;
@@ -461,23 +486,24 @@ static int vidioc_try_crop(struct file *file, void *prv, const struct v4l2_crop
 	return 0;
 }
 
-static int vidioc_s_crop(struct file *file, void *prv, const struct v4l2_crop *cr)
+static int vidioc_s_selection(struct file *file, void *prv,
+			      struct v4l2_selection *s)
 {
 	struct g2d_ctx *ctx = prv;
 	struct g2d_frame *f;
 	int ret;
 
-	ret = vidioc_try_crop(file, prv, cr);
+	ret = vidioc_try_selection(file, prv, s);
 	if (ret)
 		return ret;
-	f = get_frame(ctx, cr->type);
+	f = get_frame(ctx, s->type);
 	if (IS_ERR(f))
 		return PTR_ERR(f);
 
-	f->c_width	= cr->c.width;
-	f->c_height	= cr->c.height;
-	f->o_width	= cr->c.left;
-	f->o_height	= cr->c.top;
+	f->c_width	= s->r.width;
+	f->c_height	= s->r.height;
+	f->o_width	= s->r.left;
+	f->o_height	= s->r.top;
 	f->bottom	= f->o_height + f->c_height;
 	f->right	= f->o_width + f->c_width;
 	return 0;
@@ -585,9 +611,8 @@ static const struct v4l2_ioctl_ops g2d_ioctl_ops = {
 	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
 	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
 
-	.vidioc_g_crop			= vidioc_g_crop,
-	.vidioc_s_crop			= vidioc_s_crop,
-	.vidioc_cropcap			= vidioc_cropcap,
+	.vidioc_g_selection		= vidioc_g_selection,
+	.vidioc_s_selection		= vidioc_s_selection,
 };
 
 static const struct video_device g2d_videodev = {
@@ -680,6 +705,7 @@ static int g2d_probe(struct platform_device *pdev)
 		goto unreg_v4l2_dev;
 	}
 	*vfd = g2d_videodev;
+	set_bit(V4L2_FL_QUIRK_INVERTED_CROP, &vfd->flags);
 	vfd->lock = &dev->mutex;
 	vfd->v4l2_dev = &dev->v4l2_dev;
 	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
-- 
2.18.0

Re: [RFC PATCH 09/11] s5p-g2d: convert g/s_crop to g/s_selection

[Sylwester Nawrocki]

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Replace g/s_crop by g/s_selection and set the V4L2_FL_QUIRK_INVERTED_CROP
> flag since this is one of the old drivers that predates the selection
> API. Those old drivers allowed g_crop when it really shouldn't have since
> g_crop returns a compose rectangle instead of a crop rectangle for the
> CAPTURE stream, and vice versa for the OUTPUT stream.
>
> Also drop the now unused vidioc_cropcap.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[RFC PATCH 10/11] v4l2-ioctl: remove unused vidioc_g/s_crop

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Now that all drivers have dropped vidioc_g/s_crop we can remove
support for them in the V4L2 core.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/v4l2-dev.c   | 4 ++--
 drivers/media/v4l2-core/v4l2-ioctl.c | 4 ----
 include/media/v4l2-ioctl.h           | 8 --------
 3 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index 69e775930fc4..d81141d51faa 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -621,9 +621,9 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		SET_VALID_IOCTL(ops, VIDIOC_TRY_DECODER_CMD, vidioc_try_decoder_cmd);
 		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
 		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
-		if (ops->vidioc_g_crop || ops->vidioc_g_selection)
+		if (ops->vidioc_g_selection)
 			set_bit(_IOC_NR(VIDIOC_G_CROP), valid_ioctls);
-		if (ops->vidioc_s_crop || ops->vidioc_s_selection)
+		if (ops->vidioc_s_selection)
 			set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
 		SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
 		SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 63a92285de39..a59954d351a2 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2207,8 +2207,6 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
 	};
 	int ret;
 
-	if (ops->vidioc_g_crop)
-		return ops->vidioc_g_crop(file, fh, p);
 	/* simulate capture crop using selection api */
 
 	/* crop means compose for output devices */
@@ -2239,8 +2237,6 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
 		.r = p->c,
 	};
 
-	if (ops->vidioc_s_crop)
-		return ops->vidioc_s_crop(file, fh, p);
 	/* simulate capture crop using selection api */
 
 	/* crop means compose for output devices */
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 5848d92c30da..85fdd3f4b8ad 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -222,10 +222,6 @@ struct v4l2_fh;
  *	:ref:`VIDIOC_S_MODULATOR <vidioc_g_modulator>` ioctl
  * @vidioc_cropcap: pointer to the function that implements
  *	:ref:`VIDIOC_CROPCAP <vidioc_cropcap>` ioctl
- * @vidioc_g_crop: pointer to the function that implements
- *	:ref:`VIDIOC_G_CROP <vidioc_g_crop>` ioctl
- * @vidioc_s_crop: pointer to the function that implements
- *	:ref:`VIDIOC_S_CROP <vidioc_g_crop>` ioctl
  * @vidioc_g_selection: pointer to the function that implements
  *	:ref:`VIDIOC_G_SELECTION <vidioc_g_selection>` ioctl
  * @vidioc_s_selection: pointer to the function that implements
@@ -493,10 +489,6 @@ struct v4l2_ioctl_ops {
 	/* Crop ioctls */
 	int (*vidioc_cropcap)(struct file *file, void *fh,
 			      struct v4l2_cropcap *a);
-	int (*vidioc_g_crop)(struct file *file, void *fh,
-			     struct v4l2_crop *a);
-	int (*vidioc_s_crop)(struct file *file, void *fh,
-			     const struct v4l2_crop *a);
 	int (*vidioc_g_selection)(struct file *file, void *fh,
 				  struct v4l2_selection *s);
 	int (*vidioc_s_selection)(struct file *file, void *fh,
-- 
2.18.0

Re: [RFC PATCH 10/11] v4l2-ioctl: remove unused vidioc_g/s_crop

[Niklas Söderlund]

Hi Hans,

Thanks for your work.

On 2018-10-05 09:49:10 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Now that all drivers have dropped vidioc_g/s_crop we can remove
> support for them in the V4L2 core.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

If the quirk patch hurt my head this makes me smile!

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/v4l2-core/v4l2-dev.c   | 4 ++--
>  drivers/media/v4l2-core/v4l2-ioctl.c | 4 ----
>  include/media/v4l2-ioctl.h           | 8 --------
>  3 files changed, 2 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 69e775930fc4..d81141d51faa 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -621,9 +621,9 @@ static void determine_valid_ioctls(struct video_device *vdev)
>  		SET_VALID_IOCTL(ops, VIDIOC_TRY_DECODER_CMD, vidioc_try_decoder_cmd);
>  		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
>  		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
> -		if (ops->vidioc_g_crop || ops->vidioc_g_selection)
> +		if (ops->vidioc_g_selection)
>  			set_bit(_IOC_NR(VIDIOC_G_CROP), valid_ioctls);
> -		if (ops->vidioc_s_crop || ops->vidioc_s_selection)
> +		if (ops->vidioc_s_selection)
>  			set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
>  		SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
>  		SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 63a92285de39..a59954d351a2 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2207,8 +2207,6 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
>  	};
>  	int ret;
>  
> -	if (ops->vidioc_g_crop)
> -		return ops->vidioc_g_crop(file, fh, p);
>  	/* simulate capture crop using selection api */
>  
>  	/* crop means compose for output devices */
> @@ -2239,8 +2237,6 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
>  		.r = p->c,
>  	};
>  
> -	if (ops->vidioc_s_crop)
> -		return ops->vidioc_s_crop(file, fh, p);
>  	/* simulate capture crop using selection api */
>  
>  	/* crop means compose for output devices */
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 5848d92c30da..85fdd3f4b8ad 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -222,10 +222,6 @@ struct v4l2_fh;
>   *	:ref:`VIDIOC_S_MODULATOR <vidioc_g_modulator>` ioctl
>   * @vidioc_cropcap: pointer to the function that implements
>   *	:ref:`VIDIOC_CROPCAP <vidioc_cropcap>` ioctl
> - * @vidioc_g_crop: pointer to the function that implements
> - *	:ref:`VIDIOC_G_CROP <vidioc_g_crop>` ioctl
> - * @vidioc_s_crop: pointer to the function that implements
> - *	:ref:`VIDIOC_S_CROP <vidioc_g_crop>` ioctl
>   * @vidioc_g_selection: pointer to the function that implements
>   *	:ref:`VIDIOC_G_SELECTION <vidioc_g_selection>` ioctl
>   * @vidioc_s_selection: pointer to the function that implements
> @@ -493,10 +489,6 @@ struct v4l2_ioctl_ops {
>  	/* Crop ioctls */
>  	int (*vidioc_cropcap)(struct file *file, void *fh,
>  			      struct v4l2_cropcap *a);
> -	int (*vidioc_g_crop)(struct file *file, void *fh,
> -			     struct v4l2_crop *a);
> -	int (*vidioc_s_crop)(struct file *file, void *fh,
> -			     const struct v4l2_crop *a);
>  	int (*vidioc_g_selection)(struct file *file, void *fh,
>  				  struct v4l2_selection *s);
>  	int (*vidioc_s_selection)(struct file *file, void *fh,
> -- 
> 2.18.0
> 

-- 
Regards,
Niklas Söderlund

Re: [RFC PATCH 10/11] v4l2-ioctl: remove unused vidioc_g/s_crop

[Hans Verkuil]

On 10/05/18 12:47, Niklas Söderlund wrote:
> Hi Hans,
> 
> Thanks for your work.
> 
> On 2018-10-05 09:49:10 +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Now that all drivers have dropped vidioc_g/s_crop we can remove
>> support for them in the V4L2 core.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> 
> If the quirk patch hurt my head this makes me smile!

There you go, they cancel each other out in your head :-)

Regards,

	Hans

> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
>> ---
>>  drivers/media/v4l2-core/v4l2-dev.c   | 4 ++--
>>  drivers/media/v4l2-core/v4l2-ioctl.c | 4 ----
>>  include/media/v4l2-ioctl.h           | 8 --------
>>  3 files changed, 2 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
>> index 69e775930fc4..d81141d51faa 100644
>> --- a/drivers/media/v4l2-core/v4l2-dev.c
>> +++ b/drivers/media/v4l2-core/v4l2-dev.c
>> @@ -621,9 +621,9 @@ static void determine_valid_ioctls(struct video_device *vdev)
>>  		SET_VALID_IOCTL(ops, VIDIOC_TRY_DECODER_CMD, vidioc_try_decoder_cmd);
>>  		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
>>  		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
>> -		if (ops->vidioc_g_crop || ops->vidioc_g_selection)
>> +		if (ops->vidioc_g_selection)
>>  			set_bit(_IOC_NR(VIDIOC_G_CROP), valid_ioctls);
>> -		if (ops->vidioc_s_crop || ops->vidioc_s_selection)
>> +		if (ops->vidioc_s_selection)
>>  			set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
>>  		SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
>>  		SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 63a92285de39..a59954d351a2 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -2207,8 +2207,6 @@ static int v4l_g_crop(const struct v4l2_ioctl_ops *ops,
>>  	};
>>  	int ret;
>>  
>> -	if (ops->vidioc_g_crop)
>> -		return ops->vidioc_g_crop(file, fh, p);
>>  	/* simulate capture crop using selection api */
>>  
>>  	/* crop means compose for output devices */
>> @@ -2239,8 +2237,6 @@ static int v4l_s_crop(const struct v4l2_ioctl_ops *ops,
>>  		.r = p->c,
>>  	};
>>  
>> -	if (ops->vidioc_s_crop)
>> -		return ops->vidioc_s_crop(file, fh, p);
>>  	/* simulate capture crop using selection api */
>>  
>>  	/* crop means compose for output devices */
>> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
>> index 5848d92c30da..85fdd3f4b8ad 100644
>> --- a/include/media/v4l2-ioctl.h
>> +++ b/include/media/v4l2-ioctl.h
>> @@ -222,10 +222,6 @@ struct v4l2_fh;
>>   *	:ref:`VIDIOC_S_MODULATOR <vidioc_g_modulator>` ioctl
>>   * @vidioc_cropcap: pointer to the function that implements
>>   *	:ref:`VIDIOC_CROPCAP <vidioc_cropcap>` ioctl
>> - * @vidioc_g_crop: pointer to the function that implements
>> - *	:ref:`VIDIOC_G_CROP <vidioc_g_crop>` ioctl
>> - * @vidioc_s_crop: pointer to the function that implements
>> - *	:ref:`VIDIOC_S_CROP <vidioc_g_crop>` ioctl
>>   * @vidioc_g_selection: pointer to the function that implements
>>   *	:ref:`VIDIOC_G_SELECTION <vidioc_g_selection>` ioctl
>>   * @vidioc_s_selection: pointer to the function that implements
>> @@ -493,10 +489,6 @@ struct v4l2_ioctl_ops {
>>  	/* Crop ioctls */
>>  	int (*vidioc_cropcap)(struct file *file, void *fh,
>>  			      struct v4l2_cropcap *a);
>> -	int (*vidioc_g_crop)(struct file *file, void *fh,
>> -			     struct v4l2_crop *a);
>> -	int (*vidioc_s_crop)(struct file *file, void *fh,
>> -			     const struct v4l2_crop *a);
>>  	int (*vidioc_g_selection)(struct file *file, void *fh,
>>  				  struct v4l2_selection *s);
>>  	int (*vidioc_s_selection)(struct file *file, void *fh,
>> -- 
>> 2.18.0
>>
> 

Re: [RFC PATCH 10/11] v4l2-ioctl: remove unused vidioc_g/s_crop

[Sylwester Nawrocki]

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Now that all drivers have dropped vidioc_g/s_crop we can remove
> support for them in the V4L2 core.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

[RFC PATCH 11/11] vidioc_cropcap -> vidioc_g_pixelaspect

[Hans Verkuil]

From: Hans Verkuil <hans.verkuil@cisco.com>

Now vidioc_cropcap is only used to return the pixelaspect, so
rename it accordingly.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/bt8xx/bttv-driver.c         | 12 +++++------
 drivers/media/pci/cobalt/cobalt-v4l2.c        | 10 +++++----
 drivers/media/pci/cx18/cx18-ioctl.c           | 13 ++++++------
 drivers/media/pci/cx23885/cx23885-video.c     | 12 +++++------
 drivers/media/pci/ivtv/ivtv-ioctl.c           | 17 ++++++++-------
 drivers/media/pci/saa7134/saa7134-video.c     | 21 +++++++++----------
 drivers/media/platform/am437x/am437x-vpfe.c   | 13 ++++++------
 drivers/media/platform/davinci/vpbe_display.c | 10 ++++-----
 drivers/media/platform/davinci/vpfe_capture.c | 12 +++++------
 drivers/media/platform/rcar-vin/rcar-v4l2.c   | 10 ++++-----
 drivers/media/platform/vivid/vivid-core.c     |  9 ++++----
 drivers/media/platform/vivid/vivid-vid-cap.c  | 18 +++++++---------
 drivers/media/platform/vivid/vivid-vid-cap.h  |  2 +-
 drivers/media/platform/vivid/vivid-vid-out.c  | 18 +++++++---------
 drivers/media/platform/vivid/vivid-vid-out.h  |  2 +-
 drivers/media/usb/au0828/au0828-video.c       | 12 +++++------
 drivers/media/usb/cx231xx/cx231xx-417.c       | 12 +++++------
 drivers/media/usb/cx231xx/cx231xx-video.c     | 12 +++++------
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c      | 13 +++++++-----
 drivers/media/v4l2-core/v4l2-dev.c            |  6 +++---
 drivers/media/v4l2-core/v4l2-ioctl.c          | 15 +++++++------
 include/media/v4l2-ioctl.h                    |  8 +++----
 22 files changed, 131 insertions(+), 126 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
index b2cfcbb0008e..52cac1d3f577 100644
--- a/drivers/media/pci/bt8xx/bttv-driver.c
+++ b/drivers/media/pci/bt8xx/bttv-driver.c
@@ -2792,19 +2792,17 @@ static int bttv_g_tuner(struct file *file, void *priv,
 	return 0;
 }
 
-static int bttv_cropcap(struct file *file, void *priv,
-				struct v4l2_cropcap *cap)
+static int bttv_g_pixelaspect(struct file *file, void *priv,
+			      int type, struct v4l2_fract *f)
 {
 	struct bttv_fh *fh = priv;
 	struct bttv *btv = fh->btv;
 
-	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
-	    cap->type != V4L2_BUF_TYPE_VIDEO_OVERLAY)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
 	/* defrect and bounds are set via g_selection */
-	cap->pixelaspect = bttv_tvnorms[btv->tvnorm].cropcap.pixelaspect;
-
+	*f = bttv_tvnorms[btv->tvnorm].cropcap.pixelaspect;
 	return 0;
 }
 
@@ -3162,7 +3160,7 @@ static const struct v4l2_ioctl_ops bttv_ioctl_ops = {
 	.vidioc_g_fmt_vbi_cap           = bttv_g_fmt_vbi_cap,
 	.vidioc_try_fmt_vbi_cap         = bttv_try_fmt_vbi_cap,
 	.vidioc_s_fmt_vbi_cap           = bttv_s_fmt_vbi_cap,
-	.vidioc_cropcap                 = bttv_cropcap,
+	.vidioc_g_pixelaspect           = bttv_g_pixelaspect,
 	.vidioc_reqbufs                 = bttv_reqbufs,
 	.vidioc_querybuf                = bttv_querybuf,
 	.vidioc_qbuf                    = bttv_qbuf,
diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index 4a0205aae4b4..c088de551081 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -1077,20 +1077,22 @@ static int cobalt_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
 	return 0;
 }
 
-static int cobalt_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cc)
+static int cobalt_g_pixelaspect(struct file *file, void *fh,
+				int type, struct v4l2_fract *f)
 {
 	struct cobalt_stream *s = video_drvdata(file);
 	struct v4l2_dv_timings timings;
 	int err = 0;
 
-	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
+
 	if (s->input == 1)
 		timings = cea1080p60;
 	else
 		err = v4l2_subdev_call(s->sd, video, g_dv_timings, &timings);
 	if (!err)
-		cc->pixelaspect = v4l2_dv_timings_aspect_ratio(&timings);
+		*f = v4l2_dv_timings_aspect_ratio(&timings);
 	return err;
 }
 
@@ -1132,7 +1134,7 @@ static const struct v4l2_ioctl_ops cobalt_ioctl_ops = {
 	.vidioc_log_status		= cobalt_log_status,
 	.vidioc_streamon		= vb2_ioctl_streamon,
 	.vidioc_streamoff		= vb2_ioctl_streamoff,
-	.vidioc_cropcap			= cobalt_cropcap,
+	.vidioc_g_pixelaspect		= cobalt_g_pixelaspect,
 	.vidioc_g_selection		= cobalt_g_selection,
 	.vidioc_enum_input		= cobalt_enum_input,
 	.vidioc_g_input			= cobalt_g_input,
diff --git a/drivers/media/pci/cx18/cx18-ioctl.c b/drivers/media/pci/cx18/cx18-ioctl.c
index 854116375a7c..8c54b17f382a 100644
--- a/drivers/media/pci/cx18/cx18-ioctl.c
+++ b/drivers/media/pci/cx18/cx18-ioctl.c
@@ -441,15 +441,16 @@ static int cx18_enum_input(struct file *file, void *fh, struct v4l2_input *vin)
 	return cx18_get_input(cx, vin->index, vin);
 }
 
-static int cx18_cropcap(struct file *file, void *fh,
-			struct v4l2_cropcap *cropcap)
+static int cx18_g_pixelaspect(struct file *file, void *fh,
+			      int type, struct v4l2_fract *f)
 {
 	struct cx18 *cx = fh2id(fh)->cx;
 
-	if (cropcap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
-	cropcap->pixelaspect.numerator = cx->is_50hz ? 54 : 11;
-	cropcap->pixelaspect.denominator = cx->is_50hz ? 59 : 10;
+
+	f->numerator = cx->is_50hz ? 54 : 11;
+	f->denominator = cx->is_50hz ? 59 : 10;
 	return 0;
 }
 
@@ -1079,7 +1080,7 @@ static const struct v4l2_ioctl_ops cx18_ioctl_ops = {
 	.vidioc_g_audio                 = cx18_g_audio,
 	.vidioc_enumaudio               = cx18_enumaudio,
 	.vidioc_enum_input              = cx18_enum_input,
-	.vidioc_cropcap                 = cx18_cropcap,
+	.vidioc_g_pixelaspect           = cx18_g_pixelaspect,
 	.vidioc_g_selection             = cx18_g_selection,
 	.vidioc_g_input                 = cx18_g_input,
 	.vidioc_s_input                 = cx18_s_input,
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index a9844c4020ff..168178c1e574 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -668,17 +668,17 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
 	return 0;
 }
 
-static int vidioc_cropcap(struct file *file, void *priv,
-			  struct v4l2_cropcap *cc)
+static int vidioc_g_pixelaspect(struct file *file, void *priv,
+				int type, struct v4l2_fract *f)
 {
 	struct cx23885_dev *dev = video_drvdata(file);
 	bool is_50hz = dev->tvnorm & V4L2_STD_625_50;
 
-	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
-	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
+	f->numerator = is_50hz ? 54 : 11;
+	f->denominator = is_50hz ? 59 : 10;
 
 	return 0;
 }
@@ -1139,7 +1139,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_dqbuf         = vb2_ioctl_dqbuf,
 	.vidioc_streamon      = vb2_ioctl_streamon,
 	.vidioc_streamoff     = vb2_ioctl_streamoff,
-	.vidioc_cropcap       = vidioc_cropcap,
+	.vidioc_g_pixelaspect = vidioc_g_pixelaspect,
 	.vidioc_g_selection   = vidioc_g_selection,
 	.vidioc_s_std         = vidioc_s_std,
 	.vidioc_g_std         = vidioc_g_std,
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c b/drivers/media/pci/ivtv/ivtv-ioctl.c
index a66f8b872520..6c269ecd8d05 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -829,17 +829,18 @@ static int ivtv_enum_output(struct file *file, void *fh, struct v4l2_output *vou
 	return ivtv_get_output(itv, vout->index, vout);
 }
 
-static int ivtv_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cropcap)
+static int ivtv_g_pixelaspect(struct file *file, void *fh,
+			      int type, struct v4l2_fract *f)
 {
 	struct ivtv_open_id *id = fh2id(fh);
 	struct ivtv *itv = id->itv;
 
-	if (cropcap->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		cropcap->pixelaspect.numerator = itv->is_50hz ? 54 : 11;
-		cropcap->pixelaspect.denominator = itv->is_50hz ? 59 : 10;
-	} else if (cropcap->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
-		cropcap->pixelaspect.numerator = itv->is_out_50hz ? 54 : 11;
-		cropcap->pixelaspect.denominator = itv->is_out_50hz ? 59 : 10;
+	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+		f->numerator = itv->is_50hz ? 54 : 11;
+		f->denominator = itv->is_50hz ? 59 : 10;
+	} else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
+		f->numerator = itv->is_out_50hz ? 54 : 11;
+		f->denominator = itv->is_out_50hz ? 59 : 10;
 	} else {
 		return -EINVAL;
 	}
@@ -1923,7 +1924,7 @@ static const struct v4l2_ioctl_ops ivtv_ioctl_ops = {
 	.vidioc_enum_input		    = ivtv_enum_input,
 	.vidioc_enum_output		    = ivtv_enum_output,
 	.vidioc_enumaudout		    = ivtv_enumaudout,
-	.vidioc_cropcap			    = ivtv_cropcap,
+	.vidioc_g_pixelaspect		    = ivtv_g_pixelaspect,
 	.vidioc_s_selection		    = ivtv_s_selection,
 	.vidioc_g_selection		    = ivtv_g_selection,
 	.vidioc_g_input			    = ivtv_g_input,
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 1a22ae7cbdd9..8a02e442a21b 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -1650,23 +1650,22 @@ int saa7134_querystd(struct file *file, void *priv, v4l2_std_id *std)
 }
 EXPORT_SYMBOL_GPL(saa7134_querystd);
 
-static int saa7134_cropcap(struct file *file, void *priv,
-					struct v4l2_cropcap *cap)
+static int saa7134_g_pixelaspect(struct file *file, void *priv,
+				 int type, struct v4l2_fract *f)
 {
 	struct saa7134_dev *dev = video_drvdata(file);
 
-	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
-	    cap->type != V4L2_BUF_TYPE_VIDEO_OVERLAY)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+	    type != V4L2_BUF_TYPE_VIDEO_OVERLAY)
 		return -EINVAL;
-	cap->pixelaspect.numerator   = 1;
-	cap->pixelaspect.denominator = 1;
+
 	if (dev->tvnorm->id & V4L2_STD_525_60) {
-		cap->pixelaspect.numerator   = 11;
-		cap->pixelaspect.denominator = 10;
+		f->numerator   = 11;
+		f->denominator = 10;
 	}
 	if (dev->tvnorm->id & V4L2_STD_625_50) {
-		cap->pixelaspect.numerator   = 54;
-		cap->pixelaspect.denominator = 59;
+		f->numerator   = 54;
+		f->denominator = 59;
 	}
 	return 0;
 }
@@ -1987,7 +1986,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_g_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
 	.vidioc_try_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
 	.vidioc_s_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
-	.vidioc_cropcap			= saa7134_cropcap,
+	.vidioc_g_pixelaspect		= saa7134_g_pixelaspect,
 	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
 	.vidioc_querybuf		= vb2_ioctl_querybuf,
 	.vidioc_qbuf			= vb2_ioctl_qbuf,
diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
index 6d44531092ec..2b6882e6ca58 100644
--- a/drivers/media/platform/am437x/am437x-vpfe.c
+++ b/drivers/media/platform/am437x/am437x-vpfe.c
@@ -2081,17 +2081,18 @@ static void vpfe_stop_streaming(struct vb2_queue *vq)
 	spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags);
 }
 
-static int vpfe_cropcap(struct file *file, void *priv,
-			struct v4l2_cropcap *crop)
+static int vpfe_g_pixelaspect(struct file *file, void *priv,
+			      int type, struct v4l2_fract *f)
 {
 	struct vpfe_device *vpfe = video_drvdata(file);
 
-	vpfe_dbg(2, vpfe, "vpfe_cropcap\n");
+	vpfe_dbg(2, vpfe, "vpfe_g_pixelaspect\n");
 
-	if (vpfe->std_index >= ARRAY_SIZE(vpfe_standards))
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
+	    vpfe->std_index >= ARRAY_SIZE(vpfe_standards))
 		return -EINVAL;
 
-	crop->pixelaspect = vpfe_standards[vpfe->std_index].pixelaspect;
+	*f = vpfe_standards[vpfe->std_index].pixelaspect;
 
 	return 0;
 }
@@ -2280,7 +2281,7 @@ static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
 	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
 
-	.vidioc_cropcap			= vpfe_cropcap,
+	.vidioc_g_pixelaspect		= vpfe_g_pixelaspect,
 	.vidioc_g_selection		= vpfe_g_selection,
 	.vidioc_s_selection		= vpfe_s_selection,
 
diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
index 5c235898af7b..9e86b0d36640 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -759,18 +759,18 @@ static int vpbe_display_g_selection(struct file *file, void *priv,
 	return 0;
 }
 
-static int vpbe_display_cropcap(struct file *file, void *priv,
-			      struct v4l2_cropcap *cropcap)
+static int vpbe_display_g_pixelaspect(struct file *file, void *priv,
+				      int type, struct v4l2_fract *f)
 {
 	struct vpbe_layer *layer = video_drvdata(file);
 	struct vpbe_device *vpbe_dev = layer->disp_dev->vpbe_dev;
 
 	v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev, "VIDIOC_CROPCAP ioctl\n");
 
-	if (cropcap->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+	if (type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
-	cropcap->pixelaspect = vpbe_dev->current_timings.aspect;
+	*f = vpbe_dev->current_timings.aspect;
 	return 0;
 }
 
@@ -1263,7 +1263,7 @@ static const struct v4l2_ioctl_ops vpbe_ioctl_ops = {
 	.vidioc_streamoff	 = vb2_ioctl_streamoff,
 	.vidioc_expbuf		 = vb2_ioctl_expbuf,
 
-	.vidioc_cropcap		 = vpbe_display_cropcap,
+	.vidioc_g_pixelaspect	 = vpbe_display_g_pixelaspect,
 	.vidioc_g_selection	 = vpbe_display_g_selection,
 	.vidioc_s_selection	 = vpbe_display_s_selection,
 
diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
index ea3ddd5a42bd..9996bab98fe3 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1558,20 +1558,20 @@ static int vpfe_streamoff(struct file *file, void *priv,
 	return ret;
 }
 
-static int vpfe_cropcap(struct file *file, void *priv,
-			      struct v4l2_cropcap *crop)
+static int vpfe_g_pixelaspect(struct file *file, void *priv,
+			      int type, struct v4l2_fract *f)
 {
 	struct vpfe_device *vpfe_dev = video_drvdata(file);
 
-	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_cropcap\n");
+	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_g_pixelaspect\n");
 
-	if (crop->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 	/* If std_index is invalid, then just return (== 1:1 aspect) */
 	if (vpfe_dev->std_index >= ARRAY_SIZE(vpfe_standards))
 		return 0;
 
-	crop->pixelaspect = vpfe_standards[vpfe_dev->std_index].pixelaspect;
+	*f = vpfe_standards[vpfe_dev->std_index].pixelaspect;
 	return 0;
 }
 
@@ -1677,7 +1677,7 @@ static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
 	.vidioc_dqbuf		 = vpfe_dqbuf,
 	.vidioc_streamon	 = vpfe_streamon,
 	.vidioc_streamoff	 = vpfe_streamoff,
-	.vidioc_cropcap		 = vpfe_cropcap,
+	.vidioc_g_pixelaspect	 = vpfe_g_pixelaspect,
 	.vidioc_g_selection	 = vpfe_g_selection,
 	.vidioc_s_selection	 = vpfe_s_selection,
 };
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index dc77682b4785..7a2851790b91 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -404,16 +404,16 @@ static int rvin_s_selection(struct file *file, void *fh,
 	return 0;
 }
 
-static int rvin_cropcap(struct file *file, void *priv,
-			struct v4l2_cropcap *crop)
+static int rvin_g_pixelaspect(struct file *file, void *priv,
+			      int type, struct v4l2_fract *f)
 {
 	struct rvin_dev *vin = video_drvdata(file);
 	struct v4l2_subdev *sd = vin_to_source(vin);
 
-	if (crop->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	return v4l2_subdev_call(sd, video, g_pixelaspect, &crop->pixelaspect);
+	return v4l2_subdev_call(sd, video, g_pixelaspect, f);
 }
 
 static int rvin_enum_input(struct file *file, void *priv,
@@ -620,7 +620,7 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
 	.vidioc_g_selection		= rvin_g_selection,
 	.vidioc_s_selection		= rvin_s_selection,
 
-	.vidioc_cropcap			= rvin_cropcap,
+	.vidioc_g_pixelaspect		= rvin_g_pixelaspect,
 
 	.vidioc_enum_input		= rvin_enum_input,
 	.vidioc_g_input			= rvin_g_input,
diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index 06961e7d8036..9f7bdb2c1385 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -324,13 +324,14 @@ static int vidioc_s_dv_timings(struct file *file, void *fh, struct v4l2_dv_timin
 	return vivid_vid_out_s_dv_timings(file, fh, timings);
 }
 
-static int vidioc_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cc)
+static int vidioc_g_pixelaspect(struct file *file, void *fh,
+				int type, struct v4l2_fract *f)
 {
 	struct video_device *vdev = video_devdata(file);
 
 	if (vdev->vfl_dir == VFL_DIR_RX)
-		return vivid_vid_cap_cropcap(file, fh, cc);
-	return vivid_vid_out_cropcap(file, fh, cc);
+		return vivid_vid_cap_g_pixelaspect(file, fh, type, f);
+	return vivid_vid_out_g_pixelaspect(file, fh, type, f);
 }
 
 static int vidioc_g_selection(struct file *file, void *fh,
@@ -519,7 +520,7 @@ static const struct v4l2_ioctl_ops vivid_ioctl_ops = {
 
 	.vidioc_g_selection		= vidioc_g_selection,
 	.vidioc_s_selection		= vidioc_s_selection,
-	.vidioc_cropcap			= vidioc_cropcap,
+	.vidioc_g_pixelaspect		= vidioc_g_pixelaspect,
 
 	.vidioc_g_fmt_vbi_cap		= vidioc_g_fmt_vbi_cap,
 	.vidioc_try_fmt_vbi_cap		= vidioc_g_fmt_vbi_cap,
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
index 58e14dd1dcd3..7f0f89d7a5f6 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -1003,26 +1003,24 @@ int vivid_vid_cap_s_selection(struct file *file, void *fh, struct v4l2_selection
 	return 0;
 }
 
-int vivid_vid_cap_cropcap(struct file *file, void *priv,
-			      struct v4l2_cropcap *cap)
+int vivid_vid_cap_g_pixelaspect(struct file *file, void *priv,
+				int type, struct v4l2_fract *f)
 {
 	struct vivid_dev *dev = video_drvdata(file);
 
-	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
 	switch (vivid_get_pixel_aspect(dev)) {
 	case TPG_PIXEL_ASPECT_NTSC:
-		cap->pixelaspect.numerator = 11;
-		cap->pixelaspect.denominator = 10;
+		f->numerator = 11;
+		f->denominator = 10;
 		break;
 	case TPG_PIXEL_ASPECT_PAL:
-		cap->pixelaspect.numerator = 54;
-		cap->pixelaspect.denominator = 59;
+		f->numerator = 54;
+		f->denominator = 59;
 		break;
-	case TPG_PIXEL_ASPECT_SQUARE:
-		cap->pixelaspect.numerator = 1;
-		cap->pixelaspect.denominator = 1;
+	default:
 		break;
 	}
 	return 0;
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.h b/drivers/media/platform/vivid/vivid-vid-cap.h
index 47d8b48820df..1e422a59eeab 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.h
+++ b/drivers/media/platform/vivid/vivid-vid-cap.h
@@ -28,7 +28,7 @@ int vidioc_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
 int vidioc_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f);
 int vivid_vid_cap_g_selection(struct file *file, void *priv, struct v4l2_selection *sel);
 int vivid_vid_cap_s_selection(struct file *file, void *fh, struct v4l2_selection *s);
-int vivid_vid_cap_cropcap(struct file *file, void *priv, struct v4l2_cropcap *cap);
+int vivid_vid_cap_g_pixelaspect(struct file *file, void *priv, int type, struct v4l2_fract *f);
 int vidioc_enum_fmt_vid_overlay(struct file *file, void  *priv, struct v4l2_fmtdesc *f);
 int vidioc_g_fmt_vid_overlay(struct file *file, void *priv, struct v4l2_format *f);
 int vidioc_try_fmt_vid_overlay(struct file *file, void *priv, struct v4l2_format *f);
diff --git a/drivers/media/platform/vivid/vivid-vid-out.c b/drivers/media/platform/vivid/vivid-vid-out.c
index 50248e2176a0..164a4a7918d4 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.c
+++ b/drivers/media/platform/vivid/vivid-vid-out.c
@@ -785,26 +785,24 @@ int vivid_vid_out_s_selection(struct file *file, void *fh, struct v4l2_selection
 	return 0;
 }
 
-int vivid_vid_out_cropcap(struct file *file, void *priv,
-			      struct v4l2_cropcap *cap)
+int vivid_vid_out_g_pixelaspect(struct file *file, void *priv,
+				int type, struct v4l2_fract *f)
 {
 	struct vivid_dev *dev = video_drvdata(file);
 
-	if (cap->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+	if (type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
 		return -EINVAL;
 
 	switch (vivid_get_pixel_aspect(dev)) {
 	case TPG_PIXEL_ASPECT_NTSC:
-		cap->pixelaspect.numerator = 11;
-		cap->pixelaspect.denominator = 10;
+		f->numerator = 11;
+		f->denominator = 10;
 		break;
 	case TPG_PIXEL_ASPECT_PAL:
-		cap->pixelaspect.numerator = 54;
-		cap->pixelaspect.denominator = 59;
+		f->numerator = 54;
+		f->denominator = 59;
 		break;
-	case TPG_PIXEL_ASPECT_SQUARE:
-		cap->pixelaspect.numerator = 1;
-		cap->pixelaspect.denominator = 1;
+	default:
 		break;
 	}
 	return 0;
diff --git a/drivers/media/platform/vivid/vivid-vid-out.h b/drivers/media/platform/vivid/vivid-vid-out.h
index e87aacf843c5..8d56314f4ea1 100644
--- a/drivers/media/platform/vivid/vivid-vid-out.h
+++ b/drivers/media/platform/vivid/vivid-vid-out.h
@@ -23,7 +23,7 @@ int vidioc_try_fmt_vid_out(struct file *file, void *priv, struct v4l2_format *f)
 int vidioc_s_fmt_vid_out(struct file *file, void *priv, struct v4l2_format *f);
 int vivid_vid_out_g_selection(struct file *file, void *priv, struct v4l2_selection *sel);
 int vivid_vid_out_s_selection(struct file *file, void *fh, struct v4l2_selection *s);
-int vivid_vid_out_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cap);
+int vivid_vid_out_g_pixelaspect(struct file *file, void *priv, int type, struct v4l2_fract *f);
 int vidioc_enum_fmt_vid_out_overlay(struct file *file, void  *priv, struct v4l2_fmtdesc *f);
 int vidioc_g_fmt_vid_out_overlay(struct file *file, void *priv, struct v4l2_format *f);
 int vidioc_try_fmt_vid_out_overlay(struct file *file, void *priv, struct v4l2_format *f);
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index d2250f594cf9..7876c897cc1d 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1616,19 +1616,19 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int vidioc_cropcap(struct file *file, void *priv,
-			  struct v4l2_cropcap *cc)
+static int vidioc_g_pixelaspect(struct file *file, void *priv,
+				int type, struct v4l2_fract *f)
 {
 	struct au0828_dev *dev = video_drvdata(file);
 
-	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
 	dprintk(1, "%s called std_set %d dev_state %ld\n", __func__,
 		dev->std_set_in_tuner_core, dev->dev_state);
 
-	cc->pixelaspect.numerator = 54;
-	cc->pixelaspect.denominator = 59;
+	f->numerator = 54;
+	f->denominator = 59;
 
 	return 0;
 }
@@ -1777,7 +1777,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_enumaudio           = vidioc_enumaudio,
 	.vidioc_g_audio             = vidioc_g_audio,
 	.vidioc_s_audio             = vidioc_s_audio,
-	.vidioc_cropcap             = vidioc_cropcap,
+	.vidioc_g_pixelaspect       = vidioc_g_pixelaspect,
 	.vidioc_g_selection         = vidioc_g_selection,
 
 	.vidioc_reqbufs             = vb2_ioctl_reqbufs,
diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c
index f47d41de5ca7..e4ea4abcedab 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1500,18 +1500,18 @@ static const struct videobuf_queue_ops cx231xx_qops = {
 
 /* ------------------------------------------------------------------ */
 
-static int vidioc_cropcap(struct file *file, void *priv,
-			  struct v4l2_cropcap *cc)
+static int vidioc_g_pixelaspect(struct file *file, void *priv,
+				int type, struct v4l2_fract *f)
 {
 	struct cx231xx_fh *fh = priv;
 	struct cx231xx *dev = fh->dev;
 	bool is_50hz = dev->encodernorm.id & V4L2_STD_625_50;
 
-	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
-	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
+	f->numerator = is_50hz ? 54 : 11;
+	f->denominator = is_50hz ? 59 : 10;
 
 	return 0;
 }
@@ -1883,7 +1883,7 @@ static const struct v4l2_ioctl_ops mpeg_ioctl_ops = {
 	.vidioc_g_input		 = cx231xx_g_input,
 	.vidioc_s_input		 = cx231xx_s_input,
 	.vidioc_s_ctrl		 = vidioc_s_ctrl,
-	.vidioc_cropcap		 = vidioc_cropcap,
+	.vidioc_g_pixelaspect	 = vidioc_g_pixelaspect,
 	.vidioc_g_selection	 = vidioc_g_selection,
 	.vidioc_querycap	 = cx231xx_querycap,
 	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index a24a278bc586..503e3d8240aa 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1482,18 +1482,18 @@ int cx231xx_s_register(struct file *file, void *priv,
 }
 #endif
 
-static int vidioc_cropcap(struct file *file, void *priv,
-			  struct v4l2_cropcap *cc)
+static int vidioc_g_pixelaspect(struct file *file, void *priv,
+				int type, struct v4l2_fract *f)
 {
 	struct cx231xx_fh *fh = priv;
 	struct cx231xx *dev = fh->dev;
 	bool is_50hz = dev->norm & V4L2_STD_625_50;
 
-	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
-	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
-	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
+	f->numerator = is_50hz ? 54 : 11;
+	f->denominator = is_50hz ? 59 : 10;
 
 	return 0;
 }
@@ -2111,7 +2111,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_g_fmt_vbi_cap          = vidioc_g_fmt_vbi_cap,
 	.vidioc_try_fmt_vbi_cap        = vidioc_try_fmt_vbi_cap,
 	.vidioc_s_fmt_vbi_cap          = vidioc_s_fmt_vbi_cap,
-	.vidioc_cropcap                = vidioc_cropcap,
+	.vidioc_g_pixelaspect          = vidioc_g_pixelaspect,
 	.vidioc_g_selection            = vidioc_g_selection,
 	.vidioc_reqbufs                = vidioc_reqbufs,
 	.vidioc_querybuf               = vidioc_querybuf,
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index cea232a3302d..9b43f4e9da82 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -703,16 +703,19 @@ static int pvr2_try_ext_ctrls(struct file *file, void *priv,
 	return 0;
 }
 
-static int pvr2_cropcap(struct file *file, void *priv, struct v4l2_cropcap *cap)
+static int pvr2_g_pixelaspect(struct file *file, void *priv,
+			      int type, struct v4l2_fract *f)
 {
 	struct pvr2_v4l2_fh *fh = file->private_data;
 	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
+	struct v4l2_cropcap cap = { .type = type };
 	int ret;
 
-	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
-	ret = pvr2_hdw_get_cropcap(hdw, cap);
-	cap->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; /* paranoia */
+	ret = pvr2_hdw_get_cropcap(hdw, &cap);
+	if (!ret)
+		*f = cap.pixelaspect;
 	return ret;
 }
 
@@ -815,7 +818,7 @@ static const struct v4l2_ioctl_ops pvr2_ioctl_ops = {
 	.vidioc_g_audio			    = pvr2_g_audio,
 	.vidioc_enumaudio		    = pvr2_enumaudio,
 	.vidioc_enum_input		    = pvr2_enum_input,
-	.vidioc_cropcap			    = pvr2_cropcap,
+	.vidioc_g_pixelaspect		    = pvr2_g_pixelaspect,
 	.vidioc_s_selection		    = pvr2_s_selection,
 	.vidioc_g_selection		    = pvr2_g_selection,
 	.vidioc_g_input			    = pvr2_g_input,
diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index d81141d51faa..626ac06b94e5 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -621,14 +621,14 @@ static void determine_valid_ioctls(struct video_device *vdev)
 		SET_VALID_IOCTL(ops, VIDIOC_TRY_DECODER_CMD, vidioc_try_decoder_cmd);
 		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
 		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
-		if (ops->vidioc_g_selection)
+		if (ops->vidioc_g_selection) {
 			set_bit(_IOC_NR(VIDIOC_G_CROP), valid_ioctls);
+			set_bit(_IOC_NR(VIDIOC_CROPCAP), valid_ioctls);
+		}
 		if (ops->vidioc_s_selection)
 			set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
 		SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
 		SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
-		if (ops->vidioc_cropcap || ops->vidioc_g_selection)
-			set_bit(_IOC_NR(VIDIOC_CROPCAP), valid_ioctls);
 	} else if (is_vbi) {
 		/* vbi specific ioctls */
 		if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index a59954d351a2..26b199349fad 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2264,18 +2264,21 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
 	p->pixelaspect.numerator = 1;
 	p->pixelaspect.denominator = 1;
 
+	if (s.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		s.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	else if (s.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		s.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+
 	/*
 	 * The determine_valid_ioctls() call already should ensure
 	 * that this can never happen, but just in case...
 	 */
-	if (WARN_ON(!ops->vidioc_cropcap && !ops->vidioc_g_selection))
+	if (WARN_ON(!ops->vidioc_g_selection))
 		return -ENOTTY;
 
-	if (ops->vidioc_cropcap)
-		ret = ops->vidioc_cropcap(file, fh, p);
-
-	if (!ops->vidioc_g_selection)
-		return ret;
+	if (ops->vidioc_g_pixelaspect)
+		ret = ops->vidioc_g_pixelaspect(file, fh, s.type,
+						&p->pixelaspect);
 
 	/*
 	 * Ignore ENOTTY or ENOIOCTLCMD error returns, just use the
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index 85fdd3f4b8ad..aa4511aa5ffc 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -220,8 +220,8 @@ struct v4l2_fh;
  *	:ref:`VIDIOC_G_MODULATOR <vidioc_g_modulator>` ioctl
  * @vidioc_s_modulator: pointer to the function that implements
  *	:ref:`VIDIOC_S_MODULATOR <vidioc_g_modulator>` ioctl
- * @vidioc_cropcap: pointer to the function that implements
- *	:ref:`VIDIOC_CROPCAP <vidioc_cropcap>` ioctl
+ * @vidioc_g_pixelaspect: pointer to the function that implements
+ *	the pixelaspect part of the :ref:`VIDIOC_CROPCAP <vidioc_cropcap>` ioctl
  * @vidioc_g_selection: pointer to the function that implements
  *	:ref:`VIDIOC_G_SELECTION <vidioc_g_selection>` ioctl
  * @vidioc_s_selection: pointer to the function that implements
@@ -487,8 +487,8 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_s_modulator)(struct file *file, void *fh,
 				  const struct v4l2_modulator *a);
 	/* Crop ioctls */
-	int (*vidioc_cropcap)(struct file *file, void *fh,
-			      struct v4l2_cropcap *a);
+	int (*vidioc_g_pixelaspect)(struct file *file, void *fh,
+				    int buf_type, struct v4l2_fract *aspect);
 	int (*vidioc_g_selection)(struct file *file, void *fh,
 				  struct v4l2_selection *s);
 	int (*vidioc_s_selection)(struct file *file, void *fh,
-- 
2.18.0

Re: [RFC PATCH 11/11] vidioc_cropcap -> vidioc_g_pixelaspect

[Niklas Söderlund]

Hi Hans,

Thanks for your patch.

On 2018-10-05 09:49:11 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Now vidioc_cropcap is only used to return the pixelaspect, so
> rename it accordingly.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

For the v4l2 and rcar-vin changes:

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
>  drivers/media/pci/bt8xx/bttv-driver.c         | 12 +++++------
>  drivers/media/pci/cobalt/cobalt-v4l2.c        | 10 +++++----
>  drivers/media/pci/cx18/cx18-ioctl.c           | 13 ++++++------
>  drivers/media/pci/cx23885/cx23885-video.c     | 12 +++++------
>  drivers/media/pci/ivtv/ivtv-ioctl.c           | 17 ++++++++-------
>  drivers/media/pci/saa7134/saa7134-video.c     | 21 +++++++++----------
>  drivers/media/platform/am437x/am437x-vpfe.c   | 13 ++++++------
>  drivers/media/platform/davinci/vpbe_display.c | 10 ++++-----
>  drivers/media/platform/davinci/vpfe_capture.c | 12 +++++------
>  drivers/media/platform/rcar-vin/rcar-v4l2.c   | 10 ++++-----
>  drivers/media/platform/vivid/vivid-core.c     |  9 ++++----
>  drivers/media/platform/vivid/vivid-vid-cap.c  | 18 +++++++---------
>  drivers/media/platform/vivid/vivid-vid-cap.h  |  2 +-
>  drivers/media/platform/vivid/vivid-vid-out.c  | 18 +++++++---------
>  drivers/media/platform/vivid/vivid-vid-out.h  |  2 +-
>  drivers/media/usb/au0828/au0828-video.c       | 12 +++++------
>  drivers/media/usb/cx231xx/cx231xx-417.c       | 12 +++++------
>  drivers/media/usb/cx231xx/cx231xx-video.c     | 12 +++++------
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c      | 13 +++++++-----
>  drivers/media/v4l2-core/v4l2-dev.c            |  6 +++---
>  drivers/media/v4l2-core/v4l2-ioctl.c          | 15 +++++++------
>  include/media/v4l2-ioctl.h                    |  8 +++----
>  22 files changed, 131 insertions(+), 126 deletions(-)
> 
> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> index b2cfcbb0008e..52cac1d3f577 100644
> --- a/drivers/media/pci/bt8xx/bttv-driver.c
> +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> @@ -2792,19 +2792,17 @@ static int bttv_g_tuner(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int bttv_cropcap(struct file *file, void *priv,
> -				struct v4l2_cropcap *cap)
> +static int bttv_g_pixelaspect(struct file *file, void *priv,
> +			      int type, struct v4l2_fract *f)
>  {
>  	struct bttv_fh *fh = priv;
>  	struct bttv *btv = fh->btv;
>  
> -	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> -	    cap->type != V4L2_BUF_TYPE_VIDEO_OVERLAY)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
>  	/* defrect and bounds are set via g_selection */
> -	cap->pixelaspect = bttv_tvnorms[btv->tvnorm].cropcap.pixelaspect;
> -
> +	*f = bttv_tvnorms[btv->tvnorm].cropcap.pixelaspect;
>  	return 0;
>  }
>  
> @@ -3162,7 +3160,7 @@ static const struct v4l2_ioctl_ops bttv_ioctl_ops = {
>  	.vidioc_g_fmt_vbi_cap           = bttv_g_fmt_vbi_cap,
>  	.vidioc_try_fmt_vbi_cap         = bttv_try_fmt_vbi_cap,
>  	.vidioc_s_fmt_vbi_cap           = bttv_s_fmt_vbi_cap,
> -	.vidioc_cropcap                 = bttv_cropcap,
> +	.vidioc_g_pixelaspect           = bttv_g_pixelaspect,
>  	.vidioc_reqbufs                 = bttv_reqbufs,
>  	.vidioc_querybuf                = bttv_querybuf,
>  	.vidioc_qbuf                    = bttv_qbuf,
> diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
> index 4a0205aae4b4..c088de551081 100644
> --- a/drivers/media/pci/cobalt/cobalt-v4l2.c
> +++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
> @@ -1077,20 +1077,22 @@ static int cobalt_g_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
>  	return 0;
>  }
>  
> -static int cobalt_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cc)
> +static int cobalt_g_pixelaspect(struct file *file, void *fh,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct cobalt_stream *s = video_drvdata(file);
>  	struct v4l2_dv_timings timings;
>  	int err = 0;
>  
> -	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
> +
>  	if (s->input == 1)
>  		timings = cea1080p60;
>  	else
>  		err = v4l2_subdev_call(s->sd, video, g_dv_timings, &timings);
>  	if (!err)
> -		cc->pixelaspect = v4l2_dv_timings_aspect_ratio(&timings);
> +		*f = v4l2_dv_timings_aspect_ratio(&timings);
>  	return err;
>  }
>  
> @@ -1132,7 +1134,7 @@ static const struct v4l2_ioctl_ops cobalt_ioctl_ops = {
>  	.vidioc_log_status		= cobalt_log_status,
>  	.vidioc_streamon		= vb2_ioctl_streamon,
>  	.vidioc_streamoff		= vb2_ioctl_streamoff,
> -	.vidioc_cropcap			= cobalt_cropcap,
> +	.vidioc_g_pixelaspect		= cobalt_g_pixelaspect,
>  	.vidioc_g_selection		= cobalt_g_selection,
>  	.vidioc_enum_input		= cobalt_enum_input,
>  	.vidioc_g_input			= cobalt_g_input,
> diff --git a/drivers/media/pci/cx18/cx18-ioctl.c b/drivers/media/pci/cx18/cx18-ioctl.c
> index 854116375a7c..8c54b17f382a 100644
> --- a/drivers/media/pci/cx18/cx18-ioctl.c
> +++ b/drivers/media/pci/cx18/cx18-ioctl.c
> @@ -441,15 +441,16 @@ static int cx18_enum_input(struct file *file, void *fh, struct v4l2_input *vin)
>  	return cx18_get_input(cx, vin->index, vin);
>  }
>  
> -static int cx18_cropcap(struct file *file, void *fh,
> -			struct v4l2_cropcap *cropcap)
> +static int cx18_g_pixelaspect(struct file *file, void *fh,
> +			      int type, struct v4l2_fract *f)
>  {
>  	struct cx18 *cx = fh2id(fh)->cx;
>  
> -	if (cropcap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
> -	cropcap->pixelaspect.numerator = cx->is_50hz ? 54 : 11;
> -	cropcap->pixelaspect.denominator = cx->is_50hz ? 59 : 10;
> +
> +	f->numerator = cx->is_50hz ? 54 : 11;
> +	f->denominator = cx->is_50hz ? 59 : 10;
>  	return 0;
>  }
>  
> @@ -1079,7 +1080,7 @@ static const struct v4l2_ioctl_ops cx18_ioctl_ops = {
>  	.vidioc_g_audio                 = cx18_g_audio,
>  	.vidioc_enumaudio               = cx18_enumaudio,
>  	.vidioc_enum_input              = cx18_enum_input,
> -	.vidioc_cropcap                 = cx18_cropcap,
> +	.vidioc_g_pixelaspect           = cx18_g_pixelaspect,
>  	.vidioc_g_selection             = cx18_g_selection,
>  	.vidioc_g_input                 = cx18_g_input,
>  	.vidioc_s_input                 = cx18_s_input,
> diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
> index a9844c4020ff..168178c1e574 100644
> --- a/drivers/media/pci/cx23885/cx23885-video.c
> +++ b/drivers/media/pci/cx23885/cx23885-video.c
> @@ -668,17 +668,17 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
>  	return 0;
>  }
>  
> -static int vidioc_cropcap(struct file *file, void *priv,
> -			  struct v4l2_cropcap *cc)
> +static int vidioc_g_pixelaspect(struct file *file, void *priv,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct cx23885_dev *dev = video_drvdata(file);
>  	bool is_50hz = dev->tvnorm & V4L2_STD_625_50;
>  
> -	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
> -	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
> -	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
> +	f->numerator = is_50hz ? 54 : 11;
> +	f->denominator = is_50hz ? 59 : 10;
>  
>  	return 0;
>  }
> @@ -1139,7 +1139,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
>  	.vidioc_dqbuf         = vb2_ioctl_dqbuf,
>  	.vidioc_streamon      = vb2_ioctl_streamon,
>  	.vidioc_streamoff     = vb2_ioctl_streamoff,
> -	.vidioc_cropcap       = vidioc_cropcap,
> +	.vidioc_g_pixelaspect = vidioc_g_pixelaspect,
>  	.vidioc_g_selection   = vidioc_g_selection,
>  	.vidioc_s_std         = vidioc_s_std,
>  	.vidioc_g_std         = vidioc_g_std,
> diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c b/drivers/media/pci/ivtv/ivtv-ioctl.c
> index a66f8b872520..6c269ecd8d05 100644
> --- a/drivers/media/pci/ivtv/ivtv-ioctl.c
> +++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
> @@ -829,17 +829,18 @@ static int ivtv_enum_output(struct file *file, void *fh, struct v4l2_output *vou
>  	return ivtv_get_output(itv, vout->index, vout);
>  }
>  
> -static int ivtv_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cropcap)
> +static int ivtv_g_pixelaspect(struct file *file, void *fh,
> +			      int type, struct v4l2_fract *f)
>  {
>  	struct ivtv_open_id *id = fh2id(fh);
>  	struct ivtv *itv = id->itv;
>  
> -	if (cropcap->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -		cropcap->pixelaspect.numerator = itv->is_50hz ? 54 : 11;
> -		cropcap->pixelaspect.denominator = itv->is_50hz ? 59 : 10;
> -	} else if (cropcap->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> -		cropcap->pixelaspect.numerator = itv->is_out_50hz ? 54 : 11;
> -		cropcap->pixelaspect.denominator = itv->is_out_50hz ? 59 : 10;
> +	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +		f->numerator = itv->is_50hz ? 54 : 11;
> +		f->denominator = itv->is_50hz ? 59 : 10;
> +	} else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> +		f->numerator = itv->is_out_50hz ? 54 : 11;
> +		f->denominator = itv->is_out_50hz ? 59 : 10;
>  	} else {
>  		return -EINVAL;
>  	}
> @@ -1923,7 +1924,7 @@ static const struct v4l2_ioctl_ops ivtv_ioctl_ops = {
>  	.vidioc_enum_input		    = ivtv_enum_input,
>  	.vidioc_enum_output		    = ivtv_enum_output,
>  	.vidioc_enumaudout		    = ivtv_enumaudout,
> -	.vidioc_cropcap			    = ivtv_cropcap,
> +	.vidioc_g_pixelaspect		    = ivtv_g_pixelaspect,
>  	.vidioc_s_selection		    = ivtv_s_selection,
>  	.vidioc_g_selection		    = ivtv_g_selection,
>  	.vidioc_g_input			    = ivtv_g_input,
> diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
> index 1a22ae7cbdd9..8a02e442a21b 100644
> --- a/drivers/media/pci/saa7134/saa7134-video.c
> +++ b/drivers/media/pci/saa7134/saa7134-video.c
> @@ -1650,23 +1650,22 @@ int saa7134_querystd(struct file *file, void *priv, v4l2_std_id *std)
>  }
>  EXPORT_SYMBOL_GPL(saa7134_querystd);
>  
> -static int saa7134_cropcap(struct file *file, void *priv,
> -					struct v4l2_cropcap *cap)
> +static int saa7134_g_pixelaspect(struct file *file, void *priv,
> +				 int type, struct v4l2_fract *f)
>  {
>  	struct saa7134_dev *dev = video_drvdata(file);
>  
> -	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> -	    cap->type != V4L2_BUF_TYPE_VIDEO_OVERLAY)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> +	    type != V4L2_BUF_TYPE_VIDEO_OVERLAY)
>  		return -EINVAL;
> -	cap->pixelaspect.numerator   = 1;
> -	cap->pixelaspect.denominator = 1;
> +
>  	if (dev->tvnorm->id & V4L2_STD_525_60) {
> -		cap->pixelaspect.numerator   = 11;
> -		cap->pixelaspect.denominator = 10;
> +		f->numerator   = 11;
> +		f->denominator = 10;
>  	}
>  	if (dev->tvnorm->id & V4L2_STD_625_50) {
> -		cap->pixelaspect.numerator   = 54;
> -		cap->pixelaspect.denominator = 59;
> +		f->numerator   = 54;
> +		f->denominator = 59;
>  	}
>  	return 0;
>  }
> @@ -1987,7 +1986,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
>  	.vidioc_g_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
>  	.vidioc_try_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
>  	.vidioc_s_fmt_vbi_cap		= saa7134_try_get_set_fmt_vbi_cap,
> -	.vidioc_cropcap			= saa7134_cropcap,
> +	.vidioc_g_pixelaspect		= saa7134_g_pixelaspect,
>  	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
>  	.vidioc_querybuf		= vb2_ioctl_querybuf,
>  	.vidioc_qbuf			= vb2_ioctl_qbuf,
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c b/drivers/media/platform/am437x/am437x-vpfe.c
> index 6d44531092ec..2b6882e6ca58 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -2081,17 +2081,18 @@ static void vpfe_stop_streaming(struct vb2_queue *vq)
>  	spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags);
>  }
>  
> -static int vpfe_cropcap(struct file *file, void *priv,
> -			struct v4l2_cropcap *crop)
> +static int vpfe_g_pixelaspect(struct file *file, void *priv,
> +			      int type, struct v4l2_fract *f)
>  {
>  	struct vpfe_device *vpfe = video_drvdata(file);
>  
> -	vpfe_dbg(2, vpfe, "vpfe_cropcap\n");
> +	vpfe_dbg(2, vpfe, "vpfe_g_pixelaspect\n");
>  
> -	if (vpfe->std_index >= ARRAY_SIZE(vpfe_standards))
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
> +	    vpfe->std_index >= ARRAY_SIZE(vpfe_standards))
>  		return -EINVAL;
>  
> -	crop->pixelaspect = vpfe_standards[vpfe->std_index].pixelaspect;
> +	*f = vpfe_standards[vpfe->std_index].pixelaspect;
>  
>  	return 0;
>  }
> @@ -2280,7 +2281,7 @@ static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
>  	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
>  	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
>  
> -	.vidioc_cropcap			= vpfe_cropcap,
> +	.vidioc_g_pixelaspect		= vpfe_g_pixelaspect,
>  	.vidioc_g_selection		= vpfe_g_selection,
>  	.vidioc_s_selection		= vpfe_s_selection,
>  
> diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
> index 5c235898af7b..9e86b0d36640 100644
> --- a/drivers/media/platform/davinci/vpbe_display.c
> +++ b/drivers/media/platform/davinci/vpbe_display.c
> @@ -759,18 +759,18 @@ static int vpbe_display_g_selection(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int vpbe_display_cropcap(struct file *file, void *priv,
> -			      struct v4l2_cropcap *cropcap)
> +static int vpbe_display_g_pixelaspect(struct file *file, void *priv,
> +				      int type, struct v4l2_fract *f)
>  {
>  	struct vpbe_layer *layer = video_drvdata(file);
>  	struct vpbe_device *vpbe_dev = layer->disp_dev->vpbe_dev;
>  
>  	v4l2_dbg(1, debug, &vpbe_dev->v4l2_dev, "VIDIOC_CROPCAP ioctl\n");
>  
> -	if (cropcap->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +	if (type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
>  		return -EINVAL;
>  
> -	cropcap->pixelaspect = vpbe_dev->current_timings.aspect;
> +	*f = vpbe_dev->current_timings.aspect;
>  	return 0;
>  }
>  
> @@ -1263,7 +1263,7 @@ static const struct v4l2_ioctl_ops vpbe_ioctl_ops = {
>  	.vidioc_streamoff	 = vb2_ioctl_streamoff,
>  	.vidioc_expbuf		 = vb2_ioctl_expbuf,
>  
> -	.vidioc_cropcap		 = vpbe_display_cropcap,
> +	.vidioc_g_pixelaspect	 = vpbe_display_g_pixelaspect,
>  	.vidioc_g_selection	 = vpbe_display_g_selection,
>  	.vidioc_s_selection	 = vpbe_display_s_selection,
>  
> diff --git a/drivers/media/platform/davinci/vpfe_capture.c b/drivers/media/platform/davinci/vpfe_capture.c
> index ea3ddd5a42bd..9996bab98fe3 100644
> --- a/drivers/media/platform/davinci/vpfe_capture.c
> +++ b/drivers/media/platform/davinci/vpfe_capture.c
> @@ -1558,20 +1558,20 @@ static int vpfe_streamoff(struct file *file, void *priv,
>  	return ret;
>  }
>  
> -static int vpfe_cropcap(struct file *file, void *priv,
> -			      struct v4l2_cropcap *crop)
> +static int vpfe_g_pixelaspect(struct file *file, void *priv,
> +			      int type, struct v4l2_fract *f)
>  {
>  	struct vpfe_device *vpfe_dev = video_drvdata(file);
>  
> -	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_cropcap\n");
> +	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_g_pixelaspect\n");
>  
> -	if (crop->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  	/* If std_index is invalid, then just return (== 1:1 aspect) */
>  	if (vpfe_dev->std_index >= ARRAY_SIZE(vpfe_standards))
>  		return 0;
>  
> -	crop->pixelaspect = vpfe_standards[vpfe_dev->std_index].pixelaspect;
> +	*f = vpfe_standards[vpfe_dev->std_index].pixelaspect;
>  	return 0;
>  }
>  
> @@ -1677,7 +1677,7 @@ static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
>  	.vidioc_dqbuf		 = vpfe_dqbuf,
>  	.vidioc_streamon	 = vpfe_streamon,
>  	.vidioc_streamoff	 = vpfe_streamoff,
> -	.vidioc_cropcap		 = vpfe_cropcap,
> +	.vidioc_g_pixelaspect	 = vpfe_g_pixelaspect,
>  	.vidioc_g_selection	 = vpfe_g_selection,
>  	.vidioc_s_selection	 = vpfe_s_selection,
>  };
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index dc77682b4785..7a2851790b91 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -404,16 +404,16 @@ static int rvin_s_selection(struct file *file, void *fh,
>  	return 0;
>  }
>  
> -static int rvin_cropcap(struct file *file, void *priv,
> -			struct v4l2_cropcap *crop)
> +static int rvin_g_pixelaspect(struct file *file, void *priv,
> +			      int type, struct v4l2_fract *f)
>  {
>  	struct rvin_dev *vin = video_drvdata(file);
>  	struct v4l2_subdev *sd = vin_to_source(vin);
>  
> -	if (crop->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
> -	return v4l2_subdev_call(sd, video, g_pixelaspect, &crop->pixelaspect);
> +	return v4l2_subdev_call(sd, video, g_pixelaspect, f);
>  }
>  
>  static int rvin_enum_input(struct file *file, void *priv,
> @@ -620,7 +620,7 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>  	.vidioc_g_selection		= rvin_g_selection,
>  	.vidioc_s_selection		= rvin_s_selection,
>  
> -	.vidioc_cropcap			= rvin_cropcap,
> +	.vidioc_g_pixelaspect		= rvin_g_pixelaspect,
>  
>  	.vidioc_enum_input		= rvin_enum_input,
>  	.vidioc_g_input			= rvin_g_input,
> diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
> index 06961e7d8036..9f7bdb2c1385 100644
> --- a/drivers/media/platform/vivid/vivid-core.c
> +++ b/drivers/media/platform/vivid/vivid-core.c
> @@ -324,13 +324,14 @@ static int vidioc_s_dv_timings(struct file *file, void *fh, struct v4l2_dv_timin
>  	return vivid_vid_out_s_dv_timings(file, fh, timings);
>  }
>  
> -static int vidioc_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cc)
> +static int vidioc_g_pixelaspect(struct file *file, void *fh,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct video_device *vdev = video_devdata(file);
>  
>  	if (vdev->vfl_dir == VFL_DIR_RX)
> -		return vivid_vid_cap_cropcap(file, fh, cc);
> -	return vivid_vid_out_cropcap(file, fh, cc);
> +		return vivid_vid_cap_g_pixelaspect(file, fh, type, f);
> +	return vivid_vid_out_g_pixelaspect(file, fh, type, f);
>  }
>  
>  static int vidioc_g_selection(struct file *file, void *fh,
> @@ -519,7 +520,7 @@ static const struct v4l2_ioctl_ops vivid_ioctl_ops = {
>  
>  	.vidioc_g_selection		= vidioc_g_selection,
>  	.vidioc_s_selection		= vidioc_s_selection,
> -	.vidioc_cropcap			= vidioc_cropcap,
> +	.vidioc_g_pixelaspect		= vidioc_g_pixelaspect,
>  
>  	.vidioc_g_fmt_vbi_cap		= vidioc_g_fmt_vbi_cap,
>  	.vidioc_try_fmt_vbi_cap		= vidioc_g_fmt_vbi_cap,
> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
> index 58e14dd1dcd3..7f0f89d7a5f6 100644
> --- a/drivers/media/platform/vivid/vivid-vid-cap.c
> +++ b/drivers/media/platform/vivid/vivid-vid-cap.c
> @@ -1003,26 +1003,24 @@ int vivid_vid_cap_s_selection(struct file *file, void *fh, struct v4l2_selection
>  	return 0;
>  }
>  
> -int vivid_vid_cap_cropcap(struct file *file, void *priv,
> -			      struct v4l2_cropcap *cap)
> +int vivid_vid_cap_g_pixelaspect(struct file *file, void *priv,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct vivid_dev *dev = video_drvdata(file);
>  
> -	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
>  	switch (vivid_get_pixel_aspect(dev)) {
>  	case TPG_PIXEL_ASPECT_NTSC:
> -		cap->pixelaspect.numerator = 11;
> -		cap->pixelaspect.denominator = 10;
> +		f->numerator = 11;
> +		f->denominator = 10;
>  		break;
>  	case TPG_PIXEL_ASPECT_PAL:
> -		cap->pixelaspect.numerator = 54;
> -		cap->pixelaspect.denominator = 59;
> +		f->numerator = 54;
> +		f->denominator = 59;
>  		break;
> -	case TPG_PIXEL_ASPECT_SQUARE:
> -		cap->pixelaspect.numerator = 1;
> -		cap->pixelaspect.denominator = 1;
> +	default:
>  		break;
>  	}
>  	return 0;
> diff --git a/drivers/media/platform/vivid/vivid-vid-cap.h b/drivers/media/platform/vivid/vivid-vid-cap.h
> index 47d8b48820df..1e422a59eeab 100644
> --- a/drivers/media/platform/vivid/vivid-vid-cap.h
> +++ b/drivers/media/platform/vivid/vivid-vid-cap.h
> @@ -28,7 +28,7 @@ int vidioc_try_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f)
>  int vidioc_s_fmt_vid_cap(struct file *file, void *priv, struct v4l2_format *f);
>  int vivid_vid_cap_g_selection(struct file *file, void *priv, struct v4l2_selection *sel);
>  int vivid_vid_cap_s_selection(struct file *file, void *fh, struct v4l2_selection *s);
> -int vivid_vid_cap_cropcap(struct file *file, void *priv, struct v4l2_cropcap *cap);
> +int vivid_vid_cap_g_pixelaspect(struct file *file, void *priv, int type, struct v4l2_fract *f);
>  int vidioc_enum_fmt_vid_overlay(struct file *file, void  *priv, struct v4l2_fmtdesc *f);
>  int vidioc_g_fmt_vid_overlay(struct file *file, void *priv, struct v4l2_format *f);
>  int vidioc_try_fmt_vid_overlay(struct file *file, void *priv, struct v4l2_format *f);
> diff --git a/drivers/media/platform/vivid/vivid-vid-out.c b/drivers/media/platform/vivid/vivid-vid-out.c
> index 50248e2176a0..164a4a7918d4 100644
> --- a/drivers/media/platform/vivid/vivid-vid-out.c
> +++ b/drivers/media/platform/vivid/vivid-vid-out.c
> @@ -785,26 +785,24 @@ int vivid_vid_out_s_selection(struct file *file, void *fh, struct v4l2_selection
>  	return 0;
>  }
>  
> -int vivid_vid_out_cropcap(struct file *file, void *priv,
> -			      struct v4l2_cropcap *cap)
> +int vivid_vid_out_g_pixelaspect(struct file *file, void *priv,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct vivid_dev *dev = video_drvdata(file);
>  
> -	if (cap->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +	if (type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
>  		return -EINVAL;
>  
>  	switch (vivid_get_pixel_aspect(dev)) {
>  	case TPG_PIXEL_ASPECT_NTSC:
> -		cap->pixelaspect.numerator = 11;
> -		cap->pixelaspect.denominator = 10;
> +		f->numerator = 11;
> +		f->denominator = 10;
>  		break;
>  	case TPG_PIXEL_ASPECT_PAL:
> -		cap->pixelaspect.numerator = 54;
> -		cap->pixelaspect.denominator = 59;
> +		f->numerator = 54;
> +		f->denominator = 59;
>  		break;
> -	case TPG_PIXEL_ASPECT_SQUARE:
> -		cap->pixelaspect.numerator = 1;
> -		cap->pixelaspect.denominator = 1;
> +	default:
>  		break;
>  	}
>  	return 0;
> diff --git a/drivers/media/platform/vivid/vivid-vid-out.h b/drivers/media/platform/vivid/vivid-vid-out.h
> index e87aacf843c5..8d56314f4ea1 100644
> --- a/drivers/media/platform/vivid/vivid-vid-out.h
> +++ b/drivers/media/platform/vivid/vivid-vid-out.h
> @@ -23,7 +23,7 @@ int vidioc_try_fmt_vid_out(struct file *file, void *priv, struct v4l2_format *f)
>  int vidioc_s_fmt_vid_out(struct file *file, void *priv, struct v4l2_format *f);
>  int vivid_vid_out_g_selection(struct file *file, void *priv, struct v4l2_selection *sel);
>  int vivid_vid_out_s_selection(struct file *file, void *fh, struct v4l2_selection *s);
> -int vivid_vid_out_cropcap(struct file *file, void *fh, struct v4l2_cropcap *cap);
> +int vivid_vid_out_g_pixelaspect(struct file *file, void *priv, int type, struct v4l2_fract *f);
>  int vidioc_enum_fmt_vid_out_overlay(struct file *file, void  *priv, struct v4l2_fmtdesc *f);
>  int vidioc_g_fmt_vid_out_overlay(struct file *file, void *priv, struct v4l2_format *f);
>  int vidioc_try_fmt_vid_out_overlay(struct file *file, void *priv, struct v4l2_format *f);
> diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
> index d2250f594cf9..7876c897cc1d 100644
> --- a/drivers/media/usb/au0828/au0828-video.c
> +++ b/drivers/media/usb/au0828/au0828-video.c
> @@ -1616,19 +1616,19 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int vidioc_cropcap(struct file *file, void *priv,
> -			  struct v4l2_cropcap *cc)
> +static int vidioc_g_pixelaspect(struct file *file, void *priv,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct au0828_dev *dev = video_drvdata(file);
>  
> -	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
>  	dprintk(1, "%s called std_set %d dev_state %ld\n", __func__,
>  		dev->std_set_in_tuner_core, dev->dev_state);
>  
> -	cc->pixelaspect.numerator = 54;
> -	cc->pixelaspect.denominator = 59;
> +	f->numerator = 54;
> +	f->denominator = 59;
>  
>  	return 0;
>  }
> @@ -1777,7 +1777,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
>  	.vidioc_enumaudio           = vidioc_enumaudio,
>  	.vidioc_g_audio             = vidioc_g_audio,
>  	.vidioc_s_audio             = vidioc_s_audio,
> -	.vidioc_cropcap             = vidioc_cropcap,
> +	.vidioc_g_pixelaspect       = vidioc_g_pixelaspect,
>  	.vidioc_g_selection         = vidioc_g_selection,
>  
>  	.vidioc_reqbufs             = vb2_ioctl_reqbufs,
> diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c
> index f47d41de5ca7..e4ea4abcedab 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-417.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-417.c
> @@ -1500,18 +1500,18 @@ static const struct videobuf_queue_ops cx231xx_qops = {
>  
>  /* ------------------------------------------------------------------ */
>  
> -static int vidioc_cropcap(struct file *file, void *priv,
> -			  struct v4l2_cropcap *cc)
> +static int vidioc_g_pixelaspect(struct file *file, void *priv,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct cx231xx_fh *fh = priv;
>  	struct cx231xx *dev = fh->dev;
>  	bool is_50hz = dev->encodernorm.id & V4L2_STD_625_50;
>  
> -	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
> -	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
> -	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
> +	f->numerator = is_50hz ? 54 : 11;
> +	f->denominator = is_50hz ? 59 : 10;
>  
>  	return 0;
>  }
> @@ -1883,7 +1883,7 @@ static const struct v4l2_ioctl_ops mpeg_ioctl_ops = {
>  	.vidioc_g_input		 = cx231xx_g_input,
>  	.vidioc_s_input		 = cx231xx_s_input,
>  	.vidioc_s_ctrl		 = vidioc_s_ctrl,
> -	.vidioc_cropcap		 = vidioc_cropcap,
> +	.vidioc_g_pixelaspect	 = vidioc_g_pixelaspect,
>  	.vidioc_g_selection	 = vidioc_g_selection,
>  	.vidioc_querycap	 = cx231xx_querycap,
>  	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
> diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
> index a24a278bc586..503e3d8240aa 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-video.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-video.c
> @@ -1482,18 +1482,18 @@ int cx231xx_s_register(struct file *file, void *priv,
>  }
>  #endif
>  
> -static int vidioc_cropcap(struct file *file, void *priv,
> -			  struct v4l2_cropcap *cc)
> +static int vidioc_g_pixelaspect(struct file *file, void *priv,
> +				int type, struct v4l2_fract *f)
>  {
>  	struct cx231xx_fh *fh = priv;
>  	struct cx231xx *dev = fh->dev;
>  	bool is_50hz = dev->norm & V4L2_STD_625_50;
>  
> -	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
>  
> -	cc->pixelaspect.numerator = is_50hz ? 54 : 11;
> -	cc->pixelaspect.denominator = is_50hz ? 59 : 10;
> +	f->numerator = is_50hz ? 54 : 11;
> +	f->denominator = is_50hz ? 59 : 10;
>  
>  	return 0;
>  }
> @@ -2111,7 +2111,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
>  	.vidioc_g_fmt_vbi_cap          = vidioc_g_fmt_vbi_cap,
>  	.vidioc_try_fmt_vbi_cap        = vidioc_try_fmt_vbi_cap,
>  	.vidioc_s_fmt_vbi_cap          = vidioc_s_fmt_vbi_cap,
> -	.vidioc_cropcap                = vidioc_cropcap,
> +	.vidioc_g_pixelaspect          = vidioc_g_pixelaspect,
>  	.vidioc_g_selection            = vidioc_g_selection,
>  	.vidioc_reqbufs                = vidioc_reqbufs,
>  	.vidioc_querybuf               = vidioc_querybuf,
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> index cea232a3302d..9b43f4e9da82 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
> @@ -703,16 +703,19 @@ static int pvr2_try_ext_ctrls(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int pvr2_cropcap(struct file *file, void *priv, struct v4l2_cropcap *cap)
> +static int pvr2_g_pixelaspect(struct file *file, void *priv,
> +			      int type, struct v4l2_fract *f)
>  {
>  	struct pvr2_v4l2_fh *fh = file->private_data;
>  	struct pvr2_hdw *hdw = fh->channel.mc_head->hdw;
> +	struct v4l2_cropcap cap = { .type = type };
>  	int ret;
>  
> -	if (cap->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +	if (type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>  		return -EINVAL;
> -	ret = pvr2_hdw_get_cropcap(hdw, cap);
> -	cap->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; /* paranoia */
> +	ret = pvr2_hdw_get_cropcap(hdw, &cap);
> +	if (!ret)
> +		*f = cap.pixelaspect;
>  	return ret;
>  }
>  
> @@ -815,7 +818,7 @@ static const struct v4l2_ioctl_ops pvr2_ioctl_ops = {
>  	.vidioc_g_audio			    = pvr2_g_audio,
>  	.vidioc_enumaudio		    = pvr2_enumaudio,
>  	.vidioc_enum_input		    = pvr2_enum_input,
> -	.vidioc_cropcap			    = pvr2_cropcap,
> +	.vidioc_g_pixelaspect		    = pvr2_g_pixelaspect,
>  	.vidioc_s_selection		    = pvr2_s_selection,
>  	.vidioc_g_selection		    = pvr2_g_selection,
>  	.vidioc_g_input			    = pvr2_g_input,
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index d81141d51faa..626ac06b94e5 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -621,14 +621,14 @@ static void determine_valid_ioctls(struct video_device *vdev)
>  		SET_VALID_IOCTL(ops, VIDIOC_TRY_DECODER_CMD, vidioc_try_decoder_cmd);
>  		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMESIZES, vidioc_enum_framesizes);
>  		SET_VALID_IOCTL(ops, VIDIOC_ENUM_FRAMEINTERVALS, vidioc_enum_frameintervals);
> -		if (ops->vidioc_g_selection)
> +		if (ops->vidioc_g_selection) {
>  			set_bit(_IOC_NR(VIDIOC_G_CROP), valid_ioctls);
> +			set_bit(_IOC_NR(VIDIOC_CROPCAP), valid_ioctls);
> +		}
>  		if (ops->vidioc_s_selection)
>  			set_bit(_IOC_NR(VIDIOC_S_CROP), valid_ioctls);
>  		SET_VALID_IOCTL(ops, VIDIOC_G_SELECTION, vidioc_g_selection);
>  		SET_VALID_IOCTL(ops, VIDIOC_S_SELECTION, vidioc_s_selection);
> -		if (ops->vidioc_cropcap || ops->vidioc_g_selection)
> -			set_bit(_IOC_NR(VIDIOC_CROPCAP), valid_ioctls);
>  	} else if (is_vbi) {
>  		/* vbi specific ioctls */
>  		if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index a59954d351a2..26b199349fad 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2264,18 +2264,21 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops,
>  	p->pixelaspect.numerator = 1;
>  	p->pixelaspect.denominator = 1;
>  
> +	if (s.type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		s.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	else if (s.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +		s.type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> +
>  	/*
>  	 * The determine_valid_ioctls() call already should ensure
>  	 * that this can never happen, but just in case...
>  	 */
> -	if (WARN_ON(!ops->vidioc_cropcap && !ops->vidioc_g_selection))
> +	if (WARN_ON(!ops->vidioc_g_selection))
>  		return -ENOTTY;
>  
> -	if (ops->vidioc_cropcap)
> -		ret = ops->vidioc_cropcap(file, fh, p);
> -
> -	if (!ops->vidioc_g_selection)
> -		return ret;
> +	if (ops->vidioc_g_pixelaspect)
> +		ret = ops->vidioc_g_pixelaspect(file, fh, s.type,
> +						&p->pixelaspect);
>  
>  	/*
>  	 * Ignore ENOTTY or ENOIOCTLCMD error returns, just use the
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index 85fdd3f4b8ad..aa4511aa5ffc 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -220,8 +220,8 @@ struct v4l2_fh;
>   *	:ref:`VIDIOC_G_MODULATOR <vidioc_g_modulator>` ioctl
>   * @vidioc_s_modulator: pointer to the function that implements
>   *	:ref:`VIDIOC_S_MODULATOR <vidioc_g_modulator>` ioctl
> - * @vidioc_cropcap: pointer to the function that implements
> - *	:ref:`VIDIOC_CROPCAP <vidioc_cropcap>` ioctl
> + * @vidioc_g_pixelaspect: pointer to the function that implements
> + *	the pixelaspect part of the :ref:`VIDIOC_CROPCAP <vidioc_cropcap>` ioctl
>   * @vidioc_g_selection: pointer to the function that implements
>   *	:ref:`VIDIOC_G_SELECTION <vidioc_g_selection>` ioctl
>   * @vidioc_s_selection: pointer to the function that implements
> @@ -487,8 +487,8 @@ struct v4l2_ioctl_ops {
>  	int (*vidioc_s_modulator)(struct file *file, void *fh,
>  				  const struct v4l2_modulator *a);
>  	/* Crop ioctls */
> -	int (*vidioc_cropcap)(struct file *file, void *fh,
> -			      struct v4l2_cropcap *a);
> +	int (*vidioc_g_pixelaspect)(struct file *file, void *fh,
> +				    int buf_type, struct v4l2_fract *aspect);
>  	int (*vidioc_g_selection)(struct file *file, void *fh,
>  				  struct v4l2_selection *s);
>  	int (*vidioc_s_selection)(struct file *file, void *fh,
> -- 
> 2.18.0
> 

-- 
Regards,
Niklas Söderlund

Re: [RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[Niklas Söderlund]

Hi Hans,

I like this series, nice work!

On 2018-10-05 09:49:00 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This patch series converts the last remaining drivers that use g/s_crop and
> cropcap to g/s_selection.
> 
> The first two patches do some minor code cleanup.
> 
> The third patch adds a new video_device flag to indicate that the driver
> inverts the normal usage of g/s_crop/cropcap. This applies to the old
> Samsung drivers that predate the Selection API and that abused the existing
> crop API.
> 
> The next three patches do some code cleanup and prepare drivers for the
> removal of g/s_crop and ensure that cropcap only returns the pixelaspect.
> 
> The next three patches convert the remaining Samsung drivers and set the
> QUIRK flag for all three.
> 
> The final two patches remove vidioc_g/s_crop and rename vidioc_cropcap
> to vidioc_g_pixelaspect.
> 
> I would really appreciate it if someone from Samsung can test these
> three drivers or at the very least review the code.
> 
> Niklas, this series supersedes your 'v4l2-ioctl: fix CROPCAP type handling'
> patch. Sorry about that :-)

No worries, I'm happy my tests run without errors again :-) If 
appropriate fell free to add for the v4l2 and rcar-vin portions:

Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> Regards,
> 
> 	Hans
> 
> Hans Verkuil (11):
>   v4l2-ioctl: don't use CROP/COMPOSE_ACTIVE
>   v4l2-common.h: put backwards compat defines under #ifndef __KERNEL__
>   v4l2-ioctl: add QUIRK_INVERTED_CROP
>   davinci/vpbe: drop unused g_cropcap
>   cropcap/g_selection split
>   exynos-gsc: replace v4l2_crop by v4l2_selection
>   s5p_mfc_dec.c: convert g_crop to g_selection
>   exynos4-is: convert g/s_crop to g/s_selection
>   s5p-g2d: convert g/s_crop to g/s_selection
>   v4l2-ioctl: remove unused vidioc_g/s_crop
>   vidioc_cropcap -> vidioc_g_pixelaspect
> 
>  drivers/media/pci/bt8xx/bttv-driver.c         |  12 +-
>  drivers/media/pci/cobalt/cobalt-v4l2.c        |  48 +++++--
>  drivers/media/pci/cx18/cx18-ioctl.c           |  13 +-
>  drivers/media/pci/cx23885/cx23885-video.c     |  40 ++++--
>  drivers/media/pci/ivtv/ivtv-ioctl.c           |  17 +--
>  drivers/media/pci/saa7134/saa7134-video.c     |  21 ++-
>  drivers/media/platform/am437x/am437x-vpfe.c   |  31 ++---
>  drivers/media/platform/davinci/vpbe.c         |  23 ----
>  drivers/media/platform/davinci/vpbe_display.c |  10 +-
>  drivers/media/platform/davinci/vpfe_capture.c |  12 +-
>  drivers/media/platform/exynos-gsc/gsc-core.c  |  57 +++-----
>  drivers/media/platform/exynos-gsc/gsc-core.h  |   3 +-
>  drivers/media/platform/exynos-gsc/gsc-m2m.c   |  23 ++--
>  drivers/media/platform/exynos4-is/fimc-core.h |   6 +-
>  drivers/media/platform/exynos4-is/fimc-m2m.c  | 130 ++++++++++--------
>  drivers/media/platform/rcar-vin/rcar-v4l2.c   |  10 +-
>  drivers/media/platform/s5p-g2d/g2d.c          | 102 +++++++++-----
>  drivers/media/platform/s5p-mfc/s5p_mfc.c      |   1 +
>  drivers/media/platform/s5p-mfc/s5p_mfc_dec.c  |  49 ++++---
>  drivers/media/platform/vivid/vivid-core.c     |   9 +-
>  drivers/media/platform/vivid/vivid-vid-cap.c  |  18 ++-
>  drivers/media/platform/vivid/vivid-vid-cap.h  |   2 +-
>  drivers/media/platform/vivid/vivid-vid-out.c  |  18 ++-
>  drivers/media/platform/vivid/vivid-vid-out.h  |   2 +-
>  drivers/media/usb/au0828/au0828-video.c       |  38 +++--
>  drivers/media/usb/cpia2/cpia2_v4l.c           |  31 +++--
>  drivers/media/usb/cx231xx/cx231xx-417.c       |  41 ++++--
>  drivers/media/usb/cx231xx/cx231xx-video.c     |  41 ++++--
>  drivers/media/usb/pvrusb2/pvrusb2-v4l2.c      |  13 +-
>  drivers/media/v4l2-core/v4l2-dev.c            |   8 +-
>  drivers/media/v4l2-core/v4l2-ioctl.c          |  44 ++++--
>  include/media/davinci/vpbe.h                  |   4 -
>  include/media/v4l2-dev.h                      |  13 +-
>  include/media/v4l2-ioctl.h                    |  16 +--
>  include/uapi/linux/v4l2-common.h              |  28 ++--
>  35 files changed, 537 insertions(+), 397 deletions(-)
> 
> -- 
> 2.18.0
> 

-- 
Regards,
Niklas Söderlund

Re: [RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[Sakari Ailus]

Hi Hans,

On Fri, Oct 05, 2018 at 09:49:00AM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This patch series converts the last remaining drivers that use g/s_crop and
> cropcap to g/s_selection.
> 
> The first two patches do some minor code cleanup.
> 
> The third patch adds a new video_device flag to indicate that the driver
> inverts the normal usage of g/s_crop/cropcap. This applies to the old
> Samsung drivers that predate the Selection API and that abused the existing
> crop API.
> 
> The next three patches do some code cleanup and prepare drivers for the
> removal of g/s_crop and ensure that cropcap only returns the pixelaspect.
> 
> The next three patches convert the remaining Samsung drivers and set the
> QUIRK flag for all three.
> 
> The final two patches remove vidioc_g/s_crop and rename vidioc_cropcap
> to vidioc_g_pixelaspect.

Nice one; thanks!

For patches 1, 2, 3, 10 and 11:

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

I didn't read through the driver changes but I assume they would be fine.
:-)

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi

Re: [RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[Sylwester Nawrocki]

Hi Hans,

On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> This patch series converts the last remaining drivers that use g/s_crop and
> cropcap to g/s_selection.

Thank you for this clean up! I remember attempting conversion of those remaining
drivers to selection API long time ago but I didn't have a good idea
then how to address
that crop and compose target inversion mess so I abandoned that efforts then.

--
Regards,
Sylwester

Re: [RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[Hans Verkuil]

Hi Sylwester,

On 11/02/2018 05:16 PM, Sylwester Nawrocki wrote:
> Hi Hans,
> 
> On Fri, 5 Oct 2018 at 09:49, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> This patch series converts the last remaining drivers that use g/s_crop and
>> cropcap to g/s_selection.
> 
> Thank you for this clean up! I remember attempting conversion of those remaining
> drivers to selection API long time ago but I didn't have a good idea
> then how to address
> that crop and compose target inversion mess so I abandoned that efforts then.

Thank you for the review. One question: have you also tested this with at least
one of the affected drivers?

I'd like to have at least one Tested-by line.

Regards,

	Hans

Re: [RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[Sylwester Nawrocki]

Hi Hans,

On 11/05/2018 02:12 PM, Hans Verkuil wrote:
> Thank you for the review. One question: have you also tested this with at least
> one of the affected drivers?
> 
> I'd like to have at least one Tested-by line.

I just tested it now - video playback on Exynos4210 Trats2 so it covers 
the s5p-mfc and exynos4-is (fimc-m2m) drivers. Well done, I couldn't see 
any breakage.

You can add "Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>" 
to patches: 1, 2, 3, 7, 8, 10.

-- 
Regards,
Sylwester

Re: [RFC PATCH 00/11] Convert last remaining g/s_crop/cropcap drivers

[Hans Verkuil]

On 11/05/2018 05:08 PM, Sylwester Nawrocki wrote:
> Hi Hans,
> 
> On 11/05/2018 02:12 PM, Hans Verkuil wrote:
>> Thank you for the review. One question: have you also tested this with at least
>> one of the affected drivers?
>>
>> I'd like to have at least one Tested-by line.
> 
> I just tested it now - video playback on Exynos4210 Trats2 so it covers 
> the s5p-mfc and exynos4-is (fimc-m2m) drivers. Well done, I couldn't see 
> any breakage.
> 
> You can add "Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>" 
> to patches: 1, 2, 3, 7, 8, 10.
> 

Fantastic, I'll see if I can make a pull request for this series this week.

Regards,

	Hans