[libcamera-devel] [PATCH] Add option to list displays

[Eric Curtin]

Hi Laurent,

On Wed, 27 Oct 2021 at 22:13, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Eric,
>
> Thank you for the patch.
>
> On Wed, Oct 27, 2021 at 11:04:41AM +0100, Eric Curtin wrote:
> > xrand --query isn't always available and does not always display all
> > conneted connectors.
>
> Stupid questions, couldn't connectors also be listed through sysfs ?
>
> for conn in /sys/class/drm/card*-* ; do status=$(cat $conn/status) ; if [ $status == connected ] ; then echo $conn ; fi ; done
>
> I'm not opposed to adding this feature to the cam application, but I
> wonder if it really belongs here, especially if we consider that a next
> step would be to print supported modes.

Printing modes is where I was gonna do next. I only learned about
/sys/class/drm/card now
as a new guy to DRM, I think it would be useful for those who aren't
familiar with /sys
directory structure for drm. Finding documentation for this can be
hard and a tool is easier to maintain anyway.

Are you thinking maybe a separate binary called drm-info? (drm-utils
is a taken name by a
package that is seems not so useful in the context of the cam
application), etc? We list all
the equivalent cam information with the "cam -I -c1", it can be hard
to gather the equivalent
display info and if we can get them both at the same place, same
binary, would be nice!

>
> > Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> > ---
> >  src/cam/main.cpp | 31 ++++++++++++++++++++++++++-----
> >  src/cam/main.h   |  3 +++
> >  2 files changed, 29 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index c7f664b9..319cbf4d 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -15,6 +15,11 @@
> >  #include <libcamera/property_ids.h>
> >
> >  #include "camera_session.h"
> > +
> > +#ifdef HAVE_KMS
> > +#include "drm.h"
> > +#endif
> > +
> >  #include "event_loop.h"
> >  #include "main.h"
> >  #include "options.h"
> > @@ -137,6 +142,7 @@ int CamApp::parseOptions(int argc, char *argv[])
> >                        "Display viewfinder through DRM/KMS on specified connector",
> >                        "display", ArgumentOptional, "connector", false,
> >                        OptCamera);
> > +     parser.addOption(OptListDisplays, OptionNone, "List all displays", "list-displays");
> >  #endif
> >       parser.addOption(OptFile, OptionString,
> >                        "Write captured frames to disk\n"
> > @@ -202,7 +208,22 @@ int CamApp::run()
> >               }
> >       }
> >
> > -     /* 2. Create the camera sessions. */
> > +#ifdef HAVE_KMS
> > +     /* 2. List all displays. */
> > +     if (options_.isSet(OptListDisplays)) {
> > +             std::cout << "Available displays:" << std::endl;
> > +
> > +             DRM::Device dev;
> > +             dev.init();
>
> As Umang mentioned, errors should be checked.
>
> > +             for (const DRM::Connector &conn : dev.connectors()) {
> > +                     if (conn.status() == DRM::Connector::Connected) {
> > +                             std::cout << conn.name() << std::endl;
> > +                     }
>
> No need for curly braces in the inner if.
>
> > +             }
> > +     }
> > +#endif
> > +
> > +     /* 3. Create the camera sessions. */
> >       std::vector<std::unique_ptr<CameraSession>> sessions;
> >
> >       if (options_.isSet(OptCamera)) {
> > @@ -229,7 +250,7 @@ int CamApp::run()
> >               }
> >       }
> >
> > -     /* 3. Print camera information. */
> > +     /* 4. Print camera information. */
> >       if (options_.isSet(OptListControls) ||
> >           options_.isSet(OptListProperties) ||
> >           options_.isSet(OptInfo)) {
> > @@ -243,7 +264,7 @@ int CamApp::run()
> >               }
> >       }
> >
> > -     /* 4. Start capture. */
> > +     /* 5. Start capture. */
> >       for (const auto &session : sessions) {
> >               if (!session->options().isSet(OptCapture))
> >                       continue;
> > @@ -257,7 +278,7 @@ int CamApp::run()
> >               loopUsers_++;
> >       }
> >
> > -     /* 5. Enable hotplug monitoring. */
> > +     /* 6. Enable hotplug monitoring. */
> >       if (options_.isSet(OptMonitor)) {
> >               std::cout << "Monitoring new hotplug and unplug events" << std::endl;
> >               std::cout << "Press Ctrl-C to interrupt" << std::endl;
> > @@ -271,7 +292,7 @@ int CamApp::run()
> >       if (loopUsers_)
> >               loop_.exec();
> >
> > -     /* 6. Stop capture. */
> > +     /* 7. Stop capture. */
> >       for (const auto &session : sessions) {
> >               if (!session->options().isSet(OptCapture))
> >                       continue;
> > diff --git a/src/cam/main.h b/src/cam/main.h
> > index 1c2fab76..72050d27 100644
> > --- a/src/cam/main.h
> > +++ b/src/cam/main.h
> > @@ -21,6 +21,9 @@ enum {
> >       OptListControls = 256,
> >       OptStrictFormats = 257,
> >       OptMetadata = 258,
> > +#ifdef HAVE_KMS
> > +     OptListDisplays = 259
> > +#endif
> >  };
> >
> >  #endif /* __CAM_MAIN_H__ */
>
> --
> Regards,
>
> Laurent Pinchart
>

Is mise le meas/Regards,

Eric Curtin