From patchwork Thu Jul 17 12:59:21 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23839 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 5D9CCC3237 for ; Thu, 17 Jul 2025 12:59:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0970368F83; Thu, 17 Jul 2025 14:59:42 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="u2D+LuRk"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 29BD568F78 for ; Thu, 17 Jul 2025 14:59:39 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:7b93:8acd:d82d:248d]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 0D1DE1FA4; Thu, 17 Jul 2025 14:59:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1752757145; bh=HVix85CEOalKqOQqPNte3u8ELZDK1U7M8WvxXbc1kKk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=u2D+LuRk02B+4L7jlcTGs6/xxHqC/WdIP9OuRtwZTxZrlHc5if8rmLGQ6UMQiGQfL TJ/HYRdLY1xIN+tQApCSyBwuHAsGKjauxDlj+vVxLDQr5UwD7+YciV5yqoyVlfl5J3 nBvlQzq5oE1HNQZMaS6Udqpefly/gclxn4GThOYs= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Umang Jain , Kieran Bingham , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v3 1/5] libcamera: pipeline_handler: Move waitingRequests_ into camera class Date: Thu, 17 Jul 2025 14:59:21 +0200 Message-ID: <20250717125931.2848300-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> References: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The waiting requests of one camera should not be able to influence queuing to another camera. Currently a request that is in the waitingQueue and waiting for preparation blocks all requests of other cameras even if they are already prepared. To fix that replace the single waitingRequests_ queue with one queue per camera. Signed-off-by: Stefan Klug Reviewed-by: Umang Jain Reviewed-by: Kieran Bingham Tested-By: Sven Püschel --- Changes in v3: - Collected tag Changes in v2: - Fixed capturing on lambda expression - Added documentation for waitingRequests_ - Improved commit message Changes in v1: - Added camera param to doQueueRequests() which allows to remove a loop --- include/libcamera/internal/camera.h | 2 ++ include/libcamera/internal/pipeline_handler.h | 5 +--- src/libcamera/camera.cpp | 9 +++++++ src/libcamera/pipeline_handler.cpp | 26 ++++++++++++------- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h index 18f5c32a18e4..8a2e9ed5894d 100644 --- a/include/libcamera/internal/camera.h +++ b/include/libcamera/internal/camera.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -36,6 +37,7 @@ public: const PipelineHandler *pipe() const { return pipe_.get(); } std::list queuedRequests_; + std::queue waitingRequests_; ControlInfoMap controlInfo_; ControlList properties_; diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index 972a2fa65310..be017ad47219 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -8,7 +8,6 @@ #pragma once #include -#include #include #include #include @@ -89,13 +88,11 @@ private: virtual void disconnect(); void doQueueRequest(Request *request); - void doQueueRequests(); + void doQueueRequests(Camera *camera); std::vector> mediaDevices_; std::vector> cameras_; - std::queue waitingRequests_; - const char *name_; unsigned int useCount_; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index c180a5fdde93..2ae50eaeaa5a 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -628,6 +628,15 @@ Camera::Private::~Private() * PipelineHandler::completeRequest() */ +/** + * \var Camera::Private::waitingRequests_ + * \brief The queue of waiting requests + * + * This queue tracks all requests that can not yet be queued to the device. + * Either because they are not yet prepared or because the maximum number of + * queued requests was reached. + */ + /** * \var Camera::Private::controlInfo_ * \brief The set of controls supported by the camera diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index d84dff3c9f19..b50eda5e0f86 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -363,15 +363,16 @@ void PipelineHandler::stop(Camera *camera) /* Stop the pipeline handler and let the queued requests complete. */ stopDevice(camera); + Camera::Private *data = camera->_d(); + /* Cancel and signal as complete all waiting requests. */ - while (!waitingRequests_.empty()) { - Request *request = waitingRequests_.front(); - waitingRequests_.pop(); + while (!data->waitingRequests_.empty()) { + Request *request = data->waitingRequests_.front(); + data->waitingRequests_.pop(); cancelRequest(request); } /* Make sure no requests are pending. */ - Camera::Private *data = camera->_d(); ASSERT(data->queuedRequests_.empty()); data->requestSequence_ = 0; @@ -414,7 +415,9 @@ void PipelineHandler::registerRequest(Request *request) * Connect the request prepared signal to notify the pipeline handler * when a request is ready to be processed. */ - request->_d()->prepared.connect(this, &PipelineHandler::doQueueRequests); + request->_d()->prepared.connect(this, [this, request]() { + doQueueRequests(request->_d()->camera()); + }); } /** @@ -444,7 +447,9 @@ void PipelineHandler::queueRequest(Request *request) { LIBCAMERA_TRACEPOINT(request_queue, request); - waitingRequests_.push(request); + Camera *camera = request->_d()->camera(); + Camera::Private *data = camera->_d(); + data->waitingRequests_.push(request); request->_d()->prepare(300ms); } @@ -478,15 +483,16 @@ void PipelineHandler::doQueueRequest(Request *request) * Iterate the list of waiting requests and queue them to the device one * by one if they have been prepared. */ -void PipelineHandler::doQueueRequests() +void PipelineHandler::doQueueRequests(Camera *camera) { - while (!waitingRequests_.empty()) { - Request *request = waitingRequests_.front(); + Camera::Private *data = camera->_d(); + while (!data->waitingRequests_.empty()) { + Request *request = data->waitingRequests_.front(); if (!request->_d()->prepared_) break; doQueueRequest(request); - waitingRequests_.pop(); + data->waitingRequests_.pop(); } } From patchwork Thu Jul 17 12:59:22 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23840 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 14BC1C3237 for ; Thu, 17 Jul 2025 12:59:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C496468F8D; Thu, 17 Jul 2025 14:59:44 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="gtlnfJNJ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A204B68F7A for ; Thu, 17 Jul 2025 14:59:41 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:7b93:8acd:d82d:248d]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id A0B641E74; Thu, 17 Jul 2025 14:59:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1752757147; bh=FtP3LBEAlKfycED7DyqSkyT0cPc5DT8/6QdDfB3TVPs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gtlnfJNJpJlUTXWWfusfdDqT9AOrBrmhg/YBwVzAJ/lfXDDde7wOm0fdw36YTIzoo 7VeiNqYioRMvUR0SQjFvrukeP+qH9fSXYe3I5JCj6Msox2Xu+MeQpa1aCcsisUE/h7 rlOQeJGiKJpFeX5+Nv3u9oRVuURVb33/0BU5oyX8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham , Umang Jain , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v3 2/5] libcamera: pipeline_handler: Allow to limit the number of queued requests Date: Thu, 17 Jul 2025 14:59:22 +0200 Message-ID: <20250717125931.2848300-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> References: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a maxQueuedRequestsDevice constructor parameter to allow pipeline handler classes to limit the maximum number of requests that get queued to the device in queueRequestDevice(). The default value is set to an arbitrary number of 32 which is big enough for all currently known use cases. Signed-off-by: Kieran Bingham Signed-off-by: Stefan Klug Reviewed-by: Umang Jain Tested-By: Sven Püschel --- Changes in v3: - Updated documentation for maxQueuedRequestsDevice_ - Collected tag Changes in v2: - Moved the fix to properly handle the waitingRequests queue when stopping the device into this patch as they belong together - Added documentation for maxQueuedRequestsDevice_ - Fixed an issue with doQueueRequests() beeing called recursively Changes in v1: - Used a const member variable to carry the maximum number of requests - Improved commit message - Added docs --- include/libcamera/internal/pipeline_handler.h | 4 +- src/libcamera/pipeline_handler.cpp | 58 +++++++++++++++---- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index be017ad47219..e89d6a33e398 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -33,7 +33,8 @@ class PipelineHandler : public std::enable_shared_from_this, public Object { public: - PipelineHandler(CameraManager *manager); + PipelineHandler(CameraManager *manager, + unsigned int maxQueuedRequestsDevice = 32); virtual ~PipelineHandler(); virtual bool match(DeviceEnumerator *enumerator) = 0; @@ -80,6 +81,7 @@ protected: virtual void releaseDevice(Camera *camera); CameraManager *manager_; + const unsigned int maxQueuedRequestsDevice_; private: void unlockMediaDevices(); diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index b50eda5e0f86..e5f9e55c9783 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -62,13 +62,17 @@ LOG_DEFINE_CATEGORY(Pipeline) /** * \brief Construct a PipelineHandler instance * \param[in] manager The camera manager + * \param[in] maxQueuedRequestsDevice The maximum number of requests queued to + * the device * * In order to honour the std::enable_shared_from_this<> contract, * PipelineHandler instances shall never be constructed manually, but always * through the PipelineHandlerFactoryBase::create() function. */ -PipelineHandler::PipelineHandler(CameraManager *manager) - : manager_(manager), useCount_(0) +PipelineHandler::PipelineHandler(CameraManager *manager, + unsigned int maxQueuedRequestsDevice) + : manager_(manager), maxQueuedRequestsDevice_(maxQueuedRequestsDevice), + useCount_(0) { } @@ -360,20 +364,28 @@ void PipelineHandler::unlockMediaDevices() */ void PipelineHandler::stop(Camera *camera) { + /* + * Take all waiting requests so that they are not requeued in response + * to completeRequest() being called inside stopDevice(). Cancel them + * after the device to keep them in order. + */ + Camera::Private *data = camera->_d(); + std::queue waitingRequests; + waitingRequests.swap(data->waitingRequests_); + /* Stop the pipeline handler and let the queued requests complete. */ stopDevice(camera); - Camera::Private *data = camera->_d(); - /* Cancel and signal as complete all waiting requests. */ - while (!data->waitingRequests_.empty()) { - Request *request = data->waitingRequests_.front(); - data->waitingRequests_.pop(); + while (!waitingRequests.empty()) { + Request *request = waitingRequests.front(); + waitingRequests.pop(); cancelRequest(request); } /* Make sure no requests are pending. */ ASSERT(data->queuedRequests_.empty()); + ASSERT(data->waitingRequests_.empty()); data->requestSequence_ = 0; } @@ -430,9 +442,9 @@ void PipelineHandler::registerRequest(Request *request) * requests which have to be prepared to make sure they are ready for being * queued to the pipeline handler. * - * The queue of waiting requests is iterated and all prepared requests are - * passed to the pipeline handler in the same order they have been queued by - * calling this function. + * The queue of waiting requests is iterated and up to \a + * maxQueuedRequestsDevice_ prepared requests are passed to the pipeline handler + * in the same order they have been queued by calling this function. * * If a Request fails during the preparation phase or if the pipeline handler * fails in queuing the request to the hardware the request is cancelled. @@ -487,12 +499,19 @@ void PipelineHandler::doQueueRequests(Camera *camera) { Camera::Private *data = camera->_d(); while (!data->waitingRequests_.empty()) { + if (data->queuedRequests_.size() == maxQueuedRequestsDevice_) + break; + Request *request = data->waitingRequests_.front(); if (!request->_d()->prepared_) break; - doQueueRequest(request); + /* + * Pop the request first, in case doQueueRequests() is called + * recursively from within doQueueRequest() + */ data->waitingRequests_.pop(); + doQueueRequest(request); } } @@ -568,6 +587,9 @@ void PipelineHandler::completeRequest(Request *request) data->queuedRequests_.pop_front(); camera->requestComplete(req); } + + /* Allow any waiting requests to be queued to the pipeline. */ + doQueueRequests(camera); } /** @@ -768,6 +790,20 @@ void PipelineHandler::disconnect() * constant for the whole lifetime of the pipeline handler. */ +/** + * \var PipelineHandler::maxQueuedRequestsDevice_ + * \brief The maximum number of requests the pipeline handler shall queue to the + * device + * + * maxQueuedRequestsDevice_ limits the number of request that the + * pipeline handler shall queue to the underlying hardware, in order to + * saturate the pipeline with requests. The application may choose to queue + * as many requests as it desires, however only maxQueuedRequestsDevice_ + * requests will be queued to the hardware at a given point in time. The + * remaining requests will be kept waiting in the internal waiting + * queue, to be queued at a later stage. + */ + /** * \fn PipelineHandler::name() * \brief Retrieve the pipeline handler name From patchwork Thu Jul 17 12:59:23 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23841 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id E93F5C3237 for ; Thu, 17 Jul 2025 12:59:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A872668F8C; Thu, 17 Jul 2025 14:59:46 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="mR7fuQr9"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4A89F68F81 for ; Thu, 17 Jul 2025 14:59:44 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:7b93:8acd:d82d:248d]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 5153021F2; Thu, 17 Jul 2025 14:59:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1752757150; bh=leYeTMkyXLzD0Qchj+IHROvcS8R27QrAnfZW3CFZN/U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mR7fuQr9mVpDrmfxYvcnDFcnUiX0FCStoBMPhKEOrNoiSkFfQkt2TJ/M9lFxr+KQI 5b/HA/MHgYTv7/HRUZUZB1Rqf/vsMLm6dNBoLcfsp+6Z6MlM0F1n4YB+n3H5DWuzUd RwcvkPp5eq3l8i/HmxA8KvKYUyy++ECkt1+XkDJo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham , =?utf-8?q?Sven_P=C3=BCschel?= Subject: [PATCH v3 3/5] pipeline: rkisp1: Limit the maximum number of buffers queued in Date: Thu, 17 Jul 2025 14:59:23 +0200 Message-ID: <20250717125931.2848300-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> References: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" To keep the regulation of the algorithms as fast as possible and at the same time allow more buffers to be allocated, limit the amount of buffers that get queued into the device to the pipeline depth plus a tiny margin. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Tested-By: Sven Püschel --- Changes in v3: - Collected tags - Renamed kPipelineDepth to kRkISP1MaxQueuedRequests and made it static constexpr - Added comment to kRkISP1MaxQueuedRequests - Improved commit message Changes in v1: - Replaced function overload with constructor param --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 675f0a7490a6..7954ea82fd0d 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -155,6 +155,17 @@ private: Transform combinedTransform_; }; +namespace { + +/* + * Maximum number of requests that shall be queued into the pipeline to keep + * the regulation fast. \todo This needs revisiting as soon as buffers got + * decoupled from requests and/or a fast path for controls was implemented. + */ +static constexpr unsigned int kRkISP1MaxQueuedRequests = 4; + +} // namespace + class PipelineHandlerRkISP1 : public PipelineHandler { public: @@ -684,7 +695,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() */ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) - : PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false) + : PipelineHandler(manager, kRkISP1MaxQueuedRequests), + hasSelfPath_(true), useDewarper_(false) { } From patchwork Thu Jul 17 12:59:24 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23842 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 8EF54C3237 for ; Thu, 17 Jul 2025 12:59:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 40BEA68F8D; Thu, 17 Jul 2025 14:59:50 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="AQM1X+yb"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6833C68F83 for ; Thu, 17 Jul 2025 14:59:47 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:7b93:8acd:d82d:248d]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 690F21FA4; Thu, 17 Jul 2025 14:59:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1752757153; bh=cOJHLbFm28MpGeYPvx4YiwbaHud6V9ZzX9oVOBnr4ic=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AQM1X+ybItgLgRZyzDi1sw2PdGZ3WqVtpFq3bxw5WFRo6EjNEjQ1xccH/iwElpFo1 gYJrPlTI0vAANbui2kI3PtEmE0t159Ksh/k7kLH8xSI4YFC57PjpRj4UKwkNTMFGwz 5ouA0DbnWBy9FJMNXjz1f/E4laDKnT9ydLA4jJ4A= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , =?utf-8?q?Sven_P=C3=BCsche?= =?utf-8?q?l?= Subject: [PATCH v3 4/5] pipeline: rkisp1: Properly handle the bufferCount set in the stream configuration Date: Thu, 17 Jul 2025 14:59:24 +0200 Message-ID: <20250717125931.2848300-5-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> References: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The StreamConfiguration::bufferCount is reset to a hardcoded value of 4 in RkISP1Path::validate(). Keep the minimum value of 4 but do not reset it, if it was set to a larger value. This allows the user to set bufferCount to an arbitrary number of buffers which then can be allocated for example by the FrameBufferAllocator. If the bufferCount is set to a smaller value it gets reset to 4 again and the configuation is market as adjusted. Signed-off-by: Stefan Klug Tested-By: Sven Püschel Reviewed-by: Kieran Bingham --- Changes in v3: - Introduced a new constant kRkISP1MinBufferCount - Ensure that bufferCount is at least 4 in validate - Use the stream bufferCount for the number of import buffers, so that we don't issue cache flushes due to more buffers circulating than imported in V4L2 Changes in v1: - Removed todo comment that was solved by this change --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 +++++++++++-- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 ++----- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +--- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 7954ea82fd0d..aee267a90f4b 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -164,6 +164,8 @@ namespace { */ static constexpr unsigned int kRkISP1MaxQueuedRequests = 4; +static constexpr unsigned int kRkISP1MinBufferCount = 4; + } // namespace class PipelineHandlerRkISP1 : public PipelineHandler @@ -607,6 +609,12 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() return false; } + if (tryCfg.bufferCount < kRkISP1MinBufferCount) { + tryCfg.bufferCount = kRkISP1MinBufferCount; + if (expectedStatus == Valid) + return false; + } + cfg = tryCfg; cfg.setStream(stream); return true; @@ -796,6 +804,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera, return nullptr; cfg.colorSpace = colorSpace; + cfg.bufferCount = kRkISP1MinBufferCount; config->addConfiguration(cfg); } @@ -1129,14 +1138,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->mainPath_->isEnabled()) { - ret = mainPath_.start(); + ret = mainPath_.start(data->mainPathStream_.configuration().bufferCount); if (ret) return ret; actions += [&]() { mainPath_.stop(); }; } if (hasSelfPath_ && data->selfPath_->isEnabled()) { - ret = selfPath_.start(); + ret = selfPath_.start(data->selfPathStream_.configuration().bufferCount); 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); 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_; From patchwork Thu Jul 17 12:59:25 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23843 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 0E06DC3237 for ; Thu, 17 Jul 2025 12:59:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C926768F83; Thu, 17 Jul 2025 14:59:51 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vMZONg5i"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 53FEB68F93 for ; Thu, 17 Jul 2025 14:59:50 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:7b93:8acd:d82d:248d]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 4D8AE1AE2; Thu, 17 Jul 2025 14:59:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1752757156; bh=32FzGRPrD4vv/UWqGVTPoI/Y6AtCsuXpNcBj2K7hHvw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vMZONg5iZCuK2eJJ58TO7i/col6Y2ODEzgkhdzjFNjBrxKf1fxMS+uAmHb/91sHNn icqmLQzJPaf+aNGhO3pbzVQ0M7vM0EAaPJo/6ARCaAx2/HjJIvMfgIoIBiArrmPNPS XxWW/+mJvRW7miNsY/PZfEuL0o18HVSnxuNavk2U= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , =?utf-8?b?TsOtY29sYXMgRi4g?= =?utf-8?q?R=2E_A=2E_Prado?= , Paul Elder , =?utf-8?q?Sven_P=C3=BCschel?= , Kieran Bingham , Umang Jain Subject: [PATCH v3 5/5] libcamera: pipeline: rkisp1: Don't rely on bufferCount Date: Thu, 17 Jul 2025 14:59:25 +0200 Message-ID: <20250717125931.2848300-6-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> References: <20250717125931.2848300-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nícolas F. R. A. Prado Currently the rkisp1 pipeline handler relies on bufferCount to decide on the number of parameter and statistics buffers to allocate internally. 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. Stop relying on bufferCount for these numbers and instead set them to kRkISP1MinBufferCount. Signed-off-by: Nícolas F. R. A. Prado Signed-off-by: Paul Elder Signed-off-by: Sven Püschel Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Reviewed-by: Umang Jain --- Changes in v3: - Collected tags - Updated cmmit message --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index aee267a90f4b..3d92b50c7d27 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -164,6 +164,10 @@ namespace { */ static constexpr unsigned int kRkISP1MaxQueuedRequests = 4; +/* + * This many internal buffers (or rather parameter and statistics buffer + * pairs) ensures that the pipeline runs smoothly, without frame drops. + */ static constexpr unsigned int kRkISP1MinBufferCount = 4; } // namespace @@ -1004,24 +1008,19 @@ 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(kRkISP1MinBufferCount, ¶mBuffers_); if (ret < 0) goto error; - ret = stat_->allocateBuffers(maxCount, &statBuffers_); + ret = stat_->allocateBuffers(kRkISP1MinBufferCount, &statBuffers_); if (ret < 0) goto error; } /* If the dewarper is being used, allocate internal buffers for ISP. */ if (useDewarper_) { - ret = mainPath_.exportBuffers(maxCount, &mainPathBuffers_); + ret = mainPath_.exportBuffers(kRkISP1MinBufferCount, &mainPathBuffers_); if (ret < 0) goto error;