[{"id":2141,"web_url":"https://patchwork.libcamera.org/comment/2141/","msgid":"<20190703223958.GO5007@pendragon.ideasonboard.com>","date":"2019-07-03T22:39:58","subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: pipeline: RKISP1\n\tremove rockchip-sy-mipi-dphy","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Helen,\n\nThank you for the patch.\n\nOn Wed, Jul 03, 2019 at 05:21:53PM -0300, Helen Koike wrote:\n> Remove subdevice rockchip-sy-mipi-dphy from the pipeline.\n> Sensors are connected direcly to rkisp1-isp-subdev.\n> \n> Signed-off-by: Helen Koike \n> \n> ---\n> Hello,\n> \n> This depends on the v7[1] of driver being accepted upstream. But I'm\n> submitting anyway for reference.\n> \n> [1] https://patchwork.kernel.org/project/linux-media/list/?series=141793\n> \n> Thanks\n> Helen\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 34 +++++-------------------\n> utils/rkisp1/rkisp1-capture.sh | 4 +--\n> 2 files changed, 8 insertions(+), 30 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 4a5898d..358e2c8 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -104,7 +104,6 @@ private:\n> \tvoid bufferReady(Buffer *buffer);\n> \n> \tMediaDevice *media_;\n> -\tV4L2Subdevice *dphy_;\n> \tV4L2Subdevice *isp_;\n> \tV4L2VideoDevice *video_;\n> \n> @@ -201,8 +200,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> }\n> \n> PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> -\t: PipelineHandler(manager), dphy_(nullptr), isp_(nullptr),\n> -\t video_(nullptr)\n> +\t: PipelineHandler(manager), isp_(nullptr), video_(nullptr)\n> {\n> }\n> \n> @@ -210,7 +208,6 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()\n> {\n> \tdelete video_;\n> \tdelete isp_;\n> -\tdelete dphy_;\n> }\n> \n> /* -----------------------------------------------------------------------------\n> @@ -250,7 +247,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\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 = dphy_->entity()->getPadByIndex(0);\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> @@ -282,18 +279,14 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \n> \tLOG(RkISP1, Debug) << \"Sensor configured with \" << format.toString();\n> \n> -\tret = dphy_->setFormat(0, &format);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n> -\tret = dphy_->getFormat(1, &format);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> +\tLOG(RkISP1, Debug) << \"Configuring ISP with \" << format.toString();\n\nI think you can skip this, as the previous debugging message prints the\nexact same format.\n\n> \n> \tret = isp_->setFormat(0, &format);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> +\tLOG(RkISP1, Debug) << \"ISP configured with \" << format.toString();\n> +\n> \tV4L2DeviceFormat outputFormat = {};\n> \toutputFormat.fourcc = cfg.pixelFormat;\n> \toutputFormat.size = cfg.size;\n> @@ -393,14 +386,6 @@ int PipelineHandlerRkISP1::initLinks()\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tlink = media_->link(\"rockchip-sy-mipi-dphy\", 1, \"rkisp1-isp-subdev\", 0);\n> -\tif (!link)\n> -\t\treturn -ENODEV;\n> -\n> -\tret = link->setEnabled(true);\n> -\tif (ret < 0)\n> -\t\treturn ret;\n> -\n> \tlink = media_->link(\"rkisp1-isp-subdev\", 2, \"rkisp1_mainpath\", 0);\n> \tif (!link)\n> \t\treturn -ENODEV;\n> @@ -442,17 +427,12 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \tdm.add(\"rkisp1_mainpath\");\n> \tdm.add(\"rkisp1-statistics\");\n> \tdm.add(\"rkisp1-input-params\");\n> -\tdm.add(\"rockchip-sy-mipi-dphy\");\n> \n> \tmedia_ = acquireMediaDevice(enumerator, dm);\n> \tif (!media_)\n> \t\treturn false;\n> \n> \t/* Create the V4L2 subdevices we will need. */\n> -\tdphy_ = V4L2Subdevice::fromEntityName(media_, \"rockchip-sy-mipi-dphy\");\n> -\tif (dphy_->open() < 0)\n> -\t\treturn false;\n> -\n> \tisp_ = V4L2Subdevice::fromEntityName(media_, \"rkisp1-isp-subdev\");\n> \tif (isp_->open() < 0)\n> \t\treturn false;\n> @@ -471,10 +451,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> \t}\n> \n> \t/*\n> -\t * Enumerate all sensors connected to the CSI-2 receiver and create one\n> +\t * Enumerate all sensors connected to the ISP receiver and create one\n\ns/ISP/ISP CSI-2/\n\nApart from these small issues this looks good to me, so\n\nReviewed-by: Laurent Pinchart \n\nI will however wait until the rkisp1 patch series goes through review\nbefore applying this patch.\n\n> \t * camera instance for each of them.\n> \t */\n> -\tpad = dphy_->entity()->getPadByIndex(0);\n> +\tpad = isp_->entity()->getPadByIndex(0);\n> \tif (!pad)\n> \t\treturn false;\n> \n> diff --git a/utils/rkisp1/rkisp1-capture.sh b/utils/rkisp1/rkisp1-capture.sh\n> index cffe9fe..8a6f6eb 100755\n> --- a/utils/rkisp1/rkisp1-capture.sh\n> +++ b/utils/rkisp1/rkisp1-capture.sh\n> @@ -68,12 +68,10 @@ configure_pipeline() {\n> \n> \t$mediactl -r\n> \n> -\t$mediactl -l \"'$sensor':0 -> 'rockchip-sy-mipi-dphy':0 [1]\"\n> -\t$mediactl -l \"'rockchip-sy-mipi-dphy':1 -> 'rkisp1-isp-subdev':0 [1]\"\n> +\t$mediactl -l \"'$sensor':0 -> 'rkisp1-isp-subdev':0 [1]\"\n> \t$mediactl -l \"'rkisp1-isp-subdev':2 -> 'rkisp1_mainpath':0 [1]\"\n> \n> \t$mediactl -V \"\\\"$sensor\\\":0 [$format]\"\n> -\t$mediactl -V \"'rockchip-sy-mipi-dphy':1 [$format]\"\n> \t$mediactl -V \"'rkisp1-isp-subdev':0 [$format crop:(0,0)/$sensor_size]\"\n> \t$mediactl -V \"'rkisp1-isp-subdev':2 [fmt:$capture_mbus_code/$capture_size crop:(0,0)/$capture_size]\"\n> }","headers":{"Return-Path":"","Received":["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 430FF60C01\n\tfor ;\n\tThu, 4 Jul 2019 00:40:19 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A317024B;\n\tThu, 4 Jul 2019 00:40:18 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562193618;\n\tbh=Dwyv1KhpQrKIU77ZBGrwCBDOkUcK9vU6MMNDfOSRW9I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lLFcge7eXTMEe0Ay5PiGm1N0/Yh5YfbSDVWImnWCwIoMu/gr0MY6L9bQXqMvQ65+F\n\t7RkpK4/vXWyaHjdZEqJlAbbv10/w9hLOWD5SWzkqbFaQ++DliGzQqEOahJ/GT5aCH9\n\t8HfRDgwurFTFtOVK4FhMfAKZHAMdwWsr9Rhj2fYw=","Date":"Thu, 4 Jul 2019 01:39:58 +0300","From":"Laurent Pinchart ","To":"Helen Koike ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com","Message-ID":"<20190703223958.GO5007@pendragon.ideasonboard.com>","References":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/2] libcamera: pipeline: RKISP1\n\tremove rockchip-sy-mipi-dphy","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 03 Jul 2019 22:40:19 -0000"}}]