From patchwork Wed Nov 10 10:08:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 14516 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 532A8BDB1C for ; Wed, 10 Nov 2021 10:08:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 707D160362; Wed, 10 Nov 2021 11:08:09 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="G7ZJ8DU0"; dkim-atps=neutral Received: from mail-wm1-x333.google.com (mail-wm1-x333.google.com [IPv6:2a00:1450:4864:20::333]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0604B6034E for ; Wed, 10 Nov 2021 11:08:08 +0100 (CET) Received: by mail-wm1-x333.google.com with SMTP id b184-20020a1c1bc1000000b0033140bf8dd5so1508956wmb.5 for ; Wed, 10 Nov 2021 02:08:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=QTcjocxvUEcxxWS64VuvQEsn0E3guGnqlF2N2jhQXIA=; b=G7ZJ8DU0FOU5kEsKIMfDqw+ueTGPv0G457N9OFJPoi+rtT8Neg1/euRG/Sa5Rl1IDb VQW6ulVh15x8EWAfczygmIiiCbVl0V6/oQzSzOi8VqiFeZehThombHiOQ7Lyo5XK73pX yjRQPE7iexIWJqFrllzl7n9UcCRlw1dBSkcdC95dTB92PH5WEFQAzLJCw1ky4em8NeoJ pgM3H5JCYAetcyOPER4/tU+M6u+ocUaBOlhnE+CR2heCtLYUi8dUvzY/WDaNY/TNvhnV F81LrQ6quDp9TAtYWIVu8kBnTt8+mWK+so3b8N6DWTu9Ssrur/wDbzOWMFTWmUZmN4a+ F8Xg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=QTcjocxvUEcxxWS64VuvQEsn0E3guGnqlF2N2jhQXIA=; b=00Nt+fDCbRntMRibbILQ426tQEbtMt8Mh5i0wQAzudgjzv5jZ9XWCYeqNHsNXjZ6Xt Hae5E4JQykRQ4EKaM5JuiPBSuTFRc2ottKDtqdAV/JbqQXePmlke1wavkiEfmO4hx5UD AhC6OATmmBlx+OwgiJtzohSYCp6c5/+L12UUqxfv4jLXbxpnZ6ZS2yewblM8yMllpa9X 0wq0/uRDzYJcvG+ZPACd3H3KCJWAQVYATqPZzoypO9eotm3Gq2y9wluIt3mkImcUHelD dbRye8r2kkKbI3FzSdQ4aIk6IlTpzkIImI45bbUXejSZVCH1OLQd5JixABWI5fXlZgg+ Fn+Q== X-Gm-Message-State: AOAM532/eKavn0iXAKUMC7qeztsC7+lHOxQihzPwwv/wkAzdKe5unolW BygiKhSM8ZwyD9lad17ssTDp2GfjfsvRsH0I X-Google-Smtp-Source: ABdhPJxrrooSvciRVg7K3SAmvB8fttkHSkt3WyNcbUqpdymDo+Oez/mRKN1gcr/enypf7LZSsdkvoA== X-Received: by 2002:a7b:c8c8:: with SMTP id f8mr14886473wml.49.1636538887596; Wed, 10 Nov 2021 02:08:07 -0800 (PST) Received: from naush-laptop.pitowers.org ([2a00:1098:3142:14:8747:6ef9:ee17:3d59]) by smtp.gmail.com with ESMTPSA id d11sm21969133wrs.38.2021.11.10.02.08.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Nov 2021 02:08:07 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Wed, 10 Nov 2021 10:08:01 +0000 Message-Id: <20211110100802.349623-2-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211110100802.349623-1-naush@raspberrypi.com> References: <20211110100802.349623-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/3] pipeline: raspberrypi: Rework the internal buffer allocation scheme 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: , Cc: Roman Stratiienko Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" For simplicity, the pipeline handler would look at the maximum number of buffers set in the StreamConfiguration and allocate the same number of internal buffers for all device nodes. This would likely overallocate buffers for some nodes. Rework this logic to try and minimise overallcations without compromising performance. For ISP nodes, we only ever need 1 set of internal buffers, as the hardware runs synchronous with the requests and IPA. For Unicam nodes, allocate a minimum for 4 buffers (exported + internal), but also require at least 1 internal buffer. Signed-off-by: Naushir Patuck Reviewed-by: Kieran Bingham Reviewed-by: Umang Jain --- .../pipeline/raspberrypi/raspberrypi.cpp | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 11d3c2b120dd..5f0f00aacd59 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1211,21 +1211,43 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera) int PipelineHandlerRPi::prepareBuffers(Camera *camera) { RPiCameraData *data = cameraData(camera); + unsigned int numBuffers; + bool rawStream = false; int ret; - /* - * Decide how many internal buffers to allocate. For now, simply look - * at how many external buffers will be provided. We'll need to improve - * this logic. However, we really must have all streams allocate the same - * number of buffers to simplify error handling in queueRequestDevice(). - */ - unsigned int maxBuffers = 0; - for (const Stream *s : camera->streams()) - if (static_cast(s)->isExternal()) - maxBuffers = std::max(maxBuffers, s->configuration().bufferCount); + for (Stream *s : camera->streams()) { + if (isRaw(s->configuration().pixelFormat)) { + numBuffers = s->configuration().bufferCount; + rawStream = true; + break; + } + } + /* Decide how many internal buffers to allocate. */ for (auto const stream : data->streams_) { - ret = stream->prepareBuffers(maxBuffers); + if (stream == &data->unicam_[Unicam::Image] || + stream == &data->unicam_[Unicam::Embedded]) { + /* + * For Unicam, allocate a minimum of 4 buffers as we want + * to avoid any frame drops. If an application has configured + * a RAW stream, allocate additional buffers to make up the + * minimum, but ensure we have at least 1 set of internal + * buffers to use to minimise frame drops. + */ + constexpr unsigned int minBuffers = 4; + numBuffers = rawStream ? std::max(1, minBuffers - numBuffers) + : minBuffers; + } else { + /* + * Since the ISP runs synchronous with the IPA and requests, + * we only ever need one set of internal buffers. Any buffers + * the application wants to hold onto will already be exported + * through PipelineHandlerRPi::exportFrameBuffers(). + */ + numBuffers = 1; + } + + ret = stream->prepareBuffers(numBuffers); if (ret < 0) return ret; }