[{"id":25483,"web_url":"https://patchwork.libcamera.org/comment/25483/","msgid":"","date":"2022-10-19T23:25:33","subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Tue, Oct 18, 2022 at 05:34:38PM +0900, Paul Elder via libcamera-devel wrote:\n> Add support for the TestPatternMode control.\n> \n> Signed-off-by: Paul Elder \n> ---\n> src/ipa/rkisp1/rkisp1.cpp | 7 +++++\n\nFor IPU3, we implements TPG support in the pipeline handler, without\ninvolving the IPA module (see commit acf8d028edda \"libcamera: pipeline:\nipu3: Apply a requested test pattern mode\"). Is there a reason to do\ndifferently here ?\n\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 +++++++++++++++++++++++-\n> 2 files changed, 44 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 3f5c1a58..1aac8884 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -183,6 +183,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> \n> \t/* Return the controls handled by the IPA. */\n> \tControlInfoMap::Map ctrlMap = rkisp1Controls;\n> +\n> +\tauto tpgInfo = sensorControls.find(V4L2_CID_TEST_PATTERN);\n> +\tif (tpgInfo != sensorControls.end()) {\n> +\t\tctrlMap.emplace(&controls::draft::TestPatternMode,\n> +\t\t\t\tControlInfo(tpgInfo->second.values()));\n> +\t}\n> +\n> \t*ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> \n> \treturn 0;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 455ee2a0..42803266 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -93,6 +93,8 @@ public:\n> \tPipelineHandlerRkISP1 *pipe();\n> \tint loadIPA(unsigned int hwRevision);\n> \n> +\tint setSensorTestPattern(const ControlList *controls);\n> +\n> \tStream mainPathStream_;\n> \tStream selfPathStream_;\n> \tstd::unique_ptr sensor_;\n> @@ -104,6 +106,8 @@ public:\n> \tRkISP1MainPath *mainPath_;\n> \tRkISP1SelfPath *selfPath_;\n> \n> +\tcontrols::draft::TestPatternModeEnum testPatternMode_;\n> +\n> \tstd::unique_ptr ipa_;\n> \n> private:\n> @@ -357,6 +361,30 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> \treturn 0;\n> }\n> \n> +int RkISP1CameraData::setSensorTestPattern(const ControlList *controls)\n> +{\n> +\tif (!controls)\n> +\t\treturn 0;\n> +\n> +\tconst auto &testPattern = controls->get(controls::draft::TestPatternMode);\n> +\n> +\tif (!testPattern)\n> +\t\treturn 0;\n> +\n> +\tif (testPattern && *testPattern == testPatternMode_)\n> +\t\treturn 0;\n> +\n> +\ttestPatternMode_ = static_cast(*testPattern);\n> +\n> +\tint ret = sensor_->setTestPatternMode(testPatternMode_);\n> +\tif (ret && !sensor_->testPatternModes().empty())\n> +\t\treturn ret;\n> +\n> +\tLOG(RkISP1, Debug) << \"Set test pattern to \" << static_cast(testPatternMode_);\n> +\n> +\treturn 0;\n> +}\n> +\n> void RkISP1CameraData::paramFilled(unsigned int frame)\n> {\n> \tPipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> @@ -820,11 +848,15 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> \treturn 0;\n> }\n> \n> -int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> +int PipelineHandlerRkISP1::start(Camera *camera, const ControlList *controls)\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n> \tint ret;\n> \n> +\tret = data->setSensorTestPattern(controls);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> \t/* Allocate buffers for internal pipeline usage. */\n> \tret = allocateBuffers(camera);\n> \tif (ret)\n> @@ -923,6 +955,10 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n> \n> +\tint ret = data->setSensorTestPattern(&request->controls());\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> \tRkISP1FrameInfo *info = data->frameInfo_.create(data, request);\n> \tif (!info)\n> \t\treturn -ENOENT;","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 C7366BD16B\n\tfor ;\n\tWed, 19 Oct 2022 23:26:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1A3F162E78;\n\tThu, 20 Oct 2022 01:26:01 +0200 (CEST)","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 096CF604DA\n\tfor ;\n\tThu, 20 Oct 2022 01:25:59 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5F26E570;\n\tThu, 20 Oct 2022 01:25:58 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666221961;\n\tbh=peuUO8KlnvUl//JoxQCwYJ8jdLULFPwUGDEnyB9TH54=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JK8fm+K2avnZjwdL3fLRZhkWaX8NODnBmpjmVeyl7NI099z92c9Jtl/qB/9sLLTpp\n\tU2YVo1F+H7WRT6rTVohf9fgI7SImUiD1baOMeqWFUYujFNiWrvK36rElOe1V54LCNr\n\th3UNQjjFZFEiiWe2wx/dqiQJWdo2CcRcYdnRIJsKbaq9RZTeOKfTJMLI7h7NJ3Mm0C\n\tWJoEK+tWGQdLyXw9JVlxHof83FfMFN5Wx3bpT+eCpBqgnNRYnDhUszmTnm4MhMQSIT\n\t7BFSCZkpQ8eofmO4oz9slxFPqVXQXgYb2zWKIYQLX2G12zwoPKVAsgFktbRpKuj/rt\n\tTar3DWkv8xFsA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666221958;\n\tbh=peuUO8KlnvUl//JoxQCwYJ8jdLULFPwUGDEnyB9TH54=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZMZIUZIppOkdanDOrOJtBBZpdeyiDziZrem3/xuZ2W58b9oQCdA7hvN32hbFKSGLX\n\to2q0tIw2rzhzo0oBIdza+qjwR+s2eNInDpH/GGaMYgHUJVSt1pIkOn3u2q7ZWePSFb\n\tiuSJI0Y9GLChWBAC00H0qlfAqL/qKVcZdvdeeejI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ZMZIUZIp\"; dkim-atps=neutral","Date":"Thu, 20 Oct 2022 02:25:33 +0300","To":"Paul Elder ","Message-ID":"","References":"<20221018083438.2937184-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221018083438.2937184-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25499,"web_url":"https://patchwork.libcamera.org/comment/25499/","msgid":"<166625594885.2560709.2476214109949673774@Monstersaurus>","date":"2022-10-20T08:52:28","subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-10-20 00:25:33)\n> Hi Paul,\n> \n> Thank you for the patch.\n> \n> On Tue, Oct 18, 2022 at 05:34:38PM +0900, Paul Elder via libcamera-devel wrote:\n> > Add support for the TestPatternMode control.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > src/ipa/rkisp1/rkisp1.cpp | 7 +++++\n> \n> For IPU3, we implements TPG support in the pipeline handler, without\n> involving the IPA module (see commit acf8d028edda \"libcamera: pipeline:\n> ipu3: Apply a requested test pattern mode\"). Is there a reason to do\n> differently here ?\n\nI actually suspect we might be better going through the IPA, as I recall\nsomeone reported that if we enable the TPG on IPU3, then the AE/AGC can\nend up breaking... So we might need to let the IPA know that the gains\napplied will not correlate to the statistics of the frame received.\n\nIt would also mean we can align how we handle per-frame control for\n'which' frame to apply the test-pattern on, along with other sensor\ncontrols.\n\n> \n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 +++++++++++++++++++++++-\n> > 2 files changed, 44 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 3f5c1a58..1aac8884 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -183,6 +183,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > \n> > /* Return the controls handled by the IPA. */\n> > ControlInfoMap::Map ctrlMap = rkisp1Controls;\n> > +\n> > + auto tpgInfo = sensorControls.find(V4L2_CID_TEST_PATTERN);\n> > + if (tpgInfo != sensorControls.end()) {\n> > + ctrlMap.emplace(&controls::draft::TestPatternMode,\n> > + ControlInfo(tpgInfo->second.values()));\n> > + }\n> > +\n> > *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> > \n> > return 0;\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 455ee2a0..42803266 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -93,6 +93,8 @@ public:\n> > PipelineHandlerRkISP1 *pipe();\n> > int loadIPA(unsigned int hwRevision);\n> > \n> > + int setSensorTestPattern(const ControlList *controls);\n> > +\n> > Stream mainPathStream_;\n> > Stream selfPathStream_;\n> > std::unique_ptr sensor_;\n> > @@ -104,6 +106,8 @@ public:\n> > RkISP1MainPath *mainPath_;\n> > RkISP1SelfPath *selfPath_;\n> > \n> > + controls::draft::TestPatternModeEnum testPatternMode_;\n> > +\n> > std::unique_ptr ipa_;\n> > \n> > private:\n> > @@ -357,6 +361,30 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > return 0;\n> > }\n> > \n> > +int RkISP1CameraData::setSensorTestPattern(const ControlList *controls)\n> > +{\n> > + if (!controls)\n> > + return 0;\n> > +\n> > + const auto &testPattern = controls->get(controls::draft::TestPatternMode);\n> > +\n> > + if (!testPattern)\n> > + return 0;\n> > +\n> > + if (testPattern && *testPattern == testPatternMode_)\n> > + return 0;\n> > +\n> > + testPatternMode_ = static_cast(*testPattern);\n> > +\n> > + int ret = sensor_->setTestPatternMode(testPatternMode_);\n> > + if (ret && !sensor_->testPatternModes().empty())\n> > + return ret;\n> > +\n> > + LOG(RkISP1, Debug) << \"Set test pattern to \" << static_cast(testPatternMode_);\n> > +\n> > + return 0;\n> > +}\n> > +\n> > void RkISP1CameraData::paramFilled(unsigned int frame)\n> > {\n> > PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> > @@ -820,11 +848,15 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> > return 0;\n> > }\n> > \n> > -int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> > +int PipelineHandlerRkISP1::start(Camera *camera, const ControlList *controls)\n> > {\n> > RkISP1CameraData *data = cameraData(camera);\n> > int ret;\n> > \n> > + ret = data->setSensorTestPattern(controls);\n> > + if (ret)\n> > + return ret;\n> > +\n> > /* Allocate buffers for internal pipeline usage. */\n> > ret = allocateBuffers(camera);\n> > if (ret)\n> > @@ -923,6 +955,10 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> > {\n> > RkISP1CameraData *data = cameraData(camera);\n> > \n> > + int ret = data->setSensorTestPattern(&request->controls());\n> > + if (ret)\n> > + return ret;\n> > +\n> > RkISP1FrameInfo *info = data->frameInfo_.create(data, request);\n> > if (!info)\n> > return -ENOENT;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 B75E6C0DA4\n\tfor ;\n\tThu, 20 Oct 2022 08:52:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1417B62E9B;\n\tThu, 20 Oct 2022 10:52:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9C69D62DFF\n\tfor ;\n\tThu, 20 Oct 2022 10:52:31 +0200 (CEST)","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 3092F570;\n\tThu, 20 Oct 2022 10:52:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666255953;\n\tbh=aYpmEc0AyMm9rPiOVFoGZzkaYPlPx/sj21wNn/y+UPw=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=k2BTUCXz0zwBb2ulMOfnlyfhJTvXNDcErU6aDDPOxaozCp78cZWgzyeujx96vuOWL\n\tGn72re+rDSFZtCipayYA4QhECOp7Gyg9p9dA77s9B9UjJKULvADUx5as0QgbmMjIp9\n\tdtV7v5Gf67pF7rp+IJ49P61adWpruNs2xw0ZG1sn3AjVY+NooIR72EPXxSCj4g/sxx\n\tHFpxKUAo7tEo5PROlSKjZb5rBvkpv/I0rECKX2LME9TwXwSzPyd+lY0wYbP5ESJFYb\n\tkv3Hdjmqkr1LMpQ/fvWPfc7bX8sPBdTy6sb/e6OfGtm2H6nOtqgWFY/xXFeeMrGKlK\n\thOqWa2jdFtgJA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666255951;\n\tbh=aYpmEc0AyMm9rPiOVFoGZzkaYPlPx/sj21wNn/y+UPw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=qnAU6fLl+E8ovcshFUO+YEdLFWk8yl5b9QnmrFBPQ6MhZJFU0vnCYv3FzL+3oht+Y\n\t4l8XrpurdcZcq/agsuZKdtw2c2zEhF/RVuRpNwgR8ZfSaIVLDxVx4LI2QntpD+eDer\n\tz4tF/pij7aASvbq8lXkCllWKVtJCXs4A22qShmRo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qnAU6fLl\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221018083438.2937184-1-paul.elder@ideasonboard.com>\n\t","To":"Laurent Pinchart ,\n\tLaurent Pinchart via libcamera-devel\n\t, \n\tPaul Elder ","Date":"Thu, 20 Oct 2022 09:52:28 +0100","Message-ID":"<166625594885.2560709.2476214109949673774@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25511,"web_url":"https://patchwork.libcamera.org/comment/25511/","msgid":"","date":"2022-10-20T10:22:11","subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Thu, Oct 20, 2022 at 09:52:28AM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2022-10-20 00:25:33)\n> > On Tue, Oct 18, 2022 at 05:34:38PM +0900, Paul Elder via libcamera-devel wrote:\n> > > Add support for the TestPatternMode control.\n> > > \n> > > Signed-off-by: Paul Elder \n> > > ---\n> > > src/ipa/rkisp1/rkisp1.cpp | 7 +++++\n> > \n> > For IPU3, we implements TPG support in the pipeline handler, without\n> > involving the IPA module (see commit acf8d028edda \"libcamera: pipeline:\n> > ipu3: Apply a requested test pattern mode\"). Is there a reason to do\n> > differently here ?\n> \n> I actually suspect we might be better going through the IPA, as I recall\n> someone reported that if we enable the TPG on IPU3, then the AE/AGC can\n> end up breaking... So we might need to let the IPA know that the gains\n> applied will not correlate to the statistics of the frame received.\n\nI think the IPA module needs to be informed, yes (hopefully no sensor\nwill reflect the exposure time and analog gain settings in their test\npattern output, so we'll be able to take the TPG into account in\nalgorithms and have consistent behaviour across all sensors).\n\n> It would also mean we can align how we handle per-frame control for\n> 'which' frame to apply the test-pattern on, along with other sensor\n> controls.\n\nI'm not sure I want to allow the TPG to be controllable on a per-frame\nbasis though. That would make it quite complicated for AGC. If the TPG\nsettings have to be constant for the whole capture session, we would be\nable to simply disable AGC.\n\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 +++++++++++++++++++++++-\n> > > 2 files changed, 44 insertions(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index 3f5c1a58..1aac8884 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -183,6 +183,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > > \n> > > /* Return the controls handled by the IPA. */\n> > > ControlInfoMap::Map ctrlMap = rkisp1Controls;\n> > > +\n> > > + auto tpgInfo = sensorControls.find(V4L2_CID_TEST_PATTERN);\n> > > + if (tpgInfo != sensorControls.end()) {\n> > > + ctrlMap.emplace(&controls::draft::TestPatternMode,\n> > > + ControlInfo(tpgInfo->second.values()));\n> > > + }\n> > > +\n> > > *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> > > \n> > > return 0;\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 455ee2a0..42803266 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -93,6 +93,8 @@ public:\n> > > PipelineHandlerRkISP1 *pipe();\n> > > int loadIPA(unsigned int hwRevision);\n> > > \n> > > + int setSensorTestPattern(const ControlList *controls);\n> > > +\n> > > Stream mainPathStream_;\n> > > Stream selfPathStream_;\n> > > std::unique_ptr sensor_;\n> > > @@ -104,6 +106,8 @@ public:\n> > > RkISP1MainPath *mainPath_;\n> > > RkISP1SelfPath *selfPath_;\n> > > \n> > > + controls::draft::TestPatternModeEnum testPatternMode_;\n> > > +\n> > > std::unique_ptr ipa_;\n> > > \n> > > private:\n> > > @@ -357,6 +361,30 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > > return 0;\n> > > }\n> > > \n> > > +int RkISP1CameraData::setSensorTestPattern(const ControlList *controls)\n> > > +{\n> > > + if (!controls)\n> > > + return 0;\n> > > +\n> > > + const auto &testPattern = controls->get(controls::draft::TestPatternMode);\n> > > +\n> > > + if (!testPattern)\n> > > + return 0;\n> > > +\n> > > + if (testPattern && *testPattern == testPatternMode_)\n> > > + return 0;\n> > > +\n> > > + testPatternMode_ = static_cast(*testPattern);\n> > > +\n> > > + int ret = sensor_->setTestPatternMode(testPatternMode_);\n> > > + if (ret && !sensor_->testPatternModes().empty())\n> > > + return ret;\n> > > +\n> > > + LOG(RkISP1, Debug) << \"Set test pattern to \" << static_cast(testPatternMode_);\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > void RkISP1CameraData::paramFilled(unsigned int frame)\n> > > {\n> > > PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> > > @@ -820,11 +848,15 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> > > return 0;\n> > > }\n> > > \n> > > -int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> > > +int PipelineHandlerRkISP1::start(Camera *camera, const ControlList *controls)\n> > > {\n> > > RkISP1CameraData *data = cameraData(camera);\n> > > int ret;\n> > > \n> > > + ret = data->setSensorTestPattern(controls);\n> > > + if (ret)\n> > > + return ret;\n> > > +\n> > > /* Allocate buffers for internal pipeline usage. */\n> > > ret = allocateBuffers(camera);\n> > > if (ret)\n> > > @@ -923,6 +955,10 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> > > {\n> > > RkISP1CameraData *data = cameraData(camera);\n> > > \n> > > + int ret = data->setSensorTestPattern(&request->controls());\n> > > + if (ret)\n> > > + return ret;\n> > > +\n> > > RkISP1FrameInfo *info = data->frameInfo_.create(data, request);\n> > > if (!info)\n> > > return -ENOENT;","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 96D97C0DA4\n\tfor ;\n\tThu, 20 Oct 2022 10:22:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E50DD62EA5;\n\tThu, 20 Oct 2022 12:22:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42B35604DB\n\tfor ;\n\tThu, 20 Oct 2022 12:22:37 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8DEABD38;\n\tThu, 20 Oct 2022 12:22:36 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666261358;\n\tbh=nygVFH6HTgzEVcVRdck5S7sg149UO01sI0Xw4KE6rVQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=ZcneqoMYlpLlv+vKbScsn7ZqS2YLfQLs5PSC1bYMvUvluT6YX5S2P019O9sbgwUn6\n\tQ8NAiBILzfnvjzCRcvTxW7g3qmhmZZ0MCsedIM8zDaUSqX6lcZF/BRWZ/NhCBOgRob\n\t5gE6PtNJuNuj3GER/mFGJtDdWKnMcpRozmFS8V+hJJBoQR5bBDMhNtqgc2r4aaTt3l\n\tGfSXrZL7tzPTXe1CTban3xX4bAKp1YauRfHVt8D80MzkKavYY+ezc2/clGT4p81rDV\n\tba9T82089G6fQ4QQzrWqIGOuag7+7xNGcjV3Qvgtl01GTxZNdwirNIvcfEQhNbH3Tr\n\t3xHwL44TA0Cbg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666261356;\n\tbh=nygVFH6HTgzEVcVRdck5S7sg149UO01sI0Xw4KE6rVQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BRVvZVxYCm9N+hcyZ9TBkz/focIvcHC+YwP11dYEixZvqRTtMsqUAkwCNsbP6KSiQ\n\tFfanYt+yX/V9xFB0G8nO3jjGG9fG71mHUaw0BLSHuP78OQLA69iSpBsdaDrrL1IYu3\n\tgJ7UlEYXVn6JilQUcdAgoXrHng2CTXiN34jgXtj4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"BRVvZVxY\"; dkim-atps=neutral","Date":"Thu, 20 Oct 2022 13:22:11 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20221018083438.2937184-1-paul.elder@ideasonboard.com>\n\t\n\t<166625594885.2560709.2476214109949673774@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166625594885.2560709.2476214109949673774@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25513,"web_url":"https://patchwork.libcamera.org/comment/25513/","msgid":"<166626242112.3490151.8523903829413965927@Monstersaurus>","date":"2022-10-20T10:40:21","subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-10-20 11:22:11)\n> Hi Kieran,\n> \n> On Thu, Oct 20, 2022 at 09:52:28AM +0100, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart via libcamera-devel (2022-10-20 00:25:33)\n> > > On Tue, Oct 18, 2022 at 05:34:38PM +0900, Paul Elder via libcamera-devel wrote:\n> > > > Add support for the TestPatternMode control.\n> > > > \n> > > > Signed-off-by: Paul Elder \n> > > > ---\n> > > > src/ipa/rkisp1/rkisp1.cpp | 7 +++++\n> > > \n> > > For IPU3, we implements TPG support in the pipeline handler, without\n> > > involving the IPA module (see commit acf8d028edda \"libcamera: pipeline:\n> > > ipu3: Apply a requested test pattern mode\"). Is there a reason to do\n> > > differently here ?\n> > \n> > I actually suspect we might be better going through the IPA, as I recall\n> > someone reported that if we enable the TPG on IPU3, then the AE/AGC can\n> > end up breaking... So we might need to let the IPA know that the gains\n> > applied will not correlate to the statistics of the frame received.\n> \n> I think the IPA module needs to be informed, yes (hopefully no sensor\n> will reflect the exposure time and analog gain settings in their test\n> pattern output, so we'll be able to take the TPG into account in\n> algorithms and have consistent behaviour across all sensors).\n> \n> > It would also mean we can align how we handle per-frame control for\n> > 'which' frame to apply the test-pattern on, along with other sensor\n> > controls.\n> \n> I'm not sure I want to allow the TPG to be controllable on a per-frame\n> basis though. That would make it quite complicated for AGC. If the TPG\n> settings have to be constant for the whole capture session, we would be\n> able to simply disable AGC.\n\nYes, really I was thinking that the AGC needs to know so it can\n'disable' itself.\n\nIn particular, if there are any stages after the Sensor that will change\nthe gain, i.e. if the ISP is going to apply digital gains - then that\nlikely needs to be disabled. (Or at least know that the analog gains\nwill not be used/accounted for).\n\n--\nKieran\n\n\n> \n> > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 38 +++++++++++++++++++++++-\n> > > > 2 files changed, 44 insertions(+), 1 deletion(-)\n> > > > \n> > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > index 3f5c1a58..1aac8884 100644\n> > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > @@ -183,6 +183,13 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision,\n> > > > \n> > > > /* Return the controls handled by the IPA. */\n> > > > ControlInfoMap::Map ctrlMap = rkisp1Controls;\n> > > > +\n> > > > + auto tpgInfo = sensorControls.find(V4L2_CID_TEST_PATTERN);\n> > > > + if (tpgInfo != sensorControls.end()) {\n> > > > + ctrlMap.emplace(&controls::draft::TestPatternMode,\n> > > > + ControlInfo(tpgInfo->second.values()));\n> > > > + }\n> > > > +\n> > > > *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls);\n> > > > \n> > > > return 0;\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index 455ee2a0..42803266 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -93,6 +93,8 @@ public:\n> > > > PipelineHandlerRkISP1 *pipe();\n> > > > int loadIPA(unsigned int hwRevision);\n> > > > \n> > > > + int setSensorTestPattern(const ControlList *controls);\n> > > > +\n> > > > Stream mainPathStream_;\n> > > > Stream selfPathStream_;\n> > > > std::unique_ptr sensor_;\n> > > > @@ -104,6 +106,8 @@ public:\n> > > > RkISP1MainPath *mainPath_;\n> > > > RkISP1SelfPath *selfPath_;\n> > > > \n> > > > + controls::draft::TestPatternModeEnum testPatternMode_;\n> > > > +\n> > > > std::unique_ptr ipa_;\n> > > > \n> > > > private:\n> > > > @@ -357,6 +361,30 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n> > > > return 0;\n> > > > }\n> > > > \n> > > > +int RkISP1CameraData::setSensorTestPattern(const ControlList *controls)\n> > > > +{\n> > > > + if (!controls)\n> > > > + return 0;\n> > > > +\n> > > > + const auto &testPattern = controls->get(controls::draft::TestPatternMode);\n> > > > +\n> > > > + if (!testPattern)\n> > > > + return 0;\n> > > > +\n> > > > + if (testPattern && *testPattern == testPatternMode_)\n> > > > + return 0;\n> > > > +\n> > > > + testPatternMode_ = static_cast(*testPattern);\n> > > > +\n> > > > + int ret = sensor_->setTestPatternMode(testPatternMode_);\n> > > > + if (ret && !sensor_->testPatternModes().empty())\n> > > > + return ret;\n> > > > +\n> > > > + LOG(RkISP1, Debug) << \"Set test pattern to \" << static_cast(testPatternMode_);\n> > > > +\n> > > > + return 0;\n> > > > +}\n> > > > +\n> > > > void RkISP1CameraData::paramFilled(unsigned int frame)\n> > > > {\n> > > > PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe();\n> > > > @@ -820,11 +848,15 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> > > > return 0;\n> > > > }\n> > > > \n> > > > -int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> > > > +int PipelineHandlerRkISP1::start(Camera *camera, const ControlList *controls)\n> > > > {\n> > > > RkISP1CameraData *data = cameraData(camera);\n> > > > int ret;\n> > > > \n> > > > + ret = data->setSensorTestPattern(controls);\n> > > > + if (ret)\n> > > > + return ret;\n> > > > +\n> > > > /* Allocate buffers for internal pipeline usage. */\n> > > > ret = allocateBuffers(camera);\n> > > > if (ret)\n> > > > @@ -923,6 +955,10 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> > > > {\n> > > > RkISP1CameraData *data = cameraData(camera);\n> > > > \n> > > > + int ret = data->setSensorTestPattern(&request->controls());\n> > > > + if (ret)\n> > > > + return ret;\n> > > > +\n> > > > RkISP1FrameInfo *info = data->frameInfo_.create(data, request);\n> > > > if (!info)\n> > > > return -ENOENT;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 27003C0DA4\n\tfor ;\n\tThu, 20 Oct 2022 10:40:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C25F62EA9;\n\tThu, 20 Oct 2022 12:40:26 +0200 (CEST)","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 9797F604DB\n\tfor ;\n\tThu, 20 Oct 2022 12:40:24 +0200 (CEST)","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 0879D106;\n\tThu, 20 Oct 2022 12:40:23 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666262426;\n\tbh=M2j0ND2FqRO3NwZfmkexh4/BuvfRvu/XO+JMmq0Pf9Q=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=cGdMEcURNmWrpZIozo5HoEPIQT7P6gocn0B3KDhQxkPIvVUeP+jq7XhtQLwRPxWfF\n\tpUgxbl4BnI15X1wmecu6scK2bA/AdagzhTThg0yiLmNw2z073ixcYFqkw3T3h3sgsM\n\tdhENWQQhI3X8OoNg+WaCGg5H44hjpfxmFMJP3HByANxQIpX2o7XJmgz149V5ybBVA9\n\tLcIX0GSqNMOdoIOBiL2LOz2RnyfEn/oBHcWF7eqPWJm7HGSHNbSWF5fM/LxvfEnCR7\n\t56yo6pqu/Km1LDcQffdUt0POZ34wgdhu3XXCjyNXY050365WEd0i4CuKgZzvCM8ILW\n\t3oU7PnA5ZsV4A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666262424;\n\tbh=M2j0ND2FqRO3NwZfmkexh4/BuvfRvu/XO+JMmq0Pf9Q=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=p/OmcxzdWQSb3L9eBoa5X41iI6r0YZutRY8y1oRuOwfplavNJMiaT54voTXjxRIqN\n\tCPnSwmZE2M/k/IboR5YYsdwRbS6XebvMdfq3mfQhWbUMy2hFA+Qhy9rrxc/2a0Hqtw\n\t34PlseIgHgodMCok1WgodLgMp2kaift7QmiD5mmM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"p/Omcxzd\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221018083438.2937184-1-paul.elder@ideasonboard.com>\n\t\n\t<166625594885.2560709.2476214109949673774@Monstersaurus>\n\t","To":"Laurent Pinchart ","Date":"Thu, 20 Oct 2022 11:40:21 +0100","Message-ID":"<166626242112.3490151.8523903829413965927@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] rkisp1: Add support for sensor test\n\tpattern control","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"Laurent Pinchart via libcamera-devel\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]