Skip to contentSkip to conversation

tests/perf_pmu: Fix usage of for_each_engine_class_instance

PATCH In review Tvrtko Ursulin 4 participants 5 messages Mar 29, 2018 Library code

lore
Share & tools
lore

Tvrtko Ursulin

author
#0 lore raw

PATCH 1/1 tests/perf_pmu: Fix usage of for_each_engine_class_instance

From: Tvrtko Ursulin <[email protected]>

Wrong file descriptor was passed to the iterator. This had currently no effect, since it wasn't used in the macro, but needs to be fixed.

At the same time make the macro consistent by checking for engine presence like the other iterators do.

Added __for_each_engine_class_instance which does not check for engine presence and so is useful for enumerating all possible engines - like for instance for subtest enumeration.

And another 'wrong fd used' fixlet in the render node subtests.

Signed-off-by Tvrtko Ursulin <[email protected]>
Reported-by Michel Thierry <[email protected]>
Cc Michel Thierry <[email protected]>
---
 lib/igt_gt.h     | 12 +++++++-----
 tests/perf_pmu.c | 30 ++++++++++--------------------
 2 files changed, 17 insertions(+), 25 deletions(-)
lib/igt_gt.h +7 -5
index a517ed7b29a0..d44b7552f3c4 100644
@@ -100,11 +100,6 @@ extern const struct intel_execution_engine2 {
 	int instance;
 } intel_execution_engines2[];
 
-#define for_each_engine_class_instance(fd__, e__) \
-	for ((e__) = intel_execution_engines2;\
-	     (e__)->name; \
-	     (e__)++)
-
 unsigned int
 gem_class_instance_to_eb_flags(int gem_fd,
 			       enum drm_i915_gem_engine_class class,

@@ -122,4 +117,11 @@ void gem_require_engine(int gem_fd,
 	igt_require(gem_has_engine(gem_fd, class, instance));
 }
 
+#define __for_each_engine_class_instance(fd__, e__) \
+	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++)
+
+#define for_each_engine_class_instance(fd__, e__) \
+	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \
+		for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance))
+
 #endif /* IGT_GT_H */
tests/perf_pmu.c +10 -20
index b59af81819c7..2273ddb9e684 100644
@@ -434,10 +434,8 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
 
 	i = 0;
 	fd[0] = -1;
-	for_each_engine_class_instance(fd, e_) {
-		if (!gem_has_engine(gem_fd, e_->class, e_->instance))
-			continue;
-		else if (e == e_)
+	for_each_engine_class_instance(gem_fd, e_) {
+		if (e == e_)
 			busy_idx = i;
 
 		fd[i++] = open_group(I915_PMU_ENGINE_BUSY(e_->class,

@@ -499,10 +497,7 @@ most_busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
 	unsigned int idle_idx, i;
 
 	i = 0;
-	for_each_engine_class_instance(fd, e_) {
-		if (!gem_has_engine(gem_fd, e_->class, e_->instance))
-			continue;
-
+	for_each_engine_class_instance(gem_fd, e_) {
 		if (e == e_)
 			idle_idx = i;
 		else if (spin)

@@ -559,10 +554,7 @@ all_busy_check_all(int gem_fd, const unsigned int num_engines,
 	unsigned int i;
 
 	i = 0;
-	for_each_engine_class_instance(fd, e) {
-		if (!gem_has_engine(gem_fd, e->class, e->instance))
-			continue;
-
+	for_each_engine_class_instance(gem_fd, e) {
 		if (spin)
 			__submit_spin_batch(gem_fd, spin, e, 64);
 		else

@@ -1677,10 +1669,8 @@ igt_main
 		igt_require_gem(fd);
 		igt_require(i915_type_id() > 0);
 
-		for_each_engine_class_instance(fd, e) {
-			if (gem_has_engine(fd, e->class, e->instance))
-				num_engines++;
-		}
+		for_each_engine_class_instance(fd, e)
+			num_engines++;
 	}
 
 	/**

@@ -1689,7 +1679,7 @@ igt_main
 	igt_subtest("invalid-init")
 		invalid_init();
 
-	for_each_engine_class_instance(fd, e) {
+	__for_each_engine_class_instance(fd, e) {
 		const unsigned int pct[] = { 2, 50, 98 };
 
 		/**

@@ -1888,7 +1878,7 @@ igt_main
 			gem_quiescent_gpu(fd);
 		}
 
-		for_each_engine_class_instance(fd, e) {
+		__for_each_engine_class_instance(render_fd, e) {
 			igt_subtest_group {
 				igt_fixture {
 					gem_require_engine(render_fd,

@@ -1897,10 +1887,10 @@ igt_main
 				}
 
 				igt_subtest_f("render-node-busy-%s", e->name)
-					single(fd, e, TEST_BUSY);
+					single(render_fd, e, TEST_BUSY);
 				igt_subtest_f("render-node-busy-idle-%s",
 					      e->name)
-					single(fd, e,
+					single(render_fd, e,
 					       TEST_BUSY | TEST_TRAILING_IDLE);
 			}
 		}
Signature · 6 lines
2.14.1

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Reviewed-by on v1 1/1
#1 lore raw

Quoting Tvrtko Ursulin (2018-03-29 10:11:28)

Quoted 39 lines

From: Tvrtko Ursulin <[email protected]>

Wrong file descriptor was passed to the iterator. This had currently no effect, since it wasn't used in the macro, but needs to be fixed.

At the same time make the macro consistent by checking for engine presence like the other iterators do.

Added __for_each_engine_class_instance which does not check for engine presence and so is useful for enumerating all possible engines - like for instance for subtest enumeration.

And another 'wrong fd used' fixlet in the render node subtests.

Signed-off-by Tvrtko Ursulin <[email protected]>
Reported-by Michel Thierry <[email protected]>
Cc Michel Thierry <[email protected]>
---
 lib/igt_gt.h     | 12 +++++++-----
 tests/perf_pmu.c | 30 ++++++++++--------------------
 2 files changed, 17 insertions(+), 25 deletions(-)
lib/igt_gt.h +0 -5
index a517ed7b29a0..d44b7552f3c4 100644
@@ -100,11 +100,6 @@ extern const struct intel_execution_engine2 {
        int instance;
 } intel_execution_engines2[];
 
-#define for_each_engine_class_instance(fd__, e__) \
-       for ((e__) = intel_execution_engines2;\
-            (e__)->name; \
-            (e__)++)
-
 unsigned int
 gem_class_instance_to_eb_flags(int gem_fd,
                               enum drm_i915_gem_engine_class class,

lib/igt_gt.h void gem_require_engine(int gem_fd, · line 125

Quoted 9 lines
lib/igt_gt.h +6 -0
@@ -122,4 +117,11 @@ void gem_require_engine(int gem_fd,
        igt_require(gem_has_engine(gem_fd, class, instance));
 }
 
+#define __for_each_engine_class_instance(fd__, e__) \
+       for ((e__) = intel_execution_engines2; (e__)->name; (e__)++)
+
+#define for_each_engine_class_instance(fd__, e__) \
+       for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \
+               for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance))

Ok.

Conversions made sense.

Reviewed-by Chris Wilson <[email protected]>

-Chris
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

CI
#2 lore raw

[igt-dev] ✓ Fi.CI.BAT: success for tests/perf_pmu: Fix usage of for_each_engine_class_instance

In reply to Tvrtko Ursulin
== Series Details ==

== Series Details ==

Series: tests/perf_pmu: Fix usage of for_each_engine_class_instance
URL : https://patchwork.freedesktop.org/series/40867/
State : success

== Summary ==

IGT patchset tested on top of latest successful build 2cbd4ddf11b3eaf01f33d8bc2ad46411ec6c299a lib/igt_kms: Improve connector probing in igt_display_init(), v2.

with latest DRM-Tip kernel build CI_DRM_4007
d6e43ca115e5 drm-tip: 2018y-03m-29d-12h-46m-03s UTC integration manifest

No testlist changes.

---- Known issues:

Test kms_pipe_crc_basic:

        Subgroup read-crc-pipe-b-frame-sequence:
                fail       -> PASS       (fi-cfl-s3) fdo#103481

fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:438s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:443s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:381s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:541s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:296s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:514s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:528s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:506s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:566s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:514s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:427s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:320s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:541s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:404s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:477s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:437s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:474s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:463s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:508s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:666s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:444s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:542s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:501s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:515s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:434s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:447s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:406s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:255  dwarn:3   dfail:0   fail:1   skip:26  time:521s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:484s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1205/issues.html
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Reviewed-by on v1 1/1
#3 lore raw
In reply to Tvrtko Ursulin
Quoted 12 lines · Tvrtko Ursulin

From: Tvrtko Ursulin <[email protected]>

Wrong file descriptor was passed to the iterator. This had currently no effect, since it wasn't used in the macro, but needs to be fixed.

At the same time make the macro consistent by checking for engine presence like the other iterators do.

Added __for_each_engine_class_instance which does not check for engine presence and so is useful for enumerating all possible engines - like for instance for subtest enumeration.

Quoted 5 lines

And another 'wrong fd used' fixlet in the render node subtests.

Signed-off-by Tvrtko Ursulin <[email protected]>
Reported-by Michel Thierry <[email protected]>
Cc Michel Thierry <[email protected]>

Thanks,

Reviewed-by Michel Thierry <[email protected]>
Quoted 109 lines
---
  lib/igt_gt.h     | 12 +++++++-----
  tests/perf_pmu.c | 30 ++++++++++--------------------
  2 files changed, 17 insertions(+), 25 deletions(-)
lib/igt_gt.h +7 -5
index a517ed7b29a0..d44b7552f3c4 100644
@@ -100,11 +100,6 @@ extern const struct intel_execution_engine2 {
  	int instance;
  } intel_execution_engines2[];
 
-#define for_each_engine_class_instance(fd__, e__) \
-	for ((e__) = intel_execution_engines2;\
-	     (e__)->name; \
-	     (e__)++)
-
  unsigned int
  gem_class_instance_to_eb_flags(int gem_fd,
  			       enum drm_i915_gem_engine_class class,

@@ -122,4 +117,11 @@ void gem_require_engine(int gem_fd,
  	igt_require(gem_has_engine(gem_fd, class, instance));
  }
 
+#define __for_each_engine_class_instance(fd__, e__) \
+	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++)
+
+#define for_each_engine_class_instance(fd__, e__) \
+	for ((e__) = intel_execution_engines2; (e__)->name; (e__)++) \
+		for_if (gem_has_engine((fd__), (e__)->class, (e__)->instance))
+
  #endif /* IGT_GT_H */
tests/perf_pmu.c +8 -18
index b59af81819c7..2273ddb9e684 100644
@@ -434,10 +434,8 @@ busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
 
  	i = 0;
  	fd[0] = -1;
-	for_each_engine_class_instance(fd, e_) {
-		if (!gem_has_engine(gem_fd, e_->class, e_->instance))
-			continue;
-		else if (e == e_)
+	for_each_engine_class_instance(gem_fd, e_) {
+		if (e == e_)
  			busy_idx = i;
 
  		fd[i++] = open_group(I915_PMU_ENGINE_BUSY(e_->class,

@@ -499,10 +497,7 @@ most_busy_check_all(int gem_fd, const struct intel_execution_engine2 *e,
  	unsigned int idle_idx, i;
 
  	i = 0;
-	for_each_engine_class_instance(fd, e_) {
-		if (!gem_has_engine(gem_fd, e_->class, e_->instance))
-			continue;
-
+	for_each_engine_class_instance(gem_fd, e_) {
  		if (e == e_)
  			idle_idx = i;
  		else if (spin)

@@ -559,10 +554,7 @@ all_busy_check_all(int gem_fd, const unsigned int num_engines,
  	unsigned int i;
 
  	i = 0;
-	for_each_engine_class_instance(fd, e) {
-		if (!gem_has_engine(gem_fd, e->class, e->instance))
-			continue;
-
+	for_each_engine_class_instance(gem_fd, e) {
  		if (spin)
  			__submit_spin_batch(gem_fd, spin, e, 64);
  		else

@@ -1677,10 +1669,8 @@ igt_main
  		igt_require_gem(fd);
  		igt_require(i915_type_id() > 0);
 
-		for_each_engine_class_instance(fd, e) {
-			if (gem_has_engine(fd, e->class, e->instance))
-				num_engines++;
-		}
+		for_each_engine_class_instance(fd, e)
+			num_engines++;
  	}
 
  	/**

@@ -1689,7 +1679,7 @@ igt_main
  	igt_subtest("invalid-init")
  		invalid_init();
 
-	for_each_engine_class_instance(fd, e) {
+	__for_each_engine_class_instance(fd, e) {
  		const unsigned int pct[] = { 2, 50, 98 };
 
  		/**

@@ -1888,7 +1878,7 @@ igt_main
  			gem_quiescent_gpu(fd);
  		}
 
-		for_each_engine_class_instance(fd, e) {
+		__for_each_engine_class_instance(render_fd, e) {
  			igt_subtest_group {
  				igt_fixture {
  					gem_require_engine(render_fd,

@@ -1897,10 +1887,10 @@ igt_main
  				}

tests/perf_pmu.c igt_main · line 1896

Quoted 10 lines
tests/perf_pmu.c +2 -2
  				igt_subtest_f("render-node-busy-%s", e->name)
-					single(fd, e, TEST_BUSY);
+					single(render_fd, e, TEST_BUSY);
  				igt_subtest_f("render-node-busy-idle-%s",
  					      e->name)
-					single(fd, e,
+					single(render_fd, e,
  					       TEST_BUSY | TEST_TRAILING_IDLE);
  			}
  		}

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

CI
#4 lore raw

[igt-dev] ✗ Fi.CI.IGT: failure for tests/perf_pmu: Fix usage of for_each_engine_class_instance

In reply to Tvrtko Ursulin
== Series Details ==

== Series Details ==

Series: tests/perf_pmu: Fix usage of for_each_engine_class_instance
URL : https://patchwork.freedesktop.org/series/40867/
State : failure

== Summary ==

---- Possible new issues:

Test gem_ringfill:

        Subgroup basic-default-hang:
                pass       -> FAIL       (shard-apl)

---- Known issues:

Test kms_flip:

        Subgroup 2x-dpms-vs-vblank-race:
                pass       -> FAIL       (shard-hsw) fdo#103060
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (shard-hsw) fdo#103375
Test kms_rotation_crc:
        Subgroup sprite-rotation-180:
                fail       -> PASS       (shard-snb) fdo#103925
Test kms_sysfs_edid_timing:
                warn       -> PASS       (shard-apl) fdo#100047

fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3495 pass:1831 dwarn:1   dfail:0   fail:8   skip:1655 time:12854s
shard-hsw        total:3495 pass:1782 dwarn:1   dfail:0   fail:2   skip:1709 time:11510s
shard-snb        total:3495 pass:1375 dwarn:1   dfail:0   fail:2   skip:2117 time:7035s

Blacklisted hosts:
shard-kbl total:3495 pass:1957 dwarn:1 dfail:1 fail:7 skip:1529 time:9334s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1205/shards.html
_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev