[libcamera-devel,v4,04/12] libcamera: ipu3: Add multiple stream memory management

959 diff mbox series

Perform allocation and setup of memory sharing between the CIO2 output
and the ImgU input and allocate memory for each active stream.
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 65 +++++++++++++++++++++-------
 1 file changed, 49 insertions(+), 16 deletions(-)

Hi Jacopo,

Thank you for the patch.

On Tue, Apr 09, 2019 at 09:25:40PM +0200, Jacopo Mondi wrote:
> Perform allocation and setup of memory sharing between the CIO2 output
> and the ImgU input and allocate memory for each active stream.

No SoB ?

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 65 +++++++++++++++++++++-------
>  1 file changed, 49 insertions(+), 16 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 527213a8970a..f5d768f9f87f 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -91,6 +91,7 @@ public:
>  
>  	BufferPool vfPool_;
>  	BufferPool statPool_;
> +	BufferPool outPool_;
>  };
>  
>  class CIO2Device
> @@ -380,13 +381,23 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
>  	return 0;
>  }
>  
> +/**
> + * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and
> + * started even if not in use. As of now, if not properly configured and
> + * enabled, the ImgU processing pipeline stalls.
> + *
> + * In order to be able to start the 'viewfinder' and 'stat' nodes, we need
> + * memory to be reserved.
> + */
>  int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
>  					 const std::set<Stream *> &streams)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	Stream *stream = *streams.begin();
> +	IPU3Stream *outStream = &data->outStream_;
> +	IPU3Stream *vfStream = &data->vfStream_;
>  	CIO2Device *cio2 = &data->cio2_;
>  	ImgUDevice *imgu = data->imgu_;
> +
>  	int ret;
>  
>  	/* Share buffers between CIO2 output and ImgU input. */
> @@ -398,28 +409,49 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
>  	if (ret)
>  		return ret;
>  
> -	/* Export ImgU output buffers to the stream's pool. */
> -	ret = imgu->exportBuffers(&imgu->output_, &stream->bufferPool());
> -	if (ret)
> -		return ret;
> -
>  	/*
> -	 * Reserve memory in viewfinder and stat output devices. Use the
> -	 * same number of buffers as the ones requested for the output
> -	 * stream.
> +	 * Use for the stat's internal pools the same number of buffer as

s/pools/pool/

With this fixed,

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

> +	 * for the input pool.
> +	 * \todo To be revised when we'll actually use the stat node.
>  	 */
> -	unsigned int bufferCount = stream->bufferPool().count();
> -
> -	imgu->viewfinder_.pool->createBuffers(bufferCount);
> -	ret = imgu->exportBuffers(&imgu->viewfinder_, imgu->viewfinder_.pool);
> -	if (ret)
> -		return ret;
> -
> +	unsigned int bufferCount = pool->count();
>  	imgu->stat_.pool->createBuffers(bufferCount);
>  	ret = imgu->exportBuffers(&imgu->stat_, imgu->stat_.pool);
>  	if (ret)
>  		return ret;
>  
> +	/* Allocate buffers for each active stream. */
> +	for (Stream *s : streams) {
> +		IPU3Stream *stream = static_cast<IPU3Stream *>(s);
> +		ImgUDevice::ImgUOutput *dev = stream->device_;
> +
> +		ret = imgu->exportBuffers(dev, &stream->bufferPool());
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/*
> +	 * Allocate buffers also on non-active outputs; use the same number
> +	 * of buffers as the active ones.
> +	 */
> +	if (!outStream->active_) {
> +		bufferCount = vfStream->bufferPool().count();
> +		outStream->device_->pool->createBuffers(bufferCount);
> +		ret = imgu->exportBuffers(outStream->device_,
> +					  outStream->device_->pool);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!vfStream->active_) {
> +		bufferCount = outStream->bufferPool().count();
> +		vfStream->device_->pool->createBuffers(bufferCount);
> +		ret = imgu->exportBuffers(vfStream->device_,
> +					  vfStream->device_->pool);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -780,6 +812,7 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
>  
>  	output_.pad = PAD_OUTPUT;
>  	output_.name = "output";
> +	output_.pool = &outPool_;
>  
>  	viewfinder_.dev = V4L2Device::fromEntityName(media,
>  						     name_ + " viewfinder");