[Intel-gfx] [PATCH i-g-t] i915/perf_pmu: Replace init/read-other with a plea

[Intel-gfx] [PATCH i-g-t] i915/perf_pmu: Replace init/read-other with a plea

[Chris Wilson]

We cannot assume we know how many PMU there are exactly, so pick -1ULL
to represent all invalid metrics. Similarly, we have to rely on explicit
testing for each PMU to prove their existence and correct functioning.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/perf_pmu.c | 56 ++++++++++++-------------------------------
 1 file changed, 15 insertions(+), 41 deletions(-)

diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index e2f975a1a..db375341c 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -1165,38 +1165,12 @@ do { \
 	igt_assert_eq(errno, EINVAL);
 }
 
-static void init_other(int i915, unsigned int i, bool valid)
+static void open_invalid(int i915)
 {
 	int fd;
 
-	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
-	igt_require(!(fd < 0 && errno == ENODEV));
-	if (valid) {
-		igt_assert(fd >= 0);
-	} else {
-		igt_assert(fd < 0);
-		return;
-	}
-
-	close(fd);
-}
-
-static void read_other(int i915, unsigned int i, bool valid)
-{
-	int fd;
-
-	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
-	igt_require(!(fd < 0 && errno == ENODEV));
-	if (valid) {
-		igt_assert(fd >= 0);
-	} else {
-		igt_assert(fd < 0);
-		return;
-	}
-
-	(void)pmu_read_single(fd);
-
-	close(fd);
+	fd = perf_i915_open(i915, -1ULL);
+	igt_assert(fd < 0);
 }
 
 static bool cpu0_hotplug_support(void)
@@ -2058,6 +2032,12 @@ igt_main
 	unsigned int num_engines = 0;
 	int fd = -1;
 
+	/**
+	 * All PMU should be accompanied by a test.
+	 *
+	 * Including all the I915_PMU_OTHER(x).
+	 */
+
 	igt_fixture {
 		fd = __drm_open_driver(DRIVER_INTEL);
 
@@ -2075,6 +2055,12 @@ igt_main
 	igt_subtest("invalid-init")
 		invalid_init(fd);
 
+	/**
+	 * Double check the invalid metric does fail.
+	 */
+	igt_subtest("invalid-open")
+		open_invalid(fd);
+
 	igt_subtest_with_dynamic("faulting-read") {
 		for_each_mmap_offset_type(fd, t) {
 			igt_dynamic_f("%s", t->name)
@@ -2228,18 +2214,6 @@ igt_main
 		all_busy_check_all(fd, num_engines,
 				   TEST_BUSY | TEST_TRAILING_IDLE);
 
-	/**
-	 * Test that non-engine counters can be initialized and read. Apart
-	 * from the invalid metric which should fail.
-	 */
-	for (unsigned int i = 0; i < num_other_metrics + 1; i++) {
-		igt_subtest_f("other-init-%u", i)
-			init_other(fd, i, i < num_other_metrics);
-
-		igt_subtest_f("other-read-%u", i)
-			read_other(fd, i, i < num_other_metrics);
-	}
-
 	/**
 	 * Test counters are not affected by CPU offline/online events.
 	 */
-- 
2.29.2

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

[igt-dev] [PATCH i-g-t] i915/perf_pmu: Replace init/read-other with a plea

[Chris Wilson]

We cannot assume we know how many PMU there are exactly, so pick -1ULL
to represent all invalid metrics. Similarly, we have to rely on explicit
testing for each PMU to prove their existence and correct functioning.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/perf_pmu.c | 56 ++++++++++++-------------------------------
 1 file changed, 15 insertions(+), 41 deletions(-)

diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
index e2f975a1a..db375341c 100644
--- a/tests/i915/perf_pmu.c
+++ b/tests/i915/perf_pmu.c
@@ -1165,38 +1165,12 @@ do { \
 	igt_assert_eq(errno, EINVAL);
 }
 
-static void init_other(int i915, unsigned int i, bool valid)
+static void open_invalid(int i915)
 {
 	int fd;
 
-	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
-	igt_require(!(fd < 0 && errno == ENODEV));
-	if (valid) {
-		igt_assert(fd >= 0);
-	} else {
-		igt_assert(fd < 0);
-		return;
-	}
-
-	close(fd);
-}
-
-static void read_other(int i915, unsigned int i, bool valid)
-{
-	int fd;
-
-	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
-	igt_require(!(fd < 0 && errno == ENODEV));
-	if (valid) {
-		igt_assert(fd >= 0);
-	} else {
-		igt_assert(fd < 0);
-		return;
-	}
-
-	(void)pmu_read_single(fd);
-
-	close(fd);
+	fd = perf_i915_open(i915, -1ULL);
+	igt_assert(fd < 0);
 }
 
 static bool cpu0_hotplug_support(void)
@@ -2058,6 +2032,12 @@ igt_main
 	unsigned int num_engines = 0;
 	int fd = -1;
 
+	/**
+	 * All PMU should be accompanied by a test.
+	 *
+	 * Including all the I915_PMU_OTHER(x).
+	 */
+
 	igt_fixture {
 		fd = __drm_open_driver(DRIVER_INTEL);
 
@@ -2075,6 +2055,12 @@ igt_main
 	igt_subtest("invalid-init")
 		invalid_init(fd);
 
+	/**
+	 * Double check the invalid metric does fail.
+	 */
+	igt_subtest("invalid-open")
+		open_invalid(fd);
+
 	igt_subtest_with_dynamic("faulting-read") {
 		for_each_mmap_offset_type(fd, t) {
 			igt_dynamic_f("%s", t->name)
@@ -2228,18 +2214,6 @@ igt_main
 		all_busy_check_all(fd, num_engines,
 				   TEST_BUSY | TEST_TRAILING_IDLE);
 
-	/**
-	 * Test that non-engine counters can be initialized and read. Apart
-	 * from the invalid metric which should fail.
-	 */
-	for (unsigned int i = 0; i < num_other_metrics + 1; i++) {
-		igt_subtest_f("other-init-%u", i)
-			init_other(fd, i, i < num_other_metrics);
-
-		igt_subtest_f("other-read-%u", i)
-			read_other(fd, i, i < num_other_metrics);
-	}
-
 	/**
 	 * Test counters are not affected by CPU offline/online events.
 	 */
-- 
2.29.2

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

[igt-dev] ✗ Fi.CI.BAT: failure for i915/perf_pmu: Replace init/read-other with a plea

[Patchwork]

[-- Attachment #1.1: Type: text/plain, Size: 8658 bytes --]

== Series Details ==

Series: i915/perf_pmu: Replace init/read-other with a plea
URL   : https://patchwork.freedesktop.org/series/85011/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9494 -> IGTPW_5301
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gem:
    - fi-bsw-n3050:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-bsw-n3050/igt@i915_selftest@live@gem.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-bsw-n3050/igt@i915_selftest@live@gem.html

  * igt@i915_selftest@live@gem_contexts:
    - fi-bsw-kefka:       [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-bsw-kefka/igt@i915_selftest@live@gem_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-bsw-kefka/igt@i915_selftest@live@gem_contexts.html
    - fi-bsw-n3050:       [PASS][5] -> [INCOMPLETE][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-bsw-n3050/igt@i915_selftest@live@gem_contexts.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-bsw-n3050/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@gt_mocs:
    - fi-apl-guc:         NOTRUN -> [DMESG-WARN][7] +16 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-apl-guc/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@hugepages:
    - fi-bsw-kefka:       [PASS][8] -> [DMESG-WARN][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-bsw-kefka/igt@i915_selftest@live@hugepages.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-bsw-kefka/igt@i915_selftest@live@hugepages.html

  * igt@i915_selftest@live@workarounds:
    - fi-apl-guc:         [PASS][10] -> [DMESG-WARN][11] +3 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-apl-guc/igt@i915_selftest@live@workarounds.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-apl-guc/igt@i915_selftest@live@workarounds.html

  
#### Warnings ####

  * igt@i915_selftest@live@gt_timelines:
    - fi-apl-guc:         [INCOMPLETE][12] ([i915#2750]) -> [DMESG-WARN][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-apl-guc:         NOTRUN -> [SKIP][14] ([fdo#109271]) +17 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-apl-guc/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@core_hotunplug@unbind-rebind:
    - fi-kbl-7500u:       [PASS][15] -> [DMESG-WARN][16] ([i915#2605])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-kbl-7500u/igt@core_hotunplug@unbind-rebind.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-kbl-7500u/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-tgl-u2:          [PASS][17] -> [FAIL][18] ([i915#1888])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_module_load@reload:
    - fi-apl-guc:         [PASS][19] -> [DMESG-WARN][20] ([i915#180] / [i915#203] / [i915#62]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-apl-guc/igt@i915_module_load@reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-apl-guc/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@execlists:
    - fi-apl-guc:         NOTRUN -> [DMESG-WARN][21] ([i915#1037])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-apl-guc/igt@i915_selftest@live@execlists.html

  * igt@i915_selftest@live@ring_submission:
    - fi-apl-guc:         NOTRUN -> [DMESG-WARN][22] ([i915#203]) +8 similar issues
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-apl-guc/igt@i915_selftest@live@ring_submission.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-dvi-d1:
    - fi-bwr-2160:        [PASS][23] -> [FAIL][24] ([i915#2122])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@a-dvi-d1.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-bwr-2160/igt@kms_flip@basic-flip-vs-wf_vblank@a-dvi-d1.html

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> [FAIL][25] ([i915#1436] / [i915#2722])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-bsw-kefka/igt@runner@aborted.html
    - fi-bsw-n3050:       NOTRUN -> [FAIL][26] ([i915#1436] / [i915#2722] / [i915#483])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-bsw-n3050/igt@runner@aborted.html

  * igt@vgem_basic@setversion:
    - fi-tgl-y:           [PASS][27] -> [DMESG-WARN][28] ([i915#402]) +2 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-tgl-y/igt@vgem_basic@setversion.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-tgl-y/igt@vgem_basic@setversion.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      [DMESG-FAIL][29] ([i915#2291] / [i915#541]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@prime_self_import@basic-with_two_bos:
    - fi-tgl-y:           [DMESG-WARN][31] ([i915#402]) -> [PASS][32] +1 similar issue
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9494/fi-tgl-y/igt@prime_self_import@basic-with_two_bos.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/fi-tgl-y/igt@prime_self_import@basic-with_two_bos.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1037]: https://gitlab.freedesktop.org/drm/intel/issues/1037
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#203]: https://gitlab.freedesktop.org/drm/intel/issues/203
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#2605]: https://gitlab.freedesktop.org/drm/intel/issues/2605
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#2750]: https://gitlab.freedesktop.org/drm/intel/issues/2750
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#483]: https://gitlab.freedesktop.org/drm/intel/issues/483
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


Participating hosts (42 -> 38)
------------------------------

  Missing    (4): fi-dg1-1 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u 


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

  * CI: CI-20190529 -> None
  * IGT: IGT_5905 -> IGTPW_5301

  CI-20190529: 20190529
  CI_DRM_9494: 0daa598dbcfd00141cb7e287d6e1369916097161 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_5301: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/index.html
  IGT_5905: 3d0934900bddeb7a68f1abab4cd05077f0609e32 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@perf_pmu@invalid-open
-igt@perf_pmu@other-init-0
-igt@perf_pmu@other-init-1
-igt@perf_pmu@other-init-2
-igt@perf_pmu@other-init-3
-igt@perf_pmu@other-init-4
-igt@perf_pmu@other-read-0
-igt@perf_pmu@other-read-1
-igt@perf_pmu@other-read-2
-igt@perf_pmu@other-read-3
-igt@perf_pmu@other-read-4

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_5301/index.html

[-- Attachment #1.2: Type: text/html, Size: 10076 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

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

Re: [Intel-gfx] [igt-dev] [PATCH i-g-t] i915/perf_pmu: Replace init/read-other with a plea

[Tvrtko Ursulin]

On 16/12/2020 16:54, Chris Wilson wrote:
> We cannot assume we know how many PMU there are exactly, so pick -1ULL
> to represent all invalid metrics. Similarly, we have to rely on explicit
> testing for each PMU to prove their existence and correct functioning.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/i915/perf_pmu.c | 56 ++++++++++++-------------------------------
>   1 file changed, 15 insertions(+), 41 deletions(-)
> 
> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> index e2f975a1a..db375341c 100644
> --- a/tests/i915/perf_pmu.c
> +++ b/tests/i915/perf_pmu.c
> @@ -1165,38 +1165,12 @@ do { \
>   	igt_assert_eq(errno, EINVAL);
>   }
>   
> -static void init_other(int i915, unsigned int i, bool valid)
> +static void open_invalid(int i915)
>   {
>   	int fd;
>   
> -	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
> -	igt_require(!(fd < 0 && errno == ENODEV));
> -	if (valid) {
> -		igt_assert(fd >= 0);
> -	} else {
> -		igt_assert(fd < 0);
> -		return;
> -	}
> -
> -	close(fd);
> -}
> -
> -static void read_other(int i915, unsigned int i, bool valid)
> -{
> -	int fd;
> -
> -	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
> -	igt_require(!(fd < 0 && errno == ENODEV));
> -	if (valid) {
> -		igt_assert(fd >= 0);
> -	} else {
> -		igt_assert(fd < 0);
> -		return;
> -	}
> -
> -	(void)pmu_read_single(fd);
> -
> -	close(fd);
> +	fd = perf_i915_open(i915, -1ULL);
> +	igt_assert(fd < 0);
>   }
>   
>   static bool cpu0_hotplug_support(void)
> @@ -2058,6 +2032,12 @@ igt_main
>   	unsigned int num_engines = 0;
>   	int fd = -1;
>   
> +	/**
> +	 * All PMU should be accompanied by a test.
> +	 *
> +	 * Including all the I915_PMU_OTHER(x).
> +	 */
> +
>   	igt_fixture {
>   		fd = __drm_open_driver(DRIVER_INTEL);
>   
> @@ -2075,6 +2055,12 @@ igt_main
>   	igt_subtest("invalid-init")
>   		invalid_init(fd);
>   
> +	/**
> +	 * Double check the invalid metric does fail.
> +	 */
> +	igt_subtest("invalid-open")
> +		open_invalid(fd);
> +
>   	igt_subtest_with_dynamic("faulting-read") {
>   		for_each_mmap_offset_type(fd, t) {
>   			igt_dynamic_f("%s", t->name)
> @@ -2228,18 +2214,6 @@ igt_main
>   		all_busy_check_all(fd, num_engines,
>   				   TEST_BUSY | TEST_TRAILING_IDLE);
>   
> -	/**
> -	 * Test that non-engine counters can be initialized and read. Apart
> -	 * from the invalid metric which should fail.
> -	 */
> -	for (unsigned int i = 0; i < num_other_metrics + 1; i++) {
> -		igt_subtest_f("other-init-%u", i)
> -			init_other(fd, i, i < num_other_metrics);
> -
> -		igt_subtest_f("other-read-%u", i)
> -			read_other(fd, i, i < num_other_metrics);
> -	}
> -
>   	/**
>   	 * Test counters are not affected by CPU offline/online events.
>   	 */
> 

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] i915/perf_pmu: Replace init/read-other with a plea

[Tvrtko Ursulin]

On 16/12/2020 16:54, Chris Wilson wrote:
> We cannot assume we know how many PMU there are exactly, so pick -1ULL
> to represent all invalid metrics. Similarly, we have to rely on explicit
> testing for each PMU to prove their existence and correct functioning.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/i915/perf_pmu.c | 56 ++++++++++++-------------------------------
>   1 file changed, 15 insertions(+), 41 deletions(-)
> 
> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> index e2f975a1a..db375341c 100644
> --- a/tests/i915/perf_pmu.c
> +++ b/tests/i915/perf_pmu.c
> @@ -1165,38 +1165,12 @@ do { \
>   	igt_assert_eq(errno, EINVAL);
>   }
>   
> -static void init_other(int i915, unsigned int i, bool valid)
> +static void open_invalid(int i915)
>   {
>   	int fd;
>   
> -	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
> -	igt_require(!(fd < 0 && errno == ENODEV));
> -	if (valid) {
> -		igt_assert(fd >= 0);
> -	} else {
> -		igt_assert(fd < 0);
> -		return;
> -	}
> -
> -	close(fd);
> -}
> -
> -static void read_other(int i915, unsigned int i, bool valid)
> -{
> -	int fd;
> -
> -	fd = perf_i915_open(i915, __I915_PMU_OTHER(i));
> -	igt_require(!(fd < 0 && errno == ENODEV));
> -	if (valid) {
> -		igt_assert(fd >= 0);
> -	} else {
> -		igt_assert(fd < 0);
> -		return;
> -	}
> -
> -	(void)pmu_read_single(fd);
> -
> -	close(fd);
> +	fd = perf_i915_open(i915, -1ULL);
> +	igt_assert(fd < 0);
>   }
>   
>   static bool cpu0_hotplug_support(void)
> @@ -2058,6 +2032,12 @@ igt_main
>   	unsigned int num_engines = 0;
>   	int fd = -1;
>   
> +	/**
> +	 * All PMU should be accompanied by a test.
> +	 *
> +	 * Including all the I915_PMU_OTHER(x).
> +	 */
> +
>   	igt_fixture {
>   		fd = __drm_open_driver(DRIVER_INTEL);
>   
> @@ -2075,6 +2055,12 @@ igt_main
>   	igt_subtest("invalid-init")
>   		invalid_init(fd);
>   
> +	/**
> +	 * Double check the invalid metric does fail.
> +	 */
> +	igt_subtest("invalid-open")
> +		open_invalid(fd);
> +
>   	igt_subtest_with_dynamic("faulting-read") {
>   		for_each_mmap_offset_type(fd, t) {
>   			igt_dynamic_f("%s", t->name)
> @@ -2228,18 +2214,6 @@ igt_main
>   		all_busy_check_all(fd, num_engines,
>   				   TEST_BUSY | TEST_TRAILING_IDLE);
>   
> -	/**
> -	 * Test that non-engine counters can be initialized and read. Apart
> -	 * from the invalid metric which should fail.
> -	 */
> -	for (unsigned int i = 0; i < num_other_metrics + 1; i++) {
> -		igt_subtest_f("other-init-%u", i)
> -			init_other(fd, i, i < num_other_metrics);
> -
> -		igt_subtest_f("other-read-%u", i)
> -			read_other(fd, i, i < num_other_metrics);
> -	}
> -
>   	/**
>   	 * Test counters are not affected by CPU offline/online events.
>   	 */
> 

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