[igt-dev] [PATCH i-g-t v2 0/3] Make sure v3d/vc4 support performance monitors

[igt-dev] [PATCH i-g-t v2 0/3] Make sure v3d/vc4 support performance monitors

[Maíra Canal]

Currently, v3d and vc4 don't check if the driver supports the performance
monitor feature, which means that the tests are allowed to run even if the
driver doesn't support the tested feature. Therefore, this series adds checks
using the get_param ioctl from both drivers.

The main difference between the first and the second version of this series is
the first patch, which reworks the igt_vc4_get_param() function to adopt
do_ioctl and return the value of the parameter.

Best Regards,
- Maíra Canal

Maíra Canal (3):
  lib/igt_vc4: Rework igt_vc4_get_param()
  tests/vc4_perfmon: Make sure vc4 supports perfmon
  tests/v3d_perfmon: Make sure v3d supports perfmon

 lib/igt_vc4.c                | 10 +++-------
 lib/igt_vc4.h                |  2 +-
 tests/v3d/v3d_perfmon.c      |  4 +++-
 tests/vc4/vc4_perfmon.c      |  1 +
 tests/vc4/vc4_purgeable_bo.c |  5 +----
 5 files changed, 9 insertions(+), 13 deletions(-)

-- 
2.38.1

[igt-dev] [PATCH i-g-t v2 1/3] lib/igt_vc4: Rework igt_vc4_get_param()

[Maíra Canal]

Instead of returning the return of the DRM_IOCTL_VC4_GET_PARAM ioctl,
make igt_vc4_get_param() return the value of the parameter, considering
that the current return is not being used and the value of the parameter
is being used.

Suggested-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 lib/igt_vc4.c                | 10 +++-------
 lib/igt_vc4.h                |  2 +-
 tests/vc4/vc4_purgeable_bo.c |  5 +----
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
index 6b6ad16c..33c3a36e 100644
--- a/lib/igt_vc4.c
+++ b/lib/igt_vc4.c
@@ -168,19 +168,15 @@ uint64_t igt_vc4_get_tiling(int fd, uint32_t handle)
 	return get.modifier;
 }
 
-int igt_vc4_get_param(int fd, uint32_t param, uint64_t *val)
+uint64_t igt_vc4_get_param(int fd, uint32_t param)
 {
 	struct drm_vc4_get_param arg = {
 		.param = param,
 	};
-	int ret;
 
-	ret = igt_ioctl(fd, DRM_IOCTL_VC4_GET_PARAM, &arg);
-	if (ret)
-		return ret;
+	do_ioctl(fd, DRM_IOCTL_VC4_GET_PARAM, &arg);
 
-	*val = arg.value;
-	return 0;
+	return arg.value;
 }
 
 bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable)
diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
index 384d7d6e..4c08f175 100644
--- a/lib/igt_vc4.h
+++ b/lib/igt_vc4.h
@@ -31,7 +31,7 @@
 uint32_t igt_vc4_get_cleared_bo(int fd, size_t size, uint32_t clearval);
 int igt_vc4_create_bo(int fd, size_t size);
 void *igt_vc4_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned prot);
-int igt_vc4_get_param(int fd, uint32_t param, uint64_t *val);
+uint64_t igt_vc4_get_param(int fd, uint32_t param);
 bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable);
 bool igt_vc4_is_tiled(uint64_t modifier);
 bool igt_vc4_is_v3d(int fd);
diff --git a/tests/vc4/vc4_purgeable_bo.c b/tests/vc4/vc4_purgeable_bo.c
index 676bc1f3..27771012 100644
--- a/tests/vc4/vc4_purgeable_bo.c
+++ b/tests/vc4/vc4_purgeable_bo.c
@@ -102,11 +102,8 @@ igt_main
 	int fd, ret;
 
 	igt_fixture {
-		uint64_t val = 0;
-
 		fd = drm_open_driver(DRIVER_VC4);
-		igt_vc4_get_param(fd, DRM_VC4_PARAM_SUPPORTS_MADVISE, &val);
-		igt_require(val);
+		igt_require(igt_vc4_get_param(fd, DRM_VC4_PARAM_SUPPORTS_MADVISE));
 		IGT_INIT_LIST_HEAD(&list);
 	}
 
-- 
2.38.1

[igt-dev] [PATCH i-g-t v2 2/3] tests/vc4_perfmon: Make sure vc4 supports perfmon

[Maíra Canal]

Performance Monitors for the vc4 were introduced in Linux 4.17, so a
check if vc4 supports Performance Monitors is essential to assure that
the tests will perform correctly. Therefore, check if vc4 has
perfmon support before running the tests.

Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 tests/vc4/vc4_perfmon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vc4/vc4_perfmon.c b/tests/vc4/vc4_perfmon.c
index 30186519..664633a3 100644
--- a/tests/vc4/vc4_perfmon.c
+++ b/tests/vc4/vc4_perfmon.c
@@ -15,6 +15,7 @@ igt_main
 	igt_fixture {
 		fd = drm_open_driver(DRIVER_VC4);
 		igt_require(igt_vc4_is_v3d(fd));
+		igt_require(igt_vc4_get_param(fd, DRM_VC4_PARAM_SUPPORTS_PERFMON));
 	}
 
 	igt_describe("Make sure a perfmon cannot be created with zero counters.");
-- 
2.38.1

[igt-dev] [PATCH i-g-t v2 3/3] tests/v3d_perfmon: Make sure v3d supports perfmon

[Maíra Canal]

Performance Monitors for the v3d were introduced in Linux 5.15, so a check
if v3d supports Performance Monitors is essential to assure that the tests
will perform correctly. Therefore, check if v3d has perfmon support before
running the tests.

Reviewed-by: Melissa Wen <mwen@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 tests/v3d/v3d_perfmon.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/v3d/v3d_perfmon.c b/tests/v3d/v3d_perfmon.c
index bf983dbf..3f55ddb1 100644
--- a/tests/v3d/v3d_perfmon.c
+++ b/tests/v3d/v3d_perfmon.c
@@ -12,8 +12,10 @@ igt_main
 {
 	int fd;
 
-	igt_fixture
+	igt_fixture {
 		fd = drm_open_driver(DRIVER_V3D);
+		igt_require(igt_v3d_get_param(fd, DRM_V3D_PARAM_SUPPORTS_PERFMON));
+	}
 
 	igt_describe("Make sure a perfmon cannot be created with zero counters.");
 	igt_subtest("create-perfmon-0") {
-- 
2.38.1

Re: [igt-dev] [PATCH i-g-t v2 1/3] lib/igt_vc4: Rework igt_vc4_get_param()

[Kamil Konieczny]

Hi Maira,

On 2023-01-03 at 11:30:46 -0300, Maíra Canal wrote:
> Instead of returning the return of the DRM_IOCTL_VC4_GET_PARAM ioctl,
-------------------------- ^
error code of

> make igt_vc4_get_param() return the value of the parameter, considering
> that the current return is not being used and the value of the parameter
> is being used.
> 
> Suggested-by: Melissa Wen <mwen@igalia.com>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  lib/igt_vc4.c                | 10 +++-------
>  lib/igt_vc4.h                |  2 +-
>  tests/vc4/vc4_purgeable_bo.c |  5 +----
>  3 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/igt_vc4.c b/lib/igt_vc4.c
> index 6b6ad16c..33c3a36e 100644
> --- a/lib/igt_vc4.c
> +++ b/lib/igt_vc4.c
> @@ -168,19 +168,15 @@ uint64_t igt_vc4_get_tiling(int fd, uint32_t handle)
>  	return get.modifier;
>  }
>  

You should describe each public lib function, see for example
lib/ioctl_wrappers.c
There are some exceptions (like not all __func have one).

This change makes sense as for functions which do not assert
for errors inside we useally use __ as prefix, so old one
should be __igt_vc4_get_param

Regards,
Kamil

> -int igt_vc4_get_param(int fd, uint32_t param, uint64_t *val)
> +uint64_t igt_vc4_get_param(int fd, uint32_t param)
>  {
>  	struct drm_vc4_get_param arg = {
>  		.param = param,
>  	};
> -	int ret;
>  
> -	ret = igt_ioctl(fd, DRM_IOCTL_VC4_GET_PARAM, &arg);
> -	if (ret)
> -		return ret;
> +	do_ioctl(fd, DRM_IOCTL_VC4_GET_PARAM, &arg);
>  
> -	*val = arg.value;
> -	return 0;
> +	return arg.value;
>  }
>  
>  bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable)
> diff --git a/lib/igt_vc4.h b/lib/igt_vc4.h
> index 384d7d6e..4c08f175 100644
> --- a/lib/igt_vc4.h
> +++ b/lib/igt_vc4.h
> @@ -31,7 +31,7 @@
>  uint32_t igt_vc4_get_cleared_bo(int fd, size_t size, uint32_t clearval);
>  int igt_vc4_create_bo(int fd, size_t size);
>  void *igt_vc4_mmap_bo(int fd, uint32_t handle, uint32_t size, unsigned prot);
> -int igt_vc4_get_param(int fd, uint32_t param, uint64_t *val);
> +uint64_t igt_vc4_get_param(int fd, uint32_t param);
>  bool igt_vc4_purgeable_bo(int fd, int handle, bool purgeable);
>  bool igt_vc4_is_tiled(uint64_t modifier);
>  bool igt_vc4_is_v3d(int fd);
> diff --git a/tests/vc4/vc4_purgeable_bo.c b/tests/vc4/vc4_purgeable_bo.c
> index 676bc1f3..27771012 100644
> --- a/tests/vc4/vc4_purgeable_bo.c
> +++ b/tests/vc4/vc4_purgeable_bo.c
> @@ -102,11 +102,8 @@ igt_main
>  	int fd, ret;
>  
>  	igt_fixture {
> -		uint64_t val = 0;
> -
>  		fd = drm_open_driver(DRIVER_VC4);
> -		igt_vc4_get_param(fd, DRM_VC4_PARAM_SUPPORTS_MADVISE, &val);
> -		igt_require(val);
> +		igt_require(igt_vc4_get_param(fd, DRM_VC4_PARAM_SUPPORTS_MADVISE));
>  		IGT_INIT_LIST_HEAD(&list);
>  	}
>  
> -- 
> 2.38.1
> 

[igt-dev] ✗ Fi.CI.BAT: failure for Make sure v3d/vc4 support performance monitors

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 9074 bytes --]

== Series Details ==

Series: Make sure v3d/vc4 support performance monitors
URL   : https://patchwork.freedesktop.org/series/112370/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12541 -> IGTPW_8292
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_8292 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_8292, 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_8292/index.html

Participating hosts (42 -> 43)
------------------------------

  Additional (1): fi-rkl-11600 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries:
    - fi-icl-u2:          [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12541/fi-icl-u2/igt@debugfs_test@read_all_entries.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-icl-u2/igt@debugfs_test@read_all_entries.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - fi-rkl-11600:       NOTRUN -> [SKIP][3] ([i915#7456])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@debugfs_test@basic-hwmon.html

  * igt@gem_huc_copy@huc-copy:
    - fi-rkl-11600:       NOTRUN -> [SKIP][4] ([i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][5] ([i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@gem_lmem_swapping@basic.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-11600:       NOTRUN -> [SKIP][6] ([i915#3282])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-rkl-11600:       NOTRUN -> [SKIP][7] ([i915#7561])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_suspend@basic-s3-without-i915:
    - fi-rkl-11600:       NOTRUN -> [INCOMPLETE][8] ([i915#4817])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - bat-dg1-6:          NOTRUN -> [SKIP][9] ([fdo#111827])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/bat-dg1-6/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][10] ([fdo#111827]) +7 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - fi-rkl-11600:       NOTRUN -> [SKIP][11] ([i915#4103])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-rkl-11600:       NOTRUN -> [SKIP][12] ([fdo#109285] / [i915#4098])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@primary_page_flip:
    - fi-rkl-11600:       NOTRUN -> [SKIP][13] ([i915#1072]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@kms_psr@primary_page_flip.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-11600:       NOTRUN -> [SKIP][14] ([i915#3555] / [i915#4098])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-read:
    - fi-rkl-11600:       NOTRUN -> [SKIP][15] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@prime_vgem@basic-read.html

  * igt@prime_vgem@basic-userptr:
    - fi-rkl-11600:       NOTRUN -> [SKIP][16] ([fdo#109295] / [i915#3301] / [i915#3708])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-rkl-11600/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-icl-u2:          NOTRUN -> [FAIL][17] ([i915#4312])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-icl-u2/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@fbdev@write:
    - fi-blb-e6850:       [SKIP][18] ([fdo#109271]) -> [PASS][19] +4 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12541/fi-blb-e6850/igt@fbdev@write.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/fi-blb-e6850/igt@fbdev@write.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - {bat-rpls-1}:       [INCOMPLETE][20] ([i915#6687]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12541/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/bat-rpls-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@hangcheck:
    - bat-dg1-6:          [INCOMPLETE][22] -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12541/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/bat-dg1-6/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@migrate:
    - {bat-atsm-1}:       [DMESG-FAIL][24] ([i915#7699]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12541/bat-atsm-1/igt@i915_selftest@live@migrate.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/bat-atsm-1/igt@i915_selftest@live@migrate.html

  * igt@i915_selftest@live@slpc:
    - bat-adlp-4:         [DMESG-FAIL][26] ([i915#6367]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12541/bat-adlp-4/igt@i915_selftest@live@slpc.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/bat-adlp-4/igt@i915_selftest@live@slpc.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-c-dp-1:
    - {bat-adlp-9}:       [DMESG-WARN][28] ([i915#2867]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12541/bat-adlp-9/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-c-dp-1.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/bat-adlp-9/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-c-dp-1.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4817]: https://gitlab.freedesktop.org/drm/intel/issues/4817
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699


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

  * CI: CI-20190529 -> None
  * IGT: IGT_7106 -> IGTPW_8292

  CI-20190529: 20190529
  CI_DRM_12541: b832866fa6063614b3637598aca19aee3bc3039f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_8292: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8292/index.html
  IGT_7106: 8cce332bdc50d2b20d553d7a0221737f4399d031 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

== Logs ==

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

[-- Attachment #2: Type: text/html, Size: 10366 bytes --]