[libcamera-devel] [PATCH v8 04/17] libcamera: pipeline: ipu3: Don't rely on bufferCount
Nícolas F. R. A. Prado
nfraprado at collabora.com
Tue Aug 24 21:56:23 CEST 2021
Currently the ipu3 pipeline handler relies on bufferCount to decide on
the number of V4L2 buffer slots to reserve as well as for the number of
buffers to allocate internally for the CIO2 raw buffers and
parameter-statistics ImgU buffer pairs. Instead, the number of internal
buffers should be the minimum required by the pipeline to keep the
requests flowing without frame drops, 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>
---
Changes in v8:
- New
src/libcamera/pipeline/ipu3/cio2.cpp | 6 +++---
src/libcamera/pipeline/ipu3/cio2.h | 18 +++++++++++++++++-
src/libcamera/pipeline/ipu3/imgu.cpp | 12 ++++++------
src/libcamera/pipeline/ipu3/imgu.h | 15 ++++++++++++++-
src/libcamera/pipeline/ipu3/ipu3.cpp | 20 ++++++++------------
5 files changed, 48 insertions(+), 23 deletions(-)
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 1bcd580e251c..b940a0f6d7d6 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -225,13 +225,13 @@ int CIO2Device::exportBuffers(unsigned int count,
return output_->exportBuffers(count, buffers);
}
-int CIO2Device::start()
+int CIO2Device::start(unsigned int bufferSlotCount)
{
- int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_);
+ int ret = output_->exportBuffers(kCIO2InternalBufferCount, &buffers_);
if (ret < 0)
return ret;
- ret = output_->importBuffers(CIO2_BUFFER_COUNT);
+ ret = output_->importBuffers(bufferSlotCount);
if (ret)
LOG(IPU3, Error) << "Failed to import CIO2 buffers";
diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h
index f28e9f1ddf42..ab915b6a16fa 100644
--- a/src/libcamera/pipeline/ipu3/cio2.h
+++ b/src/libcamera/pipeline/ipu3/cio2.h
@@ -45,7 +45,7 @@ public:
int exportBuffers(unsigned int count,
std::vector<std::unique_ptr<FrameBuffer>> *buffers);
- int start();
+ int start(unsigned int bufferSlotCount);
int stop();
CameraSensor *sensor() { return sensor_.get(); }
@@ -69,6 +69,22 @@ private:
std::vector<std::unique_ptr<FrameBuffer>> buffers_;
std::queue<FrameBuffer *> availableBuffers_;
+
+ /*
+ * This many internal buffers for the CIO2 ensures that the pipeline
+ * runs smoothly, without frame drops. This number considers:
+ * - one buffer being DMA'ed to in CIO2
+ * - one buffer programmed by the CIO2 as the next buffer
+ * - one buffer under processing in ImgU
+ * - one extra idle buffer queued to CIO2, to account for possible
+ * delays in requeuing the buffer from ImgU back to CIO2
+ *
+ * Transient situations can arise when one of the parts, CIO2 or ImgU,
+ * finishes its processing first and experiences a lack of buffers, but
+ * they will shortly after return to the state described above as the
+ * other part catches up.
+ */
+ static constexpr unsigned int kCIO2InternalBufferCount = 4;
};
} /* namespace libcamera */
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index 317e482a1498..3484b378c189 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -593,22 +593,22 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,
/**
* \brief Allocate buffers for all the ImgU video devices
*/
-int ImgUDevice::allocateBuffers(unsigned int bufferCount)
+int ImgUDevice::allocateBuffers(unsigned int bufferSlotCount)
{
/* Share buffers between CIO2 output and ImgU input. */
- int ret = input_->importBuffers(bufferCount);
+ int ret = input_->importBuffers(bufferSlotCount);
if (ret) {
LOG(IPU3, Error) << "Failed to import ImgU input buffers";
return ret;
}
- ret = param_->allocateBuffers(bufferCount, ¶mBuffers_);
+ ret = param_->allocateBuffers(kImgUInternalBufferCount, ¶mBuffers_);
if (ret < 0) {
LOG(IPU3, Error) << "Failed to allocate ImgU param buffers";
goto error;
}
- ret = stat_->allocateBuffers(bufferCount, &statBuffers_);
+ ret = stat_->allocateBuffers(kImgUInternalBufferCount, &statBuffers_);
if (ret < 0) {
LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers";
goto error;
@@ -619,13 +619,13 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount)
* corresponding stream is active or inactive, as the driver needs
* buffers to be requested on the V4L2 devices in order to operate.
*/
- ret = output_->importBuffers(bufferCount);
+ ret = output_->importBuffers(bufferSlotCount);
if (ret < 0) {
LOG(IPU3, Error) << "Failed to import ImgU output buffers";
goto error;
}
- ret = viewfinder_->importBuffers(bufferCount);
+ ret = viewfinder_->importBuffers(bufferSlotCount);
if (ret < 0) {
LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers";
goto error;
diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
index 9d4915116087..61ccd17c2e31 100644
--- a/src/libcamera/pipeline/ipu3/imgu.h
+++ b/src/libcamera/pipeline/ipu3/imgu.h
@@ -61,7 +61,7 @@ public:
outputFormat);
}
- int allocateBuffers(unsigned int bufferCount);
+ int allocateBuffers(unsigned int bufferSlotCount);
void freeBuffers();
int start();
@@ -96,6 +96,19 @@ private:
std::string name_;
MediaDevice *media_;
+
+ /*
+ * This many internal buffers (or rather parameter and statistics buffer
+ * pairs) for the ImgU ensures that the pipeline runs smoothly, without
+ * frame drops. This number considers:
+ * - three buffers queued to the CIO2 (Since these buffers are bound to
+ * CIO2 buffers before queuing to the CIO2)
+ * - one buffer under processing in ImgU
+ *
+ * \todo Update this number when we make these buffers only get added to
+ * the FrameInfo after the raw buffers are dequeued from CIO2.
+ */
+ static constexpr unsigned int kImgUInternalBufferCount = 4;
};
} /* namespace libcamera */
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index eaa503a33208..cc519ae6adbe 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -156,7 +156,7 @@ private:
int initControls(IPU3CameraData *data);
int registerCameras();
- int allocateBuffers(Camera *camera);
+ int allocateBuffers(Camera *camera, unsigned int bufferSlotCount);
int freeBuffers(Camera *camera);
ImgUDevice imgu0_;
@@ -165,6 +165,8 @@ private:
MediaDevice *imguMediaDev_;
std::vector<IPABuffer> ipaBuffers_;
+
+ static constexpr unsigned int kIPU3BufferSlotCount = 16;
};
IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)
@@ -693,20 +695,14 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
* In order to be able to start the 'viewfinder' and 'stat' nodes, we need
* memory to be reserved.
*/
-int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
+int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
+ unsigned int bufferSlotCount)
{
IPU3CameraData *data = cameraData(camera);
ImgUDevice *imgu = data->imgu_;
- unsigned int bufferCount;
int ret;
- bufferCount = std::max({
- data->outStream_.configuration().bufferCount,
- data->vfStream_.configuration().bufferCount,
- data->rawStream_.configuration().bufferCount,
- });
-
- ret = imgu->allocateBuffers(bufferCount);
+ ret = imgu->allocateBuffers(bufferSlotCount);
if (ret < 0)
return ret;
@@ -758,7 +754,7 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
int ret;
/* Allocate buffers for internal pipeline usage. */
- ret = allocateBuffers(camera);
+ ret = allocateBuffers(camera, kIPU3BufferSlotCount);
if (ret)
return ret;
@@ -770,7 +766,7 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
* Start the ImgU video devices, buffers will be queued to the
* ImgU output and viewfinder when requests will be queued.
*/
- ret = cio2->start();
+ ret = cio2->start(kIPU3BufferSlotCount);
if (ret)
goto error;
--
2.33.0
More information about the libcamera-devel
mailing list