[v1,4/6] pipeline: rkisp1: Properly handle the bufferCount set in the stream configuration

23690 diff mbox series

The bufferCount is reset to a hardcoded value of 4 in RkISP1Path::validate().
Keep the default value of 4 but do not reset it, if it was changed.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

Changes in v1:
- Removed todo comment that was solved by this change
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 5 +++--
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 ++-----
 src/libcamera/pipeline/rkisp1/rkisp1_path.h   | 4 +---
 3 files changed, 6 insertions(+), 10 deletions(-)

On Mon, Jun 30, 2025 at 10:11:19AM +0200, Stefan Klug wrote:
> The bufferCount is reset to a hardcoded value of 4 in RkISP1Path::validate().
> Keep the default value of 4 but do not reset it, if it was changed.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Changes in v1:
> - Removed todo comment that was solved by this change
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 5 +++--
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 ++-----
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   | 4 +---
>  3 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index bd14ab237064..9d7b05490af6 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -790,6 +790,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
>  			return nullptr;
>  
>  		cfg.colorSpace = colorSpace;
> +		cfg.bufferCount = kPipelineDepth;
>  		config->addConfiguration(cfg);
>  	}
>  
> @@ -1123,14 +1124,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL
>  	}
>  
>  	if (data->mainPath_->isEnabled()) {
> -		ret = mainPath_.start();
> +		ret = mainPath_.start(maxQueuedRequestsDevice_);
>  		if (ret)
>  			return ret;
>  		actions += [&]() { mainPath_.stop(); };
>  	}
>  
>  	if (hasSelfPath_ && data->selfPath_->isEnabled()) {
> -		ret = selfPath_.start();
> +		ret = selfPath_.start(maxQueuedRequestsDevice_);
>  		if (ret)
>  			return ret;
>  	}
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 64018dc5b2f4..8ea5500d4080 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -249,7 +249,6 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,
>  	StreamConfiguration cfg(formats);
>  	cfg.pixelFormat = format;
>  	cfg.size = streamSize;
> -	cfg.bufferCount = RKISP1_BUFFER_COUNT;
>  
>  	return cfg;
>  }
> @@ -383,7 +382,6 @@ RkISP1Path::validate(const CameraSensor *sensor,
>  
>  	cfg->size.boundTo(maxResolution);
>  	cfg->size.expandTo(minResolution);
> -	cfg->bufferCount = RKISP1_BUFFER_COUNT;
>  
>  	V4L2DeviceFormat format;
>  	format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat);
> @@ -480,15 +478,14 @@ int RkISP1Path::configure(const StreamConfiguration &config,
>  	return 0;
>  }
>  
> -int RkISP1Path::start()
> +int RkISP1Path::start(unsigned int bufferCount)
>  {
>  	int ret;
>  
>  	if (running_)
>  		return -EBUSY;
>  
> -	/* \todo Make buffer count user configurable. */
> -	ret = video_->importBuffers(RKISP1_BUFFER_COUNT);
> +	ret = video_->importBuffers(bufferCount);

Question: how passing bufferCount=maxQueuedRequestsDevice_ makes
this user configurable? 

>  	if (ret)
>  		return ret;
>  
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index 430181d371a7..0b60c499ac64 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -58,7 +58,7 @@ public:
>  		return video_->exportBuffers(bufferCount, buffers);
>  	}
>  
> -	int start();
> +	int start(unsigned int bufferCount);
>  	void stop();
>  
>  	int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); }
> @@ -69,8 +69,6 @@ private:
>  	void populateFormats();
>  	Size filterSensorResolution(const CameraSensor *sensor);
>  
> -	static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;
> -
>  	const char *name_;
>  	bool running_;
>  
> -- 
> 2.48.1
>