From: Hans Verkuil <hverkuil@xs4all.nl>
To: Philipp Zabel <p.zabel@pengutronix.de>, linux-media@vger.kernel.org
Cc: Hans Verkuil <hans.verkuil@cisco.com>,
Steve Longerbeam <slongerbeam@gmail.com>,
Sakari Ailus <sakari.ailus@iki.fi>,
kernel@pengutronix.de
Subject: Re: [PATCH v3 1/3] media: imx: add capture compose rectangle
Date: Wed, 16 Jan 2019 16:29:07 +0100 [thread overview]
Message-ID: <f23fae30-d1ed-7817-e531-d47a47ea94a5@xs4all.nl> (raw)
In-Reply-To: <20190111111053.12551-1-p.zabel@pengutronix.de>
On 1/11/19 12:10 PM, Philipp Zabel wrote:
> Allowing to compose captured images into larger memory buffers
> will let us lift alignment restrictions on CSI crop width.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Steve Longerbeam <slongerbeam@gmail.com>
> ---
> drivers/staging/media/imx/imx-ic-prpencvf.c | 3 +-
> drivers/staging/media/imx/imx-media-capture.c | 37 +++++++++++++++++++
> drivers/staging/media/imx/imx-media-csi.c | 3 +-
> drivers/staging/media/imx/imx-media-vdic.c | 4 +-
> drivers/staging/media/imx/imx-media.h | 2 +
> 5 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
> index 28f41caba05d..fe5a77baa592 100644
> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
> @@ -366,8 +366,7 @@ static int prp_setup_channel(struct prp_priv *priv,
>
> memset(&image, 0, sizeof(image));
> image.pix = vdev->fmt.fmt.pix;
> - image.rect.width = image.pix.width;
> - image.rect.height = image.pix.height;
> + image.rect = vdev->compose;
>
> if (rot_swap_width_height) {
> swap(image.pix.width, image.pix.height);
> diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
> index b37e1186eb2f..fb985e68f9ab 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -262,6 +262,10 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
> 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.left = 0;
> + priv->vdev.compose.top = 0;
> + priv->vdev.compose.width = f->fmt.pix.width;
> + priv->vdev.compose.height = f->fmt.pix.height;
>
> return 0;
> }
> @@ -290,6 +294,34 @@ static int capture_s_std(struct file *file, void *fh, v4l2_std_id std)
> return v4l2_subdev_call(priv->src_sd, video, s_std, std);
> }
>
> +static int capture_g_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + struct capture_priv *priv = video_drvdata(file);
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_CROP:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + case V4L2_SEL_TGT_CROP_BOUNDS:
The crop rectangle is equal to the compose rectangle? That can't be right.
Does the hardware support cropping at all? Probably not, and in that case
you shouldn't support the crop target at all.
> + case V4L2_SEL_TGT_COMPOSE:
> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + case V4L2_SEL_TGT_COMPOSE_PADDED:
> + s->r = priv->vdev.compose;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int capture_s_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + return capture_g_selection(file, fh, s);
> +}
Don't implement s_selection unless you can actually change the selection
rectangle(s).
> +
> static int capture_g_parm(struct file *file, void *fh,
> struct v4l2_streamparm *a)
> {
> @@ -350,6 +382,9 @@ static const struct v4l2_ioctl_ops capture_ioctl_ops = {
> .vidioc_g_std = capture_g_std,
> .vidioc_s_std = capture_s_std,
>
> + .vidioc_g_selection = capture_g_selection,
> + .vidioc_s_selection = capture_s_selection,
> +
> .vidioc_g_parm = capture_g_parm,
> .vidioc_s_parm = capture_s_parm,
>
> @@ -687,6 +722,8 @@ 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,
> &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 4223f8d418ae..c4523afe7b48 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -413,8 +413,7 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>
> memset(&image, 0, sizeof(image));
> image.pix = vdev->fmt.fmt.pix;
> - image.rect.width = image.pix.width;
> - image.rect.height = image.pix.height;
> + image.rect = vdev->compose;
>
> csi_idmac_setup_vb2_buf(priv, phys);
>
> diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
> index 482250d47e7c..e08d296cf4eb 100644
> --- a/drivers/staging/media/imx/imx-media-vdic.c
> +++ b/drivers/staging/media/imx/imx-media-vdic.c
> @@ -263,10 +263,10 @@ static int setup_vdi_channel(struct vdic_priv *priv,
>
> memset(&image, 0, sizeof(image));
> image.pix = vdev->fmt.fmt.pix;
> + image.rect = vdev->compose;
> /* one field to VDIC channels */
> image.pix.height /= 2;
> - image.rect.width = image.pix.width;
> - image.rect.height = image.pix.height;
> + image.rect.height /= 2;
> image.phys0 = phys0;
> image.phys1 = phys1;
>
> diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
> index bc7feb81937c..7a0e658753f0 100644
> --- a/drivers/staging/media/imx/imx-media.h
> +++ b/drivers/staging/media/imx/imx-media.h
> @@ -80,6 +80,8 @@ struct imx_media_video_dev {
>
> /* the user format */
> struct v4l2_format fmt;
> + /* the compose rectangle */
> + struct v4l2_rect compose;
> const struct imx_media_pixfmt *cc;
>
> /* links this vdev to master list */
>
Regards,
Hans
next prev parent reply other threads:[~2019-01-16 15:29 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-11 11:10 [PATCH v3 1/3] media: imx: add capture compose rectangle Philipp Zabel
2019-01-11 11:10 ` [PATCH v3 2/3] media: imx: set compose rectangle to mbus format Philipp Zabel
2019-01-16 15:28 ` Hans Verkuil
2019-01-17 13:01 ` Philipp Zabel
2019-01-17 13:54 ` Hans Verkuil
2019-01-11 11:10 ` [PATCH v3 3/3] media: imx: lift CSI and PRP ENC/VF width alignment restriction Philipp Zabel
2019-01-11 18:13 ` Steve Longerbeam
2019-01-16 15:29 ` Hans Verkuil [this message]
2019-01-17 12:46 ` [PATCH v3 1/3] media: imx: add capture compose rectangle Philipp Zabel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f23fae30-d1ed-7817-e531-d47a47ea94a5@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=hans.verkuil@cisco.com \
--cc=kernel@pengutronix.de \
--cc=linux-media@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=sakari.ailus@iki.fi \
--cc=slongerbeam@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).