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

Message ID 20190409192548.20325-5-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: ipu3: Multiple streams support
Related show

Commit Message

Jacopo Mondi April 9, 2019, 7:25 p.m. UTC
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(-)

Comments

Laurent Pinchart April 15, 2019, 12:36 p.m. UTC | #1
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");

Patch

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
+	 * 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");