Using render nodes in i-g-t

Using render nodes in i-g-t

[Damien Lespiau]

Hi,

This small series add the possibility for a test to ask for a render device.
This allows us to run render tests as a regular users.

-- 
Damien

[PATCH 1/5] lib: Remove stale comment

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/drmtest.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index abcbba9..b94981e 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -168,8 +168,6 @@ void gem_quiescent_gpu(int fd)
 /**
  * drm_get_card() - get an intel card number for use in /dev or /sys
  *
- * @master: -1 not a master, 0 don't care, 1 is the master
- *
  * returns -1 on error
  */
 int drm_get_card(void)
-- 
1.8.3.1

[PATCH 2/5] lib: Close non intel fds in drm_get_card()

[Damien Lespiau]

When going through card%u devices, close the ones that we were able to
open but weren't intel devices.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/drmtest.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index b94981e..3b80920 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -187,8 +187,10 @@ int drm_get_card(void)
 		if (fd == -1)
 			continue;
 
-		if (!is_intel(fd))
+		if (!is_intel(fd)) {
+			close(fd);
 			continue;
+		}
 
 		close(fd);
 		return i;
-- 
1.8.3.1

[PATCH 3/5] lib: Add a drm_open_any_render() that will try to use render nodes

[Damien Lespiau]

I was fedup with having to run my tests as root and not being able to
use my usual setup for tests that only exercise the GT part of the GPU.

Render nodes to the rescue!

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 lib/drmtest.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/drmtest.h |  1 +
 2 files changed, 59 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 3b80920..8164ef9 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -222,6 +222,35 @@ static int __drm_open_any(void)
 	return fd;
 }
 
+static int __drm_open_any_render(void)
+{
+	char *name;
+	int i, fd;
+
+	for (i = 128; i < (128 + 16); i++) {
+		int ret;
+
+		ret = asprintf(&name, "/dev/dri/renderD%u", i);
+		igt_assert(ret != -1);
+
+		fd = open(name, O_RDWR);
+		free(name);
+
+		if (fd == -1)
+			continue;
+
+		if (!is_intel(fd)) {
+			close(fd);
+			fd = -1;
+			continue;
+		}
+
+		return fd;
+	}
+
+	return fd;
+}
+
 static void quiescent_gpu_at_exit(int sig)
 {
 	int fd;
@@ -233,6 +262,17 @@ static void quiescent_gpu_at_exit(int sig)
 	}
 }
 
+static void quiescent_gpu_at_exit_render(int sig)
+{
+	int fd;
+
+	fd = __drm_open_any_render();
+	if (fd >= 0) {
+		gem_quiescent_gpu(fd);
+		close(fd);
+	}
+}
+
 int drm_open_any(void)
 {
 	static int open_count;
@@ -249,6 +289,24 @@ int drm_open_any(void)
 	return fd;
 }
 
+int drm_open_any_render(void)
+{
+	static int open_count;
+	int fd = __drm_open_any_render();
+
+	/* no render nodes, fallback to drm_open_any() */
+	if (fd == -1)
+		return drm_open_any();
+
+	if (__sync_fetch_and_add(&open_count, 1))
+		return fd;
+
+	gem_quiescent_gpu(fd);
+	igt_install_exit_handler(quiescent_gpu_at_exit_render);
+
+	return fd;
+}
+
 int __gem_set_tiling(int fd, uint32_t handle, int tiling, int stride)
 {
 	struct drm_i915_gem_set_tiling st;
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 609e7d8..f5e2708 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -50,6 +50,7 @@ drm_intel_bo * gem_handle_to_libdrm_bo(drm_intel_bufmgr *bufmgr, int fd,
 
 int drm_get_card(void);
 int drm_open_any(void);
+int drm_open_any_render(void);
 
 void gem_quiescent_gpu(int fd);
 
-- 
1.8.3.1

Re: [PATCH 3/5] lib: Add a drm_open_any_render() that will try to use render nodes

[Ben Widawsky]

On Thu, Oct 24, 2013 at 03:40:54PM +0100, Damien Lespiau wrote:
> I was fedup with having to run my tests as root and not being able to
> use my usual setup for tests that only exercise the GT part of the GPU.
> 
> Render nodes to the rescue!

I think usually this is a bad idea except for pretty isolated proof of
concept kind of things. Mostly intel-gpu-tools is testing our kernel
driver, and therefore running without a reboot anyway is sort of
unusual.

What do you think about providing some sort of option (maybe via env
var) instead of what you did in patch 5?

> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/drmtest.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/drmtest.h |  1 +
>  2 files changed, 59 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 3b80920..8164ef9 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -222,6 +222,35 @@ static int __drm_open_any(void)
>  	return fd;
>  }
>  
> +static int __drm_open_any_render(void)
> +{
> +	char *name;
> +	int i, fd;
> +
> +	for (i = 128; i < (128 + 16); i++) {
> +		int ret;
> +
> +		ret = asprintf(&name, "/dev/dri/renderD%u", i);
> +		igt_assert(ret != -1);
> +
> +		fd = open(name, O_RDWR);
> +		free(name);
> +
> +		if (fd == -1)
> +			continue;
> +
> +		if (!is_intel(fd)) {
> +			close(fd);
> +			fd = -1;
> +			continue;
> +		}
> +
> +		return fd;
> +	}
> +
> +	return fd;
> +}
> +
>  static void quiescent_gpu_at_exit(int sig)
>  {
>  	int fd;
> @@ -233,6 +262,17 @@ static void quiescent_gpu_at_exit(int sig)
>  	}
>  }
>  
> +static void quiescent_gpu_at_exit_render(int sig)
> +{
> +	int fd;
> +
> +	fd = __drm_open_any_render();
> +	if (fd >= 0) {
> +		gem_quiescent_gpu(fd);
> +		close(fd);
> +	}
> +}
> +
>  int drm_open_any(void)
>  {
>  	static int open_count;
> @@ -249,6 +289,24 @@ int drm_open_any(void)
>  	return fd;
>  }
>  
> +int drm_open_any_render(void)
> +{
> +	static int open_count;
> +	int fd = __drm_open_any_render();
> +
> +	/* no render nodes, fallback to drm_open_any() */
> +	if (fd == -1)
> +		return drm_open_any();
> +
> +	if (__sync_fetch_and_add(&open_count, 1))
> +		return fd;
> +
> +	gem_quiescent_gpu(fd);
> +	igt_install_exit_handler(quiescent_gpu_at_exit_render);
> +
> +	return fd;
> +}
> +
>  int __gem_set_tiling(int fd, uint32_t handle, int tiling, int stride)
>  {
>  	struct drm_i915_gem_set_tiling st;
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 609e7d8..f5e2708 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -50,6 +50,7 @@ drm_intel_bo * gem_handle_to_libdrm_bo(drm_intel_bufmgr *bufmgr, int fd,
>  
>  int drm_get_card(void);
>  int drm_open_any(void);
> +int drm_open_any_render(void);
>  
>  void gem_quiescent_gpu(int fd);
>  
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 3/5] lib: Add a drm_open_any_render() that will try to use render nodes

[Damien Lespiau]

On Thu, Oct 24, 2013 at 01:43:22PM -0700, Ben Widawsky wrote:
> On Thu, Oct 24, 2013 at 03:40:54PM +0100, Damien Lespiau wrote:
> > I was fedup with having to run my tests as root and not being able to
> > use my usual setup for tests that only exercise the GT part of the GPU.
> > 
> > Render nodes to the rescue!
> 
> I think usually this is a bad idea except for pretty isolated proof of
> concept kind of things. Mostly intel-gpu-tools is testing our kernel
> driver, and therefore running without a reboot anyway is sort of
> unusual.

Hum? I fail to see the link. It should be strickly equivalent, with the
added bonus that you can write and execute a test as a normal user.

Actually, as Daniel said on IRC, we'd most likely want to run quite a
few of the tests with both a fd from card0 and a fd from the render node
to ensure that we can function correctly from the render node 

-- 
Damien

Re: [PATCH 3/5] lib: Add a drm_open_any_render() that will try to use render nodes

[Damien Lespiau]

On Thu, Oct 24, 2013 at 10:01:21PM +0100, Damien Lespiau wrote:
> On Thu, Oct 24, 2013 at 01:43:22PM -0700, Ben Widawsky wrote:
> > On Thu, Oct 24, 2013 at 03:40:54PM +0100, Damien Lespiau wrote:
> > > I was fedup with having to run my tests as root and not being able to
> > > use my usual setup for tests that only exercise the GT part of the GPU.
> > > 
> > > Render nodes to the rescue!
> > 
> > I think usually this is a bad idea except for pretty isolated proof of
> > concept kind of things. Mostly intel-gpu-tools is testing our kernel
> > driver, and therefore running without a reboot anyway is sort of
> > unusual.
> 
> Hum? I fail to see the link. It should be strickly equivalent, with the
> added bonus that you can write and execute a test as a normal user.

By "normal user" I mean with a DRM master running and without having to
do the authentification dance.

-- 
Damien

Re: [PATCH 3/5] lib: Add a drm_open_any_render() that will try to use render nodes

[Ben Widawsky]

On Thu, Oct 24, 2013 at 10:05:43PM +0100, Damien Lespiau wrote:
> On Thu, Oct 24, 2013 at 10:01:21PM +0100, Damien Lespiau wrote:
> > On Thu, Oct 24, 2013 at 01:43:22PM -0700, Ben Widawsky wrote:
> > > On Thu, Oct 24, 2013 at 03:40:54PM +0100, Damien Lespiau wrote:
> > > > I was fedup with having to run my tests as root and not being able to
> > > > use my usual setup for tests that only exercise the GT part of the GPU.
> > > > 
> > > > Render nodes to the rescue!
> > > 
> > > I think usually this is a bad idea except for pretty isolated proof of
> > > concept kind of things. Mostly intel-gpu-tools is testing our kernel
> > > driver, and therefore running without a reboot anyway is sort of
> > > unusual.
> > 
> > Hum? I fail to see the link. It should be strickly equivalent, with the
> > added bonus that you can write and execute a test as a normal user.
> 
> By "normal user" I mean with a DRM master running and without having to
> do the authentification dance.
> 
> -- 
> Damien

You're right, this was a half baked thought. What I was thinking is,
condoning running tests on a live system is a bad idea - but forcing
rendernodes doesn't effect that.

Also, I agree with Daniel. Getting some simultaneous tests would be
nice.

Practically, requiring root won't go away I think, since certain things
like our forcewake debugfs require root. Anyway, carry on.

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 4/5] tests/gem_render_copy: Use render nodes

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/gem_render_copy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/gem_render_copy.c b/tests/gem_render_copy.c
index 40c37f4..e481733 100644
--- a/tests/gem_render_copy.c
+++ b/tests/gem_render_copy.c
@@ -104,7 +104,7 @@ int main(int argc, char **argv)
 	}
 
 	igt_fixture {
-		data.drm_fd = drm_open_any();
+		data.drm_fd = drm_open_any_render();
 		data.devid = intel_get_drm_devid(data.drm_fd);
 
 		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
-- 
1.8.3.1

[PATCH 5/5] tests: Convert the ctx test to use render nodes when possible

[Damien Lespiau]

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 tests/gem_ctx_bad_destroy.c | 2 +-
 tests/gem_ctx_bad_exec.c    | 2 +-
 tests/gem_ctx_basic.c       | 4 ++--
 tests/gem_ctx_create.c      | 2 +-
 tests/gem_ctx_exec.c        | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/gem_ctx_bad_destroy.c b/tests/gem_ctx_bad_destroy.c
index 7852618..e93642f 100644
--- a/tests/gem_ctx_bad_destroy.c
+++ b/tests/gem_ctx_bad_destroy.c
@@ -62,7 +62,7 @@ int main(int argc, char *argv[])
 
 	igt_skip_on_simulation();
 
-	fd = drm_open_any();
+	fd = drm_open_any_render();
 
 	ctx_id = gem_context_create(fd);
 
diff --git a/tests/gem_ctx_bad_exec.c b/tests/gem_ctx_bad_exec.c
index dadd663..c60e693 100644
--- a/tests/gem_ctx_bad_exec.c
+++ b/tests/gem_ctx_bad_exec.c
@@ -91,7 +91,7 @@ int main(int argc, char *argv[])
 	igt_subtest_init(argc, argv);
 
 	igt_fixture {
-		fd = drm_open_any();
+		fd = drm_open_any_render();
 
 		ctx_id = gem_context_create(fd);
 
diff --git a/tests/gem_ctx_basic.c b/tests/gem_ctx_basic.c
index 47f4b4b..bc21a8a 100644
--- a/tests/gem_ctx_basic.c
+++ b/tests/gem_ctx_basic.c
@@ -64,7 +64,7 @@ static void *work(void *arg)
 	int i;
 
 	if (multiple_fds)
-		td_fd = fd = drm_open_any();
+		td_fd = fd = drm_open_any_render();
 	else
 		td_fd = fd;
 
@@ -133,7 +133,7 @@ int main(int argc, char *argv[])
 {
 	int i;
 
-	fd = drm_open_any();
+	fd = drm_open_any_render();
 	devid = intel_get_drm_devid(fd);
 
 	if (igt_run_in_simulation()) {
diff --git a/tests/gem_ctx_create.c b/tests/gem_ctx_create.c
index f22522b..1d9ff86 100644
--- a/tests/gem_ctx_create.c
+++ b/tests/gem_ctx_create.c
@@ -47,7 +47,7 @@ int main(int argc, char *argv[])
 	create.ctx_id = rand();
 	create.pad = rand();
 
-	fd = drm_open_any();
+	fd = drm_open_any_render();
 
 	ret = drmIoctl(fd, CONTEXT_CREATE_IOCTL, &create);
 	if (ret != 0 && (errno == ENODEV || errno == EINVAL)) {
diff --git a/tests/gem_ctx_exec.c b/tests/gem_ctx_exec.c
index 5f2c59f..620a944 100644
--- a/tests/gem_ctx_exec.c
+++ b/tests/gem_ctx_exec.c
@@ -102,7 +102,7 @@ int main(int argc, char *argv[])
 
 	igt_skip_on_simulation();
 
-	fd = drm_open_any();
+	fd = drm_open_any_render();
 
 	ctx_id = gem_context_create(fd);
 	handle = gem_create(fd, 4096);
-- 
1.8.3.1

Re: Using render nodes in i-g-t

[Damien Lespiau]

On Thu, Oct 24, 2013 at 03:40:51PM +0100, Damien Lespiau wrote:
> Hi,
> 
> This small series add the possibility for a test to ask for a render device.
> This allows us to run render tests as a regular users.

Pushed, there's a bit more work before we can run the right subset of
tests on both render nodes and legacy nodes. Wrote down some ideas in
the wiki.

-- 
Damien