[{"id":12507,"web_url":"https://patchwork.libcamera.org/comment/12507/","msgid":"<20200915004044.GR15543@pendragon.ideasonboard.com>","date":"2020-09-15T00:40:44","subject":"Re: [libcamera-devel] [PATCH v2 03/13] libcamera: pipeline: rkisp1:\n\tSetup links as part of configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, Sep 14, 2020 at 04:21:39PM +0200, Niklas Söderlund wrote:\n> In preparation of supporting both the main and self path configure all\n> the media graph links as a part of the configuration step. Before this\n> change the link between ISP and DMA engine was setup at match time as\n> the only supported path was the main path and only the link between\n> sensor and ISP was updated at part of the configuration step.\n> \n> The main path is still the only path between ISP and DMA engine that is\n> possible to enable.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v1\n> - Pass sensor directly to initLinks().\n> - Remove string variable form link selection.\n> - Rewrite commit message.\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 79 +++++++++++++-----------\n> 1 file changed, 42 insertions(+), 37 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 86a3e3c63b17762b..96046ec32c91cff3 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -213,7 +213,8 @@ private:\n> \tfriend RkISP1CameraData;\n> \tfriend RkISP1Frames;\n> \n> -\tint initLinks();\n> +\tint initLinks(const Camera *camera, const CameraSensor *sensor,\n> +\t\t const RkISP1CameraConfiguration &config);\n> \tint createCamera(MediaEntity *sensor);\n> \tvoid tryCompleteRequest(Request *request);\n> \tvoid bufferReady(FrameBuffer *buffer);\n> @@ -603,28 +604,9 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \tCameraSensor *sensor = data->sensor_;\n> \tint ret;\n> \n> -\t/*\n> -\t * Configure the sensor links: enable the link corresponding to this\n> -\t * camera and disable all the other sensor links.\n> -\t */\n> -\tconst MediaPad *pad = isp_->entity()->getPadByIndex(0);\n> -\n> -\tfor (MediaLink *link : pad->links()) {\n> -\t\tbool enable = link->source()->entity() == sensor->entity();\n> -\n> -\t\tif (!!(link->flags() & MEDIA_LNK_FL_ENABLED) == enable)\n> -\t\t\tcontinue;\n> -\n> -\t\tLOG(RkISP1, Debug)\n> -\t\t\t<< (enable ? \"Enabling\" : \"Disabling\")\n> -\t\t\t<< \" link from sensor '\"\n> -\t\t\t<< link->source()->entity()->name()\n> -\t\t\t<< \"' to ISP\";\n> -\n> -\t\tret = link->setEnabled(enable);\n> -\t\tif (ret < 0)\n> -\t\t\treturn ret;\n> -\t}\n> +\tret = initLinks(camera, sensor, *config);\n> +\tif (ret)\n> +\t\treturn ret;\n> \n> \t/*\n> \t * Configure the format on the sensor output and propagate it through\n> @@ -927,22 +909,51 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> * Match and Setup\n> */\n> \n> -int PipelineHandlerRkISP1::initLinks()\n> +int PipelineHandlerRkISP1::initLinks(const Camera *camera,\n> +\t\t\t\t const CameraSensor *sensor,\n> +\t\t\t\t const RkISP1CameraConfiguration &config)\n> {\n> -\tMediaLink *link;\n> +\tRkISP1CameraData *data = cameraData(camera);\n> \tint ret;\n> \n> \tret = media_->disableLinks();\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tlink = media_->link(\"rkisp1_isp\", 2, \"rkisp1_resizer_mainpath\", 0);\n> -\tif (!link)\n> -\t\treturn -ENODEV;\n> +\t/*\n> +\t * Configure the sensor links: enable the link corresponding to this\n> +\t * camera.\n> +\t */\n> +\tconst MediaPad *pad = isp_->entity()->getPadByIndex(0);\n> +\tfor (MediaLink *link : pad->links()) {\n> +\t\tif (link->source()->entity() != sensor->entity())\n> +\t\t\tcontinue;\n> \n> -\tret = link->setEnabled(true);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> +\t\tLOG(RkISP1, Debug)\n> +\t\t\t<< \"Enabling link from sensor '\"\n> +\t\t\t<< link->source()->entity()->name()\n> +\t\t\t<< \"' to ISP\";\n> +\n> +\t\tret = link->setEnabled(true);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\tfor (const StreamConfiguration &cfg : config) {\n> +\t\tMediaLink *link = nullptr;\n> +\t\tif (cfg.stream() == &data->stream_)\n> +\t\t\tlink = media_->link(\"rkisp1_isp\", 2,\n> +\t\t\t\t\t \"rkisp1_resizer_mainpath\", 0);\n> +\t\telse\n> +\t\t\treturn -EINVAL;\n> +\n> +\t\tif (!link)\n> +\t\t\treturn -ENODEV;\n\n\t\tif (cfg.stream() != &data->stream_)\n\t\t\treturn -EINVAL;\n\n\t\tMediaLink *link = media_->link(\"rkisp1_isp\", 2,\n\t\t\t\t\t \"rkisp1_resizer_mainpath\", 0);\n\t\tif (!link)\n\t\t\treturn -ENODEV;\n\n(It probably affects subsequent patches, but here at least it looks\nneater :-)\n\nReviewed-by: Laurent Pinchart \n\n> +\n> +\t\tret = link->setEnabled(true);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\t}\n> \n> \treturn 0;\n> }\n> @@ -1024,12 +1035,6 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n> \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n> \n> -\t/* Configure default links. */\n> -\tif (initLinks() < 0) {\n> -\t\tLOG(RkISP1, Error) << \"Failed to setup links\";\n> -\t\treturn false;\n> -\t}\n> -\n> \t/*\n> \t * Enumerate all sensors connected to the ISP and create one\n> \t * camera instance for each of them.","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 45436BF01C\n\tfor ;\n\tTue, 15 Sep 2020 00:41:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA5F762D27;\n\tTue, 15 Sep 2020 02:41:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7701362901\n\tfor ;\n\tTue, 15 Sep 2020 02:41:12 +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 B3D6A275;\n\tTue, 15 Sep 2020 02:41:11 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"q/pXlxxj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600130471;\n\tbh=OwtaTkvfyTVbDKSHFn8Lk1Vi+G6jEm+m0GK4argcJC8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=q/pXlxxjazrLyzLMsqOuW3DJX5TmCoj4l/91rgXYaJR2IuOTg+mNw602tME+Qofjn\n\tnQtPOtUjsYwmxC7bme1UjDkqks9PydvE37+tIkBnLp/lel6MJM/ugfVmszE4M/5+Bi\n\tn0ejUI2i8rxZ/auMl55v8Qai6K1clUFSrueuZrZc=","Date":"Tue, 15 Sep 2020 03:40:44 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200915004044.GR15543@pendragon.ideasonboard.com>","References":"<20200914142149.63857-1-niklas.soderlund@ragnatech.se>\n\t<20200914142149.63857-4-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200914142149.63857-4-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 03/13] libcamera: pipeline: rkisp1:\n\tSetup links as part of configuration","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]