[igt-dev] [PATCH 1/3] tests/amdgpu/amd_mall: Add crc check

[igt-dev] [PATCH 1/3] tests/amdgpu/amd_mall: Add crc check

[Aurabindo Pillai]

In addition to checking whether display scanout from MALL cache is
triggered, double check that the same pattern is being displayed. This
can detect any corruption due to incorrect MALL size calculations

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
---
 tests/amdgpu/amd_mall.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/amdgpu/amd_mall.c b/tests/amdgpu/amd_mall.c
index c50d046ee..6016d5e8c 100644
--- a/tests/amdgpu/amd_mall.c
+++ b/tests/amdgpu/amd_mall.c
@@ -117,6 +117,7 @@ static void test_mall_ss(data_t *data)
 	igt_fb_t rfb;
 	int exec_ret;
 	struct line_check line = {0};
+	igt_crc_t test_crc, ref_crc;
 
 	test_init(data);
 
@@ -124,6 +125,7 @@ static void test_mall_ss(data_t *data)
 	igt_plane_set_fb(data->primary, &rfb);
 	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
 
+	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
 	sleep(MALL_SETTLE_DELAY);
 
 	igt_system_cmd(exec_ret, "umr -O bits -r *.*.HUBP0_HUBP_MALL_STATUS | grep MALL_IN_USE");
@@ -135,6 +137,10 @@ static void test_mall_ss(data_t *data)
 
 	igt_assert_eq(line.found, 1);
 
+	igt_pipe_crc_collect_crc(data->pipe_crc, &test_crc);
+
+	igt_assert_crc_equal(&ref_crc, &test_crc);
+
 	igt_remove_fb(data->fd, &rfb);
 	test_fini(data);
 }
-- 
2.41.0

[igt-dev] [PATCH 2/3] tests/amdgpu/amd_mall: remove UMR dependency

[Aurabindo Pillai]

Remove the dependency on the userspace tool UMR to check MALL status
and fully rely on debugfs for simplicity of test setup.

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
---
 lib/igt_amd.c           | 26 +++++++++++++-------------
 lib/igt_amd.h           |  2 +-
 tests/amdgpu/amd_mall.c | 31 +++++++------------------------
 3 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/lib/igt_amd.c b/lib/igt_amd.c
index 1a720ff56..49ca9e6d9 100644
--- a/lib/igt_amd.c
+++ b/lib/igt_amd.c
@@ -1182,27 +1182,27 @@ static bool get_dm_capabilites(int drm_fd, char *buf, size_t size) {
  * @brief check if AMDGPU mall_capable interface entry exist and defined
  *
  * @param drm_fd DRM file descriptor
- * @return true if mall_capable debugfs interface exists and defined
- * @return false otherwise
+ * @return true if dm capabilities interface exists and MALL is supported
+ * @return false if capabilites could not be read.
  */
-bool igt_amd_is_mall_capable(int drm_fd)
+void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
 {
-	char buf[1024], mall_read[10];
+	char buf[1024];
 	char *mall_loc;
 
-	if (!get_dm_capabilites(drm_fd, buf, 1024))
-		return false;
+	*supported = false;
+	*enabled = false;
 
-	mall_loc = strstr(buf,"mall: ");
-	if (!mall_loc)
+	if (!get_dm_capabilites(drm_fd, buf, 1024))
 		return false;
 
-	sscanf(mall_loc, "mall: %s", mall_read);
-
-	if (!strcmp(mall_read, "yes"))
-		return true;
+	mall_loc = strstr(buf, "mall supported: yes");
+	if (mall_loc)
+		*supported = true;
 
-	return false;
+	mall_loc = strstr(buf, "enabled: yes");
+	if (mall_loc && *supported)
+		*enabled = true;
 }
 
 /**
diff --git a/lib/igt_amd.h b/lib/igt_amd.h
index c05b4b730..db226e0d0 100644
--- a/lib/igt_amd.h
+++ b/lib/igt_amd.h
@@ -199,6 +199,6 @@ bool igt_amd_has_visual_confirm(int drm_fd);
 int  igt_amd_get_visual_confirm(int drm_fd);
 bool igt_amd_set_visual_confirm(int drm_fd, enum amdgpu_debug_visual_confirm option);
 
-bool igt_amd_is_mall_capable(int drm_fd);
+void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled);
 bool igt_amd_output_has_odm_combine_segments(int drm_fd, char *connector_name);
 #endif /* IGT_AMD_H */
diff --git a/tests/amdgpu/amd_mall.c b/tests/amdgpu/amd_mall.c
index 6016d5e8c..178203e8b 100644
--- a/tests/amdgpu/amd_mall.c
+++ b/tests/amdgpu/amd_mall.c
@@ -57,7 +57,8 @@ struct line_check {
 static void test_init(data_t *data)
 {
 	igt_display_t *display = &data->display;
-	bool mall_capable = false;
+	u32 mall_capable = false;
+	u32 mall_en = false;
 
 	/* It doesn't matter which pipe we choose on amdpgu. */
 	data->pipe_id = PIPE_A;
@@ -65,7 +66,7 @@ static void test_init(data_t *data)
 
 	igt_display_reset(display);
 
-	mall_capable =  igt_amd_is_mall_capable(data->fd);
+	igt_amd_get_mall_status(data->fd, &mall_capable, &mall_en);
 	igt_require_f(mall_capable, "Requires hardware that supports MALL cache\n");
 
 	/* find a connected output */
@@ -101,44 +102,26 @@ static void test_fini(data_t *data)
 	igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
 }
 
-static bool check_cmd_output(const char *line, void *data)
-{
-	struct line_check *check = data;
-
-	if (strstr(line, check->substr)) {
-		check->found++;
-	}
-
-	return false;
-}
 static void test_mall_ss(data_t *data)
 {
 	igt_display_t *display = &data->display;
 	igt_fb_t rfb;
-	int exec_ret;
-	struct line_check line = {0};
 	igt_crc_t test_crc, ref_crc;
+	u32 mall_supp, mall_en;
 
 	test_init(data);
 
 	igt_create_pattern_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, 0, &rfb);
 	igt_plane_set_fb(data->primary, &rfb);
 	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
-
 	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
-	sleep(MALL_SETTLE_DELAY);
-
-	igt_system_cmd(exec_ret, "umr -O bits -r *.*.HUBP0_HUBP_MALL_STATUS | grep MALL_IN_USE");
-
-	igt_skip_on_f(exec_ret != IGT_EXIT_SUCCESS, "Error running UMR\n");
 
-	line.substr = "1 (0x00000001)";
-	igt_log_buffer_inspect(check_cmd_output, &line);
+	sleep(MALL_SETTLE_DELAY);
 
-	igt_assert_eq(line.found, 1);
+	igt_amd_get_mall_status(data->fd, &mall_supp, &mall_en);
+	igt_fail_on_f(!(mall_supp && mall_en), "MALL did not get enabled\n");
 
 	igt_pipe_crc_collect_crc(data->pipe_crc, &test_crc);
-
 	igt_assert_crc_equal(&ref_crc, &test_crc);
 
 	igt_remove_fb(data->fd, &rfb);
-- 
2.41.0

[igt-dev] [PATCH 3/3] lib/amdgpu: checkpatch.pl fixes

[Aurabindo Pillai]

fix checkpatch.pl identified issue: typo and braces

Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
---
 lib/igt_amd.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/igt_amd.c b/lib/igt_amd.c
index 49ca9e6d9..3199bf159 100644
--- a/lib/igt_amd.c
+++ b/lib/igt_amd.c
@@ -1154,7 +1154,8 @@ void igt_amd_allow_edp_hotplug_detect(int drm_fd, char *connector_name, bool ena
 	close(hpd_fd);
 }
 
-static bool get_dm_capabilites(int drm_fd, char *buf, size_t size) {
+static bool get_dm_capabilities(int drm_fd, char *buf, size_t size)
+{
 	int ret, fd;
 	bool has_capablities = amd_has_debugfs(drm_fd, DEBUGFS_DM_CAPABILITIES);
 
@@ -1183,7 +1184,7 @@ static bool get_dm_capabilites(int drm_fd, char *buf, size_t size) {
  *
  * @param drm_fd DRM file descriptor
  * @return true if dm capabilities interface exists and MALL is supported
- * @return false if capabilites could not be read.
+ * @return false if capabilities could not be read.
  */
 void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
 {
@@ -1193,7 +1194,7 @@ void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
 	*supported = false;
 	*enabled = false;
 
-	if (!get_dm_capabilites(drm_fd, buf, 1024))
+	if (!get_dm_capabilities(drm_fd, buf, 1024))
 		return false;
 
 	mall_loc = strstr(buf, "mall supported: yes");
-- 
2.41.0

[igt-dev] ✗ Fi.CI.BUILD: failure for series starting with [1/3] tests/amdgpu/amd_mall: Add crc check

[Patchwork]

== Series Details ==

Series: series starting with [1/3] tests/amdgpu/amd_mall: Add crc check
URL   : https://patchwork.freedesktop.org/series/122852/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
5d48d1fb231f449fe2f80cda14ea7a1ecfda59fa tests/amdgpu: Fix build error due to external file include

Tail of build.log:
[402/1626] Compiling C object 'tests/59830eb@@kms_plane@exe/kms_plane.c.o'.
[403/1626] Compiling C object 'tests/59830eb@@kms_rotation_crc@exe/kms_rotation_crc.c.o'.
[404/1626] Compiling C object 'tests/59830eb@@i915_suspend@exe/i915_i915_suspend.c.o'.
[405/1626] Compiling C object 'tests/59830eb@@gem_caching@exe/i915_gem_caching.c.o'.
[406/1626] Compiling C object 'tests/59830eb@@sw_sync@exe/sw_sync.c.o'.
[407/1626] Compiling C object 'lib/76b5a35@@igt-intel_batchbuffer_c@sta/intel_batchbuffer.c.o'.
[408/1626] Generating intel_lid.1 with a custom command.
[409/1626] Compiling C object 'tests/59830eb@@kms_lease@exe/kms_lease.c.o'.
[410/1626] Generating intel_upload_blit_large_gtt.1 with a custom command.
[411/1626] Generating intel_reg.1 with a custom command.
[412/1626] Compiling C object 'tests/59830eb@@gem_ctx_freq@exe/i915_gem_ctx_freq.c.o'.
[413/1626] Compiling C object 'tests/59830eb@@vgem_basic@exe/vgem_basic.c.o'.
[414/1626] Compiling C object 'tests/59830eb@@testdisplay@exe/testdisplay.c.o'.
[415/1626] Generating intel_stepping.1 with a custom command.
[416/1626] Compiling C object 'tests/59830eb@@gem_close_race@exe/i915_gem_close_race.c.o'.
[417/1626] Compiling C object 'lib/76b5a35@@igt-igt_core_c@sta/igt_core.c.o'.
[418/1626] Generating intel_upload_blit_small.1 with a custom command.
[419/1626] Generating intel_panel_fitter.1 with a custom command.
[420/1626] Generating intel_upload_blit_large.1 with a custom command.
[421/1626] Generating intel_upload_blit_large_map.1 with a custom command.
[422/1626] Generating intel_vbt_decode.1 with a custom command.
[423/1626] Compiling C object 'tests/59830eb@@syncobj_wait@exe/syncobj_wait.c.o'.
[424/1626] Compiling C object 'lib/76b5a35@@igt-rendercopy_gen8_c@sta/rendercopy_gen8.c.o'.
[425/1626] Compiling C object 'tests/59830eb@@gem_ctx_create@exe/i915_gem_ctx_create.c.o'.
[426/1626] Compiling C object 'tests/59830eb@@gem_evict_alignment@exe/i915_gem_evict_alignment.c.o'.
[427/1626] Compiling C object 'tests/59830eb@@gem_create@exe/i915_gem_create.c.o'.
[428/1626] Compiling C object 'tests/59830eb@@drm_fdinfo@exe/i915_drm_fdinfo.c.o'.
[429/1626] Compiling C object 'lib/76b5a35@@igt-igt_chamelium_c@sta/igt_chamelium.c.o'.
[430/1626] Compiling C object 'tests/59830eb@@prime_vgem@exe/prime_vgem.c.o'.
[431/1626] Compiling C object 'lib/76b5a35@@igt-rendercopy_gen9_c@sta/rendercopy_gen9.c.o'.
[432/1626] Compiling C object 'tests/59830eb@@gem_ctx_switch@exe/i915_gem_ctx_switch.c.o'.
[433/1626] Compiling C object 'tests/59830eb@@kms_cursor_legacy@exe/kms_cursor_legacy.c.o'.
[434/1626] Compiling C object 'tests/59830eb@@api_intel_allocator@exe/i915_api_intel_allocator.c.o'.
[435/1626] Compiling C object 'tests/59830eb@@kms_plane_scaling@exe/kms_plane_scaling.c.o'.
[436/1626] Compiling C object 'tests/59830eb@@kms_big_fb@exe/i915_kms_big_fb.c.o'.
[437/1626] Compiling C object 'tests/59830eb@@gem_blits@exe/i915_gem_blits.c.o'.
[438/1626] Compiling C object 'tests/59830eb@@gem_ctx_engines@exe/i915_gem_ctx_engines.c.o'.
[439/1626] Compiling C object 'tests/59830eb@@gem_busy@exe/i915_gem_busy.c.o'.
[440/1626] Compiling C object 'tests/59830eb@@gem_ctx_isolation@exe/i915_gem_ctx_isolation.c.o'.
[441/1626] Compiling C object 'tests/59830eb@@syncobj_timeline@exe/syncobj_timeline.c.o'.
[442/1626] Compiling C object 'lib/76b5a35@@igt-i915_intel_decode_c@sta/i915_intel_decode.c.o'.
[443/1626] Compiling C object 'tests/59830eb@@gem_userptr_blits@exe/i915_gem_userptr_blits.c.o'.
[444/1626] Compiling C object 'tests/59830eb@@gem_eio@exe/i915_gem_eio.c.o'.
[445/1626] Compiling C object 'tests/59830eb@@api_intel_bb@exe/i915_api_intel_bb.c.o'.
[446/1626] Compiling C object 'tests/59830eb@@gem_ctx_shared@exe/i915_gem_ctx_shared.c.o'.
[447/1626] Compiling C object 'lib/76b5a35@@igt-igt_fb_c@sta/igt_fb.c.o'.
[448/1626] Compiling C object 'tests/59830eb@@gem_concurrent_blit@exe/i915_gem_concurrent_blit.c.o'.
[449/1626] Compiling C object 'lib/76b5a35@@igt-igt_kms_c@sta/igt_kms.c.o'.
[450/1626] Generating i915-perf-equations with a custom command.
ninja: build stopped: subcommand failed.

[igt-dev] ✗ GitLab.Pipeline: warning for series starting with [1/3] tests/amdgpu/amd_mall: Add crc check

[Patchwork]

== Series Details ==

Series: series starting with [1/3] tests/amdgpu/amd_mall: Add crc check
URL   : https://patchwork.freedesktop.org/series/122852/
State : warning

== Summary ==

Pipeline status: FAILED.

see https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/969821 for the overview.

build:tests-fedora has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/47999493):
  [2/1397] Linking static target lib/libigt-igt_kms_c.a.
  [3/1397] Linking static target lib/libigt-igt_fb_c.a.
  [4/1397] Compiling C object 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o'.
  FAILED: lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o 
  cc -Ilib/76b5a35@@igt-igt_amd_c@sta -Ilib -I../lib -I../include -I../include/drm-uapi -I../include/linux-uapi -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/valgrind -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fPIC -pthread '-DIGT_DATADIR="/opt/igt/share/igt-gpu-tools"' '-DIGT_SRCDIR="/builds/gfx-ci/igt-ci-tags/tests"' '-DIGT_LOG_DOMAIN="igt_amd"' -MD -MQ 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -MF 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o.d' -o 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -c ../lib/igt_amd.c
  ../lib/igt_amd.c: In function ‘igt_amd_get_mall_status’:
  ../lib/igt_amd.c:1198:10: error: ‘return’ with a value, in function returning void [-Werror=return-type]
   1198 |   return false;
        |          ^~~~~
  ../lib/igt_amd.c:1189:6: note: declared here
   1189 | void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
        |      ^~~~~~~~~~~~~~~~~~~~~~~
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1692898951:step_script
  section_start:1692898951:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1692898951:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-clang has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/47999497):
  ninja: Entering directory `build'
  [1/1425] Generating version.h with a custom command.
  [2/1421] Linking static target lib/libigt-igt_kms_c.a.
  [3/1421] Linking static target lib/libigt-igt_fb_c.a.
  [4/1421] Linking static target lib/libigt-igt_vmwgfx_c.a.
  [5/1421] Linking static target lib/libigt-igt_edid_c.a.
  [6/1421] Compiling C object 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o'.
  FAILED: lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o 
  clang -Ilib/76b5a35@@igt-igt_amd_c@sta -Ilib -I../lib -I../include -I../include/drm-uapi -I../include/linux-uapi -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/valgrind -Xclang -fcolor-diagnostics -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fPIC -pthread '-DIGT_DATADIR="/usr/local/share/igt-gpu-tools"' '-DIGT_SRCDIR="/builds/gfx-ci/igt-ci-tags/tests"' '-DIGT_LOG_DOMAIN="igt_amd"' -MD -MQ 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -MF 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o.d' -o 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -c ../lib/igt_amd.c
  ../lib/igt_amd.c:1198:3: error: void function 'igt_amd_get_mall_status' should not return a value [-Wreturn-type]
                  return false;
                  ^      ~~~~~
  1 error generated.
  ninja: build stopped: subcommand failed.
  section_end:1692898960:step_script
  section_start:1692898960:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1692898960:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-no-libdrm-nouveau has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/47999496):
  [2/1245] Linking static target lib/libigt-igt_kms_c.a.
  [3/1245] Linking static target lib/libigt-igt_fb_c.a.
  [4/1245] Compiling C object 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o'.
  FAILED: lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o 
  cc -Ilib/76b5a35@@igt-igt_amd_c@sta -Ilib -I../lib -I../include -I../include/drm-uapi -I../include/linux-uapi -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/valgrind -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fPIC -pthread '-DIGT_DATADIR="/usr/local/share/igt-gpu-tools"' '-DIGT_SRCDIR="/builds/gfx-ci/igt-ci-tags/tests"' '-DIGT_LOG_DOMAIN="igt_amd"' -MD -MQ 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -MF 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o.d' -o 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -c ../lib/igt_amd.c
  ../lib/igt_amd.c: In function ‘igt_amd_get_mall_status’:
  ../lib/igt_amd.c:1198:10: error: ‘return’ with a value, in function returning void [-Werror=return-type]
   1198 |   return false;
        |          ^~~~~
  ../lib/igt_amd.c:1189:6: note: declared here
   1189 | void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
        |      ^~~~~~~~~~~~~~~~~~~~~~~
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1692898951:step_script
  section_start:1692898951:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1692898952:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-no-libunwind has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/47999494):
  [2/1395] Linking static target lib/libigt-igt_kms_c.a.
  [3/1395] Linking static target lib/libigt-igt_fb_c.a.
  [4/1395] Compiling C object 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o'.
  FAILED: lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o 
  cc -Ilib/76b5a35@@igt-igt_amd_c@sta -Ilib -I../lib -I../include -I../include/drm-uapi -I../include/linux-uapi -I../lib/stubs/syscalls -I. -I../ -I../lib/stubs/libunwind -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/valgrind -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fPIC -pthread '-DIGT_DATADIR="/usr/local/share/igt-gpu-tools"' '-DIGT_SRCDIR="/builds/gfx-ci/igt-ci-tags/tests"' '-DIGT_LOG_DOMAIN="igt_amd"' -MD -MQ 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -MF 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o.d' -o 'lib/76b5a35@@igt-igt_amd_c@sta/igt_amd.c.o' -c ../lib/igt_amd.c
  ../lib/igt_amd.c: In function ‘igt_amd_get_mall_status’:
  ../lib/igt_amd.c:1198:10: error: ‘return’ with a value, in function returning void [-Werror=return-type]
   1198 |   return false;
        |          ^~~~~
  ../lib/igt_amd.c:1189:6: note: declared here
   1189 | void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
        |      ^~~~~~~~~~~~~~~~~~~~~~~
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1692898951:step_script
  section_start:1692898951:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1692898959:cleanup_file_variables
  ERROR: Job failed: exit code 1
  

build:tests-fedora-oldest-meson has failed (https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/jobs/47999495):
  [2/1407] Linking static target lib/libigt-igt_kms_c.a.
  [3/1407] Linking static target lib/libigt-igt_fb_c.a.
  [4/1407] Compiling C object 'lib/lib@@igt-igt_amd_c@sta/igt_amd.c.o'.
  FAILED: lib/lib@@igt-igt_amd_c@sta/igt_amd.c.o 
  cc -Ilib/lib@@igt-igt_amd_c@sta -Ilib -I../lib -I../include -I../include/drm-uapi -I../include/linux-uapi -I../lib/stubs/syscalls -I. -I../ -I/usr/include/cairo -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/libdrm -I/usr/include/libdrm/nouveau -I/usr/include/valgrind -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=gnu11 -O2 -g -D_GNU_SOURCE -include config.h -D_FORTIFY_SOURCE=2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat=2 -Wimplicit-fallthrough=0 -Wlogical-op -Wmissing-declarations -Wmissing-format-attribute -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wuninitialized -Wunused -Wno-clobbered -Wno-maybe-uninitialized -Wno-missing-field-initializers -Wno-pointer-arith -Wno-address-of-packed-member -Wno-sign-compare -Wno-type-limits -Wno-unused-parameter -Wno-unused-result -Werror=address -Werror=array-bounds -Werror=implicit -Werror=init-self -Werror=int-to-pointer-cast -Werror=main -Werror=missing-braces -Werror=nonnull -Werror=pointer-to-int-cast -Werror=return-type -Werror=sequence-point -Werror=trigraphs -Werror=write-strings -fno-builtin-malloc -fno-builtin-calloc -fPIC -pthread '-DIGT_DATADIR="/usr/local/share/igt-gpu-tools"' '-DIGT_SRCDIR="/builds/gfx-ci/igt-ci-tags/tests"' '-DIGT_LOG_DOMAIN="igt_amd"'  -MD -MQ 'lib/lib@@igt-igt_amd_c@sta/igt_amd.c.o' -MF 'lib/lib@@igt-igt_amd_c@sta/igt_amd.c.o.d' -o 'lib/lib@@igt-igt_amd_c@sta/igt_amd.c.o' -c ../lib/igt_amd.c
  ../lib/igt_amd.c: In function ‘igt_amd_get_mall_status’:
  ../lib/igt_amd.c:1198:10: error: ‘return’ with a value, in function returning void [-Werror=return-type]
   1198 |   return false;
        |          ^~~~~
  ../lib/igt_amd.c:1189:6: note: declared here
   1189 | void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
        |      ^~~~~~~~~~~~~~~~~~~~~~~
  cc1: some warnings being treated as errors
  ninja: build stopped: subcommand failed.
  section_end:1692898960:step_script
  section_start:1692898960:cleanup_file_variables
  Cleaning up project directory and file based variables
  section_end:1692898961:cleanup_file_variables
  ERROR: Job failed: exit code 1

== Logs ==

For more details see: https://gitlab.freedesktop.org/gfx-ci/igt-ci-tags/-/pipelines/969821

Re: [igt-dev] [PATCH 2/3] tests/amdgpu/amd_mall: remove UMR dependency

[Alex Hung]

On 2023-08-24 10:15, Aurabindo Pillai wrote:
> Remove the dependency on the userspace tool UMR to check MALL status
> and fully rely on debugfs for simplicity of test setup.
> 
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> ---
>   lib/igt_amd.c           | 26 +++++++++++++-------------
>   lib/igt_amd.h           |  2 +-
>   tests/amdgpu/amd_mall.c | 31 +++++++------------------------
>   3 files changed, 21 insertions(+), 38 deletions(-)
> 
> diff --git a/lib/igt_amd.c b/lib/igt_amd.c
> index 1a720ff56..49ca9e6d9 100644
> --- a/lib/igt_amd.c
> +++ b/lib/igt_amd.c
> @@ -1182,27 +1182,27 @@ static bool get_dm_capabilites(int drm_fd, char *buf, size_t size) {
>    * @brief check if AMDGPU mall_capable interface entry exist and defined
>    *
>    * @param drm_fd DRM file descriptor
> - * @return true if mall_capable debugfs interface exists and defined
> - * @return false otherwise
> + * @return true if dm capabilities interface exists and MALL is supported
> + * @return false if capabilites could not be read.
>    */
> -bool igt_amd_is_mall_capable(int drm_fd)
> +void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
>   {
> -	char buf[1024], mall_read[10];
> +	char buf[1024];
>   	char *mall_loc;
>   
> -	if (!get_dm_capabilites(drm_fd, buf, 1024))
> -		return false;
> +	*supported = false;
> +	*enabled = false;

supported and enabled are u32 but assigned false and true. Why can't 
they be boolean?

>   
> -	mall_loc = strstr(buf,"mall: ");
> -	if (!mall_loc)
> +	if (!get_dm_capabilites(drm_fd, buf, 1024))
>   		return false;
>   
> -	sscanf(mall_loc, "mall: %s", mall_read);
> -
> -	if (!strcmp(mall_read, "yes"))
> -		return true;
> +	mall_loc = strstr(buf, "mall supported: yes");
> +	if (mall_loc)
> +		*supported = true;
>   
> -	return false;
> +	mall_loc = strstr(buf, "enabled: yes");
> +	if (mall_loc && *supported)
> +		*enabled = true;
>   }
>   
>   /**
> diff --git a/lib/igt_amd.h b/lib/igt_amd.h
> index c05b4b730..db226e0d0 100644
> --- a/lib/igt_amd.h
> +++ b/lib/igt_amd.h
> @@ -199,6 +199,6 @@ bool igt_amd_has_visual_confirm(int drm_fd);
>   int  igt_amd_get_visual_confirm(int drm_fd);
>   bool igt_amd_set_visual_confirm(int drm_fd, enum amdgpu_debug_visual_confirm option);
>   
> -bool igt_amd_is_mall_capable(int drm_fd);
> +void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled);
>   bool igt_amd_output_has_odm_combine_segments(int drm_fd, char *connector_name);
>   #endif /* IGT_AMD_H */
> diff --git a/tests/amdgpu/amd_mall.c b/tests/amdgpu/amd_mall.c
> index 6016d5e8c..178203e8b 100644
> --- a/tests/amdgpu/amd_mall.c
> +++ b/tests/amdgpu/amd_mall.c
> @@ -57,7 +57,8 @@ struct line_check {
>   static void test_init(data_t *data)
>   {
>   	igt_display_t *display = &data->display;
> -	bool mall_capable = false;
> +	u32 mall_capable = false;
> +	u32 mall_en = false;

Same here. Why are boolean values assigned to u32?

>   
>   	/* It doesn't matter which pipe we choose on amdpgu. */
>   	data->pipe_id = PIPE_A;
> @@ -65,7 +66,7 @@ static void test_init(data_t *data)
>   
>   	igt_display_reset(display);
>   
> -	mall_capable =  igt_amd_is_mall_capable(data->fd);
> +	igt_amd_get_mall_status(data->fd, &mall_capable, &mall_en);
>   	igt_require_f(mall_capable, "Requires hardware that supports MALL cache\n");
>   
>   	/* find a connected output */
> @@ -101,44 +102,26 @@ static void test_fini(data_t *data)
>   	igt_display_commit_atomic(&data->display, DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
>   }
>   
> -static bool check_cmd_output(const char *line, void *data)
> -{
> -	struct line_check *check = data;
> -
> -	if (strstr(line, check->substr)) {
> -		check->found++;
> -	}
> -
> -	return false;
> -}
>   static void test_mall_ss(data_t *data)
>   {
>   	igt_display_t *display = &data->display;
>   	igt_fb_t rfb;
> -	int exec_ret;
> -	struct line_check line = {0};
>   	igt_crc_t test_crc, ref_crc;
> +	u32 mall_supp, mall_en;
>   
>   	test_init(data);
>   
>   	igt_create_pattern_fb(data->fd, data->w, data->h, DRM_FORMAT_XRGB8888, 0, &rfb);
>   	igt_plane_set_fb(data->primary, &rfb);
>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> -
>   	igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
> -	sleep(MALL_SETTLE_DELAY);
> -
> -	igt_system_cmd(exec_ret, "umr -O bits -r *.*.HUBP0_HUBP_MALL_STATUS | grep MALL_IN_USE");
> -
> -	igt_skip_on_f(exec_ret != IGT_EXIT_SUCCESS, "Error running UMR\n");
>   
> -	line.substr = "1 (0x00000001)";
> -	igt_log_buffer_inspect(check_cmd_output, &line);
> +	sleep(MALL_SETTLE_DELAY);
>   
> -	igt_assert_eq(line.found, 1);
> +	igt_amd_get_mall_status(data->fd, &mall_supp, &mall_en);
> +	igt_fail_on_f(!(mall_supp && mall_en), "MALL did not get enabled\n");
>   
>   	igt_pipe_crc_collect_crc(data->pipe_crc, &test_crc);
> -
>   	igt_assert_crc_equal(&ref_crc, &test_crc);
>   
>   	igt_remove_fb(data->fd, &rfb);

Re: [igt-dev] [PATCH 2/3] tests/amdgpu/amd_mall: remove UMR dependency

[Aurabindo Pillai]

On 2023-08-24 23:49, Alex Hung wrote:
> 
> 
> On 2023-08-24 10:15, Aurabindo Pillai wrote:
>> Remove the dependency on the userspace tool UMR to check MALL status
>> and fully rely on debugfs for simplicity of test setup.
>>
>> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
>> ---
>>   lib/igt_amd.c           | 26 +++++++++++++-------------
>>   lib/igt_amd.h           |  2 +-
>>   tests/amdgpu/amd_mall.c | 31 +++++++------------------------
>>   3 files changed, 21 insertions(+), 38 deletions(-)
>>
>> diff --git a/lib/igt_amd.c b/lib/igt_amd.c
>> index 1a720ff56..49ca9e6d9 100644
>> --- a/lib/igt_amd.c
>> +++ b/lib/igt_amd.c
>> @@ -1182,27 +1182,27 @@ static bool get_dm_capabilites(int drm_fd, 
>> char *buf, size_t size) {
>>    * @brief check if AMDGPU mall_capable interface entry exist and 
>> defined
>>    *
>>    * @param drm_fd DRM file descriptor
>> - * @return true if mall_capable debugfs interface exists and defined
>> - * @return false otherwise
>> + * @return true if dm capabilities interface exists and MALL is 
>> supported
>> + * @return false if capabilites could not be read.
>>    */
>> -bool igt_amd_is_mall_capable(int drm_fd)
>> +void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled)
>>   {
>> -    char buf[1024], mall_read[10];
>> +    char buf[1024];
>>       char *mall_loc;
>> -    if (!get_dm_capabilites(drm_fd, buf, 1024))
>> -        return false;
>> +    *supported = false;
>> +    *enabled = false;
> 
> supported and enabled are u32 but assigned false and true. Why can't 
> they be boolean?

In driver we use u32* for reading any reg fields of sizes less than u32 
as well, but there is no need to follow that in IGT. Just sent an 
updated v2

> 
>> -    mall_loc = strstr(buf,"mall: ");
>> -    if (!mall_loc)
>> +    if (!get_dm_capabilites(drm_fd, buf, 1024))
>>           return false;
>> -    sscanf(mall_loc, "mall: %s", mall_read);
>> -
>> -    if (!strcmp(mall_read, "yes"))
>> -        return true;
>> +    mall_loc = strstr(buf, "mall supported: yes");
>> +    if (mall_loc)
>> +        *supported = true;
>> -    return false;
>> +    mall_loc = strstr(buf, "enabled: yes");
>> +    if (mall_loc && *supported)
>> +        *enabled = true;
>>   }
>>   /**
>> diff --git a/lib/igt_amd.h b/lib/igt_amd.h
>> index c05b4b730..db226e0d0 100644
>> --- a/lib/igt_amd.h
>> +++ b/lib/igt_amd.h
>> @@ -199,6 +199,6 @@ bool igt_amd_has_visual_confirm(int drm_fd);
>>   int  igt_amd_get_visual_confirm(int drm_fd);
>>   bool igt_amd_set_visual_confirm(int drm_fd, enum 
>> amdgpu_debug_visual_confirm option);
>> -bool igt_amd_is_mall_capable(int drm_fd);
>> +void igt_amd_get_mall_status(int drm_fd, u32 *supported, u32 *enabled);
>>   bool igt_amd_output_has_odm_combine_segments(int drm_fd, char 
>> *connector_name);
>>   #endif /* IGT_AMD_H */
>> diff --git a/tests/amdgpu/amd_mall.c b/tests/amdgpu/amd_mall.c
>> index 6016d5e8c..178203e8b 100644
>> --- a/tests/amdgpu/amd_mall.c
>> +++ b/tests/amdgpu/amd_mall.c
>> @@ -57,7 +57,8 @@ struct line_check {
>>   static void test_init(data_t *data)
>>   {
>>       igt_display_t *display = &data->display;
>> -    bool mall_capable = false;
>> +    u32 mall_capable = false;
>> +    u32 mall_en = false;
> 
> Same here. Why are boolean values assigned to u32?
> 
>>       /* It doesn't matter which pipe we choose on amdpgu. */
>>       data->pipe_id = PIPE_A;
>> @@ -65,7 +66,7 @@ static void test_init(data_t *data)
>>       igt_display_reset(display);
>> -    mall_capable =  igt_amd_is_mall_capable(data->fd);
>> +    igt_amd_get_mall_status(data->fd, &mall_capable, &mall_en);
>>       igt_require_f(mall_capable, "Requires hardware that supports 
>> MALL cache\n");
>>       /* find a connected output */
>> @@ -101,44 +102,26 @@ static void test_fini(data_t *data)
>>       igt_display_commit_atomic(&data->display, 
>> DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
>>   }
>> -static bool check_cmd_output(const char *line, void *data)
>> -{
>> -    struct line_check *check = data;
>> -
>> -    if (strstr(line, check->substr)) {
>> -        check->found++;
>> -    }
>> -
>> -    return false;
>> -}
>>   static void test_mall_ss(data_t *data)
>>   {
>>       igt_display_t *display = &data->display;
>>       igt_fb_t rfb;
>> -    int exec_ret;
>> -    struct line_check line = {0};
>>       igt_crc_t test_crc, ref_crc;
>> +    u32 mall_supp, mall_en;
>>       test_init(data);
>>       igt_create_pattern_fb(data->fd, data->w, data->h, 
>> DRM_FORMAT_XRGB8888, 0, &rfb);
>>       igt_plane_set_fb(data->primary, &rfb);
>>       igt_display_commit_atomic(display, 
>> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
>> -
>>       igt_pipe_crc_collect_crc(data->pipe_crc, &ref_crc);
>> -    sleep(MALL_SETTLE_DELAY);
>> -
>> -    igt_system_cmd(exec_ret, "umr -O bits -r 
>> *.*.HUBP0_HUBP_MALL_STATUS | grep MALL_IN_USE");
>> -
>> -    igt_skip_on_f(exec_ret != IGT_EXIT_SUCCESS, "Error running UMR\n");
>> -    line.substr = "1 (0x00000001)";
>> -    igt_log_buffer_inspect(check_cmd_output, &line);
>> +    sleep(MALL_SETTLE_DELAY);
>> -    igt_assert_eq(line.found, 1);
>> +    igt_amd_get_mall_status(data->fd, &mall_supp, &mall_en);
>> +    igt_fail_on_f(!(mall_supp && mall_en), "MALL did not get 
>> enabled\n");
>>       igt_pipe_crc_collect_crc(data->pipe_crc, &test_crc);
>> -
>>       igt_assert_crc_equal(&ref_crc, &test_crc);
>>       igt_remove_fb(data->fd, &rfb);

-- 
--

Thanks & Regards,
Jay