[libcamera-devel,10/10] libcamera: ipu3: Allow zero-copy RAW stream capture
diff mbox series

Message ID 20200602013909.3170593-11-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: ipu3: Allow zero-copy RAW stream
Related show

Commit Message

Niklas Söderlund June 2, 2020, 1:39 a.m. UTC 
With the refactored CIO2 interface it's now easy to add zero-copy for
buffers in the RAW stream. Use the internally allocated buffers inside
the CIO2Device if no buffer for the RAW stream is provided by the
application, or use the application provided one if one is.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/ipu3/cio2.cpp | 52 +++++++++++--------
 src/libcamera/pipeline/ipu3/cio2.h   |  6 +--
 src/libcamera/pipeline/ipu3/ipu3.cpp | 76 +++++++++++-----------------
 3 files changed, 62 insertions(+), 72 deletions(-)

Comments

Jacopo Mondi June 2, 2020, 1:28 p.m. UTC | #1 
Hi Niklas,

not my strongest experties area, but I only have two minor remarks on
this one.

On Tue, Jun 02, 2020 at 03:39:09AM +0200, Niklas Söderlund wrote:
> With the refactored CIO2 interface it's now easy to add zero-copy for
> buffers in the RAW stream. Use the internally allocated buffers inside
> the CIO2Device if no buffer for the RAW stream is provided by the
> application, or use the application provided one if one is.

s/if one is./if any./

>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp | 52 +++++++++++--------
>  src/libcamera/pipeline/ipu3/cio2.h   |  6 +--
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 76 +++++++++++-----------------
>  3 files changed, 62 insertions(+), 72 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index 895848d2b3a1fba9..bbf18910c0e7abd4 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -205,31 +205,16 @@ int CIO2Device::exportBuffers(unsigned int count,
>  	return output_->exportBuffers(count, buffers);
>  }
>
> -FrameBuffer *CIO2Device::getBuffer()
> -{
> -	if (availableBuffers_.empty()) {
> -		LOG(IPU3, Error) << "CIO2 buffer underrun";
> -		return nullptr;
> -	}
> -
> -	FrameBuffer *buffer = availableBuffers_.front();
> -
> -	availableBuffers_.pop();
> -
> -	return buffer;
> -}
> -
> -void CIO2Device::putBuffer(FrameBuffer *buffer)
> -{
> -	availableBuffers_.push(buffer);
> -}
> -
>  int CIO2Device::start()
>  {
> -	int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_);
> +	int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_);
>  	if (ret < 0)
>  		return ret;
>
> +	ret = output_->importBuffers(CIO2_BUFFER_COUNT);
> +	if (ret)
> +		LOG(IPU3, Error) << "Failed to import CIO2 buffers";
> +
>  	for (std::unique_ptr<FrameBuffer> &buffer : buffers_)
>  		availableBuffers_.push(buffer.get());
>
> @@ -253,11 +238,36 @@ int CIO2Device::stop()
>  	return ret;
>  }
>
> -int CIO2Device::queueBuffer(FrameBuffer *buffer)
> +int CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
>  {
> +	FrameBuffer *buffer = rawBuffer;
> +
> +	/* If no buffer provided in request, use an internal one. */
> +	if (!buffer) {
> +		if (availableBuffers_.empty()) {
> +			LOG(IPU3, Error) << "CIO2 buffer underrun";
> +			return -EINVAL;
> +		}
> +
> +		buffer = availableBuffers_.front();
> +		availableBuffers_.pop();
> +	}
> +
> +	buffer->setRequest(request);
> +
>  	return output_->queueBuffer(buffer);
>  }
>
> +void CIO2Device::tryReturnBuffer(FrameBuffer *buffer)
> +{
> +	for (const std::unique_ptr<FrameBuffer> &buf : buffers_) {
> +		if (buf.get() == buffer) {
> +			availableBuffers_.push(buffer);
> +			break;
> +		}
> +	}
> +}
> +
>  V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code)
>  {
>  	switch (code) {
> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> index 032b91c082889a63..1a9b700bf4e7e15c 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.h
> +++ b/src/libcamera/pipeline/ipu3/cio2.h
> @@ -47,9 +47,6 @@ public:
>  	int exportBuffers(unsigned int count,
>  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>
> -	FrameBuffer *getBuffer();
> -	void putBuffer(FrameBuffer *buffer);
> -
>  	int start();
>  	int stop();
>
> @@ -57,7 +54,8 @@ public:
>  	Size resolution() const { return sensor_->resolution(); }
>  	ControlList properties() const { return sensor_->properties(); }
>
> -	int queueBuffer(FrameBuffer *buffer);
> +	int queueBuffer(Request *request, FrameBuffer *rawBuffer);
> +	void tryReturnBuffer(FrameBuffer *buffer);
>  	Signal<FrameBuffer *> bufferReady;
>
>  private:
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 636c1c54627b5777..03e954d0f30d39cc 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -121,7 +121,6 @@ public:
>  	}
>
>  	void imguOutputBufferReady(FrameBuffer *buffer);
> -	void imguInputBufferReady(FrameBuffer *buffer);
>  	void cio2BufferReady(FrameBuffer *buffer);
>
>  	CIO2Device cio2_;
> @@ -730,25 +729,31 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	FrameBuffer *buffer;
>  	int error = 0;
>
> -	/* Get a CIO2 buffer, associate it with the request and queue it. */
> -	buffer = data->cio2_.getBuffer();
> -	if (!buffer)
> -		return -EINVAL;
> +	/* Search for a RAW buffer in the request, if any. */
> +	FrameBuffer *rawBuffer = nullptr;
> +	for (auto it : request->buffers()) {
> +		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> +		if (!stream->raw_)
> +			continue;
>
> -	buffer->setRequest(request);
> -	data->cio2_.queueBuffer(buffer);
> +		rawBuffer = it.second;
> +		break;
> +	}
>
> +	/* Queue a buffer on the CIO2, the buffer may come from the request. */
> +	error = data->cio2_.queueBuffer(request, rawBuffer);
> +	if (error)
> +		return error;
> +
> +	/* Queue all buffers from the request aimed for the ImgU. */
>  	for (auto it : request->buffers()) {
>  		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> -		buffer = it.second;
> -
> -		/* Skip raw streams, they are copied from the CIO2 buffer. */
>  		if (stream->raw_)
>  			continue;
>
> +		FrameBuffer *buffer = it.second;
>  		int ret = stream->device_->dev->queueBuffer(buffer);
>  		if (ret < 0)
>  			error = ret;
> @@ -878,8 +883,8 @@ int PipelineHandlerIPU3::registerCameras()
>  		 */
>  		data->cio2_.bufferReady.connect(data.get(),
>  					&IPU3CameraData::cio2BufferReady);
> -		data->imgu_->input_->bufferReady.connect(data.get(),
> -					&IPU3CameraData::imguInputBufferReady);
> +		data->imgu_->input_->bufferReady.connect(&data->cio2_,
> +					&CIO2Device::tryReturnBuffer);
>  		data->imgu_->output_.dev->bufferReady.connect(data.get(),
>  					&IPU3CameraData::imguOutputBufferReady);
>  		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
> @@ -908,22 +913,6 @@ int PipelineHandlerIPU3::registerCameras()
>   * Buffer Ready slots
>   */
>
> -/**
> - * \brief Handle buffers completion at the ImgU input
> - * \param[in] buffer The completed buffer
> - *
> - * Buffers completed from the ImgU input are immediately queued back to the
> - * CIO2 unit to continue frame capture.
> - */
> -void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer)
> -{
> -	/* \todo Handle buffer failures when state is set to BufferError. */
> -	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> -		return;
> -
> -	cio2_.putBuffer(buffer);
> -}
> -
>  /**
>   * \brief Handle buffers completion at the ImgU output
>   * \param[in] buffer The completed buffer
> @@ -956,27 +945,20 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>  		return;
>
>  	Request *request = buffer->request();
> -	FrameBuffer *raw = request->findBuffer(&rawStream_);
>
> -	if (!raw) {
> -		/* No RAW buffers present, just queue to IMGU. */
> -		imgu_->input_->queueBuffer(buffer);
> -		return;
> -	}
> -
> -	/* RAW buffers present, special care is needed. */
> -	if (request->buffers().size() > 1)
> -		imgu_->input_->queueBuffer(buffer);
> -
> -	if (raw->copyFrom(buffer))
> -		LOG(IPU3, Debug) << "Copy of FrameBuffer failed";
> -
> -	pipe_->completeBuffer(camera_, request, raw);
> -
> -	if (request->buffers().size() == 1) {
> -		cio2_.putBuffer(buffer);
> +	/*
> +	 * If the requests contains a buffer for the RAW stream and the buffer
> +	 * completed by the CIO2 satisfy all pending buffers in the request

s/satisfy/satisfies

The rest looks good

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> +	 * complete the request now, there is no need for this request to be
> +	 * processed by the ImGU.
> +	 */
> +	if (request->findBuffer(&rawStream_) &&
> +	    pipe_->completeBuffer(camera_, request, buffer)) {
>  		pipe_->completeRequest(camera_, request);
> +		return;
>  	}
> +
> +	imgu_->input_->queueBuffer(buffer);
>  }
>
>  /* -----------------------------------------------------------------------------
> --
> 2.26.2
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart June 5, 2020, 10:56 p.m. UTC | #2 
Hi Niklas,

Thank you for the patch.

On Tue, Jun 02, 2020 at 03:28:20PM +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> not my strongest experties area, but I only have two minor remarks on
> this one.
> 
> On Tue, Jun 02, 2020 at 03:39:09AM +0200, Niklas Söderlund wrote:
> > With the refactored CIO2 interface it's now easy to add zero-copy for
> > buffers in the RAW stream. Use the internally allocated buffers inside
> > the CIO2Device if no buffer for the RAW stream is provided by the
> > application, or use the application provided one if one is.
> 
> s/if one is./if any./

"or use the application-provided buffer if any."

> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/libcamera/pipeline/ipu3/cio2.cpp | 52 +++++++++++--------
> >  src/libcamera/pipeline/ipu3/cio2.h   |  6 +--
> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 76 +++++++++++-----------------
> >  3 files changed, 62 insertions(+), 72 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> > index 895848d2b3a1fba9..bbf18910c0e7abd4 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> > +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> > @@ -205,31 +205,16 @@ int CIO2Device::exportBuffers(unsigned int count,
> >  	return output_->exportBuffers(count, buffers);
> >  }
> >
> > -FrameBuffer *CIO2Device::getBuffer()
> > -{
> > -	if (availableBuffers_.empty()) {
> > -		LOG(IPU3, Error) << "CIO2 buffer underrun";
> > -		return nullptr;
> > -	}
> > -
> > -	FrameBuffer *buffer = availableBuffers_.front();
> > -
> > -	availableBuffers_.pop();
> > -
> > -	return buffer;
> > -}
> > -
> > -void CIO2Device::putBuffer(FrameBuffer *buffer)
> > -{
> > -	availableBuffers_.push(buffer);
> > -}
> > -
> >  int CIO2Device::start()
> >  {
> > -	int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_);
> > +	int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_);
> >  	if (ret < 0)
> >  		return ret;
> >
> > +	ret = output_->importBuffers(CIO2_BUFFER_COUNT);
> > +	if (ret)
> > +		LOG(IPU3, Error) << "Failed to import CIO2 buffers";
> > +
> >  	for (std::unique_ptr<FrameBuffer> &buffer : buffers_)
> >  		availableBuffers_.push(buffer.get());
> >
> > @@ -253,11 +238,36 @@ int CIO2Device::stop()
> >  	return ret;
> >  }
> >
> > -int CIO2Device::queueBuffer(FrameBuffer *buffer)
> > +int CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
> >  {
> > +	FrameBuffer *buffer = rawBuffer;
> > +
> > +	/* If no buffer provided in request, use an internal one. */

s/provided/is provided/
s/request/the request/

> > +	if (!buffer) {
> > +		if (availableBuffers_.empty()) {
> > +			LOG(IPU3, Error) << "CIO2 buffer underrun";
> > +			return -EINVAL;
> > +		}
> > +
> > +		buffer = availableBuffers_.front();
> > +		availableBuffers_.pop();
> > +	}
> > +
> > +	buffer->setRequest(request);
> > +
> >  	return output_->queueBuffer(buffer);
> >  }
> >
> > +void CIO2Device::tryReturnBuffer(FrameBuffer *buffer)
> > +{
> > +	for (const std::unique_ptr<FrameBuffer> &buf : buffers_) {
> > +		if (buf.get() == buffer) {
> > +			availableBuffers_.push(buffer);
> > +			break;
> > +		}
> > +	}

I believe this will be a common need. Should the FrameBuffer class be
extended with an internal (or external) bool flag ? Or a void *owner (a
bit of a hack) ? Or something similar but better ? Bonus points if the
API to do so can be hidden from applications.

> > +}
> > +
> >  V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code)
> >  {
> >  	switch (code) {
> > diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
> > index 032b91c082889a63..1a9b700bf4e7e15c 100644
> > --- a/src/libcamera/pipeline/ipu3/cio2.h
> > +++ b/src/libcamera/pipeline/ipu3/cio2.h
> > @@ -47,9 +47,6 @@ public:
> >  	int exportBuffers(unsigned int count,
> >  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> >
> > -	FrameBuffer *getBuffer();
> > -	void putBuffer(FrameBuffer *buffer);
> > -
> >  	int start();
> >  	int stop();
> >
> > @@ -57,7 +54,8 @@ public:
> >  	Size resolution() const { return sensor_->resolution(); }
> >  	ControlList properties() const { return sensor_->properties(); }
> >
> > -	int queueBuffer(FrameBuffer *buffer);
> > +	int queueBuffer(Request *request, FrameBuffer *rawBuffer);
> > +	void tryReturnBuffer(FrameBuffer *buffer);
> >  	Signal<FrameBuffer *> bufferReady;
> >
> >  private:
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 636c1c54627b5777..03e954d0f30d39cc 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -121,7 +121,6 @@ public:
> >  	}
> >
> >  	void imguOutputBufferReady(FrameBuffer *buffer);
> > -	void imguInputBufferReady(FrameBuffer *buffer);
> >  	void cio2BufferReady(FrameBuffer *buffer);
> >
> >  	CIO2Device cio2_;
> > @@ -730,25 +729,31 @@ void PipelineHandlerIPU3::stop(Camera *camera)
> >  int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
> >  {
> >  	IPU3CameraData *data = cameraData(camera);
> > -	FrameBuffer *buffer;
> >  	int error = 0;
> >
> > -	/* Get a CIO2 buffer, associate it with the request and queue it. */
> > -	buffer = data->cio2_.getBuffer();
> > -	if (!buffer)
> > -		return -EINVAL;
> > +	/* Search for a RAW buffer in the request, if any. */
> > +	FrameBuffer *rawBuffer = nullptr;
> > +	for (auto it : request->buffers()) {
> > +		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> > +		if (!stream->raw_)
> > +			continue;
> >
> > -	buffer->setRequest(request);
> > -	data->cio2_.queueBuffer(buffer);
> > +		rawBuffer = it.second;
> > +		break;
> > +	}

Can't you use Request::findBuffer() ?

> >
> > +	/* Queue a buffer on the CIO2, the buffer may come from the request. */
> > +	error = data->cio2_.queueBuffer(request, rawBuffer);

How about

	/*
	 * Queue a buffer on the CIO2, using the raw stream buffer provided in
	 * the request, if any, or a CIO2 internal buffer otherwise.
	 */
	FrameBuffer *rawBuffer = request->findBuffer(&data->rawStream_);
	error = data->cio2_.queueBuffer(request, rawBuffer);

> > +	if (error)
> > +		return error;
> > +
> > +	/* Queue all buffers from the request aimed for the ImgU. */
> >  	for (auto it : request->buffers()) {
> >  		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> > -		buffer = it.second;
> > -
> > -		/* Skip raw streams, they are copied from the CIO2 buffer. */
> >  		if (stream->raw_)
> >  			continue;
> >
> > +		FrameBuffer *buffer = it.second;
> >  		int ret = stream->device_->dev->queueBuffer(buffer);
> >  		if (ret < 0)
> >  			error = ret;
> > @@ -878,8 +883,8 @@ int PipelineHandlerIPU3::registerCameras()
> >  		 */
> >  		data->cio2_.bufferReady.connect(data.get(),
> >  					&IPU3CameraData::cio2BufferReady);
> > -		data->imgu_->input_->bufferReady.connect(data.get(),
> > -					&IPU3CameraData::imguInputBufferReady);
> > +		data->imgu_->input_->bufferReady.connect(&data->cio2_,
> > +					&CIO2Device::tryReturnBuffer);
> >  		data->imgu_->output_.dev->bufferReady.connect(data.get(),
> >  					&IPU3CameraData::imguOutputBufferReady);
> >  		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
> > @@ -908,22 +913,6 @@ int PipelineHandlerIPU3::registerCameras()
> >   * Buffer Ready slots
> >   */
> >
> > -/**
> > - * \brief Handle buffers completion at the ImgU input
> > - * \param[in] buffer The completed buffer
> > - *
> > - * Buffers completed from the ImgU input are immediately queued back to the
> > - * CIO2 unit to continue frame capture.
> > - */
> > -void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer)
> > -{
> > -	/* \todo Handle buffer failures when state is set to BufferError. */
> > -	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> > -		return;

We're losing this, but I think it's actually not needed.

> > -
> > -	cio2_.putBuffer(buffer);
> > -}
> > -
> >  /**
> >   * \brief Handle buffers completion at the ImgU output
> >   * \param[in] buffer The completed buffer
> > @@ -956,27 +945,20 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> >  		return;
> >
> >  	Request *request = buffer->request();
> > -	FrameBuffer *raw = request->findBuffer(&rawStream_);
> >
> > -	if (!raw) {
> > -		/* No RAW buffers present, just queue to IMGU. */
> > -		imgu_->input_->queueBuffer(buffer);
> > -		return;
> > -	}
> > -
> > -	/* RAW buffers present, special care is needed. */
> > -	if (request->buffers().size() > 1)
> > -		imgu_->input_->queueBuffer(buffer);
> > -
> > -	if (raw->copyFrom(buffer))
> > -		LOG(IPU3, Debug) << "Copy of FrameBuffer failed";
> > -
> > -	pipe_->completeBuffer(camera_, request, raw);
> > -
> > -	if (request->buffers().size() == 1) {
> > -		cio2_.putBuffer(buffer);
> > +	/*
> > +	 * If the requests contains a buffer for the RAW stream and the buffer
> > +	 * completed by the CIO2 satisfy all pending buffers in the request
> 
> s/satisfy/satisfies
> 
> The rest looks good
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> > +	 * complete the request now, there is no need for this request to be
> > +	 * processed by the ImGU.
> > +	 */
> > +	if (request->findBuffer(&rawStream_) &&
> > +	    pipe_->completeBuffer(camera_, request, buffer)) {
> >  		pipe_->completeRequest(camera_, request);
> > +		return;
> >  	}

I find the comment a bit hard to parse. How about the following ?

	/*
	 * If the request contains a buffer for the RAW stream only, complete it
	 * now as there's no need for ImgU processing.
	 */

> > +
> > +	imgu_->input_->queueBuffer(buffer);
> >  }
> >
> >  /* -----------------------------------------------------------------------------

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 895848d2b3a1fba9..bbf18910c0e7abd4 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -205,31 +205,16 @@  int CIO2Device::exportBuffers(unsigned int count,
 	return output_->exportBuffers(count, buffers);
 }
 
-FrameBuffer *CIO2Device::getBuffer()
-{
-	if (availableBuffers_.empty()) {
-		LOG(IPU3, Error) << "CIO2 buffer underrun";
-		return nullptr;
-	}
-
-	FrameBuffer *buffer = availableBuffers_.front();
-
-	availableBuffers_.pop();
-
-	return buffer;
-}
-
-void CIO2Device::putBuffer(FrameBuffer *buffer)
-{
-	availableBuffers_.push(buffer);
-}
-
 int CIO2Device::start()
 {
-	int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_);
+	int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_);
 	if (ret < 0)
 		return ret;
 
+	ret = output_->importBuffers(CIO2_BUFFER_COUNT);
+	if (ret)
+		LOG(IPU3, Error) << "Failed to import CIO2 buffers";
+
 	for (std::unique_ptr<FrameBuffer> &buffer : buffers_)
 		availableBuffers_.push(buffer.get());
 
@@ -253,11 +238,36 @@  int CIO2Device::stop()
 	return ret;
 }
 
-int CIO2Device::queueBuffer(FrameBuffer *buffer)
+int CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
 {
+	FrameBuffer *buffer = rawBuffer;
+
+	/* If no buffer provided in request, use an internal one. */
+	if (!buffer) {
+		if (availableBuffers_.empty()) {
+			LOG(IPU3, Error) << "CIO2 buffer underrun";
+			return -EINVAL;
+		}
+
+		buffer = availableBuffers_.front();
+		availableBuffers_.pop();
+	}
+
+	buffer->setRequest(request);
+
 	return output_->queueBuffer(buffer);
 }
 
+void CIO2Device::tryReturnBuffer(FrameBuffer *buffer)
+{
+	for (const std::unique_ptr<FrameBuffer> &buf : buffers_) {
+		if (buf.get() == buffer) {
+			availableBuffers_.push(buffer);
+			break;
+		}
+	}
+}
+
 V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code)
 {
 	switch (code) {
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index 032b91c082889a63..1a9b700bf4e7e15c 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -47,9 +47,6 @@  public:
 	int exportBuffers(unsigned int count,
 			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
 
-	FrameBuffer *getBuffer();
-	void putBuffer(FrameBuffer *buffer);
-
 	int start();
 	int stop();
 
@@ -57,7 +54,8 @@  public:
 	Size resolution() const { return sensor_->resolution(); }
 	ControlList properties() const { return sensor_->properties(); }
 
-	int queueBuffer(FrameBuffer *buffer);
+	int queueBuffer(Request *request, FrameBuffer *rawBuffer);
+	void tryReturnBuffer(FrameBuffer *buffer);
 	Signal<FrameBuffer *> bufferReady;
 
 private:
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 636c1c54627b5777..03e954d0f30d39cc 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -121,7 +121,6 @@  public:
 	}
 
 	void imguOutputBufferReady(FrameBuffer *buffer);
-	void imguInputBufferReady(FrameBuffer *buffer);
 	void cio2BufferReady(FrameBuffer *buffer);
 
 	CIO2Device cio2_;
@@ -730,25 +729,31 @@  void PipelineHandlerIPU3::stop(Camera *camera)
 int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
 {
 	IPU3CameraData *data = cameraData(camera);
-	FrameBuffer *buffer;
 	int error = 0;
 
-	/* Get a CIO2 buffer, associate it with the request and queue it. */
-	buffer = data->cio2_.getBuffer();
-	if (!buffer)
-		return -EINVAL;
+	/* Search for a RAW buffer in the request, if any. */
+	FrameBuffer *rawBuffer = nullptr;
+	for (auto it : request->buffers()) {
+		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
+		if (!stream->raw_)
+			continue;
 
-	buffer->setRequest(request);
-	data->cio2_.queueBuffer(buffer);
+		rawBuffer = it.second;
+		break;
+	}
 
+	/* Queue a buffer on the CIO2, the buffer may come from the request. */
+	error = data->cio2_.queueBuffer(request, rawBuffer);
+	if (error)
+		return error;
+
+	/* Queue all buffers from the request aimed for the ImgU. */
 	for (auto it : request->buffers()) {
 		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
-		buffer = it.second;
-
-		/* Skip raw streams, they are copied from the CIO2 buffer. */
 		if (stream->raw_)
 			continue;
 
+		FrameBuffer *buffer = it.second;
 		int ret = stream->device_->dev->queueBuffer(buffer);
 		if (ret < 0)
 			error = ret;
@@ -878,8 +883,8 @@  int PipelineHandlerIPU3::registerCameras()
 		 */
 		data->cio2_.bufferReady.connect(data.get(),
 					&IPU3CameraData::cio2BufferReady);
-		data->imgu_->input_->bufferReady.connect(data.get(),
-					&IPU3CameraData::imguInputBufferReady);
+		data->imgu_->input_->bufferReady.connect(&data->cio2_,
+					&CIO2Device::tryReturnBuffer);
 		data->imgu_->output_.dev->bufferReady.connect(data.get(),
 					&IPU3CameraData::imguOutputBufferReady);
 		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
@@ -908,22 +913,6 @@  int PipelineHandlerIPU3::registerCameras()
  * Buffer Ready slots
  */
 
-/**
- * \brief Handle buffers completion at the ImgU input
- * \param[in] buffer The completed buffer
- *
- * Buffers completed from the ImgU input are immediately queued back to the
- * CIO2 unit to continue frame capture.
- */
-void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer)
-{
-	/* \todo Handle buffer failures when state is set to BufferError. */
-	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
-		return;
-
-	cio2_.putBuffer(buffer);
-}
-
 /**
  * \brief Handle buffers completion at the ImgU output
  * \param[in] buffer The completed buffer
@@ -956,27 +945,20 @@  void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
 		return;
 
 	Request *request = buffer->request();
-	FrameBuffer *raw = request->findBuffer(&rawStream_);
 
-	if (!raw) {
-		/* No RAW buffers present, just queue to IMGU. */
-		imgu_->input_->queueBuffer(buffer);
-		return;
-	}
-
-	/* RAW buffers present, special care is needed. */
-	if (request->buffers().size() > 1)
-		imgu_->input_->queueBuffer(buffer);
-
-	if (raw->copyFrom(buffer))
-		LOG(IPU3, Debug) << "Copy of FrameBuffer failed";
-
-	pipe_->completeBuffer(camera_, request, raw);
-
-	if (request->buffers().size() == 1) {
-		cio2_.putBuffer(buffer);
+	/*
+	 * If the requests contains a buffer for the RAW stream and the buffer
+	 * completed by the CIO2 satisfy all pending buffers in the request
+	 * complete the request now, there is no need for this request to be
+	 * processed by the ImGU.
+	 */
+	if (request->findBuffer(&rawStream_) &&
+	    pipe_->completeBuffer(camera_, request, buffer)) {
 		pipe_->completeRequest(camera_, request);
+		return;
 	}
+
+	imgu_->input_->queueBuffer(buffer);
 }
 
 /* -----------------------------------------------------------------------------