[PATCH i-g-t 1/8] i915/gem_eio: Check that context create fails when wedged

[PATCH i-g-t 1/8] i915/gem_eio: Check that context create fails when wedged

[Chris Wilson]

Lock down the new uABI that DRM_IOCTL_I915_GEM_CONTEXT_CREATE returns
-EIO when wedged.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tests/i915/gem_eio.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index ac85a2eff..3c54820c9 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -118,6 +118,17 @@ static void test_throttle(int fd)
 	trigger_reset(fd);
 }
 
+static void test_context_create(int fd)
+{
+	uint32_t ctx;
+
+	wedge_gpu(fd);
+
+	igt_assert_eq(__gem_context_create(fd, &ctx), -EIO);
+
+	trigger_reset(fd);
+}
+
 static void test_execbuf(int fd)
 {
 	struct drm_i915_gem_execbuffer2 execbuf;
@@ -807,6 +818,9 @@ igt_main
 	igt_subtest("throttle")
 		test_throttle(fd);
 
+	igt_subtest("context-create")
+		test_context_create(fd);
+
 	igt_subtest("execbuf")
 		test_execbuf(fd);
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 1/8] i915/gem_eio: Check that context create fails when wedged

[Chris Wilson]

Lock down the new uABI that DRM_IOCTL_I915_GEM_CONTEXT_CREATE returns
-EIO when wedged.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tests/i915/gem_eio.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index ac85a2eff..3c54820c9 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -118,6 +118,17 @@ static void test_throttle(int fd)
 	trigger_reset(fd);
 }
 
+static void test_context_create(int fd)
+{
+	uint32_t ctx;
+
+	wedge_gpu(fd);
+
+	igt_assert_eq(__gem_context_create(fd, &ctx), -EIO);
+
+	trigger_reset(fd);
+}
+
 static void test_execbuf(int fd)
 {
 	struct drm_i915_gem_execbuffer2 execbuf;
@@ -807,6 +818,9 @@ igt_main
 	igt_subtest("throttle")
 		test_throttle(fd);
 
+	igt_subtest("context-create")
+		test_context_create(fd);
+
 	igt_subtest("execbuf")
 		test_execbuf(fd);
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 2/8] i915/gem_eio: Check we only ban the context

[Chris Wilson]

In trigger the ban, we only want to observe the local context be banned
and not the fpriv as a whole.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tests/i915/gem_eio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index 3c54820c9..3afdbd69e 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -324,8 +324,15 @@ static void __test_banned(int fd)
 		igt_spin_t *hang;
 
 		if (__gem_execbuf(fd, &execbuf) == -EIO) {
+			uint32_t ctx = 0;
+
 			igt_info("Banned after causing %lu hangs\n", count);
 			igt_assert(count > 1);
+
+			/* Only this context, not the file, should be banned */
+			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
+			igt_assert_neq(ctx, 0);
+			gem_context_destroy(fd, ctx);
 			return;
 		}
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 2/8] i915/gem_eio: Check we only ban the context

[Chris Wilson]

In trigger the ban, we only want to observe the local context be banned
and not the fpriv as a whole.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tests/i915/gem_eio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index 3c54820c9..3afdbd69e 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -324,8 +324,15 @@ static void __test_banned(int fd)
 		igt_spin_t *hang;
 
 		if (__gem_execbuf(fd, &execbuf) == -EIO) {
+			uint32_t ctx = 0;
+
 			igt_info("Banned after causing %lu hangs\n", count);
 			igt_assert(count > 1);
+
+			/* Only this context, not the file, should be banned */
+			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
+			igt_assert_neq(ctx, 0);
+			gem_context_destroy(fd, ctx);
 			return;
 		}
 
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/8] i915/gem_eio: Check we only ban the context

[Antonio Argenziano]

On 17/02/19 06:35, Chris Wilson wrote:
> In trigger the ban, we only want to observe the local context be banned
> and not the fpriv as a whole.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>   tests/i915/gem_eio.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index 3c54820c9..3afdbd69e 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -324,8 +324,15 @@ static void __test_banned(int fd)
>   		igt_spin_t *hang;
>   
>   		if (__gem_execbuf(fd, &execbuf) == -EIO) {
> +			uint32_t ctx = 0;
> +
>   			igt_info("Banned after causing %lu hangs\n", count);
>   			igt_assert(count > 1);
> +
> +			/* Only this context, not the file, should be banned */
> +			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);

Should we check submission works on the new context?

Antonio

> +			igt_assert_neq(ctx, 0);
> +			gem_context_destroy(fd, ctx);
>   			return;
>   		}
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 2/8] i915/gem_eio: Check we only ban the context

[Antonio Argenziano]

On 17/02/19 06:35, Chris Wilson wrote:
> In trigger the ban, we only want to observe the local context be banned
> and not the fpriv as a whole.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>   tests/i915/gem_eio.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index 3c54820c9..3afdbd69e 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -324,8 +324,15 @@ static void __test_banned(int fd)
>   		igt_spin_t *hang;
>   
>   		if (__gem_execbuf(fd, &execbuf) == -EIO) {
> +			uint32_t ctx = 0;
> +
>   			igt_info("Banned after causing %lu hangs\n", count);
>   			igt_assert(count > 1);
> +
> +			/* Only this context, not the file, should be banned */
> +			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);

Should we check submission works on the new context?

Antonio

> +			igt_assert_neq(ctx, 0);
> +			gem_context_destroy(fd, ctx);
>   			return;
>   		}
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/8] i915/gem_eio: Check we only ban the context

[Chris Wilson]

Quoting Antonio Argenziano (2019-02-19 16:53:57)
> 
> 
> On 17/02/19 06:35, Chris Wilson wrote:
> > In trigger the ban, we only want to observe the local context be banned
> > and not the fpriv as a whole.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> >   tests/i915/gem_eio.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> > 
> > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> > index 3c54820c9..3afdbd69e 100644
> > --- a/tests/i915/gem_eio.c
> > +++ b/tests/i915/gem_eio.c
> > @@ -324,8 +324,15 @@ static void __test_banned(int fd)
> >               igt_spin_t *hang;
> >   
> >               if (__gem_execbuf(fd, &execbuf) == -EIO) {
> > +                     uint32_t ctx = 0;
> > +
> >                       igt_info("Banned after causing %lu hangs\n", count);
> >                       igt_assert(count > 1);
> > +
> > +                     /* Only this context, not the file, should be banned */
> > +                     igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
> 
> Should we check submission works on the new context?

Why not? In for a penny, in for a pound.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 2/8] i915/gem_eio: Check we only ban the context

[Chris Wilson]

Quoting Antonio Argenziano (2019-02-19 16:53:57)
> 
> 
> On 17/02/19 06:35, Chris Wilson wrote:
> > In trigger the ban, we only want to observe the local context be banned
> > and not the fpriv as a whole.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> >   tests/i915/gem_eio.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> > 
> > diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> > index 3c54820c9..3afdbd69e 100644
> > --- a/tests/i915/gem_eio.c
> > +++ b/tests/i915/gem_eio.c
> > @@ -324,8 +324,15 @@ static void __test_banned(int fd)
> >               igt_spin_t *hang;
> >   
> >               if (__gem_execbuf(fd, &execbuf) == -EIO) {
> > +                     uint32_t ctx = 0;
> > +
> >                       igt_info("Banned after causing %lu hangs\n", count);
> >                       igt_assert(count > 1);
> > +
> > +                     /* Only this context, not the file, should be banned */
> > +                     igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
> 
> Should we check submission works on the new context?

Why not? In for a penny, in for a pound.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t] i915/gem_eio: Check we only ban the context

[Chris Wilson]

In trigger the ban, we only want to observe the local context be banned
and not the fpriv as a whole.

v2: And send an execbuf down the new context.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
---
 tests/i915/gem_eio.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index ac85a2eff..b0be128ef 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -313,8 +313,20 @@ static void __test_banned(int fd)
 		igt_spin_t *hang;
 
 		if (__gem_execbuf(fd, &execbuf) == -EIO) {
+			uint32_t ctx = 0;
+
 			igt_info("Banned after causing %lu hangs\n", count);
 			igt_assert(count > 1);
+
+			/* Only this context, not the file, should be banned */
+			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
+			igt_assert_neq(ctx, 0);
+
+			/* And check it actually works! */
+			execbuf.rsvd1 = ctx;
+			gem_execbuf(fd, &execbuf);
+
+			gem_context_destroy(fd, ctx);
 			return;
 		}
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t] i915/gem_eio: Check we only ban the context

[Chris Wilson]

In trigger the ban, we only want to observe the local context be banned
and not the fpriv as a whole.

v2: And send an execbuf down the new context.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Antonio Argenziano <antonio.argenziano@intel.com>
---
 tests/i915/gem_eio.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index ac85a2eff..b0be128ef 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -313,8 +313,20 @@ static void __test_banned(int fd)
 		igt_spin_t *hang;
 
 		if (__gem_execbuf(fd, &execbuf) == -EIO) {
+			uint32_t ctx = 0;
+
 			igt_info("Banned after causing %lu hangs\n", count);
 			igt_assert(count > 1);
+
+			/* Only this context, not the file, should be banned */
+			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
+			igt_assert_neq(ctx, 0);
+
+			/* And check it actually works! */
+			execbuf.rsvd1 = ctx;
+			gem_execbuf(fd, &execbuf);
+
+			gem_context_destroy(fd, ctx);
 			return;
 		}
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t] i915/gem_eio: Check we only ban the context

[Antonio Argenziano]

On 19/02/19 09:11, Chris Wilson wrote:
> In trigger the ban, we only want to observe the local context be banned
> and not the fpriv as a whole.
> 
> v2: And send an execbuf down the new context.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>

Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

> ---
>   tests/i915/gem_eio.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index ac85a2eff..b0be128ef 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -313,8 +313,20 @@ static void __test_banned(int fd)
>   		igt_spin_t *hang;
>   
>   		if (__gem_execbuf(fd, &execbuf) == -EIO) {
> +			uint32_t ctx = 0;
> +
>   			igt_info("Banned after causing %lu hangs\n", count);
>   			igt_assert(count > 1);
> +
> +			/* Only this context, not the file, should be banned */
> +			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
> +			igt_assert_neq(ctx, 0);
> +
> +			/* And check it actually works! */
> +			execbuf.rsvd1 = ctx;
> +			gem_execbuf(fd, &execbuf);
> +
> +			gem_context_destroy(fd, ctx);
>   			return;
>   		}
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t] i915/gem_eio: Check we only ban the context

[Antonio Argenziano]

On 19/02/19 09:11, Chris Wilson wrote:
> In trigger the ban, we only want to observe the local context be banned
> and not the fpriv as a whole.
> 
> v2: And send an execbuf down the new context.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Antonio Argenziano <antonio.argenziano@intel.com>

Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

> ---
>   tests/i915/gem_eio.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index ac85a2eff..b0be128ef 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -313,8 +313,20 @@ static void __test_banned(int fd)
>   		igt_spin_t *hang;
>   
>   		if (__gem_execbuf(fd, &execbuf) == -EIO) {
> +			uint32_t ctx = 0;
> +
>   			igt_info("Banned after causing %lu hangs\n", count);
>   			igt_assert(count > 1);
> +
> +			/* Only this context, not the file, should be banned */
> +			igt_assert_neq(__gem_context_create(fd, &ctx), -EIO);
> +			igt_assert_neq(ctx, 0);
> +
> +			/* And check it actually works! */
> +			execbuf.rsvd1 = ctx;
> +			gem_execbuf(fd, &execbuf);
> +
> +			gem_context_destroy(fd, ctx);
>   			return;
>   		}
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 3/8] lib: Restore the i915.reset modparam before cleaning up

[Chris Wilson]

We force a reset on test exit so that we can rapidly cleanup after a
naughty test, it is not unknown for us to leave a queue of hanging
batches around. However, if we have also fiddled with the i915.reset
parameter in the meantime, this can leave the kernel unable to fulfil
our request (and those naughty batches continue to disrupt testing).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 lib/drmtest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 1964795a6..6c0a0e381 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -54,6 +54,7 @@
 #include "igt_device.h"
 #include "igt_gt.h"
 #include "igt_kmod.h"
+#include "igt_sysfs.h"
 #include "version.h"
 #include "config.h"
 #include "intel_reg.h"
@@ -345,6 +346,7 @@ static void __cancel_work_at_exit(int fd)
 {
 	igt_terminate_spin_batches(); /* for older kernels */
 
+	igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
 	igt_drop_caches_set(fd,
 			    /* cancel everything */
 			    DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 3/8] lib: Restore the i915.reset modparam before cleaning up

[Chris Wilson]

We force a reset on test exit so that we can rapidly cleanup after a
naughty test, it is not unknown for us to leave a queue of hanging
batches around. However, if we have also fiddled with the i915.reset
parameter in the meantime, this can leave the kernel unable to fulfil
our request (and those naughty batches continue to disrupt testing).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
---
 lib/drmtest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index 1964795a6..6c0a0e381 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -54,6 +54,7 @@
 #include "igt_device.h"
 #include "igt_gt.h"
 #include "igt_kmod.h"
+#include "igt_sysfs.h"
 #include "version.h"
 #include "config.h"
 #include "intel_reg.h"
@@ -345,6 +346,7 @@ static void __cancel_work_at_exit(int fd)
 {
 	igt_terminate_spin_batches(); /* for older kernels */
 
+	igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
 	igt_drop_caches_set(fd,
 			    /* cancel everything */
 			    DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 3/8] lib: Restore the i915.reset modparam before cleaning up

[Antonio Argenziano]

On 17/02/19 06:35, Chris Wilson wrote:
> We force a reset on test exit so that we can rapidly cleanup after a
> naughty test, it is not unknown for us to leave a queue of hanging
> batches around. However, if we have also fiddled with the i915.reset
> parameter in the meantime, this can leave the kernel unable to fulfil

typo -------------------------------------------------------------^

> our request (and those naughty batches continue to disrupt testing).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>

Re-enabling reset sounds good.

Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

> ---
>   lib/drmtest.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 1964795a6..6c0a0e381 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -54,6 +54,7 @@
>   #include "igt_device.h"
>   #include "igt_gt.h"
>   #include "igt_kmod.h"
> +#include "igt_sysfs.h"
>   #include "version.h"
>   #include "config.h"
>   #include "intel_reg.h"
> @@ -345,6 +346,7 @@ static void __cancel_work_at_exit(int fd)
>   {
>   	igt_terminate_spin_batches(); /* for older kernels */
>   
> +	igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
>   	igt_drop_caches_set(fd,
>   			    /* cancel everything */
>   			    DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 3/8] lib: Restore the i915.reset modparam before cleaning up

[Antonio Argenziano]

On 17/02/19 06:35, Chris Wilson wrote:
> We force a reset on test exit so that we can rapidly cleanup after a
> naughty test, it is not unknown for us to leave a queue of hanging
> batches around. However, if we have also fiddled with the i915.reset
> parameter in the meantime, this can leave the kernel unable to fulfil

typo -------------------------------------------------------------^

> our request (and those naughty batches continue to disrupt testing).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>

Re-enabling reset sounds good.

Acked-by: Antonio Argenziano <antonio.argenziano@intel.com>

> ---
>   lib/drmtest.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 1964795a6..6c0a0e381 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -54,6 +54,7 @@
>   #include "igt_device.h"
>   #include "igt_gt.h"
>   #include "igt_kmod.h"
> +#include "igt_sysfs.h"
>   #include "version.h"
>   #include "config.h"
>   #include "intel_reg.h"
> @@ -345,6 +346,7 @@ static void __cancel_work_at_exit(int fd)
>   {
>   	igt_terminate_spin_batches(); /* for older kernels */
>   
> +	igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
>   	igt_drop_caches_set(fd,
>   			    /* cancel everything */
>   			    DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/8] lib: Restore the i915.reset modparam before cleaning up

[Chris Wilson]

Quoting Antonio Argenziano (2019-02-19 18:22:26)
> 
> 
> On 17/02/19 06:35, Chris Wilson wrote:
> > We force a reset on test exit so that we can rapidly cleanup after a
> > naughty test, it is not unknown for us to leave a queue of hanging
> > batches around. However, if we have also fiddled with the i915.reset
> > parameter in the meantime, this can leave the kernel unable to fulfil
> 
> typo -------------------------------------------------------------^

I'm Chiefly British still. So just the one 'l' is enough to fulfil me.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 3/8] lib: Restore the i915.reset modparam before cleaning up

[Chris Wilson]

Quoting Antonio Argenziano (2019-02-19 18:22:26)
> 
> 
> On 17/02/19 06:35, Chris Wilson wrote:
> > We force a reset on test exit so that we can rapidly cleanup after a
> > naughty test, it is not unknown for us to leave a queue of hanging
> > batches around. However, if we have also fiddled with the i915.reset
> > parameter in the meantime, this can leave the kernel unable to fulfil
> 
> typo -------------------------------------------------------------^

I'm Chiefly British still. So just the one 'l' is enough to fulfil me.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 4/8] i915/gem_create: Verify that all new objects are clear

[Chris Wilson]

The kernel must not return stale information back to userspace when they
create a new object. For that purpose, we always clear objects on
creation, so verify that this is so.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 tests/i915/gem_create.c | 71 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c
index 25c5e8088..9de2263d5 100644
--- a/tests/i915/gem_create.c
+++ b/tests/i915/gem_create.c
@@ -44,6 +44,7 @@
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <getopt.h>
+#include <pthread.h>
 
 #include <drm.h>
 
@@ -141,6 +142,73 @@ static void invalid_nonaligned_size(int fd)
 	gem_close(fd, handle);
 }
 
+static uint64_t get_npages(uint64_t *global, uint64_t npages)
+{
+	uint64_t try, old, max;
+
+	max = *global;
+	do {
+		old = max;
+		try = npages % (max / 2);
+		max -= try;
+	} while ((max = __sync_val_compare_and_swap(global, old, max)) != old);
+
+	return try;
+}
+
+struct thread_clear {
+	uint64_t max;
+	int timeout;
+	int i915;
+};
+
+static void *thread_clear(void *data)
+{
+	struct thread_clear *arg = data;
+	int i915 = arg->i915;
+
+	igt_until_timeout(arg->timeout) {
+		uint32_t handle;
+		uint64_t npages;
+
+		npages = random();
+		npages <<= 32;
+		npages |= random();
+		npages = get_npages(&arg->max, npages);
+
+		handle = gem_create(i915, npages << 12);
+		for (uint64_t page = 0; page < npages; page++) {
+			uint64_t x;
+
+			gem_read(i915, handle,
+					page % (4096 - sizeof(x)),
+					&x, sizeof(x));
+			igt_assert_eq_u64(x, 0);
+		}
+		gem_close(i915, handle);
+
+		__sync_add_and_fetch(&arg->max, npages);
+	}
+
+	return NULL;
+}
+
+static void always_clear(int i915, int timeout)
+{
+	struct thread_clear arg = {
+		.i915 = i915,
+		.timeout = timeout,
+		.max = intel_get_avail_ram_mb() << (20 - 12), /* in pages */
+	};
+	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
+	pthread_t thread[ncpus];
+
+	for (int i = 0; i < ncpus; i++)
+		pthread_create(&thread[i], NULL, thread_clear, &arg);
+	for (int i = 0; i < ncpus; i++)
+		pthread_join(thread[i], NULL);
+}
+
 igt_main
 {
 	int fd = -1;
@@ -162,4 +230,7 @@ igt_main
 
 	igt_subtest("create-invalid-nonaligned")
 		invalid_nonaligned_size(fd);
+
+	igt_subtest("create-clear")
+		always_clear(fd, 30);
 }
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 4/8] i915/gem_create: Verify that all new objects are clear

[Chris Wilson]

The kernel must not return stale information back to userspace when they
create a new object. For that purpose, we always clear objects on
creation, so verify that this is so.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
---
 tests/i915/gem_create.c | 71 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/tests/i915/gem_create.c b/tests/i915/gem_create.c
index 25c5e8088..9de2263d5 100644
--- a/tests/i915/gem_create.c
+++ b/tests/i915/gem_create.c
@@ -44,6 +44,7 @@
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <getopt.h>
+#include <pthread.h>
 
 #include <drm.h>
 
@@ -141,6 +142,73 @@ static void invalid_nonaligned_size(int fd)
 	gem_close(fd, handle);
 }
 
+static uint64_t get_npages(uint64_t *global, uint64_t npages)
+{
+	uint64_t try, old, max;
+
+	max = *global;
+	do {
+		old = max;
+		try = npages % (max / 2);
+		max -= try;
+	} while ((max = __sync_val_compare_and_swap(global, old, max)) != old);
+
+	return try;
+}
+
+struct thread_clear {
+	uint64_t max;
+	int timeout;
+	int i915;
+};
+
+static void *thread_clear(void *data)
+{
+	struct thread_clear *arg = data;
+	int i915 = arg->i915;
+
+	igt_until_timeout(arg->timeout) {
+		uint32_t handle;
+		uint64_t npages;
+
+		npages = random();
+		npages <<= 32;
+		npages |= random();
+		npages = get_npages(&arg->max, npages);
+
+		handle = gem_create(i915, npages << 12);
+		for (uint64_t page = 0; page < npages; page++) {
+			uint64_t x;
+
+			gem_read(i915, handle,
+					page % (4096 - sizeof(x)),
+					&x, sizeof(x));
+			igt_assert_eq_u64(x, 0);
+		}
+		gem_close(i915, handle);
+
+		__sync_add_and_fetch(&arg->max, npages);
+	}
+
+	return NULL;
+}
+
+static void always_clear(int i915, int timeout)
+{
+	struct thread_clear arg = {
+		.i915 = i915,
+		.timeout = timeout,
+		.max = intel_get_avail_ram_mb() << (20 - 12), /* in pages */
+	};
+	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
+	pthread_t thread[ncpus];
+
+	for (int i = 0; i < ncpus; i++)
+		pthread_create(&thread[i], NULL, thread_clear, &arg);
+	for (int i = 0; i < ncpus; i++)
+		pthread_join(thread[i], NULL);
+}
+
 igt_main
 {
 	int fd = -1;
@@ -162,4 +230,7 @@ igt_main
 
 	igt_subtest("create-invalid-nonaligned")
 		invalid_nonaligned_size(fd);
+
+	igt_subtest("create-clear")
+		always_clear(fd, 30);
 }
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 5/8] i915/gem_exec_big: Add a single shot test

[Chris Wilson]

CI complains that the exhaustive test of trying every size up to the
limit is too slow, so add a simple test that tries to submit one
extreme batch buffer and check all the relocations land.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105555
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_big.c    | 70 ++++++++++++++++++++++++++++++------
 tests/intel-ci/blacklist.txt |  1 +
 2 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/tests/i915/gem_exec_big.c b/tests/i915/gem_exec_big.c
index a15672f66..015f59e29 100644
--- a/tests/i915/gem_exec_big.c
+++ b/tests/i915/gem_exec_big.c
@@ -71,7 +71,7 @@ static void exec1(int fd, uint32_t handle, uint64_t reloc_ofs, unsigned flags, c
 	gem_exec[0].relocs_ptr = to_user_pointer(gem_reloc);
 	gem_exec[0].alignment = 0;
 	gem_exec[0].offset = 0;
-	gem_exec[0].flags = 0;
+	gem_exec[0].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
 	gem_exec[0].rsvd1 = 0;
 	gem_exec[0].rsvd2 = 0;
 
@@ -154,12 +154,11 @@ static void execN(int fd, uint32_t handle, uint64_t batch_size, unsigned flags,
 	gem_exec[0].handle = handle;
 	gem_exec[0].relocation_count = nreloc;
 	gem_exec[0].relocs_ptr = to_user_pointer(gem_reloc);
+	gem_exec[0].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(gem_exec);
 	execbuf.buffer_count = 1;
-	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = 8;
 	execbuf.flags = flags;
 
 	/* Avoid hitting slowpaths in the reloc processing which might yield a
@@ -197,16 +196,10 @@ static void execN(int fd, uint32_t handle, uint64_t batch_size, unsigned flags,
 #undef reloc_ofs
 }
 
-igt_simple_main
+static void exhaustive(int fd)
 {
 	uint32_t batch[2] = {MI_BATCH_BUFFER_END};
 	uint64_t batch_size, max, ggtt_max, reloc_ofs;
-	int fd;
-
-	fd = drm_open_driver(DRIVER_INTEL);
-	igt_require_gem(fd);
-
-	use_64bit_relocs = intel_gen(intel_get_drm_devid(fd)) >= 8;
 
 	max = 3 * gem_aperture_size(fd) / 4;
 	ggtt_max = 3 * gem_global_aperture_size(fd) / 4;
@@ -258,6 +251,61 @@ igt_simple_main
 		else
 			batch_size *= 2;
 	}
+}
+
+static void single(int i915)
+{
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	uint64_t batch_size, limit;
+	uint32_t handle;
+	void *ptr;
+
+	batch_size = (intel_get_avail_ram_mb() - 128) << 20; /* CI slack */
+	limit = gem_aperture_size(i915) - (256 << 10); /* low pages reserved */
+	if (!gem_uses_full_ppgtt(i915))
+		limit = 3 * limit / 4;
+
+	batch_size = min(batch_size, limit);
+	batch_size = ALIGN(batch_size, 4096);
+	igt_info("Submitting a %'"PRId64"MiB batch, %saperture size %'"PRId64"MiB\n",
+		 batch_size >> 20,
+		 gem_uses_full_ppgtt(i915) ? "" : "shared ",
+		 gem_aperture_size(i915) >> 20);
+	intel_require_memory(1, batch_size, CHECK_RAM);
+
+	handle = gem_create(i915, batch_size);
+	gem_write(i915, handle, 0, &bbe, sizeof(bbe));
+
+	if (!FORCE_PREAD_PWRITE && gem_has_llc(i915))
+		ptr = __gem_mmap__cpu(i915, handle, 0, batch_size, PROT_READ);
+	else if (!FORCE_PREAD_PWRITE && gem_mmap__has_wc(i915))
+		ptr = __gem_mmap__wc(i915, handle, 0, batch_size, PROT_READ);
+	else
+		ptr = NULL;
+
+	execN(i915, handle, batch_size, 0, ptr);
+
+	if (ptr)
+		munmap(ptr, batch_size);
+}
+
+igt_main
+{
+	int i915 = -1;
+
+	igt_fixture {
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
+
+		use_64bit_relocs = intel_gen(intel_get_drm_devid(i915)) >= 8;
+	}
+
+	igt_subtest("single")
+		single(i915);
+
+	igt_subtest("exhaustive")
+		exhaustive(i915);
 
-	close(fd);
+	igt_fixture
+		close(i915);
 }
diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt
index f9ad47ea0..9a1dd0e63 100644
--- a/tests/intel-ci/blacklist.txt
+++ b/tests/intel-ci/blacklist.txt
@@ -28,6 +28,7 @@ igt@gem_ctx_thrash(@.*)?
 igt@gem_evict_alignment(@.*)?
 igt@gem_evict_everything(@.*)?
 igt@gem_exec_alignment@(?!.*single).*
+igt@gem_exec_big@(?!.*single).*
 igt@gem_exec_capture@many-(?!4K-).*
 igt@gem_exec_fence@(?!.*basic).*
 igt@gem_exec_flush@(?!.*basic).*
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 5/8] i915/gem_exec_big: Add a single shot test

[Chris Wilson]

CI complains that the exhaustive test of trying every size up to the
limit is too slow, so add a simple test that tries to submit one
extreme batch buffer and check all the relocations land.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105555
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/i915/gem_exec_big.c    | 70 ++++++++++++++++++++++++++++++------
 tests/intel-ci/blacklist.txt |  1 +
 2 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/tests/i915/gem_exec_big.c b/tests/i915/gem_exec_big.c
index a15672f66..015f59e29 100644
--- a/tests/i915/gem_exec_big.c
+++ b/tests/i915/gem_exec_big.c
@@ -71,7 +71,7 @@ static void exec1(int fd, uint32_t handle, uint64_t reloc_ofs, unsigned flags, c
 	gem_exec[0].relocs_ptr = to_user_pointer(gem_reloc);
 	gem_exec[0].alignment = 0;
 	gem_exec[0].offset = 0;
-	gem_exec[0].flags = 0;
+	gem_exec[0].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
 	gem_exec[0].rsvd1 = 0;
 	gem_exec[0].rsvd2 = 0;
 
@@ -154,12 +154,11 @@ static void execN(int fd, uint32_t handle, uint64_t batch_size, unsigned flags,
 	gem_exec[0].handle = handle;
 	gem_exec[0].relocation_count = nreloc;
 	gem_exec[0].relocs_ptr = to_user_pointer(gem_reloc);
+	gem_exec[0].flags = EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(gem_exec);
 	execbuf.buffer_count = 1;
-	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = 8;
 	execbuf.flags = flags;
 
 	/* Avoid hitting slowpaths in the reloc processing which might yield a
@@ -197,16 +196,10 @@ static void execN(int fd, uint32_t handle, uint64_t batch_size, unsigned flags,
 #undef reloc_ofs
 }
 
-igt_simple_main
+static void exhaustive(int fd)
 {
 	uint32_t batch[2] = {MI_BATCH_BUFFER_END};
 	uint64_t batch_size, max, ggtt_max, reloc_ofs;
-	int fd;
-
-	fd = drm_open_driver(DRIVER_INTEL);
-	igt_require_gem(fd);
-
-	use_64bit_relocs = intel_gen(intel_get_drm_devid(fd)) >= 8;
 
 	max = 3 * gem_aperture_size(fd) / 4;
 	ggtt_max = 3 * gem_global_aperture_size(fd) / 4;
@@ -258,6 +251,61 @@ igt_simple_main
 		else
 			batch_size *= 2;
 	}
+}
+
+static void single(int i915)
+{
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
+	uint64_t batch_size, limit;
+	uint32_t handle;
+	void *ptr;
+
+	batch_size = (intel_get_avail_ram_mb() - 128) << 20; /* CI slack */
+	limit = gem_aperture_size(i915) - (256 << 10); /* low pages reserved */
+	if (!gem_uses_full_ppgtt(i915))
+		limit = 3 * limit / 4;
+
+	batch_size = min(batch_size, limit);
+	batch_size = ALIGN(batch_size, 4096);
+	igt_info("Submitting a %'"PRId64"MiB batch, %saperture size %'"PRId64"MiB\n",
+		 batch_size >> 20,
+		 gem_uses_full_ppgtt(i915) ? "" : "shared ",
+		 gem_aperture_size(i915) >> 20);
+	intel_require_memory(1, batch_size, CHECK_RAM);
+
+	handle = gem_create(i915, batch_size);
+	gem_write(i915, handle, 0, &bbe, sizeof(bbe));
+
+	if (!FORCE_PREAD_PWRITE && gem_has_llc(i915))
+		ptr = __gem_mmap__cpu(i915, handle, 0, batch_size, PROT_READ);
+	else if (!FORCE_PREAD_PWRITE && gem_mmap__has_wc(i915))
+		ptr = __gem_mmap__wc(i915, handle, 0, batch_size, PROT_READ);
+	else
+		ptr = NULL;
+
+	execN(i915, handle, batch_size, 0, ptr);
+
+	if (ptr)
+		munmap(ptr, batch_size);
+}
+
+igt_main
+{
+	int i915 = -1;
+
+	igt_fixture {
+		i915 = drm_open_driver(DRIVER_INTEL);
+		igt_require_gem(i915);
+
+		use_64bit_relocs = intel_gen(intel_get_drm_devid(i915)) >= 8;
+	}
+
+	igt_subtest("single")
+		single(i915);
+
+	igt_subtest("exhaustive")
+		exhaustive(i915);
 
-	close(fd);
+	igt_fixture
+		close(i915);
 }
diff --git a/tests/intel-ci/blacklist.txt b/tests/intel-ci/blacklist.txt
index f9ad47ea0..9a1dd0e63 100644
--- a/tests/intel-ci/blacklist.txt
+++ b/tests/intel-ci/blacklist.txt
@@ -28,6 +28,7 @@ igt@gem_ctx_thrash(@.*)?
 igt@gem_evict_alignment(@.*)?
 igt@gem_evict_everything(@.*)?
 igt@gem_exec_alignment@(?!.*single).*
+igt@gem_exec_big@(?!.*single).*
 igt@gem_exec_capture@many-(?!4K-).*
 igt@gem_exec_fence@(?!.*basic).*
 igt@gem_exec_flush@(?!.*basic).*
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 6/8] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

basic-allocations was written to demonstrate a flaw in our continual
reallocation of cmdparser shadow bo, largely fixed by keeping a small
cache of bo of different lengths (to speed up the search for the correct
sized bo). We only care enough to exercise the slowdown by submitting
lots of execbufs, and can see the effect of bo caching on the rate, so
replace the fixed number of iterations with a timeout and count how many
batches we could submit instead.

Similarly, we now do not need to wait for all of our queue to complete
as we can tell the kernel to drop the queue instead.

References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/gem_exec_parse.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
index b653b1bdc..62e8d0a51 100644
--- a/tests/i915/gem_exec_parse.c
+++ b/tests/i915/gem_exec_parse.c
@@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
 
 static void test_allocations(int fd)
 {
-	uint32_t bbe = MI_BATCH_BUFFER_END;
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_exec_object2 obj[17];
-	int i, j;
+	unsigned long count;
 
 	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
 
 	memset(obj, 0, sizeof(obj));
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		uint64_t size = 1ull << (12 + i);
 
 		obj[i].handle = gem_create(fd, size);
@@ -322,17 +322,21 @@ static void test_allocations(int fd)
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffer_count = 1;
-	for (j = 0; j < 16384; j++) {
-		igt_progress("allocations ", j, 16384);
-		i = rand() % ARRAY_SIZE(obj);
+
+	count = 0;
+	igt_until_timeout(20) {
+		int i = rand() % ARRAY_SIZE(obj);
 		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
 		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
 		execbuf.batch_start_offset += 64 * (rand() % 64);
 		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
 		gem_execbuf(fd, &execbuf);
+		count++;
 	}
+	igt_info("Submitted %lu execbufs\n", count);
+	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
 
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		gem_sync(fd, obj[i].handle);
 		gem_close(fd, obj[i].handle);
 	}
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 6/8] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

basic-allocations was written to demonstrate a flaw in our continual
reallocation of cmdparser shadow bo, largely fixed by keeping a small
cache of bo of different lengths (to speed up the search for the correct
sized bo). We only care enough to exercise the slowdown by submitting
lots of execbufs, and can see the effect of bo caching on the rate, so
replace the fixed number of iterations with a timeout and count how many
batches we could submit instead.

Similarly, we now do not need to wait for all of our queue to complete
as we can tell the kernel to drop the queue instead.

References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/gem_exec_parse.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
index b653b1bdc..62e8d0a51 100644
--- a/tests/i915/gem_exec_parse.c
+++ b/tests/i915/gem_exec_parse.c
@@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
 
 static void test_allocations(int fd)
 {
-	uint32_t bbe = MI_BATCH_BUFFER_END;
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_exec_object2 obj[17];
-	int i, j;
+	unsigned long count;
 
 	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
 
 	memset(obj, 0, sizeof(obj));
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		uint64_t size = 1ull << (12 + i);
 
 		obj[i].handle = gem_create(fd, size);
@@ -322,17 +322,21 @@ static void test_allocations(int fd)
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffer_count = 1;
-	for (j = 0; j < 16384; j++) {
-		igt_progress("allocations ", j, 16384);
-		i = rand() % ARRAY_SIZE(obj);
+
+	count = 0;
+	igt_until_timeout(20) {
+		int i = rand() % ARRAY_SIZE(obj);
 		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
 		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
 		execbuf.batch_start_offset += 64 * (rand() % 64);
 		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
 		gem_execbuf(fd, &execbuf);
+		count++;
 	}
+	igt_info("Submitted %lu execbufs\n", count);
+	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
 
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		gem_sync(fd, obj[i].handle);
 		gem_close(fd, obj[i].handle);
 	}
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 6/8] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Tvrtko Ursulin]

On 17/02/2019 14:35, Chris Wilson wrote:
> basic-allocations was written to demonstrate a flaw in our continual
> reallocation of cmdparser shadow bo, largely fixed by keeping a small
> cache of bo of different lengths (to speed up the search for the correct
> sized bo). We only care enough to exercise the slowdown by submitting
> lots of execbufs, and can see the effect of bo caching on the rate, so
> replace the fixed number of iterations with a timeout and count how many
> batches we could submit instead.
> 
> Similarly, we now do not need to wait for all of our queue to complete
> as we can tell the kernel to drop the queue instead.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/i915/gem_exec_parse.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> index b653b1bdc..62e8d0a51 100644
> --- a/tests/i915/gem_exec_parse.c
> +++ b/tests/i915/gem_exec_parse.c
> @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
>   
>   static void test_allocations(int fd)
>   {
> -	uint32_t bbe = MI_BATCH_BUFFER_END;
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
>   	struct drm_i915_gem_execbuffer2 execbuf;
>   	struct drm_i915_gem_exec_object2 obj[17];
> -	int i, j;
> +	unsigned long count;
>   
>   	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
>   
>   	memset(obj, 0, sizeof(obj));
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		uint64_t size = 1ull << (12 + i);
>   
>   		obj[i].handle = gem_create(fd, size);
> @@ -322,17 +322,21 @@ static void test_allocations(int fd)
>   
>   	memset(&execbuf, 0, sizeof(execbuf));
>   	execbuf.buffer_count = 1;
> -	for (j = 0; j < 16384; j++) {
> -		igt_progress("allocations ", j, 16384);
> -		i = rand() % ARRAY_SIZE(obj);
> +
> +	count = 0;
> +	igt_until_timeout(20) {
> +		int i = rand() % ARRAY_SIZE(obj);
>   		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
>   		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
>   		execbuf.batch_start_offset += 64 * (rand() % 64);
>   		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
>   		gem_execbuf(fd, &execbuf);
> +		count++;
>   	}
> +	igt_info("Submitted %lu execbufs\n", count);
> +	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
>   
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		gem_sync(fd, obj[i].handle);
>   		gem_close(fd, obj[i].handle);
>   	}
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 6/8] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Tvrtko Ursulin]

On 17/02/2019 14:35, Chris Wilson wrote:
> basic-allocations was written to demonstrate a flaw in our continual
> reallocation of cmdparser shadow bo, largely fixed by keeping a small
> cache of bo of different lengths (to speed up the search for the correct
> sized bo). We only care enough to exercise the slowdown by submitting
> lots of execbufs, and can see the effect of bo caching on the rate, so
> replace the fixed number of iterations with a timeout and count how many
> batches we could submit instead.
> 
> Similarly, we now do not need to wait for all of our queue to complete
> as we can tell the kernel to drop the queue instead.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/i915/gem_exec_parse.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> index b653b1bdc..62e8d0a51 100644
> --- a/tests/i915/gem_exec_parse.c
> +++ b/tests/i915/gem_exec_parse.c
> @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
>   
>   static void test_allocations(int fd)
>   {
> -	uint32_t bbe = MI_BATCH_BUFFER_END;
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
>   	struct drm_i915_gem_execbuffer2 execbuf;
>   	struct drm_i915_gem_exec_object2 obj[17];
> -	int i, j;
> +	unsigned long count;
>   
>   	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
>   
>   	memset(obj, 0, sizeof(obj));
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		uint64_t size = 1ull << (12 + i);
>   
>   		obj[i].handle = gem_create(fd, size);
> @@ -322,17 +322,21 @@ static void test_allocations(int fd)
>   
>   	memset(&execbuf, 0, sizeof(execbuf));
>   	execbuf.buffer_count = 1;
> -	for (j = 0; j < 16384; j++) {
> -		igt_progress("allocations ", j, 16384);
> -		i = rand() % ARRAY_SIZE(obj);
> +
> +	count = 0;
> +	igt_until_timeout(20) {
> +		int i = rand() % ARRAY_SIZE(obj);
>   		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
>   		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
>   		execbuf.batch_start_offset += 64 * (rand() % 64);
>   		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
>   		gem_execbuf(fd, &execbuf);
> +		count++;
>   	}
> +	igt_info("Submitted %lu execbufs\n", count);
> +	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
>   
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		gem_sync(fd, obj[i].handle);
>   		gem_close(fd, obj[i].handle);
>   	}
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 7/8] kms_fence_pin_leak: Ask for the GPU before use

[Chris Wilson]

Check that the GPU even exists before submitting a batch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109589
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/kms_fence_pin_leak.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/kms_fence_pin_leak.c b/tests/kms_fence_pin_leak.c
index 62c52b627..e6c8b33c3 100644
--- a/tests/kms_fence_pin_leak.c
+++ b/tests/kms_fence_pin_leak.c
@@ -201,6 +201,7 @@ igt_simple_main
 	igt_skip_on_simulation();
 
 	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+	igt_require_gem(data.drm_fd);
 
 	data.devid = intel_get_drm_devid(data.drm_fd);
 
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 7/8] kms_fence_pin_leak: Ask for the GPU before use

[Chris Wilson]

Check that the GPU even exists before submitting a batch.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109589
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/kms_fence_pin_leak.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/kms_fence_pin_leak.c b/tests/kms_fence_pin_leak.c
index 62c52b627..e6c8b33c3 100644
--- a/tests/kms_fence_pin_leak.c
+++ b/tests/kms_fence_pin_leak.c
@@ -201,6 +201,7 @@ igt_simple_main
 	igt_skip_on_simulation();
 
 	data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+	igt_require_gem(data.drm_fd);
 
 	data.devid = intel_get_drm_devid(data.drm_fd);
 
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
requires using GEM to do so). It doesn't belong in the general paddock
of all driver tests, so move it into the i915/ stable.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
---
 tests/Makefile.sources                | 5 ++++-
 tests/{ => i915}/kms_fence_pin_leak.c | 0
 tests/meson.build                     | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)
 rename tests/{ => i915}/kms_fence_pin_leak.c (100%)

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d2c4f9fe9..9972b2dd1 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -40,7 +40,6 @@ TESTS_progs = \
 	kms_dp_dsc \
 	kms_draw_crc \
 	kms_fbcon_fbt \
-	kms_fence_pin_leak \
 	kms_flip \
 	kms_flip_event_leak \
 	kms_flip_tiling \
@@ -99,6 +98,10 @@ TESTS_progs = \
 	vgem_slow \
 	$(NULL)
 
+TESTS_progs += \
+	i915/kms_fence_pin_leak \
+	$(NULL)
+
 TESTS_progs += gem_bad_reloc
 gem_bad_reloc_SOURCES = i915/gem_bad_reloc.c
 
diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
similarity index 100%
rename from tests/kms_fence_pin_leak.c
rename to tests/i915/kms_fence_pin_leak.c
diff --git a/tests/meson.build b/tests/meson.build
index ec980651a..08e55b9c0 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -27,7 +27,6 @@ test_progs = [
 	'kms_dp_dsc',
 	'kms_draw_crc',
 	'kms_fbcon_fbt',
-	'kms_fence_pin_leak',
 	'kms_flip',
 	'kms_flip_event_leak',
 	'kms_flip_tiling',
@@ -100,6 +99,7 @@ i915_progs = [
 	'fb_tiling',
 	'getparams_basic',
 	'hangman',
+	'kms_fence_pin_leak',
 	'missed_irq',
 	'module_load',
 	'query',
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
requires using GEM to do so). It doesn't belong in the general paddock
of all driver tests, so move it into the i915/ stable.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
---
 tests/Makefile.sources                | 5 ++++-
 tests/{ => i915}/kms_fence_pin_leak.c | 0
 tests/meson.build                     | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)
 rename tests/{ => i915}/kms_fence_pin_leak.c (100%)

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index d2c4f9fe9..9972b2dd1 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -40,7 +40,6 @@ TESTS_progs = \
 	kms_dp_dsc \
 	kms_draw_crc \
 	kms_fbcon_fbt \
-	kms_fence_pin_leak \
 	kms_flip \
 	kms_flip_event_leak \
 	kms_flip_tiling \
@@ -99,6 +98,10 @@ TESTS_progs = \
 	vgem_slow \
 	$(NULL)
 
+TESTS_progs += \
+	i915/kms_fence_pin_leak \
+	$(NULL)
+
 TESTS_progs += gem_bad_reloc
 gem_bad_reloc_SOURCES = i915/gem_bad_reloc.c
 
diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
similarity index 100%
rename from tests/kms_fence_pin_leak.c
rename to tests/i915/kms_fence_pin_leak.c
diff --git a/tests/meson.build b/tests/meson.build
index ec980651a..08e55b9c0 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -27,7 +27,6 @@ test_progs = [
 	'kms_dp_dsc',
 	'kms_draw_crc',
 	'kms_fbcon_fbt',
-	'kms_fence_pin_leak',
 	'kms_flip',
 	'kms_flip_event_leak',
 	'kms_flip_tiling',
@@ -100,6 +99,7 @@ i915_progs = [
 	'fb_tiling',
 	'getparams_basic',
 	'hangman',
+	'kms_fence_pin_leak',
 	'missed_irq',
 	'module_load',
 	'query',
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> requires using GEM to do so). It doesn't belong in the general paddock
> of all driver tests, so move it into the i915/ stable.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Acked-by: Petri Latvala <petri.latvala@intel.com>
> ---
>  tests/Makefile.sources                | 5 ++++-
>  tests/{ => i915}/kms_fence_pin_leak.c | 0
>  tests/meson.build                     | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
>  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index d2c4f9fe9..9972b2dd1 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -40,7 +40,6 @@ TESTS_progs = \
>  	kms_dp_dsc \
>  	kms_draw_crc \
>  	kms_fbcon_fbt \
> -	kms_fence_pin_leak \
>  	kms_flip \
>  	kms_flip_event_leak \
>  	kms_flip_tiling \
> @@ -99,6 +98,10 @@ TESTS_progs = \
>  	vgem_slow \
>  	$(NULL)
>  
> +TESTS_progs += \
> +	i915/kms_fence_pin_leak \
> +	$(NULL)

This just moves it around, so we will end up having binary named
'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.

That still will install as
$PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak

If you want to prefix it:
 TESTS_progs += i915_kms_fence_pin_leak
 i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c

Oterwise:
 TESTS_progs += kms_fence_pin_leak
 kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c

> diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> similarity index 100%
> rename from tests/kms_fence_pin_leak.c
> rename to tests/i915/kms_fence_pin_leak.c
> diff --git a/tests/meson.build b/tests/meson.build
> index ec980651a..08e55b9c0 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -27,7 +27,6 @@ test_progs = [
>  	'kms_dp_dsc',
>  	'kms_draw_crc',
>  	'kms_fbcon_fbt',
> -	'kms_fence_pin_leak',
>  	'kms_flip',
>  	'kms_flip_event_leak',
>  	'kms_flip_tiling',
> @@ -100,6 +99,7 @@ i915_progs = [
>  	'fb_tiling',
>  	'getparams_basic',
>  	'hangman',
> +	'kms_fence_pin_leak',
>  	'missed_irq',
>  	'module_load',
>  	'query',

Here, with meson, it will get prefixed with i915_. I'll add a comment on
top of i915_progs just to be more explicit.

Do we have any conclusion on prefixing Intel-specific kms tests
yet?

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> requires using GEM to do so). It doesn't belong in the general paddock
> of all driver tests, so move it into the i915/ stable.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Acked-by: Petri Latvala <petri.latvala@intel.com>
> ---
>  tests/Makefile.sources                | 5 ++++-
>  tests/{ => i915}/kms_fence_pin_leak.c | 0
>  tests/meson.build                     | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
>  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index d2c4f9fe9..9972b2dd1 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -40,7 +40,6 @@ TESTS_progs = \
>  	kms_dp_dsc \
>  	kms_draw_crc \
>  	kms_fbcon_fbt \
> -	kms_fence_pin_leak \
>  	kms_flip \
>  	kms_flip_event_leak \
>  	kms_flip_tiling \
> @@ -99,6 +98,10 @@ TESTS_progs = \
>  	vgem_slow \
>  	$(NULL)
>  
> +TESTS_progs += \
> +	i915/kms_fence_pin_leak \
> +	$(NULL)

This just moves it around, so we will end up having binary named
'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.

That still will install as
$PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak

If you want to prefix it:
 TESTS_progs += i915_kms_fence_pin_leak
 i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c

Oterwise:
 TESTS_progs += kms_fence_pin_leak
 kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c

> diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> similarity index 100%
> rename from tests/kms_fence_pin_leak.c
> rename to tests/i915/kms_fence_pin_leak.c
> diff --git a/tests/meson.build b/tests/meson.build
> index ec980651a..08e55b9c0 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -27,7 +27,6 @@ test_progs = [
>  	'kms_dp_dsc',
>  	'kms_draw_crc',
>  	'kms_fbcon_fbt',
> -	'kms_fence_pin_leak',
>  	'kms_flip',
>  	'kms_flip_event_leak',
>  	'kms_flip_tiling',
> @@ -100,6 +99,7 @@ i915_progs = [
>  	'fb_tiling',
>  	'getparams_basic',
>  	'hangman',
> +	'kms_fence_pin_leak',
>  	'missed_irq',
>  	'module_load',
>  	'query',

Here, with meson, it will get prefixed with i915_. I'll add a comment on
top of i915_progs just to be more explicit.

Do we have any conclusion on prefixing Intel-specific kms tests
yet?

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > requires using GEM to do so). It doesn't belong in the general paddock
> > of all driver tests, so move it into the i915/ stable.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > ---
> >  tests/Makefile.sources                | 5 ++++-
> >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> >  tests/meson.build                     | 2 +-
> >  3 files changed, 5 insertions(+), 2 deletions(-)
> >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index d2c4f9fe9..9972b2dd1 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -40,7 +40,6 @@ TESTS_progs = \
> >       kms_dp_dsc \
> >       kms_draw_crc \
> >       kms_fbcon_fbt \
> > -     kms_fence_pin_leak \
> >       kms_flip \
> >       kms_flip_event_leak \
> >       kms_flip_tiling \
> > @@ -99,6 +98,10 @@ TESTS_progs = \
> >       vgem_slow \
> >       $(NULL)
> >  
> > +TESTS_progs += \
> > +     i915/kms_fence_pin_leak \
> > +     $(NULL)
> 
> This just moves it around, so we will end up having binary named
> 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> 
> That still will install as
> $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> 
> If you want to prefix it:
>  TESTS_progs += i915_kms_fence_pin_leak
>  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> 
> Oterwise:
>  TESTS_progs += kms_fence_pin_leak
>  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> 
> > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > similarity index 100%
> > rename from tests/kms_fence_pin_leak.c
> > rename to tests/i915/kms_fence_pin_leak.c
> > diff --git a/tests/meson.build b/tests/meson.build
> > index ec980651a..08e55b9c0 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -27,7 +27,6 @@ test_progs = [
> >       'kms_dp_dsc',
> >       'kms_draw_crc',
> >       'kms_fbcon_fbt',
> > -     'kms_fence_pin_leak',
> >       'kms_flip',
> >       'kms_flip_event_leak',
> >       'kms_flip_tiling',
> > @@ -100,6 +99,7 @@ i915_progs = [
> >       'fb_tiling',
> >       'getparams_basic',
> >       'hangman',
> > +     'kms_fence_pin_leak',
> >       'missed_irq',
> >       'module_load',
> >       'query',
> 
> Here, with meson, it will get prefixed with i915_. I'll add a comment on
> top of i915_progs just to be more explicit.
> 
> Do we have any conclusion on prefixing Intel-specific kms tests
> yet?

I'd rather not have tests renamed. That's my personal preference. Either
it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
or it should be installed under tests/i915/.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > requires using GEM to do so). It doesn't belong in the general paddock
> > of all driver tests, so move it into the i915/ stable.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > ---
> >  tests/Makefile.sources                | 5 ++++-
> >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> >  tests/meson.build                     | 2 +-
> >  3 files changed, 5 insertions(+), 2 deletions(-)
> >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index d2c4f9fe9..9972b2dd1 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -40,7 +40,6 @@ TESTS_progs = \
> >       kms_dp_dsc \
> >       kms_draw_crc \
> >       kms_fbcon_fbt \
> > -     kms_fence_pin_leak \
> >       kms_flip \
> >       kms_flip_event_leak \
> >       kms_flip_tiling \
> > @@ -99,6 +98,10 @@ TESTS_progs = \
> >       vgem_slow \
> >       $(NULL)
> >  
> > +TESTS_progs += \
> > +     i915/kms_fence_pin_leak \
> > +     $(NULL)
> 
> This just moves it around, so we will end up having binary named
> 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> 
> That still will install as
> $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> 
> If you want to prefix it:
>  TESTS_progs += i915_kms_fence_pin_leak
>  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> 
> Oterwise:
>  TESTS_progs += kms_fence_pin_leak
>  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> 
> > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > similarity index 100%
> > rename from tests/kms_fence_pin_leak.c
> > rename to tests/i915/kms_fence_pin_leak.c
> > diff --git a/tests/meson.build b/tests/meson.build
> > index ec980651a..08e55b9c0 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -27,7 +27,6 @@ test_progs = [
> >       'kms_dp_dsc',
> >       'kms_draw_crc',
> >       'kms_fbcon_fbt',
> > -     'kms_fence_pin_leak',
> >       'kms_flip',
> >       'kms_flip_event_leak',
> >       'kms_flip_tiling',
> > @@ -100,6 +99,7 @@ i915_progs = [
> >       'fb_tiling',
> >       'getparams_basic',
> >       'hangman',
> > +     'kms_fence_pin_leak',
> >       'missed_irq',
> >       'module_load',
> >       'query',
> 
> Here, with meson, it will get prefixed with i915_. I'll add a comment on
> top of i915_progs just to be more explicit.
> 
> Do we have any conclusion on prefixing Intel-specific kms tests
> yet?

I'd rather not have tests renamed. That's my personal preference. Either
it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
or it should be installed under tests/i915/.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

Quoting Chris Wilson (2019-02-18 13:42:48)
> Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > requires using GEM to do so). It doesn't belong in the general paddock
> > > of all driver tests, so move it into the i915/ stable.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > ---
> > >  tests/Makefile.sources                | 5 ++++-
> > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > >  tests/meson.build                     | 2 +-
> > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > 
> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > index d2c4f9fe9..9972b2dd1 100644
> > > --- a/tests/Makefile.sources
> > > +++ b/tests/Makefile.sources
> > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > >       kms_dp_dsc \
> > >       kms_draw_crc \
> > >       kms_fbcon_fbt \
> > > -     kms_fence_pin_leak \
> > >       kms_flip \
> > >       kms_flip_event_leak \
> > >       kms_flip_tiling \
> > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > >       vgem_slow \
> > >       $(NULL)
> > >  
> > > +TESTS_progs += \
> > > +     i915/kms_fence_pin_leak \
> > > +     $(NULL)
> > 
> > This just moves it around, so we will end up having binary named
> > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > 
> > That still will install as
> > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > 
> > If you want to prefix it:
> >  TESTS_progs += i915_kms_fence_pin_leak
> >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > 
> > Oterwise:
> >  TESTS_progs += kms_fence_pin_leak
> >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > 
> > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > similarity index 100%
> > > rename from tests/kms_fence_pin_leak.c
> > > rename to tests/i915/kms_fence_pin_leak.c
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index ec980651a..08e55b9c0 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -27,7 +27,6 @@ test_progs = [
> > >       'kms_dp_dsc',
> > >       'kms_draw_crc',
> > >       'kms_fbcon_fbt',
> > > -     'kms_fence_pin_leak',
> > >       'kms_flip',
> > >       'kms_flip_event_leak',
> > >       'kms_flip_tiling',
> > > @@ -100,6 +99,7 @@ i915_progs = [
> > >       'fb_tiling',
> > >       'getparams_basic',
> > >       'hangman',
> > > +     'kms_fence_pin_leak',
> > >       'missed_irq',
> > >       'module_load',
> > >       'query',
> > 
> > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > top of i915_progs just to be more explicit.
> > 
> > Do we have any conclusion on prefixing Intel-specific kms tests
> > yet?
> 
> I'd rather not have tests renamed. That's my personal preference. Either
> it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> or it should be installed under tests/i915/.

As a case in point, the renaming of benchmarks by meson breaks
scripts. Please do not do that.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

Quoting Chris Wilson (2019-02-18 13:42:48)
> Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > requires using GEM to do so). It doesn't belong in the general paddock
> > > of all driver tests, so move it into the i915/ stable.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > ---
> > >  tests/Makefile.sources                | 5 ++++-
> > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > >  tests/meson.build                     | 2 +-
> > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > 
> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > index d2c4f9fe9..9972b2dd1 100644
> > > --- a/tests/Makefile.sources
> > > +++ b/tests/Makefile.sources
> > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > >       kms_dp_dsc \
> > >       kms_draw_crc \
> > >       kms_fbcon_fbt \
> > > -     kms_fence_pin_leak \
> > >       kms_flip \
> > >       kms_flip_event_leak \
> > >       kms_flip_tiling \
> > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > >       vgem_slow \
> > >       $(NULL)
> > >  
> > > +TESTS_progs += \
> > > +     i915/kms_fence_pin_leak \
> > > +     $(NULL)
> > 
> > This just moves it around, so we will end up having binary named
> > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > 
> > That still will install as
> > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > 
> > If you want to prefix it:
> >  TESTS_progs += i915_kms_fence_pin_leak
> >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > 
> > Oterwise:
> >  TESTS_progs += kms_fence_pin_leak
> >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > 
> > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > similarity index 100%
> > > rename from tests/kms_fence_pin_leak.c
> > > rename to tests/i915/kms_fence_pin_leak.c
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index ec980651a..08e55b9c0 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -27,7 +27,6 @@ test_progs = [
> > >       'kms_dp_dsc',
> > >       'kms_draw_crc',
> > >       'kms_fbcon_fbt',
> > > -     'kms_fence_pin_leak',
> > >       'kms_flip',
> > >       'kms_flip_event_leak',
> > >       'kms_flip_tiling',
> > > @@ -100,6 +99,7 @@ i915_progs = [
> > >       'fb_tiling',
> > >       'getparams_basic',
> > >       'hangman',
> > > +     'kms_fence_pin_leak',
> > >       'missed_irq',
> > >       'module_load',
> > >       'query',
> > 
> > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > top of i915_progs just to be more explicit.
> > 
> > Do we have any conclusion on prefixing Intel-specific kms tests
> > yet?
> 
> I'd rather not have tests renamed. That's my personal preference. Either
> it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> or it should be installed under tests/i915/.

As a case in point, the renaming of benchmarks by meson breaks
scripts. Please do not do that.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Mon, Feb 18, 2019 at 01:43:50PM +0000, Chris Wilson wrote:
> Quoting Chris Wilson (2019-02-18 13:42:48)
> > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > of all driver tests, so move it into the i915/ stable.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > > ---
> > > >  tests/Makefile.sources                | 5 ++++-
> > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > >  tests/meson.build                     | 2 +-
> > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > 
> > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > index d2c4f9fe9..9972b2dd1 100644
> > > > --- a/tests/Makefile.sources
> > > > +++ b/tests/Makefile.sources
> > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > >       kms_dp_dsc \
> > > >       kms_draw_crc \
> > > >       kms_fbcon_fbt \
> > > > -     kms_fence_pin_leak \
> > > >       kms_flip \
> > > >       kms_flip_event_leak \
> > > >       kms_flip_tiling \
> > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > >       vgem_slow \
> > > >       $(NULL)
> > > >  
> > > > +TESTS_progs += \
> > > > +     i915/kms_fence_pin_leak \
> > > > +     $(NULL)
> > > 
> > > This just moves it around, so we will end up having binary named
> > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > 
> > > That still will install as
> > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > 
> > > If you want to prefix it:
> > >  TESTS_progs += i915_kms_fence_pin_leak
> > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > 
> > > Oterwise:
> > >  TESTS_progs += kms_fence_pin_leak
> > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > 
> > > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > > similarity index 100%
> > > > rename from tests/kms_fence_pin_leak.c
> > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > index ec980651a..08e55b9c0 100644
> > > > --- a/tests/meson.build
> > > > +++ b/tests/meson.build
> > > > @@ -27,7 +27,6 @@ test_progs = [
> > > >       'kms_dp_dsc',
> > > >       'kms_draw_crc',
> > > >       'kms_fbcon_fbt',
> > > > -     'kms_fence_pin_leak',
> > > >       'kms_flip',
> > > >       'kms_flip_event_leak',
> > > >       'kms_flip_tiling',
> > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > >       'fb_tiling',
> > > >       'getparams_basic',
> > > >       'hangman',
> > > > +     'kms_fence_pin_leak',
> > > >       'missed_irq',
> > > >       'module_load',
> > > >       'query',
> > > 
> > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > top of i915_progs just to be more explicit.
> > > 
> > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > yet?
> > 
> > I'd rather not have tests renamed. That's my personal preference. Either
> > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > or it should be installed under tests/i915/.

You mean you want the binaries to be a straight s/\.c//?

I personally like the current way mostly because we avoid 'i915/i915_'
redundancy and the resulting binaries dir is flat, which makes them
PATH-friendly.

The way we handle gem_ adds a little inconsistency, but i915_gem_ would
be too mouthful.

But if what you are proposing is really essential we can stir the pot
some more and i915_ all the .c(s).

> As a case in point, the renaming of benchmarks by meson breaks
> scripts. Please do not do that.
> -Chris

I lack the context here. Do we have any inconsistencies in how autotools
and meson generate binaries? Which scripts are getting broken?

I can look into that and add more GitLab CI/CD consistency checks.

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Mon, Feb 18, 2019 at 01:43:50PM +0000, Chris Wilson wrote:
> Quoting Chris Wilson (2019-02-18 13:42:48)
> > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > of all driver tests, so move it into the i915/ stable.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > > ---
> > > >  tests/Makefile.sources                | 5 ++++-
> > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > >  tests/meson.build                     | 2 +-
> > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > 
> > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > index d2c4f9fe9..9972b2dd1 100644
> > > > --- a/tests/Makefile.sources
> > > > +++ b/tests/Makefile.sources
> > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > >       kms_dp_dsc \
> > > >       kms_draw_crc \
> > > >       kms_fbcon_fbt \
> > > > -     kms_fence_pin_leak \
> > > >       kms_flip \
> > > >       kms_flip_event_leak \
> > > >       kms_flip_tiling \
> > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > >       vgem_slow \
> > > >       $(NULL)
> > > >  
> > > > +TESTS_progs += \
> > > > +     i915/kms_fence_pin_leak \
> > > > +     $(NULL)
> > > 
> > > This just moves it around, so we will end up having binary named
> > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > 
> > > That still will install as
> > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > 
> > > If you want to prefix it:
> > >  TESTS_progs += i915_kms_fence_pin_leak
> > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > 
> > > Oterwise:
> > >  TESTS_progs += kms_fence_pin_leak
> > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > 
> > > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > > similarity index 100%
> > > > rename from tests/kms_fence_pin_leak.c
> > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > index ec980651a..08e55b9c0 100644
> > > > --- a/tests/meson.build
> > > > +++ b/tests/meson.build
> > > > @@ -27,7 +27,6 @@ test_progs = [
> > > >       'kms_dp_dsc',
> > > >       'kms_draw_crc',
> > > >       'kms_fbcon_fbt',
> > > > -     'kms_fence_pin_leak',
> > > >       'kms_flip',
> > > >       'kms_flip_event_leak',
> > > >       'kms_flip_tiling',
> > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > >       'fb_tiling',
> > > >       'getparams_basic',
> > > >       'hangman',
> > > > +     'kms_fence_pin_leak',
> > > >       'missed_irq',
> > > >       'module_load',
> > > >       'query',
> > > 
> > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > top of i915_progs just to be more explicit.
> > > 
> > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > yet?
> > 
> > I'd rather not have tests renamed. That's my personal preference. Either
> > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > or it should be installed under tests/i915/.

You mean you want the binaries to be a straight s/\.c//?

I personally like the current way mostly because we avoid 'i915/i915_'
redundancy and the resulting binaries dir is flat, which makes them
PATH-friendly.

The way we handle gem_ adds a little inconsistency, but i915_gem_ would
be too mouthful.

But if what you are proposing is really essential we can stir the pot
some more and i915_ all the .c(s).

> As a case in point, the renaming of benchmarks by meson breaks
> scripts. Please do not do that.
> -Chris

I lack the context here. Do we have any inconsistencies in how autotools
and meson generate binaries? Which scripts are getting broken?

I can look into that and add more GitLab CI/CD consistency checks.

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

Quoting Arkadiusz Hiler (2019-02-18 14:01:11)
> On Mon, Feb 18, 2019 at 01:43:50PM +0000, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-02-18 13:42:48)
> > > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > > of all driver tests, so move it into the i915/ stable.
> > > > > 
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > > > ---
> > > > >  tests/Makefile.sources                | 5 ++++-
> > > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > > >  tests/meson.build                     | 2 +-
> > > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > > 
> > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > > index d2c4f9fe9..9972b2dd1 100644
> > > > > --- a/tests/Makefile.sources
> > > > > +++ b/tests/Makefile.sources
> > > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > > >       kms_dp_dsc \
> > > > >       kms_draw_crc \
> > > > >       kms_fbcon_fbt \
> > > > > -     kms_fence_pin_leak \
> > > > >       kms_flip \
> > > > >       kms_flip_event_leak \
> > > > >       kms_flip_tiling \
> > > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > > >       vgem_slow \
> > > > >       $(NULL)
> > > > >  
> > > > > +TESTS_progs += \
> > > > > +     i915/kms_fence_pin_leak \
> > > > > +     $(NULL)
> > > > 
> > > > This just moves it around, so we will end up having binary named
> > > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > > 
> > > > That still will install as
> > > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > > 
> > > > If you want to prefix it:
> > > >  TESTS_progs += i915_kms_fence_pin_leak
> > > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > 
> > > > Oterwise:
> > > >  TESTS_progs += kms_fence_pin_leak
> > > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > 
> > > > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > > > similarity index 100%
> > > > > rename from tests/kms_fence_pin_leak.c
> > > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > index ec980651a..08e55b9c0 100644
> > > > > --- a/tests/meson.build
> > > > > +++ b/tests/meson.build
> > > > > @@ -27,7 +27,6 @@ test_progs = [
> > > > >       'kms_dp_dsc',
> > > > >       'kms_draw_crc',
> > > > >       'kms_fbcon_fbt',
> > > > > -     'kms_fence_pin_leak',
> > > > >       'kms_flip',
> > > > >       'kms_flip_event_leak',
> > > > >       'kms_flip_tiling',
> > > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > > >       'fb_tiling',
> > > > >       'getparams_basic',
> > > > >       'hangman',
> > > > > +     'kms_fence_pin_leak',
> > > > >       'missed_irq',
> > > > >       'module_load',
> > > > >       'query',
> > > > 
> > > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > > top of i915_progs just to be more explicit.
> > > > 
> > > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > > yet?
> > > 
> > > I'd rather not have tests renamed. That's my personal preference. Either
> > > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > > or it should be installed under tests/i915/.
> 
> You mean you want the binaries to be a straight s/\.c//?

Yes. Or that we don't report test names at all, but the path to the
source of the test.
 
> I personally like the current way mostly because we avoid 'i915/i915_'
> redundancy and the resulting binaries dir is flat, which makes them
> PATH-friendly.

They should not be in the PATH!!!!

There doesn't need to be any i915/i915_ redundancy as we then have i915/
in the name.

> The way we handle gem_ adds a little inconsistency, but i915_gem_ would
> be too mouthful.
> 
> But if what you are proposing is really essential we can stir the pot
> some more and i915_ all the .c(s).
> 
> > As a case in point, the renaming of benchmarks by meson breaks
> > scripts. Please do not do that.
> 
> I lack the context here. Do we have any inconsistencies in how autotools
> and meson generate binaries? Which scripts are getting broken?

Yes, meson adds _bench but all of our scripts in igt expect the
existing names. That may only be 15 different scripts...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Chris Wilson]

Quoting Arkadiusz Hiler (2019-02-18 14:01:11)
> On Mon, Feb 18, 2019 at 01:43:50PM +0000, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-02-18 13:42:48)
> > > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > > of all driver tests, so move it into the i915/ stable.
> > > > > 
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > > > ---
> > > > >  tests/Makefile.sources                | 5 ++++-
> > > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > > >  tests/meson.build                     | 2 +-
> > > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > > 
> > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > > index d2c4f9fe9..9972b2dd1 100644
> > > > > --- a/tests/Makefile.sources
> > > > > +++ b/tests/Makefile.sources
> > > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > > >       kms_dp_dsc \
> > > > >       kms_draw_crc \
> > > > >       kms_fbcon_fbt \
> > > > > -     kms_fence_pin_leak \
> > > > >       kms_flip \
> > > > >       kms_flip_event_leak \
> > > > >       kms_flip_tiling \
> > > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > > >       vgem_slow \
> > > > >       $(NULL)
> > > > >  
> > > > > +TESTS_progs += \
> > > > > +     i915/kms_fence_pin_leak \
> > > > > +     $(NULL)
> > > > 
> > > > This just moves it around, so we will end up having binary named
> > > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > > 
> > > > That still will install as
> > > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > > 
> > > > If you want to prefix it:
> > > >  TESTS_progs += i915_kms_fence_pin_leak
> > > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > 
> > > > Oterwise:
> > > >  TESTS_progs += kms_fence_pin_leak
> > > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > 
> > > > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > > > similarity index 100%
> > > > > rename from tests/kms_fence_pin_leak.c
> > > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > index ec980651a..08e55b9c0 100644
> > > > > --- a/tests/meson.build
> > > > > +++ b/tests/meson.build
> > > > > @@ -27,7 +27,6 @@ test_progs = [
> > > > >       'kms_dp_dsc',
> > > > >       'kms_draw_crc',
> > > > >       'kms_fbcon_fbt',
> > > > > -     'kms_fence_pin_leak',
> > > > >       'kms_flip',
> > > > >       'kms_flip_event_leak',
> > > > >       'kms_flip_tiling',
> > > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > > >       'fb_tiling',
> > > > >       'getparams_basic',
> > > > >       'hangman',
> > > > > +     'kms_fence_pin_leak',
> > > > >       'missed_irq',
> > > > >       'module_load',
> > > > >       'query',
> > > > 
> > > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > > top of i915_progs just to be more explicit.
> > > > 
> > > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > > yet?
> > > 
> > > I'd rather not have tests renamed. That's my personal preference. Either
> > > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > > or it should be installed under tests/i915/.
> 
> You mean you want the binaries to be a straight s/\.c//?

Yes. Or that we don't report test names at all, but the path to the
source of the test.
 
> I personally like the current way mostly because we avoid 'i915/i915_'
> redundancy and the resulting binaries dir is flat, which makes them
> PATH-friendly.

They should not be in the PATH!!!!

There doesn't need to be any i915/i915_ redundancy as we then have i915/
in the name.

> The way we handle gem_ adds a little inconsistency, but i915_gem_ would
> be too mouthful.
> 
> But if what you are proposing is really essential we can stir the pot
> some more and i915_ all the .c(s).
> 
> > As a case in point, the renaming of benchmarks by meson breaks
> > scripts. Please do not do that.
> 
> I lack the context here. Do we have any inconsistencies in how autotools
> and meson generate binaries? Which scripts are getting broken?

Yes, meson adds _bench but all of our scripts in igt expect the
existing names. That may only be 15 different scripts...
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Mon, Feb 18, 2019 at 02:08:12PM +0000, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-02-18 14:01:11)
> > On Mon, Feb 18, 2019 at 01:43:50PM +0000, Chris Wilson wrote:
> > > Quoting Chris Wilson (2019-02-18 13:42:48)
> > > > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > > > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > > > of all driver tests, so move it into the i915/ stable.
> > > > > > 
> > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > > > > ---
> > > > > >  tests/Makefile.sources                | 5 ++++-
> > > > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > > > >  tests/meson.build                     | 2 +-
> > > > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > > > 
> > > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > > > index d2c4f9fe9..9972b2dd1 100644
> > > > > > --- a/tests/Makefile.sources
> > > > > > +++ b/tests/Makefile.sources
> > > > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > > > >       kms_dp_dsc \
> > > > > >       kms_draw_crc \
> > > > > >       kms_fbcon_fbt \
> > > > > > -     kms_fence_pin_leak \
> > > > > >       kms_flip \
> > > > > >       kms_flip_event_leak \
> > > > > >       kms_flip_tiling \
> > > > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > > > >       vgem_slow \
> > > > > >       $(NULL)
> > > > > >  
> > > > > > +TESTS_progs += \
> > > > > > +     i915/kms_fence_pin_leak \
> > > > > > +     $(NULL)
> > > > > 
> > > > > This just moves it around, so we will end up having binary named
> > > > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > > > 
> > > > > That still will install as
> > > > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > > > 
> > > > > If you want to prefix it:
> > > > >  TESTS_progs += i915_kms_fence_pin_leak
> > > > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > > 
> > > > > Oterwise:
> > > > >  TESTS_progs += kms_fence_pin_leak
> > > > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > > 
> > > > > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > > > > similarity index 100%
> > > > > > rename from tests/kms_fence_pin_leak.c
> > > > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > > index ec980651a..08e55b9c0 100644
> > > > > > --- a/tests/meson.build
> > > > > > +++ b/tests/meson.build
> > > > > > @@ -27,7 +27,6 @@ test_progs = [
> > > > > >       'kms_dp_dsc',
> > > > > >       'kms_draw_crc',
> > > > > >       'kms_fbcon_fbt',
> > > > > > -     'kms_fence_pin_leak',
> > > > > >       'kms_flip',
> > > > > >       'kms_flip_event_leak',
> > > > > >       'kms_flip_tiling',
> > > > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > > > >       'fb_tiling',
> > > > > >       'getparams_basic',
> > > > > >       'hangman',
> > > > > > +     'kms_fence_pin_leak',
> > > > > >       'missed_irq',
> > > > > >       'module_load',
> > > > > >       'query',
> > > > > 
> > > > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > > > top of i915_progs just to be more explicit.
> > > > > 
> > > > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > > > yet?
> > > > 
> > > > I'd rather not have tests renamed. That's my personal preference. Either
> > > > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > > > or it should be installed under tests/i915/.
> > 
> > You mean you want the binaries to be a straight s/\.c//?
> 
> Yes. Or that we don't report test names at all, but the path to the
> source of the test.
>  
> > I personally like the current way mostly because we avoid 'i915/i915_'
> > redundancy and the resulting binaries dir is flat, which makes them
> > PATH-friendly.
> 
> They should not be in the PATH!!!!
> 
> There doesn't need to be any i915/i915_ redundancy as we then have i915/
> in the name.

That's why its libexec after installing, but I admit to committing the
crime of adding $SRC/build/tests to PATH at times, for convenience when
working on tests.

I think that we are pretty far from being able to handle
igt@tata/toto@subtoto in the CI (quotation needed, Petri?)
and i915/i915_ would be overall less messy of the two.

> > The way we handle gem_ adds a little inconsistency, but i915_gem_ would
> > be too mouthful.
> > 
> > But if what you are proposing is really essential we can stir the pot
> > some more and i915_ all the .c(s).
> > 
> > > As a case in point, the renaming of benchmarks by meson breaks
> > > scripts. Please do not do that.
> > 
> > I lack the context here. Do we have any inconsistencies in how autotools
> > and meson generate binaries? Which scripts are getting broken?
> 
> Yes, meson adds _bench but all of our scripts in igt expect the
> existing names. That may only be 15 different scripts...
> -Chris

Oh. Let me check what we can do about it. I'll make it consisten one way or
the other :-)

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 8/8] kms_fence_pin_leak: Move beneath i915/

[Arkadiusz Hiler]

On Mon, Feb 18, 2019 at 02:08:12PM +0000, Chris Wilson wrote:
> Quoting Arkadiusz Hiler (2019-02-18 14:01:11)
> > On Mon, Feb 18, 2019 at 01:43:50PM +0000, Chris Wilson wrote:
> > > Quoting Chris Wilson (2019-02-18 13:42:48)
> > > > Quoting Arkadiusz Hiler (2019-02-18 13:37:07)
> > > > > On Sun, Feb 17, 2019 at 02:35:56PM +0000, Chris Wilson wrote:
> > > > > > kms_fence_pin_leak tests smooth sharp edges that are i915 specific (and
> > > > > > requires using GEM to do so). It doesn't belong in the general paddock
> > > > > > of all driver tests, so move it into the i915/ stable.
> > > > > > 
> > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > > > > ---
> > > > > >  tests/Makefile.sources                | 5 ++++-
> > > > > >  tests/{ => i915}/kms_fence_pin_leak.c | 0
> > > > > >  tests/meson.build                     | 2 +-
> > > > > >  3 files changed, 5 insertions(+), 2 deletions(-)
> > > > > >  rename tests/{ => i915}/kms_fence_pin_leak.c (100%)
> > > > > > 
> > > > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > > > > > index d2c4f9fe9..9972b2dd1 100644
> > > > > > --- a/tests/Makefile.sources
> > > > > > +++ b/tests/Makefile.sources
> > > > > > @@ -40,7 +40,6 @@ TESTS_progs = \
> > > > > >       kms_dp_dsc \
> > > > > >       kms_draw_crc \
> > > > > >       kms_fbcon_fbt \
> > > > > > -     kms_fence_pin_leak \
> > > > > >       kms_flip \
> > > > > >       kms_flip_event_leak \
> > > > > >       kms_flip_tiling \
> > > > > > @@ -99,6 +98,10 @@ TESTS_progs = \
> > > > > >       vgem_slow \
> > > > > >       $(NULL)
> > > > > >  
> > > > > > +TESTS_progs += \
> > > > > > +     i915/kms_fence_pin_leak \
> > > > > > +     $(NULL)
> > > > > 
> > > > > This just moves it around, so we will end up having binary named
> > > > > 'kms_fence_pin_leak' but in $SRC/tests/i915 dir instead.
> > > > > 
> > > > > That still will install as
> > > > > $PREFIX/libexec/igt-gpu-tools/kms_fence_pin_leak
> > > > > 
> > > > > If you want to prefix it:
> > > > >  TESTS_progs += i915_kms_fence_pin_leak
> > > > >  i915_kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > > 
> > > > > Oterwise:
> > > > >  TESTS_progs += kms_fence_pin_leak
> > > > >  kms_fence_pin_leak_SOURCES = i915/kms_fence_pin_leak.c
> > > > > 
> > > > > > diff --git a/tests/kms_fence_pin_leak.c b/tests/i915/kms_fence_pin_leak.c
> > > > > > similarity index 100%
> > > > > > rename from tests/kms_fence_pin_leak.c
> > > > > > rename to tests/i915/kms_fence_pin_leak.c
> > > > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > > > index ec980651a..08e55b9c0 100644
> > > > > > --- a/tests/meson.build
> > > > > > +++ b/tests/meson.build
> > > > > > @@ -27,7 +27,6 @@ test_progs = [
> > > > > >       'kms_dp_dsc',
> > > > > >       'kms_draw_crc',
> > > > > >       'kms_fbcon_fbt',
> > > > > > -     'kms_fence_pin_leak',
> > > > > >       'kms_flip',
> > > > > >       'kms_flip_event_leak',
> > > > > >       'kms_flip_tiling',
> > > > > > @@ -100,6 +99,7 @@ i915_progs = [
> > > > > >       'fb_tiling',
> > > > > >       'getparams_basic',
> > > > > >       'hangman',
> > > > > > +     'kms_fence_pin_leak',
> > > > > >       'missed_irq',
> > > > > >       'module_load',
> > > > > >       'query',
> > > > > 
> > > > > Here, with meson, it will get prefixed with i915_. I'll add a comment on
> > > > > top of i915_progs just to be more explicit.
> > > > > 
> > > > > Do we have any conclusion on prefixing Intel-specific kms tests
> > > > > yet?
> > > > 
> > > > I'd rather not have tests renamed. That's my personal preference. Either
> > > > it is tests/i915/i915_kms_fence_pin_leak (but installed under tests/!)
> > > > or it should be installed under tests/i915/.
> > 
> > You mean you want the binaries to be a straight s/\.c//?
> 
> Yes. Or that we don't report test names at all, but the path to the
> source of the test.
>  
> > I personally like the current way mostly because we avoid 'i915/i915_'
> > redundancy and the resulting binaries dir is flat, which makes them
> > PATH-friendly.
> 
> They should not be in the PATH!!!!
> 
> There doesn't need to be any i915/i915_ redundancy as we then have i915/
> in the name.

That's why its libexec after installing, but I admit to committing the
crime of adding $SRC/build/tests to PATH at times, for convenience when
working on tests.

I think that we are pretty far from being able to handle
igt@tata/toto@subtoto in the CI (quotation needed, Petri?)
and i915/i915_ would be overall less messy of the two.

> > The way we handle gem_ adds a little inconsistency, but i915_gem_ would
> > be too mouthful.
> > 
> > But if what you are proposing is really essential we can stir the pot
> > some more and i915_ all the .c(s).
> > 
> > > As a case in point, the renaming of benchmarks by meson breaks
> > > scripts. Please do not do that.
> > 
> > I lack the context here. Do we have any inconsistencies in how autotools
> > and meson generate binaries? Which scripts are getting broken?
> 
> Yes, meson adds _bench but all of our scripts in igt expect the
> existing names. That may only be 15 different scripts...
> -Chris

Oh. Let me check what we can do about it. I'll make it consisten one way or
the other :-)

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/8] i915/gem_eio: Check that context create fails when wedged

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/8] i915/gem_eio: Check that context create fails when wedged
URL   : https://patchwork.freedesktop.org/series/56807/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5616 -> IGTPW_2433
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56807/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2433 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@i915_selftest@live_execlists:
    - {fi-icl-y}:         INCOMPLETE [fdo#109567] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109567]: https://bugs.freedesktop.org/show_bug.cgi?id=109567


Participating hosts (40 -> 36)
------------------------------

  Additional (2): fi-glk-j4005 fi-pnv-d510 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-whl-u 


Build changes
-------------

    * IGT: IGT_4833 -> IGTPW_2433

  CI_DRM_5616: 3479bf1d93b4e556be2804ff39ad41293fc48e4f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2433: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2433/
  IGT_4833: 7802324e86ddf947cba847e910f75b1a8affe8d7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_create@create-clear
+igt@gem_eio@context-create
+igt@gem_exec_big@exhaustive
+igt@gem_exec_big@single
+igt@i915_kms_fence_pin_leak
-igt@gem_exec_big
-igt@kms_fence_pin_leak

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2433/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.IGT: failure for series starting with [i-g-t,1/8] i915/gem_eio: Check that context create fails when wedged

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/8] i915/gem_eio: Check that context create fails when wedged
URL   : https://patchwork.freedesktop.org/series/56807/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5616_full -> IGTPW_2433_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2433_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2433_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56807/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_2433_full:

### IGT changes ###

#### Possible regressions ####

  * {igt@gem_eio@context-create} (NEW):
    - shard-hsw:          NOTRUN -> FAIL
    - shard-glk:          NOTRUN -> FAIL
    - shard-apl:          NOTRUN -> FAIL
    - shard-snb:          NOTRUN -> FAIL

  * {igt@gem_exec_big@single} (NEW):
    - shard-kbl:          NOTRUN -> FAIL +1

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking:
    - shard-apl:          PASS -> FAIL

  
New tests
---------

  New tests have been introduced between CI_DRM_5616_full and IGTPW_2433_full:

### New IGT tests (4) ###

  * igt@gem_create@create-clear:
    - Statuses : 4 pass(s)
    - Exec time: [35.11, 39.24] s

  * igt@gem_eio@context-create:
    - Statuses : 5 fail(s)
    - Exec time: [0.03, 0.30] s

  * igt@gem_exec_big@single:
    - Statuses : 1 fail(s) 1 incomplete(s) 3 pass(s)
    - Exec time: [0.0, 223.04] s

  * igt@i915_kms_fence_pin_leak:
    - Statuses : 5 pass(s)
    - Exec time: [1.28, 1.64] s

  

Known issues
------------

  Here are the changes found in IGTPW_2433_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * {igt@gem_exec_big@single} (NEW):
    - shard-glk:          NOTRUN -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@gem_exec_schedule@preempt-other-chain-bsd:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-apl:          PASS -> FAIL [fdo#103232] +4

  * igt@kms_flip@2x-flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          PASS -> FAIL [fdo#103167] +3

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166] +2
    - shard-apl:          PASS -> FAIL [fdo#103166]
    - shard-kbl:          PASS -> FAIL [fdo#103166]

  * igt@kms_vblank@pipe-a-ts-continuation-modeset-rpm:
    - shard-apl:          PASS -> FAIL [fdo#104894]
    - shard-kbl:          PASS -> FAIL [fdo#104894]

  
#### Possible fixes ####

  * igt@gem_exec_schedule@out-order-blt:
    - shard-glk:          FAIL -> PASS

  * igt@gem_linear_blits@normal:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-apl:          FAIL -> PASS
    - shard-kbl:          FAIL -> PASS

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +6

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          FAIL [fdo#102887] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS +6

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-kbl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-mmap-gtt:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_plane@pixel-format-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS +4

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1
    - shard-kbl:          FAIL [fdo#103166] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * igt@kms_vblank@pipe-b-ts-continuation-modeset-hang:
    - shard-apl:          FAIL [fdo#104894] -> PASS

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-snb:          {SKIP} [fdo#109271] -> PASS

  * igt@sw_sync@sync_busy_fork:
    - shard-glk:          INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@tools_test@tools_test:
    - shard-apl:          {SKIP} [fdo#109271] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4833 -> IGTPW_2433
    * Piglit: piglit_4509 -> None

  CI_DRM_5616: 3479bf1d93b4e556be2804ff39ad41293fc48e4f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2433: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2433/
  IGT_4833: 7802324e86ddf947cba847e910f75b1a8affe8d7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2433/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 1/8] i915/gem_eio: Check that context create fails when wedged

[Antonio Argenziano]

On 17/02/19 06:35, Chris Wilson wrote:
> Lock down the new uABI that DRM_IOCTL_I915_GEM_CONTEXT_CREATE returns
> -EIO when wedged.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

LGTM.

Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

> ---
>   tests/i915/gem_eio.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index ac85a2eff..3c54820c9 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -118,6 +118,17 @@ static void test_throttle(int fd)
>   	trigger_reset(fd);
>   }
>   
> +static void test_context_create(int fd)
> +{
> +	uint32_t ctx;
> +
> +	wedge_gpu(fd);
> +
> +	igt_assert_eq(__gem_context_create(fd, &ctx), -EIO);
> +
> +	trigger_reset(fd);
> +}
> +
>   static void test_execbuf(int fd)
>   {
>   	struct drm_i915_gem_execbuffer2 execbuf;
> @@ -807,6 +818,9 @@ igt_main
>   	igt_subtest("throttle")
>   		test_throttle(fd);
>   
> +	igt_subtest("context-create")
> +		test_context_create(fd);
> +
>   	igt_subtest("execbuf")
>   		test_execbuf(fd);
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t 1/8] i915/gem_eio: Check that context create fails when wedged

[Antonio Argenziano]

On 17/02/19 06:35, Chris Wilson wrote:
> Lock down the new uABI that DRM_IOCTL_I915_GEM_CONTEXT_CREATE returns
> -EIO when wedged.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>

LGTM.

Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com>

> ---
>   tests/i915/gem_eio.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
> index ac85a2eff..3c54820c9 100644
> --- a/tests/i915/gem_eio.c
> +++ b/tests/i915/gem_eio.c
> @@ -118,6 +118,17 @@ static void test_throttle(int fd)
>   	trigger_reset(fd);
>   }
>   
> +static void test_context_create(int fd)
> +{
> +	uint32_t ctx;
> +
> +	wedge_gpu(fd);
> +
> +	igt_assert_eq(__gem_context_create(fd, &ctx), -EIO);
> +
> +	trigger_reset(fd);
> +}
> +
>   static void test_execbuf(int fd)
>   {
>   	struct drm_i915_gem_execbuffer2 execbuf;
> @@ -807,6 +818,9 @@ igt_main
>   	igt_subtest("throttle")
>   		test_throttle(fd);
>   
> +	igt_subtest("context-create")
> +		test_context_create(fd);
> +
>   	igt_subtest("execbuf")
>   		test_execbuf(fd);
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✗ Fi.CI.BAT: failure for series starting with [i-g-t,1/8] i915/gem_eio: Check that context create fails when wedged (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/8] i915/gem_eio: Check that context create fails when wedged (rev2)
URL   : https://patchwork.freedesktop.org/series/56807/
State : failure

== Summary ==

Applying: i915/gem_eio: Check that context create fails when wedged
Applying: i915/gem_eio: Check we only ban the context
Applying: lib: Restore the i915.reset modparam before cleaning up
Applying: i915/gem_create: Verify that all new objects are clear
Using index info to reconstruct a base tree...
M	tests/i915/gem_create.c
Falling back to patching base and 3-way merge...
Auto-merging tests/i915/gem_create.c
CONFLICT (content): Merge conflict in tests/i915/gem_create.c
Patch failed at 0004 i915/gem_create: Verify that all new objects are clear
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev