[igt-dev] [PATCH i-g-t v4] tests/i915/gem_exec_async: Update with engine discovery

[igt-dev] [PATCH i-g-t v4] tests/i915/gem_exec_async: Update with engine discovery

[Ramalingam C]

Legacy execbuf abi tests are prefixed with legacy. New test are added to
run on physical engines accessed through engine discovery.

So legacy tests run on the unconfigured (with engine map) context and
use a new helper gem_eb_flags_to_engine to look up the engine from the
intel_execution_engines2 static list. This is only to enable the core
test code to be shared.

Places where new contexts are created had to be updated to either
equally configure the contexts or not.

v2:
  retained the test as it is for legacy uapi testing and duplciated for
	new engine discovery [Tvrtko]
v3:
  Few nits addressed [Tvrtko]
v4:
  In legacy uAPI test path, iterate through for_each_engine [Tvrtko]

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 tests/i915/gem_exec_async.c | 52 +++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 14 deletions(-)

diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
index 9a06af7e29cb..93cfc3d28541 100644
--- a/tests/i915/gem_exec_async.c
+++ b/tests/i915/gem_exec_async.c
@@ -80,9 +80,10 @@ static void store_dword(int fd, unsigned ring,
 	gem_close(fd, obj[1].handle);
 }
 
-static void one(int fd, unsigned ring, uint32_t flags)
+static void one(int fd, const struct intel_execution_engine2 *e2, bool legacy)
 {
 	const int gen = intel_gen(intel_get_drm_devid(fd));
+	const struct intel_execution_engine2 *other2;
 	struct drm_i915_gem_exec_object2 obj[2];
 #define SCRATCH 0
 #define BATCH 1
@@ -138,20 +139,33 @@ static void one(int fd, unsigned ring, uint32_t flags)
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffers_ptr = to_user_pointer(obj);
 	execbuf.buffer_count = 2;
-	execbuf.flags = ring | flags;
+	execbuf.flags = e2->flags;
 	igt_require(__gem_execbuf(fd, &execbuf) == 0);
 	gem_close(fd, obj[BATCH].handle);
 
 	i = 0;
-	for_each_physical_engine(fd, other) {
-		if (other == ring)
-			continue;
+	if (legacy) {
+		for_each_engine(fd, other) {
+			if (e2->flags == other)
+				continue;
 
-		if (!gem_can_store_dword(fd, other))
-			continue;
+			if (!gem_can_store_dword(fd, other))
+				continue;
+
+			store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
+			i++;
+		}
+	} else {
+		__for_each_physical_engine(fd, other2) {
+			if (gem_engine_is_equal(e2, other2))
+				continue;
 
-		store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
-		i++;
+			if (!gem_class_can_store_dword(fd, other2->class))
+				continue;
+
+			store_dword(fd, other2->flags, obj[SCRATCH].handle, 4*i, i);
+			i++;
+		}
 	}
 
 	*batch = MI_BATCH_BUFFER_END;
@@ -185,7 +199,9 @@ static bool has_async_execbuf(int fd)
 
 igt_main
 {
+	const struct intel_execution_engine2 *e2;
 	const struct intel_execution_engine *e;
+	struct intel_execution_engine2 e2__;
 	int fd = -1;
 
 	igt_skip_on_simulation();
@@ -200,14 +216,22 @@ igt_main
 	}
 
 	for (e = intel_execution_engines; e->name; e++) {
-		/* default exec-id is purely symbolic */
-		if (e->exec_id == 0)
+		e2__ = gem_eb_flags_to_engine(e->exec_id | e->flags);
+		if (e2__.flags == -1)
 			continue;
+		e2 = &e2__;
 
-		igt_subtest_f("concurrent-writes-%s", e->name) {
+		igt_subtest_f("legacy-concurrent-writes-%s", e2->name) {
 			igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags));
-			igt_require(gem_can_store_dword(fd, e->exec_id | e->flags));
-			one(fd, e->exec_id, e->flags);
+			igt_require(gem_class_can_store_dword(fd, e2->class));
+			one(fd, e2, true);
+		}
+	}
+
+	__for_each_physical_engine(fd, e2) {
+		igt_subtest_f("concurrent-writes-%s", e2->name) {
+			igt_require(gem_class_can_store_dword(fd, e2->class));
+			one(fd, e2, false);
 		}
 	}
 
-- 
2.19.1

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

[igt-dev] ✗ Fi.CI.BAT: failure for tests/i915/gem_exec_async: Update with engine discovery (rev2)

[Patchwork]

== Series Details ==

Series: tests/i915/gem_exec_async: Update with engine discovery (rev2)
URL   : https://patchwork.freedesktop.org/series/64162/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6552 -> IGTPW_3295
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_3295 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_3295, 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/64162/revisions/2/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ctx_create@basic-files:
    - fi-skl-gvtdvm:      [PASS][1] -> [TIMEOUT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6552/fi-skl-gvtdvm/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3295/fi-skl-gvtdvm/igt@gem_ctx_create@basic-files.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@read_all_entries:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6552/fi-icl-u3/igt@debugfs_test@read_all_entries.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3295/fi-icl-u3/igt@debugfs_test@read_all_entries.html

  
#### Possible fixes ####

  * igt@gem_exec_reloc@basic-write-gtt:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6552/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3295/fi-icl-u3/igt@gem_exec_reloc@basic-write-gtt.html

  
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724


Participating hosts (55 -> 42)
------------------------------

  Missing    (13): fi-kbl-soraka fi-kbl-7567u fi-ilk-m540 fi-hsw-4200u fi-bdw-gvtdvm fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-ctg-p8600 fi-icl-y fi-icl-guc fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5112 -> IGTPW_3295

  CI-20190529: 20190529
  CI_DRM_6552: f9b5c6777cb0082fb7461ea8a57206b8de2d70dd @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3295: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3295/
  IGT_5112: 7e4d10507088055413769a020dd674f52b4bc1b0 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@gem_exec_async@concurrent-writes-bcs0
+igt@gem_exec_async@concurrent-writes-rcs0
+igt@gem_exec_async@concurrent-writes-vcs0
+igt@gem_exec_async@concurrent-writes-vcs1
+igt@gem_exec_async@concurrent-writes-vcs2
+igt@gem_exec_async@concurrent-writes-vecs0
+igt@gem_exec_async@legacy-concurrent-writes-bcs0
+igt@gem_exec_async@legacy-concurrent-writes-default
+igt@gem_exec_async@legacy-concurrent-writes-rcs0
+igt@gem_exec_async@legacy-concurrent-writes-vcs0
+igt@gem_exec_async@legacy-concurrent-writes-vcs1
+igt@gem_exec_async@legacy-concurrent-writes-vecs0
-igt@gem_exec_async@concurrent-writes-blt
-igt@gem_exec_async@concurrent-writes-bsd
-igt@gem_exec_async@concurrent-writes-bsd1
-igt@gem_exec_async@concurrent-writes-bsd2
-igt@gem_exec_async@concurrent-writes-render
-igt@gem_exec_async@concurrent-writes-vebox

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t v4] tests/i915/gem_exec_async: Update with engine discovery

[Ramalingam C]

On 2019-07-29 at 15:26:54 +0100, Tvrtko Ursulin wrote:
> 
> On 25/07/2019 09:45, Ramalingam C wrote:
> > Legacy execbuf abi tests are prefixed with legacy. New test are added to
> > run on physical engines accessed through engine discovery.
> > 
> > So legacy tests run on the unconfigured (with engine map) context and
> > use a new helper gem_eb_flags_to_engine to look up the engine from the
> > intel_execution_engines2 static list. This is only to enable the core
> > test code to be shared.
> > 
> > Places where new contexts are created had to be updated to either
> > equally configure the contexts or not.
> > 
> > v2:
> >    retained the test as it is for legacy uapi testing and duplciated for
> > 	new engine discovery [Tvrtko]
> > v3:
> >    Few nits addressed [Tvrtko]
> > v4:
> >    In legacy uAPI test path, iterate through for_each_engine [Tvrtko]
> > 
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >   tests/i915/gem_exec_async.c | 52 +++++++++++++++++++++++++++----------
> >   1 file changed, 38 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
> > index 9a06af7e29cb..93cfc3d28541 100644
> > --- a/tests/i915/gem_exec_async.c
> > +++ b/tests/i915/gem_exec_async.c
> > @@ -80,9 +80,10 @@ static void store_dword(int fd, unsigned ring,
> >   	gem_close(fd, obj[1].handle);
> >   }
> > -static void one(int fd, unsigned ring, uint32_t flags)
> > +static void one(int fd, const struct intel_execution_engine2 *e2, bool legacy)
> >   {
> >   	const int gen = intel_gen(intel_get_drm_devid(fd));
> > +	const struct intel_execution_engine2 *other2;
> >   	struct drm_i915_gem_exec_object2 obj[2];
> >   #define SCRATCH 0
> >   #define BATCH 1
> > @@ -138,20 +139,33 @@ static void one(int fd, unsigned ring, uint32_t flags)
> >   	memset(&execbuf, 0, sizeof(execbuf));
> >   	execbuf.buffers_ptr = to_user_pointer(obj);
> >   	execbuf.buffer_count = 2;
> > -	execbuf.flags = ring | flags;
> > +	execbuf.flags = e2->flags;
> >   	igt_require(__gem_execbuf(fd, &execbuf) == 0);
> >   	gem_close(fd, obj[BATCH].handle);
> >   	i = 0;
> > -	for_each_physical_engine(fd, other) {
> > -		if (other == ring)
> > -			continue;
> > +	if (legacy) {
> > +		for_each_engine(fd, other) {
> > +			if (e2->flags == other)
> > +				continue;
> 
> Have you checked if we get I915_EXEC_DEFAULT and I915_EXEC_RENDER here, and
> what happens in that case, and what happens with I915_EXEC_BSD and
> I915_EXEC_BSD1/2?
Sorry I remember you mentioned to fix this in previous version. I guess
below implementation will help in this regard

bool gem_legacy_engine_is_same(unsigned eb_flag1, unsigned eb_flag2)
{
        if ((eb_flag1 == I915_EXEC_DEFAULT && eb_flag2 == I915_EXEC_RENDER) ||
             (eb_flag1 == I915_EXEC_RENDER && eb_flag2 == I915_EXEC_DEFAULT))
                return true;

        if (eb_flag1 & ~(3<<13) == eb_flag2 & ~(3<<13))
                return true;

        if (eb_flag1 == eb_flag2)
                return true;

        return false;
}

or in the above function we could change the eb_flag as below

if (eb_flag1 == I915_EXEC_DEFAULT)
	eb_flag1 = I915_EXEC_RENDER;

sounds good?

-Ram
> 
> Regards,
> 
> Tvrtko
> 
> > -		if (!gem_can_store_dword(fd, other))
> > -			continue;
> > +			if (!gem_can_store_dword(fd, other))
> > +				continue;
> > +
> > +			store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
> > +			i++;
> > +		}
> > +	} else {
> > +		__for_each_physical_engine(fd, other2) {
> > +			if (gem_engine_is_equal(e2, other2))
> > +				continue;
> > -		store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
> > -		i++;
> > +			if (!gem_class_can_store_dword(fd, other2->class))
> > +				continue;
> > +
> > +			store_dword(fd, other2->flags, obj[SCRATCH].handle, 4*i, i);
> > +			i++;
> > +		}
> >   	}
> >   	*batch = MI_BATCH_BUFFER_END;
> > @@ -185,7 +199,9 @@ static bool has_async_execbuf(int fd)
> >   igt_main
> >   {
> > +	const struct intel_execution_engine2 *e2;
> >   	const struct intel_execution_engine *e;
> > +	struct intel_execution_engine2 e2__;
> >   	int fd = -1;
> >   	igt_skip_on_simulation();
> > @@ -200,14 +216,22 @@ igt_main
> >   	}
> >   	for (e = intel_execution_engines; e->name; e++) {
> > -		/* default exec-id is purely symbolic */
> > -		if (e->exec_id == 0)
> > +		e2__ = gem_eb_flags_to_engine(e->exec_id | e->flags);
> > +		if (e2__.flags == -1)
> >   			continue;
> > +		e2 = &e2__;
> > -		igt_subtest_f("concurrent-writes-%s", e->name) {
> > +		igt_subtest_f("legacy-concurrent-writes-%s", e2->name) {
> >   			igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags));
> > -			igt_require(gem_can_store_dword(fd, e->exec_id | e->flags));
> > -			one(fd, e->exec_id, e->flags);
> > +			igt_require(gem_class_can_store_dword(fd, e2->class));
> > +			one(fd, e2, true);
> > +		}
> > +	}
> > +
> > +	__for_each_physical_engine(fd, e2) {
> > +		igt_subtest_f("concurrent-writes-%s", e2->name) {
> > +			igt_require(gem_class_can_store_dword(fd, e2->class));
> > +			one(fd, e2, false);
> >   		}
> >   	}
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v4] tests/i915/gem_exec_async: Update with engine discovery

[Tvrtko Ursulin]

On 25/07/2019 09:45, Ramalingam C wrote:
> Legacy execbuf abi tests are prefixed with legacy. New test are added to
> run on physical engines accessed through engine discovery.
> 
> So legacy tests run on the unconfigured (with engine map) context and
> use a new helper gem_eb_flags_to_engine to look up the engine from the
> intel_execution_engines2 static list. This is only to enable the core
> test code to be shared.
> 
> Places where new contexts are created had to be updated to either
> equally configure the contexts or not.
> 
> v2:
>    retained the test as it is for legacy uapi testing and duplciated for
> 	new engine discovery [Tvrtko]
> v3:
>    Few nits addressed [Tvrtko]
> v4:
>    In legacy uAPI test path, iterate through for_each_engine [Tvrtko]
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>   tests/i915/gem_exec_async.c | 52 +++++++++++++++++++++++++++----------
>   1 file changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_async.c b/tests/i915/gem_exec_async.c
> index 9a06af7e29cb..93cfc3d28541 100644
> --- a/tests/i915/gem_exec_async.c
> +++ b/tests/i915/gem_exec_async.c
> @@ -80,9 +80,10 @@ static void store_dword(int fd, unsigned ring,
>   	gem_close(fd, obj[1].handle);
>   }
>   
> -static void one(int fd, unsigned ring, uint32_t flags)
> +static void one(int fd, const struct intel_execution_engine2 *e2, bool legacy)
>   {
>   	const int gen = intel_gen(intel_get_drm_devid(fd));
> +	const struct intel_execution_engine2 *other2;
>   	struct drm_i915_gem_exec_object2 obj[2];
>   #define SCRATCH 0
>   #define BATCH 1
> @@ -138,20 +139,33 @@ static void one(int fd, unsigned ring, uint32_t flags)
>   	memset(&execbuf, 0, sizeof(execbuf));
>   	execbuf.buffers_ptr = to_user_pointer(obj);
>   	execbuf.buffer_count = 2;
> -	execbuf.flags = ring | flags;
> +	execbuf.flags = e2->flags;
>   	igt_require(__gem_execbuf(fd, &execbuf) == 0);
>   	gem_close(fd, obj[BATCH].handle);
>   
>   	i = 0;
> -	for_each_physical_engine(fd, other) {
> -		if (other == ring)
> -			continue;
> +	if (legacy) {
> +		for_each_engine(fd, other) {
> +			if (e2->flags == other)
> +				continue;

Have you checked if we get I915_EXEC_DEFAULT and I915_EXEC_RENDER here, 
and what happens in that case, and what happens with I915_EXEC_BSD and 
I915_EXEC_BSD1/2?

Regards,

Tvrtko

>   
> -		if (!gem_can_store_dword(fd, other))
> -			continue;
> +			if (!gem_can_store_dword(fd, other))
> +				continue;
> +
> +			store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
> +			i++;
> +		}
> +	} else {
> +		__for_each_physical_engine(fd, other2) {
> +			if (gem_engine_is_equal(e2, other2))
> +				continue;
>   
> -		store_dword(fd, other, obj[SCRATCH].handle, 4*i, i);
> -		i++;
> +			if (!gem_class_can_store_dword(fd, other2->class))
> +				continue;
> +
> +			store_dword(fd, other2->flags, obj[SCRATCH].handle, 4*i, i);
> +			i++;
> +		}
>   	}
>   
>   	*batch = MI_BATCH_BUFFER_END;
> @@ -185,7 +199,9 @@ static bool has_async_execbuf(int fd)
>   
>   igt_main
>   {
> +	const struct intel_execution_engine2 *e2;
>   	const struct intel_execution_engine *e;
> +	struct intel_execution_engine2 e2__;
>   	int fd = -1;
>   
>   	igt_skip_on_simulation();
> @@ -200,14 +216,22 @@ igt_main
>   	}
>   
>   	for (e = intel_execution_engines; e->name; e++) {
> -		/* default exec-id is purely symbolic */
> -		if (e->exec_id == 0)
> +		e2__ = gem_eb_flags_to_engine(e->exec_id | e->flags);
> +		if (e2__.flags == -1)
>   			continue;
> +		e2 = &e2__;
>   
> -		igt_subtest_f("concurrent-writes-%s", e->name) {
> +		igt_subtest_f("legacy-concurrent-writes-%s", e2->name) {
>   			igt_require(gem_ring_has_physical_engine(fd, e->exec_id | e->flags));
> -			igt_require(gem_can_store_dword(fd, e->exec_id | e->flags));
> -			one(fd, e->exec_id, e->flags);
> +			igt_require(gem_class_can_store_dword(fd, e2->class));
> +			one(fd, e2, true);
> +		}
> +	}
> +
> +	__for_each_physical_engine(fd, e2) {
> +		igt_subtest_f("concurrent-writes-%s", e2->name) {
> +			igt_require(gem_class_can_store_dword(fd, e2->class));
> +			one(fd, e2, false);
>   		}
>   	}
>   
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev