[libcamera-devel] [PATCH 05/11] pipeline: ipu3: Add needsImgu flag to IPU3Frames

[Laurent Pinchart]

Hi Dan,

Thank you for the patch.

On Sat, Mar 18, 2023 at 11:40:08PM +0000, Daniel Scally via libcamera-devel wrote:
> Some sensors using the IPU3 pipeline do not transfer data to the
> Imgu, so we need the ability to disable the related processing
> from this class. Add a boolean flag denoting whether or not to
> run the Imgu related processing. For now, hard code this to true
> in IPU3CameraData::queuePendingRequests().
> 
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
>  src/libcamera/pipeline/ipu3/frames.cpp | 45 +++++++++++++++-----------
>  src/libcamera/pipeline/ipu3/frames.h   |  3 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp   |  2 +-
>  3 files changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
> index a4c3477c..55b90e9d 100644
> --- a/src/libcamera/pipeline/ipu3/frames.cpp
> +++ b/src/libcamera/pipeline/ipu3/frames.cpp
> @@ -40,28 +40,32 @@ void IPU3Frames::clear()
>  	availableStatBuffers_ = {};
>  }
>  
> -IPU3Frames::Info *IPU3Frames::create(Request *request)
> +IPU3Frames::Info *IPU3Frames::create(Request *request, bool needsImgu)

Why do we need this flag per frame ? I expect the ImgU to either be used
for all frames, or for none.

>  {
>  	unsigned int id = request->sequence();
> +	FrameBuffer *paramBuffer = nullptr;
> +	FrameBuffer *statBuffer = nullptr;
>  
> -	if (availableParamBuffers_.empty()) {
> -		LOG(IPU3, Debug) << "Parameters buffer underrun";
> -		return nullptr;
> -	}
> +	if (needsImgu) {
> +		if (availableParamBuffers_.empty()) {
> +			LOG(IPU3, Debug) << "Parameters buffer underrun";
> +			return nullptr;
> +		}
>  
> -	if (availableStatBuffers_.empty()) {
> -		LOG(IPU3, Debug) << "Statistics buffer underrun";
> -		return nullptr;
> -	}
> +		if (availableStatBuffers_.empty()) {
> +			LOG(IPU3, Debug) << "Statistics buffer underrun";
> +			return nullptr;
> +		}
>  
> -	FrameBuffer *paramBuffer = availableParamBuffers_.front();
> -	FrameBuffer *statBuffer = availableStatBuffers_.front();
> +		paramBuffer = availableParamBuffers_.front();
> +		statBuffer = availableStatBuffers_.front();
>  
> -	paramBuffer->_d()->setRequest(request);
> -	statBuffer->_d()->setRequest(request);
> +		paramBuffer->_d()->setRequest(request);
> +		statBuffer->_d()->setRequest(request);
>  
> -	availableParamBuffers_.pop();
> -	availableStatBuffers_.pop();
> +		availableParamBuffers_.pop();
> +		availableStatBuffers_.pop();
> +	}
>  
>  	/* \todo Remove the dynamic allocation of Info */
>  	std::unique_ptr<Info> info = std::make_unique<Info>();
> @@ -73,6 +77,7 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
>  	info->statBuffer = statBuffer;
>  	info->paramDequeued = false;
>  	info->metadataProcessed = false;
> +	info->needsImgu = needsImgu;
>  
>  	frameInfo_[id] = std::move(info);
>  
> @@ -96,11 +101,13 @@ bool IPU3Frames::tryComplete(IPU3Frames::Info *info)
>  	if (request->hasPendingBuffers())
>  		return false;
>  
> -	if (!info->metadataProcessed)
> -		return false;
> +	if (info->needsImgu) {
> +		if (!info->metadataProcessed)
> +			return false;
>  
> -	if (!info->paramDequeued)
> -		return false;
> +		if (!info->paramDequeued)
> +			return false;
> +	}
>  
>  	remove(info);
>  
> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h
> index 6e3cb915..d58fd998 100644
> --- a/src/libcamera/pipeline/ipu3/frames.h
> +++ b/src/libcamera/pipeline/ipu3/frames.h
> @@ -40,6 +40,7 @@ public:
>  
>  		bool paramDequeued;
>  		bool metadataProcessed;
> +		bool needsImgu;
>  	};
>  
>  	IPU3Frames();
> @@ -48,7 +49,7 @@ public:
>  		  const std::vector<std::unique_ptr<FrameBuffer>> &statBuffers);
>  	void clear();
>  
> -	Info *create(Request *request);
> +	Info *create(Request *request, bool needsImgu);
>  	void remove(Info *info);
>  	bool tryComplete(Info *info);
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index d0d55651..3077c6bb 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -806,7 +806,7 @@ void IPU3CameraData::queuePendingRequests()
>  	while (!pendingRequests_.empty()) {
>  		Request *request = pendingRequests_.front();
>  
> -		IPU3Frames::Info *info = frameInfos_.create(request);
> +		IPU3Frames::Info *info = frameInfos_.create(request, true);
>  		if (!info)
>  			break;
>  

-- 
Regards,

Laurent Pinchart