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

[Laurent Pinchart]

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.

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.
	 */

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