[PATCH 1/5] libcamera: rkisp1: Make tryCompleteRequest() params agnostic

[Jacopo Mondi]

Hi Dan

On Mon, Feb 12, 2024 at 03:35:28PM +0000, Daniel Scally wrote:
> tryCompleteRequest() doesn't actually need to know the status of the
> parameters buffer to decide whether it's ok to complete the Request
> or not - remove the check. Since setting the paramsDequeued flag is
> all that the paramsReady slot actually does, that can be removed too.

When PipelineHandlerRkISP1::tryCompleteRequest() is called
succesfully, it destroies the RkISP1Frames associated with the
completed request.

int RkISP1Frames::destroy(unsigned int frame)
{
	RkISP1FrameInfo *info = find(frame);
	if (!info)
		return -ENOENT;

	pipe_->availableParamBuffers_.push(info->paramBuffer);
	pipe_->availableStatBuffers_.push(info->statBuffer);

	frameInfo_.erase(info->frame);

	delete info;

	return 0;
}


This
	pipe_->availableParamBuffers_.push(info->paramBuffer);

returns the parameter buffer to the list of available ones, but to do
so, shouldn't we make sure it has been de-queued from the video output
device first ?

>
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 20 --------------------
>  1 file changed, 20 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 586b46d6..5460dc11 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -60,7 +60,6 @@ struct RkISP1FrameInfo {
>  	FrameBuffer *mainPathBuffer;
>  	FrameBuffer *selfPathBuffer;
>
> -	bool paramDequeued;
>  	bool metadataProcessed;
>  };
>
> @@ -177,7 +176,6 @@ private:
>  	int createCamera(MediaEntity *sensor);
>  	void tryCompleteRequest(RkISP1FrameInfo *info);
>  	void bufferReady(FrameBuffer *buffer);
> -	void paramReady(FrameBuffer *buffer);
>  	void statReady(FrameBuffer *buffer);
>  	void frameStart(uint32_t sequence);
>
> @@ -248,7 +246,6 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>  	info->mainPathBuffer = mainPathBuffer;
>  	info->selfPathBuffer = selfPathBuffer;
>  	info->statBuffer = statBuffer;
> -	info->paramDequeued = false;
>  	info->metadataProcessed = false;
>
>  	frameInfo_[frame] = info;
> @@ -1213,7 +1210,6 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (hasSelfPath_)
>  		selfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
>  	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
> -	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
>
>  	/*
>  	 * Enumerate all sensors connected to the ISP and create one
> @@ -1243,9 +1239,6 @@ void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)
>  	if (!info->metadataProcessed)
>  		return;
>
> -	if (!isRaw_ && !info->paramDequeued)
> -		return;
> -
>  	data->frameInfo_.destroy(info->frame);
>
>  	completeRequest(request);
> @@ -1287,19 +1280,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  	tryCompleteRequest(info);
>  }
>
> -void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
> -{
> -	ASSERT(activeCamera_);
> -	RkISP1CameraData *data = cameraData(activeCamera_);
> -
> -	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
> -	if (!info)
> -		return;
> -
> -	info->paramDequeued = true;
> -	tryCompleteRequest(info);
> -}
> -
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  {
>  	ASSERT(activeCamera_);
> --
> 2.34.1
>