[{"id":1955,"web_url":"https://patchwork.libcamera.org/comment/1955/","msgid":"<20190619210509.GP21753@pendragon.ideasonboard.com>","date":"2019-06-19T21:05:09","subject":"Re: [libcamera-devel] [PATCH v4 5/6] libcamera: ipu3: Set pipe_mode\n\tbased on stream configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Wed, Jun 19, 2019 at 01:08:57PM +0200, Jacopo Mondi wrote:\n> Set the ImgU pipe_mode control based on the active stream configuration.\n> Use 'Video' pipe mode unless the viewfinder stream is not active.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 34 ++++++++++++++++++++++++++++\n> 1 file changed, 34 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 1c0a9825b4cd..8c26d2c8c60c 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -22,6 +22,8 @@\n> #include \"media_device.h\"\n> #include \"pipeline_handler.h\"\n> #include \"utils.h\"\n> +#include \"v4l2_controls.h\"\n> +#include \"v4l2_device.h\"\n\nIs v4l2_device.h needed ? It should be included by both v4l2_subdevice.h\nand v4l2_videodevice.h.\n\n> #include \"v4l2_subdevice.h\"\n> #include \"v4l2_videodevice.h\"\n> \n> @@ -194,6 +196,12 @@ private:\n> class PipelineHandlerIPU3 : public PipelineHandler\n> {\n> public:\n> +\tstatic constexpr unsigned int V4L2_CID_IPU3_PIPE_MODE = 0x009819c1;\n> +\tenum IPU3PipeModes {\n> +\t\tIPU3PipeModeVideo = 0,\n> +\t\tIPU3PipeModeStillCapture = 1,\n> +\t};\n> +\n> \tPipelineHandlerIPU3(CameraManager *manager);\n> \n> \tCameraConfiguration *generateConfiguration(Camera *camera,\n> @@ -535,7 +543,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t * As we need to set format also on the non-active streams, use\n> \t * the configuration of the active one for that purpose (there should\n> \t * be at least one active stream in the configuration request).\n> +\t *\n> +\t * Also set the IPU3 pipe mode: video mode by default, unless the only\n> +\t * requested stream is the still capture output one.\n> +\t * \\todo: This works as long as only two concurrent streams per pipe\n> +\t * are supported.\n\ns/todo:/todo/\n\n> \t */\n> +\tint pipeMode = IPU3PipeModeVideo;\n\nWould it be easier to read if written\n\n\tint pipeMode = vfStream->active_ ? IPU3PipeModeVideo : IPU3PipeModeStillCapture;\n\n(and moved further down, where used) ?\n\n> \tif (!outStream->active_) {\n> \t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n> \t\tif (ret)\n> @@ -546,6 +560,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n> \t\tif (ret)\n> \t\t\treturn ret;\n> +\n> +\t\tpipeMode = IPU3PipeModeStillCapture;\n> \t}\n> \n> \t/*\n> @@ -559,6 +575,24 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> \tif (ret)\n> \t\treturn ret;\n> \n> +\t/* Apply the \"pipe_mode\" control to the ImgU subdevice. */\n> +\tV4L2Controls ctrls;\n> +\tctrls.add(V4L2_CID_IPU3_PIPE_MODE, pipeMode);\n> +\tret = imgu->imgu_->setControls(&ctrls);\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Unable to set pipe_mode control\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = imgu->imgu_->getControls(&ctrls);\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Unable to get pipe_mode control value\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tpipeMode = ctrls.get(V4L2_CID_IPU3_PIPE_MODE);\n> +\tLOG(IPU3, Debug) << \"ImgU pipe mode set to: \"\n> +\t\t\t << (pipeMode ? \"'Still Capture'\" : \"'Video'\");\n\nDo we really need the get part ?\n\n> \treturn 0;\n> }\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 C1EDF60BC5\n\tfor ;\n\tWed, 19 Jun 2019 23:05:27 +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 47F41333;\n\tWed, 19 Jun 2019 23:05:27 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560978327;\n\tbh=ggqMS20F2gm79JYrJ1uN/kfnNHNTP3Er8K9CRUYa934=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZmzUIVVk9fFoFJNPu3HS7uygwJoIq5qVNQVaNVcuMzqgaM6zOZb6FjJbMBTT61NHU\n\tzLHeNI48vwNDaJXMB5wIfKz7Ux8l+yta+Iw19rGfFXiRnY2P/vJyWW52vEK2xBknk/\n\tt7MKsJ/+ZDgfzoqtLOtnuAWWNs4+1JY5/ZAtzkxI=","Date":"Thu, 20 Jun 2019 00:05:09 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190619210509.GP21753@pendragon.ideasonboard.com>","References":"<20190619110858.20980-1-jacopo@jmondi.org>\n\t<20190619110858.20980-6-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190619110858.20980-6-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v4 5/6] libcamera: ipu3: Set pipe_mode\n\tbased on stream configuration","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, 19 Jun 2019 21:05:27 -0000"}}]