[PATCH i-g-t 0/7] Miscellaneous fixes and improvements

[PATCH i-g-t 0/7] Miscellaneous fixes and improvements

[Thomas Wood]

The following series includes various fixes and improvements to
intel-gpu-tools, including a new check for command line handling.

Thomas Wood (7):
  Fix uninitialised variable warnings
  lib: various documentation fixes
  lib: add common min and max macros
  quick_dump: fix distcheck
  configure: include the skylake quick_dump files in EXTRA_DIST
  drm_lib.sh: add standard command line options
  tests: add a check for command line handling

 benchmarks/gem_userptr_benchmark.c |  4 +--
 configure.ac                       |  2 +-
 lib/igt_aux.h                      |  4 +++
 lib/igt_core.h                     |  1 +
 lib/igt_debugfs.h                  |  4 +--
 lib/rendercopy_gen9.c              |  6 ++--
 tests/Makefile.sources             |  6 ++++
 tests/drm_lib.sh                   | 11 +++++++
 tests/eviction_common.c            |  2 --
 tests/gem_ppgtt.c                  |  6 ++--
 tests/gem_userptr_blits.c          | 18 +++++------
 tests/igt_command_line.sh          | 61 ++++++++++++++++++++++++++++++++++++++
 tests/kms_flip.c                   |  8 ++---
 tools/intel_display_poller.c       |  4 +--
 tools/intel_reg_dumper.c           |  8 ++---
 tools/quick_dump/Makefile.am       |  2 +-
 16 files changed, 111 insertions(+), 36 deletions(-)
 create mode 100755 tests/igt_command_line.sh

-- 
2.1.0

[PATCH i-g-t 1/7] Fix uninitialised variable warnings

[Thomas Wood]

Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 benchmarks/gem_userptr_benchmark.c |  4 ++--
 tests/gem_ppgtt.c                  |  6 +++---
 tests/gem_userptr_blits.c          | 16 ++++++++--------
 tools/intel_reg_dumper.c           |  8 ++++----
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/benchmarks/gem_userptr_benchmark.c b/benchmarks/gem_userptr_benchmark.c
index 4d7442b..148bcd3 100644
--- a/benchmarks/gem_userptr_benchmark.c
+++ b/benchmarks/gem_userptr_benchmark.c
@@ -137,7 +137,7 @@ static void free_handle_ptr(uint32_t handle)
 
 static uint32_t create_userptr_bo(int fd, int size)
 {
-	void *ptr;
+	void *ptr = NULL;
 	uint32_t handle;
 	int ret;
 
@@ -160,7 +160,7 @@ static void free_userptr_bo(int fd, uint32_t handle)
 static int has_userptr(int fd)
 {
 	uint32_t handle = 0;
-	void *ptr;
+	void *ptr = NULL;
 	uint32_t oldflags;
 	int ret;
 
diff --git a/tests/gem_ppgtt.c b/tests/gem_ppgtt.c
index 0ee4f5b..f0b5a9e 100644
--- a/tests/gem_ppgtt.c
+++ b/tests/gem_ppgtt.c
@@ -85,8 +85,8 @@ static void scratch_buf_fini(struct igt_buf *buf)
 static void fork_rcs_copy(int target, dri_bo **dst, int count, unsigned flags)
 #define CREATE_CONTEXT 0x1
 {
-	igt_render_copyfunc_t render_copy;
-	int devid;
+	igt_render_copyfunc_t render_copy = NULL;
+	int devid = 0;
 
 	for (int child = 0; child < count; child++) {
 		int fd = drm_open_any();
@@ -149,7 +149,7 @@ static void fork_rcs_copy(int target, dri_bo **dst, int count, unsigned flags)
 
 static void fork_bcs_copy(int target, dri_bo **dst, int count)
 {
-	int devid;
+	int devid = 0;
 
 	for (int child = 0; child < count; child++) {
 		drm_intel_bufmgr *bufmgr;
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
index a729f31..7b3b26f 100644
--- a/tests/gem_userptr_blits.c
+++ b/tests/gem_userptr_blits.c
@@ -446,7 +446,7 @@ check_gpu(int fd, uint32_t handle, uint32_t val)
 static int has_userptr(int fd)
 {
 	uint32_t handle = 0;
-	void *ptr;
+	void *ptr = NULL;
 	uint32_t oldflags;
 	int ret;
 
@@ -501,7 +501,7 @@ static int test_input_checking(int fd)
 static int test_access_control(int fd)
 {
 	igt_fork(child, 1) {
-		void *ptr;
+		void *ptr = NULL;
 		int ret;
 		uint32_t handle;
 
@@ -636,7 +636,7 @@ static int test_forbidden_ops(int fd)
 {
 	struct drm_i915_gem_pread gem_pread;
 	struct drm_i915_gem_pwrite gem_pwrite;
-	void *ptr;
+	void *ptr = NULL;
 	int ret;
 	uint32_t handle;
 	char buf[PAGE_SIZE];
@@ -807,7 +807,7 @@ static int test_dmabuf(void)
 
 static int test_usage_restrictions(int fd)
 {
-	void *ptr;
+	void *ptr = NULL;
 	int ret;
 	uint32_t handle;
 
@@ -838,7 +838,7 @@ static int test_create_destroy(int fd, int time)
 {
 	struct timespec start, now;
 	uint32_t handle;
-	void *ptr;
+	void *ptr = NULL;
 	int n;
 
 	igt_fork_signal_helper();
@@ -866,7 +866,7 @@ static int test_create_destroy(int fd, int time)
 
 static int test_coherency(int fd, int count)
 {
-	uint32_t *memory;
+	uint32_t *memory = NULL;
 	uint32_t *cpu, *cpu_val;
 	uint32_t *gpu, *gpu_val;
 	uint32_t start = 0;
@@ -1030,7 +1030,7 @@ static void test_major_evictions(int fd, int size, int count)
 
 static void test_overlap(int fd, int expected)
 {
-	char *ptr;
+	char *ptr = NULL;
 	int ret;
 	uint32_t handle, handle2;
 
@@ -1188,7 +1188,7 @@ static void test_stress_mm(int fd)
 	pthread_t t;
 	unsigned int loops = 100000;
 	uint32_t handle;
-	void *ptr;
+	void *ptr = NULL;
 	struct stress_thread_data stdata;
 
 	memset(&stdata, 0, sizeof(stdata));
diff --git a/tools/intel_reg_dumper.c b/tools/intel_reg_dumper.c
index 0b6d887..7ef2023 100644
--- a/tools/intel_reg_dumper.c
+++ b/tools/intel_reg_dumper.c
@@ -147,7 +147,7 @@ DEBUGSTRING(i830_debug_dspcntr)
 DEBUGSTRING(i830_debug_pipeconf)
 {
 	const char *enabled = val & PIPEACONF_ENABLE ? "enabled" : "disabled";
-	const char *bit30, *interlace;
+	const char *bit30, *interlace = NULL;
 
 	if (IS_965(devid))
 		bit30 = val & I965_PIPECONF_ACTIVE ? "active" : "inactive";
@@ -156,7 +156,7 @@ DEBUGSTRING(i830_debug_pipeconf)
 		    val & PIPEACONF_DOUBLE_WIDE ? "double-wide" : "single-wide";
 
 	if (HAS_PCH_SPLIT(devid)) {
-		const char *bpc, *rotation;
+		const char *bpc, *rotation = NULL;
 
 		switch ((val >> 21) & 7) {
 		case 0:
@@ -1833,7 +1833,7 @@ DEBUGSTRING(hsw_debug_sinterrupt)
 DEBUGSTRING(ilk_debug_blc_pwm_cpu_ctl2)
 {
 	int enable, blinking, granularity;
-	const char *pipe;
+	const char *pipe = NULL;
 
 	enable = (val >> 31) & 1;
 
@@ -1920,7 +1920,7 @@ DEBUGSTRING(hsw_debug_blc_misc_ctl)
 DEBUGSTRING(hsw_debug_util_pin_ctl)
 {
 	int enable, data, inverted_polarity;
-	const char *transcoder, *mode;
+	const char *transcoder = NULL, *mode;
 
 	enable = (val >> 31) & 1;
 
-- 
2.1.0

[PATCH i-g-t 2/7] lib: various documentation fixes

[Thomas Wood]

Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 lib/igt_core.h        | 1 +
 lib/igt_debugfs.h     | 4 ++--
 lib/rendercopy_gen9.c | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/igt_core.h b/lib/igt_core.h
index e42dc33..b8f6702 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -268,6 +268,7 @@ void igt_exit(void) __attribute__((noreturn));
  * igt_assert_cmpint:
  * @n1: first value
  * @cmp: compare operator
+ * @ncmp: negated version of @cmp
  * @n2: second value
  *
  * Fails (sub-)test if the condition is not met
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index adcf83b..8ac1539 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -146,7 +146,6 @@ int igt_open_forcewake_handle(void);
 
 /**
  * stop_ring_flags:
- *
  * @STOP_RING_NONE: Can be used to clear the pending stop (warning: hang might
  * be declared already). Returned by igt_get_stop_rings() if there is
  * no currently stopped rings.
@@ -157,7 +156,8 @@ int igt_open_forcewake_handle(void);
  * @STOP_RING_ALL: All rings
  * @STOP_RING_ALLOW_ERRORS: Driver will not omit expected DRM_ERRORS
  * @STOP_RING_ALLOW_BAN: Driver will use context ban policy
- * @STOP_RING_DEFAULT: STOP_RING_ALL | STOP_RING_ALLOW_ERRORS
+ * @STOP_RING_DEFAULTS: STOP_RING_ALL | STOP_RING_ALLOW_ERRORS
+ *
  * Enumeration of all supported flags for igt_set_stop_rings().
  *
  */
diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
index 92265c1..9ff4b3a 100644
--- a/lib/rendercopy_gen9.c
+++ b/lib/rendercopy_gen9.c
@@ -286,7 +286,7 @@ gen8_fill_ps(struct intel_batchbuffer *batch,
 	return offset;
 }
 
-/**
+/*
  * gen7_fill_vertex_buffer_data populate vertex buffer with data.
  *
  * The vertex buffer consists of 3 vertices to construct a RECTLIST. The 4th
@@ -327,7 +327,7 @@ gen7_fill_vertex_buffer_data(struct intel_batchbuffer *batch,
 	return offset;
 }
 
-/**
+/*
  * gen6_emit_vertex_elements - The vertex elements describe the contents of the
  * vertex buffer. We pack the vertex buffer in a semi weird way, conforming to
  * what gen6_rendercopy did. The most straightforward would be to store
@@ -384,7 +384,7 @@ gen6_emit_vertex_elements(struct intel_batchbuffer *batch) {
 		  GEN6_VFCOMPONENT_STORE_1_FLT << VE1_VFCOMPONENT_3_SHIFT);
 }
 
-/**
+/*
  * gen7_emit_vertex_buffer emit the vertex buffers command
  *
  * @batch
-- 
2.1.0

[PATCH i-g-t 3/7] lib: add common min and max macros

[Thomas Wood]

Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 lib/igt_aux.h                | 4 ++++
 tests/eviction_common.c      | 2 --
 tests/gem_userptr_blits.c    | 2 --
 tests/kms_flip.c             | 8 +++-----
 tools/intel_display_poller.c | 4 +---
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index d74bb8c..415614b 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -83,4 +83,8 @@ bool intel_check_memory(uint32_t count, uint32_t size, unsigned mode);
 #define CHECK_RAM 0x1
 #define CHECK_SWAP 0x2
 
+
+#define min(a, b) ((a) < (b) ? (a) : (b))
+#define max(a, b) ((a) < (b) ? (a) : (b))
+
 #endif /* IGT_AUX_H */
diff --git a/tests/eviction_common.c b/tests/eviction_common.c
index 6de68a2..03d8e8c 100644
--- a/tests/eviction_common.c
+++ b/tests/eviction_common.c
@@ -170,8 +170,6 @@ static int swapping_evictions(int fd, struct igt_eviction_test_ops *ops,
 	return 0;
 }
 
-#define min(a, b) ((a) < (b) ? (a) : (b))
-
 static int forking_evictions(int fd, struct igt_eviction_test_ops *ops,
 				int surface_size, int working_surfaces,
 				int trash_surfaces, unsigned flags)
diff --git a/tests/gem_userptr_blits.c b/tests/gem_userptr_blits.c
index 7b3b26f..376f59d 100644
--- a/tests/gem_userptr_blits.c
+++ b/tests/gem_userptr_blits.c
@@ -981,8 +981,6 @@ static int can_swap(void)
 	return 1;
 }
 
-#define min(a, b) ((a) < (b) ? (a) : (b))
-
 static void test_forking_evictions(int fd, int size, int count,
 			     unsigned flags)
 {
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 901f013..f6c25a2 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -84,8 +84,6 @@
 #define DRM_CAP_TIMESTAMP_MONOTONIC 6
 #endif
 
-#define max(a, b) ((a) > (b) ? (a) : (b))
-
 drmModeRes *resources;
 int drm_fd;
 static drm_intel_bufmgr *bufmgr;
@@ -925,7 +923,7 @@ static unsigned int run_test_step(struct test_output *o)
 	if (o->flags & TEST_PAN) {
 		int count = do_flip ?
 			o->flip_state.count : o->vblank_state.count;
-		int x_ofs = count * 10 > o->fb_width - o->kmode[0].hdisplay ? o->fb_width - o->kmode[0].hdisplay : count * 10;
+		int x_ofs = min(count * 10, o->fb_width - o->kmode[0].hdisplay);
 
 		/* Make sure DSPSURF changes value */
 		if (o->flags & TEST_HANG)
@@ -1396,7 +1394,7 @@ static int run_test(int duration, int flags)
 
 	igt_require(modes);
 	duration = duration * 1000 / modes;
-	duration = duration < 500 ? 500 : duration;
+	duration = max(500, duration);
 
 	/* Find any connected displays */
 	for (i = 0; i < resources->count_connectors; i++) {
@@ -1460,7 +1458,7 @@ static int run_pair(int duration, int flags)
 	 * configuration at all. So skip in that case. */
 	igt_require(modes);
 	duration = duration * 1000 / modes;
-	duration = duration < 500 ? 500 : duration;
+	duration = max(duration, 500);
 
 	/* Find a pair of connected displays */
 	for (i = 0; i < resources->count_connectors; i++) {
diff --git a/tools/intel_display_poller.c b/tools/intel_display_poller.c
index ebc594a..2eab6c4 100644
--- a/tools/intel_display_poller.c
+++ b/tools/intel_display_poller.c
@@ -36,9 +36,7 @@
 #include "intel_io.h"
 #include "igt_debugfs.h"
 #include "drmtest.h"
-
-#define min(a,b) ((a) < (b) ? (a) : (b))
-#define max(a,b) ((a) > (b) ? (a) : (b))
+#include "igt_aux.h"
 
 enum test {
 	TEST_INVALID,
-- 
2.1.0

[PATCH i-g-t 4/7] quick_dump: fix distcheck

[Thomas Wood]

Commit 711a17a (quick_dump: Move base_display.txt to indivual platforms)
renamed base_display.txt to common_display.txt, but didn't update
Makefile.am accordingly. Since common_display.txt is now included in the
platform files, it is automatically included in $(QUICK_DUMP_EXTRA_DIST)
and therefore just the reference to base_display.txt needs removing.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 tools/quick_dump/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/quick_dump/Makefile.am b/tools/quick_dump/Makefile.am
index b183af5..e7e1f62 100644
--- a/tools/quick_dump/Makefile.am
+++ b/tools/quick_dump/Makefile.am
@@ -23,7 +23,7 @@ all-local: I915ChipsetPython.la
 
 CLEANFILES = chipset_wrap_python.c chipset.py _chipset.so
 EXTRA_DIST = $(QUICK_DUMP_EXTRA_DIST) \
-	      base_display.txt base_interrupt.txt base_other.txt base_power.txt base_rings.txt \
+	      base_interrupt.txt base_other.txt base_power.txt base_rings.txt \
 	      quick_dump.py \
 	      reg_access.py \
 	      chipset.i chipset.py
-- 
2.1.0

[PATCH i-g-t 5/7] configure: include the skylake quick_dump files in EXTRA_DIST

[Thomas Wood]

Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 9cdd202..03e856d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -201,7 +201,7 @@ fi
 AM_CONDITIONAL(BUILD_TESTS, [test "x$BUILD_TESTS" = xyes])
 AC_DEFINE_UNQUOTED(TARGET_CPU_PLATFORM, ["$host_cpu"], [Target platform])
 
-files="broadwell cherryview haswell ivybridge sandybridge valleyview"
+files="broadwell cherryview haswell ivybridge sandybridge valleyview skylake"
 for file in $files; do
 	QUICK_DUMP_EXTRA_DIST="$QUICK_DUMP_EXTRA_DIST $file `tr '\n' ' ' < tools/quick_dump/$file`"
 done
-- 
2.1.0

[PATCH i-g-t 6/7] drm_lib.sh: add standard command line options

[Thomas Wood]

Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 tests/drm_lib.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tests/drm_lib.sh b/tests/drm_lib.sh
index d71e6ae..6a8a310 100755
--- a/tests/drm_lib.sh
+++ b/tests/drm_lib.sh
@@ -9,6 +9,17 @@ for arg in $@ ; do
 		--run-subtest)
 			exit 79
 			;;
+		--debug)
+			IGT_LOG_LEVEL=debug
+			;;
+		--help)
+			echo "Usage: `basename $0` [OPTIONS]"
+			echo "  --list-subtests"
+			echo "  --run-subtest <pattern>"
+			echo "  --debug"
+			echo "  --help"
+			exit 0
+			;;
 	esac
 done
 
-- 
2.1.0

[PATCH i-g-t 7/7] tests: add a check for command line handling

[Thomas Wood]

Check that command line handling works consistently across all tests.

Signed-off-by: Thomas Wood <thomas.wood@intel.com>
---
 tests/Makefile.sources    |  6 +++++
 tests/igt_command_line.sh | 61 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)
 create mode 100755 tests/igt_command_line.sh

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 89596e6..88bb854 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -202,8 +202,13 @@ TESTS_testsuite = \
 	igt_simulation \
 	$(NULL)
 
+TESTS_testsuite_scripts = \
+	igt_command_line.sh \
+	$(NULL)
+
 TESTS = \
 	$(TESTS_testsuite) \
+	$(TESTS_testsuite_scripts) \
 	$(NULL)
 
 # Test that exercise specific asserts in the test framework library and are
@@ -229,6 +234,7 @@ scripts = \
 	ddx_intel_after_fbdev \
 	debugfs_wedged \
 	drm_lib.sh \
+	igt_command_line.sh \
 	$(NULL)
 
 IMAGES = pass.png 1080p-left.png 1080p-right.png
diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh
new file mode 100755
index 0000000..075f1d5
--- /dev/null
+++ b/tests/igt_command_line.sh
@@ -0,0 +1,61 @@
+#!/bin/sh
+#
+# Copyright © 2014 Intel Corporation
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the "Software"),
+# to deal in the Software without restriction, including without limitation
+# the rights to use, copy, modify, merge, publish, distribute, sublicense,
+# and/or sell copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice (including the next
+# paragraph) shall be included in all copies or substantial portions of the
+# Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+# IN THE SOFTWARE.
+
+#
+# Check that command line handling works consistently across all tests
+#
+
+for test in `cat single-tests.txt multi-tests.txt`; do
+
+	if [ "$test" = "TESTLIST" -o "$test" = "END" ]; then
+		continue
+	fi
+
+	# if the test is a script, it will be in $srcdir
+	if [ ! -x $test ]; then
+		if [ -x $srcdir/$test ]; then
+			test=$srcdir/$test
+		fi
+	fi
+
+	echo "$test:"
+
+	# check invalid option handling
+	echo "  Checking invalid option handling..."
+	./$test --invalid-option 2> /dev/null && exit 99
+
+	# check valid options succeed
+	echo "  Checking valid option handling..."
+	./$test --help > /dev/null || exit 99
+
+	# check --list-subtests works correctly
+	echo "  Checking subtest enumeration..."
+	./$test --list-subtests > /dev/null
+	if [ $? -ne 0 -a $? -ne 79 ]; then
+		exit 99
+	fi
+
+	# check invalid subtest handling
+	echo "  Checking invalid subtest handling..."
+	./$test --run-subtest invalid-subtest > /dev/null 2>&1 && exit 99
+done
-- 
2.1.0

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

Re: [PATCH i-g-t 1/7] Fix uninitialised variable warnings

[Chris Wilson]

On Thu, Oct 16, 2014 at 05:49:47PM +0100, Thomas Wood wrote:
> Signed-off-by: Thomas Wood <thomas.wood@intel.com>

Is something wrong with your compiler? ;)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre