[libcamera-devel] [PATCH] libcamera: pipeline: ipu3: Stop IPA before stopping devices

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Mon, Mar 08, 2021 at 04:29:15PM +0000, Kieran Bingham wrote:
> The IPA should be stopped before the hardware devices to ensure that
> all asynchronous actions have completed within the IPA before resources
> are removed and released.

This makes sense,

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

I however wonder if we should set a "stopping" flag before calling
data->ipa_->stop(), to let the handlers for all the asynchronous actions
invoked during stop() know that we're stopping. This would for instance
allow us to avoid requeing buffers unnecessarily. Maybe it's not the
best way to handle the issue, and in any case, if it's needed, it's a
candidate for a patch on top.

Does this solve the buffer queue after buffer free bug ?

> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 2b4d31500533..498f85634a83 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -764,13 +764,13 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  	IPU3CameraData *data = cameraData(camera);
>  	int ret = 0;
>  
> +	data->ipa_->stop();
> +
>  	ret |= data->imgu_->stop();
>  	ret |= data->cio2_.stop();
>  	if (ret)
>  		LOG(IPU3, Warning) << "Failed to stop camera " << camera->id();
>  
> -	data->ipa_->stop();
> -
>  	freeBuffers(camera);
>  }
>  

-- 
Regards,

Laurent Pinchart