[libcamera-devel] [PATCH v9 07/18] libcamera: pipeline: rkisp1: Don't rely on bufferCount
Jacopo Mondi
jacopo at jmondi.org
Fri Dec 16 16:25:36 CET 2022
Hi Paul
On Fri, Dec 16, 2022 at 09:29:28PM +0900, Paul Elder via libcamera-devel wrote:
> From: Nícolas F. R. A. Prado <nfraprado at collabora.com>
>
> Currently the rkisp1 pipeline handler relies on bufferCount to decide on
> the number of parameter and statistics buffers to allocate internally
> and for the number of V4L2 buffer slots to reserve. Instead, the number
> of internal buffers should be the minimum required by the pipeline to
> keep the requests flowing, in order to avoid wasting memory, while the
> number of V4L2 buffer slots should be greater than the expected number
> of requests queued by the application, in order to avoid thrashing
> dmabuf mappings, which would degrade performance.
>
> Stop relying on bufferCount for these numbers and instead set them to
> appropriate, and independent, constants.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v9:
> - rebased
>
> Changes in v8:
> - New
> ---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++++++++++-------
> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 3 +--
> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 ++
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 8fe37c4f..5028ef1a 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -199,6 +199,16 @@ private:
> Camera *activeCamera_;
>
> const MediaPad *ispSink_;
> +
> + /*
> + * This many internal buffers (or rather parameter and statistics buffer
> + * pairs) ensures that the pipeline runs smoothly, without frame drops.
> + * This number considers:
> + * - three buffers queued to the driver, which is also the minimum
> + * required to start streaming
> + * - one buffer being processed on the IPA
> + */
> + static constexpr unsigned int kRkISP1InternalBufferCount = 4;
I indeed see
drivers/media/platform/rockchip/rkisp1/rkisp1-capture.c:#define RKISP1_MIN_BUFFERS_NEEDED 3
This seems quite right
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> };
>
> RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
> @@ -835,17 +845,12 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> unsigned int ipaBufferId = 1;
> int ret;
>
> - unsigned int maxCount = std::max({
> - data->mainPathStream_.configuration().bufferCount,
> - data->selfPathStream_.configuration().bufferCount,
> - });
> -
> if (!isRaw_) {
> - ret = param_->allocateBuffers(maxCount, ¶mBuffers_);
> + ret = param_->allocateBuffers(kRkISP1InternalBufferCount, ¶mBuffers_);
> if (ret < 0)
> goto error;
>
> - ret = stat_->allocateBuffers(maxCount, &statBuffers_);
> + ret = stat_->allocateBuffers(kRkISP1InternalBufferCount, &statBuffers_);
> if (ret < 0)
> goto error;
> }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 5079b268..a168e0ad 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -370,8 +370,7 @@ int RkISP1Path::start()
> if (running_)
> return -EBUSY;
>
> - /* \todo Make buffer count user configurable. */
> - ret = video_->importBuffers(RKISP1_BUFFER_COUNT);
> + ret = video_->importBuffers(kRkISP1BufferSlotCount);
> if (ret)
> return ret;
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index bdf3f95b..5b53783c 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -76,6 +76,8 @@ private:
> std::unique_ptr<V4L2Subdevice> resizer_;
> std::unique_ptr<V4L2VideoDevice> video_;
> MediaLink *link_;
> +
> + static constexpr unsigned int kRkISP1BufferSlotCount = 16;
> };
>
> class RkISP1MainPath : public RkISP1Path
> --
> 2.35.1
>
More information about the libcamera-devel
mailing list