From patchwork Tue Jul 16 07:27:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1712 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 3406B60C23 for ; Tue, 16 Jul 2019 09:26: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 0BEBD1C0002; Tue, 16 Jul 2019 07:26:30 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Jul 2019 09:27:38 +0200 Message-Id: <20190716072739.2071-2-jacopo@jmondi.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190716072739.2071-1-jacopo@jmondi.org> References: <20190716072739.2071-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/2] libcamera: ipu3: Disable links 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: Tue, 16 Jul 2019 07:26:32 -0000 With the current IPU3 kernel driver implementation, a linked pipe shall be used (buffers should be queued on it) in order not to block all other pipes. Currently all links on the ImgU device are only disabled at match() time, implying that once an ImgU pipe gets linked, it should be used until the whole pipeline is not re-matched and links disabled again. This is a severe limitation for applications that wants to switch between cameras using different pipes going through a full library tear-down and reload. Perform link disabling at configure() time as well, so that a camera configuration operation always unlock the usage of the assigned pipe, regardless of the previously linked ones. 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 Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 54 ++++++++++++++++------------ 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 159a9312f95e..bae3072b177f 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -499,6 +499,37 @@ 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 used + * 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 the usage of the two + * ImgU 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. @@ -784,29 +815,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; From patchwork Tue Jul 16 07:27:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1713 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 4341160C23 for ; Tue, 16 Jul 2019 09:26: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 B68681C000C; Tue, 16 Jul 2019 07:26:32 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Jul 2019 09:27:39 +0200 Message-Id: <20190716072739.2071-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190716072739.2071-1-jacopo@jmondi.org> References: <20190716072739.2071-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/2] libcamera: ipu3: Do not re-queue cancelled 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: Tue, 16 Jul 2019 07:26:34 -0000 When a video device is stopped all the buffers there queued are released and their state is set to BufferCancelled. Currently, on buffer completion, cancelled 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 cancelled. For the ImgU output buffer this is not required, as cancelled request should be reported to applications in order to report them failure of the capture operations. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- src/libcamera/pipeline/ipu3/ipu3.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) -- 2.21.0 diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index bae3072b177f..c27fe75577f5 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -927,6 +927,10 @@ int PipelineHandlerIPU3::registerCameras() */ void IPU3CameraData::imguInputBufferReady(Buffer *buffer) { + /* \todo Handle buffer failures when state is set to BufferError. */ + if (buffer->status() == Buffer::BufferCancelled) + return; + cio2_.output_->queueBuffer(buffer); } @@ -957,6 +961,10 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) */ void IPU3CameraData::cio2BufferReady(Buffer *buffer) { + /* \todo Handle buffer failures when state is set to BufferError. */ + if (buffer->status() == Buffer::BufferCancelled) + return; + imgu_->input_->queueBuffer(buffer); }