2019-04-28 19:13:20

by Steve Longerbeam

[permalink] [raw]
Subject: [PATCH v2 4/8] Revert "media: imx: Set capture compose rectangle in capture_device_set_format"

Rvert this commit, as imx_media_capture_device_set_format() will be
removed.

This reverts commit 5964cbd8692252615370b77eb96764dd70c2f837.

Signed-off-by: Steve Longerbeam <[email protected]>
---
drivers/staging/media/imx/imx-ic-prpencvf.c | 5 ++--
drivers/staging/media/imx/imx-media-capture.c | 24 +++++++++----------
drivers/staging/media/imx/imx-media-csi.c | 5 ++--
drivers/staging/media/imx/imx-media-utils.c | 20 ++++------------
drivers/staging/media/imx/imx-media.h | 6 ++---
5 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index 8242d88dfb82..afaa3a8b15e9 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -910,7 +910,6 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
const struct imx_media_pixfmt *cc;
struct v4l2_pix_format vdev_fmt;
struct v4l2_mbus_framefmt *fmt;
- struct v4l2_rect vdev_compose;
int ret = 0;

if (sdformat->pad >= PRPENCVF_NUM_PADS)
@@ -952,11 +951,11 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
priv->cc[sdformat->pad] = cc;

/* propagate output pad format to capture device */
- imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
+ imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
&priv->format_mbus[PRPENCVF_SRC_PAD],
priv->cc[PRPENCVF_SRC_PAD]);
mutex_unlock(&priv->lock);
- imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
+ imx_media_capture_device_set_format(vdev, &vdev_fmt);

return 0;
out:
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 335084a6b0cd..555f6204660b 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -205,8 +205,7 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,

static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
struct v4l2_subdev_format *fmt_src,
- struct v4l2_format *f,
- struct v4l2_rect *compose)
+ struct v4l2_format *f)
{
const struct imx_media_pixfmt *cc, *cc_src;

@@ -246,8 +245,7 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
}
}

- imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, compose,
- &fmt_src->format, cc);
+ imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);

return 0;
}
@@ -265,7 +263,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
if (ret)
return ret;

- return __capture_try_fmt_vid_cap(priv, &fmt_src, f, NULL);
+ return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
}

static int capture_s_fmt_vid_cap(struct file *file, void *fh,
@@ -273,7 +271,6 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
{
struct capture_priv *priv = video_drvdata(file);
struct v4l2_subdev_format fmt_src;
- struct v4l2_rect compose;
int ret;

if (vb2_is_busy(&priv->q)) {
@@ -287,14 +284,17 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
if (ret)
return ret;

- ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f, &compose);
+ ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
if (ret)
return ret;

priv->vdev.fmt.fmt.pix = f->fmt.pix;
priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
CS_SEL_ANY, true);
- priv->vdev.compose = compose;
+ priv->vdev.compose.left = 0;
+ priv->vdev.compose.top = 0;
+ priv->vdev.compose.width = fmt_src.format.width;
+ priv->vdev.compose.height = fmt_src.format.height;

return 0;
}
@@ -655,8 +655,7 @@ static struct video_device capture_videodev = {
};

void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
- const struct v4l2_pix_format *pix,
- const struct v4l2_rect *compose)
+ struct v4l2_pix_format *pix)
{
struct capture_priv *priv = to_capture_priv(vdev);

@@ -664,7 +663,6 @@ void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
priv->vdev.fmt.fmt.pix = *pix;
priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY,
true);
- priv->vdev.compose = *compose;
mutex_unlock(&priv->mutex);
}
EXPORT_SYMBOL_GPL(imx_media_capture_device_set_format);
@@ -770,8 +768,10 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
}

vdev->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix, &vdev->compose,
+ imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
&fmt_src.format, NULL);
+ vdev->compose.width = fmt_src.format.width;
+ vdev->compose.height = fmt_src.format.height;
vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
CS_SEL_ANY, false);

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index c70fa6b509ae..09b89a72eaa2 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1512,7 +1512,6 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
struct v4l2_pix_format vdev_fmt;
struct v4l2_mbus_framefmt *fmt;
struct v4l2_rect *crop, *compose;
- struct v4l2_rect vdev_compose;
int ret;

if (sdformat->pad >= CSI_NUM_PADS)
@@ -1568,11 +1567,11 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
priv->cc[sdformat->pad] = cc;

/* propagate IDMAC output pad format to capture device */
- imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
+ imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
&priv->format_mbus[CSI_SRC_PAD_IDMAC],
priv->cc[CSI_SRC_PAD_IDMAC]);
mutex_unlock(&priv->lock);
- imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
+ imx_media_capture_device_set_format(vdev, &vdev_fmt);

return 0;
out:
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 8a6e57652402..a80ef0b087ad 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -577,8 +577,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
EXPORT_SYMBOL_GPL(imx_media_fill_default_mbus_fields);

int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
- struct v4l2_rect *compose,
- const struct v4l2_mbus_framefmt *mbus,
+ struct v4l2_mbus_framefmt *mbus,
const struct imx_media_pixfmt *cc)
{
u32 width;
@@ -625,17 +624,6 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
stride * pix->height;

- /*
- * set capture compose rectangle, which is fixed to the
- * source subdevice mbus format.
- */
- if (compose) {
- compose->left = 0;
- compose->top = 0;
- compose->width = mbus->width;
- compose->height = mbus->height;
- }
-
return 0;
}
EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_pix_fmt);
@@ -647,11 +635,13 @@ int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,

memset(image, 0, sizeof(*image));

- ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, &image->rect,
- mbus, NULL);
+ ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, mbus, NULL);
if (ret)
return ret;

+ image->rect.width = mbus->width;
+ image->rect.height = mbus->height;
+
return 0;
}
EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_ipu_image);
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index 71e20f53ed7b..ddea972b7bc4 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -174,8 +174,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
struct v4l2_mbus_framefmt *fmt,
bool ic_route);
int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
- struct v4l2_rect *compose,
- const struct v4l2_mbus_framefmt *mbus,
+ struct v4l2_mbus_framefmt *mbus,
const struct imx_media_pixfmt *cc);
int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
struct v4l2_mbus_framefmt *mbus);
@@ -280,8 +279,7 @@ void imx_media_capture_device_unregister(struct imx_media_video_dev *vdev);
struct imx_media_buffer *
imx_media_capture_device_next_buf(struct imx_media_video_dev *vdev);
void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
- const struct v4l2_pix_format *pix,
- const struct v4l2_rect *compose);
+ struct v4l2_pix_format *pix);
void imx_media_capture_device_error(struct imx_media_video_dev *vdev);

/* subdev group ids */
--
2.17.1


2019-04-30 22:02:20

by Rui Miguel Silva

[permalink] [raw]
Subject: Re: [PATCH v2 4/8] Revert "media: imx: Set capture compose rectangle in capture_device_set_format"

Hi Steve,
On Sun 28 Apr 2019 at 20:09, Steve Longerbeam wrote:
> Rvert this commit, as imx_media_capture_device_set_format() will be
> removed.
>
> This reverts commit 5964cbd8692252615370b77eb96764dd70c2f837.
>
> Signed-off-by: Steve Longerbeam <[email protected]>
> ---
> drivers/staging/media/imx/imx-ic-prpencvf.c | 5 ++--
> drivers/staging/media/imx/imx-media-capture.c | 24 +++++++++----------
> drivers/staging/media/imx/imx-media-csi.c | 5 ++--
> drivers/staging/media/imx/imx-media-utils.c | 20 ++++------------
> drivers/staging/media/imx/imx-media.h | 6 ++---
> 5 files changed, 23 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 8242d88dfb82..afaa3a8b15e9 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -910,7 +910,6 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
> const struct imx_media_pixfmt *cc;
> struct v4l2_pix_format vdev_fmt;
> struct v4l2_mbus_framefmt *fmt;
> - struct v4l2_rect vdev_compose;
> int ret = 0;
>
> if (sdformat->pad >= PRPENCVF_NUM_PADS)
> @@ -952,11 +951,11 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
> priv->cc[sdformat->pad] = cc;
>
> /* propagate output pad format to capture device */
> - imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
> + imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
> &priv->format_mbus[PRPENCVF_SRC_PAD],
> priv->cc[PRPENCVF_SRC_PAD]);
> mutex_unlock(&priv->lock);
> - imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
> + imx_media_capture_device_set_format(vdev, &vdev_fmt);
>
> return 0;
> out:
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index 335084a6b0cd..555f6204660b 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -205,8 +205,7 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,
>
> static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
> struct v4l2_subdev_format *fmt_src,
> - struct v4l2_format *f,
> - struct v4l2_rect *compose)
> + struct v4l2_format *f)
> {
> const struct imx_media_pixfmt *cc, *cc_src;
>
> @@ -246,8 +245,7 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
> }
> }
>
> - imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, compose,
> - &fmt_src->format, cc);
> + imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);
>
> return 0;
> }
> @@ -265,7 +263,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
> if (ret)
> return ret;
>
> - return __capture_try_fmt_vid_cap(priv, &fmt_src, f, NULL);
> + return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
> }
>
> static int capture_s_fmt_vid_cap(struct file *file, void *fh,
> @@ -273,7 +271,6 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
> {
> struct capture_priv *priv = video_drvdata(file);
> struct v4l2_subdev_format fmt_src;
> - struct v4l2_rect compose;
> int ret;
>
> if (vb2_is_busy(&priv->q)) {
> @@ -287,14 +284,17 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
> if (ret)
> return ret;
>
> - ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f, &compose);
> + ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
> if (ret)
> return ret;
>
> priv->vdev.fmt.fmt.pix = f->fmt.pix;
> priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
> CS_SEL_ANY, true);
> - priv->vdev.compose = compose;
> + priv->vdev.compose.left = 0;
> + priv->vdev.compose.top = 0;
> + priv->vdev.compose.width = fmt_src.format.width;
> + priv->vdev.compose.height = fmt_src.format.height;
>
> return 0;
> }
> @@ -655,8 +655,7 @@ static struct video_device capture_videodev = {
> };
>
> void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
> - const struct v4l2_pix_format *pix,
> - const struct v4l2_rect *compose)
> + struct v4l2_pix_format *pix)
> {
> struct capture_priv *priv = to_capture_priv(vdev);
>
> @@ -664,7 +663,6 @@ void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
> priv->vdev.fmt.fmt.pix = *pix;
> priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY,
> true);
> - priv->vdev.compose = *compose;
> mutex_unlock(&priv->mutex);
> }
> EXPORT_SYMBOL_GPL(imx_media_capture_device_set_format);
> @@ -770,8 +768,10 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
> }
>
> vdev->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> - imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix, &vdev->compose,
> + imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
> &fmt_src.format, NULL);
> + vdev->compose.width = fmt_src.format.width;
> + vdev->compose.height = fmt_src.format.height;
> vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
> CS_SEL_ANY, false);
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index c70fa6b509ae..09b89a72eaa2 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -1512,7 +1512,6 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
> struct v4l2_pix_format vdev_fmt;
> struct v4l2_mbus_framefmt *fmt;
> struct v4l2_rect *crop, *compose;
> - struct v4l2_rect vdev_compose;
> int ret;
>
> if (sdformat->pad >= CSI_NUM_PADS)
> @@ -1568,11 +1567,11 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
> priv->cc[sdformat->pad] = cc;
>
> /* propagate IDMAC output pad format to capture device */
> - imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
> + imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
> &priv->format_mbus[CSI_SRC_PAD_IDMAC],
> priv->cc[CSI_SRC_PAD_IDMAC]);
> mutex_unlock(&priv->lock);
> - imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
> + imx_media_capture_device_set_format(vdev, &vdev_fmt);
>
> return 0;
> out:
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 8a6e57652402..a80ef0b087ad 100644
> --- a/drivers/staging/media/imx/imx-media-utils.c
> +++ b/drivers/staging/media/imx/imx-media-utils.c
> @@ -577,8 +577,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
> EXPORT_SYMBOL_GPL(imx_media_fill_default_mbus_fields);
>
> int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> - struct v4l2_rect *compose,
> - const struct v4l2_mbus_framefmt *mbus,
> + struct v4l2_mbus_framefmt *mbus,
> const struct imx_media_pixfmt *cc)
> {
> u32 width;
> @@ -625,17 +624,6 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
> stride * pix->height;
>
> - /*
> - * set capture compose rectangle, which is fixed to the
> - * source subdevice mbus format.
> - */
> - if (compose) {
> - compose->left = 0;
> - compose->top = 0;
> - compose->width = mbus->width;
> - compose->height = mbus->height;
> - }
> -
> return 0;
> }
> EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_pix_fmt);
> @@ -647,11 +635,13 @@ int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
>
> memset(image, 0, sizeof(*image));
>
> - ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, &image->rect,
> - mbus, NULL);
> + ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, mbus, NULL);
> if (ret)
> return ret;
>
> + image->rect.width = mbus->width;
> + image->rect.height = mbus->height;
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_ipu_image);
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index 71e20f53ed7b..ddea972b7bc4 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -174,8 +174,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
> struct v4l2_mbus_framefmt *fmt,
> bool ic_route);
> int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> - struct v4l2_rect *compose,
> - const struct v4l2_mbus_framefmt *mbus,
> + struct v4l2_mbus_framefmt *mbus,
> const struct imx_media_pixfmt *cc);

Also here you change this function arguments and only remove it
from imx7-media-csi.c in the next patch, triggering:


vers/staging/media/imx/imx7-media-csi.c: In function 'imx7_csi_set_fmt':
drivers/staging/media/imx/imx7-media-csi.c:1089:43: error: passing argument 2 of 'imx_media_mbus_fmt_to_pix_fmt' from incompatible pointer type [-Werror=incompatible-pointer-types]
imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
^~~~~~~~~~~~~
In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
drivers/staging/media/imx/imx-media.h:177:34: note: expected 'struct v4l2_mbus_framefmt *' but argument is of type 'struct v4l2_rect *'
struct v4l2_mbus_framefmt *mbus,
~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
drivers/staging/media/imx/imx7-media-csi.c:1090:11: error: passing argument 3 of 'imx_media_mbus_fmt_to_pix_fmt' from incompatible pointer type [-Werror=incompatible-pointer-types]
&csi->format_mbus[IMX7_CSI_PAD_SRC],
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
drivers/staging/media/imx/imx-media.h:178:38: note: expected 'const struct imx_media_pixfmt *' but argument is of type 'struct v4l2_mbus_framefmt *'
const struct imx_media_pixfmt *cc);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
drivers/staging/media/imx/imx7-media-csi.c:1089:2: error: too many arguments to function 'imx_media_mbus_fmt_to_pix_fmt'
imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
drivers/staging/media/imx/imx-media.h:176:5: note: declared here
int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/imx/imx7-media-csi.c:1093:2: error: too many arguments to function 'imx_media_capture_device_set_format'
imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
drivers/staging/media/imx/imx-media.h:281:6: note: declared here
void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,

---
Cheers,
Rui

2019-04-30 22:08:47

by Steve Longerbeam

[permalink] [raw]
Subject: Re: [PATCH v2 4/8] Revert "media: imx: Set capture compose rectangle in capture_device_set_format"

Yes, because the reverted commit was merged before the imx7 support, I
will fix this for v3.

Steve


On 4/30/19 3:00 PM, Rui Miguel Silva wrote:
> Hi Steve,
> On Sun 28 Apr 2019 at 20:09, Steve Longerbeam wrote:
>> Rvert this commit, as imx_media_capture_device_set_format() will be
>> removed.
>>
>> This reverts commit 5964cbd8692252615370b77eb96764dd70c2f837.
>>
>> Signed-off-by: Steve Longerbeam <[email protected]>
>> ---
>> drivers/staging/media/imx/imx-ic-prpencvf.c | 5 ++--
>> drivers/staging/media/imx/imx-media-capture.c | 24 +++++++++----------
>> drivers/staging/media/imx/imx-media-csi.c | 5 ++--
>> drivers/staging/media/imx/imx-media-utils.c | 20 ++++------------
>> drivers/staging/media/imx/imx-media.h | 6 ++---
>> 5 files changed, 23 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> index 8242d88dfb82..afaa3a8b15e9 100644
>> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
>> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> @@ -910,7 +910,6 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>> const struct imx_media_pixfmt *cc;
>> struct v4l2_pix_format vdev_fmt;
>> struct v4l2_mbus_framefmt *fmt;
>> - struct v4l2_rect vdev_compose;
>> int ret = 0;
>>
>> if (sdformat->pad >= PRPENCVF_NUM_PADS)
>> @@ -952,11 +951,11 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>> priv->cc[sdformat->pad] = cc;
>>
>> /* propagate output pad format to capture device */
>> - imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
>> + imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
>> &priv->format_mbus[PRPENCVF_SRC_PAD],
>> priv->cc[PRPENCVF_SRC_PAD]);
>> mutex_unlock(&priv->lock);
>> - imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
>> + imx_media_capture_device_set_format(vdev, &vdev_fmt);
>>
>> return 0;
>> out:
>> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
>> index 335084a6b0cd..555f6204660b 100644
>> --- a/drivers/staging/media/imx/imx-media-capture.c
>> +++ b/drivers/staging/media/imx/imx-media-capture.c
>> @@ -205,8 +205,7 @@ static int capture_g_fmt_vid_cap(struct file *file, void *fh,
>>
>> static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
>> struct v4l2_subdev_format *fmt_src,
>> - struct v4l2_format *f,
>> - struct v4l2_rect *compose)
>> + struct v4l2_format *f)
>> {
>> const struct imx_media_pixfmt *cc, *cc_src;
>>
>> @@ -246,8 +245,7 @@ static int __capture_try_fmt_vid_cap(struct capture_priv *priv,
>> }
>> }
>>
>> - imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, compose,
>> - &fmt_src->format, cc);
>> + imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src->format, cc);
>>
>> return 0;
>> }
>> @@ -265,7 +263,7 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
>> if (ret)
>> return ret;
>>
>> - return __capture_try_fmt_vid_cap(priv, &fmt_src, f, NULL);
>> + return __capture_try_fmt_vid_cap(priv, &fmt_src, f);
>> }
>>
>> static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>> @@ -273,7 +271,6 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>> {
>> struct capture_priv *priv = video_drvdata(file);
>> struct v4l2_subdev_format fmt_src;
>> - struct v4l2_rect compose;
>> int ret;
>>
>> if (vb2_is_busy(&priv->q)) {
>> @@ -287,14 +284,17 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
>> if (ret)
>> return ret;
>>
>> - ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f, &compose);
>> + ret = __capture_try_fmt_vid_cap(priv, &fmt_src, f);
>> if (ret)
>> return ret;
>>
>> priv->vdev.fmt.fmt.pix = f->fmt.pix;
>> priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
>> CS_SEL_ANY, true);
>> - priv->vdev.compose = compose;
>> + priv->vdev.compose.left = 0;
>> + priv->vdev.compose.top = 0;
>> + priv->vdev.compose.width = fmt_src.format.width;
>> + priv->vdev.compose.height = fmt_src.format.height;
>>
>> return 0;
>> }
>> @@ -655,8 +655,7 @@ static struct video_device capture_videodev = {
>> };
>>
>> void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
>> - const struct v4l2_pix_format *pix,
>> - const struct v4l2_rect *compose)
>> + struct v4l2_pix_format *pix)
>> {
>> struct capture_priv *priv = to_capture_priv(vdev);
>>
>> @@ -664,7 +663,6 @@ void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
>> priv->vdev.fmt.fmt.pix = *pix;
>> priv->vdev.cc = imx_media_find_format(pix->pixelformat, CS_SEL_ANY,
>> true);
>> - priv->vdev.compose = *compose;
>> mutex_unlock(&priv->mutex);
>> }
>> EXPORT_SYMBOL_GPL(imx_media_capture_device_set_format);
>> @@ -770,8 +768,10 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
>> }
>>
>> vdev->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> - imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix, &vdev->compose,
>> + imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
>> &fmt_src.format, NULL);
>> + vdev->compose.width = fmt_src.format.width;
>> + vdev->compose.height = fmt_src.format.height;
>> vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
>> CS_SEL_ANY, false);
>>
>> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
>> index c70fa6b509ae..09b89a72eaa2 100644
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -1512,7 +1512,6 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
>> struct v4l2_pix_format vdev_fmt;
>> struct v4l2_mbus_framefmt *fmt;
>> struct v4l2_rect *crop, *compose;
>> - struct v4l2_rect vdev_compose;
>> int ret;
>>
>> if (sdformat->pad >= CSI_NUM_PADS)
>> @@ -1568,11 +1567,11 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
>> priv->cc[sdformat->pad] = cc;
>>
>> /* propagate IDMAC output pad format to capture device */
>> - imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
>> + imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt,
>> &priv->format_mbus[CSI_SRC_PAD_IDMAC],
>> priv->cc[CSI_SRC_PAD_IDMAC]);
>> mutex_unlock(&priv->lock);
>> - imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
>> + imx_media_capture_device_set_format(vdev, &vdev_fmt);
>>
>> return 0;
>> out:
>> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
>> index 8a6e57652402..a80ef0b087ad 100644
>> --- a/drivers/staging/media/imx/imx-media-utils.c
>> +++ b/drivers/staging/media/imx/imx-media-utils.c
>> @@ -577,8 +577,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
>> EXPORT_SYMBOL_GPL(imx_media_fill_default_mbus_fields);
>>
>> int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>> - struct v4l2_rect *compose,
>> - const struct v4l2_mbus_framefmt *mbus,
>> + struct v4l2_mbus_framefmt *mbus,
>> const struct imx_media_pixfmt *cc)
>> {
>> u32 width;
>> @@ -625,17 +624,6 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>> pix->sizeimage = cc->planar ? ((stride * pix->height * cc->bpp) >> 3) :
>> stride * pix->height;
>>
>> - /*
>> - * set capture compose rectangle, which is fixed to the
>> - * source subdevice mbus format.
>> - */
>> - if (compose) {
>> - compose->left = 0;
>> - compose->top = 0;
>> - compose->width = mbus->width;
>> - compose->height = mbus->height;
>> - }
>> -
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_pix_fmt);
>> @@ -647,11 +635,13 @@ int imx_media_mbus_fmt_to_ipu_image(struct ipu_image *image,
>>
>> memset(image, 0, sizeof(*image));
>>
>> - ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, &image->rect,
>> - mbus, NULL);
>> + ret = imx_media_mbus_fmt_to_pix_fmt(&image->pix, mbus, NULL);
>> if (ret)
>> return ret;
>>
>> + image->rect.width = mbus->width;
>> + image->rect.height = mbus->height;
>> +
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(imx_media_mbus_fmt_to_ipu_image);
>> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
>> index 71e20f53ed7b..ddea972b7bc4 100644
>> --- a/drivers/staging/media/imx/imx-media.h
>> +++ b/drivers/staging/media/imx/imx-media.h
>> @@ -174,8 +174,7 @@ void imx_media_fill_default_mbus_fields(struct v4l2_mbus_framefmt *tryfmt,
>> struct v4l2_mbus_framefmt *fmt,
>> bool ic_route);
>> int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
>> - struct v4l2_rect *compose,
>> - const struct v4l2_mbus_framefmt *mbus,
>> + struct v4l2_mbus_framefmt *mbus,
>> const struct imx_media_pixfmt *cc);
> Also here you change this function arguments and only remove it
> from imx7-media-csi.c in the next patch, triggering:
>
>
> vers/staging/media/imx/imx7-media-csi.c: In function 'imx7_csi_set_fmt':
> drivers/staging/media/imx/imx7-media-csi.c:1089:43: error: passing argument 2 of 'imx_media_mbus_fmt_to_pix_fmt' from incompatible pointer type [-Werror=incompatible-pointer-types]
> imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
> ^~~~~~~~~~~~~
> In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
> drivers/staging/media/imx/imx-media.h:177:34: note: expected 'struct v4l2_mbus_framefmt *' but argument is of type 'struct v4l2_rect *'
> struct v4l2_mbus_framefmt *mbus,
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
> drivers/staging/media/imx/imx7-media-csi.c:1090:11: error: passing argument 3 of 'imx_media_mbus_fmt_to_pix_fmt' from incompatible pointer type [-Werror=incompatible-pointer-types]
> &csi->format_mbus[IMX7_CSI_PAD_SRC],
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
> drivers/staging/media/imx/imx-media.h:178:38: note: expected 'const struct imx_media_pixfmt *' but argument is of type 'struct v4l2_mbus_framefmt *'
> const struct imx_media_pixfmt *cc);
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
> drivers/staging/media/imx/imx7-media-csi.c:1089:2: error: too many arguments to function 'imx_media_mbus_fmt_to_pix_fmt'
> imx_media_mbus_fmt_to_pix_fmt(&vdev_fmt, &vdev_compose,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
> drivers/staging/media/imx/imx-media.h:176:5: note: declared here
> int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/staging/media/imx/imx7-media-csi.c:1093:2: error: too many arguments to function 'imx_media_capture_device_set_format'
> imx_media_capture_device_set_format(vdev, &vdev_fmt, &vdev_compose);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/staging/media/imx/imx7-media-csi.c:30:
> drivers/staging/media/imx/imx-media.h:281:6: note: declared here
> void imx_media_capture_device_set_format(struct imx_media_video_dev *vdev,
>
> ---
> Cheers,
> Rui
>