[libcamera-devel] [PATCH] libcamera: ipu3: set V4L2_CID_INTEL_IPU3_MODE

[Jacopo Mondi]

Hi Jean-Michel

On Wed, Feb 10, 2021 at 08:48:15PM +0100, Jean-Michel Hautbois wrote:
> Hi Jacopo !
>
> On 10/02/2021 18:43, Jacopo Mondi wrote:
> > Hi Jean-Michel
> >
> > On Tue, Feb 09, 2021 at 03:52:44PM +0100, Jean-Michel Hautbois wrote:
> >> In order to get the stats back, the imgu subdev needs to have the
> >> V4L2_CID_INTEL_IPU3_MODE control set.
> >> Set it to video mode by default to get the stats at each frame.
> >>
> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> >> ---
> >>  src/libcamera/pipeline/ipu3/ipu3.cpp | 3 +--
> >>  1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> index 61f7bf43..104cb532 100644
> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >> @@ -552,8 +552,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >>  	/* Apply the "pipe_mode" control to the ImgU subdevice. */
> >>  	ControlList ctrls(imgu->imgu_->controls());
> >>  	ctrls.set(V4L2_CID_IPU3_PIPE_MODE,
> >> -		  static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo :
> >> -				       IPU3PipeModeStillCapture));
> >> +		  static_cast<int32_t>(IPU3PipeModeVideo));
> >
> > I'm not opposed to this, and know we got to know what effect the pipe
> > mode has, at least on statistics generation.
> >
> > I wonder what other features the StillCapture mode provides.
> > Could you record with a todo that the usage of the pipe mode has to be
> > clarified ?
>
> I don't know if we can know more than what is already in the kernel
> documentation ?
> https://linuxtv.org/downloads/v4l-dvb-apis/admin-guide/ipu3.html#running-mode-and-firmware-binary-selection
>
> I can add a TODO, in a v2 ?

Yes please, I still think we might want to use the StilCapture mode
for, no surprise, still capture of raw frames, which, if I'm not
mistaken should not involve the IPA and for which we don't need
statistics.

If my understanding is correct, something along the lines of:
/*
 * \todo Set the ImgU pipe mode to 'Video' unconditionally to have statistics
 * generated and re-consider if 'Still Capture' should be used for
 * high quality RAW capture operations that do not involve the IPA.
 */

?

Thanks
  j

>
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> Thanks for that ;-).
>
> >
> > Thanks
> >   j
> >>  	ret = imgu->imgu_->setControls(&ctrls);
> >>  	if (ret) {
> >>  		LOG(IPU3, Error) << "Unable to set pipe_mode control";
> >> --
> >> 2.27.0
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> >