[intel-gpu-tools,1/3] drmtest: Add non-i915 device open helpers

Submitted by Micah Fedke on April 21, 2015, 3:03 p.m.

Details

Message ID 1429628629-2727-2-git-send-email-micah.fedke@collabora.co.uk
State New
Headers show

Not browsing as part of any series.

Commit Message

Micah Fedke April 21, 2015, 3:03 p.m.
From: Daniel Stone <daniels@collabora.com>

Add helpers which will open a device which may or may not be i915 to
test on.

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 lib/drmtest.c     | 139 ++++++++++++++++++++++++++++++++++++++++++++++--------
 lib/drmtest.h     |   6 ++-
 tests/gem_alive.c |   2 +-
 3 files changed, 125 insertions(+), 22 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/drmtest.c b/lib/drmtest.c
index ee5c123..c73434e 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -190,16 +190,7 @@  void gem_quiescent_gpu(int fd)
 	gem_close(fd, handle);
 }
 
-/**
- * drm_get_card:
- *
- * Get an i915 drm card index number for use in /dev or /sys. The minor index of
- * the legacy node is returned, not of the control or render node.
- *
- * Returns:
- * The i915 drm index or -1 on error
- */
-int drm_get_card(void)
+static int __drm_get_card(bool anygpu)
 {
 	char *name;
 	int i, fd;
@@ -216,7 +207,7 @@  int drm_get_card(void)
 		if (fd == -1)
 			continue;
 
-		if (!is_i915_device(fd) || !is_intel(fd)) {
+		if (!anygpu && (!is_i915_device(fd) || !is_intel(fd))) {
 			close(fd);
 			continue;
 		}
@@ -230,8 +221,50 @@  int drm_get_card(void)
 	return -1;
 }
 
-/** Open the first DRM device we can find, searching up to 16 device nodes */
-int __drm_open_any(void)
+/**
+ * drm_get_card:
+ *
+ * Get an i915 drm card index number for use in /dev or /sys. The minor index of
+ * the legacy node is returned, not of the control or render node.
+ *
+ * The non-Intel-specific version is drm_get_any_card().
+ *
+ * Returns:
+ * The i915 drm index or -1 on error
+ */
+int drm_get_card(void)
+{
+	return __drm_get_card(false);
+}
+
+/**
+ * drm_get_any_card:
+ *
+ * Get any drm card index number for use in /dev or /sys. The minor index of
+ * the legacy node is returned, not of the control or render node.
+ *
+ * The Intel-specific version is drm_get_card().
+ *
+ * Returns:
+ * The drm index or -1 on error
+ */
+int drm_get_any_card(void)
+{
+	return __drm_get_card(true);
+}
+
+/**
+ * __drm_open_any:
+ *
+ * Open the first DRM device we can find, searching up to 16 device nodes
+ *
+ * @anygpu: If TRUE, will open any device; if FALSE, will only open Intel
+ *          devices
+ *
+ * Returns:
+ * An open DRM fd or -1 on error
+ */
+int __drm_open_any(bool anygpu)
 {
 	for (int i = 0; i < 16; i++) {
 		char name[80];
@@ -242,7 +275,7 @@  int __drm_open_any(void)
 		if (fd == -1)
 			continue;
 
-		if (is_i915_device(fd) && is_intel(fd))
+		if (anygpu || (is_i915_device(fd) && is_intel(fd)))
 			return fd;
 
 		close(fd);
@@ -252,7 +285,7 @@  int __drm_open_any(void)
 	return -1;
 }
 
-static int __drm_open_any_render(void)
+static int __drm_open_any_render(bool anygpu)
 {
 	char *name;
 	int i, fd;
@@ -269,7 +302,7 @@  static int __drm_open_any_render(void)
 		if (fd == -1)
 			continue;
 
-		if (!is_i915_device(fd) || !is_intel(fd)) {
+		if (!anygpu && (!is_i915_device(fd) || !is_intel(fd))) {
 			close(fd);
 			fd = -1;
 			continue;
@@ -312,12 +345,14 @@  static void quiescent_gpu_at_exit_render(int sig)
  * Open an i915 drm legacy device node. This function always returns a valid
  * file descriptor.
  *
+ * drm_open_any_any() is the non-Intel-specific version of this function.
+ *
  * Returns: a i915 drm file descriptor
  */
 int drm_open_any(void)
 {
 	static int open_count;
-	int fd = __drm_open_any();
+	int fd = __drm_open_any(false);
 
 	igt_require(fd >= 0);
 
@@ -325,13 +360,34 @@  int drm_open_any(void)
 		return fd;
 
 	gem_quiescent_gpu(fd);
-	at_exit_drm_fd = __drm_open_any();
+	at_exit_drm_fd = __drm_open_any(false);
 	igt_install_exit_handler(quiescent_gpu_at_exit);
 
 	return fd;
 }
 
 /**
+ * drm_open_any_any:
+ *
+ * Literally the worst-named function I've ever written.
+ *
+ * Open a drm legacy device node. This function always returns a valid
+ * file descriptor.
+ *
+ * drm_open_any() is the Intel-specific version of this function.
+ *
+ * Returns: a drm file descriptor
+ */
+int drm_open_any_any(void)
+{
+	int fd = __drm_open_any(true);
+
+	igt_require(fd >= 0);
+
+	return fd;
+}
+
+/**
  * drm_open_any_master:
  *
  * Open an i915 drm legacy device node and ensure that it is drm master.
@@ -351,17 +407,39 @@  int drm_open_any_master(void)
 }
 
 /**
+ * drm_open_any_master:
+ *
+ * Open an i915 drm legacy device node and ensure that it is drm master.
+ *
+ * Returns:
+ * The i915 drm file descriptor or -1 on error
+ */
+int drm_open_any_master_any(void)
+{
+	int fd = drm_open_any_any();
+
+	igt_require(fd >= 0);
+	igt_require_f(drmSetMaster(fd) == 0, "Can't become DRM master, "
+		      "please check if no other DRM client is running.\n");
+
+	return fd;
+}
+
+/**
  * drm_open_any_render:
  *
  * Open an i915 drm render device node.
  *
+ * drm_open_any_render_any() is the non-Intel-specific version of this
+ * function.
+ *
  * Returns:
  * The i915 drm file descriptor or -1 on error
  */
 int drm_open_any_render(void)
 {
 	static int open_count;
-	int fd = __drm_open_any_render();
+	int fd = __drm_open_any_render(false);
 
 	/* no render nodes, fallback to drm_open_any() */
 	if (fd == -1)
@@ -370,9 +448,30 @@  int drm_open_any_render(void)
 	if (__sync_fetch_and_add(&open_count, 1))
 		return fd;
 
-	at_exit_drm_render_fd = __drm_open_any();
+	at_exit_drm_render_fd = __drm_open_any(false);
 	gem_quiescent_gpu(fd);
 	igt_install_exit_handler(quiescent_gpu_at_exit_render);
 
 	return fd;
 }
+
+/**
+ * drm_open_any_render_any:
+ *
+ * Open a drm render device node.
+ *
+ * drm_open_any_render() is the Intel-specific version of this function.
+ *
+ * Returns:
+ * The drm file descriptor or -1 on error
+ */
+int drm_open_any_render_any(void)
+{
+	int fd = __drm_open_any_render(true);
+
+	/* no render nodes, fallback to drm_open_any() */
+	if (fd == -1)
+		return drm_open_any_any();
+
+	return fd;
+}
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 508cc83..84b9b05 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -71,10 +71,14 @@  static inline void *igt_mmap64(void *addr, size_t length, int prot, int flags,
 #define ALIGN(v, a) (((v) + (a)-1) & ~((a)-1))
 
 int drm_get_card(void);
-int __drm_open_any(void);
+int drm_get_any_card(void);
+int __drm_open_any(bool anygpu);
 int drm_open_any(void);
+int drm_open_any_any(void);
 int drm_open_any_master(void);
+int drm_open_any_master_any(void);
 int drm_open_any_render(void);
+int drm_open_any_render_any(void);
 
 void gem_quiescent_gpu(int fd);
 
diff --git a/tests/gem_alive.c b/tests/gem_alive.c
index 390a54f..53f2466 100644
--- a/tests/gem_alive.c
+++ b/tests/gem_alive.c
@@ -14,7 +14,7 @@  int main(void)
 
 	signal(SIGALRM, SIG_IGN);
 
-	fd = __drm_open_any();
+	fd = __drm_open_any(false);
 	if (fd < 0)
 		return IGT_EXIT_SKIP;
 

Comments

Hi,

On 21 April 2015 at 16:03, Micah Fedke <micah.fedke@collabora.co.uk> wrote:
> + * drm_open_any_any:
> + *
> + * Literally the worst-named function I've ever written.

And I stand by this. This is really an RFC, partly to find out whether
it would be better to find a new name for these functions (open a
modeset-capable DRM node, whether it's Intel or otherwise), or whether
adding a flag to drm_open_any and friends to specify Intel or generic,
with the resulting mega-Cocci run, would be better.

Cheers,
Daniel
On Tue, Apr 21, 2015 at 5:06 PM, Daniel Stone <daniel@fooishbar.org> wrote:
> On 21 April 2015 at 16:03, Micah Fedke <micah.fedke@collabora.co.uk> wrote:
>> + * drm_open_any_any:
>> + *
>> + * Literally the worst-named function I've ever written.
>
> And I stand by this. This is really an RFC, partly to find out whether
> it would be better to find a new name for these functions (open a
> modeset-capable DRM node, whether it's Intel or otherwise), or whether
> adding a flag to drm_open_any and friends to specify Intel or generic,
> with the resulting mega-Cocci run, would be better.

Yeah I think a new param to restrict the driver name (i915, nouveau,
...) would be better. Plus maybe a drm_require_driver so that testcase
could open a generic driver and then only skip specific subtest.
Initial pass could be done with cocci+ follow-up patches to convert
patches from i915 specific to generic.

One thing to keep in mind is how we'll treat machines with more than 1
gpu. I do care about that at least for the prime testcases since I
have a box somewhere with i915+nouveau to run them, and that should
keep working. Aside: We should convert those tests to use
drm_open_any("nouveau") instead of the hand-rolled one. A possible
idea would be to convert the igt_main macros into a loop which would
go over all the available drm devices (well the different ones, we
don't want to separately test control/render/legacy nodes ofc). What
we definitely need is an enviroment variable to override the default
choice.
-Daniel
Forgotten to add Thomas as the igt maintainer.
-Daniel

On Fri, May 8, 2015 at 3:03 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Apr 21, 2015 at 5:06 PM, Daniel Stone <daniel@fooishbar.org> wrote:
>> On 21 April 2015 at 16:03, Micah Fedke <micah.fedke@collabora.co.uk> wrote:
>>> + * drm_open_any_any:
>>> + *
>>> + * Literally the worst-named function I've ever written.
>>
>> And I stand by this. This is really an RFC, partly to find out whether
>> it would be better to find a new name for these functions (open a
>> modeset-capable DRM node, whether it's Intel or otherwise), or whether
>> adding a flag to drm_open_any and friends to specify Intel or generic,
>> with the resulting mega-Cocci run, would be better.
>
> Yeah I think a new param to restrict the driver name (i915, nouveau,
> ...) would be better. Plus maybe a drm_require_driver so that testcase
> could open a generic driver and then only skip specific subtest.
> Initial pass could be done with cocci+ follow-up patches to convert
> patches from i915 specific to generic.
>
> One thing to keep in mind is how we'll treat machines with more than 1
> gpu. I do care about that at least for the prime testcases since I
> have a box somewhere with i915+nouveau to run them, and that should
> keep working. Aside: We should convert those tests to use
> drm_open_any("nouveau") instead of the hand-rolled one. A possible
> idea would be to convert the igt_main macros into a loop which would
> go over all the available drm devices (well the different ones, we
> don't want to separately test control/render/legacy nodes ofc). What
> we definitely need is an enviroment variable to override the default
> choice.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Daniel,

Thanks.

I'm working to implement the driver name param and drm_require_driver 
now.  The prime stuff I'll have to familiarize myself with a bit more 
(and maybe dredge up some hw) but your suggestions sound reasonable enough.


-mf

On 05/22/2015 02:17 AM, Daniel Vetter wrote:
> Forgotten to add Thomas as the igt maintainer.
> -Daniel
>
> On Fri, May 8, 2015 at 3:03 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Tue, Apr 21, 2015 at 5:06 PM, Daniel Stone <daniel@fooishbar.org> wrote:
>>> On 21 April 2015 at 16:03, Micah Fedke <micah.fedke@collabora.co.uk> wrote:
>>>> + * drm_open_any_any:
>>>> + *
>>>> + * Literally the worst-named function I've ever written.
>>>
>>> And I stand by this. This is really an RFC, partly to find out whether
>>> it would be better to find a new name for these functions (open a
>>> modeset-capable DRM node, whether it's Intel or otherwise), or whether
>>> adding a flag to drm_open_any and friends to specify Intel or generic,
>>> with the resulting mega-Cocci run, would be better.
>>
>> Yeah I think a new param to restrict the driver name (i915, nouveau,
>> ...) would be better. Plus maybe a drm_require_driver so that testcase
>> could open a generic driver and then only skip specific subtest.
>> Initial pass could be done with cocci+ follow-up patches to convert
>> patches from i915 specific to generic.
>>
>> One thing to keep in mind is how we'll treat machines with more than 1
>> gpu. I do care about that at least for the prime testcases since I
>> have a box somewhere with i915+nouveau to run them, and that should
>> keep working. Aside: We should convert those tests to use
>> drm_open_any("nouveau") instead of the hand-rolled one. A possible
>> idea would be to convert the igt_main macros into a loop which would
>> go over all the available drm devices (well the different ones, we
>> don't want to separately test control/render/legacy nodes ofc). What
>> we definitely need is an enviroment variable to override the default
>> choice.
>> -Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>
>
>