From patchwork Tue Jun 23 02:09:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4128 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9962E603BF for ; Tue, 23 Jun 2020 04:09:43 +0200 (CEST) X-Halon-ID: 99e4d2a0-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 99e4d2a0-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:42 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:21 +0200 Message-Id: <20200623020930.1781469-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 01/10] libcamera: camera_sensor: Make mbusCodes() return a set 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-List-Received-Date: Tue, 23 Jun 2020 02:09:44 -0000 It makes little sens for a CameraSensor to support the same media bus format multiple times, change the container from a vector<> to a set<>. This was discovered while working with the IPU3 pipeline handler that Unnecessarily had to carry code to sort vectors in order to find intersections. As this change requires switching the container to set<> in that pipeline handler as well fix this while we are at it. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/internal/camera_sensor.h | 4 ++-- include/libcamera/internal/formats.h | 2 +- src/libcamera/camera_sensor.cpp | 1 - src/libcamera/formats.cpp | 7 +++---- src/libcamera/pipeline/ipu3/ipu3.cpp | 13 +++++-------- test/camera-sensor.cpp | 2 +- 6 files changed, 12 insertions(+), 17 deletions(-) diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index 7f07413f95602881..6d92818f16e4b752 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -50,7 +50,7 @@ public: const std::string &model() const { return model_; } const MediaEntity *entity() const { return entity_; } - const std::vector &mbusCodes() const { return mbusCodes_; } + const std::set &mbusCodes() const { return mbusCodes_; } const std::vector &sizes() const { return sizes_; } const Size &resolution() const { return resolution_; } @@ -77,7 +77,7 @@ private: ImageFormats formats_; Size resolution_; - std::vector mbusCodes_; + std::set mbusCodes_; std::vector sizes_; ControlList properties_; diff --git a/include/libcamera/internal/formats.h b/include/libcamera/internal/formats.h index 4b172efc6588d374..e8aa555dcbe99350 100644 --- a/include/libcamera/internal/formats.h +++ b/include/libcamera/internal/formats.h @@ -24,7 +24,7 @@ public: int addFormat(unsigned int format, const std::vector &sizes); bool isEmpty() const; - std::vector formats() const; + std::set formats() const; const std::vector &sizes(unsigned int format) const; const std::map> &data() const; diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index b14b4051dca606b9..5ab50de660534f3f 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -251,7 +251,6 @@ int CameraSensor::init() } mbusCodes_ = formats_.formats(); - std::sort(mbusCodes_.begin(), mbusCodes_.end()); for (const auto &format : formats_.data()) { const std::vector &ranges = format.second; diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp index 1272de29c802c539..6234fd98f3e2002c 100644 --- a/src/libcamera/formats.cpp +++ b/src/libcamera/formats.cpp @@ -68,14 +68,13 @@ bool ImageFormats::isEmpty() const * \brief Retrieve a list of all supported image formats * \return List of pixel formats or media bus codes */ -std::vector ImageFormats::formats() const +std::set ImageFormats::formats() const { - std::vector formats; - formats.reserve(data_.size()); + std::set formats; /* \todo: Should this be cached instead of computed each time? */ for (auto const &it : data_) - formats.push_back(it.first); + formats.insert(it.first); return formats; } diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 5544288b14f739c0..1a59de0c58975b3c 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1441,15 +1441,12 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) /* * Make sure the sensor produces at least one format compatible with * the CIO2 requirements. - * - * utils::set_overlap requires the ranges to be sorted, keep the - * cio2Codes vector sorted in ascending order. */ - const std::vector cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }; - const std::vector &sensorCodes = sensor_->mbusCodes(); + const std::set cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10 }; + const std::set &sensorCodes = sensor_->mbusCodes(); if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(), cio2Codes.begin(), cio2Codes.end())) { LOG(IPU3, Error) diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp index 8c7fd1d2d4445db3..146b1b7d6c294fdb 100644 --- a/test/camera-sensor.cpp +++ b/test/camera-sensor.cpp @@ -67,7 +67,7 @@ protected: return TestFail; } - const std::vector &codes = sensor_->mbusCodes(); + const std::set &codes = sensor_->mbusCodes(); auto iter = std::find(codes.begin(), codes.end(), MEDIA_BUS_FMT_ARGB8888_1X32); if (iter == codes.end()) { From patchwork Tue Jun 23 02:09:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4129 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A079609B3 for ; Tue, 23 Jun 2020 04:09:44 +0200 (CEST) X-Halon-ID: 9a3f21d2-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9a3f21d2-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:43 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:22 +0200 Message-Id: <20200623020930.1781469-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 02/10] libcamera: ipu3: Fold mediaBusToFormat() into only caller 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-List-Received-Date: Tue, 23 Jun 2020 02:09:44 -0000 Make the code easier to read and refactor. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 38 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 1a59de0c58975b3c..c525e30a5ad35011 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -134,8 +134,6 @@ public: int start(); int stop(); - static V4L2PixelFormat mediaBusToFormat(unsigned int code); - V4L2VideoDevice *output_; V4L2Subdevice *csi2_; CameraSensor *sensor_; @@ -1503,7 +1501,25 @@ int CIO2Device::configure(const Size &size, if (ret) return ret; - outputFormat->fourcc = mediaBusToFormat(sensorFormat.mbus_code); + V4L2PixelFormat v4l2Format; + switch (sensorFormat.mbus_code) { + case MEDIA_BUS_FMT_SBGGR10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); + break; + case MEDIA_BUS_FMT_SGBRG10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); + break; + case MEDIA_BUS_FMT_SGRBG10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); + break; + case MEDIA_BUS_FMT_SRGGB10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); + break; + default: + return -EINVAL; + } + + outputFormat->fourcc = v4l2Format; outputFormat->size = sensorFormat.size; outputFormat->planesCount = 1; @@ -1577,22 +1593,6 @@ int CIO2Device::stop() return output_->streamOff(); } -V4L2PixelFormat CIO2Device::mediaBusToFormat(unsigned int code) -{ - switch (code) { - case MEDIA_BUS_FMT_SBGGR10_1X10: - return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); - case MEDIA_BUS_FMT_SGBRG10_1X10: - return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); - case MEDIA_BUS_FMT_SGRBG10_1X10: - return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); - case MEDIA_BUS_FMT_SRGGB10_1X10: - return V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); - default: - return {}; - } -} - REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3); } /* namespace libcamera */ From patchwork Tue Jun 23 02:09:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4130 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DA44B609D6 for ; Tue, 23 Jun 2020 04:09:44 +0200 (CEST) X-Halon-ID: 9a9076d0-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9a9076d0-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:43 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:23 +0200 Message-Id: <20200623020930.1781469-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 03/10] libcamera: ipu3: Breakout stream assignment to new function 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-List-Received-Date: Tue, 23 Jun 2020 02:09:46 -0000 Selecting which stream is the most suitable for the requested configuration is mixed with adjusting the requested format when validating configurations. This is hard to read and got worse when support for Bayer formats was added. Break it out to a separate function. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- * Changes since v2 - Document the fact that the number of requested streams in config_ should be validated before calling assignStreams(). Add an ASSERT() and comment to catch the issue early if this should ever change. * Changes since v1 - Update commit message. - Rename updateStreams() to assignStreams(). - Revert and keep old comment on how streams are picked. - Do not modify behavior on how streams are picked which means assignStreams() now can't fail so no need for it to return an int, switch to void. --- src/libcamera/pipeline/ipu3/ipu3.cpp | 88 ++++++++++++++++------------ 1 file changed, 52 insertions(+), 36 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index c525e30a5ad35011..3b2ec684244881e5 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -191,6 +191,7 @@ private: static constexpr unsigned int IPU3_BUFFER_COUNT = 4; static constexpr unsigned int IPU3_MAX_STREAMS = 3; + void assignStreams(); void adjustStream(StreamConfiguration &cfg, bool scale); /* @@ -257,6 +258,50 @@ IPU3CameraConfiguration::IPU3CameraConfiguration(Camera *camera, data_ = data; } +void IPU3CameraConfiguration::assignStreams() +{ + /* + * Verify and update all configuration entries, and assign a stream to + * each of them. The viewfinder stream can scale, while the output + * stream can crop only, so select the output stream when the requested + * resolution is equal to the sensor resolution, and the viewfinder + * stream otherwise. + */ + std::set availableStreams = { + &data_->outStream_, + &data_->vfStream_, + &data_->rawStream_, + }; + + /* + * The caller is responsible to limit the number of requested streams + * to a number supported by the pipeline before calling this function. + */ + ASSERT(availableStreams.size() >= config_.size()); + + streams_.clear(); + streams_.reserve(config_.size()); + + for (const StreamConfiguration &cfg : config_) { + const PixelFormatInfo &info = + PixelFormatInfo::info(cfg.pixelFormat); + const IPU3Stream *stream; + + if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) + stream = &data_->rawStream_; + else if (cfg.size == sensorFormat_.size) + stream = &data_->outStream_; + else + stream = &data_->vfStream_; + + if (availableStreams.find(stream) == availableStreams.end()) + stream = *availableStreams.begin(); + + streams_.push_back(stream); + availableStreams.erase(stream); + } +} + void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) { /* The only pixel format the driver supports is NV12. */ @@ -343,41 +388,14 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() if (!sensorFormat_.size.width || !sensorFormat_.size.height) sensorFormat_.size = sensor->resolution(); - /* - * Verify and update all configuration entries, and assign a stream to - * each of them. The viewfinder stream can scale, while the output - * stream can crop only, so select the output stream when the requested - * resolution is equal to the sensor resolution, and the viewfinder - * stream otherwise. - */ - std::set availableStreams = { - &data_->outStream_, - &data_->vfStream_, - &data_->rawStream_, - }; - - streams_.clear(); - streams_.reserve(config_.size()); + /* Assign streams to each configuration entry. */ + assignStreams(); + /* Verify and adjust configuration if needed. */ for (unsigned int i = 0; i < config_.size(); ++i) { StreamConfiguration &cfg = config_[i]; - const PixelFormat pixelFormat = cfg.pixelFormat; - const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat); - const Size size = cfg.size; - const IPU3Stream *stream; - - if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) - stream = &data_->rawStream_; - else if (cfg.size == sensorFormat_.size) - stream = &data_->outStream_; - else - stream = &data_->vfStream_; - - if (availableStreams.find(stream) == availableStreams.end()) - stream = *availableStreams.begin(); - - LOG(IPU3, Debug) - << "Assigned '" << stream->name_ << "' to stream " << i; + const StreamConfiguration oldCfg = cfg; + const IPU3Stream *stream = streams_[i]; if (stream->raw_) { const auto &itFormat = @@ -394,15 +412,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() cfg.bufferCount = IPU3_BUFFER_COUNT; - if (cfg.pixelFormat != pixelFormat || cfg.size != size) { + if (cfg.pixelFormat != oldCfg.pixelFormat || + cfg.size != oldCfg.size) { LOG(IPU3, Debug) << "Stream " << i << " configuration adjusted to " << cfg.toString(); status = Adjusted; } - - streams_.push_back(stream); - availableStreams.erase(stream); } return status; From patchwork Tue Jun 23 02:09:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4131 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 59167609C3 for ; Tue, 23 Jun 2020 04:09:45 +0200 (CEST) X-Halon-ID: 9b02fbb7-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9b02fbb7-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:44 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:24 +0200 Message-Id: <20200623020930.1781469-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 04/10] libcamera: ipu3: Calculate number of buffers for ImgU 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-List-Received-Date: Tue, 23 Jun 2020 02:09:47 -0000 Decouple the number of buffers to allocate for the ImgU from the number of buffers allocated for the CIO2. Instead of blindly following the CIO2 pick the maximum number of buffers requested for any stream facing applications. This is potentially wasteful, as each stream could allocate just as many buffers as requested by the application instead of the maximum from the set. But this is not more wasteful than what is already used by the pipeline and should be fixed on top after the decoupling of the two processing units. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v1 - Use std::max({ ... }) --- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 3b2ec684244881e5..7967fde9c39b1547 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -735,7 +735,11 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) if (ret < 0) return ret; - bufferCount = ret; + bufferCount = std::max({ + data->outStream_.configuration().bufferCount, + data->vfStream_.configuration().bufferCount, + data->rawStream_.configuration().bufferCount, + }); ret = imgu->allocateBuffers(data, bufferCount); if (ret < 0) { From patchwork Tue Jun 23 02:09:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4132 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C27C9603BF for ; Tue, 23 Jun 2020 04:09:46 +0200 (CEST) X-Halon-ID: 9b59b4eb-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9b59b4eb-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:45 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:25 +0200 Message-Id: <20200623020930.1781469-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 05/10] libcamera: ipu3: cio2: Move the CIO2Device class to separate files 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-List-Received-Date: Tue, 23 Jun 2020 02:09:47 -0000 In preparation of refactoring the IPU3 pipeline handler breakout the CIO2Device into its own .cpp and .h file, no functional change. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v1 - Remove CIO2Device::mediaBusToFormat() which was unintentionally left in. - Retain 2019 as copyright for new files. - Use forward declaration and only include headers in cio2.cpp --- src/libcamera/pipeline/ipu3/cio2.cpp | 233 ++++++++++++++++++++++ src/libcamera/pipeline/ipu3/cio2.h | 55 ++++++ src/libcamera/pipeline/ipu3/ipu3.cpp | 246 +----------------------- src/libcamera/pipeline/ipu3/meson.build | 1 + 4 files changed, 291 insertions(+), 244 deletions(-) create mode 100644 src/libcamera/pipeline/ipu3/cio2.cpp create mode 100644 src/libcamera/pipeline/ipu3/cio2.h diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp new file mode 100644 index 0000000000000000..dbbcf79c7b2ed5c1 --- /dev/null +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -0,0 +1,233 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * cio2.cpp - Intel IPU3 CIO2 + */ + +#include "cio2.h" + +#include + +#include "libcamera/internal/camera_sensor.h" +#include "libcamera/internal/media_device.h" +#include "libcamera/internal/v4l2_subdevice.h" +#include "libcamera/internal/v4l2_videodevice.h" + +namespace libcamera { + +LOG_DECLARE_CATEGORY(IPU3) + +CIO2Device::CIO2Device() + : output_(nullptr), csi2_(nullptr), sensor_(nullptr) +{ +} + +CIO2Device::~CIO2Device() +{ + delete output_; + delete csi2_; + delete sensor_; +} + +/** + * \brief Initialize components of the CIO2 device with \a index + * \param[in] media The CIO2 media device + * \param[in] index The CIO2 device index + * + * Create and open the video device and subdevices in the CIO2 instance at \a + * index, if a supported image sensor is connected to the CSI-2 receiver of + * this CIO2 instance. Enable the media links connecting the CIO2 components + * to prepare for capture operations and cached the sensor maximum size. + * + * \return 0 on success or a negative error code otherwise + * \retval -ENODEV No supported image sensor is connected to this CIO2 instance + */ +int CIO2Device::init(const MediaDevice *media, unsigned int index) +{ + int ret; + + /* + * Verify that a sensor subdevice is connected to this CIO2 instance + * and enable the media link between the two. + */ + std::string csi2Name = "ipu3-csi2 " + std::to_string(index); + MediaEntity *csi2Entity = media->getEntityByName(csi2Name); + const std::vector &pads = csi2Entity->pads(); + if (pads.empty()) + return -ENODEV; + + /* IPU3 CSI-2 receivers have a single sink pad at index 0. */ + MediaPad *sink = pads[0]; + const std::vector &links = sink->links(); + if (links.empty()) + return -ENODEV; + + MediaLink *link = links[0]; + MediaEntity *sensorEntity = link->source()->entity(); + sensor_ = new CameraSensor(sensorEntity); + ret = sensor_->init(); + if (ret) + return ret; + + ret = link->setEnabled(true); + if (ret) + return ret; + + /* + * Make sure the sensor produces at least one format compatible with + * the CIO2 requirements. + */ + const std::set cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10 }; + const std::set &sensorCodes = sensor_->mbusCodes(); + if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(), + cio2Codes.begin(), cio2Codes.end())) { + LOG(IPU3, Error) + << "Sensor " << sensor_->entity()->name() + << " has not format compatible with the IPU3"; + return -EINVAL; + } + + /* + * \todo Define when to open and close video device nodes, as they + * might impact on power consumption. + */ + + csi2_ = new V4L2Subdevice(csi2Entity); + ret = csi2_->open(); + if (ret) + return ret; + + std::string cio2Name = "ipu3-cio2 " + std::to_string(index); + output_ = V4L2VideoDevice::fromEntityName(media, cio2Name); + ret = output_->open(); + if (ret) + return ret; + + return 0; +} + +/** + * \brief Configure the CIO2 unit + * \param[in] size The requested CIO2 output frame size + * \param[out] outputFormat The CIO2 unit output image format + * \return 0 on success or a negative error code otherwise + */ +int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat) +{ + V4L2SubdeviceFormat sensorFormat; + int ret; + + /* + * Apply the selected format to the sensor, the CSI-2 receiver and + * the CIO2 output device. + */ + sensorFormat = sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10 }, + size); + ret = sensor_->setFormat(&sensorFormat); + if (ret) + return ret; + + ret = csi2_->setFormat(0, &sensorFormat); + if (ret) + return ret; + + V4L2PixelFormat v4l2Format; + switch (sensorFormat.mbus_code) { + case MEDIA_BUS_FMT_SBGGR10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); + break; + case MEDIA_BUS_FMT_SGBRG10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); + break; + case MEDIA_BUS_FMT_SGRBG10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); + break; + case MEDIA_BUS_FMT_SRGGB10_1X10: + v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); + break; + default: + return -EINVAL; + } + + outputFormat->fourcc = v4l2Format; + outputFormat->size = sensorFormat.size; + outputFormat->planesCount = 1; + + ret = output_->setFormat(outputFormat); + if (ret) + return ret; + + LOG(IPU3, Debug) << "CIO2 output format " << outputFormat->toString(); + + return 0; +} + +/** + * \brief Allocate frame buffers for the CIO2 output + * + * Allocate frame buffers in the CIO2 video device to be used to capture frames + * from the CIO2 output. The buffers are stored in the CIO2Device::buffers_ + * vector. + * + * \return Number of buffers allocated or negative error code + */ +int CIO2Device::allocateBuffers() +{ + int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); + if (ret < 0) + return ret; + + for (std::unique_ptr &buffer : buffers_) + availableBuffers_.push(buffer.get()); + + return ret; +} + +void CIO2Device::freeBuffers() +{ + /* The default std::queue constructor is explicit with gcc 5 and 6. */ + availableBuffers_ = std::queue{}; + + buffers_.clear(); + + if (output_->releaseBuffers()) + LOG(IPU3, Error) << "Failed to release CIO2 buffers"; +} + +FrameBuffer *CIO2Device::getBuffer() +{ + if (availableBuffers_.empty()) { + LOG(IPU3, Error) << "CIO2 buffer underrun"; + return nullptr; + } + + FrameBuffer *buffer = availableBuffers_.front(); + + availableBuffers_.pop(); + + return buffer; +} + +void CIO2Device::putBuffer(FrameBuffer *buffer) +{ + availableBuffers_.push(buffer); +} + +int CIO2Device::start() +{ + return output_->streamOn(); +} + +int CIO2Device::stop() +{ + return output_->streamOff(); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h new file mode 100644 index 0000000000000000..b2c4f89d682d6cfb --- /dev/null +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * cio2.h - Intel IPU3 CIO2 + */ +#ifndef __LIBCAMERA_PIPELINE_IPU3_CIO2_H__ +#define __LIBCAMERA_PIPELINE_IPU3_CIO2_H__ + +#include +#include +#include + +namespace libcamera { + +class CameraSensor; +class FrameBuffer; +class MediaDevice; +class V4L2DeviceFormat; +class V4L2Subdevice; +class V4L2VideoDevice; +struct Size; + +class CIO2Device +{ +public: + static constexpr unsigned int CIO2_BUFFER_COUNT = 4; + + CIO2Device(); + ~CIO2Device(); + + int init(const MediaDevice *media, unsigned int index); + int configure(const Size &size, V4L2DeviceFormat *outputFormat); + + int allocateBuffers(); + void freeBuffers(); + + FrameBuffer *getBuffer(); + void putBuffer(FrameBuffer *buffer); + + int start(); + int stop(); + + V4L2VideoDevice *output_; + V4L2Subdevice *csi2_; + CameraSensor *sensor_; + +private: + std::vector> buffers_; + std::queue availableBuffers_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_PIPELINE_IPU3_CIO2_H__ */ diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 7967fde9c39b1547..6e5eb5609a3c2388 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -28,6 +28,8 @@ #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" +#include "cio2.h" + namespace libcamera { LOG_DEFINE_CATEGORY(IPU3) @@ -104,45 +106,6 @@ public: /* \todo Add param video device for 3A tuning */ }; -class CIO2Device -{ -public: - static constexpr unsigned int CIO2_BUFFER_COUNT = 4; - - CIO2Device() - : output_(nullptr), csi2_(nullptr), sensor_(nullptr) - { - } - - ~CIO2Device() - { - delete output_; - delete csi2_; - delete sensor_; - } - - int init(const MediaDevice *media, unsigned int index); - int configure(const Size &size, - V4L2DeviceFormat *outputFormat); - - int allocateBuffers(); - void freeBuffers(); - - FrameBuffer *getBuffer(); - void putBuffer(FrameBuffer *buffer); - - int start(); - int stop(); - - V4L2VideoDevice *output_; - V4L2Subdevice *csi2_; - CameraSensor *sensor_; - -private: - std::vector> buffers_; - std::queue availableBuffers_; -}; - class IPU3Stream : public Stream { public: @@ -1408,211 +1371,6 @@ int ImgUDevice::enableLinks(bool enable) return linkSetup(name_, PAD_STAT, statName, 0, enable); } -/*------------------------------------------------------------------------------ - * CIO2 Device - */ - -/** - * \brief Initialize components of the CIO2 device with \a index - * \param[in] media The CIO2 media device - * \param[in] index The CIO2 device index - * - * Create and open the video device and subdevices in the CIO2 instance at \a - * index, if a supported image sensor is connected to the CSI-2 receiver of - * this CIO2 instance. Enable the media links connecting the CIO2 components - * to prepare for capture operations and cached the sensor maximum size. - * - * \return 0 on success or a negative error code otherwise - * \retval -ENODEV No supported image sensor is connected to this CIO2 instance - */ -int CIO2Device::init(const MediaDevice *media, unsigned int index) -{ - int ret; - - /* - * Verify that a sensor subdevice is connected to this CIO2 instance - * and enable the media link between the two. - */ - std::string csi2Name = "ipu3-csi2 " + std::to_string(index); - MediaEntity *csi2Entity = media->getEntityByName(csi2Name); - const std::vector &pads = csi2Entity->pads(); - if (pads.empty()) - return -ENODEV; - - /* IPU3 CSI-2 receivers have a single sink pad at index 0. */ - MediaPad *sink = pads[0]; - const std::vector &links = sink->links(); - if (links.empty()) - return -ENODEV; - - MediaLink *link = links[0]; - MediaEntity *sensorEntity = link->source()->entity(); - sensor_ = new CameraSensor(sensorEntity); - ret = sensor_->init(); - if (ret) - return ret; - - ret = link->setEnabled(true); - if (ret) - return ret; - - /* - * Make sure the sensor produces at least one format compatible with - * the CIO2 requirements. - */ - const std::set cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }; - const std::set &sensorCodes = sensor_->mbusCodes(); - if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(), - cio2Codes.begin(), cio2Codes.end())) { - LOG(IPU3, Error) - << "Sensor " << sensor_->entity()->name() - << " has not format compatible with the IPU3"; - return -EINVAL; - } - - /* - * \todo Define when to open and close video device nodes, as they - * might impact on power consumption. - */ - - csi2_ = new V4L2Subdevice(csi2Entity); - ret = csi2_->open(); - if (ret) - return ret; - - std::string cio2Name = "ipu3-cio2 " + std::to_string(index); - output_ = V4L2VideoDevice::fromEntityName(media, cio2Name); - ret = output_->open(); - if (ret) - return ret; - - return 0; -} - -/** - * \brief Configure the CIO2 unit - * \param[in] size The requested CIO2 output frame size - * \param[out] outputFormat The CIO2 unit output image format - * \return 0 on success or a negative error code otherwise - */ -int CIO2Device::configure(const Size &size, - V4L2DeviceFormat *outputFormat) -{ - V4L2SubdeviceFormat sensorFormat; - int ret; - - /* - * Apply the selected format to the sensor, the CSI-2 receiver and - * the CIO2 output device. - */ - sensorFormat = sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }, - size); - ret = sensor_->setFormat(&sensorFormat); - if (ret) - return ret; - - ret = csi2_->setFormat(0, &sensorFormat); - if (ret) - return ret; - - V4L2PixelFormat v4l2Format; - switch (sensorFormat.mbus_code) { - case MEDIA_BUS_FMT_SBGGR10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); - break; - case MEDIA_BUS_FMT_SGBRG10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); - break; - case MEDIA_BUS_FMT_SGRBG10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); - break; - case MEDIA_BUS_FMT_SRGGB10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); - break; - default: - return -EINVAL; - } - - outputFormat->fourcc = v4l2Format; - outputFormat->size = sensorFormat.size; - outputFormat->planesCount = 1; - - ret = output_->setFormat(outputFormat); - if (ret) - return ret; - - LOG(IPU3, Debug) << "CIO2 output format " << outputFormat->toString(); - - return 0; -} - -/** - * \brief Allocate frame buffers for the CIO2 output - * - * Allocate frame buffers in the CIO2 video device to be used to capture frames - * from the CIO2 output. The buffers are stored in the CIO2Device::buffers_ - * vector. - * - * \return Number of buffers allocated or negative error code - */ -int CIO2Device::allocateBuffers() -{ - int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); - if (ret < 0) - return ret; - - for (std::unique_ptr &buffer : buffers_) - availableBuffers_.push(buffer.get()); - - return ret; -} - -void CIO2Device::freeBuffers() -{ - /* The default std::queue constructor is explicit with gcc 5 and 6. */ - availableBuffers_ = std::queue{}; - - buffers_.clear(); - - if (output_->releaseBuffers()) - LOG(IPU3, Error) << "Failed to release CIO2 buffers"; -} - -FrameBuffer *CIO2Device::getBuffer() -{ - if (availableBuffers_.empty()) { - LOG(IPU3, Error) << "CIO2 buffer underrun"; - return nullptr; - } - - FrameBuffer *buffer = availableBuffers_.front(); - - availableBuffers_.pop(); - - return buffer; -} - -void CIO2Device::putBuffer(FrameBuffer *buffer) -{ - availableBuffers_.push(buffer); -} - -int CIO2Device::start() -{ - return output_->streamOn(); -} - -int CIO2Device::stop() -{ - return output_->streamOff(); -} - REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3); } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build index 0e8c5a14f2b3317b..b2602d30123f908d 100644 --- a/src/libcamera/pipeline/ipu3/meson.build +++ b/src/libcamera/pipeline/ipu3/meson.build @@ -1,5 +1,6 @@ # SPDX-License-Identifier: CC0-1.0 libcamera_sources += files([ + 'cio2.cpp', 'ipu3.cpp', ]) From patchwork Tue Jun 23 02:09:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4133 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4C8D3609A5 for ; Tue, 23 Jun 2020 04:09:47 +0200 (CEST) X-Halon-ID: 9c2e33f2-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9c2e33f2-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:46 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:26 +0200 Message-Id: <20200623020930.1781469-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 06/10] libcamera: ipu3: cio2: Consolidate information about formats 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-List-Received-Date: Tue, 23 Jun 2020 02:09:47 -0000 Instead of spreading the mapping between media bus codes and V4L2 FourCC all over the CIO2 code collect it in a single map and extract the data from it. This is done in preparation of adding PixelFormat information to the mix. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/cio2.cpp | 50 +++++++++++++--------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index dbbcf79c7b2ed5c1..f23128d412e6b1a5 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -18,6 +18,17 @@ namespace libcamera { LOG_DECLARE_CATEGORY(IPU3) +namespace { + +static const std::map mbusCodesToInfo = { + { MEDIA_BUS_FMT_SBGGR10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) }, + { MEDIA_BUS_FMT_SGBRG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) }, + { MEDIA_BUS_FMT_SGRBG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) }, + { MEDIA_BUS_FMT_SRGGB10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) }, +}; + +} /* namespace */ + CIO2Device::CIO2Device() : output_(nullptr), csi2_(nullptr), sensor_(nullptr) { @@ -78,10 +89,10 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) * Make sure the sensor produces at least one format compatible with * the CIO2 requirements. */ - const std::set cio2Codes{ MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }; + std::set cio2Codes; + std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(), + std::inserter(cio2Codes, cio2Codes.begin()), + [](const std::map::value_type &pair){ return pair.first; }); const std::set &sensorCodes = sensor_->mbusCodes(); if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(), cio2Codes.begin(), cio2Codes.end())) { @@ -125,11 +136,12 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat) * Apply the selected format to the sensor, the CSI-2 receiver and * the CIO2 output device. */ - sensorFormat = sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }, - size); + std::vector mbusCodes; + std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(), + std::back_inserter(mbusCodes), + [](const std::map::value_type &pair){ return pair.first; }); + + sensorFormat = sensor_->getFormat(mbusCodes, size); ret = sensor_->setFormat(&sensorFormat); if (ret) return ret; @@ -138,25 +150,11 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat) if (ret) return ret; - V4L2PixelFormat v4l2Format; - switch (sensorFormat.mbus_code) { - case MEDIA_BUS_FMT_SBGGR10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10); - break; - case MEDIA_BUS_FMT_SGBRG10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10); - break; - case MEDIA_BUS_FMT_SGRBG10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10); - break; - case MEDIA_BUS_FMT_SRGGB10_1X10: - v4l2Format = V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10); - break; - default: + const auto &itInfo = mbusCodesToInfo.find(sensorFormat.mbus_code); + if (itInfo == mbusCodesToInfo.end()) return -EINVAL; - } - outputFormat->fourcc = v4l2Format; + outputFormat->fourcc = itInfo->second; outputFormat->size = sensorFormat.size; outputFormat->planesCount = 1; From patchwork Tue Jun 23 02:09:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4134 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 26170609C2 for ; Tue, 23 Jun 2020 04:09:48 +0200 (CEST) X-Halon-ID: 9c80c80f-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9c80c80f-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:47 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:27 +0200 Message-Id: <20200623020930.1781469-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 07/10] libcamera: ipu3: cio2: Add function to generate configuration 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-List-Received-Date: Tue, 23 Jun 2020 02:09:48 -0000 Collect the code used to generate configurations for the CIO2 block in the CIO2Device class. This allows simplifying the code and allow further changes to only happen at one code location. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v2 - Remove unneeded code to pick sensor FourCC. - Remove desiredPixelFormat from generateConfiguration() as it's not needed. - Rename sensorFormat_ cio2Configuration_ - Consolidate all format information in a single table. * Changes since v1 - Use anonymous namespace instead of static for sensorMbusToPixel map. - Handle case where the requested mbus code is not supported by the sensor. - Update commit message. --- src/libcamera/pipeline/ipu3/cio2.cpp | 54 +++++++++++++++++++++++---- src/libcamera/pipeline/ipu3/cio2.h | 3 ++ src/libcamera/pipeline/ipu3/ipu3.cpp | 56 +++++++--------------------- 3 files changed, 63 insertions(+), 50 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index f23128d412e6b1a5..d6bab896706dd60e 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -9,6 +9,9 @@ #include +#include +#include + #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/v4l2_subdevice.h" @@ -20,11 +23,16 @@ LOG_DECLARE_CATEGORY(IPU3) namespace { -static const std::map mbusCodesToInfo = { - { MEDIA_BUS_FMT_SBGGR10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) }, - { MEDIA_BUS_FMT_SGBRG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) }, - { MEDIA_BUS_FMT_SGRBG10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) }, - { MEDIA_BUS_FMT_SRGGB10_1X10, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) }, +struct MbusInfo { + PixelFormat pixelFormat; + V4L2PixelFormat fourcc; +}; + +static const std::map mbusCodesToInfo = { + { MEDIA_BUS_FMT_SBGGR10_1X10, { formats::SBGGR10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } }, + { MEDIA_BUS_FMT_SGBRG10_1X10, { formats::SGBRG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } }, + { MEDIA_BUS_FMT_SGRBG10_1X10, { formats::SGRBG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } }, + { MEDIA_BUS_FMT_SRGGB10_1X10, { formats::SRGGB10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } }, }; } /* namespace */ @@ -92,7 +100,7 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) std::set cio2Codes; std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(), std::inserter(cio2Codes, cio2Codes.begin()), - [](const std::map::value_type &pair){ return pair.first; }); + [](const std::map::value_type &pair){ return pair.first; }); const std::set &sensorCodes = sensor_->mbusCodes(); if (!utils::set_overlap(sensorCodes.begin(), sensorCodes.end(), cio2Codes.begin(), cio2Codes.end())) { @@ -139,7 +147,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat) std::vector mbusCodes; std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(), std::back_inserter(mbusCodes), - [](const std::map::value_type &pair){ return pair.first; }); + [](const std::map::value_type &pair){ return pair.first; }); sensorFormat = sensor_->getFormat(mbusCodes, size); ret = sensor_->setFormat(&sensorFormat); @@ -154,7 +162,7 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat) if (itInfo == mbusCodesToInfo.end()) return -EINVAL; - outputFormat->fourcc = itInfo->second; + outputFormat->fourcc = itInfo->second.fourcc; outputFormat->size = sensorFormat.size; outputFormat->planesCount = 1; @@ -167,6 +175,36 @@ int CIO2Device::configure(const Size &size, V4L2DeviceFormat *outputFormat) return 0; } +StreamConfiguration +CIO2Device::generateConfiguration(const Size &desiredSize) const +{ + StreamConfiguration cfg; + + /* If no desired size use the sensor resolution. */ + Size size = sensor_->resolution(); + if (desiredSize.width && desiredSize.height) + size = desiredSize; + + /* Query the sensor static information for closest match. */ + std::vector mbusCodes; + std::transform(mbusCodesToInfo.begin(), mbusCodesToInfo.end(), + std::back_inserter(mbusCodes), + [](const std::map::value_type &pair){ return pair.first; }); + + V4L2SubdeviceFormat sensorFormat = sensor_->getFormat(mbusCodes, size); + + if (!sensorFormat.mbus_code) { + LOG(IPU3, Error) << "Sensor does not support mbus code"; + return {}; + } + + cfg.size = sensorFormat.size; + cfg.pixelFormat = mbusCodesToInfo.at(sensorFormat.mbus_code).pixelFormat; + cfg.bufferCount = CIO2_BUFFER_COUNT; + + return cfg; +} + /** * \brief Allocate frame buffers for the CIO2 output * diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index b2c4f89d682d6cfb..6276573f2b585b25 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -20,6 +20,7 @@ class V4L2DeviceFormat; class V4L2Subdevice; class V4L2VideoDevice; struct Size; +struct StreamConfiguration; class CIO2Device { @@ -32,6 +33,8 @@ public: int init(const MediaDevice *media, unsigned int index); int configure(const Size &size, V4L2DeviceFormat *outputFormat); + StreamConfiguration generateConfiguration(const Size &desiredSize) const; + int allocateBuffers(); void freeBuffers(); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 6e5eb5609a3c2388..c0e727e592f46883 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -36,13 +36,6 @@ LOG_DEFINE_CATEGORY(IPU3) class IPU3CameraData; -static const std::map sensorMbusToPixel = { - { MEDIA_BUS_FMT_SBGGR10_1X10, formats::SBGGR10_IPU3 }, - { MEDIA_BUS_FMT_SGBRG10_1X10, formats::SGBRG10_IPU3 }, - { MEDIA_BUS_FMT_SGRBG10_1X10, formats::SGRBG10_IPU3 }, - { MEDIA_BUS_FMT_SRGGB10_1X10, formats::SRGGB10_IPU3 }, -}; - class ImgUDevice { public: @@ -147,7 +140,7 @@ public: Status validate() override; - const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } + const StreamConfiguration &cio2Format() const { return cio2Configuration_; }; const std::vector &streams() { return streams_; } private: @@ -165,7 +158,7 @@ private: std::shared_ptr camera_; const IPU3CameraData *data_; - V4L2SubdeviceFormat sensorFormat_; + StreamConfiguration cio2Configuration_; std::vector streams_; }; @@ -252,7 +245,7 @@ void IPU3CameraConfiguration::assignStreams() if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) stream = &data_->rawStream_; - else if (cfg.size == sensorFormat_.size) + else if (cfg.size == cio2Configuration_.size) stream = &data_->outStream_; else stream = &data_->vfStream_; @@ -277,8 +270,8 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) */ if (!cfg.size.width || !cfg.size.height) { cfg.size.width = 1280; - cfg.size.height = 1280 * sensorFormat_.size.height - / sensorFormat_.size.width; + cfg.size.height = 1280 * cio2Configuration_.size.height + / cio2Configuration_.size.width; } /* @@ -297,7 +290,7 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) * \todo: Properly support cropping when the ImgU driver * interface will be cleaned up. */ - cfg.size = sensorFormat_.size; + cfg.size = cio2Configuration_.size; } /* @@ -313,7 +306,6 @@ void IPU3CameraConfiguration::adjustStream(StreamConfiguration &cfg, bool scale) CameraConfiguration::Status IPU3CameraConfiguration::validate() { - const CameraSensor *sensor = data_->cio2_.sensor_; Status status = Valid; if (config_.empty()) @@ -340,16 +332,10 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() size.height = cfg.size.height; } - if (!size.width || !size.height) - size = sensor->resolution(); - - sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }, - size); - if (!sensorFormat_.size.width || !sensorFormat_.size.height) - sensorFormat_.size = sensor->resolution(); + /* Generate raw configuration from CIO2. */ + cio2Configuration_ = data_->cio2_.generateConfiguration(size); + if (!cio2Configuration_.pixelFormat.isValid()) + return Invalid; /* Assign streams to each configuration entry. */ assignStreams(); @@ -361,20 +347,13 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate() const IPU3Stream *stream = streams_[i]; if (stream->raw_) { - const auto &itFormat = - sensorMbusToPixel.find(sensorFormat_.mbus_code); - if (itFormat == sensorMbusToPixel.end()) - return Invalid; - - cfg.pixelFormat = itFormat->second; - cfg.size = sensorFormat_.size; + cfg = cio2Configuration_; } else { bool scale = stream == &data_->vfStream_; adjustStream(config_[i], scale); + cfg.bufferCount = IPU3_BUFFER_COUNT; } - cfg.bufferCount = IPU3_BUFFER_COUNT; - if (cfg.pixelFormat != oldCfg.pixelFormat || cfg.size != oldCfg.size) { LOG(IPU3, Debug) @@ -454,14 +433,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, cfg.size = data->cio2_.sensor_->resolution(); - V4L2SubdeviceFormat sensorFormat = - data->cio2_.sensor_->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10 }, - cfg.size); - cfg.pixelFormat = - sensorMbusToPixel.at(sensorFormat.mbus_code); + cfg = data->cio2_.generateConfiguration(cfg.size); break; } @@ -575,7 +547,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) * Pass the requested stream size to the CIO2 unit and get back the * adjusted format to be propagated to the ImgU output devices. */ - const Size &sensorSize = config->sensorFormat().size; + const Size &sensorSize = config->cio2Format().size; V4L2DeviceFormat cio2Format = {}; ret = cio2->configure(sensorSize, &cio2Format); if (ret) From patchwork Tue Jun 23 02:09:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4135 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C08A6609C2 for ; Tue, 23 Jun 2020 04:09:48 +0200 (CEST) X-Halon-ID: 9d031aaf-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9d031aaf-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:47 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:28 +0200 Message-Id: <20200623020930.1781469-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 08/10] libcamera: ipu3: cio2: Make the V4L2 devices private 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-List-Received-Date: Tue, 23 Jun 2020 02:09:49 -0000 In order to make the CIO2 easier to extend with new features make the V4L2 devices (sensor, CIO2 and video device) private members. This requires a few helper functions to be added to allow for the IPU3 driver to still be able to interact with all parts of the CIO2. These helper functions will later be extended to add new features to the IPU3 pipeline. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v2 - Style changes. * Changes since v1 - Drop sensor access helpers and replace with a sensor() call to get hold of the CameraSensor pointer directly. --- src/libcamera/pipeline/ipu3/cio2.cpp | 20 +++++++++++++++++++- src/libcamera/pipeline/ipu3/cio2.h | 17 ++++++++++++++--- src/libcamera/pipeline/ipu3/ipu3.cpp | 19 ++++++++----------- 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index d6bab896706dd60e..3d7348782b0fa6cb 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -38,7 +38,7 @@ static const std::map mbusCodesToInfo = { } /* namespace */ CIO2Device::CIO2Device() - : output_(nullptr), csi2_(nullptr), sensor_(nullptr) + : sensor_(nullptr), csi2_(nullptr), output_(nullptr) { } @@ -126,6 +126,8 @@ int CIO2Device::init(const MediaDevice *media, unsigned int index) if (ret) return ret; + output_->bufferReady.connect(this, &CIO2Device::cio2BufferReady); + return 0; } @@ -226,6 +228,12 @@ int CIO2Device::allocateBuffers() return ret; } +int CIO2Device::exportBuffers(unsigned int count, + std::vector> *buffers) +{ + return output_->exportBuffers(count, buffers); +} + void CIO2Device::freeBuffers() { /* The default std::queue constructor is explicit with gcc 5 and 6. */ @@ -266,4 +274,14 @@ int CIO2Device::stop() return output_->streamOff(); } +int CIO2Device::queueBuffer(FrameBuffer *buffer) +{ + return output_->queueBuffer(buffer); +} + +void CIO2Device::cio2BufferReady(FrameBuffer *buffer) +{ + bufferReady.emit(buffer); +} + } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index 6276573f2b585b25..cc898f13fd73f865 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -11,6 +11,8 @@ #include #include +#include + namespace libcamera { class CameraSensor; @@ -36,6 +38,8 @@ public: StreamConfiguration generateConfiguration(const Size &desiredSize) const; int allocateBuffers(); + int exportBuffers(unsigned int count, + std::vector> *buffers); void freeBuffers(); FrameBuffer *getBuffer(); @@ -44,11 +48,18 @@ public: int start(); int stop(); - V4L2VideoDevice *output_; - V4L2Subdevice *csi2_; - CameraSensor *sensor_; + CameraSensor *sensor() { return sensor_; } + + int queueBuffer(FrameBuffer *buffer); + Signal bufferReady; private: + void cio2BufferReady(FrameBuffer *buffer); + + CameraSensor *sensor_; + V4L2Subdevice *csi2_; + V4L2VideoDevice *output_; + std::vector> buffers_; std::queue availableBuffers_; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index c0e727e592f46883..2d1ec707ea4b08fe 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -431,7 +431,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, stream = &data->rawStream_; - cfg.size = data->cio2_.sensor_->resolution(); + cfg.size = data->cio2_.sensor()->resolution(); cfg = data->cio2_.generateConfiguration(cfg.size); break; @@ -460,7 +460,7 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, * available sensor resolution and to the IPU3 * alignment constraints. */ - const Size &res = data->cio2_.sensor_->resolution(); + const Size &res = data->cio2_.sensor()->resolution(); unsigned int width = std::min(1280U, res.width); unsigned int height = std::min(720U, res.height); cfg.size = { width & ~7, height & ~3 }; @@ -640,14 +640,11 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, IPU3CameraData *data = cameraData(camera); IPU3Stream *ipu3stream = static_cast(stream); unsigned int count = stream->configuration().bufferCount; - V4L2VideoDevice *video; if (ipu3stream->raw_) - video = data->cio2_.output_; - else - video = ipu3stream->device_->dev; + return data->cio2_.exportBuffers(count, buffers); - return video->exportBuffers(count, buffers); + return ipu3stream->device_->dev->exportBuffers(count, buffers); } /** @@ -757,7 +754,7 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) return -EINVAL; buffer->setRequest(request); - data->cio2_.output_->queueBuffer(buffer); + data->cio2_.queueBuffer(buffer); for (auto it : request->buffers()) { IPU3Stream *stream = static_cast(it.first); @@ -870,7 +867,7 @@ int PipelineHandlerIPU3::registerCameras() continue; /* Initialize the camera properties. */ - data->properties_ = cio2->sensor_->properties(); + data->properties_ = cio2->sensor()->properties(); /** * \todo Dynamically assign ImgU and output devices to each @@ -894,7 +891,7 @@ int PipelineHandlerIPU3::registerCameras() * associated ImgU input where they get processed and * returned through the ImgU main and secondary outputs. */ - data->cio2_.output_->bufferReady.connect(data.get(), + data->cio2_.bufferReady.connect(data.get(), &IPU3CameraData::cio2BufferReady); data->imgu_->input_->bufferReady.connect(data.get(), &IPU3CameraData::imguInputBufferReady); @@ -904,7 +901,7 @@ int PipelineHandlerIPU3::registerCameras() &IPU3CameraData::imguOutputBufferReady); /* Create and register the Camera instance. */ - std::string cameraName = cio2->sensor_->entity()->name(); + std::string cameraName = cio2->sensor()->entity()->name(); std::shared_ptr camera = Camera::create(this, cameraName, streams); From patchwork Tue Jun 23 02:09:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4136 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 83483603BD for ; Tue, 23 Jun 2020 04:09:49 +0200 (CEST) X-Halon-ID: 9d630613-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9d630613-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:48 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:29 +0200 Message-Id: <20200623020930.1781469-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 09/10] libcamera: ipu3: cio2: Hide buffer allocation and freeing from users 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-List-Received-Date: Tue, 23 Jun 2020 02:09:49 -0000 The allocation and freeing of buffers for the CIO2 is handled in the IPU3 pipeline handlers start() and stop() functions. These functions also call CIO2Device start() and stop() at the appropriate times so move the CIO2 buffer allocation/freeing inside the CIO2Device and reduce the complexity of the exposed interface. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- * Changes since v2 - Stop IMGU before CIO2 * Changes since v1 - Fix potential leaking of buffers if start fails. --- src/libcamera/pipeline/ipu3/cio2.cpp | 63 +++++++++++++--------------- src/libcamera/pipeline/ipu3/cio2.h | 2 - src/libcamera/pipeline/ipu3/ipu3.cpp | 14 ++----- 3 files changed, 32 insertions(+), 47 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index 3d7348782b0fa6cb..efaa460b246697a6 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -207,44 +207,12 @@ CIO2Device::generateConfiguration(const Size &desiredSize) const return cfg; } -/** - * \brief Allocate frame buffers for the CIO2 output - * - * Allocate frame buffers in the CIO2 video device to be used to capture frames - * from the CIO2 output. The buffers are stored in the CIO2Device::buffers_ - * vector. - * - * \return Number of buffers allocated or negative error code - */ -int CIO2Device::allocateBuffers() -{ - int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); - if (ret < 0) - return ret; - - for (std::unique_ptr &buffer : buffers_) - availableBuffers_.push(buffer.get()); - - return ret; -} - int CIO2Device::exportBuffers(unsigned int count, std::vector> *buffers) { return output_->exportBuffers(count, buffers); } -void CIO2Device::freeBuffers() -{ - /* The default std::queue constructor is explicit with gcc 5 and 6. */ - availableBuffers_ = std::queue{}; - - buffers_.clear(); - - if (output_->releaseBuffers()) - LOG(IPU3, Error) << "Failed to release CIO2 buffers"; -} - FrameBuffer *CIO2Device::getBuffer() { if (availableBuffers_.empty()) { @@ -266,12 +234,39 @@ void CIO2Device::putBuffer(FrameBuffer *buffer) int CIO2Device::start() { - return output_->streamOn(); + int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); + if (ret < 0) + return ret; + + for (std::unique_ptr &buffer : buffers_) + availableBuffers_.push(buffer.get()); + + ret = output_->streamOn(); + if (ret) { + /* The default std::queue constructor is explicit with gcc 5 and 6. */ + availableBuffers_ = std::queue{}; + + buffers_.clear(); + + output_->releaseBuffers(); + } + + return ret; } int CIO2Device::stop() { - return output_->streamOff(); + int ret = output_->streamOff(); + + /* The default std::queue constructor is explicit with gcc 5 and 6. */ + availableBuffers_ = std::queue{}; + + buffers_.clear(); + + if (output_->releaseBuffers()) + LOG(IPU3, Error) << "Failed to release CIO2 buffers"; + + return ret; } int CIO2Device::queueBuffer(FrameBuffer *buffer) diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index cc898f13fd73f865..405e6c03755367c4 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -37,10 +37,8 @@ public: StreamConfiguration generateConfiguration(const Size &desiredSize) const; - int allocateBuffers(); int exportBuffers(unsigned int count, std::vector> *buffers); - void freeBuffers(); FrameBuffer *getBuffer(); void putBuffer(FrameBuffer *buffer); diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 2d1ec707ea4b08fe..4818027e8db1f7a3 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -658,15 +658,10 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream, int PipelineHandlerIPU3::allocateBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); - CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; unsigned int bufferCount; int ret; - ret = cio2->allocateBuffers(); - if (ret < 0) - return ret; - bufferCount = std::max({ data->outStream_.configuration().bufferCount, data->vfStream_.configuration().bufferCount, @@ -674,10 +669,8 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera) }); ret = imgu->allocateBuffers(data, bufferCount); - if (ret < 0) { - cio2->freeBuffers(); + if (ret < 0) return ret; - } return 0; } @@ -686,7 +679,6 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera) { IPU3CameraData *data = cameraData(camera); - data->cio2_.freeBuffers(); data->imgu_->freeBuffers(data); return 0; @@ -731,10 +723,10 @@ error: void PipelineHandlerIPU3::stop(Camera *camera) { IPU3CameraData *data = cameraData(camera); - int ret; + int ret = 0; - ret = data->cio2_.stop(); ret |= data->imgu_->stop(); + ret |= data->cio2_.stop(); if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->name(); From patchwork Tue Jun 23 02:09:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 4137 Return-Path: Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 69500603BF for ; Tue, 23 Jun 2020 04:09:50 +0200 (CEST) X-Halon-ID: 9dd3b906-b4f6-11ea-933e-005056917a89 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (p4fca2eca.dip0.t-ipconnect.de [79.202.46.202]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA id 9dd3b906-b4f6-11ea-933e-005056917a89; Tue, 23 Jun 2020 04:09:49 +0200 (CEST) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 23 Jun 2020 04:09:30 +0200 Message-Id: <20200623020930.1781469-11-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> References: <20200623020930.1781469-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 10/10] libcamera: ipu3: Allow zero-copy RAW stream capture 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-List-Received-Date: Tue, 23 Jun 2020 02:09:50 -0000 With the refactored CIO2 interface it's now easy to add zero-copy for buffers in the RAW stream. Use the internally allocated buffers inside the CIO2Device if no buffer for the RAW stream is provided by the application, or use the application-provided buffer if any. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v2 - Add todo of potential common need code. * Changes since v1 - Update comments. - Use Request::findBuffer() instead of own logic. --- src/libcamera/pipeline/ipu3/cio2.cpp | 58 ++++++++++++++--------- src/libcamera/pipeline/ipu3/cio2.h | 7 ++- src/libcamera/pipeline/ipu3/ipu3.cpp | 69 +++++++++------------------- 3 files changed, 61 insertions(+), 73 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp index efaa460b246697a6..ab7f96dad19b0560 100644 --- a/src/libcamera/pipeline/ipu3/cio2.cpp +++ b/src/libcamera/pipeline/ipu3/cio2.cpp @@ -213,31 +213,16 @@ int CIO2Device::exportBuffers(unsigned int count, return output_->exportBuffers(count, buffers); } -FrameBuffer *CIO2Device::getBuffer() -{ - if (availableBuffers_.empty()) { - LOG(IPU3, Error) << "CIO2 buffer underrun"; - return nullptr; - } - - FrameBuffer *buffer = availableBuffers_.front(); - - availableBuffers_.pop(); - - return buffer; -} - -void CIO2Device::putBuffer(FrameBuffer *buffer) -{ - availableBuffers_.push(buffer); -} - int CIO2Device::start() { - int ret = output_->allocateBuffers(CIO2_BUFFER_COUNT, &buffers_); + int ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_); if (ret < 0) return ret; + ret = output_->importBuffers(CIO2_BUFFER_COUNT); + if (ret) + LOG(IPU3, Error) << "Failed to import CIO2 buffers"; + for (std::unique_ptr &buffer : buffers_) availableBuffers_.push(buffer.get()); @@ -269,11 +254,42 @@ int CIO2Device::stop() return ret; } -int CIO2Device::queueBuffer(FrameBuffer *buffer) +int CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer) { + FrameBuffer *buffer = rawBuffer; + + /* If no buffer is provided in the request, use an internal one. */ + if (!buffer) { + if (availableBuffers_.empty()) { + LOG(IPU3, Error) << "CIO2 buffer underrun"; + return -EINVAL; + } + + buffer = availableBuffers_.front(); + availableBuffers_.pop(); + } + + buffer->setRequest(request); + return output_->queueBuffer(buffer); } +void CIO2Device::tryReturnBuffer(FrameBuffer *buffer) +{ + /* + * \todo Once more pipelines deal with buffers that may be allocated + * internally or externally at the this pattern might become a common + * need. At that point this check should be moved to something clever + * in FrameBuffer. + */ + for (const std::unique_ptr &buf : buffers_) { + if (buf.get() == buffer) { + availableBuffers_.push(buffer); + break; + } + } +} + void CIO2Device::cio2BufferReady(FrameBuffer *buffer) { bufferReady.emit(buffer); diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h index 405e6c03755367c4..9f2e8a98af7043eb 100644 --- a/src/libcamera/pipeline/ipu3/cio2.h +++ b/src/libcamera/pipeline/ipu3/cio2.h @@ -18,6 +18,7 @@ namespace libcamera { class CameraSensor; class FrameBuffer; class MediaDevice; +class Request; class V4L2DeviceFormat; class V4L2Subdevice; class V4L2VideoDevice; @@ -40,15 +41,13 @@ public: int exportBuffers(unsigned int count, std::vector> *buffers); - FrameBuffer *getBuffer(); - void putBuffer(FrameBuffer *buffer); - int start(); int stop(); CameraSensor *sensor() { return sensor_; } - int queueBuffer(FrameBuffer *buffer); + int queueBuffer(Request *request, FrameBuffer *rawBuffer); + void tryReturnBuffer(FrameBuffer *buffer); Signal bufferReady; private: diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 4818027e8db1f7a3..6c43169eb0915965 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -122,7 +122,6 @@ public: } void imguOutputBufferReady(FrameBuffer *buffer); - void imguInputBufferReady(FrameBuffer *buffer); void cio2BufferReady(FrameBuffer *buffer); CIO2Device cio2_; @@ -737,25 +736,24 @@ void PipelineHandlerIPU3::stop(Camera *camera) int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request) { IPU3CameraData *data = cameraData(camera); - FrameBuffer *buffer; int error = 0; - /* Get a CIO2 buffer, associate it with the request and queue it. */ - buffer = data->cio2_.getBuffer(); - if (!buffer) - return -EINVAL; - - buffer->setRequest(request); - data->cio2_.queueBuffer(buffer); + /* + * Queue a buffer on the CIO2, using the raw stream buffer provided in + * the request, if any, or a CIO2 internal buffer otherwise. + */ + FrameBuffer *rawBuffer = request->findBuffer(&data->rawStream_); + error = data->cio2_.queueBuffer(request, rawBuffer); + if (error) + return error; + /* Queue all buffers from the request aimed for the ImgU. */ for (auto it : request->buffers()) { IPU3Stream *stream = static_cast(it.first); - buffer = it.second; - - /* Skip raw streams, they are copied from the CIO2 buffer. */ if (stream->raw_) continue; + FrameBuffer *buffer = it.second; int ret = stream->device_->dev->queueBuffer(buffer); if (ret < 0) error = ret; @@ -885,8 +883,8 @@ int PipelineHandlerIPU3::registerCameras() */ data->cio2_.bufferReady.connect(data.get(), &IPU3CameraData::cio2BufferReady); - data->imgu_->input_->bufferReady.connect(data.get(), - &IPU3CameraData::imguInputBufferReady); + data->imgu_->input_->bufferReady.connect(&data->cio2_, + &CIO2Device::tryReturnBuffer); data->imgu_->output_.dev->bufferReady.connect(data.get(), &IPU3CameraData::imguOutputBufferReady); data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(), @@ -915,22 +913,6 @@ int PipelineHandlerIPU3::registerCameras() * Buffer Ready slots */ -/** - * \brief Handle buffers completion at the ImgU input - * \param[in] buffer The completed buffer - * - * Buffers completed from the ImgU input are immediately queued back to the - * CIO2 unit to continue frame capture. - */ -void IPU3CameraData::imguInputBufferReady(FrameBuffer *buffer) -{ - /* \todo Handle buffer failures when state is set to BufferError. */ - if (buffer->metadata().status == FrameMetadata::FrameCancelled) - return; - - cio2_.putBuffer(buffer); -} - /** * \brief Handle buffers completion at the ImgU output * \param[in] buffer The completed buffer @@ -963,27 +945,18 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) return; Request *request = buffer->request(); - FrameBuffer *raw = request->findBuffer(&rawStream_); - if (!raw) { - /* No RAW buffers present, just queue to IMGU. */ - imgu_->input_->queueBuffer(buffer); - return; - } - - /* RAW buffers present, special care is needed. */ - if (request->buffers().size() > 1) - imgu_->input_->queueBuffer(buffer); - - if (raw->copyFrom(buffer)) - LOG(IPU3, Debug) << "Copy of FrameBuffer failed"; - - pipe_->completeBuffer(camera_, request, raw); - - if (request->buffers().size() == 1) { - cio2_.putBuffer(buffer); + /* + * If the request contains a buffer for the RAW stream only, complete it + * now as there's no need for ImgU processing. + */ + if (request->findBuffer(&rawStream_) && + pipe_->completeBuffer(camera_, request, buffer)) { pipe_->completeRequest(camera_, request); + return; } + + imgu_->input_->queueBuffer(buffer); } /* -----------------------------------------------------------------------------