[PATCH 0/8] i-g-t: vebox test case

[PATCH 0/8] i-g-t: vebox test case

[Zhong Li]

Add vebox test cases for intel-gfx-tool

Xiang, Haihao (3):
  gem_ring_sync_loop: check the rings supported by the kernel
  gem_ring_sync_loop: test the new ring
  tests: storedw on VEBOX

Zhong Li (5):
  gem_cs_tlb.c: add vebox test case
  gem_exec_nop.c: add vebox test case
  gem_non_secure_batch.c:add bsd and blt ring test case
  gem_non_secure_batch.c add vebox ring test case
  gem_ring_sync_loop.c: fix an operator error

 lib/intel_chipset.h            |    2 +
 tests/Makefile.am              |    1 +
 tests/gem_cs_tlb.c             |    6 +-
 tests/gem_exec_nop.c           |    5 +-
 tests/gem_non_secure_batch.c   |   27 +++++--
 tests/gem_ring_sync_loop.c     |   49 ++++++++++++-
 tests/gem_storedw_loop_vebox.c |  153 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 234 insertions(+), 9 deletions(-)
 create mode 100644 tests/gem_storedw_loop_vebox.c

-- 
1.7.9.5

[PATCH 1/8] gem_ring_sync_loop: check the rings supported by the kernel

[Zhong Li]

From: "Xiang, Haihao" <haihao.xiang@intel.com>

Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 tests/gem_ring_sync_loop.c |   37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
index b689bcd..2875cf3 100644
--- a/tests/gem_ring_sync_loop.c
+++ b/tests/gem_ring_sync_loop.c
@@ -55,15 +55,46 @@ static drm_intel_bo *target_buffer;
 #define MI_COND_BATCH_BUFFER_END	(0x36<<23 | 1)
 #define MI_DO_COMPARE			(1<<21)
 
+static int
+get_num_rings(int fd)
+{
+	int num_rings = 1;	/* render ring is always available */
+	drm_i915_getparam_t gp;
+	int ret, tmp;
+
+	memset(&gp, 0, sizeof(gp));
+	gp.value = &tmp;
+
+	gp.param = I915_PARAM_HAS_BSD;
+	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+	
+	if ((ret == 0) & (*gp.value > 0))
+		num_rings++;
+	else
+		goto skip;
+	
+	gp.param = I915_PARAM_HAS_BLT;
+	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+
+	if ((ret == 0) & (*gp.value > 0))
+		num_rings++;
+	else
+		goto skip;
+
+skip:	
+	return num_rings;
+}
+
 static void
-store_dword_loop(void)
+store_dword_loop(int fd)
 {
 	int i;
+	int num_rings = get_num_rings(fd);
 
 	srandom(0xdeadbeef);
 
 	for (i = 0; i < 0x100000; i++) {
-		int ring = random() % 3 + 1;
+		int ring = random() % num_rings + 1;
 
 		if (ring == I915_EXEC_RENDER) {
 			BEGIN_BATCH(4);
@@ -127,7 +158,7 @@ int main(int argc, char **argv)
 		exit(-1);
 	}
 
-	store_dword_loop();
+	store_dword_loop(fd);
 
 	drm_intel_bo_unreference(target_buffer);
 	intel_batchbuffer_free(batch);
-- 
1.7.9.5

[PATCH 2/8] gem_ring_sync_loop: test the new ring

[Zhong Li]

From: "Xiang, Haihao" <haihao.xiang@intel.com>

The code is surround by a #ifdef...#endif to avoid to break compiling against
the current libdrm release

Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 tests/gem_ring_sync_loop.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
index 2875cf3..955bf34 100644
--- a/tests/gem_ring_sync_loop.c
+++ b/tests/gem_ring_sync_loop.c
@@ -81,6 +81,18 @@ get_num_rings(int fd)
 	else
 		goto skip;
 
+#ifdef I915_PARAM_HAS_VEBOX /* remove it once the upstream libdrm support VEBOX */
+
+	gp.param = I915_PARAM_HAS_VEBOX;
+	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
+
+	if ((ret == 0) & (*gp.value > 0))
+		num_rings++;
+	else
+		goto skip;
+
+#endif
+
 skip:	
 	return num_rings;
 }
-- 
1.7.9.5

[PATCH 3/8] tests: storedw on VEBOX

[Zhong Li]

From: "Xiang, Haihao" <haihao.xiang@intel.com>

Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 lib/intel_chipset.h            |    2 +
 tests/Makefile.am              |    1 +
 tests/gem_storedw_loop_vebox.c |  153 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)
 create mode 100644 tests/gem_storedw_loop_vebox.c

diff --git a/lib/intel_chipset.h b/lib/intel_chipset.h
index e027b32..0aca4aa 100755
--- a/lib/intel_chipset.h
+++ b/lib/intel_chipset.h
@@ -278,4 +278,6 @@
 #define IS_CRESTLINE(devid)	((devid) == PCI_CHIP_I965_GM || \
 				 (devid) == PCI_CHIP_I965_GME)
 
+#define HAS_VEBOX_RING(devid)   (IS_HASWELL(devid))
+
 #endif /* _INTEL_CHIPSET_H */
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f8758cd..9bb7b1d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -78,6 +78,7 @@ TESTS_progs = \
 	gem_storedw_loop_render \
 	gem_storedw_loop_blt \
 	gem_storedw_loop_bsd \
+	gem_storedw_loop_vebox \
 	gem_storedw_batches_loop \
 	gem_double_irq_loop \
 	gem_ring_sync_loop \
diff --git a/tests/gem_storedw_loop_vebox.c b/tests/gem_storedw_loop_vebox.c
new file mode 100644
index 0000000..3f06102
--- /dev/null
+++ b/tests/gem_storedw_loop_vebox.c
@@ -0,0 +1,153 @@
+/*
+ * Copyright © 2009 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.
+ *
+ * Authors:
+ *    Xiang, Haihao <haihao.xiang@intel.com>  
+ *    Eric Anholt <eric@anholt.net>
+ *    Jesse Barnes <jbarnes@virtuousgeek.org> (based on gem_bad_blit.c)
+ *
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <assert.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/time.h>
+#include "drm.h"
+#include "i915_drm.h"
+#include "drmtest.h"
+#include "intel_bufmgr.h"
+#include "intel_batchbuffer.h"
+#include "intel_gpu_tools.h"
+
+static drm_intel_bufmgr *bufmgr;
+struct intel_batchbuffer *batch;
+static drm_intel_bo *target_buffer;
+static int has_ppgtt = 0;
+
+/*
+ * Testcase: Basic vebox MI check using MI_STORE_DATA_IMM
+ */
+
+static void
+store_dword_loop(void)
+{
+	int cmd, i, val = 0;
+	uint32_t *buf;
+
+	cmd = MI_STORE_DWORD_IMM;
+	if (!has_ppgtt)
+		cmd |= MI_MEM_VIRTUAL;
+
+	for (i = 0; i < 0x100000; i++) {
+		BEGIN_BATCH(4);
+		OUT_BATCH(cmd);
+		OUT_BATCH(0); /* reserved */
+		OUT_RELOC(target_buffer, I915_GEM_DOMAIN_INSTRUCTION,
+			  I915_GEM_DOMAIN_INSTRUCTION, 0);
+		OUT_BATCH(val);
+		ADVANCE_BATCH();
+
+		intel_batchbuffer_flush_on_ring(batch, I915_EXEC_VEBOX);
+
+		drm_intel_bo_map(target_buffer, 0);
+
+		buf = target_buffer->virtual;
+		if (buf[0] != val) {
+			fprintf(stderr,
+				"value mismatch: cur 0x%08x, stored 0x%08x\n",
+				buf[0], val);
+			exit(-1);
+		}
+
+		drm_intel_bo_unmap(target_buffer);
+
+		val++;
+	}
+
+	drm_intel_bo_map(target_buffer, 0);
+	buf = target_buffer->virtual;
+
+	printf("completed %d writes successfully, current value: 0x%08x\n", i,
+			buf[0]);
+	drm_intel_bo_unmap(target_buffer);
+}
+
+int main(int argc, char **argv)
+{
+	int fd;
+	int devid;
+
+	if (argc != 1) {
+		fprintf(stderr, "usage: %s\n", argv[0]);
+		exit(-1);
+	}
+
+	fd = drm_open_any();
+	devid = intel_get_drm_devid(fd);
+
+	if (!HAS_VEBOX_RING(devid)) {
+		fprintf(stderr, "Doesn't have vebox ring\n");
+		return 77;
+	}
+
+	has_ppgtt = gem_uses_aliasing_ppgtt(fd);
+
+	/* This only works with ppgtt */
+	if (!has_ppgtt) {
+		fprintf(stderr, "no ppgtt detected, which is required\n");
+		return 77;
+	}
+
+	bufmgr = drm_intel_bufmgr_gem_init(fd, 4096);
+	if (!bufmgr) {
+		fprintf(stderr, "failed to init libdrm\n");
+		exit(-1);
+	}
+	drm_intel_bufmgr_gem_enable_reuse(bufmgr);
+
+	batch = intel_batchbuffer_alloc(bufmgr, devid);
+	if (!batch) {
+		fprintf(stderr, "failed to create batch buffer\n");
+		exit(-1);
+	}
+
+	target_buffer = drm_intel_bo_alloc(bufmgr, "target bo", 4096, 4096);
+	if (!target_buffer) {
+		fprintf(stderr, "failed to alloc target buffer\n");
+		exit(-1);
+	}
+
+	store_dword_loop();
+
+	drm_intel_bo_unreference(target_buffer);
+	intel_batchbuffer_free(batch);
+	drm_intel_bufmgr_destroy(bufmgr);
+
+	close(fd);
+
+	return 0;
+}
-- 
1.7.9.5

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

Re: [PATCH 3/8] tests: storedw on VEBOX

[Ben Widawsky]

On Tue, Apr 23, 2013 at 03:06:43PM +0800, Zhong Li wrote:
> From: "Xiang, Haihao" <haihao.xiang@intel.com>
> 
> Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
> Signed-off-by: Zhong Li <zhong.li@intel.com>

I've pushed up to here.


With a couple of corrections to gem_ring_sync_loop: test the new ring:

diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
index 955bf34..3607fca 100644
--- a/tests/gem_ring_sync_loop.c
+++ b/tests/gem_ring_sync_loop.c
@@ -55,6 +55,7 @@ static drm_intel_bo *target_buffer;
 #define MI_COND_BATCH_BUFFER_END       (0x36<<23 | 1)
 #define MI_DO_COMPARE                  (1<<21)
 
+#define LOCAL_I915_PARAM_HAS_VEBOX  22
 static int
 get_num_rings(int fd)
 {
@@ -67,12 +68,12 @@ get_num_rings(int fd)
 
        gp.param = I915_PARAM_HAS_BSD;
        ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
-       
+
        if ((ret == 0) & (*gp.value > 0))
                num_rings++;
        else
                goto skip;
-       
+
        gp.param = I915_PARAM_HAS_BLT;
        ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 
@@ -81,9 +82,7 @@ get_num_rings(int fd)
        else
                goto skip;
 
-#ifdef I915_PARAM_HAS_VEBOX /* remove it once the upstream libdrm support VEBOX */
-
-       gp.param = I915_PARAM_HAS_VEBOX;
+       gp.param = LOCAL_I915_PARAM_HAS_VEBOX;
        ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 
        if ((ret == 0) & (*gp.value > 0))
@@ -91,9 +90,8 @@ get_num_rings(int fd)
        else
                goto skip;
 
-#endif
 
-skip:  
+skip:
        return num_rings;
 }
 




And with a couple of corrections to this file:

diff --git a/tests/gem_storedw_loop_vebox.c b/tests/gem_storedw_loop_vebox.c
index 3f06102..4593c3f 100644
--- a/tests/gem_storedw_loop_vebox.c
+++ b/tests/gem_storedw_loop_vebox.c
@@ -1,5 +1,5 @@
 /*
- * Copyright � 2009 Intel Corporation
+ * Copyright � 2012 Intel Corporation
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -21,9 +21,7 @@
  * IN THE SOFTWARE.
  *
  * Authors:
- *    Xiang, Haihao <haihao.xiang@intel.com>  
- *    Eric Anholt <eric@anholt.net>
- *    Jesse Barnes <jbarnes@virtuousgeek.org> (based on gem_bad_blit.c)
+ *    Xiang, Haihao <haihao.xiang@intel.com> (based on gem_store_dw_loop_*)
  *
  */
 
@@ -48,6 +46,8 @@ struct intel_batchbuffer *batch;
 static drm_intel_bo *target_buffer;
 static int has_ppgtt = 0;
 
+#define LOCAL_I915_EXEC_VEBOX (4<<0)
+
 /*
  * Testcase: Basic vebox MI check using MI_STORE_DATA_IMM
  */
@@ -71,7 +71,7 @@ store_dword_loop(void)
                OUT_BATCH(val);
                ADVANCE_BATCH();
 
-               intel_batchbuffer_flush_on_ring(batch, I915_EXEC_VEBOX);
+               intel_batchbuffer_flush_on_ring(batch, LOCAL_I915_EXEC_VEBOX);



On top of this, I've pushed updates to make the test look more like
modern tests.

[snip]
-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/8] gem_cs_tlb.c: add vebox test case

[Zhong Li]

Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 tests/gem_cs_tlb.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/gem_cs_tlb.c b/tests/gem_cs_tlb.c
index 9f49ac9..6c190d7 100644
--- a/tests/gem_cs_tlb.c
+++ b/tests/gem_cs_tlb.c
@@ -172,7 +172,11 @@ int main(int argc, char **argv)
 	if (drmtest_run_subtest("blt"))
 		if (HAS_BLT_RING(devid))
 			run_on_ring(fd, I915_EXEC_BLT, "blt");
-
+	
+	if (drmtest_run_subtest("vebox"))
+		if (HAS_BLT_RING(devid))
+			run_on_ring(fd, I915_EXEC_VEBOX, "vebox");   
+	
 	close(fd);
 
 	return skipped_all ? 77 : 0;
-- 
1.7.9.5

Re: [PATCH 4/8] gem_cs_tlb.c: add vebox test case

[Ben Widawsky]

On Tue, Apr 23, 2013 at 03:06:44PM +0800, Zhong Li wrote:
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  tests/gem_cs_tlb.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_cs_tlb.c b/tests/gem_cs_tlb.c
> index 9f49ac9..6c190d7 100644
> --- a/tests/gem_cs_tlb.c
> +++ b/tests/gem_cs_tlb.c
> @@ -172,7 +172,11 @@ int main(int argc, char **argv)
>  	if (drmtest_run_subtest("blt"))
>  		if (HAS_BLT_RING(devid))
>  			run_on_ring(fd, I915_EXEC_BLT, "blt");
> -
> +	
> +	if (drmtest_run_subtest("vebox"))
> +		if (HAS_BLT_RING(devid))
> +			run_on_ring(fd, I915_EXEC_VEBOX, "vebox");   
> +	
>  	close(fd);
>  
>  	return skipped_all ? 77 : 0;
> -- 
> 1.7.9.5
> 


Please watch your whitespace, and s/HAS_BLT_RING/HAS_VEBOX_RING/

I've fixed both locally.

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 4/8] gem_cs_tlb.c: add vebox test case

[Ben Widawsky]

On Tue, Apr 23, 2013 at 03:06:44PM +0800, Zhong Li wrote:
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  tests/gem_cs_tlb.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_cs_tlb.c b/tests/gem_cs_tlb.c
> index 9f49ac9..6c190d7 100644
> --- a/tests/gem_cs_tlb.c
> +++ b/tests/gem_cs_tlb.c
> @@ -172,7 +172,11 @@ int main(int argc, char **argv)
>  	if (drmtest_run_subtest("blt"))
>  		if (HAS_BLT_RING(devid))
>  			run_on_ring(fd, I915_EXEC_BLT, "blt");
> -
> +	
> +	if (drmtest_run_subtest("vebox"))
> +		if (HAS_BLT_RING(devid))
> +			run_on_ring(fd, I915_EXEC_VEBOX, "vebox");   
> +	
>  	close(fd);
>  
>  	return skipped_all ? 77 : 0;
> -- 
> 1.7.9.5
> 

Oh, also, s/I915_EXEC_VEBOX/LOCAL_I915_EXEC_VEBOX so it can build with
older libdrm.

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 5/8] gem_exec_nop.c: add vebox test case

[Zhong Li]

Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 tests/gem_exec_nop.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
index 8608592..d0fa5d5 100644
--- a/tests/gem_exec_nop.c
+++ b/tests/gem_exec_nop.c
@@ -133,7 +133,10 @@ int main(int argc, char **argv)
 	if (drmtest_run_subtest("blt"))
 		if (HAS_BLT_RING(devid))
 			loop(fd, handle, I915_EXEC_BLT, "blt");
-
+	
+	if (drmtest_run_subtest("vebox"))
+		if (HAS_BLT_RING(devid))
+			loop(fd, handle, I915_EXEC_VEBOX, "vebox");
 
 	gem_close(fd, handle);
 
-- 
1.7.9.5

Re: [PATCH 5/8] gem_exec_nop.c: add vebox test case

[Ben Widawsky]

On Tue, Apr 23, 2013 at 03:06:45PM +0800, Zhong Li wrote:
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  tests/gem_exec_nop.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c
> index 8608592..d0fa5d5 100644
> --- a/tests/gem_exec_nop.c
> +++ b/tests/gem_exec_nop.c
> @@ -133,7 +133,10 @@ int main(int argc, char **argv)
>  	if (drmtest_run_subtest("blt"))
>  		if (HAS_BLT_RING(devid))
>  			loop(fd, handle, I915_EXEC_BLT, "blt");
> -
> +	
> +	if (drmtest_run_subtest("vebox"))
> +		if (HAS_BLT_RING(devid))
> +			loop(fd, handle, I915_EXEC_VEBOX, "vebox");
>  
>  	gem_close(fd, handle);
>  
> -- 
> 1.7.9.5
> 

Patch has the same problems as the previous ones. Fixed locally.

I don't mind so much that you made copy/paste mistakes since I do that
ALL the time too. The whitespace mistakes however should be part of your
editor setup. Please try to correct that ASAP.

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 5/8] gem_exec_nop.c: add vebox test case

[Li, Zhong]

Sorry for the copy error. In fact, I was going to send out patches which fix it.
Thanks

-----Original Message-----
From: Ben Widawsky [mailto:benjamin.widawsky@intel.com] 
Sent: Sunday, April 28, 2013 2:46 AM
To: Li, Zhong
Cc: intel-gfx@lists.freedesktop.org; Xiang, Haihao; Vetter, Daniel
Subject: Re: [PATCH 5/8] gem_exec_nop.c: add vebox test case

On Tue, Apr 23, 2013 at 03:06:45PM +0800, Zhong Li wrote:
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  tests/gem_exec_nop.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/gem_exec_nop.c b/tests/gem_exec_nop.c index 
> 8608592..d0fa5d5 100644
> --- a/tests/gem_exec_nop.c
> +++ b/tests/gem_exec_nop.c
> @@ -133,7 +133,10 @@ int main(int argc, char **argv)
>  	if (drmtest_run_subtest("blt"))
>  		if (HAS_BLT_RING(devid))
>  			loop(fd, handle, I915_EXEC_BLT, "blt");
> -
> +	
> +	if (drmtest_run_subtest("vebox"))
> +		if (HAS_BLT_RING(devid))
> +			loop(fd, handle, I915_EXEC_VEBOX, "vebox");
>  
>  	gem_close(fd, handle);
>  
> --
> 1.7.9.5
> 

Patch has the same problems as the previous ones. Fixed locally.

I don't mind so much that you made copy/paste mistakes since I do that
ALL the time too. The whitespace mistakes however should be part of your
editor setup. Please try to correct that ASAP.

-- 
Ben Widawsky, Intel Open Source Technology Center

[PATCH 6/8] gem_non_secure_batch.c:add bsd and blt ring test case

[Zhong Li]

Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 tests/gem_non_secure_batch.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tests/gem_non_secure_batch.c b/tests/gem_non_secure_batch.c
index 9148b00..5d743c4 100644
--- a/tests/gem_non_secure_batch.c
+++ b/tests/gem_non_secure_batch.c
@@ -48,7 +48,7 @@ struct intel_batchbuffer *batch;
 /*
  * Testcase: Basic check of non-secure batches
  *
- * This test tries to stop the render ring with a MI_LOAD_REG command, which
+ * This test tries to stop the Render/BSD/BLT/VEBOX ring (if exist) with a MI_LOAD_REG command, which
  * should fail if the non-secure handling works correctly.
  */
 
@@ -57,7 +57,7 @@ struct intel_batchbuffer *batch;
 static int num_rings = 1;
 
 static void
-mi_lri_loop(void)
+mi_lri_loop(unsigned int ring_buffer_addr)
 {
 	int i;
 
@@ -68,7 +68,7 @@ mi_lri_loop(void)
 
 		BEGIN_BATCH(4);
 		OUT_BATCH(MI_LOAD_REGISTER_IMM | 1);
-		OUT_BATCH(0x203c); /* RENDER RING CTL */
+		OUT_BATCH(ring_buffer_addr); /*RING CTL */
 		OUT_BATCH(0); /* try to stop the ring */
 		OUT_BATCH(MI_NOOP);
 		ADVANCE_BATCH();
@@ -96,6 +96,8 @@ int main(int argc, char **argv)
 	if (HAS_BLT_RING(devid))
 		num_rings++;
 
+	if (HAS_VEBOX_RING(devid))
+		num_rings++;
 
 	printf("num rings detected: %i\n", num_rings);
 
@@ -112,7 +114,20 @@ int main(int argc, char **argv)
 		exit(-1);
 	}
 
-	mi_lri_loop();
+	printf("try to stop render ring\n");
+	mi_lri_loop(0x203c);
+
+	if (HAS_BSD_RING(devid)){
+		printf("try to stop bsd ring\n");
+		mi_lri_loop(0x1203C);
+	}
+
+	if (HAS_BLT_RING(devid)){
+		printf("try to stop blt ring\n");
+		mi_lri_loop(0x2203C);
+	}
+
+
 	gem_quiescent_gpu(fd);
 
 	intel_batchbuffer_free(batch);
-- 
1.7.9.5

[PATCH 7/8] gem_non_secure_batch.c add vebox ring test case

[Zhong Li]

Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 tests/gem_non_secure_batch.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/gem_non_secure_batch.c b/tests/gem_non_secure_batch.c
index 5d743c4..bdb3e0b 100644
--- a/tests/gem_non_secure_batch.c
+++ b/tests/gem_non_secure_batch.c
@@ -127,6 +127,10 @@ int main(int argc, char **argv)
 		mi_lri_loop(0x2203C);
 	}
 
+	if (HAS_VEBOX_RING(devid)){ 
+		printf("try to stop vebox ring\n"); 
+		mi_lri_loop(0x1A03C); 
+	} 
 
 	gem_quiescent_gpu(fd);
 
-- 
1.7.9.5

[PATCH 8/8] gem_ring_sync_loop.c: fix an operator error

[Zhong Li]

Signed-off-by: Zhong Li <zhong.li@intel.com>
---
 tests/gem_ring_sync_loop.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
index 955bf34..cb79e7c 100644
--- a/tests/gem_ring_sync_loop.c
+++ b/tests/gem_ring_sync_loop.c
@@ -68,7 +68,7 @@ get_num_rings(int fd)
 	gp.param = I915_PARAM_HAS_BSD;
 	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 	
-	if ((ret == 0) & (*gp.value > 0))
+	if ((ret == 0) && (*gp.value > 0))
 		num_rings++;
 	else
 		goto skip;
@@ -76,7 +76,7 @@ get_num_rings(int fd)
 	gp.param = I915_PARAM_HAS_BLT;
 	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 
-	if ((ret == 0) & (*gp.value > 0))
+	if ((ret == 0) && (*gp.value > 0))
 		num_rings++;
 	else
 		goto skip;
@@ -86,7 +86,7 @@ get_num_rings(int fd)
 	gp.param = I915_PARAM_HAS_VEBOX;
 	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
 
-	if ((ret == 0) & (*gp.value > 0))
+	if ((ret == 0) && (*gp.value > 0))
 		num_rings++;
 	else
 		goto skip;
-- 
1.7.9.5

Re: [PATCH 8/8] gem_ring_sync_loop.c: fix an operator error

[Ben Widawsky]

On Tue, Apr 23, 2013 at 03:06:48PM +0800, Zhong Li wrote:
> Signed-off-by: Zhong Li <zhong.li@intel.com>
> ---
>  tests/gem_ring_sync_loop.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/gem_ring_sync_loop.c b/tests/gem_ring_sync_loop.c
> index 955bf34..cb79e7c 100644
> --- a/tests/gem_ring_sync_loop.c
> +++ b/tests/gem_ring_sync_loop.c
> @@ -68,7 +68,7 @@ get_num_rings(int fd)
>  	gp.param = I915_PARAM_HAS_BSD;
>  	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
>  	
> -	if ((ret == 0) & (*gp.value > 0))
> +	if ((ret == 0) && (*gp.value > 0))
>  		num_rings++;
>  	else
>  		goto skip;
> @@ -76,7 +76,7 @@ get_num_rings(int fd)
>  	gp.param = I915_PARAM_HAS_BLT;
>  	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
>  
> -	if ((ret == 0) & (*gp.value > 0))
> +	if ((ret == 0) && (*gp.value > 0))
>  		num_rings++;
>  	else
>  		goto skip;
> @@ -86,7 +86,7 @@ get_num_rings(int fd)
>  	gp.param = I915_PARAM_HAS_VEBOX;
>  	ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp);
>  
> -	if ((ret == 0) & (*gp.value > 0))
> +	if ((ret == 0) && (*gp.value > 0))
>  		num_rings++;
>  	else
>  		goto skip;
> -- 
> 1.7.9.5
> 

So when I went through the series initially, I had this squashed into
patch 2. Unfortunately, I messed this up before I ended up pushing, so
now we have broken get_num_rings for several patches.

In the future when you submit a series like this, please add this kind
of fix directly into the patch it fixes.

Thanks.

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 0/8] i-g-t: vebox test case

[Ben Widawsky]

On Tue, Apr 23, 2013 at 03:06:40PM +0800, Zhong Li wrote:
> Add vebox test cases for intel-gfx-tool

I've modified and pushed everything in this series except for the secure
stuff. Daniel, now is the time to chime in if you want more tests.

[snip]
-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 0/8] i-g-t: vebox test case

[Daniel Vetter]

On Sat, Apr 27, 2013 at 9:01 PM, Ben Widawsky
<benjamin.widawsky@intel.com> wrote:
> On Tue, Apr 23, 2013 at 03:06:40PM +0800, Zhong Li wrote:
>> Add vebox test cases for intel-gfx-tool
>
> I've modified and pushed everything in this series except for the secure
> stuff. Daniel, now is the time to chime in if you want more tests.

gem_dummy_reloc_loop seems to miss vecs support still. This test was
pretty good at catching some of the interrrupt fail we've seen on
gen6+, so imo a useful one. dummy_reloc is pretty similar to
ring_sync_loop, so shouldn't be a big fuss to update.

Wrt the patches you've dropped, that test is disabled by default any,
so I don't think we need vecs support (or other rings fwiw) at all
there.

Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch