linux-kernel - Re: [PATCH v5 06/23] staging: media: tegra-video: vi: adjust get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPVz0n277DZ8S5wX5n0gCaOpng64uEfx3VVue4Zp0n5bsDevqQ@mail.gmail.com>
Date: Mon, 27 Oct 2025 18:49:57 +0200
From: Svyatoslav Ryhel <clamor95@...il.com>
To: Hans Verkuil <hverkuil+cisco@...nel.org>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, 
	Sowjanya Komatineni <skomatineni@...dia.com>, Luca Ceresoli <luca.ceresoli@...tlin.com>, 
	Prashant Gaikwad <pgaikwad@...dia.com>, Michael Turquette <mturquette@...libre.com>, 
	Stephen Boyd <sboyd@...nel.org>, Mikko Perttunen <mperttunen@...dia.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Jonas Schwöbel <jonasschwoebel@...oo.de>, 
	Dmitry Osipenko <digetx@...il.com>, Charan Pedumuru <charan.pedumuru@...il.com>, 
	Diogo Ivo <diogo.ivo@...nico.ulisboa.pt>, Aaron Kling <webgeek1234@...il.com>, 
	Arnd Bergmann <arnd@...db.de>, dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org, 
	linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-media@...r.kernel.org, linux-clk@...r.kernel.org, 
	linux-staging@...ts.linux.dev
Subject: Re: [PATCH v5 06/23] staging: media: tegra-video: vi: adjust
 get_selection op check

пн, 27 жовт. 2025 р. о 17:44 Hans Verkuil <hverkuil+cisco@...nel.org> пише:
>
> 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 <clamor95@...il.com>
> > ---
> >  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