On 17/06/2020 03:41, Sowjanya Komatineni wrote: > This patch adds selection v4l2 ioctl operations to allow configuring > a selection rectangle in the sensor through the Tegra video device > node. > > Some sensor drivers supporting crop uses try_crop rectangle from > v4l2_subdev_pad_config during try format for computing binning. > > So with selection ops support, this patch also updates try format > to use try crop rectangle either from subdev frame size enumeration > or from subdev crop boundary. > > Signed-off-by: Sowjanya Komatineni <skomatineni@xxxxxxxxxx> > --- > drivers/staging/media/tegra-video/vi.c | 106 +++++++++++++++++++++++++++++++++ > 1 file changed, 106 insertions(+) > > diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c > index 506c263..f9eb96b 100644 > --- a/drivers/staging/media/tegra-video/vi.c > +++ b/drivers/staging/media/tegra-video/vi.c > @@ -427,6 +427,13 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan, > struct v4l2_subdev *subdev; > struct v4l2_subdev_format fmt; > struct v4l2_subdev_pad_config *pad_cfg; > + struct v4l2_subdev_frame_size_enum fse = { > + .which = V4L2_SUBDEV_FORMAT_TRY, > + }; > + struct v4l2_subdev_selection sdsel = { > + .which = V4L2_SUBDEV_FORMAT_ACTIVE, > + .target = V4L2_SEL_TGT_CROP_BOUNDS, > + }; > int ret; > > subdev = tegra_channel_get_remote_subdev(chan, true); > @@ -449,6 +456,24 @@ static int __tegra_channel_try_format(struct tegra_vi_channel *chan, > fmt.which = V4L2_SUBDEV_FORMAT_TRY; > fmt.pad = 0; > v4l2_fill_mbus_format(&fmt.format, pix, fmtinfo->code); > + > + /* > + * Attempt to obtain the format size from subdev. > + * If not available, try to get crop boundary from subdev. > + */ > + fse.code = fmtinfo->code; > + ret = v4l2_subdev_call(subdev, pad, enum_frame_size, pad_cfg, &fse); > + if (ret) { > + ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel); > + if (ret) > + return -EINVAL; > + pad_cfg->try_crop.width = sdsel.r.width; > + pad_cfg->try_crop.height = sdsel.r.height; > + } else { > + pad_cfg->try_crop.width = fse.max_width; > + pad_cfg->try_crop.height = fse.max_height; > + } > + > ret = v4l2_subdev_call(subdev, pad, set_fmt, pad_cfg, &fmt); > if (ret < 0) > return ret; > @@ -540,6 +565,85 @@ static int tegra_channel_set_subdev_active_fmt(struct tegra_vi_channel *chan) > return 0; > } > > +static int tegra_channel_g_selection(struct file *file, void *priv, > + struct v4l2_selection *sel) > +{ > + struct tegra_vi_channel *chan = video_drvdata(file); > + struct v4l2_subdev *subdev; > + struct v4l2_subdev_format fmt = { > + .which = V4L2_SUBDEV_FORMAT_ACTIVE, > + }; > + struct v4l2_subdev_selection sdsel = { > + .which = V4L2_SUBDEV_FORMAT_ACTIVE, > + .target = sel->target, > + }; > + int ret; > + > + if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) > + return -ENOTTY; > + > + if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > + return -EINVAL; > + /* > + * Try the get selection operation and fallback to get format if not > + * implemented. > + */ > + subdev = tegra_channel_get_remote_subdev(chan, true); > + ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, &sdsel); > + if (!ret) > + sel->r = sdsel.r; > + if (ret != -ENOIOCTLCMD) > + return ret; > + > + ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt); > + if (ret < 0) > + return ret; > + > + sel->r.left = 0; > + sel->r.top = 0; > + sel->r.width = fmt.format.width; > + sel->r.height = fmt.format.height; > + > + return 0; > +} > + > +static int tegra_channel_s_selection(struct file *file, void *fh, > + struct v4l2_selection *sel) > +{ > + struct tegra_vi_channel *chan = video_drvdata(file); > + struct v4l2_subdev *subdev; > + int ret; > + struct v4l2_subdev_selection sdsel = { > + .which = V4L2_SUBDEV_FORMAT_ACTIVE, > + .target = sel->target, > + .flags = sel->flags, > + .r = sel->r, > + }; > + This function doesn't check if the subdev actually supports set_selection. The imx219 is one such driver: it supports get_selection, but not set_selection. So this code should add these lines to fix the v4l2-compliance fail: subdev = tegra_channel_get_remote_subdev(chan, true); if (!v4l2_subdev_has_op(subdev, pad, set_selection)) return -ENOTTY; > + if (IS_ENABLED(CONFIG_VIDEO_TEGRA_TPG)) > + return -ENOTTY; > + > + if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) > + return -EINVAL; > + > + if (vb2_is_busy(&chan->queue)) > + return -EBUSY; > + > + subdev = tegra_channel_get_remote_subdev(chan, true); And this line can be dropped. Regards, Hans > + ret = v4l2_subdev_call(subdev, pad, set_selection, NULL, &sdsel); > + if (!ret) { > + sel->r = sdsel.r; > + /* > + * Subdev active format resolution may have changed during > + * set selection operation. So, update channel format to > + * the sub-device active format. > + */ > + return tegra_channel_set_subdev_active_fmt(chan); > + } > + > + return ret; > +} > + > static int tegra_channel_enum_input(struct file *file, void *fh, > struct v4l2_input *inp) > { > @@ -597,6 +701,8 @@ static const struct v4l2_ioctl_ops tegra_channel_ioctl_ops = { > .vidioc_streamoff = vb2_ioctl_streamoff, > .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, > .vidioc_unsubscribe_event = v4l2_event_unsubscribe, > + .vidioc_g_selection = tegra_channel_g_selection, > + .vidioc_s_selection = tegra_channel_s_selection, > }; > > /* >