Re: [PATCH v1 09/19] staging: media: tegra-video: vi: add flip controls only if no source controls are provided

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hello Svyatoslav,

On Fri, 5 Sep 2025 19:05:16 +0300
Svyatoslav Ryhel <clamor95@gmail.com> wrote:

> пт, 5 вер. 2025 р. о 18:59 Luca Ceresoli <luca.ceresoli@bootlin.com> пише:
> >
> > Hello Svyatoslav,
> >
> > On Tue, 19 Aug 2025 15:16:21 +0300
> > Svyatoslav Ryhel <clamor95@gmail.com> wrote:
> >  
> > > Add HFLIP and VFLIP from SoC only if camera sensor does not provide those
> > > controls.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  drivers/staging/media/tegra-video/vi.c | 9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/tegra-video/vi.c b/drivers/staging/media/tegra-video/vi.c
> > > index 4f67adc395ac..61b65a2c1436 100644
> > > --- a/drivers/staging/media/tegra-video/vi.c
> > > +++ b/drivers/staging/media/tegra-video/vi.c
> > > @@ -961,6 +961,7 @@ static int tegra_channel_setup_ctrl_handler(struct tegra_vi_channel *chan)
> > >       }
> > >  #else
> > >       struct v4l2_subdev *subdev;
> > > +     struct v4l2_ctrl *hflip, *vflip;
> > >
> > >       /* custom control */
> > >       v4l2_ctrl_new_custom(&chan->ctrl_handler, &syncpt_timeout_ctrl, NULL);
> > > @@ -986,11 +987,13 @@ static int tegra_channel_setup_ctrl_handler(struct tegra_vi_channel *chan)
> > >               return ret;
> > >       }
> > >
> > > -     if (chan->vi->soc->has_h_v_flip) {
> > > +     hflip = v4l2_ctrl_find(subdev->ctrl_handler, V4L2_CID_HFLIP);
> > > +     if (chan->vi->soc->has_h_v_flip && !hflip)
> > >               v4l2_ctrl_new_std(&chan->ctrl_handler, &vi_ctrl_ops, V4L2_CID_HFLIP, 0, 1, 1, 0);
> > > -             v4l2_ctrl_new_std(&chan->ctrl_handler, &vi_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);
> > > -     }
> > >
> > > +     vflip = v4l2_ctrl_find(subdev->ctrl_handler, V4L2_CID_VFLIP);
> > > +     if (chan->vi->soc->has_h_v_flip && !vflip)
> > > +             v4l2_ctrl_new_std(&chan->ctrl_handler, &vi_ctrl_ops, V4L2_CID_VFLIP, 0, 1, 1, 0);  
> >
> > Based on my understanding of V4L2, this should not be done.
> > AFAIK subdevs should expose what the hardware block can do,
> > independently from other subdevs. It is up to userspace (e.g.
> > libcamera) to use the most appropriate control when there are redundant
> > ones.
> >  
> 
> This driver is video-centric, interactions are done via /dev/videoX
> not subdevices like media-centric derivers do. Conversion is possible
> but it is not scope of this patchset and in case such conversion takes
> place, one who will do that, will definitely know what to do.
> Video-centric drivers expose all controls within single video device
> and it cannot hold duplicates of controls, this causes error. So this
> solution exposes camera flip controls and if camera has none, SoC
> controls are exposed.

Sorry, forgot to think in the past. :-)
Thanks for the clarification, it makes sense now!

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com