From patchwork Mon Jul 15 05:59:33 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1699 X-Patchwork-Delegate: jacopo@jmondi.org Return-Path: Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B20F961A96 for ; Mon, 15 Jul 2019 07:58:30 +0200 (CEST) X-Originating-IP: 104.132.146.101 Received: from uno.localdomain (unknown [104.132.146.101]) (Authenticated sender: jacopo@jmondi.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 853B31C0003; Mon, 15 Jul 2019 05:58:29 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Mon, 15 Jul 2019 07:59:33 +0200 Message-Id: <20190715055935.21233-2-jacopo@jmondi.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190715055935.21233-1-jacopo@jmondi.org> References: <20190715055935.21233-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/3] libcamera: ipu3: Move link disabling at configure time X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jul 2019 05:58:30 -0000 With the current IPU3 kernel driver implementation, if links are enabled on an ImgU pipe buffers should be queued on its output devices in order not to block the other pipe. Currently all links on the ImgU device are disabled at match() time, implying that once an ImgU pipe gets linked, it should be used until the whole pipeline handler is not re-matched and links disabled again. This is a severe limitation for applications that wants to switch between cameras using different pipes without going through a full library tear-down and re-initialization. Move the link disabling at configure() time, so that a camera configuration operation always unlock the usage of the assigned pipe, regardless of the status of the one previously in use. Unfortunately this requires a camera start/stop sequence to always go through a configure step, a requirement that is not enforced by the Camera state machine. Signed-off-by: Jacopo Mondi --- src/libcamera/pipeline/ipu3/ipu3.cpp | 55 +++++++++++++++------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 22987dbf1460..5204487684c2 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -496,6 +496,35 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) ImgUDevice *imgu = data->imgu_; int ret; + /* + * FIXME: enabled links in one ImgU pipe interfere with capture + * operations on the other one. This can be easily triggered by + * capturing from one camera and then trying to capture from the other + * one right after, without disabling media links on the first pipe. + * + * The tricky part here is where to disable links on the ImgU instance + * which is currently not in use: + * 1) Link enable/disable cannot be done at start()/stop() time as video + * devices needs to be linked first before format can be configured on + * them. + * 2) As link enable has to be done at the least in configure(), + * before configuring formats, the only place where to disable links + * would be 'stop()', but the Camera class state machine allows + * start()<->stop() sequences without any configure() in between. + * + * As of now, disable all links in the ImgU media graph before + * configuring the device, to allow alternate usage of two pipes. + * + * As a consequence, a Camera using an ImgU shall be configured before + * any start()/stop() sequence. An application that wants to + * pre-configure all the camera and then start/stop them alternatively + * without going through any re-configuration (a sequence that is + * allowed by the Camera state machine) would now fail on the IPU3. + */ + ret = imguMediaDev_->disableLinks(); + if (ret) + return ret; + /* * \todo: Enable links selectively based on the requested streams. * As of now, enable all links unconditionally. @@ -778,32 +807,6 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) if (cio2MediaDev_->disableLinks()) return false; - /* - * FIXME: enabled links in one ImgU instance interfere with capture - * operations on the other one. This can be easily triggered by - * capturing from one camera and then trying to capture from the other - * one right after, without disabling media links in the media graph - * first. - * - * The tricky part here is where to disable links on the ImgU instance - * which is currently not in use: - * 1) Link enable/disable cannot be done at start/stop time as video - * devices needs to be linked first before format can be configured on - * them. - * 2) As link enable has to be done at the least in configure(), - * before configuring formats, the only place where to disable links - * would be 'stop()', but the Camera class state machine allows - * start()<->stop() sequences without any configure() in between. - * - * As of now, disable all links in the media graph at 'match()' time, - * to allow testing different cameras in different test applications - * runs. A test application that would use two distinct cameras without - * going through a library teardown->match() sequence would fail - * at the moment. - */ - ret = imguMediaDev_->disableLinks(); - if (ret) - return ret; ret = registerCameras(); From patchwork Mon Jul 15 05:59:34 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1700 X-Patchwork-Delegate: jacopo@jmondi.org Return-Path: Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 76E9261AA0 for ; Mon, 15 Jul 2019 07:58:32 +0200 (CEST) X-Originating-IP: 104.132.146.101 Received: from uno.localdomain (unknown [104.132.146.101]) (Authenticated sender: jacopo@jmondi.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 3EC7B1C0005; Mon, 15 Jul 2019 05:58:30 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Mon, 15 Jul 2019 07:59:34 +0200 Message-Id: <20190715055935.21233-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190715055935.21233-1-jacopo@jmondi.org> References: <20190715055935.21233-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/3] libcamera: ipu3: Do not re-queue failed buffers X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jul 2019 05:58:32 -0000 When a video device is stopped all the buffers there queued are released and their state is set to failure. Currently, on buffer completion, failed buffers are blindly re-queued to the ImgU input or CIO2 output devices, preventing them to be re-started succesfully in future capture sessions. Fix that by inspecting the buffers status and skip re-queueing if they're reported as failing. For the ImgU output buffers this is not required, as failed buffes should be anyhow delivered to applications in order to report their failure. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 5204487684c2..11bf3af66ae6 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -916,6 +916,9 @@ int PipelineHandlerIPU3::registerCameras() */ void IPU3CameraData::imguInputBufferReady(Buffer *buffer) { + if (buffer->status() != Buffer::BufferSuccess) + return; + cio2_.output_->queueBuffer(buffer); } @@ -946,6 +949,9 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) */ void IPU3CameraData::cio2BufferReady(Buffer *buffer) { + if (buffer->status() != Buffer::BufferSuccess) + return; + imgu_->input_->queueBuffer(buffer); } From patchwork Mon Jul 15 05:59:35 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1701 Return-Path: Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2C41261A74 for ; Mon, 15 Jul 2019 07:58:34 +0200 (CEST) X-Originating-IP: 104.132.146.101 Received: from uno.localdomain (unknown [104.132.146.101]) (Authenticated sender: jacopo@jmondi.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 053721C0003; Mon, 15 Jul 2019 05:58:32 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Mon, 15 Jul 2019 07:59:35 +0200 Message-Id: <20190715055935.21233-4-jacopo@jmondi.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190715055935.21233-1-jacopo@jmondi.org> References: <20190715055935.21233-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/3] libcamera: message: Add user message types X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Jul 2019 05:58:34 -0000 Add an operation to the Message class to create unique message type to be used by derived classes to uniquely identify the messages they handle. Signed-off-by: Jacopo Mondi --- src/libcamera/include/message.h | 8 ++++++++ src/libcamera/message.cpp | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/libcamera/include/message.h b/src/libcamera/include/message.h index db17d647c280..e7dd85a606a3 100644 --- a/src/libcamera/include/message.h +++ b/src/libcamera/include/message.h @@ -7,6 +7,8 @@ #ifndef __LIBCAMERA_MESSAGE_H__ #define __LIBCAMERA_MESSAGE_H__ +#include + namespace libcamera { class Object; @@ -19,6 +21,7 @@ public: enum Type { None = 0, SignalMessage = 1, + UserMessage = 1000, }; Message(Type type); @@ -27,11 +30,16 @@ public: Type type() const { return type_; } Object *receiver() const { return receiver_; } + static Type registerUserMessageType(); + private: friend class Thread; Type type_; Object *receiver_; + + static std::mutex mutex_; + static Type nextUserType_; }; class SignalMessage : public Message diff --git a/src/libcamera/message.cpp b/src/libcamera/message.cpp index 0580c1051622..f82a3115595d 100644 --- a/src/libcamera/message.cpp +++ b/src/libcamera/message.cpp @@ -6,6 +6,7 @@ */ #include "message.h" +#include "thread.h" #include "log.h" @@ -31,6 +32,9 @@ namespace libcamera { LOG_DEFINE_CATEGORY(Message) +std::mutex Message::mutex_; +Message::Type Message::nextUserType_ = Message::UserMessage; + /** * \class Message * \brief A message that can be posted to a Thread @@ -68,6 +72,22 @@ Message::~Message() * \return The message receiver */ +/** + * \brief Retrieve a unique identifier for user defined message types + * + * Derived classes can define their own message types and this method provides + * them a unique identifier to use for that purpose. + * + * \return A new unique message type + */ +Message::Type Message::registerUserMessageType() +{ + MutexLocker locker(mutex_); + + return nextUserType_ = static_cast + (static_cast(nextUserType_) + 1); +} + /** * \class SignalMessage * \brief A message carrying a Signal across threads