[libcamera-devel] [PATCH v4 1/2] libcamera: ipu3: Move ipa configuration from start() to configure()

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Tue, Mar 16, 2021 at 04:40:22PM +0100, Jean-Michel Hautbois wrote:
> IPA was configured after all the pipeline devices were started,
> including IPA itself...
> Move it at the end of configure() call.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

The change looks good. Could you make sure it doesn't break anything if
we call configure() twice without any start() in-between ? This check
should be added to lc-compliance (once we get it merged), in the
meantime, you can modify any test application to duplicate the
configure() call. Bonus points if this can be run under valgrind too.

Once done,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 2ea13ec9..bb61ef4a 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -633,6 +633,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  		return ret;
>  	}
>  
> +	std::map<uint32_t, ControlInfoMap> entityControls;
> +	entityControls.emplace(0, data->cio2_.sensor()->controls());
> +	data->ipa_->configure(entityControls);
> +
>  	return 0;
>  }
>  
> @@ -717,7 +721,6 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera)
>  
>  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
>  {
> -	std::map<uint32_t, ControlInfoMap> entityControls;
>  	IPU3CameraData *data = cameraData(camera);
>  	CIO2Device *cio2 = &data->cio2_;
>  	ImgUDevice *imgu = data->imgu_;
> @@ -744,9 +747,6 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
>  	if (ret)
>  		goto error;
>  
> -	entityControls.emplace(0, data->cio2_.sensor()->controls());
> -	data->ipa_->configure(entityControls);
> -
>  	return 0;
>  
>  error:

-- 
Regards,

Laurent Pinchart