[{"id":20659,"web_url":"https://patchwork.libcamera.org/comment/20659/","msgid":"<163585303389.1097798.9412146408900278012@Monstersaurus>","date":"2021-11-02T11:37:13","subject":"Re: [libcamera-devel] [PATCH v3 2/2] libcamera: pipeline: ipu3:\n\tApply the requested test pattern mode","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Hirokazu Honda (2021-11-02 02:55:22)\n> This enables ipu3 pipeline handler to apply the test pattern mode\n> per frame.\n> \n> Signed-off-by: Hirokazu Honda \n> Reviewed-by: Jean-Michel Hautbois \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 41 ++++++++++++++++++++++++++--\n> 1 file changed, 39 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index eb714aa6..e81ba63c 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -59,6 +59,7 @@ public:\n> void statBufferReady(FrameBuffer *buffer);\n> void queuePendingRequests();\n> void cancelPendingRequests();\n> + void frameStart(uint32_t sequence);\n> \n> CIO2Device cio2_;\n> ImgUDevice *imgu_;\n> @@ -78,6 +79,7 @@ public:\n> std::unique_ptr ipa_;\n> \n> std::queue pendingRequests_;\n> + std::queue processingRequests_;\n> \n> ControlInfoMap ipaControls_;\n> \n> @@ -569,6 +571,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> cio2->sensor()->sensorInfo(&sensorInfo);\n> data->cropRegion_ = sensorInfo.analogCrop;\n> \n> + cio2->sensor()->setTestPatternMode(controls::draft::TestPatternModeOff);\n> +\n> /*\n> * Configure the H/V flip controls based on the combination of\n> * the sensor and user transform.\n> @@ -797,11 +801,13 @@ void PipelineHandlerIPU3::stop(Camera *camera)\n> int ret = 0;\n> \n> data->cancelPendingRequests();\n> + data->processingRequests_ = {};\n> \n> data->ipa_->stop();\n> \n> ret |= data->imgu_->stop();\n> ret |= data->cio2_.stop();\n> +\n> if (ret)\n> LOG(IPU3, Warning) << \"Failed to stop camera \" << camera->id();\n> \n> @@ -852,6 +858,8 @@ void IPU3CameraData::queuePendingRequests()\n> \n> info->rawBuffer = rawBuffer;\n> \n> + processingRequests_.push(request);\n> +\n> ipa::ipu3::IPU3Event ev;\n> ev.op = ipa::ipu3::EventProcessControls;\n> ev.frame = info->id;\n> @@ -1130,8 +1138,8 @@ int PipelineHandlerIPU3::registerCameras()\n> data->delayedCtrls_ =\n> std::make_unique(cio2->sensor()->device(),\n> params);\n> - data->cio2_.frameStart().connect(data->delayedCtrls_.get(),\n> - &DelayedControls::applyControls);\n> + data->cio2_.frameStart().connect(data.get(),\n> + &IPU3CameraData::frameStart);\n> \n> /* Convert the sensor rotation to a transformation */\n> int32_t rotation = 0;\n> @@ -1422,6 +1430,35 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> ipa_->processEvent(ev);\n> }\n> \n> +void IPU3CameraData::frameStart(uint32_t sequence)\n> +{\n> + if (!processingRequests_.empty()) {\n> + /* Assumes applying the test pattern mode affects immediately. */\n> + Request *request = processingRequests_.front();\n> + processingRequests_.pop();\n> +\n> + if (request->controls().contains(controls::draft::TestPatternMode)) {\n> + const int32_t testPatternMode = request->controls().get(\n> + controls::draft::TestPatternMode);\n> +\n> + LOG(IPU3, Debug) << \"Apply test pattern mode: \"\n> + << testPatternMode;\n> +\n> + int ret = cio2_.sensor()->setTestPatternMode(testPatternMode);\n> + if (ret) {\n> + LOG(IPU3, Error) << \"Failed to set test pattern mode: \"\n> + << ret;\n> + } else {\n> + request->metadata().set(controls::draft::TestPatternMode,\n> + testPatternMode);\n> + }\n\nSeeing all this make me a bit concerned that we're making\nTestPatternMode a 'special case', when it's just a control on the\nCameraSensor class. This code here would have to be duplicated for every\npipeline handler.\n\nCan this be handled inside the CameraSensor class itself, and simply\npass the request (and the cnotrol list, or a filtered control list) into\nthe CameraSensor?\n\n\n\n> + }\n> + }\n> +\n> + /* Controls that don't affect immediately are applied in delayedCtrls. */\n> + delayedCtrls_->applyControls(sequence);\n> +}\n> +\n> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)\n> \n> } /* namespace libcamera */\n> -- \n> 2.33.1.1089.g2158813163f-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 49275BDB1C\n\tfor ;\n\tTue, 2 Nov 2021 11:37:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E54D60325;\n\tTue, 2 Nov 2021 12:37:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 81027600B8\n\tfor ;\n\tTue, 2 Nov 2021 12:37:16 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0FC7C3E5;\n\tTue, 2 Nov 2021 12:37:16 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Yo+K8JRN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635853036;\n\tbh=KJ/3PGJPVi9YNxQkdb00SeETBw2QQYAOGg+KmvtIZVk=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Yo+K8JRNmnmPuju3yQQcV4kvaSHOLuYKA83jJDi2akYdB5DdC3aF5LjZiQEe7mC55\n\tahZddlYGuNVhVhQK1QZrgwO7T521IGfCEaNKdJuID2GHCbWSRdYGVnJmmmX0lIPA4V\n\txc1MYImlJXac15LZGLieuqPe587GXEi4rX64a4To=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211102025522.735085-2-hiroh@chromium.org>","References":"<20211102025522.735085-1-hiroh@chromium.org>\n\t<20211102025522.735085-2-hiroh@chromium.org>","From":"Kieran Bingham ","To":"Hirokazu Honda , libcamera-devel@lists.libcamera.org","Date":"Tue, 02 Nov 2021 11:37:13 +0000","Message-ID":"<163585303389.1097798.9412146408900278012@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v3 2/2] libcamera: pipeline: ipu3:\n\tApply the requested test pattern mode","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]