From patchwork Wed Dec 28 22:29:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Paul Elder X-Patchwork-Id: 18063 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 23D08C3220 for ; Wed, 28 Dec 2022 22:30:30 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7281E625C8; Wed, 28 Dec 2022 23:30:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1672266629; bh=n57lT7DNtJle+qcAg9+E5WWWv74y0tC+ehajux5sFhA=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=xUbPY7iRePqJCRTQx7IVe23Wdo/iqSW1hncOHigt0TDmoXCJugpm90FRYhGoD0BqK 7YiBj5Q/VbR8ZcJm3q56ienJyAim62CGzbqPgM/3mPWHL6OpNOoDAiRa2WPRyLMoIY 0wJE8emqvp+FXu7CY7dxWDQ1IPiWOGOEZXvFv5i5vbbLkW8W6b4cl1TOTYbU99Gldr A00Q6R+3j75OF4nOpm7uVAEStuff7d/zdaHhiMZlbjyi0xaeFrivmwv88WQeKR4rc6 Zo/ZD90h9PM9J8kcb7Nkha9/EZH9zTLLIojnPEIQkDXic/yYio99a0IPGen7HQtK39 +o68gAAodRXTA== 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 F2C8E625DD for ; Wed, 28 Dec 2022 23:30:25 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="hcdvECQX"; dkim-atps=neutral Received: from pyrite.mediacom.info (unknown [IPv6:2604:2d80:ad8a:9000:1bf9:855b:22de:3645]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BAFD76D0; Wed, 28 Dec 2022 23:30:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1672266625; bh=n57lT7DNtJle+qcAg9+E5WWWv74y0tC+ehajux5sFhA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hcdvECQXLKrYs5Njvm+Z9zr8B4HdzauO9bvV5JVSfNY9l8rOzNYALEP01BUDZme+b LMD1oXBW7PBiq/m7xki8to+mnbaTcR3FGOQaFsc7RPb2Owcm08AHWyLfjY5mFNDjCh QE91A8cUhIPvLaEuontL3o4PInMKth+GHwJz+YEA= To: libcamera-devel@lists.libcamera.org Date: Wed, 28 Dec 2022 16:29:51 -0600 Message-Id: <20221228223003.2265712-8-paul.elder@ideasonboard.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221228223003.2265712-1-paul.elder@ideasonboard.com> References: <20221228223003.2265712-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v10 07/19] libcamera: pipeline: rkisp1: Don't rely on bufferCount 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: , X-Patchwork-Original-From: Paul Elder via libcamera-devel From: Paul Elder Reply-To: Paul Elder 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 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 Reviewed-by: Paul Elder Signed-off-by: Paul Elder Reviewed-by: Jacopo Mondi --- 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 5994b974..27d70c6e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -200,6 +200,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; }; RkISP1Frames::RkISP1Frames(PipelineHandler *pipe) @@ -836,17 +846,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 resizer_; std::unique_ptr video_; MediaLink *link_; + + static constexpr unsigned int kRkISP1BufferSlotCount = 16; }; class RkISP1MainPath : public RkISP1Path