On 8/14/2025 8:24 PM, Vikash Garodia wrote:
>
>
> On 8/13/2025 3:08 PM, Dikshita Agarwal wrote:
>> Add support for G/S_SELECTION V4L2 ioctls for the encoder video
>> device with necessary hooks. This allows userspace to query and
>> configure rectangular selection areas such as crop.
>>
>> Tested-by: Vikash Garodia <quic_vgarodia@xxxxxxxxxxx> # X1E80100
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx>
>> ---
>> drivers/media/platform/qcom/iris/iris_venc.c | 28 ++++++++++++
>> drivers/media/platform/qcom/iris/iris_venc.h | 1 +
>> drivers/media/platform/qcom/iris/iris_vidc.c | 65 ++++++++++++++++++++++------
>> 3 files changed, 80 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_venc.c b/drivers/media/platform/qcom/iris/iris_venc.c
>> index 3dbcce23cbe94cf0edc4421694a3ba11faa5eb96..930f5afe9489d01be193f1dbe429d33f5401b468 100644
>> --- a/drivers/media/platform/qcom/iris/iris_venc.c
>> +++ b/drivers/media/platform/qcom/iris/iris_venc.c
>> @@ -297,3 +297,31 @@ int iris_venc_subscribe_event(struct iris_inst *inst,
>> return -EINVAL;
>> }
>> }
>> +
>> +int iris_venc_s_selection(struct iris_inst *inst, struct v4l2_selection *s)
>> +{
>> + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
>> + return -EINVAL;
>> +
>> + switch (s->target) {
>> + case V4L2_SEL_TGT_CROP:
>> + s->r.left = 0;
>> + s->r.top = 0;
>> +
>> + if (s->r.width > inst->fmt_src->fmt.pix_mp.width ||
>> + s->r.height > inst->fmt_src->fmt.pix_mp.height)
>> + return -EINVAL;
>> +
>> + inst->crop.left = s->r.left;
>> + inst->crop.top = s->r.top;
>> + inst->crop.width = s->r.width;
>> + inst->crop.height = s->r.height;
>> + inst->fmt_dst->fmt.pix_mp.width = inst->crop.width;
>> + inst->fmt_dst->fmt.pix_mp.height = inst->crop.height;
>> + return iris_venc_s_fmt_output(inst, inst->fmt_dst);
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>
> Why do you need a return here ?
We actually don't, it should be safe to remove this, will fix.
Thanks,
Dikshita
>
>> +}
>> diff --git a/drivers/media/platform/qcom/iris/iris_venc.h b/drivers/media/platform/qcom/iris/iris_venc.h
>> index 2d9614ae18e8a2318df6673fbeae5ee33c99b596..72c6e25d87113baa6d2219ae478b7d7df1aed7bf 100644
>> --- a/drivers/media/platform/qcom/iris/iris_venc.h
>> +++ b/drivers/media/platform/qcom/iris/iris_venc.h
>> @@ -15,5 +15,6 @@ int iris_venc_try_fmt(struct iris_inst *inst, struct v4l2_format *f);
>> int iris_venc_s_fmt(struct iris_inst *inst, struct v4l2_format *f);
>> int iris_venc_validate_format(struct iris_inst *inst, u32 pixelformat);
>> int iris_venc_subscribe_event(struct iris_inst *inst, const struct v4l2_event_subscription *sub);
>> +int iris_venc_s_selection(struct iris_inst *inst, struct v4l2_selection *s);
>>
>> #endif
>> diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
>> index d8c94074153e9b1ceac4f911210ddbf89bbe3533..2074682a35fd1c4c9f5d29fdaee3392d98bf8923 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
>> @@ -462,29 +462,64 @@ static int iris_g_selection(struct file *filp, void *fh, struct v4l2_selection *
>> {
>> struct iris_inst *inst = iris_get_inst(filp, NULL);
>>
>> - if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
>> + inst->domain == DECODER)
>> return -EINVAL;
>>
>> - switch (s->target) {
>> - case V4L2_SEL_TGT_CROP_BOUNDS:
>> - case V4L2_SEL_TGT_CROP_DEFAULT:
>> - case V4L2_SEL_TGT_CROP:
>> - case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>> - case V4L2_SEL_TGT_COMPOSE_PADDED:
>> - case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>> - case V4L2_SEL_TGT_COMPOSE:
>> + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT &&
>> + inst->domain == ENCODER)
>> + return -EINVAL;
>> +
>> + if (inst->domain == DECODER) {
>> + switch (s->target) {
>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>> + case V4L2_SEL_TGT_CROP:
>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>> + case V4L2_SEL_TGT_COMPOSE_PADDED:
>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>> + case V4L2_SEL_TGT_COMPOSE:
>> + s->r.left = inst->crop.left;
>> + s->r.top = inst->crop.top;
>> + s->r.width = inst->crop.width;
>> + s->r.height = inst->crop.height;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + } else if (inst->domain == ENCODER) {
>> + switch (s->target) {
>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>> + s->r.width = inst->fmt_src->fmt.pix_mp.width;
>> + s->r.height = inst->fmt_src->fmt.pix_mp.height;
>> + break;
>> + case V4L2_SEL_TGT_CROP:
>> + s->r.width = inst->crop.width;
>> + s->r.height = inst->crop.height;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> s->r.left = inst->crop.left;
>> s->r.top = inst->crop.top;
>> - s->r.width = inst->crop.width;
>> - s->r.height = inst->crop.height;
>> - break;
>> - default:
>> - return -EINVAL;
>> }
>>
>> return 0;
>> }
>>
>> +static int iris_s_selection(struct file *filp, void *fh, struct v4l2_selection *s)
>> +{
>> + struct iris_inst *inst = iris_get_inst(filp, NULL);
>
> s->r.left and s->r.top are signed and can be negative. Need to range bound
> within 0 to src dimension ?
>
>> +
>> + if (inst->domain == DECODER)
>> + return -EINVAL;
>> + else if (inst->domain == ENCODER)
>> + return iris_venc_s_selection(inst, s);
>> +
>> + return -EINVAL;
>> +}
>> +
>> static int iris_subscribe_event(struct v4l2_fh *fh, const struct v4l2_event_subscription *sub)
>> {
>> struct iris_inst *inst = container_of(fh, struct iris_inst, fh);
>> @@ -591,6 +626,8 @@ static const struct v4l2_ioctl_ops iris_v4l2_ioctl_ops_enc = {
>> .vidioc_querycap = iris_querycap,
>> .vidioc_subscribe_event = iris_subscribe_event,
>> .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>> + .vidioc_g_selection = iris_g_selection,
>> + .vidioc_s_selection = iris_s_selection,
>> };
>>
>> void iris_init_ops(struct iris_core *core)
>>
