пн, 27 жовт. 2025 р. о 17:44 Hans Verkuil <[email protected]> пише:
>
> Hi Svyatoslav,
>
> On 22/10/2025 16:20, Svyatoslav Ryhel wrote:
> > Get_selection operation may be implemented only for sink pad and may
> > return error code. Set try_crop to 0 instead of returning error.
>
> Can you mention why try_crop is set to 0 instead of returning an error?
>
> That would be good to have in the commit log. And in fact, it's not
> clear to me either why you want this.
>
> >
> > Signed-off-by: Svyatoslav Ryhel <[email protected]>
> > ---
> > drivers/staging/media/tegra-video/vi.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/staging/media/tegra-video/vi.c
> > b/drivers/staging/media/tegra-video/vi.c
> > index 7c44a3448588..856b7c18b551 100644
> > --- a/drivers/staging/media/tegra-video/vi.c
> > +++ b/drivers/staging/media/tegra-video/vi.c
> > @@ -476,15 +476,11 @@ static int __tegra_channel_try_format(struct
> > tegra_vi_channel *chan,
> > fse.code = fmtinfo->code;
> > ret = v4l2_subdev_call(subdev, pad, enum_frame_size, sd_state, &fse);
> > if (ret) {
> > - if (!v4l2_subdev_has_op(subdev, pad, get_selection)) {
> > + if (!v4l2_subdev_has_op(subdev, pad, get_selection) ||
> > + v4l2_subdev_call(subdev, pad, get_selection, NULL,
> > &sdsel)) {
> > try_crop->width = 0;
> > try_crop->height = 0;
>
> This looks all a bit magical. Which subdev is queried here? I.e. what is the
> corresponding
> subdev driver that implements get_selection?
>
Camera sensor subdev, Tegra VI driver directly interacts with camera sensor.
> > } else {
> > - ret = v4l2_subdev_call(subdev, pad, get_selection,
> > - NULL, &sdsel);
> > - if (ret)
> > - return -EINVAL;
> > -
> > try_crop->width = sdsel.r.width;
> > try_crop->height = sdsel.r.height;
> > }
>
> It looks odd (esp. setting try_crop to 0), and I wonder if this code path has
> been tested.
>
Yes it was tested.
Original code checked if the camera sensor has get_selection
implemented and if such operation is supported then it applies width
and height from get_selection, else zeroes. This works just fine with
most cameras and v4l2 compliance tests pass fine, with most but not
with mt9m114 which implements get_selection only for its ifp source
pad while sink pad always returns -EINVAL, hence VI driver fails with
-EINVAL too. To address drivers like mt9m114 instead of just fail with
-EINVAL if get_selection returns error try_crop width and height will
be set the same as get_selection is not implemented.
> Regards,
>
> Hans
