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

[Jacopo Mondi]

Hi Laurent,

On Fri, Feb 12, 2021 at 04:17:49PM +0200, Laurent Pinchart wrote:
> Hi Jean-Michel,
>
> Thank you for the patch.
>
> On Fri, Feb 12, 2021 at 02:18:51PM +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>
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 61f7bf43..6957e3a9 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -551,9 +551,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >
> >  	/* Apply the "pipe_mode" control to the ImgU subdevice. */
> >  	ControlList ctrls(imgu->imgu_->controls());
> > +	/*
> > +	 * \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.
>
> Raw images are captured at the output of the CIO2, the ImgU won't be in
> the pipeline. However, when capturing raw frames we still want to run
> the IPA, so we'll need statistics, which will be produced by the ImgU.
> The output images will be discarded in that case.

Correct, sorry I was confused, of course RAW won't go through the ImgU

>
> I however agree that we need to check what the use cases for still
> capture mode are. If the mode exists, it's probably meant for something
> (although one never knows, I think you have first hand experience with
> this). How about the following comment ?
>
> 	/*
> 	 * Set the ImgU pipe mode to 'Video' unconditionally to have statistics
> 	 * generated.
> 	 *
> 	 * \todo Figure out what the 'Still Capture' mode is meant for, and use
> 	 * it accordingly.
> 	 */

Agree. The question stays, what is still capture for if it does not
generate statistics and goes through the ImgU...

>
> With this,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > +	 */
> >  	ctrls.set(V4L2_CID_IPU3_PIPE_MODE,
> > -		  static_cast<int32_t>(vfCfg ? IPU3PipeModeVideo :
> > -				       IPU3PipeModeStillCapture));
> > +		  static_cast<int32_t>(IPU3PipeModeVideo));
> >  	ret = imgu->imgu_->setControls(&ctrls);
> >  	if (ret) {
> >  		LOG(IPU3, Error) << "Unable to set pipe_mode control";
>
> --
> Regards,
>
> Laurent Pinchart
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel