[PATCH v4 0/2] tools/igt_power: fix Xe crash via drmtest path helpers

[PATCH v4 0/2] tools/igt_power: fix Xe crash via drmtest path helpers

[Sebastian Brzezinka]

igt_power opened its DRM device fd with a raw open() call, which skips
the Xe device cache initialisation performed by the drmtest open
helpers. This caused an assertion failure in xe_query the moment
igt_power_open() called into is_intel_dgfx() on a Xe device.

The fix is split into two patches. The first adds __drm_open_driver_path()
to lib/drmtest, a non-asserting helper that validates the fd is a real DRM
device (DRM_IOCTL_VERSION) before initialising the Xe cache. The second
switches igt_power to that helper and its matching __drm_close_driver(),
completely removing the direct Xe dependency from the tool.

v4:
- rename drm_open_driver_path to __drm_open_driver_path (no asserts/requires)
- add DRM_IOCTL_VERSION check to verify fd is a DRM device before proceeding
- split the single patch into two: lib helper addition and igt_power

v3:
- remove redundant if (fd >= 0) guards on both close call sites

v2:
- move Xe cache setup/teardown out of igt_power
- add drm_open_driver_path() and use __drm_close_driver()
- drop the direct xe/xe_query dependency from igt_power
   
Sebastian Brzezinka (2):
  lib/drmtest: add __drm_open_driver_path()
  tools/igt_power: fix crash on Xe devices by initializing xe_device
    cache

 lib/drmtest.c     | 30 ++++++++++++++++++++++++++++++
 lib/drmtest.h     |  1 +
 tools/igt_power.c |  6 +++---
 3 files changed, 34 insertions(+), 3 deletions(-)

-- 
2.53.0

[PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Sebastian Brzezinka]

Add __drm_open_driver_path() as a path-based DRM open helper. It opens
the given path with O_RDWR, verifies the resulting fd is a DRM device by
issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
(returning -1 and closing on failure), logs the opened device path, and
populates the xe_device cache when the device is Xe.

The __ prefix signals that the function does not throw assertions or
igt_require() calls, consistent with the existing __drm_open_driver*
family.

Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 lib/drmtest.c | 30 ++++++++++++++++++++++++++++++
 lib/drmtest.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 4a788ea7a..7007569d2 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -379,6 +379,36 @@ err:
 	return -1;
 }
 
+/**
+ * __drm_open_driver_path:
+ * @path: DRM device node path
+ *
+ * Opens @path with O_RDWR and populates the xe_device cache for Xe fds.
+ *
+ * Returns: DRM file descriptor or -1 on error
+ */
+int __drm_open_driver_path(const char *path)
+{
+	char name[12] = "";
+	int fd;
+
+	fd = open(path, O_RDWR);
+	if (fd < 0)
+		return -1;
+
+	if (__get_drm_device_name(fd, name, sizeof(name) - 1)) {
+		close(fd);
+		return -1;
+	}
+
+	log_opened_device_path(path);
+
+	if (is_xe_device(fd))
+		xe_device_get(fd);
+
+	return fd;
+}
+
 static struct {
 	int fd;
 	struct stat stat;
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 37874d729..305bbc86b 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -123,6 +123,7 @@ int drm_open_driver_another(int idx, int chipset);
 int drm_open_driver(int chipset);
 int drm_open_driver_master(int chipset);
 int drm_open_driver_render(int chipset);
+int __drm_open_driver_path(const char *path);
 int __drm_open_driver_another(int idx, int chipset);
 int __drm_open_driver(int chipset);
 int __drm_open_driver_render(int chipset);
-- 
2.53.0

[PATCH v4 2/2] tools/igt_power: fix crash on Xe devices by initializing xe_device cache

[Sebastian Brzezinka]

Opening a DRM device with plain open() does not initialize the Xe device
cache. When igt_power_open() calls is_intel_dgfx() -> xe_has_vram(),
it reaches find_in_cache() which asserts the cache entry exists, causing
a crash on Xe devices with: xe_query-CRITICAL: Failed assertion: xe_dev

Switch both open/close call sites to __drm_open_driver_path() and
__drm_close_driver() so that Xe cache setup and teardown are paired with
the fd lifecycle, keeping igt_power free of Xe-specific handling.

Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 tools/igt_power.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/igt_power.c b/tools/igt_power.c
index 6dd180d34..97d95e7b4 100644
--- a/tools/igt_power.c
+++ b/tools/igt_power.c
@@ -38,7 +38,7 @@ static bool prepare(struct measurement *m)
 		int ret, fd = -1;
 
 		if (m->drm_device) {
-			fd = open(m->drm_device, O_RDONLY);
+			fd = __drm_open_driver_path(m->drm_device);
 			if (fd < 0) {
 				fprintf(stderr, "Unable to open drm device %s (%d)\n",
 					m->drm_device, -errno);
@@ -54,12 +54,12 @@ static bool prepare(struct measurement *m)
 			else
 				fprintf(stderr, "Unable to open rapl domain %s (%d)\n",
 					m->rapl_domain, ret);
-			close(fd);
 
+			__drm_close_driver(fd);
 			return false;
 		}
 
-		close(fd);
+		__drm_close_driver(fd);
 	}
 
 	return true;
-- 
2.53.0

Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Jani Nikula]

On Tue, 26 May 2026, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
> the given path with O_RDWR, verifies the resulting fd is a DRM device by
> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
> (returning -1 and closing on failure), logs the opened device path, and
> populates the xe_device cache when the device is Xe.
>
> The __ prefix signals that the function does not throw assertions or
> igt_require() calls, consistent with the existing __drm_open_driver*
> family.

FWIW, I think the __ prefix should indicate "implementation detail,
don't call directly" or something along those lines.

I think it's problematic to encourage tests to call __ prefixed
functions.


BR,
Jani.


>
> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> ---
>  lib/drmtest.c | 30 ++++++++++++++++++++++++++++++
>  lib/drmtest.h |  1 +
>  2 files changed, 31 insertions(+)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 4a788ea7a..7007569d2 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -379,6 +379,36 @@ err:
>  	return -1;
>  }
>  
> +/**
> + * __drm_open_driver_path:
> + * @path: DRM device node path
> + *
> + * Opens @path with O_RDWR and populates the xe_device cache for Xe fds.
> + *
> + * Returns: DRM file descriptor or -1 on error
> + */
> +int __drm_open_driver_path(const char *path)
> +{
> +	char name[12] = "";
> +	int fd;
> +
> +	fd = open(path, O_RDWR);
> +	if (fd < 0)
> +		return -1;
> +
> +	if (__get_drm_device_name(fd, name, sizeof(name) - 1)) {
> +		close(fd);
> +		return -1;
> +	}
> +
> +	log_opened_device_path(path);
> +
> +	if (is_xe_device(fd))
> +		xe_device_get(fd);
> +
> +	return fd;
> +}
> +
>  static struct {
>  	int fd;
>  	struct stat stat;
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 37874d729..305bbc86b 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -123,6 +123,7 @@ int drm_open_driver_another(int idx, int chipset);
>  int drm_open_driver(int chipset);
>  int drm_open_driver_master(int chipset);
>  int drm_open_driver_render(int chipset);
> +int __drm_open_driver_path(const char *path);
>  int __drm_open_driver_another(int idx, int chipset);
>  int __drm_open_driver(int chipset);
>  int __drm_open_driver_render(int chipset);

-- 
Jani Nikula, Intel

Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Sebastian Brzezinka]

Hi Jani,

On Tue May 26, 2026 at 1:51 PM CEST, Jani Nikula wrote:
> On Tue, 26 May 2026, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
>> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
>> the given path with O_RDWR, verifies the resulting fd is a DRM device by
>> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
>> (returning -1 and closing on failure), logs the opened device path, and
>> populates the xe_device cache when the device is Xe.
>>
>> The __ prefix signals that the function does not throw assertions or
>> igt_require() calls, consistent with the existing __drm_open_driver*
>> family.
>
> FWIW, I think the __ prefix should indicate "implementation detail,
> don't call directly" or something along those lines.
>
> I think it's problematic to encourage tests to call __ prefixed
> functions.
In general I agree with you, but in v3 Kamil comment on this:
```
>>Could you rename it into __drm_open_driver_path?
>>The idea is that all __functions should not throw asserts nor
>>require.
```

So I added an explanation in the comment.

-- 
Best regards,
Sebastian

Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Jani Nikula]

On Tue, 26 May 2026, "Sebastian Brzezinka" <sebastian.brzezinka@intel.com> wrote:
> Hi Jani,
>
> On Tue May 26, 2026 at 1:51 PM CEST, Jani Nikula wrote:
>> On Tue, 26 May 2026, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
>>> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
>>> the given path with O_RDWR, verifies the resulting fd is a DRM device by
>>> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
>>> (returning -1 and closing on failure), logs the opened device path, and
>>> populates the xe_device cache when the device is Xe.
>>>
>>> The __ prefix signals that the function does not throw assertions or
>>> igt_require() calls, consistent with the existing __drm_open_driver*
>>> family.
>>
>> FWIW, I think the __ prefix should indicate "implementation detail,
>> don't call directly" or something along those lines.
>>
>> I think it's problematic to encourage tests to call __ prefixed
>> functions.
> In general I agree with you, but in v3 Kamil comment on this:
> ```
>>>Could you rename it into __drm_open_driver_path?
>>>The idea is that all __functions should not throw asserts nor
>>>require.
> ```
>
> So I added an explanation in the comment.

I obviously disagree with all of that, but *shrug*.

BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Kamil Konieczny]

Hi Jani,
On 2026-05-26 at 15:07:57 +0300, Jani Nikula wrote:
> On Tue, 26 May 2026, "Sebastian Brzezinka" <sebastian.brzezinka@intel.com> wrote:
> > Hi Jani,
> >
> > On Tue May 26, 2026 at 1:51 PM CEST, Jani Nikula wrote:
> >> On Tue, 26 May 2026, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
> >>> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
> >>> the given path with O_RDWR, verifies the resulting fd is a DRM device by
> >>> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
> >>> (returning -1 and closing on failure), logs the opened device path, and
> >>> populates the xe_device cache when the device is Xe.
> >>>
> >>> The __ prefix signals that the function does not throw assertions or
> >>> igt_require() calls, consistent with the existing __drm_open_driver*
> >>> family.
> >>
> >> FWIW, I think the __ prefix should indicate "implementation detail,
> >> don't call directly" or something along those lines.
> >>
> >> I think it's problematic to encourage tests to call __ prefixed
> >> functions.


> > In general I agree with you, but in v3 Kamil comment on this:
> > ```
> >>>Could you rename it into __drm_open_driver_path?
> >>>The idea is that all __functions should not throw asserts nor
> >>>require.
> > ```
> >
> > So I added an explanation in the comment.
> 
> I obviously disagree with all of that, but *shrug*.

We could implement separete lib functions for tools but
that will require duplicate effort.

The idea with __function is that it should be implemented
without any igt_assert nor igt_require so tools could use them.
We could go with function() but then developers could
later on change behaviour and break a tool.

Or we could add some suffix to a name? function_safe()?

Btw why tool/igt_power couldn't just read sysfs? Why does it
needs to open /dev/dri/card* to get any info? +cc Ville

Regards,
Kamil

> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Ville Syrjälä]

On Wed, May 27, 2026 at 03:22:47PM +0200, Kamil Konieczny wrote:
> Hi Jani,
> On 2026-05-26 at 15:07:57 +0300, Jani Nikula wrote:
> > On Tue, 26 May 2026, "Sebastian Brzezinka" <sebastian.brzezinka@intel.com> wrote:
> > > Hi Jani,
> > >
> > > On Tue May 26, 2026 at 1:51 PM CEST, Jani Nikula wrote:
> > >> On Tue, 26 May 2026, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
> > >>> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
> > >>> the given path with O_RDWR, verifies the resulting fd is a DRM device by
> > >>> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
> > >>> (returning -1 and closing on failure), logs the opened device path, and
> > >>> populates the xe_device cache when the device is Xe.
> > >>>
> > >>> The __ prefix signals that the function does not throw assertions or
> > >>> igt_require() calls, consistent with the existing __drm_open_driver*
> > >>> family.
> > >>
> > >> FWIW, I think the __ prefix should indicate "implementation detail,
> > >> don't call directly" or something along those lines.
> > >>
> > >> I think it's problematic to encourage tests to call __ prefixed
> > >> functions.
> 
> 
> > > In general I agree with you, but in v3 Kamil comment on this:
> > > ```
> > >>>Could you rename it into __drm_open_driver_path?
> > >>>The idea is that all __functions should not throw asserts nor
> > >>>require.
> > > ```
> > >
> > > So I added an explanation in the comment.
> > 
> > I obviously disagree with all of that, but *shrug*.
> 
> We could implement separete lib functions for tools but
> that will require duplicate effort.
> 
> The idea with __function is that it should be implemented
> without any igt_assert nor igt_require so tools could use them.
> We could go with function() but then developers could
> later on change behaviour and break a tool.
> 
> Or we could add some suffix to a name? function_safe()?
> 
> Btw why tool/igt_power couldn't just read sysfs? Why does it
> needs to open /dev/dri/card* to get any info? +cc Ville

It opens the device just because lib/igt_power and its
dependencies want the fd.

-- 
Ville Syrjälä
Intel

Re: [PATCH v4 0/2] tools/igt_power: fix Xe crash via drmtest path helpers

[Kamil Konieczny]

Hi Sebastian,
On 2026-05-26 at 13:39:52 +0200, Sebastian Brzezinka wrote:

Please keep i-g-t string after PATCH, so it will look like:

[PATCH i-g-t v4 0/2] tools/igt_power: fix Xe crash via drmtest path

Maybe that is a reason why it is not on igt patchwork?

Regards,
Kamil

> igt_power opened its DRM device fd with a raw open() call, which skips
> the Xe device cache initialisation performed by the drmtest open
> helpers. This caused an assertion failure in xe_query the moment
> igt_power_open() called into is_intel_dgfx() on a Xe device.
> 
> The fix is split into two patches. The first adds __drm_open_driver_path()
> to lib/drmtest, a non-asserting helper that validates the fd is a real DRM
> device (DRM_IOCTL_VERSION) before initialising the Xe cache. The second
> switches igt_power to that helper and its matching __drm_close_driver(),
> completely removing the direct Xe dependency from the tool.
> 
> v4:
> - rename drm_open_driver_path to __drm_open_driver_path (no asserts/requires)
> - add DRM_IOCTL_VERSION check to verify fd is a DRM device before proceeding
> - split the single patch into two: lib helper addition and igt_power
> 
> v3:
> - remove redundant if (fd >= 0) guards on both close call sites
> 
> v2:
> - move Xe cache setup/teardown out of igt_power
> - add drm_open_driver_path() and use __drm_close_driver()
> - drop the direct xe/xe_query dependency from igt_power
>    
> Sebastian Brzezinka (2):
>   lib/drmtest: add __drm_open_driver_path()
>   tools/igt_power: fix crash on Xe devices by initializing xe_device
>     cache
> 
>  lib/drmtest.c     | 30 ++++++++++++++++++++++++++++++
>  lib/drmtest.h     |  1 +
>  tools/igt_power.c |  6 +++---
>  3 files changed, 34 insertions(+), 3 deletions(-)
> 
> -- 
> 2.53.0
> 

[PATCH i-g-t v4 0/2] tools/igt_power: fix Xe crash via drmtest path helpers

[Sebastian Brzezinka]

igt_power opened its DRM device fd with a raw open() call, which skips
the Xe device cache initialisation performed by the drmtest open
helpers. This caused an assertion failure in xe_query the moment
igt_power_open() called into is_intel_dgfx() on a Xe device.

The fix is split into two patches. The first adds __drm_open_driver_path()
to lib/drmtest, a non-asserting helper that validates the fd is a real DRM
device (DRM_IOCTL_VERSION) before initialising the Xe cache. The second
switches igt_power to that helper and its matching __drm_close_driver(),
completely removing the direct Xe dependency from the tool.

v4:
- rename drm_open_driver_path to __drm_open_driver_path (no asserts/requires)
- add DRM_IOCTL_VERSION check to verify fd is a DRM device before proceeding
- split the single patch into two: lib helper addition and igt_power

v3:
- remove redundant if (fd >= 0) guards on both close call sites

v2:
- move Xe cache setup/teardown out of igt_power
- add drm_open_driver_path() and use __drm_close_driver()
- drop the direct xe/xe_query dependency from igt_power
   
Sebastian Brzezinka (2):
  lib/drmtest: add __drm_open_driver_path()
  tools/igt_power: fix crash on Xe devices by initializing xe_device
    cache

 lib/drmtest.c     | 30 ++++++++++++++++++++++++++++++
 lib/drmtest.h     |  1 +
 tools/igt_power.c |  6 +++---
 3 files changed, 34 insertions(+), 3 deletions(-)

-- 
2.53.0

[PATCH i-g-t v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Sebastian Brzezinka]

Add __drm_open_driver_path() as a path-based DRM open helper. It opens
the given path with O_RDWR, verifies the resulting fd is a DRM device by
issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
(returning -1 and closing on failure), logs the opened device path, and
populates the xe_device cache when the device is Xe.

The __ prefix signals that the function does not throw assertions or
igt_require() calls, consistent with the existing __drm_open_driver*
family.

Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 lib/drmtest.c | 30 ++++++++++++++++++++++++++++++
 lib/drmtest.h |  1 +
 2 files changed, 31 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 4a788ea7a..7007569d2 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -379,6 +379,36 @@ err:
 	return -1;
 }
 
+/**
+ * __drm_open_driver_path:
+ * @path: DRM device node path
+ *
+ * Opens @path with O_RDWR and populates the xe_device cache for Xe fds.
+ *
+ * Returns: DRM file descriptor or -1 on error
+ */
+int __drm_open_driver_path(const char *path)
+{
+	char name[12] = "";
+	int fd;
+
+	fd = open(path, O_RDWR);
+	if (fd < 0)
+		return -1;
+
+	if (__get_drm_device_name(fd, name, sizeof(name) - 1)) {
+		close(fd);
+		return -1;
+	}
+
+	log_opened_device_path(path);
+
+	if (is_xe_device(fd))
+		xe_device_get(fd);
+
+	return fd;
+}
+
 static struct {
 	int fd;
 	struct stat stat;
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 37874d729..305bbc86b 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -123,6 +123,7 @@ int drm_open_driver_another(int idx, int chipset);
 int drm_open_driver(int chipset);
 int drm_open_driver_master(int chipset);
 int drm_open_driver_render(int chipset);
+int __drm_open_driver_path(const char *path);
 int __drm_open_driver_another(int idx, int chipset);
 int __drm_open_driver(int chipset);
 int __drm_open_driver_render(int chipset);
-- 
2.53.0

[PATCH i-g-t v4 2/2] tools/igt_power: fix crash on Xe devices by initializing xe_device cache

[Sebastian Brzezinka]

Opening a DRM device with plain open() does not initialize the Xe device
cache. When igt_power_open() calls is_intel_dgfx() -> xe_has_vram(),
it reaches find_in_cache() which asserts the cache entry exists, causing
a crash on Xe devices with: xe_query-CRITICAL: Failed assertion: xe_dev

Switch both open/close call sites to __drm_open_driver_path() and
__drm_close_driver() so that Xe cache setup and teardown are paired with
the fd lifecycle, keeping igt_power free of Xe-specific handling.

Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
---
 tools/igt_power.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/igt_power.c b/tools/igt_power.c
index 6dd180d34..97d95e7b4 100644
--- a/tools/igt_power.c
+++ b/tools/igt_power.c
@@ -38,7 +38,7 @@ static bool prepare(struct measurement *m)
 		int ret, fd = -1;
 
 		if (m->drm_device) {
-			fd = open(m->drm_device, O_RDONLY);
+			fd = __drm_open_driver_path(m->drm_device);
 			if (fd < 0) {
 				fprintf(stderr, "Unable to open drm device %s (%d)\n",
 					m->drm_device, -errno);
@@ -54,12 +54,12 @@ static bool prepare(struct measurement *m)
 			else
 				fprintf(stderr, "Unable to open rapl domain %s (%d)\n",
 					m->rapl_domain, ret);
-			close(fd);
 
+			__drm_close_driver(fd);
 			return false;
 		}
 
-		close(fd);
+		__drm_close_driver(fd);
 	}
 
 	return true;
-- 
2.53.0

Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Kamil Konieczny]

Hi Sebastian,
On 2026-05-26 at 13:39:53 +0200, Sebastian Brzezinka wrote:
> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
> the given path with O_RDWR, verifies the resulting fd is a DRM device by
> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
> (returning -1 and closing on failure), logs the opened device path, and
> populates the xe_device cache when the device is Xe.
> 
> The __ prefix signals that the function does not throw assertions or
> igt_require() calls, consistent with the existing __drm_open_driver*
> family.
> 
> Signed-off-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
> ---
>  lib/drmtest.c | 30 ++++++++++++++++++++++++++++++
>  lib/drmtest.h |  1 +
>  2 files changed, 31 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 4a788ea7a..7007569d2 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -379,6 +379,36 @@ err:
>  	return -1;
>  }
>  
> +/**
> + * __drm_open_driver_path:
> + * @path: DRM device node path
> + *
> + * Opens @path with O_RDWR and populates the xe_device cache for Xe fds.
> + *
> + * Returns: DRM file descriptor or -1 on error
> + */
> +int __drm_open_driver_path(const char *path)

This should be

int drm_open_driver_path(const char *path)

see below.

> +{
> +	char name[12] = "";
> +	int fd;
> +
> +	fd = open(path, O_RDWR);
> +	if (fd < 0)
> +		return -1;
> +
> +	if (__get_drm_device_name(fd, name, sizeof(name) - 1)) {
> +		close(fd);
> +		return -1;
> +	}
> +

Please verify here that name[0] != 0.

> +	log_opened_device_path(path);
> +
> +	if (is_xe_device(fd))
> +		xe_device_get(fd);

xe_device_get() uses igt_assert() so use drm_open_driver_path()
as function name.

Regards,
Kamil

> +
> +	return fd;
> +}
> +
>  static struct {
>  	int fd;
>  	struct stat stat;
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 37874d729..305bbc86b 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -123,6 +123,7 @@ int drm_open_driver_another(int idx, int chipset);
>  int drm_open_driver(int chipset);
>  int drm_open_driver_master(int chipset);
>  int drm_open_driver_render(int chipset);
> +int __drm_open_driver_path(const char *path);
>  int __drm_open_driver_another(int idx, int chipset);
>  int __drm_open_driver(int chipset);
>  int __drm_open_driver_render(int chipset);
> -- 
> 2.53.0
> 

Re: [PATCH v4 1/2] lib/drmtest: add __drm_open_driver_path()

[Gustavo Sousa]

Kamil Konieczny <kamil.konieczny@linux.intel.com> writes:

> Hi Jani,
> On 2026-05-26 at 15:07:57 +0300, Jani Nikula wrote:
>> On Tue, 26 May 2026, "Sebastian Brzezinka" <sebastian.brzezinka@intel.com> wrote:
>> > Hi Jani,
>> >
>> > On Tue May 26, 2026 at 1:51 PM CEST, Jani Nikula wrote:
>> >> On Tue, 26 May 2026, Sebastian Brzezinka <sebastian.brzezinka@intel.com> wrote:
>> >>> Add __drm_open_driver_path() as a path-based DRM open helper. It opens
>> >>> the given path with O_RDWR, verifies the resulting fd is a DRM device by
>> >>> issuing DRM_IOCTL_VERSION via __get_drm_device_name() before proceeding
>> >>> (returning -1 and closing on failure), logs the opened device path, and
>> >>> populates the xe_device cache when the device is Xe.
>> >>>
>> >>> The __ prefix signals that the function does not throw assertions or
>> >>> igt_require() calls, consistent with the existing __drm_open_driver*
>> >>> family.
>> >>
>> >> FWIW, I think the __ prefix should indicate "implementation detail,
>> >> don't call directly" or something along those lines.
>> >>
>> >> I think it's problematic to encourage tests to call __ prefixed
>> >> functions.
>
>
>> > In general I agree with you, but in v3 Kamil comment on this:
>> > ```
>> >>>Could you rename it into __drm_open_driver_path?
>> >>>The idea is that all __functions should not throw asserts nor
>> >>>require.
>> > ```
>> >
>> > So I added an explanation in the comment.
>> 
>> I obviously disagree with all of that, but *shrug*.
>
> We could implement separete lib functions for tools but
> that will require duplicate effort.
>
> The idea with __function is that it should be implemented
> without any igt_assert nor igt_require so tools could use them.
> We could go with function() but then developers could
> later on change behaviour and break a tool.
>
> Or we could add some suffix to a name? function_safe()?

Given the number of assertions that already exist under lib/, perhaps
this would be somewhat hard to achieve, but I feel like we should go the
other way around: a suffix to denote that a function uses IGT
assertions.

I.o.w., do_something() would be usable from both test and tools code;
do_something_asserting() would be a version of the same function that
contains assertions.

"asserting" is a bit long for a prefix, we could try to come up with
something better.

--
Gustavo Sousa

>
> Btw why tool/igt_power couldn't just read sysfs? Why does it
> needs to open /dev/dri/card* to get any info? +cc Ville
>
> Regards,
> Kamil
>
>> 
>> BR,
>> Jani.
>> 
>> 
>> -- 
>> Jani Nikula, Intel