Re: [Intel-gfx] [PATCH i-g-t v2 4/7] tests/gem_scheduler: Add gem_scheduler test

2016-03-07 Thread Dave Gordon

On 02/03/16 20:15, Chris Wilson wrote:

+static struct ring {
+   const char *name;
+   int id;
+   bool exists;
+} rings[] = {
+   { "render", I915_EXEC_RENDER, false },
+   { "bsd1",I915_EXEC_BSD | 1<<13, false },
+   { "bsd2",I915_EXEC_BSD | 2<<13, false },


This is wrong. The timeline is coupled to the exec_id, which is the same
for "both" BSD engines. To fix that, properly split up the two rings
with separate ids and deprecate HAS_BSD2.


+   { "blt",I915_EXEC_BLT, false },
+   { "vebox",  I915_EXEC_VEBOX, false },
+};
+
+#define NBR_RINGS (sizeof(rings)/sizeof(struct ring))


Also see intel_execution_engines to save on duplicating code.
-Chris


exec_id is indeed the same for both BSD engines, but it is used ONLY in
i915_gem_busy_ioctl(), whose result can tell you that "a" BSD engine is 
busy but not which one -- probably callers don't care anyway.


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


Re: [Intel-gfx] [PATCH i-g-t v2 4/7] tests/gem_scheduler: Add gem_scheduler test

2016-03-04 Thread Morton, Derek J
I don't really understand your statement, can you elaborate please. What do you 
mean by timeline? Are you saying the driver needs changing? By deprecate 
HAS_BSD2 did you mean some change that would remove the need for 
gem_has_bsd2()? Doesn't I915_EXEC_BSD | 1<<13 and I915_EXEC_BSD | 2<<13 
constitute separate exec_id's?

//Derek

-Original Message-
From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] 
Sent: Wednesday, March 2, 2016 8:15 PM
To: Morton, Derek J 
Cc: intel-gfx@lists.freedesktop.org; daniel.vet...@ffwll.ch
Subject: Re: [Intel-gfx] [PATCH i-g-t v2 4/7] tests/gem_scheduler: Add 
gem_scheduler test

> +static struct ring {
> + const char *name;
> + int id;
> + bool exists;
> +} rings[] = {
> + { "render", I915_EXEC_RENDER, false },
> + { "bsd1",I915_EXEC_BSD | 1<<13, false },
> + { "bsd2",I915_EXEC_BSD | 2<<13, false },

This is wrong. The timeline is coupled to the exec_id, which is the same for 
"both" BSD engines. To fix that, properly split up the two rings with separate 
ids and deprecate HAS_BSD2.

> + { "blt",I915_EXEC_BLT, false },
> + { "vebox",  I915_EXEC_VEBOX, false }, };
> +
> +#define NBR_RINGS (sizeof(rings)/sizeof(struct ring))

Also see intel_execution_engines to save on duplicating code.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t v2 4/7] tests/gem_scheduler: Add gem_scheduler test

2016-03-02 Thread Chris Wilson
> +static struct ring {
> + const char *name;
> + int id;
> + bool exists;
> +} rings[] = {
> + { "render", I915_EXEC_RENDER, false },
> + { "bsd1",I915_EXEC_BSD | 1<<13, false },
> + { "bsd2",I915_EXEC_BSD | 2<<13, false },

This is wrong. The timeline is coupled to the exec_id, which is the same
for "both" BSD engines. To fix that, properly split up the two rings
with separate ids and deprecate HAS_BSD2.

> + { "blt",I915_EXEC_BLT, false },
> + { "vebox",  I915_EXEC_VEBOX, false },
> +};
> +
> +#define NBR_RINGS (sizeof(rings)/sizeof(struct ring))

Also see intel_execution_engines to save on duplicating code.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t v2 4/7] tests/gem_scheduler: Add gem_scheduler test

2016-03-02 Thread Derek Morton
This is intended to test the scheduler behaviour is correct.
The subtests are
-basic
Tests that batch buffers of the same priority submitted to a ring
execute in the order they are submitted.
-read
Submits a batch buffer with a read dependency to a buffer object to
a ring which is held in the scheduler queue by a long running batch
buffer. Submit batch buffers to other rings that have a read dependency
to the same buffer object. Ensure they execute before the batch buffer
being held up behind the long running batch buffer.
-write
Submits a batch buffer with a write dependency to a buffer object to
a ring which is held in the scheduler queue by a long running batch
buffer. Submit batch buffers to other rings that have a write dependency
to the same buffer object. Submit batch buffers with no interdependencies
to all rings. Ensure the batch buffers that have write dependencies are
executed in submission order but the batch buffers without interdependencies
do not get held up.

v2: Addressed review comments from Daniele Ceraolo Spurio

Signed-off-by: Derek Morton 
---
 tests/Makefile.sources |   1 +
 tests/gem_scheduler.c  | 451 +
 2 files changed, 452 insertions(+)
 create mode 100644 tests/gem_scheduler.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index f8b18b0..c88e045 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -66,6 +66,7 @@ TESTS_progs_M = \
gem_request_retire \
gem_reset_stats \
gem_ringfill \
+   gem_scheduler \
gem_set_tiling_vs_blt \
gem_softpin \
gem_stolen \
diff --git a/tests/gem_scheduler.c b/tests/gem_scheduler.c
new file mode 100644
index 000..762b69f
--- /dev/null
+++ b/tests/gem_scheduler.c
@@ -0,0 +1,451 @@
+/*
+ * Copyright © 2016 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:
+ *Derek Morton 
+ *
+ */
+
+#include "igt.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+IGT_TEST_DESCRIPTION("Check scheduler behaviour. Basic tests ensure 
independant "
+ "batch buffers of the same priority are executed in "
+ "submission order. Read-read tests ensure "
+ "batch buffers with a read dependency to the same buffer "
+ "object do not block each other. Write-write dependency "
+ "tests ensure batch buffers with a write dependency to a "
+ "buffer object will be executed in submission order but "
+ "will not block execution of other independant batch "
+ "buffers.");
+
+#define SEC_TO_NSEC (1000 * 1000 * 1000)
+
+static struct ring {
+   const char *name;
+   int id;
+   bool exists;
+} rings[] = {
+   { "render", I915_EXEC_RENDER, false },
+   { "bsd1",I915_EXEC_BSD | 1<<13, false },
+   { "bsd2",I915_EXEC_BSD | 2<<13, false },
+   { "blt",I915_EXEC_BLT, false },
+   { "vebox",  I915_EXEC_VEBOX, false },
+};
+
+#define NBR_RINGS (sizeof(rings)/sizeof(struct ring))
+
+static void check_rings(int fd) {
+   int loop;
+   for(loop=0; loop < NBR_RINGS; loop++) {
+   if(gem_has_ring(fd, rings[loop].id)) {
+   if(rings[loop].id == (I915_EXEC_BSD | 2<<13))
+   rings[loop].exists = gem_has_bsd2(fd);
+   else
+   rings[loop].exists = true;
+   }
+   }
+}
+
+static drm_intel_bo *create_and_check_bo(drm_intel_bufmgr *bufmgr, const char 
*desc)
+{
+   drm_intel_bo *bo = drm_intel_bo_alloc(bufmgr, desc, BATCH_SZ, BATCH_SZ);
+   igt_assert_f(bo, "Failed allocating %s\n", desc);
+   return bo;
+}
+
+static struct intel_batchbuffer *create_delay_bb(int fd, drm_intel_