diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index aa544d7b..5a2c5644 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -236,7 +236,6 @@ StreamConfiguration CIO2Device::generateConfiguration(Size size) const cfg.size = sensorFormat.size; cfg.pixelFormat = mbusCodesToPixelFormat.at(sensorFormat.code); - cfg.bufferCount = kBufferCount; return cfg; } @@ -337,13 +336,14 @@ int CIO2Device::exportBuffers(unsigned int count, return output_->exportBuffers(count, buffers); } -int CIO2Device::start() +int CIO2Device::start(unsigned int internalBufferCount, + unsigned int bufferSlotCount) { - int ret = output_->exportBuffers(kBufferCount, &buffers_); + int ret = output_->exportBuffers(internalBufferCount, &buffers_); if (ret < 0) return ret; - ret = output_->importBuffers(kBufferCount); + 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 963c2f6b..3c6cc8bc 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -31,8 +31,6 @@ enum class Transform; class CIO2Device { public: - static constexpr unsigned int kBufferCount = 4; - CIO2Device(); std::vector formats() const; @@ -50,7 +48,7 @@ public: V4L2SubdeviceFormat getSensorFormat(const std::vector &mbusCodes, const Size &size) const; - int start(); + int start(unsigned int internalBufferCount, unsigned int bufferSlotCount); int stop(); CameraSensor *sensor() { return sensor_.get(); } diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index 7be78091..50b981f2 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -576,22 +576,23 @@ 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 internalBufferCount, + 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(internalBufferCount, ¶mBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU param buffers"; goto error; } - ret = stat_->allocateBuffers(bufferCount, &statBuffers_); + ret = stat_->allocateBuffers(internalBufferCount, &statBuffers_); if (ret < 0) { LOG(IPU3, Error) << "Failed to allocate ImgU stat buffers"; goto error; @@ -602,13 +603,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 fa508316..47d6ed0c 100644 --- a/src/libcamera/pipeline/ipu3/imgu.h +++ b/src/libcamera/pipeline/ipu3/imgu.h @@ -84,7 +84,8 @@ public: outputFormat); } - int allocateBuffers(unsigned int bufferCount); + int allocateBuffers(unsigned int internalBufferCount, + unsigned int bufferSlotCount); void freeBuffers(); int start(); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 30b74d4e..665466c7 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -159,7 +159,7 @@ private: int updateControls(IPU3CameraData *data); int registerCameras(); - int allocateBuffers(Camera *camera); + int allocateBuffers(Camera *camera, unsigned int bufferSlotCount); int freeBuffers(Camera *camera); ImgUDevice imgu0_; @@ -170,6 +170,7 @@ private: std::vector ipaBuffers_; static constexpr unsigned int kMinimumRequests = 3; + static constexpr unsigned int kIPU3BufferSlotCount = 16; }; IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data) @@ -660,20 +661,25 @@ 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); + /* + * 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. + */ + ret = imgu->allocateBuffers(kMinimumRequests + 1, bufferSlotCount); if (ret < 0) return ret; @@ -731,7 +737,7 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis return ret; /* Allocate buffers for internal pipeline usage. */ - ret = allocateBuffers(camera); + ret = allocateBuffers(camera, kIPU3BufferSlotCount); if (ret) return ret; @@ -744,8 +750,21 @@ 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. + * + * 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. */ - ret = cio2->start(); + ret = cio2->start(kMinimumRequests + 1, kIPU3BufferSlotCount); if (ret) goto error;