[{"id":20594,"web_url":"https://patchwork.libcamera.org/comment/20594/","msgid":"<142d7faf-5560-55b0-a67e-d1ba6b3915fa@ideasonboard.com>","date":"2021-10-27T18:23:48","subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Eric,\n\nThank you for the patch.\n\nOn 10/27/21 3:34 PM, Eric Curtin wrote:\n> xrand --query isn't always available and does not always display all\n> conneted connectors.\n\ns/conneted/connected/\n\nI believe commit message can be a little bit more descriptive providing \nsome context around xrand --query.\n\nAlso a comment on the subject, it should be prefixed with `cam: `\n\n     cam: Add option to list displays\n\n\n>\n> Signed-off-by: Eric Curtin \n> ---\n> src/cam/main.cpp | 31 ++++++++++++++++++++++++++-----\n> src/cam/main.h | 3 +++\n> 2 files changed, 29 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index c7f664b9..319cbf4d 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -15,6 +15,11 @@\n> #include \n> \n> #include \"camera_session.h\"\n> +\n> +#ifdef HAVE_KMS\n> +#include \"drm.h\"\n> +#endif\n> +\n> #include \"event_loop.h\"\n> #include \"main.h\"\n> #include \"options.h\"\n> @@ -137,6 +142,7 @@ int CamApp::parseOptions(int argc, char *argv[])\n> \t\t\t \"Display viewfinder through DRM/KMS on specified connector\",\n> \t\t\t \"display\", ArgumentOptional, \"connector\", false,\n> \t\t\t OptCamera);\n> +\tparser.addOption(OptListDisplays, OptionNone, \"List all displays\", \"list-displays\");\n\n\nCan there are displays with status ::Disconnected or ::Unknown, I am not \nsure. If yes, I would reword the description to\n\n         \"List all connected displays\"\n\n(just a suggestion, if it makes sense to you)\n\n> #endif\n> \tparser.addOption(OptFile, OptionString,\n> \t\t\t \"Write captured frames to disk\\n\"\n> @@ -202,7 +208,22 @@ int CamApp::run()\n> \t\t}\n> \t}\n> \n> -\t/* 2. Create the camera sessions. */\n> +#ifdef HAVE_KMS\n> +\t/* 2. List all displays. */\n> +\tif (options_.isSet(OptListDisplays)) {\n> +\t\tstd::cout << \"Available displays:\" << std::endl;\n> +\n> +\t\tDRM::Device dev;\n> +\t\tdev.init();\n\n\nThis returns an integer value which should be checked.\n\n> +\t\tfor (const DRM::Connector &conn : dev.connectors()) {\n> +\t\t\tif (conn.status() == DRM::Connector::Connected) {\n> +\t\t\t\tstd::cout << conn.name() << std::endl;\n\n\nI wonder if we should also print the type (conn.type()) along with the \nconnector name.\n\nThe patch looks to me overall.\n\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +#endif\n> +\n> +\t/* 3. Create the camera sessions. */\n> \tstd::vector> sessions;\n> \n> \tif (options_.isSet(OptCamera)) {\n> @@ -229,7 +250,7 @@ int CamApp::run()\n> \t\t}\n> \t}\n> \n> -\t/* 3. Print camera information. */\n> +\t/* 4. Print camera information. */\n> \tif (options_.isSet(OptListControls) ||\n> \t options_.isSet(OptListProperties) ||\n> \t options_.isSet(OptInfo)) {\n> @@ -243,7 +264,7 @@ int CamApp::run()\n> \t\t}\n> \t}\n> \n> -\t/* 4. Start capture. */\n> +\t/* 5. Start capture. */\n> \tfor (const auto &session : sessions) {\n> \t\tif (!session->options().isSet(OptCapture))\n> \t\t\tcontinue;\n> @@ -257,7 +278,7 @@ int CamApp::run()\n> \t\tloopUsers_++;\n> \t}\n> \n> -\t/* 5. Enable hotplug monitoring. */\n> +\t/* 6. Enable hotplug monitoring. */\n> \tif (options_.isSet(OptMonitor)) {\n> \t\tstd::cout << \"Monitoring new hotplug and unplug events\" << std::endl;\n> \t\tstd::cout << \"Press Ctrl-C to interrupt\" << std::endl;\n> @@ -271,7 +292,7 @@ int CamApp::run()\n> \tif (loopUsers_)\n> \t\tloop_.exec();\n> \n> -\t/* 6. Stop capture. */\n> +\t/* 7. Stop capture. */\n> \tfor (const auto &session : sessions) {\n> \t\tif (!session->options().isSet(OptCapture))\n> \t\t\tcontinue;\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> index 1c2fab76..72050d27 100644\n> --- a/src/cam/main.h\n> +++ b/src/cam/main.h\n> @@ -21,6 +21,9 @@ enum {\n> \tOptListControls = 256,\n> \tOptStrictFormats = 257,\n> \tOptMetadata = 258,\n> +#ifdef HAVE_KMS\n> +\tOptListDisplays = 259\n> +#endif\n> };\n> \n> #endif /* __CAM_MAIN_H__ */","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 8923DBDB1C\n\tfor ;\n\tWed, 27 Oct 2021 18:23:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D4CEB64882;\n\tWed, 27 Oct 2021 20:23:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54FD760123\n\tfor ;\n\tWed, 27 Oct 2021 20:23:55 +0200 (CEST)","from [192.168.1.106] (unknown [103.251.226.211])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3B5FE276;\n\tWed, 27 Oct 2021 20:23:54 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"gDsMAxOp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635359035;\n\tbh=E3GELZJ4SZ/KQVChXzf1frbY93AJjuglOWMiYzekN3U=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=gDsMAxOpF4k5dcg75RNNjGqz/5FAi493Bxf652EMAZ5i6J0+IDlgi+OlFJ6s7S6g/\n\txnqtCweY7IF9Zx4WpBMIDNvJm0dp5SKpXYup1FpbQ3fJxN2DZGTIJomrMbTLQAsfKB\n\txcaS3QdS9CUJvehrK0cck7L2p+mlTXDOzDM0wN+E=","To":"Eric Curtin , libcamera-devel@lists.libcamera.org","References":"<20211027100441.18248-1-ecurtin@redhat.com>","From":"Umang Jain ","Message-ID":"<142d7faf-5560-55b0-a67e-d1ba6b3915fa@ideasonboard.com>","Date":"Wed, 27 Oct 2021 23:53:48 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20211027100441.18248-1-ecurtin@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","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\" "}},{"id":20596,"web_url":"https://patchwork.libcamera.org/comment/20596/","msgid":"","date":"2021-10-27T21:12:42","subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Eric,\n\nThank you for the patch.\n\nOn Wed, Oct 27, 2021 at 11:04:41AM +0100, Eric Curtin wrote:\n> xrand --query isn't always available and does not always display all\n> conneted connectors.\n\nStupid questions, couldn't connectors also be listed through sysfs ?\n\nfor conn in /sys/class/drm/card*-* ; do status=$(cat $conn/status) ; if [ $status == connected ] ; then echo $conn ; fi ; done\n\nI'm not opposed to adding this feature to the cam application, but I\nwonder if it really belongs here, especially if we consider that a next\nstep would be to print supported modes.\n\n> Signed-off-by: Eric Curtin \n> ---\n> src/cam/main.cpp | 31 ++++++++++++++++++++++++++-----\n> src/cam/main.h | 3 +++\n> 2 files changed, 29 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index c7f664b9..319cbf4d 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -15,6 +15,11 @@\n> #include \n> \n> #include \"camera_session.h\"\n> +\n> +#ifdef HAVE_KMS\n> +#include \"drm.h\"\n> +#endif\n> +\n> #include \"event_loop.h\"\n> #include \"main.h\"\n> #include \"options.h\"\n> @@ -137,6 +142,7 @@ int CamApp::parseOptions(int argc, char *argv[])\n> \t\t\t \"Display viewfinder through DRM/KMS on specified connector\",\n> \t\t\t \"display\", ArgumentOptional, \"connector\", false,\n> \t\t\t OptCamera);\n> +\tparser.addOption(OptListDisplays, OptionNone, \"List all displays\", \"list-displays\");\n> #endif\n> \tparser.addOption(OptFile, OptionString,\n> \t\t\t \"Write captured frames to disk\\n\"\n> @@ -202,7 +208,22 @@ int CamApp::run()\n> \t\t}\n> \t}\n> \n> -\t/* 2. Create the camera sessions. */\n> +#ifdef HAVE_KMS\n> +\t/* 2. List all displays. */\n> +\tif (options_.isSet(OptListDisplays)) {\n> +\t\tstd::cout << \"Available displays:\" << std::endl;\n> +\n> +\t\tDRM::Device dev;\n> +\t\tdev.init();\n\nAs Umang mentioned, errors should be checked.\n\n> +\t\tfor (const DRM::Connector &conn : dev.connectors()) {\n> +\t\t\tif (conn.status() == DRM::Connector::Connected) {\n> +\t\t\t\tstd::cout << conn.name() << std::endl;\n> +\t\t\t}\n\nNo need for curly braces in the inner if.\n\n> +\t\t}\n> +\t}\n> +#endif\n> +\n> +\t/* 3. Create the camera sessions. */\n> \tstd::vector> sessions;\n> \n> \tif (options_.isSet(OptCamera)) {\n> @@ -229,7 +250,7 @@ int CamApp::run()\n> \t\t}\n> \t}\n> \n> -\t/* 3. Print camera information. */\n> +\t/* 4. Print camera information. */\n> \tif (options_.isSet(OptListControls) ||\n> \t options_.isSet(OptListProperties) ||\n> \t options_.isSet(OptInfo)) {\n> @@ -243,7 +264,7 @@ int CamApp::run()\n> \t\t}\n> \t}\n> \n> -\t/* 4. Start capture. */\n> +\t/* 5. Start capture. */\n> \tfor (const auto &session : sessions) {\n> \t\tif (!session->options().isSet(OptCapture))\n> \t\t\tcontinue;\n> @@ -257,7 +278,7 @@ int CamApp::run()\n> \t\tloopUsers_++;\n> \t}\n> \n> -\t/* 5. Enable hotplug monitoring. */\n> +\t/* 6. Enable hotplug monitoring. */\n> \tif (options_.isSet(OptMonitor)) {\n> \t\tstd::cout << \"Monitoring new hotplug and unplug events\" << std::endl;\n> \t\tstd::cout << \"Press Ctrl-C to interrupt\" << std::endl;\n> @@ -271,7 +292,7 @@ int CamApp::run()\n> \tif (loopUsers_)\n> \t\tloop_.exec();\n> \n> -\t/* 6. Stop capture. */\n> +\t/* 7. Stop capture. */\n> \tfor (const auto &session : sessions) {\n> \t\tif (!session->options().isSet(OptCapture))\n> \t\t\tcontinue;\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> index 1c2fab76..72050d27 100644\n> --- a/src/cam/main.h\n> +++ b/src/cam/main.h\n> @@ -21,6 +21,9 @@ enum {\n> \tOptListControls = 256,\n> \tOptStrictFormats = 257,\n> \tOptMetadata = 258,\n> +#ifdef HAVE_KMS\n> +\tOptListDisplays = 259\n> +#endif\n> };\n> \n> #endif /* __CAM_MAIN_H__ */","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 C7C94BDB1C\n\tfor ;\n\tWed, 27 Oct 2021 21:13:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9EA16487F;\n\tWed, 27 Oct 2021 23:13:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94E3C60123\n\tfor ;\n\tWed, 27 Oct 2021 23:13:06 +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 1BB92596;\n\tWed, 27 Oct 2021 23:13:06 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YymBWzgr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635369186;\n\tbh=Mw8FwqgnHKeOsIPZeg11XR0MyzVimRyzb+c6u36u5QM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YymBWzgr4u59yfwXXnwLdFoi6UL0CB6NBamH6rwIULn1S+BvtU2m5lt+EfWZOawqA\n\tw3SKD+aRdHieV1cPXkwegh76LiN+cIyqyWqUYusqnQPd1MvjAFkrpUnBPlZRuLU31Y\n\tGUFPsIRzDIzvmwHwBqeNcSLUzfiDLFJChUZXw9kI=","Date":"Thu, 28 Oct 2021 00:12:42 +0300","From":"Laurent Pinchart ","To":"Eric Curtin ","Message-ID":"","References":"<20211027100441.18248-1-ecurtin@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211027100441.18248-1-ecurtin@redhat.com>","Subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20613,"web_url":"https://patchwork.libcamera.org/comment/20613/","msgid":"","date":"2021-10-28T14:35:06","subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","submitter":{"id":101,"url":"https://patchwork.libcamera.org/api/people/101/","name":"Eric Curtin","email":"ecurtin@redhat.com"},"content":"Hi Umang,\n\nOn Wed, 27 Oct 2021 at 19:29, Umang Jain wrote:\n>\n> Hi Eric,\n>\n> Thank you for the patch.\n>\n> On 10/27/21 3:34 PM, Eric Curtin wrote:\n> > xrand --query isn't always available and does not always display all\n> > conneted connectors.\n>\n> s/conneted/connected/\n>\n> I believe commit message can be a little bit more descriptive providing\n> some context around xrand --query.\n>\n> Also a comment on the subject, it should be prefixed with `cam: `\n>\n> cam: Add option to list displays\n>\n>\n> >\n> > Signed-off-by: Eric Curtin \n> > ---\n> > src/cam/main.cpp | 31 ++++++++++++++++++++++++++-----\n> > src/cam/main.h | 3 +++\n> > 2 files changed, 29 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index c7f664b9..319cbf4d 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -15,6 +15,11 @@\n> > #include \n> >\n> > #include \"camera_session.h\"\n> > +\n> > +#ifdef HAVE_KMS\n> > +#include \"drm.h\"\n> > +#endif\n> > +\n> > #include \"event_loop.h\"\n> > #include \"main.h\"\n> > #include \"options.h\"\n> > @@ -137,6 +142,7 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > \"Display viewfinder through DRM/KMS on specified connector\",\n> > \"display\", ArgumentOptional, \"connector\", false,\n> > OptCamera);\n> > + parser.addOption(OptListDisplays, OptionNone, \"List all displays\", \"list-displays\");\n>\n>\n> Can there are displays with status ::Disconnected or ::Unknown, I am not\n> sure. If yes, I would reword the description to\n>\n> \"List all connected displays\"\n>\n> (just a suggestion, if it makes sense to you)\n\nYou got me thinking, I might just list all displays connected, disconnected and\nunknown. How many displays can there be? Might as well display all,\nthe user could\nalways filter via grep.\n\n>\n> > #endif\n> > parser.addOption(OptFile, OptionString,\n> > \"Write captured frames to disk\\n\"\n> > @@ -202,7 +208,22 @@ int CamApp::run()\n> > }\n> > }\n> >\n> > - /* 2. Create the camera sessions. */\n> > +#ifdef HAVE_KMS\n> > + /* 2. List all displays. */\n> > + if (options_.isSet(OptListDisplays)) {\n> > + std::cout << \"Available displays:\" << std::endl;\n> > +\n> > + DRM::Device dev;\n> > + dev.init();\n>\n>\n> This returns an integer value which should be checked.\n>\n> > + for (const DRM::Connector &conn : dev.connectors()) {\n> > + if (conn.status() == DRM::Connector::Connected) {\n> > + std::cout << conn.name() << std::endl;\n>\n>\n> I wonder if we should also print the type (conn.type()) along with the\n> connector name.\n\nThe type is included in the name and deduced from this map, (interestingly\na \"DP\" type is often physically a HDMI port):\n\nconst std::map connectorTypeNames{\n{ DRM_MODE_CONNECTOR_Unknown, \"Unknown\" },\n{ DRM_MODE_CONNECTOR_VGA, \"VGA\" },\n{ DRM_MODE_CONNECTOR_DVII, \"DVI-I\" },\n{ DRM_MODE_CONNECTOR_DVID, \"DVI-D\" },\n{ DRM_MODE_CONNECTOR_DVIA, \"DVI-A\" },\n{ DRM_MODE_CONNECTOR_Composite, \"Composite\" },\n{ DRM_MODE_CONNECTOR_SVIDEO, \"S-Video\" },\n{ DRM_MODE_CONNECTOR_LVDS, \"LVDS\" },\n{ DRM_MODE_CONNECTOR_Component, \"Component\" },\n{ DRM_MODE_CONNECTOR_9PinDIN, \"9-Pin-DIN\" },\n{ DRM_MODE_CONNECTOR_DisplayPort, \"DP\" },\n{ DRM_MODE_CONNECTOR_HDMIA, \"HDMI-A\" },\n{ DRM_MODE_CONNECTOR_HDMIB, \"HDMI-B\" },\n{ DRM_MODE_CONNECTOR_TV, \"TV\" },\n{ DRM_MODE_CONNECTOR_eDP, \"eDP\" },\n{ DRM_MODE_CONNECTOR_VIRTUAL, \"Virtual\" },\n{ DRM_MODE_CONNECTOR_DSI, \"DSI\" },\n{ DRM_MODE_CONNECTOR_DPI, \"DPI\" },\n};\n\n>\n> The patch looks to me overall.\n>\n> > + }\n> > + }\n> > + }\n> > +#endif\n> > +\n> > + /* 3. Create the camera sessions. */\n> > std::vector> sessions;\n> >\n> > if (options_.isSet(OptCamera)) {\n> > @@ -229,7 +250,7 @@ int CamApp::run()\n> > }\n> > }\n> >\n> > - /* 3. Print camera information. */\n> > + /* 4. Print camera information. */\n> > if (options_.isSet(OptListControls) ||\n> > options_.isSet(OptListProperties) ||\n> > options_.isSet(OptInfo)) {\n> > @@ -243,7 +264,7 @@ int CamApp::run()\n> > }\n> > }\n> >\n> > - /* 4. Start capture. */\n> > + /* 5. Start capture. */\n> > for (const auto &session : sessions) {\n> > if (!session->options().isSet(OptCapture))\n> > continue;\n> > @@ -257,7 +278,7 @@ int CamApp::run()\n> > loopUsers_++;\n> > }\n> >\n> > - /* 5. Enable hotplug monitoring. */\n> > + /* 6. Enable hotplug monitoring. */\n> > if (options_.isSet(OptMonitor)) {\n> > std::cout << \"Monitoring new hotplug and unplug events\" << std::endl;\n> > std::cout << \"Press Ctrl-C to interrupt\" << std::endl;\n> > @@ -271,7 +292,7 @@ int CamApp::run()\n> > if (loopUsers_)\n> > loop_.exec();\n> >\n> > - /* 6. Stop capture. */\n> > + /* 7. Stop capture. */\n> > for (const auto &session : sessions) {\n> > if (!session->options().isSet(OptCapture))\n> > continue;\n> > diff --git a/src/cam/main.h b/src/cam/main.h\n> > index 1c2fab76..72050d27 100644\n> > --- a/src/cam/main.h\n> > +++ b/src/cam/main.h\n> > @@ -21,6 +21,9 @@ enum {\n> > OptListControls = 256,\n> > OptStrictFormats = 257,\n> > OptMetadata = 258,\n> > +#ifdef HAVE_KMS\n> > + OptListDisplays = 259\n> > +#endif\n> > };\n> >\n> > #endif /* __CAM_MAIN_H__ */\n>\n\nAbout to resend patch with yours and Laurents suggestions.\n\nIs mise le meas/Regards,\n\nEric Curtin","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 652E9BF415\n\tfor ;\n\tThu, 28 Oct 2021 14:35:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC994600BA;\n\tThu, 28 Oct 2021 16:35:23 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A2E9A600B5\n\tfor ;\n\tThu, 28 Oct 2021 16:35:21 +0200 (CEST)","from mail-oi1-f197.google.com (mail-oi1-f197.google.com\n\t[209.85.167.197]) (Using TLS) by relay.mimecast.com with ESMTP id\n\tus-mta-124-2z4hyJ5IOSeeJ_PSCMCR-Q-1; Thu, 28 Oct 2021 10:35:19 -0400","by mail-oi1-f197.google.com with SMTP id\n\tn196-20020acad6cd000000b0029a267b839eso3189326oig.19\n\tfor ;\n\tThu, 28 Oct 2021 07:35:18 -0700 (PDT)"],"Authentication-Results":["lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"QAY2IY66\"; dkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ecurtin@redhat.com"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1635431720;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=yia21gJw9uFmLLU4jZSx7XL8heD2vzqJ4pzJGf4CKK4=;\n\tb=QAY2IY66FIXgxRoMgJP/VazXI8YOVKswKtCh66F6k7W50GMJVuexJs/+1iPt4NctMYsLoc\n\tFvI0sH3lkmDD+WBR3nWTYA3O6MWbLTe2v65M9V2O1IbuEFcxSUuJ7uJ1kLyPwnj9p/cASQ\n\tPlmrKlmQXuMk5YjfiJNcK3XX7/bSqDU=","X-MC-Unique":"2z4hyJ5IOSeeJ_PSCMCR-Q-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=yia21gJw9uFmLLU4jZSx7XL8heD2vzqJ4pzJGf4CKK4=;\n\tb=D1ebnMToy37fKDj45Mlr/3YNBZ69u0R0uN1RM1e4nAMwNy+vIWV+KD9Na+X3sp+u+A\n\tqFBmnra4XMvdlZH6UanzWdv6Bq89uxkfwNQgxRXvCmyfOvLb0xE3hzEth2Ijc+Vb2pmK\n\tElon0Im9+BockGxrupfUTqxaYbRBhU0b3rK2LWfcWRbx911g+grzdML0e9UK+PV//zQa\n\ttbRAv+qnSafgoyLTzyv8/XteQpHLPf53L5F4XlvcrtInj0QT+5dzA08Z8gYQcqj/Ou9B\n\tu+Ez0PIUkn3BfsQjXmh7PXxO9+4CMoXxUMb7XslL8B1kiUsrt0vndH2kSf5JjPi/lTcp\n\tcTPw==","X-Gm-Message-State":"AOAM53262X4pGe5bVAyaWAQgW9xY5J1wZIx7uyNt0x1i50L5XG/TCAFh\n\tT3rRrSt5IPsJqlqhwMfKKCaXWB9tIj1eYmzJdceoYlhkzbO9aU7J3q7Yvv1sjmDhJfWUNAYVtZo\n\ta/sjo2/mACUdaGau69sFv/34SQ+hr7HAuFvdf5g3wkT6OfajxtA==","X-Received":["by 2002:a9d:5ee:: with SMTP id 101mr3934412otd.289.1635431718190;\n\tThu, 28 Oct 2021 07:35:18 -0700 (PDT)","by 2002:a9d:5ee:: with SMTP id 101mr3934381otd.289.1635431717850;\n\tThu, 28 Oct 2021 07:35:17 -0700 (PDT)"],"X-Google-Smtp-Source":"ABdhPJwncbZC6k6H2WdIrG1OY4WKQ0fKJexfWAUcZeFDKP2MohyDj+Rv35UkbZ5lVaWAyAks0+HbBp2yHxxdJ7LgdbI=","MIME-Version":"1.0","References":"<20211027100441.18248-1-ecurtin@redhat.com>\n\t<142d7faf-5560-55b0-a67e-d1ba6b3915fa@ideasonboard.com>","In-Reply-To":"<142d7faf-5560-55b0-a67e-d1ba6b3915fa@ideasonboard.com>","From":"Eric Curtin ","Date":"Thu, 28 Oct 2021 15:35:06 +0100","Message-ID":"","To":"Umang Jain ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20614,"web_url":"https://patchwork.libcamera.org/comment/20614/","msgid":"","date":"2021-10-28T15:26:53","subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","submitter":{"id":101,"url":"https://patchwork.libcamera.org/api/people/101/","name":"Eric Curtin","email":"ecurtin@redhat.com"},"content":"Hi Laurent,\n\nOn Wed, 27 Oct 2021 at 22:13, Laurent Pinchart\n wrote:\n>\n> Hi Eric,\n>\n> Thank you for the patch.\n>\n> On Wed, Oct 27, 2021 at 11:04:41AM +0100, Eric Curtin wrote:\n> > xrand --query isn't always available and does not always display all\n> > conneted connectors.\n>\n> Stupid questions, couldn't connectors also be listed through sysfs ?\n>\n> for conn in /sys/class/drm/card*-* ; do status=$(cat $conn/status) ; if [ $status == connected ] ; then echo $conn ; fi ; done\n>\n> I'm not opposed to adding this feature to the cam application, but I\n> wonder if it really belongs here, especially if we consider that a next\n> step would be to print supported modes.\n\nPrinting modes is where I was gonna do next. I only learned about\n/sys/class/drm/card now\nas a new guy to DRM, I think it would be useful for those who aren't\nfamiliar with /sys\ndirectory structure for drm. Finding documentation for this can be\nhard and a tool is easier to maintain anyway.\n\nAre you thinking maybe a separate binary called drm-info? (drm-utils\nis a taken name by a\npackage that is seems not so useful in the context of the cam\napplication), etc? We list all\nthe equivalent cam information with the \"cam -I -c1\", it can be hard\nto gather the equivalent\ndisplay info and if we can get them both at the same place, same\nbinary, would be nice!\n\n>\n> > Signed-off-by: Eric Curtin \n> > ---\n> > src/cam/main.cpp | 31 ++++++++++++++++++++++++++-----\n> > src/cam/main.h | 3 +++\n> > 2 files changed, 29 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index c7f664b9..319cbf4d 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -15,6 +15,11 @@\n> > #include \n> >\n> > #include \"camera_session.h\"\n> > +\n> > +#ifdef HAVE_KMS\n> > +#include \"drm.h\"\n> > +#endif\n> > +\n> > #include \"event_loop.h\"\n> > #include \"main.h\"\n> > #include \"options.h\"\n> > @@ -137,6 +142,7 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > \"Display viewfinder through DRM/KMS on specified connector\",\n> > \"display\", ArgumentOptional, \"connector\", false,\n> > OptCamera);\n> > + parser.addOption(OptListDisplays, OptionNone, \"List all displays\", \"list-displays\");\n> > #endif\n> > parser.addOption(OptFile, OptionString,\n> > \"Write captured frames to disk\\n\"\n> > @@ -202,7 +208,22 @@ int CamApp::run()\n> > }\n> > }\n> >\n> > - /* 2. Create the camera sessions. */\n> > +#ifdef HAVE_KMS\n> > + /* 2. List all displays. */\n> > + if (options_.isSet(OptListDisplays)) {\n> > + std::cout << \"Available displays:\" << std::endl;\n> > +\n> > + DRM::Device dev;\n> > + dev.init();\n>\n> As Umang mentioned, errors should be checked.\n>\n> > + for (const DRM::Connector &conn : dev.connectors()) {\n> > + if (conn.status() == DRM::Connector::Connected) {\n> > + std::cout << conn.name() << std::endl;\n> > + }\n>\n> No need for curly braces in the inner if.\n>\n> > + }\n> > + }\n> > +#endif\n> > +\n> > + /* 3. Create the camera sessions. */\n> > std::vector> sessions;\n> >\n> > if (options_.isSet(OptCamera)) {\n> > @@ -229,7 +250,7 @@ int CamApp::run()\n> > }\n> > }\n> >\n> > - /* 3. Print camera information. */\n> > + /* 4. Print camera information. */\n> > if (options_.isSet(OptListControls) ||\n> > options_.isSet(OptListProperties) ||\n> > options_.isSet(OptInfo)) {\n> > @@ -243,7 +264,7 @@ int CamApp::run()\n> > }\n> > }\n> >\n> > - /* 4. Start capture. */\n> > + /* 5. Start capture. */\n> > for (const auto &session : sessions) {\n> > if (!session->options().isSet(OptCapture))\n> > continue;\n> > @@ -257,7 +278,7 @@ int CamApp::run()\n> > loopUsers_++;\n> > }\n> >\n> > - /* 5. Enable hotplug monitoring. */\n> > + /* 6. Enable hotplug monitoring. */\n> > if (options_.isSet(OptMonitor)) {\n> > std::cout << \"Monitoring new hotplug and unplug events\" << std::endl;\n> > std::cout << \"Press Ctrl-C to interrupt\" << std::endl;\n> > @@ -271,7 +292,7 @@ int CamApp::run()\n> > if (loopUsers_)\n> > loop_.exec();\n> >\n> > - /* 6. Stop capture. */\n> > + /* 7. Stop capture. */\n> > for (const auto &session : sessions) {\n> > if (!session->options().isSet(OptCapture))\n> > continue;\n> > diff --git a/src/cam/main.h b/src/cam/main.h\n> > index 1c2fab76..72050d27 100644\n> > --- a/src/cam/main.h\n> > +++ b/src/cam/main.h\n> > @@ -21,6 +21,9 @@ enum {\n> > OptListControls = 256,\n> > OptStrictFormats = 257,\n> > OptMetadata = 258,\n> > +#ifdef HAVE_KMS\n> > + OptListDisplays = 259\n> > +#endif\n> > };\n> >\n> > #endif /* __CAM_MAIN_H__ */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n\nIs mise le meas/Regards,\n\nEric Curtin","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 5B48DBDB1C\n\tfor ;\n\tThu, 28 Oct 2021 15:27:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 644AC600BC;\n\tThu, 28 Oct 2021 17:27:10 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 88F2E600B5\n\tfor ;\n\tThu, 28 Oct 2021 17:27:08 +0200 (CEST)","from mail-ot1-f70.google.com (mail-ot1-f70.google.com\n\t[209.85.210.70]) (Using TLS) by relay.mimecast.com with ESMTP id\n\tus-mta-155-6qdzOj5ePrKBEVddVYXBLg-1; Thu, 28 Oct 2021 11:27:05 -0400","by mail-ot1-f70.google.com with SMTP id\n\t70-20020a9d0ecc000000b0054e6d44e1adso3600110otj.2\n\tfor ;\n\tThu, 28 Oct 2021 08:27:05 -0700 (PDT)"],"Authentication-Results":["lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"hRm5K4bS\"; dkim-atps=neutral","relay.mimecast.com;\n\tauth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ecurtin@redhat.com"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1635434827;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=7bafWQcKi5807cc19f+DWsz0FBbxQAxHxGYOiLvHVGk=;\n\tb=hRm5K4bSqvMtcrSUCMWdMUUT1KO6vkhj/PSPa8uPoW97TxSTjnNnpSYiIQhwSbIfzJvpOE\n\tyCLfWQerdQzqenGXov41kVoQZF8oiyqEuDUh3wcY1pyJupUc3F03Yv4ljcsxpe1L0d0mqS\n\tvpr0CeU6bvN8Ifi09uYMWv9MJpd38vE=","X-MC-Unique":"6qdzOj5ePrKBEVddVYXBLg-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=7bafWQcKi5807cc19f+DWsz0FBbxQAxHxGYOiLvHVGk=;\n\tb=qqz3TSn/51c98YquUpWgO3oc+ANmsXU81LGdsY4TSK6s3Y9YAuaCrblcyZYCvDor2h\n\tCV1bL3JMyneYxkCRtfQBx7VVSO+7vpk5/bXxBJFBzdEzkIFMQj1QB+LH67k1R/ecGiWH\n\tPkqz0sTuMppveDnToT1OocBYZi1/fMROlZ4buRXN/SsX35JwT5QpyYYw65E5K+jEnHJh\n\tF/32BrKQTo6FKQRQ3qlLP13YOL9XQQJvYRMcDVnaKiH5lJqIkBAe1zZ8MFfiqqOIAP0o\n\tvCvIGAflBX00NW/hTvSMhs0FSo3gPz5p1YrcfkpruIOnlQ+QHFyhFKMY7SVtjuEeQbAj\n\tCPrA==","X-Gm-Message-State":"AOAM533Q8ah82F4glhZ7M74MMrSm7fy34QcY0WZZ645pozDcZXXVi6XM\n\txPf+P17wstMJpNHa+MLgdymDsJgVA1UUGkf/Ww1hQigKDhg/dEWKowicPf5tULhQ7mqkiMvXbpy\n\tOEbmems6cQhP/+68cC4SQLhegezF9nFAkSuhnmN2BGUXNPaYq+w==","X-Received":["by 2002:aca:b10a:: with SMTP id a10mr3576958oif.74.1635434824786;\n\tThu, 28 Oct 2021 08:27:04 -0700 (PDT)","by 2002:aca:b10a:: with SMTP id a10mr3576926oif.74.1635434824461;\n\tThu, 28 Oct 2021 08:27:04 -0700 (PDT)"],"X-Google-Smtp-Source":"ABdhPJwHGsUrRvgK7txFz/A0Gstio8vHNiul9MbvA/z2v9Je+dtLqdDf5uihx7jLzcG9tchPvljDDyoT7BsMlr+yy+M=","MIME-Version":"1.0","References":"<20211027100441.18248-1-ecurtin@redhat.com>\n\t","In-Reply-To":"","From":"Eric Curtin ","Date":"Thu, 28 Oct 2021 16:26:53 +0100","Message-ID":"","To":"Laurent Pinchart ","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20615,"web_url":"https://patchwork.libcamera.org/comment/20615/","msgid":"<163543929158.2909386.3651605624840055151@Monstersaurus>","date":"2021-10-28T16:41:31","subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Eric Curtin (2021-10-28 16:26:53)\n> Hi Laurent,\n> \n> On Wed, 27 Oct 2021 at 22:13, Laurent Pinchart\n> wrote:\n> >\n> > Hi Eric,\n> >\n> > Thank you for the patch.\n> >\n> > On Wed, Oct 27, 2021 at 11:04:41AM +0100, Eric Curtin wrote:\n> > > xrand --query isn't always available and does not always display all\n> > > conneted connectors.\n> >\n> > Stupid questions, couldn't connectors also be listed through sysfs ?\n> >\n> > for conn in /sys/class/drm/card*-* ; do status=$(cat $conn/status) ; if [ $status == connected ] ; then echo $conn ; fi ; done\n> >\n> > I'm not opposed to adding this feature to the cam application, but I\n> > wonder if it really belongs here, especially if we consider that a next\n> > step would be to print supported modes.\n\nI think it's helpful to have it in cam. Cam has support to render on a\nDRM device, but you have to 'know' what device. This is a nice\nconvenience helper to those who do not know other ways to interogate\nDRM.\n\n\n\n> \n> Printing modes is where I was gonna do next. I only learned about\n> /sys/class/drm/card now\n> as a new guy to DRM, I think it would be useful for those who aren't\n> familiar with /sys\n> directory structure for drm. Finding documentation for this can be\n> hard and a tool is easier to maintain anyway.\n> \n> Are you thinking maybe a separate binary called drm-info? (drm-utils\n> is a taken name by a\n> package that is seems not so useful in the context of the cam\n> application), etc? We list all\n> the equivalent cam information with the \"cam -I -c1\", it can be hard\n> to gather the equivalent\n> display info and if we can get them both at the same place, same\n> binary, would be nice!\n> \n> >\n> > > Signed-off-by: Eric Curtin \n> > > ---\n> > > src/cam/main.cpp | 31 ++++++++++++++++++++++++++-----\n> > > src/cam/main.h | 3 +++\n> > > 2 files changed, 29 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > > index c7f664b9..319cbf4d 100644\n> > > --- a/src/cam/main.cpp\n> > > +++ b/src/cam/main.cpp\n> > > @@ -15,6 +15,11 @@\n> > > #include \n> > >\n> > > #include \"camera_session.h\"\n> > > +\n> > > +#ifdef HAVE_KMS\n> > > +#include \"drm.h\"\n> > > +#endif\n> > > +\n> > > #include \"event_loop.h\"\n> > > #include \"main.h\"\n> > > #include \"options.h\"\n> > > @@ -137,6 +142,7 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > > \"Display viewfinder through DRM/KMS on specified connector\",\n> > > \"display\", ArgumentOptional, \"connector\", false,\n> > > OptCamera);\n> > > + parser.addOption(OptListDisplays, OptionNone, \"List all displays\", \"list-displays\");\n> > > #endif\n> > > parser.addOption(OptFile, OptionString,\n> > > \"Write captured frames to disk\\n\"\n> > > @@ -202,7 +208,22 @@ int CamApp::run()\n> > > }\n> > > }\n> > >\n> > > - /* 2. Create the camera sessions. */\n> > > +#ifdef HAVE_KMS\n> > > + /* 2. List all displays. */\n> > > + if (options_.isSet(OptListDisplays)) {\n> > > + std::cout << \"Available displays:\" << std::endl;\n> > > +\n> > > + DRM::Device dev;\n> > > + dev.init();\n> >\n> > As Umang mentioned, errors should be checked.\n> >\n> > > + for (const DRM::Connector &conn : dev.connectors()) {\n> > > + if (conn.status() == DRM::Connector::Connected) {\n> > > + std::cout << conn.name() << std::endl;\n> > > + }\n> >\n> > No need for curly braces in the inner if.\n\nI would be tempted to put any DRM specific code into src/cam/drm.cpp\nthough, and try to reduce the code that is added to main.cpp.\n\n\n> >\n> > > + }\n> > > + }\n> > > +#endif\n> > > +\n> > > + /* 3. Create the camera sessions. */\n> > > std::vector> sessions;\n> > >\n> > > if (options_.isSet(OptCamera)) {\n> > > @@ -229,7 +250,7 @@ int CamApp::run()\n> > > }\n> > > }\n> > >\n> > > - /* 3. Print camera information. */\n> > > + /* 4. Print camera information. */\n> > > if (options_.isSet(OptListControls) ||\n> > > options_.isSet(OptListProperties) ||\n> > > options_.isSet(OptInfo)) {\n> > > @@ -243,7 +264,7 @@ int CamApp::run()\n> > > }\n> > > }\n> > >\n> > > - /* 4. Start capture. */\n> > > + /* 5. Start capture. */\n> > > for (const auto &session : sessions) {\n> > > if (!session->options().isSet(OptCapture))\n> > > continue;\n> > > @@ -257,7 +278,7 @@ int CamApp::run()\n> > > loopUsers_++;\n> > > }\n> > >\n> > > - /* 5. Enable hotplug monitoring. */\n> > > + /* 6. Enable hotplug monitoring. */\n> > > if (options_.isSet(OptMonitor)) {\n> > > std::cout << \"Monitoring new hotplug and unplug events\" << std::endl;\n> > > std::cout << \"Press Ctrl-C to interrupt\" << std::endl;\n> > > @@ -271,7 +292,7 @@ int CamApp::run()\n> > > if (loopUsers_)\n> > > loop_.exec();\n> > >\n> > > - /* 6. Stop capture. */\n> > > + /* 7. Stop capture. */\n> > > for (const auto &session : sessions) {\n> > > if (!session->options().isSet(OptCapture))\n> > > continue;\n> > > diff --git a/src/cam/main.h b/src/cam/main.h\n> > > index 1c2fab76..72050d27 100644\n> > > --- a/src/cam/main.h\n> > > +++ b/src/cam/main.h\n> > > @@ -21,6 +21,9 @@ enum {\n> > > OptListControls = 256,\n> > > OptStrictFormats = 257,\n> > > OptMetadata = 258,\n> > > +#ifdef HAVE_KMS\n> > > + OptListDisplays = 259\n> > > +#endif\n> > > };\n> > >\n> > > #endif /* __CAM_MAIN_H__ */\n> >\n> > --\n> > Regards,\n> >\n> > Laurent Pinchart\n> >\n> \n> Is mise le meas/Regards,\n> \n> Eric Curtin\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 F007EBDB1C\n\tfor ;\n\tThu, 28 Oct 2021 16:41:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 63E60600BA;\n\tThu, 28 Oct 2021 18:41:36 +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 3E51D600B5\n\tfor ;\n\tThu, 28 Oct 2021 18:41:35 +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 BCD59FE0;\n\tThu, 28 Oct 2021 18:41:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ik5N7zNV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635439294;\n\tbh=4ZJwvdnQ6CEHu7KyvhD4blo0Td2mWtlH1HG8DEGBxH4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ik5N7zNVSpbRUMs2pYjg/VPiAYsNkRuPS4VHhRkfCW85fGnEoLLewzjZLPOGbCWfc\n\th6C/9eDUIsdGIgR5G4O5Hmrf+NJkrNQ9j/TuceH1Fi6BGLmeEKjKDcorc2P8FCYH0n\n\trmyd1vPfZWwIpQL4Pvx5Nmk2j8LIhmgPrXP1d0fk=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211027100441.18248-1-ecurtin@redhat.com>\n\t\n\t","From":"Kieran Bingham ","To":"Eric Curtin ,\n\tLaurent Pinchart ","Date":"Thu, 28 Oct 2021 17:41:31 +0100","Message-ID":"<163543929158.2909386.3651605624840055151@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20618,"web_url":"https://patchwork.libcamera.org/comment/20618/","msgid":"","date":"2021-10-28T18:07:41","subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Oct 28, 2021 at 05:41:31PM +0100, Kieran Bingham wrote:\n> Quoting Eric Curtin (2021-10-28 16:26:53)\n> > On Wed, 27 Oct 2021 at 22:13, Laurent Pinchart wrote:\n> > > On Wed, Oct 27, 2021 at 11:04:41AM +0100, Eric Curtin wrote:\n> > > > xrand --query isn't always available and does not always display all\n> > > > conneted connectors.\n> > >\n> > > Stupid questions, couldn't connectors also be listed through sysfs ?\n> > >\n> > > for conn in /sys/class/drm/card*-* ; do status=$(cat $conn/status) ; if [ $status == connected ] ; then echo $conn ; fi ; done\n> > >\n> > > I'm not opposed to adding this feature to the cam application, but I\n> > > wonder if it really belongs here, especially if we consider that a next\n> > > step would be to print supported modes.\n> \n> I think it's helpful to have it in cam. Cam has support to render on a\n> DRM device, but you have to 'know' what device. This is a nice\n> convenience helper to those who do not know other ways to interogate\n> DRM.\n\nI'll side with the majority then :-)\n\n> > Printing modes is where I was gonna do next. I only learned about /sys/class/drm/card now\n> > as a new guy to DRM, I think it would be useful for those who aren't familiar with /sys\n> > directory structure for drm. Finding documentation for this can be\n> > hard and a tool is easier to maintain anyway.\n> > \n> > Are you thinking maybe a separate binary called drm-info? (drm-utils is a taken name by a\n> > package that is seems not so useful in the context of the cam application), etc? We list all\n> > the equivalent cam information with the \"cam -I -c1\", it can be hard to gather the equivalent\n> > display info and if we can get them both at the same place, same binary, would be nice!\n\ncam is a tool that is meant to exercise the whole libcamera API, in a\nbit of a swiss army knife kind of way. It has a few extra features not\nrelated to cameras as such, including support for DRM/KMS output. I'd\nlike to keep the focus on the camera side, and not solve every other\nside issue that the world may be experiencing. In this specific case,\nthere's a (in my opinion) not too difficult way to find the connectors\nand their states from sysfs, but at the same time the implementation of\na similar feature in cam is not too intrusive (at least so far). I'm\nthus OK with it, but if we later add an option to print modes, and then\nlater something else, I may regret this decision :-)\n\n> > > > Signed-off-by: Eric Curtin \n> > > > ---\n> > > > src/cam/main.cpp | 31 ++++++++++++++++++++++++++-----\n> > > > src/cam/main.h | 3 +++\n> > > > 2 files changed, 29 insertions(+), 5 deletions(-)\n> > > >\n> > > > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > > > index c7f664b9..319cbf4d 100644\n> > > > --- a/src/cam/main.cpp\n> > > > +++ b/src/cam/main.cpp\n> > > > @@ -15,6 +15,11 @@\n> > > > #include \n> > > >\n> > > > #include \"camera_session.h\"\n> > > > +\n> > > > +#ifdef HAVE_KMS\n> > > > +#include \"drm.h\"\n> > > > +#endif\n> > > > +\n> > > > #include \"event_loop.h\"\n> > > > #include \"main.h\"\n> > > > #include \"options.h\"\n> > > > @@ -137,6 +142,7 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > > > \"Display viewfinder through DRM/KMS on specified connector\",\n> > > > \"display\", ArgumentOptional, \"connector\", false,\n> > > > OptCamera);\n> > > > + parser.addOption(OptListDisplays, OptionNone, \"List all displays\", \"list-displays\");\n> > > > #endif\n> > > > parser.addOption(OptFile, OptionString,\n> > > > \"Write captured frames to disk\\n\"\n> > > > @@ -202,7 +208,22 @@ int CamApp::run()\n> > > > }\n> > > > }\n> > > >\n> > > > - /* 2. Create the camera sessions. */\n> > > > +#ifdef HAVE_KMS\n> > > > + /* 2. List all displays. */\n> > > > + if (options_.isSet(OptListDisplays)) {\n> > > > + std::cout << \"Available displays:\" << std::endl;\n> > > > +\n> > > > + DRM::Device dev;\n> > > > + dev.init();\n> > >\n> > > As Umang mentioned, errors should be checked.\n> > >\n> > > > + for (const DRM::Connector &conn : dev.connectors()) {\n> > > > + if (conn.status() == DRM::Connector::Connected) {\n> > > > + std::cout << conn.name() << std::endl;\n> > > > + }\n> > >\n> > > No need for curly braces in the inner if.\n> \n> I would be tempted to put any DRM specific code into src/cam/drm.cpp\n> though, and try to reduce the code that is added to main.cpp.\n> \n> > > > + }\n> > > > + }\n> > > > +#endif\n> > > > +\n> > > > + /* 3. Create the camera sessions. */\n> > > > std::vector> sessions;\n> > > >\n> > > > if (options_.isSet(OptCamera)) {\n> > > > @@ -229,7 +250,7 @@ int CamApp::run()\n> > > > }\n> > > > }\n> > > >\n> > > > - /* 3. Print camera information. */\n> > > > + /* 4. Print camera information. */\n> > > > if (options_.isSet(OptListControls) ||\n> > > > options_.isSet(OptListProperties) ||\n> > > > options_.isSet(OptInfo)) {\n> > > > @@ -243,7 +264,7 @@ int CamApp::run()\n> > > > }\n> > > > }\n> > > >\n> > > > - /* 4. Start capture. */\n> > > > + /* 5. Start capture. */\n> > > > for (const auto &session : sessions) {\n> > > > if (!session->options().isSet(OptCapture))\n> > > > continue;\n> > > > @@ -257,7 +278,7 @@ int CamApp::run()\n> > > > loopUsers_++;\n> > > > }\n> > > >\n> > > > - /* 5. Enable hotplug monitoring. */\n> > > > + /* 6. Enable hotplug monitoring. */\n> > > > if (options_.isSet(OptMonitor)) {\n> > > > std::cout << \"Monitoring new hotplug and unplug events\" << std::endl;\n> > > > std::cout << \"Press Ctrl-C to interrupt\" << std::endl;\n> > > > @@ -271,7 +292,7 @@ int CamApp::run()\n> > > > if (loopUsers_)\n> > > > loop_.exec();\n> > > >\n> > > > - /* 6. Stop capture. */\n> > > > + /* 7. Stop capture. */\n> > > > for (const auto &session : sessions) {\n> > > > if (!session->options().isSet(OptCapture))\n> > > > continue;\n> > > > diff --git a/src/cam/main.h b/src/cam/main.h\n> > > > index 1c2fab76..72050d27 100644\n> > > > --- a/src/cam/main.h\n> > > > +++ b/src/cam/main.h\n> > > > @@ -21,6 +21,9 @@ enum {\n> > > > OptListControls = 256,\n> > > > OptStrictFormats = 257,\n> > > > OptMetadata = 258,\n> > > > +#ifdef HAVE_KMS\n> > > > + OptListDisplays = 259\n> > > > +#endif\n> > > > };\n> > > >\n> > > > #endif /* __CAM_MAIN_H__ */","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 7C244BDB1C\n\tfor ;\n\tThu, 28 Oct 2021 18:08:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4C19600BA;\n\tThu, 28 Oct 2021 20:08:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EAA24600B5\n\tfor ;\n\tThu, 28 Oct 2021 20:08:05 +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 54937FE0;\n\tThu, 28 Oct 2021 20:08:05 +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=\"qtEPoGCV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635444485;\n\tbh=fVI/TSiVkZbLVGF+gUVO33z/GRLEsawQZmRkHwmsBb4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qtEPoGCVrjSAJ66JlGd6VArNcpCfn4lyLv/Vfy49O5n7A2ISJ06LcsFXwjWA2EVzb\n\tI8Ew1cY02e9Omi6evfsVu0k+jqA/LONOwX0EurtvDkXTVRcxMPboo0QZscWc9yJgtZ\n\t62h9lVng/utJT2dVzvYbHhOe8zwo7Wi9Su9e/QXM=","Date":"Thu, 28 Oct 2021 21:07:41 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211027100441.18248-1-ecurtin@redhat.com>\n\t\n\t\n\t<163543929158.2909386.3651605624840055151@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163543929158.2909386.3651605624840055151@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH] Add option to list displays","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]