[libcamera-devel] [PATCH v7 07/13] libcamera: ipu3: Implement memory handling

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Apr 2 19:39:23 CEST 2019


Hi Jacopo,

Thank you for the patch.

On Tue, Apr 02, 2019 at 07:13:03PM +0200, Jacopo Mondi wrote:
> Implement buffer allocation and relase in IPU3 pipeline handlers.

s/relase/release/

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

> As the pipeline handler currently supports a single stream, provide two
> internal buffer pools for 'viewfinder' and 'stat' video devices, and
> export the 'output' video device buffers to the stream's pool. This
> works around the fact that the ImgU requires buffers to be queued on all
> its outputs, even when they are not in use.
> 
> Share buffers between the CIO2 output and the ImgU input video devices,
> as the output of the former should immediately be provided to the
> latter for further processing.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 154 ++++++++++++++++++++++++---
>  1 file changed, 140 insertions(+), 14 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 57e4bb89eaa7..7eac36f0c561 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -56,6 +56,7 @@ public:
>  		V4L2Device *dev;
>  		unsigned int pad;
>  		std::string name;
> +		BufferPool *pool;
>  	};
>  
>  	ImgUDevice()
> @@ -81,6 +82,10 @@ public:
>  	int configureOutput(const ImgUOutput *output,
>  			    const StreamConfiguration &config);
>  
> +	int importBuffers(BufferPool *pool);
> +	int exportBuffers(ImgUOutput *output, BufferPool *pool);
> +	void freeBuffers();
> +
>  	unsigned int index_;
>  	std::string name_;
>  	MediaDevice *media_;
> @@ -91,11 +96,16 @@ public:
>  	ImgUOutput viewfinder_;
>  	ImgUOutput stat_;
>  	/* \todo Add param video device for 3A tuning */
> +
> +	BufferPool vfPool_;
> +	BufferPool statPool_;
>  };
>  
>  class CIO2Device
>  {
>  public:
> +	static constexpr unsigned int CIO2_BUFFER_COUNT = 4;
> +
>  	CIO2Device()
>  		: output_(nullptr), csi2_(nullptr), sensor_(nullptr)
>  	{
> @@ -112,6 +122,9 @@ public:
>  	int configure(const StreamConfiguration &config,
>  		      V4L2DeviceFormat *format);
>  
> +	BufferPool *exportBuffers();
> +	void freeBuffers();
> +
>  	V4L2Device *output_;
>  	V4L2Subdevice *csi2_;
>  	V4L2Subdevice *sensor_;
> @@ -119,6 +132,8 @@ public:
>  	/* Maximum sizes and the mbus code used to produce them. */
>  	unsigned int mbusCode_;
>  	Size maxSize_;
> +
> +	BufferPool pool_;
>  };
>  
>  class PipelineHandlerIPU3 : public PipelineHandler
> @@ -282,18 +297,41 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
>  
>  int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
>  {
> -	const StreamConfiguration &cfg = stream->configuration();
>  	IPU3CameraData *data = cameraData(camera);
> -	V4L2Device *cio2 = data->cio2_.output_;
> +	CIO2Device *cio2 = &data->cio2_;
> +	ImgUDevice *imgu = data->imgu_;
> +	int ret;
>  
> -	if (!cfg.bufferCount)
> -		return -EINVAL;
> +	/* Share buffers between CIO2 output and ImgU input. */
> +	BufferPool *pool = cio2->exportBuffers();
> +	if (!pool)
> +		return -ENOMEM;
>  
> -	int ret = cio2->exportBuffers(&stream->bufferPool());
> -	if (ret) {
> -		LOG(IPU3, Error) << "Failed to request memory";
> +	ret = imgu->importBuffers(pool);
> +	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.
> +	 */
> +	unsigned int bufferCount = stream->bufferPool().count();
> +
> +	imgu->viewfinder_.pool->createBuffers(bufferCount);
> +	ret = imgu->exportBuffers(&imgu->viewfinder_, imgu->viewfinder_.pool);
> +	if (ret)
> +		return ret;
> +
> +	imgu->stat_.pool->createBuffers(bufferCount);
> +	ret = imgu->exportBuffers(&imgu->stat_, imgu->stat_.pool);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	return 0;
>  }
> @@ -301,13 +339,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
>  int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	V4L2Device *cio2 = data->cio2_.output_;
>  
> -	int ret = cio2->releaseBuffers();
> -	if (ret) {
> -		LOG(IPU3, Error) << "Failed to release memory";
> -		return ret;
> -	}
> +	data->cio2_.freeBuffers();
> +	data->imgu_->freeBuffers();
>  
>  	return 0;
>  }
> @@ -567,6 +601,7 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
>  
>  	viewfinder_.pad = PAD_VF;
>  	viewfinder_.name = "viewfinder";
> +	viewfinder_.pool = &vfPool_;
>  
>  	stat_.dev = V4L2Device::fromEntityName(media, name_ + " 3a stat");
>  	ret = stat_.dev->open();
> @@ -575,6 +610,7 @@ int ImgUDevice::init(MediaDevice *media, unsigned int index)
>  
>  	stat_.pad = PAD_STAT;
>  	stat_.name = "stat";
> +	stat_.pool = &statPool_;
>  
>  	return 0;
>  }
> @@ -679,6 +715,69 @@ int ImgUDevice::configureOutput(const ImgUOutput *output,
>  	return 0;
>  }
>  
> +/**
> + * \brief Import buffers from CIO2 device into the ImgU
> + * \param[in] pool The buffer pool to import
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int ImgUDevice::importBuffers(BufferPool *pool)
> +{
> +	int ret = input_->importBuffers(pool);
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to import ImgU input buffers";
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * \brief Export buffers from \a output to the provided \a pool
> + * \param[in] output The ImgU output device
> + * \param[in] pool The buffer pool where to export buffers
> + *
> + * Export memory buffers reserved in the video device memory associated with
> + * \a output id to the buffer pool provided as argument.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int ImgUDevice::exportBuffers(ImgUOutput *output, BufferPool *pool)
> +{
> +	int ret = output->dev->exportBuffers(pool);
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to export ImgU "
> +				 << output->name << " buffers";
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * \brief Release buffers for all the ImgU video devices
> + */
> +void ImgUDevice::freeBuffers()
> +{
> +	int ret;
> +
> +	ret = output_.dev->releaseBuffers();
> +	if (ret)
> +		LOG(IPU3, Error) << "Failed to release ImgU output buffers";
> +
> +	ret = stat_.dev->releaseBuffers();
> +	if (ret)
> +		LOG(IPU3, Error) << "Failed to release ImgU stat buffers";
> +
> +	ret = viewfinder_.dev->releaseBuffers();
> +	if (ret)
> +		LOG(IPU3, Error) << "Failed to release ImgU viewfinder buffers";
> +
> +	ret = input_->releaseBuffers();
> +	if (ret)
> +		LOG(IPU3, Error) << "Failed to release ImgU input buffers";
> +}
> +
>  /*------------------------------------------------------------------------------
>   * CIO2 Device
>   */
> @@ -845,6 +944,33 @@ int CIO2Device::configure(const StreamConfiguration &config,
>  	return 0;
>  }
>  
> +/**
> + * \brief Allocate CIO2 memory buffers and export them in a BufferPool
> + *
> + * Allocate memory buffers in the CIO2 video device and export them to
> + * a buffer pool that can be imported by another device.
> + *
> + * \return The buffer pool with export buffers on success or nullptr otherwise
> + */
> +BufferPool *CIO2Device::exportBuffers()
> +{
> +	pool_.createBuffers(CIO2_BUFFER_COUNT);
> +
> +	int ret = output_->exportBuffers(&pool_);
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to export CIO2 buffers";
> +		return nullptr;
> +	}
> +
> +	return &pool_;
> +}
> +
> +void CIO2Device::freeBuffers()
> +{
> +	if (output_->releaseBuffers())
> +		LOG(IPU3, Error) << "Failed to release CIO2 buffers";
> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);
>  
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list