Re: [PATCH i-g-t v5] lib/drmtest: Create proper error report when open driver fails

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 06 May 2026, Kamil Konieczny <kamil.konieczny@linux.intel.com> wrote:
> When first try for opening driver fails then opening function is
> searching for next available device. When that also fails
> reporting is printing last errno which could mislead developer:
>
> $ build/tests/kms_dp_aux_dev
> IGT-Version: 2.3-NO-GIT (x86_64) (Linux: 6.17.0-19-generic x86_64)
> Test requirement not met in function drm_open_driver, file ../lib/drmtest.c:754:
> Test requirement: !(fd<0)
> No known gpu found for chipset flags 0x4294965755 (any)
> Last errno: 2, No such file or directory
> SKIP (0.006s)
>
> ls /dev/dri
> by-path  card1  renderD128
>
> The real problem here is lack of permissions as there was a card
> but program could not open it. Break looking for a card as soon
> as an error is different from ENOENT. It will create a proper
> error report which will print:
>
> Last errno: 13, Permission denied
>
> v3: break only when errno actually happens in open() (Kamil)
> v4: removed 'else', removed errno printing (Zbigniew)
> v5: use explicit strerror() (Kamil)
>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Karthik B S <karthik.b.s@intel.com>
> Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
>  lib/drmtest.c          | 26 +++++++++++++++++++++-----
>  lib/drmtest.h          |  2 +-
>  lib/igt_sriov_device.c |  2 +-
>  3 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 4a788ea7a..084737edf 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -323,6 +323,7 @@ static void log_opened_device_path(const char *device_path)
>   * __drm_open_device:
>   * @name: DRM node name
>   * @chipset: OR'd flags for chipset to be opened
> + * @error: pointer for saving errno when open() fails
>   *
>   * Open a drm legacy device node with given @name and compatible with given
>   * @chipset flag.
> @@ -334,8 +335,10 @@ static void log_opened_device_path(const char *device_path)
>   * name, even when driver was excluded by ANY or is not listed in known drivers.
>   *
>   * Returns: DRM file descriptor or -1 on error
> + *
> + * When open() succeeds, sets @error to zero otherwise to errno
>   */
> -int __drm_open_device(const char *name, unsigned int chipset)
> +int __drm_open_device(const char *name, unsigned int chipset, int *error)

Why can't you just return -errno? I think this is unnecessarily
complicated.

BR,
Jani.

>  {
>  	const char *forced;
>  	char dev_name[16] = "";
> @@ -343,8 +346,15 @@ int __drm_open_device(const char *name, unsigned int chipset)
>  	int fd;
>  
>  	fd = open(name, O_RDWR);
> -	if (fd == -1)
> +	if (fd == -1) {
> +		if (error)
> +			*error = errno;
> +
>  		return -1;
> +	}
> +
> +	if (error)
> +		*error = 0;
>  
>  	if (__get_drm_device_name(fd, dev_name, sizeof(dev_name) - 1) == -1)
>  		goto err;
> @@ -426,6 +436,7 @@ static bool _is_already_opened(const char *path, int as_idx)
>  static int __search_and_open(const char *base, int offset, unsigned int chipset, int as_idx)
>  {
>  	const char *forced;
> +	int err;
>  
>  	forced = forced_driver();
>  	if (forced)
> @@ -440,9 +451,14 @@ static int __search_and_open(const char *base, int offset, unsigned int chipset,
>  		if (_is_already_opened(name, as_idx))
>  			continue;
>  
> -		fd = __drm_open_device(name, chipset);
> +		fd = __drm_open_device(name, chipset, &err);
>  		if (fd != -1)
>  			return fd;
> +
> +		if (err) {
> +			igt_debug("Error at open %s %s\n", name, strerror(err));
> +			break;
> +		}
>  	}
>  
>  	return -1;
> @@ -500,13 +516,13 @@ static int __open_driver_exact(const char *name, unsigned int chipset)
>  {
>  	int fd;
>  
> -	fd = __drm_open_device(name, chipset);
> +	fd = __drm_open_device(name, chipset, NULL);
>  	if (fd != -1)
>  		return fd;
>  
>  	drm_load_module(chipset);
>  
> -	return __drm_open_device(name, chipset);
> +	return __drm_open_device(name, chipset, NULL);
>  }
>  
>  /*
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 37874d729..126feffda 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -117,7 +117,7 @@ unsigned int drm_get_chipset(int fd);
>   */
>  #define IS_ALIGNED(v, a)	(((v) & ((typeof(v))(a) - 1)) == 0)
>  
> -int __drm_open_device(const char *name, unsigned int chipset);
> +int __drm_open_device(const char *name, unsigned int chipset, int *error);
>  void drm_load_module(unsigned int chipset);
>  int drm_open_driver_another(int idx, int chipset);
>  int drm_open_driver(int chipset);
> diff --git a/lib/igt_sriov_device.c b/lib/igt_sriov_device.c
> index 1f4c3ac04..21d23298b 100644
> --- a/lib/igt_sriov_device.c
> +++ b/lib/igt_sriov_device.c
> @@ -284,7 +284,7 @@ int igt_sriov_open_vf_drm_device(int pf, unsigned int vf_num)
>  	if (!found)
>  		return -1;
>  
> -	fd = __drm_open_device(dev_name, DRIVER_ANY);
> +	fd = __drm_open_device(dev_name, DRIVER_ANY, NULL);
>  	if (fd >= 0 && is_xe_device(fd))
>  		xe_device_get(fd);

-- 
Jani Nikula, Intel