From patchwork Fri Apr 9 21:38:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 11883 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 24EEDBD16B for ; Fri, 9 Apr 2021 21:38:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E7DC3687F5; Fri, 9 Apr 2021 23:38:49 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 10F4E687F7 for ; Fri, 9 Apr 2021 23:38:48 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fcbb:7aa5:bea8:667e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 124991F46A99; Fri, 9 Apr 2021 22:38:45 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Fri, 9 Apr 2021 18:38:13 -0300 Message-Id: <20210409213815.356837-2-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210409213815.356837-1-nfraprado@collabora.com> References: <20210409213815.356837-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 1/3] lc-compliance: Test queuing more requests than hardware depth 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: Collabora Kernel ML Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Currently some pipeline handlers allocate a fixed number of buffers for internal usage, and if no internal buffer is available when a request is received, it fails [1]. Extend the current lc-compliance tests so they also test sending more simultaneous requests in order to detect that issue. [1] https://bugs.libcamera.org/show_bug.cgi?id=24 Signed-off-by: Nícolas F. R. A. Prado --- src/lc-compliance/simple_capture.cpp | 25 +++++++++++++++------- src/lc-compliance/simple_capture.h | 2 +- src/lc-compliance/single_stream.cpp | 31 ++++++++++++++++------------ 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp index 0ff720bfd005..61a8c0f40eef 100644 --- a/src/lc-compliance/simple_capture.cpp +++ b/src/lc-compliance/simple_capture.cpp @@ -18,10 +18,13 @@ SimpleCapture::~SimpleCapture() { } -Results::Result SimpleCapture::configure(StreamRole role) +Results::Result SimpleCapture::configure(StreamRole role, unsigned int numBuffers) { config_ = camera_->generateConfiguration({ role }); + if (numBuffers > 0) + config_->at(0).bufferCount = numBuffers; + if (config_->validate() != CameraConfiguration::Valid) { config_.reset(); return { Results::Fail, "Configuration not valid" }; @@ -32,6 +35,10 @@ Results::Result SimpleCapture::configure(StreamRole role) return { Results::Fail, "Failed to configure camera" }; } + if (numBuffers > 0 && config_->at(0).bufferCount != numBuffers) + return { Results::Skip, + "Pipeline doesn't support overriding bufferCount" }; + return { Results::Pass, "Configure camera" }; } @@ -77,14 +84,14 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) Stream *stream = config_->at(0).stream(); const std::vector> &buffers = allocator_->buffers(stream); + /* Cache buffers.size() before we destroy it in stop() */ + unsigned int bufSize = buffers.size(); /* No point in testing less requests then the camera depth. */ - if (buffers.size() > numRequests) { - /* Cache buffers.size() before we destroy it in stop() */ - int buffers_size = buffers.size(); + if (bufSize > numRequests) { stop(); - return { Results::Skip, "Camera needs " + std::to_string(buffers_size) + return { Results::Skip, "Camera needs " + std::to_string(bufSize) + " requests, can't test only " + std::to_string(numRequests) }; } @@ -125,7 +132,8 @@ Results::Result SimpleCaptureBalanced::capture(unsigned int numRequests) " request, wanted " + std::to_string(captureLimit_) }; return { Results::Pass, "Balanced capture of " + - std::to_string(numRequests) + " requests" }; + std::to_string(numRequests) + " requests, using " + + std::to_string(bufSize) + " buffers"}; } int SimpleCaptureBalanced::queueRequest(Request *request) @@ -197,7 +205,10 @@ Results::Result SimpleCaptureUnbalanced::capture(unsigned int numRequests) stop(); delete loop_; - return { status ? Results::Fail : Results::Pass, "Unbalanced capture of " + std::to_string(numRequests) + " requests" }; + return { status ? Results::Fail : + Results::Pass, "Unbalanced capture of " + + std::to_string(numRequests) + " requests, using " + + std::to_string(buffers.size()) + " buffers"}; } void SimpleCaptureUnbalanced::requestComplete(Request *request) diff --git a/src/lc-compliance/simple_capture.h b/src/lc-compliance/simple_capture.h index 4693c13404ce..f96fb3224290 100644 --- a/src/lc-compliance/simple_capture.h +++ b/src/lc-compliance/simple_capture.h @@ -17,7 +17,7 @@ class SimpleCapture { public: - Results::Result configure(libcamera::StreamRole role); + Results::Result configure(libcamera::StreamRole role, unsigned int numBuffers); protected: SimpleCapture(std::shared_ptr camera); diff --git a/src/lc-compliance/single_stream.cpp b/src/lc-compliance/single_stream.cpp index 60dacd63bf2b..9e33e231f4a9 100644 --- a/src/lc-compliance/single_stream.cpp +++ b/src/lc-compliance/single_stream.cpp @@ -14,11 +14,11 @@ using namespace libcamera; Results::Result testRequestBalance(std::shared_ptr camera, StreamRole role, unsigned int startCycles, - unsigned int numRequests) + unsigned int numRequests, unsigned int numBuffers) { SimpleCaptureBalanced capture(camera); - Results::Result ret = capture.configure(role); + Results::Result ret = capture.configure(role, numBuffers); if (ret.first != Results::Pass) return ret; @@ -28,17 +28,17 @@ Results::Result testRequestBalance(std::shared_ptr camera, return ret; } - return { Results::Pass, "Balanced capture of " + - std::to_string(numRequests) + " requests with " + + return { Results::Pass, ret.second + " and " + std::to_string(startCycles) + " start cycles" }; } Results::Result testRequestUnbalance(std::shared_ptr camera, - StreamRole role, unsigned int numRequests) + StreamRole role, unsigned int numRequests, + unsigned int numBuffers) { SimpleCaptureUnbalanced capture(camera); - Results::Result ret = capture.configure(role); + Results::Result ret = capture.configure(role, numBuffers); if (ret.first != Results::Pass) return ret; @@ -54,8 +54,10 @@ Results testSingleStream(std::shared_ptr camera) { "viewfinder", Viewfinder }, }; static const std::vector numRequests = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 }; + /* 0 means default */ + static const std::vector numBuffers = { 0, 5, 8 }; - Results results(numRequests.size() * roles.size() * 3); + Results results(numRequests.size() * roles.size() * numBuffers.size() * 3); for (const auto &role : roles) { std::cout << "= Test role " << role.first << std::endl; @@ -67,8 +69,9 @@ Results testSingleStream(std::shared_ptr camera) * complete N requests to the application. */ std::cout << "* Test single capture cycles" << std::endl; - for (unsigned int num : numRequests) - results.add(testRequestBalance(camera, role.second, 1, num)); + for (unsigned int buf : numBuffers) + for (unsigned int num : numRequests) + results.add(testRequestBalance(camera, role.second, 1, num, buf)); /* * Test multiple start/stop cycles @@ -78,8 +81,9 @@ Results testSingleStream(std::shared_ptr camera) * error path but is only tested by single-capture applications. */ std::cout << "* Test multiple start/stop cycles" << std::endl; - for (unsigned int num : numRequests) - results.add(testRequestBalance(camera, role.second, 3, num)); + for (unsigned int buf : numBuffers) + for (unsigned int num : numRequests) + results.add(testRequestBalance(camera, role.second, 3, num, buf)); /* * Test unbalanced stop @@ -89,8 +93,9 @@ Results testSingleStream(std::shared_ptr camera) * of buffers coming back from the video device while stopping. */ std::cout << "* Test unbalanced stop" << std::endl; - for (unsigned int num : numRequests) - results.add(testRequestUnbalance(camera, role.second, num)); + for (unsigned int buf : numBuffers) + for (unsigned int num : numRequests) + results.add(testRequestUnbalance(camera, role.second, num, buf)); } return results; From patchwork Fri Apr 9 21:38:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 11884 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 599E1BD16B for ; Fri, 9 Apr 2021 21:38:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1DA9368801; Fri, 9 Apr 2021 23:38:51 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 500CC687EF for ; Fri, 9 Apr 2021 23:38:50 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fcbb:7aa5:bea8:667e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 5E6B51F46A97; Fri, 9 Apr 2021 22:38:48 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Fri, 9 Apr 2021 18:38:14 -0300 Message-Id: <20210409213815.356837-3-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210409213815.356837-1-nfraprado@collabora.com> References: <20210409213815.356837-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 2/3] libcamera: pipeline: rkisp1: Allow bufferCount to be user-configurable 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: Collabora Kernel ML Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Allow the StreamConfiguration's bufferCount to be changed by the user. This allows the user to, after increasing bufferCount, allocate more buffers through FrameBufferAllocator. Signed-off-by: Nícolas F. R. A. Prado --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 12 +++++++----- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 ++-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 549f4a4e61a8..c7b93b2804c7 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -792,7 +792,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->mainPath_->isEnabled()) { - ret = mainPath_.start(); + ret = mainPath_.start(data->mainPathStream_.configuration()); if (ret) { param_->streamOff(); stat_->streamOff(); @@ -803,7 +803,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->selfPath_->isEnabled()) { - ret = selfPath_.start(); + ret = selfPath_.start(data->selfPathStream_.configuration()); if (ret) { mainPath_.stop(); param_->streamOff(); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 25f482eb8d8e..a03b5c23b87e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -61,7 +61,7 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution) StreamConfiguration cfg(formats); cfg.pixelFormat = formats::NV12; cfg.size = maxResolution; - cfg.bufferCount = RKISP1_BUFFER_COUNT; + cfg.bufferCount = RKISP1_MIN_BUFFER_COUNT; return cfg; } @@ -77,7 +77,10 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg) cfg->size.boundTo(maxResolution_); cfg->size.expandTo(minResolution_); - cfg->bufferCount = RKISP1_BUFFER_COUNT; + if (cfg->bufferCount < RKISP1_MIN_BUFFER_COUNT) { + cfg->bufferCount = RKISP1_MIN_BUFFER_COUNT; + status = CameraConfiguration::Adjusted; + } V4L2DeviceFormat format; format.fourcc = video_->toV4L2PixelFormat(cfg->pixelFormat); @@ -164,15 +167,14 @@ int RkISP1Path::configure(const StreamConfiguration &config, return 0; } -int RkISP1Path::start() +int RkISP1Path::start(const StreamConfiguration &config) { int ret; if (running_) return -EBUSY; - /* \todo Make buffer count user configurable. */ - ret = video_->importBuffers(RKISP1_BUFFER_COUNT); + ret = video_->importBuffers(config.bufferCount); if (ret) return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 3b3e37d258d0..13291da89dc5 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -49,14 +49,14 @@ public: return video_->exportBuffers(bufferCount, buffers); } - int start(); + int start(const StreamConfiguration &config); void stop(); int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); } Signal &bufferReady() { return video_->bufferReady; } private: - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; + static constexpr unsigned int RKISP1_MIN_BUFFER_COUNT = 4; const char *name_; bool running_; From patchwork Fri Apr 9 21:38:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= X-Patchwork-Id: 11885 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 8430BBD16B for ; Fri, 9 Apr 2021 21:38:53 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5229F68800; Fri, 9 Apr 2021 23:38:53 +0200 (CEST) Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AFB84687EF for ; Fri, 9 Apr 2021 23:38:52 +0200 (CEST) Received: from localhost.localdomain (unknown [IPv6:2804:14c:1a9:2978:fcbb:7aa5:bea8:667e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: nfraprado) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 87F9A1F46AB2; Fri, 9 Apr 2021 22:38:50 +0100 (BST) From: =?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= To: libcamera-devel@lists.libcamera.org Date: Fri, 9 Apr 2021 18:38:15 -0300 Message-Id: <20210409213815.356837-4-nfraprado@collabora.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210409213815.356837-1-nfraprado@collabora.com> References: <20210409213815.356837-1-nfraprado@collabora.com> MIME-Version: 1.0 Subject: [libcamera-devel] [RFC PATCH 3/3] libcamera: pipeline: rkisp1: Make fixed amount of internal buffer allocation more explicit 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: Collabora Kernel ML Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Now that bufferCount can be increased and there is a lc-compliance test in place to check if the pipeline can handle more requests than it has internal buffers, we need to be able to test it. Make the internal buffer allocation always constant, so that by increasing bufferCount we can simulate this scenario. Scaling the internal buffers amount with bufferCount only hides the issue, that should be handled by queueing the overflowing requests. Signed-off-by: Nícolas F. R. A. Prado --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++------- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 ++-- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c7b93b2804c7..0dc474f343fc 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -685,16 +685,11 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) unsigned int ipaBufferId = 1; int ret; - unsigned int maxCount = std::max({ - data->mainPathStream_.configuration().bufferCount, - data->selfPathStream_.configuration().bufferCount, - }); - - ret = param_->allocateBuffers(maxCount, ¶mBuffers_); + ret = param_->allocateBuffers(RKISP1_MIN_BUFFER_COUNT, ¶mBuffers_); if (ret < 0) goto error; - ret = stat_->allocateBuffers(maxCount, &statBuffers_); + ret = stat_->allocateBuffers(RKISP1_MIN_BUFFER_COUNT, &statBuffers_); if (ret < 0) goto error; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 13291da89dc5..4ab4c0516ebc 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -21,6 +21,8 @@ namespace libcamera { +static constexpr unsigned int RKISP1_MIN_BUFFER_COUNT = 4; + class MediaDevice; class V4L2Subdevice; struct StreamConfiguration; @@ -56,8 +58,6 @@ public: Signal &bufferReady() { return video_->bufferReady; } private: - static constexpr unsigned int RKISP1_MIN_BUFFER_COUNT = 4; - const char *name_; bool running_;