[v3,07/14] v4l: ti-vpe: Add selection API in VPE driver
Commit Message
Add selection ioctl ops. For VPE, cropping makes sense only for the input to
VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense
only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output
in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP
results in selecting a rectangle region within the source buffer.
Setting the crop/compose rectangles should successfully result in
re-configuration of registers which are affected when either source or
destination dimensions change, set_srcdst_params() is called for this purpose.
Signed-off-by: Archit Taneja <archit@ti.com>
---
drivers/media/platform/ti-vpe/vpe.c | 141 ++++++++++++++++++++++++++++++++++++
1 file changed, 141 insertions(+)
Comments
Hi Archit,
A few small comments below...
On 03/11/14 09:33, Archit Taneja wrote:
> Add selection ioctl ops. For VPE, cropping makes sense only for the input to
> VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense
> only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
>
> For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output
> in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP
> results in selecting a rectangle region within the source buffer.
>
> Setting the crop/compose rectangles should successfully result in
> re-configuration of registers which are affected when either source or
> destination dimensions change, set_srcdst_params() is called for this purpose.
>
> Signed-off-by: Archit Taneja <archit@ti.com>
> ---
> drivers/media/platform/ti-vpe/vpe.c | 141 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 141 insertions(+)
>
> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
> index ece9b96..4abb85c 100644
> --- a/drivers/media/platform/ti-vpe/vpe.c
> +++ b/drivers/media/platform/ti-vpe/vpe.c
> @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx,
> {
> switch (type) {
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
> return &ctx->q_data[Q_DATA_SRC];
> case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> return &ctx->q_data[Q_DATA_DST];
> default:
> BUG();
> @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
> return set_srcdst_params(ctx);
> }
>
> +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
> +{
> + struct vpe_q_data *q_data;
> +
> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
> + return -EINVAL;
> +
> + q_data = get_q_data(ctx, s->type);
> + if (!q_data)
> + return -EINVAL;
> +
> + switch (s->target) {
> + case V4L2_SEL_TGT_COMPOSE:
> + /*
> + * COMPOSE target is only valid for capture buffer type, for
> + * output buffer type, assign existing crop parameters to the
> + * selection rectangle
> + */
> + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> + break;
Shouldn't this return -EINVAL?
> +
> + s->r = q_data->c_rect;
> + return 0;
> +
> + case V4L2_SEL_TGT_CROP:
> + /*
> + * CROP target is only valid for output buffer type, for capture
> + * buffer type, assign existing compose parameters to the
> + * selection rectangle
> + */
> + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> + break;
Ditto.
> +
> + s->r = q_data->c_rect;
> + return 0;
> +
> + /*
> + * bound and default crop/compose targets are invalid targets to
> + * try/set
> + */
> + default:
> + return -EINVAL;
> + }
> +
> + if (s->r.top < 0 || s->r.left < 0) {
> + vpe_err(ctx->dev, "negative values for top and left\n");
> + s->r.top = s->r.left = 0;
> + }
> +
> + v4l_bound_align_image(&s->r.width, MIN_W, q_data->width, 1,
> + &s->r.height, MIN_H, q_data->height, H_ALIGN, S_ALIGN);
> +
> + /* adjust left/top if cropping rectangle is out of bounds */
> + if (s->r.left + s->r.width > q_data->width)
> + s->r.left = q_data->width - s->r.width;
> + if (s->r.top + s->r.height > q_data->height)
> + s->r.top = q_data->height - s->r.height;
> +
> + return 0;
> +}
> +
> +static int vpe_g_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + struct vpe_ctx *ctx = file2ctx(file);
> + struct vpe_q_data *q_data;
> +
> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
> + return -EINVAL;
> +
> + q_data = get_q_data(ctx, s->type);
> + if (!q_data)
> + return -EINVAL;
> +
> + switch (s->target) {
> + /* return width and height from S_FMT of the respective buffer type */
> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> + case V4L2_SEL_TGT_CROP_BOUNDS:
> + case V4L2_SEL_TGT_CROP_DEFAULT:
> + s->r.left = 0;
> + s->r.top = 0;
> + s->r.width = q_data->width;
> + s->r.height = q_data->height;
The crop targets only make sense for type OUTPUT and the compose only for
type CAPTURE. Add some checks for that.
> + return 0;
> +
> + /*
> + * CROP target holds for the output buffer type, and COMPOSE target
> + * holds for the capture buffer type. We still return the c_rect params
> + * for both the target types.
> + */
> + case V4L2_SEL_TGT_COMPOSE:
> + case V4L2_SEL_TGT_CROP:
> + s->r.left = q_data->c_rect.left;
> + s->r.top = q_data->c_rect.top;
> + s->r.width = q_data->c_rect.width;
> + s->r.height = q_data->c_rect.height;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +
> +static int vpe_s_selection(struct file *file, void *fh,
> + struct v4l2_selection *s)
> +{
> + struct vpe_ctx *ctx = file2ctx(file);
> + struct vpe_q_data *q_data;
> + struct v4l2_selection sel = *s;
> + int ret;
> +
> + ret = __vpe_try_selection(ctx, &sel);
> + if (ret)
> + return ret;
> +
> + q_data = get_q_data(ctx, sel.type);
> + if (!q_data)
> + return -EINVAL;
> +
> + if ((q_data->c_rect.left == sel.r.left) &&
> + (q_data->c_rect.top == sel.r.top) &&
> + (q_data->c_rect.width == sel.r.width) &&
> + (q_data->c_rect.height == sel.r.height)) {
> + vpe_dbg(ctx->dev,
> + "requested crop/compose values are already set\n");
> + return 0;
> + }
> +
> + q_data->c_rect = sel.r;
> +
> + return set_srcdst_params(ctx);
> +}
> +
> static int vpe_reqbufs(struct file *file, void *priv,
> struct v4l2_requestbuffers *reqbufs)
> {
> @@ -1674,6 +1812,9 @@ static const struct v4l2_ioctl_ops vpe_ioctl_ops = {
> .vidioc_try_fmt_vid_out_mplane = vpe_try_fmt,
> .vidioc_s_fmt_vid_out_mplane = vpe_s_fmt,
>
> + .vidioc_g_selection = vpe_g_selection,
> + .vidioc_s_selection = vpe_s_selection,
> +
> .vidioc_reqbufs = vpe_reqbufs,
> .vidioc_querybuf = vpe_querybuf,
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote:
> Hi Archit,
>
> A few small comments below...
>
> On 03/11/14 09:33, Archit Taneja wrote:
>> Add selection ioctl ops. For VPE, cropping makes sense only for the input to
>> VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense
>> only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
>>
>> For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output
>> in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP
>> results in selecting a rectangle region within the source buffer.
>>
>> Setting the crop/compose rectangles should successfully result in
>> re-configuration of registers which are affected when either source or
>> destination dimensions change, set_srcdst_params() is called for this purpose.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>> drivers/media/platform/ti-vpe/vpe.c | 141 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 141 insertions(+)
>>
>> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
>> index ece9b96..4abb85c 100644
>> --- a/drivers/media/platform/ti-vpe/vpe.c
>> +++ b/drivers/media/platform/ti-vpe/vpe.c
>> @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx,
>> {
>> switch (type) {
>> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>> return &ctx->q_data[Q_DATA_SRC];
>> case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>> return &ctx->q_data[Q_DATA_DST];
>> default:
>> BUG();
>> @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
>> return set_srcdst_params(ctx);
>> }
>>
>> +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
>> +{
>> + struct vpe_q_data *q_data;
>> +
>> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
>> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
>> + return -EINVAL;
>> +
>> + q_data = get_q_data(ctx, s->type);
>> + if (!q_data)
>> + return -EINVAL;
>> +
>> + switch (s->target) {
>> + case V4L2_SEL_TGT_COMPOSE:
>> + /*
>> + * COMPOSE target is only valid for capture buffer type, for
>> + * output buffer type, assign existing crop parameters to the
>> + * selection rectangle
>> + */
>> + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> + break;
>
> Shouldn't this return -EINVAL?
compose only makes sense for CAPTURE. So, it breaks and performs the
calculations after the switch statement.
>
>> +
>> + s->r = q_data->c_rect;
>> + return 0;
The above 2 lines are called for if we try to set compose for OUTPUT. I
don't return an error here, just keep the size as the original rect
size, and return 0.
I'll replace these 2 lines with 'return -EINVAL;'
>> +
>> + case V4L2_SEL_TGT_CROP:
>> + /*
>> + * CROP target is only valid for output buffer type, for capture
>> + * buffer type, assign existing compose parameters to the
>> + * selection rectangle
>> + */
>> + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
>> + break;
>
> Ditto.
>
>> +
>> + s->r = q_data->c_rect;
>> + return 0;
>> +
>> + /*
>> + * bound and default crop/compose targets are invalid targets to
>> + * try/set
>> + */
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + if (s->r.top < 0 || s->r.left < 0) {
>> + vpe_err(ctx->dev, "negative values for top and left\n");
>> + s->r.top = s->r.left = 0;
>> + }
>> +
>> + v4l_bound_align_image(&s->r.width, MIN_W, q_data->width, 1,
>> + &s->r.height, MIN_H, q_data->height, H_ALIGN, S_ALIGN);
>> +
>> + /* adjust left/top if cropping rectangle is out of bounds */
>> + if (s->r.left + s->r.width > q_data->width)
>> + s->r.left = q_data->width - s->r.width;
>> + if (s->r.top + s->r.height > q_data->height)
>> + s->r.top = q_data->height - s->r.height;
>> +
>> + return 0;
>> +}
>> +
>> +static int vpe_g_selection(struct file *file, void *fh,
>> + struct v4l2_selection *s)
>> +{
>> + struct vpe_ctx *ctx = file2ctx(file);
>> + struct vpe_q_data *q_data;
>> +
>> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
>> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
>> + return -EINVAL;
>> +
>> + q_data = get_q_data(ctx, s->type);
>> + if (!q_data)
>> + return -EINVAL;
>> +
>> + switch (s->target) {
>> + /* return width and height from S_FMT of the respective buffer type */
>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>> + s->r.left = 0;
>> + s->r.top = 0;
>> + s->r.width = q_data->width;
>> + s->r.height = q_data->height;
>
> The crop targets only make sense for type OUTPUT and the compose only for
> type CAPTURE. Add some checks for that.
I return the image size for G_FMT irrespective of what combination of
crop/compose CAPTURE/OUTPUT it is. I suppose it is better to return an
error if the user tries to configure a wrong combination.
Thanks,
Archit
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/11/14 13:46, Archit Taneja wrote:
> On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote:
>> Hi Archit,
>>
>> A few small comments below...
>>
>> On 03/11/14 09:33, Archit Taneja wrote:
>>> Add selection ioctl ops. For VPE, cropping makes sense only for the input to
>>> VPE(or V4L2_BUF_TYPE_VIDEO_OUTPUT/MPLANE buffers) and composing makes sense
>>> only for the output of VPE(or V4L2_BUF_TYPE_VIDEO_CAPTURE/MPLANE buffers).
>>>
>>> For the CAPTURE type, V4L2_SEL_TGT_COMPOSE results in VPE writing the output
>>> in a rectangle within the capture buffer. For the OUTPUT type, V4L2_SEL_TGT_CROP
>>> results in selecting a rectangle region within the source buffer.
>>>
>>> Setting the crop/compose rectangles should successfully result in
>>> re-configuration of registers which are affected when either source or
>>> destination dimensions change, set_srcdst_params() is called for this purpose.
>>>
>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>> ---
>>> drivers/media/platform/ti-vpe/vpe.c | 141 ++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 141 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/ti-vpe/vpe.c b/drivers/media/platform/ti-vpe/vpe.c
>>> index ece9b96..4abb85c 100644
>>> --- a/drivers/media/platform/ti-vpe/vpe.c
>>> +++ b/drivers/media/platform/ti-vpe/vpe.c
>>> @@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx,
>>> {
>>> switch (type) {
>>> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>>> + case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>>> return &ctx->q_data[Q_DATA_SRC];
>>> case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>>> + case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>>> return &ctx->q_data[Q_DATA_DST];
>>> default:
>>> BUG();
>>> @@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
>>> return set_srcdst_params(ctx);
>>> }
>>>
>>> +static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
>>> +{
>>> + struct vpe_q_data *q_data;
>>> +
>>> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
>>> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
>>> + return -EINVAL;
>>> +
>>> + q_data = get_q_data(ctx, s->type);
>>> + if (!q_data)
>>> + return -EINVAL;
>>> +
>>> + switch (s->target) {
>>> + case V4L2_SEL_TGT_COMPOSE:
>>> + /*
>>> + * COMPOSE target is only valid for capture buffer type, for
>>> + * output buffer type, assign existing crop parameters to the
>>> + * selection rectangle
>>> + */
>>> + if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>> + break;
>>
>> Shouldn't this return -EINVAL?
>
> compose only makes sense for CAPTURE. So, it breaks and performs the calculations after the switch statement.
It's so easy to get confused here.
>
>>
>>> +
>>> + s->r = q_data->c_rect;
>>> + return 0;
>
> The above 2 lines are called for if we try to set compose for OUTPUT. I don't return an error here, just keep the size as the original rect size, and return 0.
>
> I'll replace these 2 lines with 'return -EINVAL;'
Yes, that makes more sense.
>
>>> +
>>> + case V4L2_SEL_TGT_CROP:
>>> + /*
>>> + * CROP target is only valid for output buffer type, for capture
>>> + * buffer type, assign existing compose parameters to the
>>> + * selection rectangle
>>> + */
>>> + if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
>>> + break;
>>
>> Ditto.
>>
>>> +
>>> + s->r = q_data->c_rect;
>>> + return 0;
>>> +
>>> + /*
>>> + * bound and default crop/compose targets are invalid targets to
>>> + * try/set
>>> + */
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + if (s->r.top < 0 || s->r.left < 0) {
>>> + vpe_err(ctx->dev, "negative values for top and left\n");
>>> + s->r.top = s->r.left = 0;
>>> + }
>>> +
>>> + v4l_bound_align_image(&s->r.width, MIN_W, q_data->width, 1,
>>> + &s->r.height, MIN_H, q_data->height, H_ALIGN, S_ALIGN);
>>> +
>>> + /* adjust left/top if cropping rectangle is out of bounds */
>>> + if (s->r.left + s->r.width > q_data->width)
>>> + s->r.left = q_data->width - s->r.width;
>>> + if (s->r.top + s->r.height > q_data->height)
>>> + s->r.top = q_data->height - s->r.height;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int vpe_g_selection(struct file *file, void *fh,
>>> + struct v4l2_selection *s)
>>> +{
>>> + struct vpe_ctx *ctx = file2ctx(file);
>>> + struct vpe_q_data *q_data;
>>> +
>>> + if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
>>> + (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
>>> + return -EINVAL;
>>> +
>>> + q_data = get_q_data(ctx, s->type);
>>> + if (!q_data)
>>> + return -EINVAL;
>>> +
>>> + switch (s->target) {
>>> + /* return width and height from S_FMT of the respective buffer type */
>>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>>> + s->r.left = 0;
>>> + s->r.top = 0;
>>> + s->r.width = q_data->width;
>>> + s->r.height = q_data->height;
>>
>> The crop targets only make sense for type OUTPUT and the compose only for
>> type CAPTURE. Add some checks for that.
>
> I return the image size for G_FMT irrespective of what combination of
> crop/compose CAPTURE/OUTPUT it is. I suppose it is better to return
> an error if the user tries to configure a wrong combination.
Yes. If for no other reason that I plan on adding crop/compose/scaling support
to v4l2-compliance soon, and this will probably be one of the tests.
>
> Thanks,
> Archit
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 11 March 2014 06:19 PM, Hans Verkuil wrote:
> On 03/11/14 13:46, Archit Taneja wrote:
>> On Tuesday 11 March 2014 05:51 PM, Hans Verkuil wrote:
>>> Hi Archit,
>>>
>>> A few small comments below...
>>>
>>> On 03/11/14 09:33, Archit Taneja wrote:
<snip>
> Yes. If for no other reason that I plan on adding crop/compose/scaling support
> to v4l2-compliance soon, and this will probably be one of the tests.
Thanks, will update. Thanks for reviewing of the series.
Archit
>
>>
>> Thanks,
>> Archit
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -410,8 +410,10 @@ static struct vpe_q_data *get_q_data(struct vpe_ctx *ctx,
{
switch (type) {
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_OUTPUT:
return &ctx->q_data[Q_DATA_SRC];
case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+ case V4L2_BUF_TYPE_VIDEO_CAPTURE:
return &ctx->q_data[Q_DATA_DST];
default:
BUG();
@@ -1587,6 +1589,142 @@ static int vpe_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
return set_srcdst_params(ctx);
}
+static int __vpe_try_selection(struct vpe_ctx *ctx, struct v4l2_selection *s)
+{
+ struct vpe_q_data *q_data;
+
+ if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
+ (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s->type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s->target) {
+ case V4L2_SEL_TGT_COMPOSE:
+ /*
+ * COMPOSE target is only valid for capture buffer type, for
+ * output buffer type, assign existing crop parameters to the
+ * selection rectangle
+ */
+ if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ break;
+
+ s->r = q_data->c_rect;
+ return 0;
+
+ case V4L2_SEL_TGT_CROP:
+ /*
+ * CROP target is only valid for output buffer type, for capture
+ * buffer type, assign existing compose parameters to the
+ * selection rectangle
+ */
+ if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
+ break;
+
+ s->r = q_data->c_rect;
+ return 0;
+
+ /*
+ * bound and default crop/compose targets are invalid targets to
+ * try/set
+ */
+ default:
+ return -EINVAL;
+ }
+
+ if (s->r.top < 0 || s->r.left < 0) {
+ vpe_err(ctx->dev, "negative values for top and left\n");
+ s->r.top = s->r.left = 0;
+ }
+
+ v4l_bound_align_image(&s->r.width, MIN_W, q_data->width, 1,
+ &s->r.height, MIN_H, q_data->height, H_ALIGN, S_ALIGN);
+
+ /* adjust left/top if cropping rectangle is out of bounds */
+ if (s->r.left + s->r.width > q_data->width)
+ s->r.left = q_data->width - s->r.width;
+ if (s->r.top + s->r.height > q_data->height)
+ s->r.top = q_data->height - s->r.height;
+
+ return 0;
+}
+
+static int vpe_g_selection(struct file *file, void *fh,
+ struct v4l2_selection *s)
+{
+ struct vpe_ctx *ctx = file2ctx(file);
+ struct vpe_q_data *q_data;
+
+ if ((s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) &&
+ (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT))
+ return -EINVAL;
+
+ q_data = get_q_data(ctx, s->type);
+ if (!q_data)
+ return -EINVAL;
+
+ switch (s->target) {
+ /* return width and height from S_FMT of the respective buffer type */
+ case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+ case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+ case V4L2_SEL_TGT_CROP_BOUNDS:
+ case V4L2_SEL_TGT_CROP_DEFAULT:
+ s->r.left = 0;
+ s->r.top = 0;
+ s->r.width = q_data->width;
+ s->r.height = q_data->height;
+ return 0;
+
+ /*
+ * CROP target holds for the output buffer type, and COMPOSE target
+ * holds for the capture buffer type. We still return the c_rect params
+ * for both the target types.
+ */
+ case V4L2_SEL_TGT_COMPOSE:
+ case V4L2_SEL_TGT_CROP:
+ s->r.left = q_data->c_rect.left;
+ s->r.top = q_data->c_rect.top;
+ s->r.width = q_data->c_rect.width;
+ s->r.height = q_data->c_rect.height;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+
+static int vpe_s_selection(struct file *file, void *fh,
+ struct v4l2_selection *s)
+{
+ struct vpe_ctx *ctx = file2ctx(file);
+ struct vpe_q_data *q_data;
+ struct v4l2_selection sel = *s;
+ int ret;
+
+ ret = __vpe_try_selection(ctx, &sel);
+ if (ret)
+ return ret;
+
+ q_data = get_q_data(ctx, sel.type);
+ if (!q_data)
+ return -EINVAL;
+
+ if ((q_data->c_rect.left == sel.r.left) &&
+ (q_data->c_rect.top == sel.r.top) &&
+ (q_data->c_rect.width == sel.r.width) &&
+ (q_data->c_rect.height == sel.r.height)) {
+ vpe_dbg(ctx->dev,
+ "requested crop/compose values are already set\n");
+ return 0;
+ }
+
+ q_data->c_rect = sel.r;
+
+ return set_srcdst_params(ctx);
+}
+
static int vpe_reqbufs(struct file *file, void *priv,
struct v4l2_requestbuffers *reqbufs)
{
@@ -1674,6 +1812,9 @@ static const struct v4l2_ioctl_ops vpe_ioctl_ops = {
.vidioc_try_fmt_vid_out_mplane = vpe_try_fmt,
.vidioc_s_fmt_vid_out_mplane = vpe_s_fmt,
+ .vidioc_g_selection = vpe_g_selection,
+ .vidioc_s_selection = vpe_s_selection,
+
.vidioc_reqbufs = vpe_reqbufs,
.vidioc_querybuf = vpe_querybuf,