[libcamera-devel] [PATCH] cam: drm: Support /dev/dri cards other than 0

[Jacopo Mondi]

Hi Eric

On Wed, Jun 01, 2022 at 04:23:45PM +0100, Eric Curtin via libcamera-devel wrote:
> Existing code is hardcoded to card0. Since recent fedora upgrades, we
> have noticed on more than one machine that card1 is present as the
> lowest numbered device, could theoretically be higher. This technique
> tries every file starting with card and continue only when we have
> successfully opened one. These devices with card1 as the lowest device
> were simply failing when they do not see a /dev/dri/card0 file present.
>
> Reported-by: Ian Mullins <imullins at redhat.com>
> Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> ---
>  src/cam/drm.cpp | 37 +++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/src/cam/drm.cpp b/src/cam/drm.cpp
> index 42c5a3b1..5a322819 100644
> --- a/src/cam/drm.cpp
> +++ b/src/cam/drm.cpp
> @@ -8,6 +8,7 @@
>  #include "drm.h"
>
>  #include <algorithm>
> +#include <dirent.h>
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <iostream>
> @@ -393,8 +394,10 @@ Device::~Device()
>
>  int Device::init()
>  {
> -	constexpr size_t NODE_NAME_MAX = sizeof("/dev/dri/card255");
> -	char name[NODE_NAME_MAX];
> +	constexpr size_t DIR_NAME_MAX = sizeof("/dev/dri/");
> +	constexpr size_t BASE_NAME_MAX = sizeof("card255");
> +	constexpr size_t NODE_NAME_MAX = DIR_NAME_MAX + BASE_NAME_MAX - 1;
> +	char name[NODE_NAME_MAX] = "/dev/dri/";
>  	int ret;
>
>  	/*
> @@ -404,14 +407,28 @@ int Device::init()
>  	 * from drmOpen() is of no practical use as any modern system will
>  	 * handle that through udev or an equivalent component.
>  	 */
> -	snprintf(name, sizeof(name), "/dev/dri/card%u", 0);
> -	fd_ = open(name, O_RDWR | O_CLOEXEC);
> -	if (fd_ < 0) {
> -		ret = -errno;
> -		std::cerr
> -			<< "Failed to open DRM/KMS device " << name << ": "
> -			<< strerror(-ret) << std::endl;
> -		return ret;
> +	DIR *folder = opendir(name);
> +	if (folder) {
> +		for (struct dirent *res; (res = readdir(folder));) {
> +			if (strlen(res->d_name) > 4 &&

I feel this might be a bit simplified, maybe using std::filesystem

	const std::filesystem::path dri("/dev/dri");
	for (const auto &dir : std::filesystem::directory_iterator(dri)) {
		const std::string &direntry = dir.path().filename().string();

		if (direntry.find("card") == std::string::npos)
			continue;

		fd_ = open(dir.path().string().c_str(), O_RDWR | O_CLOEXEC);

                ...
        }

> +			    !strncmp(res->d_name, "card", 4)) {
> +				memcpy(name + DIR_NAME_MAX - 1, res->d_name,
> +				       BASE_NAME_MAX);
> +				fd_ = open(name, O_RDWR | O_CLOEXEC);
> +				if (fd_ < 0) {
> +					ret = -errno;
> +					std::cerr
> +						<< "Failed to open DRM/KMS device "
> +						<< name << ": "
> +						<< strerror(-ret) << std::endl;
> +					continue;
> +				}
> +
> +				break;
> +			}
> +		}
> +
> +		closedir(folder);

What if no card is found ?
Should fd_ be initialized and here checked ?

Thanks
   j

>  	}
>
>  	/*
> --
> 2.35.3
>