[libcamera-devel] [PATCH v2] cam: drm: Skip DRM devices not capable of mode setting

[Eric Curtin]

On Wed, 28 Sept 2022 at 11:27, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> The DRM helper picks the first DRM card that it can open. On platforms
> that have a standalone GPU, this risks selecting a device corresponding
> to the GPU instead of the display controller. Fix this by skipping
> devices that don't support the KMS mode setting API. Some legacy display
> controllers would be skipped as well, but libcamera doesn't run on those
> systems anyway.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes since v1:
>
> - Use DRM_CAP_DUMB_BUFFER instead of DRM_CLIENT_CAP_ATOMIC
> ---
>  src/cam/drm.cpp | 35 +++++++++++++++++++++++++++--------
>  1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp
> index b0602c942853..3bb950fd157a 100644
> --- a/src/cam/drm.cpp
> +++ b/src/cam/drm.cpp
> @@ -430,7 +430,8 @@ int Device::init()
>  int Device::openCard()
>  {
>         const std::string dirName = "/dev/dri/";
> -       int ret = -ENOENT;
> +       bool found = false;
> +       int ret;
>
>         /*
>          * Open the first DRM/KMS device beginning with /dev/dri/card. The
> @@ -449,24 +450,42 @@ int Device::openCard()
>         }
>
>         for (struct dirent *res; (res = readdir(folder));) {
> +               uint64_t cap;
> +
>                 if (strncmp(res->d_name, "card", 4))
>                         continue;
>
>                 const std::string devName = dirName + res->d_name;
>                 fd_ = open(devName.c_str(), O_RDWR | O_CLOEXEC);
> -               if (fd_ >= 0) {
> -                       ret = 0;
> -                       break;
> +               if (fd_ < 0) {
> +                       ret = -errno;
> +                       std::cerr << "Failed to open DRM/KMS device " << devName << ": "
> +                                 << strerror(-ret) << std::endl;
> +                       continue;
>                 }
>
> -               ret = -errno;
> -               std::cerr << "Failed to open DRM/KMS device " << devName << ": "
> -                         << strerror(-ret) << std::endl;
> +               /*
> +                * Skip devices that don't support the modeset API, to avoid
> +                * selecting a DRM device corresponding to a GPU. There is no
> +                * modeset capability, but the kernel returns an error for most
> +                * caps if mode setting isn't support but the driver. The
> +                * DRM_CAP_DUMB_BUFFER capability is one of those, other would
> +                * do as well. The capability value itself isn't relevant.
> +                */
> +               ret = drmGetCap(fd_, DRM_CAP_DUMB_BUFFER, &cap);
> +               if (ret < 0) {

Since this ends up being an ioctl I guess this is fine. In modetest.c
they do it the `if (ret || cap == 0)` way, but a failed ioctl returns
-1 so this is fine too. Plus the author of that code in modetest.c is
Laurent Pinchart in 2014 :)

Reviewed-by: Eric Curtin <ecurtin at redhat.com>


> +                       drmClose(fd_);
> +                       fd_ = -1;
> +                       continue;
> +               }
> +
> +               found = true;
> +               break;
>         }
>
>         closedir(folder);
>
> -       return ret;
> +       return found ? 0 : -ENOENT;
>  }
>
>  int Device::getResources()
> --
> Regards,
>
> Laurent Pinchart
>