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

[Laurent Pinchart]

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 at chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart at 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;
>  }

-- 
Regards,

Laurent Pinchart