{"id":23277,"url":"https://patchwork.libcamera.org/api/patches/23277/?format=json","web_url":"https://patchwork.libcamera.org/patch/23277/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20250428090413.38234-6-s.pueschel@pengutronix.de>","date":"2025-04-28T09:02:30","name":"[v11,05/19] libcamera: pipeline: rkisp1: Don't rely on bufferCount","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"8d1126d89cae86802b37651a10c81d288dfb0e1f","submitter":{"id":225,"url":"https://patchwork.libcamera.org/api/people/225/?format=json","name":"Sven Püschel","email":"s.pueschel@pengutronix.de"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/23277/mbox/","series":[{"id":5148,"url":"https://patchwork.libcamera.org/api/series/5148/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=5148","date":"2025-04-28T09:02:25","name":"lc-compliance: Add test to queue more requests than hardware depth","version":11,"mbox":"https://patchwork.libcamera.org/series/5148/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/23277/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/23277/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4D489C331F\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Apr 2025 09:05:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A9EC568B32;\n\tMon, 28 Apr 2025 11:05:17 +0200 (CEST)","from metis.whiteo.stw.pengutronix.de\n\t(metis.whiteo.stw.pengutronix.de [IPv6:2a0a:edc0:2:b01:1d::104])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E06C368AD6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Apr 2025 11:05:06 +0200 (CEST)","from ptz.office.stw.pengutronix.de ([2a0a:edc0:0:900:1d::77]\n\thelo=peter.guest.stw.pengutronix.de)\n\tby metis.whiteo.stw.pengutronix.de with esmtp (Exim 4.92)\n\t(envelope-from <s.pueschel@pengutronix.de>)\n\tid 1u9KQE-0001au-H3; Mon, 28 Apr 2025 11:05:06 +0200"],"From":"=?utf-8?q?Sven_P=C3=BCschel?= <s.pueschel@pengutronix.de>","To":"libcamera-devel@lists.libcamera.org","Cc":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= <nfraprado@collabora.com>,\n\tPaul Elder <paul.elder@ideasonboard.com>, \n\tJacopo Mondi <jacopo@jmondi.org>, =?utf-8?q?Sven_P=C3=BCschel?=\n\t<s.pueschel@pengutronix.de>","Subject":"[PATCH v11 05/19] libcamera: pipeline: rkisp1: Don't rely on\n\tbufferCount","Date":"Mon, 28 Apr 2025 11:02:30 +0200","Message-ID":"<20250428090413.38234-6-s.pueschel@pengutronix.de>","X-Mailer":"git-send-email 2.49.0","In-Reply-To":"<20250428090413.38234-1-s.pueschel@pengutronix.de>","References":"<20250428090413.38234-1-s.pueschel@pengutronix.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","X-SA-Exim-Connect-IP":"2a0a:edc0:0:900:1d::77","X-SA-Exim-Mail-From":"s.pueschel@pengutronix.de","X-SA-Exim-Scanned":"No (on metis.whiteo.stw.pengutronix.de);\n\tSAEximRunCond expanded to false","X-PTX-Original-Recipient":"libcamera-devel@lists.libcamera.org","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"From: Nícolas F. R. A. Prado <nfraprado@collabora.com>\n\nCurrently the rkisp1 pipeline handler relies on bufferCount to decide on\nthe number of parameter and statistics buffers to allocate internally\nand for the number of V4L2 buffer slots to reserve. Instead, the number\nof internal buffers should be the minimum required by the pipeline to\nkeep the requests flowing, in order to avoid wasting memory, while the\nnumber of V4L2 buffer slots should be greater than the expected number\nof requests queued by the application, in order to avoid thrashing\ndmabuf mappings, which would degrade performance.\n\nStop relying on bufferCount for these numbers and instead set them to\nappropriate, and independent, constants.\n\nSigned-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\nSigned-off-by: Sven Püschel <s.pueschel@pengutronix.de>\n\n---\nChanges in v11:\n- rebased\n\nChanges in v9:\n- rebased\n\nChanges in v8:\n- New\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 21 ++++++++++++-------\n src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |  3 +--\n src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  2 ++\n 3 files changed, 16 insertions(+), 10 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 4e8eff70..81bf1c55 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -225,6 +225,16 @@ private:\n \tCamera *activeCamera_;\n \n \tconst MediaPad *ispSink_;\n+\n+\t/*\n+\t * This many internal buffers (or rather parameter and statistics buffer\n+\t * pairs) ensures that the pipeline runs smoothly, without frame drops.\n+\t * This number considers:\n+\t * - three buffers queued to the driver, which is also the minimum\n+\t *   required to start streaming\n+\t * - one buffer being processed on the IPA\n+\t */\n+\tstatic constexpr unsigned int kRkISP1InternalBufferCount = 4;\n };\n \n RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n@@ -981,24 +991,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n \tunsigned int ipaBufferId = 1;\n \tint ret;\n \n-\tunsigned int maxCount = std::max({\n-\t\tdata->mainPathStream_.configuration().bufferCount,\n-\t\tdata->selfPathStream_.configuration().bufferCount,\n-\t});\n-\n \tif (!isRaw_) {\n-\t\tret = param_->allocateBuffers(maxCount, &paramBuffers_);\n+\t\tret = param_->allocateBuffers(kRkISP1InternalBufferCount, &paramBuffers_);\n \t\tif (ret < 0)\n \t\t\tgoto error;\n \n-\t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n+\t\tret = stat_->allocateBuffers(kRkISP1InternalBufferCount, &statBuffers_);\n \t\tif (ret < 0)\n \t\t\tgoto error;\n \t}\n \n \t/* If the dewarper is being used, allocate internal buffers for ISP. */\n \tif (useDewarper_) {\n-\t\tret = mainPath_.exportBuffers(maxCount, &mainPathBuffers_);\n+\t\tret = mainPath_.exportBuffers(kRkISP1InternalBufferCount, &mainPathBuffers_);\n \t\tif (ret < 0)\n \t\t\tgoto error;\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\nindex 64018dc5..6a5b22ba 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n@@ -487,8 +487,7 @@ int RkISP1Path::start()\n \tif (running_)\n \t\treturn -EBUSY;\n \n-\t/* \\todo Make buffer count user configurable. */\n-\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n+\tret = video_->importBuffers(kRkISP1BufferSlotCount);\n \tif (ret)\n \t\treturn ret;\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\nindex 430181d3..7365275d 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n@@ -88,6 +88,8 @@ private:\n \t * which are guaranteed to be supported by the pipeline.\n \t */\n \tstd::map<const CameraSensor *, std::vector<Size>> sensorSizesMap_;\n+\n+\tstatic constexpr unsigned int kRkISP1BufferSlotCount = 16;\n };\n \n class RkISP1MainPath : public RkISP1Path\n","prefixes":["v11","05/19"]}