[libcamera-devel,v5,1/2] pipeline: ipu3: Store requests in the case a buffer shortage

12257 diff mbox series

PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a
request when there are not sufficient buffers for the request.
Since the request will be successful if it is queued later when
enough buffers are available. The requests failed due to a buffer
shortage should be stored and retried later in the FIFO order.
This introduces the queue in IPU3CameraData to do that.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/cio2.cpp   |  2 +-
 src/libcamera/pipeline/ipu3/frames.cpp |  4 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp   | 80 +++++++++++++++++++-------
 3 files changed, 63 insertions(+), 23 deletions(-)

Hi Hiro,

Thank you for the patch.

On Thu, May 13, 2021 at 11:29:45AM +0900, Hirokazu Honda wrote:
> PipelineHandlerIPU3 returns -ENOBUFS and -ENOMEM on queueing a
> request when there are not sufficient buffers for the request.
> Since the request will be successful if it is queued later when
> enough buffers are available. The requests failed due to a buffer
> shortage should be stored and retried later in the FIFO order.
> This introduces the queue in IPU3CameraData to do that.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  src/libcamera/pipeline/ipu3/cio2.cpp   |  2 +-
>  src/libcamera/pipeline/ipu3/frames.cpp |  4 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp   | 80 +++++++++++++++++++-------
>  3 files changed, 63 insertions(+), 23 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
> index 3cd777d1..8bbef174 100644
> --- a/src/libcamera/pipeline/ipu3/cio2.cpp
> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp
> @@ -272,7 +272,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
>  	/* If no buffer is provided in the request, use an internal one. */
>  	if (!buffer) {
>  		if (availableBuffers_.empty()) {
> -			LOG(IPU3, Error) << "CIO2 buffer underrun";
> +			LOG(IPU3, Debug) << "CIO2 buffer underrun";
>  			return nullptr;
>  		}
>  
> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> index d0f55ab9..29d9aafc 100644
> --- a/src/libcamera/pipeline/ipu3/frames.cpp
> +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> @@ -44,12 +44,12 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
>  	unsigned int id = request->sequence();
>  
>  	if (availableParamBuffers_.empty()) {
> -		LOG(IPU3, Error) << "Parameters buffer underrun";
> +		LOG(IPU3, Debug) << "Parameters buffer underrun";
>  		return nullptr;
>  	}
>  
>  	if (availableStatBuffers_.empty()) {
> -		LOG(IPU3, Error) << "Statistics buffer underrun";
> +		LOG(IPU3, Debug) << "Statistics buffer underrun";
>  		return nullptr;
>  	}
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ade8ffbd..6961d498 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -66,6 +66,8 @@ public:
>  	void cio2BufferReady(FrameBuffer *buffer);
>  	void paramBufferReady(FrameBuffer *buffer);
>  	void statBufferReady(FrameBuffer *buffer);
> +	void queuePendingRequests();
> +	void cancelPendingRequests();
>  
>  	CIO2Device cio2_;
>  	ImgUDevice *imgu_;
> @@ -84,6 +86,8 @@ public:
>  
>  	std::unique_ptr<ipa::ipu3::IPAProxyIPU3> ipa_;
>  
> +	std::queue<Request *> pendingRequests_;
> +
>  private:
>  	void queueFrameAction(unsigned int id,
>  			      const ipa::ipu3::IPU3Action &action);
> @@ -764,6 +768,8 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  	IPU3CameraData *data = cameraData(camera);
>  	int ret = 0;
>  
> +	data->cancelPendingRequests();
> +
>  	data->ipa_->stop();
>  
>  	ret |= data->imgu_->stop();
> @@ -774,32 +780,66 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  	freeBuffers(camera);
>  }
>  
> -int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
> +void IPU3CameraData::cancelPendingRequests()
>  {
> -	IPU3CameraData *data = cameraData(camera);
> +	while (!pendingRequests_.empty()) {
> +		Request *request = pendingRequests_.front();
>  
> -	IPU3Frames::Info *info = data->frameInfos_.create(request);
> -	if (!info)
> -		return -ENOBUFS;
> +		for (auto it : request->buffers()) {
> +			FrameBuffer *buffer = it.second;
> +			buffer->cancel();
> +			pipe_->completeBuffer(request, buffer);
> +		}
>  
> -	/*
> -	 * Queue a buffer on the CIO2, using the raw stream buffer provided in
> -	 * the request, if any, or a CIO2 internal buffer otherwise.
> -	 */
> -	FrameBuffer *reqRawBuffer = request->findBuffer(&data->rawStream_);
> -	FrameBuffer *rawBuffer = data->cio2_.queueBuffer(request, reqRawBuffer);
> -	if (!rawBuffer) {
> -		data->frameInfos_.remove(info);
> -		return -ENOMEM;
> +		pipe_->completeRequest(request);
> +		pendingRequests_.pop();
>  	}
> +}
>  
> -	info->rawBuffer = rawBuffer;
> +void IPU3CameraData::queuePendingRequests()
> +{
> +	while (!pendingRequests_.empty()) {
> +		Request *request = pendingRequests_.front();
>  
> -	ipa::ipu3::IPU3Event ev;
> -	ev.op = ipa::ipu3::EventProcessControls;
> -	ev.frame = info->id;
> -	ev.controls = request->controls();
> -	data->ipa_->processEvent(ev);
> +		IPU3Frames::Info *info = frameInfos_.create(request);
> +		if (!info)
> +			break;
> +
> +		/*
> +		 * Queue a buffer on the CIO2, using the raw stream buffer
> +		 * provided in the request, if any, or a CIO2 internal buffer
> +		 * otherwise.
> +		 */
> +		FrameBuffer *reqRawBuffer = request->findBuffer(&rawStream_);
> +		FrameBuffer *rawBuffer = cio2_.queueBuffer(request, reqRawBuffer);
> +		/*
> +		 * \todo If queueBuffer fails in queuing a buffer to the device,
> +		 * report the request as error by cancelling the request and
> +		 * calling PipelineHandler::completeRequest().
> +		 */
> +		if (!rawBuffer) {
> +			frameInfos_.remove(info);
> +			break;
> +		}
> +
> +		info->rawBuffer = rawBuffer;
> +
> +		ipa::ipu3::IPU3Event ev;
> +		ev.op = ipa::ipu3::EventProcessControls;
> +		ev.frame = info->id;
> +		ev.controls = request->controls();
> +		ipa_->processEvent(ev);
> +
> +		pendingRequests_.pop();
> +	}
> +}
> +
> +int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
> +{
> +	IPU3CameraData *data = cameraData(camera);
> +
> +	data->pendingRequests_.push(request);
> +	data->queuePendingRequests();
>  
>  	return 0;
>  }