From patchwork Fri Nov 12 10:03:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 14588 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 00F40BF415 for ; Fri, 12 Nov 2021 10:03:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B0B4B60362; Fri, 12 Nov 2021 11:03:52 +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="lkk8ZtOs"; dkim-atps=neutral Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7AA6060234 for ; Fri, 12 Nov 2021 11:03:51 +0100 (CET) Received: by mail-wr1-x429.google.com with SMTP id n29so14477840wra.11 for ; Fri, 12 Nov 2021 02:03:51 -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=P0NCeHwS7qg46ZoOFp+cM5MU1LaBQjc7ALSHiOuZ6cs=; b=lkk8ZtOsWFlyed0y5zMEU4v43p4KPuZAX+ovlMcnQM0yqsF7HKsYuHgtU1ZHDfwtLM ypxUyAbcnvnxGq31kPh70NGkY08gorEm2qHtFurDl3oBqdKAevvH/Dac/DDfVOL7kwIG VDeoOU58LOiC0G1KuVHocVGsw2JJDC574/UYPIOuDQDXlXhr6jTZqsezUydwOtFL1L/C uIwiPPPh862fkRYQspJhPpVmqtqtejUyEO927IAgn4ldPTcuwbDzzIWIJOKxOzfJDIZR ege5h5D8siA5jyZvzcFgUfaRPCbe+8G49PiNdieNcKS+vI3h/Ycj8Q4kdbQjwcTSmjEN up5g== 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=P0NCeHwS7qg46ZoOFp+cM5MU1LaBQjc7ALSHiOuZ6cs=; b=v5oVqL1i0aC5Ikkz0lbh+nO+TgT57hPXd/q2HudwPWr9kcI8M8XopsY+M/m16cWlSP uqq4pqpimahR/qdIXzmyydptAwsX714sgxY8hm7XuRt0ppuBIeJYDK230yxeBAtJU853 EcV74Eo8m138b+KFYGTdiE+ssFsQZTHBY+jcynvlZrAOjfK9MQHBlokoC3eaAyvriOxb ohFgL7lGp2G0mbecuXPrRAOCwoUjT37nuQSHy6C1v0BbdSPFOiDTC/fIbQVucNAt1Rf3 EX+AvT/C/N/e2QWI4UPkOvNsZb5Va0KZw85xabjqP+lPh8XsUusVBgWedkfmQE2B/7GZ Vz8g== X-Gm-Message-State: AOAM532VreweANCI1AQpChmSON6JMG/akzeb+18Xl0byP3uEyctOzV5k lnAVQTovEVdzJ32WAW3J+LnUlCiTEmZrv4Fo X-Google-Smtp-Source: ABdhPJzDRXbhTyBpQFbfuTyqkWwC0nnDPr/sTstNgBk3/2lj/s7MOa66nDzr1sUqVqRaakNZuAwMrg== X-Received: by 2002:a5d:6c6b:: with SMTP id r11mr17054706wrz.231.1636711430994; Fri, 12 Nov 2021 02:03:50 -0800 (PST) Received: from naush-laptop.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id o9sm5283155wrs.4.2021.11.12.02.03.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Nov 2021 02:03:50 -0800 (PST) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 12 Nov 2021 10:03:04 +0000 Message-Id: <20211112100305.2217099-3-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211112100305.2217099-1-naush@raspberrypi.com> References: <20211112100305.2217099-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 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 currently look at the maximum number of buffers set in the StreamConfiguration by the user 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. The key change is to mostly decouple the number of internal buffers allocated from number of buffers requested by the user through the StreamConfiguration. 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 2 internal buffers to minimise frame drops. Signed-off-by: Naushir Patuck Reviewed-by: Kieran Bingham Reviewed-by: Umang Jain Reviewed-by: Laurent Pinchart --- .../pipeline/raspberrypi/raspberrypi.cpp | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 11d3c2b120dd..4f6c699a4379 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1211,21 +1211,42 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera) int PipelineHandlerRPi::prepareBuffers(Camera *camera) { RPiCameraData *data = cameraData(camera); + unsigned int numRawBuffers = 0; 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)) { + numRawBuffers = s->configuration().bufferCount; + break; + } + } + /* Decide how many internal buffers to allocate. */ for (auto const stream : data->streams_) { - ret = stream->prepareBuffers(maxBuffers); + unsigned int numBuffers; + + 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 2 sets of internal + * buffers to use to minimise frame drops. + */ + constexpr unsigned int minBuffers = 4; + numBuffers = std::max(2, minBuffers - numRawBuffers); + } 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; }