All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf
@ 2015-03-25 14:21 Tvrtko Ursulin
  2015-03-25 14:28 ` Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-03-25 14:21 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.

It uses the fact DRM allocation policy is set as ABI and allocates
space in order. That means that we should be able to easily get
two bos mapped at adjacent GTT addresses and then test that the
pad to size flag will move them apart.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources       |   1 +
 tests/gem_exec_pad_to_size.c | 276 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 277 insertions(+)
 create mode 100644 tests/gem_exec_pad_to_size.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index a165978..e8b212a 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -33,6 +33,7 @@ TESTS_progs_M = \
 	gem_exec_bad_domains \
 	gem_exec_faulting_reloc \
 	gem_exec_nop \
+	gem_exec_pad_to_size \
 	gem_exec_params \
 	gem_exec_parse \
 	gem_fenced_exec_thrash \
diff --git a/tests/gem_exec_pad_to_size.c b/tests/gem_exec_pad_to_size.c
new file mode 100644
index 0000000..3584e5c
--- /dev/null
+++ b/tests/gem_exec_pad_to_size.c
@@ -0,0 +1,276 @@
+/*
+ * Copyright © 2015 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:
+ *    Tvrtko Ursulin <tvrtko.ursulin@intel.com>
+ *
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "igt_core.h"
+#include "drmtest.h"
+#include "intel_reg.h"
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+struct local_drm_i915_gem_exec_object2 {
+	/**
+	 * User's handle for a buffer to be bound into the GTT for this
+	 * operation.
+	 */
+	__u32 handle;
+
+	/** Number of relocations to be performed on this buffer */
+	__u32 relocation_count;
+	/**
+	 * Pointer to array of struct drm_i915_gem_relocation_entry containing
+	 * the relocations to be performed in this buffer.
+	 */
+	__u64 relocs_ptr;
+
+	/** Required alignment in graphics aperture */
+	__u64 alignment;
+
+	/**
+	 * Returned value of the updated offset of the object, for future
+	 * presumed_offset writes.
+	 */
+	__u64 offset;
+
+#define LOCAL_EXEC_OBJECT_NEEDS_FENCE (1<<0)
+#define LOCAL_EXEC_OBJECT_NEEDS_GTT	(1<<1)
+#define LOCAL_EXEC_OBJECT_WRITE	(1<<2)
+#define LOCAL_EXEC_OBJECT_PAD_TO_SIZE (1<<3)
+#define __LOCAL_EXEC_OBJECT_UNKNOWN_FLAGS -(LOCAL_EXEC_OBJECT_PAD_TO_SIZE<<1)
+	__u64 flags;
+
+	__u64 pad_to_size;
+	__u64 rsvd2;
+};
+
+static int
+exec(int fd, uint32_t handles[3], uint32_t pad_to_size[2], uint64_t *offsets)
+{
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct local_drm_i915_gem_exec_object2 gem_exec[3];
+	int ret = 0;
+
+	memset(gem_exec, 0, sizeof(gem_exec));
+
+	gem_exec[0].handle = handles[1];
+	gem_exec[0].relocation_count = 0;
+	gem_exec[0].relocs_ptr = 0;
+	gem_exec[0].alignment = 0;
+	gem_exec[0].offset = 0;
+	gem_exec[0].flags = pad_to_size[0] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[0].pad_to_size = pad_to_size[0];
+	gem_exec[0].rsvd2 = 0;
+
+	gem_exec[1].handle = handles[2];
+	gem_exec[1].relocation_count = 0;
+	gem_exec[1].relocs_ptr = 0;
+	gem_exec[1].alignment = 0;
+	gem_exec[1].offset = 0;
+	gem_exec[1].flags = pad_to_size[1] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[1].pad_to_size = pad_to_size[1];
+	gem_exec[1].rsvd2 = 0;
+
+	gem_exec[2].handle = handles[0];
+	gem_exec[2].relocation_count = 0;
+	gem_exec[2].relocs_ptr = 0;
+	gem_exec[2].alignment = 0;
+	gem_exec[2].offset = 0;
+	gem_exec[2].flags = 0;
+	gem_exec[2].pad_to_size = 0;
+	gem_exec[2].rsvd2 = 0;
+
+	execbuf.buffers_ptr = (uintptr_t)gem_exec;
+	execbuf.buffer_count = 3;
+	execbuf.batch_start_offset = 0;
+	execbuf.batch_len = 8;
+	execbuf.cliprects_ptr = 0;
+	execbuf.num_cliprects = 0;
+	execbuf.DR1 = 0;
+	execbuf.DR4 = 0;
+	execbuf.flags = I915_EXEC_RENDER;
+	i915_execbuffer2_set_context_id(execbuf, 0);
+	execbuf.rsvd2 = 0;
+
+	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
+			ret = -errno;
+
+	if (ret == 0) {
+		gem_sync(fd, handles[0]);
+
+		if (offsets) {
+			offsets[0] = gem_exec[0].offset;
+			offsets[1] = gem_exec[1].offset;
+		}
+	}
+
+	return ret;
+}
+
+static void
+require_pad_to_size(int fd, uint32_t handles[3])
+{
+	igt_require(exec(fd, handles, (uint32_t[2]){ PAGE_SIZE, PAGE_SIZE },
+			 NULL) == 0);
+}
+
+static void
+not_aligned(int fd, uint32_t handles[3])
+{
+	require_pad_to_size(fd, handles);
+
+	igt_require(exec(fd, handles, (uint32_t[2]){1 ,1},
+			 NULL) == -EINVAL);
+}
+
+static void
+padding(int fd, uint32_t handles[3])
+{
+	uint32_t pad_to_size[2] = {0, 0};
+	uint64_t offsets[2];
+	uint32_t distance;
+	bool neighbours = false;
+	unsigned int try, idx;
+	const unsigned int max_tries = 1024; /* Finger in the air. */
+	uint32_t *loc_handles;
+	uint32_t eb_handles[3];
+
+	require_pad_to_size(fd, handles);
+
+	loc_handles = calloc(max_tries * 2, sizeof(uint32_t));
+	igt_assert(loc_handles);
+
+	/* Try with passed in handles first. */
+	loc_handles[0] = handles[1];
+	loc_handles[1] = handles[2];
+
+	/* Try to get two buffer object next to each other in GTT space. */
+	for (try = 0, idx = 0; try < max_tries;) {
+		eb_handles[0] = handles[0];
+		eb_handles[1] = loc_handles[idx];
+		eb_handles[2] = loc_handles[idx + 1];
+
+		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
+				offsets) == 0);
+
+		if (offsets[1] > offsets[0]) {
+			distance = offsets[1] - offsets[0];
+			if (distance == PAGE_SIZE)
+				neighbours = true;
+			pad_to_size[0] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
+		} else {
+			distance = offsets[0] - offsets[1];
+			if (distance == PAGE_SIZE)
+				neighbours = true;
+			pad_to_size[1] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
+		}
+
+		if (neighbours)
+			break;
+
+		try++;
+		idx +=2;
+
+		loc_handles[idx] = gem_create(fd, PAGE_SIZE);
+		igt_assert(loc_handles[idx]);
+		loc_handles[idx + 1] = gem_create(fd, PAGE_SIZE);
+		igt_assert(loc_handles[idx + 1]);
+	}
+
+	/* Test can't confidently run. */
+	if (!neighbours)
+		goto cleanup;
+
+	/* Re-exec with padding set. */
+	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
+
+	if (offsets[1] > offsets[0])
+		distance = offsets[1] - offsets[0];
+	else
+		distance = offsets[0] - offsets[1];
+
+	/* Check that object are now away from each other. */
+	igt_assert(distance >= 2 * PAGE_SIZE);
+
+cleanup:
+	/* Cleanup our bos. */
+	for (idx = 0; loc_handles[idx] && (idx < try * 2); idx++)
+		gem_close(fd, loc_handles[2 + idx]);
+
+	free(loc_handles);
+
+	igt_require(neighbours);
+}
+
+uint32_t batch[2] = {MI_BATCH_BUFFER_END};
+uint32_t handles[3];
+int fd;
+
+igt_main
+{
+	igt_fixture {
+		fd = drm_open_any();
+
+		handles[0] = gem_create(fd, PAGE_SIZE);
+		gem_write(fd, handles[0], 0, batch, sizeof(batch));
+
+		handles[1] = gem_create(fd, PAGE_SIZE);
+		handles[2] = gem_create(fd, PAGE_SIZE);
+	}
+
+	igt_subtest("pad_to_size")
+		require_pad_to_size(fd, handles);
+
+	igt_subtest("not_aligned")
+		not_aligned(fd, handles);
+
+	igt_subtest("padding")
+		padding(fd, handles);
+
+	igt_fixture {
+		gem_close(fd, handles[0]);
+		gem_close(fd, handles[1]);
+		gem_close(fd, handles[2]);
+
+		close(fd);
+	}
+}
-- 
2.3.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:21 [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf Tvrtko Ursulin
@ 2015-03-25 14:28 ` Chris Wilson
  2015-03-25 14:32   ` Tvrtko Ursulin
  2015-04-01 11:21 ` [RFC i-g-t v2] " Tvrtko Ursulin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2015-03-25 14:28 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Mar 25, 2015 at 02:21:00PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.
> 
> It uses the fact DRM allocation policy is set as ABI and allocates
> space in order.

Ssh. That's not what I meant to say. I meant that the policy is not ABI,
and we abuse our internal knowledge to get a working test, as we do
elsewhere in igt.

> That means that we should be able to easily get
> two bos mapped at adjacent GTT addresses and then test that the
> pad to size flag will move them apart.

> +	/* Try to get two buffer object next to each other in GTT space. */
> +	for (try = 0, idx = 0; try < max_tries;) {
> +		eb_handles[0] = handles[0];
> +		eb_handles[1] = loc_handles[idx];
> +		eb_handles[2] = loc_handles[idx + 1];
> +
> +		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
> +				offsets) == 0);
> +
> +		if (offsets[1] > offsets[0]) {
> +			distance = offsets[1] - offsets[0];
> +			if (distance == PAGE_SIZE)
> +				neighbours = true;
> +			pad_to_size[0] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
> +		} else {
> +			distance = offsets[0] - offsets[1];
> +			if (distance == PAGE_SIZE)
> +				neighbours = true;
> +			pad_to_size[1] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
> +		}
> +
> +		if (neighbours)
> +			break;

What's important to make this trick work is to allocate new handles
every time. That way we fill up the GTT and thereby hope to skip over
the fragmented part.
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:28 ` Chris Wilson
@ 2015-03-25 14:32   ` Tvrtko Ursulin
  2015-03-25 14:41     ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-03-25 14:32 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx, Tvrtko Ursulin


On 03/25/2015 02:28 PM, Chris Wilson wrote:
> On Wed, Mar 25, 2015 at 02:21:00PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.
>>
>> It uses the fact DRM allocation policy is set as ABI and allocates
>> space in order.
>
> Ssh. That's not what I meant to say. I meant that the policy is not ABI,
> and we abuse our internal knowledge to get a working test, as we do
> elsewhere in igt.

Ah, OK. :)

>> That means that we should be able to easily get
>> two bos mapped at adjacent GTT addresses and then test that the
>> pad to size flag will move them apart.
>
>> +	/* Try to get two buffer object next to each other in GTT space. */
>> +	for (try = 0, idx = 0; try < max_tries;) {
>> +		eb_handles[0] = handles[0];
>> +		eb_handles[1] = loc_handles[idx];
>> +		eb_handles[2] = loc_handles[idx + 1];
>> +
>> +		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
>> +				offsets) == 0);
>> +
>> +		if (offsets[1] > offsets[0]) {
>> +			distance = offsets[1] - offsets[0];
>> +			if (distance == PAGE_SIZE)
>> +				neighbours = true;
>> +			pad_to_size[0] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
>> +		} else {
>> +			distance = offsets[0] - offsets[1];
>> +			if (distance == PAGE_SIZE)
>> +				neighbours = true;
>> +			pad_to_size[1] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
>> +		}
>> +
>> +		if (neighbours)
>> +			break;
>
> What's important to make this trick work is to allocate new handles
> every time. That way we fill up the GTT and thereby hope to skip over
> the fragmented part.

It does do that!

Only perhaps I need to make sure they are kept bound so maybe keep them 
mapped or something while trying new ones?

Regards,

Tvrtko




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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:32   ` Tvrtko Ursulin
@ 2015-03-25 14:41     ` Chris Wilson
  2015-03-25 14:57       ` Tvrtko Ursulin
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2015-03-25 14:41 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Mar 25, 2015 at 02:32:05PM +0000, Tvrtko Ursulin wrote:
> 
> On 03/25/2015 02:28 PM, Chris Wilson wrote:
> >What's important to make this trick work is to allocate new handles
> >every time. That way we fill up the GTT and thereby hope to skip over
> >the fragmented part.
> 
> It does do that!

I was just looking for some filler for the email!

> Only perhaps I need to make sure they are kept bound so maybe keep
> them mapped or something while trying new ones?

What I was expecting to see were gem_create()s every loop. I only feel
confident in say that a pair of freshly allocated and bound objects are
likely to line up against each other (so long as space is not
fragmented).

I would allocate a new device fd for the test, leak the individual
handles on every try, then close the fd at the end of the test to avoid
having to track all the allocations. That's the pattern I was looking
for when I skimmed over your test.
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:41     ` Chris Wilson
@ 2015-03-25 14:57       ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-03-25 14:57 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx, Tvrtko Ursulin


On 03/25/2015 02:41 PM, Chris Wilson wrote:
> On Wed, Mar 25, 2015 at 02:32:05PM +0000, Tvrtko Ursulin wrote:
>>
>> On 03/25/2015 02:28 PM, Chris Wilson wrote:
>>> What's important to make this trick work is to allocate new handles
>>> every time. That way we fill up the GTT and thereby hope to skip over
>>> the fragmented part.
>>
>> It does do that!
>
> I was just looking for some filler for the email!

Try Reviewed-by? :D

>> Only perhaps I need to make sure they are kept bound so maybe keep
>> them mapped or something while trying new ones?
>
> What I was expecting to see were gem_create()s every loop. I only feel
> confident in say that a pair of freshly allocated and bound objects are
> likely to line up against each other (so long as space is not
> fragmented).
>
> I would allocate a new device fd for the test, leak the individual
> handles on every try, then close the fd at the end of the test to avoid
> having to track all the allocations. That's the pattern I was looking
> for when I skimmed over your test.

Apart from reusing the fd, tracking the handles and allocating on all 
loops but the first one when it re-uses the passed in ones, it is 
exactly like that! :)

So far it always manages to get neighbouring BOs on my system on first 
try. Which is probably not unexpected since is is PPGTT.

One thing I wasn't sure about is max_tries = 1024. That will be 8MB of 
filling to avoid fragmentation. Will that be enough on GGTT systems?

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC i-g-t v2] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:21 [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf Tvrtko Ursulin
  2015-03-25 14:28 ` Chris Wilson
@ 2015-04-01 11:21 ` Tvrtko Ursulin
  2015-04-01 13:06   ` Chris Wilson
  2015-04-01 15:14 ` [RFC i-g-t v3] " Tvrtko Ursulin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-01 11:21 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.

Similar to some other tests, it uses knowledge of the DRM
allocation policy in order to get two objects mapped adjacent
to each other. It is then possible to verify that the pad to
size flag will move them apart.

v2: Correct commit message. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources       |   1 +
 tests/gem_exec_pad_to_size.c | 276 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 277 insertions(+)
 create mode 100644 tests/gem_exec_pad_to_size.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 0a974a6..5f21728 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -34,6 +34,7 @@ TESTS_progs_M = \
 	gem_exec_bad_domains \
 	gem_exec_faulting_reloc \
 	gem_exec_nop \
+	gem_exec_pad_to_size \
 	gem_exec_params \
 	gem_exec_parse \
 	gem_fenced_exec_thrash \
diff --git a/tests/gem_exec_pad_to_size.c b/tests/gem_exec_pad_to_size.c
new file mode 100644
index 0000000..3584e5c
--- /dev/null
+++ b/tests/gem_exec_pad_to_size.c
@@ -0,0 +1,276 @@
+/*
+ * Copyright © 2015 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:
+ *    Tvrtko Ursulin <tvrtko.ursulin@intel.com>
+ *
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "igt_core.h"
+#include "drmtest.h"
+#include "intel_reg.h"
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+struct local_drm_i915_gem_exec_object2 {
+	/**
+	 * User's handle for a buffer to be bound into the GTT for this
+	 * operation.
+	 */
+	__u32 handle;
+
+	/** Number of relocations to be performed on this buffer */
+	__u32 relocation_count;
+	/**
+	 * Pointer to array of struct drm_i915_gem_relocation_entry containing
+	 * the relocations to be performed in this buffer.
+	 */
+	__u64 relocs_ptr;
+
+	/** Required alignment in graphics aperture */
+	__u64 alignment;
+
+	/**
+	 * Returned value of the updated offset of the object, for future
+	 * presumed_offset writes.
+	 */
+	__u64 offset;
+
+#define LOCAL_EXEC_OBJECT_NEEDS_FENCE (1<<0)
+#define LOCAL_EXEC_OBJECT_NEEDS_GTT	(1<<1)
+#define LOCAL_EXEC_OBJECT_WRITE	(1<<2)
+#define LOCAL_EXEC_OBJECT_PAD_TO_SIZE (1<<3)
+#define __LOCAL_EXEC_OBJECT_UNKNOWN_FLAGS -(LOCAL_EXEC_OBJECT_PAD_TO_SIZE<<1)
+	__u64 flags;
+
+	__u64 pad_to_size;
+	__u64 rsvd2;
+};
+
+static int
+exec(int fd, uint32_t handles[3], uint32_t pad_to_size[2], uint64_t *offsets)
+{
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct local_drm_i915_gem_exec_object2 gem_exec[3];
+	int ret = 0;
+
+	memset(gem_exec, 0, sizeof(gem_exec));
+
+	gem_exec[0].handle = handles[1];
+	gem_exec[0].relocation_count = 0;
+	gem_exec[0].relocs_ptr = 0;
+	gem_exec[0].alignment = 0;
+	gem_exec[0].offset = 0;
+	gem_exec[0].flags = pad_to_size[0] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[0].pad_to_size = pad_to_size[0];
+	gem_exec[0].rsvd2 = 0;
+
+	gem_exec[1].handle = handles[2];
+	gem_exec[1].relocation_count = 0;
+	gem_exec[1].relocs_ptr = 0;
+	gem_exec[1].alignment = 0;
+	gem_exec[1].offset = 0;
+	gem_exec[1].flags = pad_to_size[1] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[1].pad_to_size = pad_to_size[1];
+	gem_exec[1].rsvd2 = 0;
+
+	gem_exec[2].handle = handles[0];
+	gem_exec[2].relocation_count = 0;
+	gem_exec[2].relocs_ptr = 0;
+	gem_exec[2].alignment = 0;
+	gem_exec[2].offset = 0;
+	gem_exec[2].flags = 0;
+	gem_exec[2].pad_to_size = 0;
+	gem_exec[2].rsvd2 = 0;
+
+	execbuf.buffers_ptr = (uintptr_t)gem_exec;
+	execbuf.buffer_count = 3;
+	execbuf.batch_start_offset = 0;
+	execbuf.batch_len = 8;
+	execbuf.cliprects_ptr = 0;
+	execbuf.num_cliprects = 0;
+	execbuf.DR1 = 0;
+	execbuf.DR4 = 0;
+	execbuf.flags = I915_EXEC_RENDER;
+	i915_execbuffer2_set_context_id(execbuf, 0);
+	execbuf.rsvd2 = 0;
+
+	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
+			ret = -errno;
+
+	if (ret == 0) {
+		gem_sync(fd, handles[0]);
+
+		if (offsets) {
+			offsets[0] = gem_exec[0].offset;
+			offsets[1] = gem_exec[1].offset;
+		}
+	}
+
+	return ret;
+}
+
+static void
+require_pad_to_size(int fd, uint32_t handles[3])
+{
+	igt_require(exec(fd, handles, (uint32_t[2]){ PAGE_SIZE, PAGE_SIZE },
+			 NULL) == 0);
+}
+
+static void
+not_aligned(int fd, uint32_t handles[3])
+{
+	require_pad_to_size(fd, handles);
+
+	igt_require(exec(fd, handles, (uint32_t[2]){1 ,1},
+			 NULL) == -EINVAL);
+}
+
+static void
+padding(int fd, uint32_t handles[3])
+{
+	uint32_t pad_to_size[2] = {0, 0};
+	uint64_t offsets[2];
+	uint32_t distance;
+	bool neighbours = false;
+	unsigned int try, idx;
+	const unsigned int max_tries = 1024; /* Finger in the air. */
+	uint32_t *loc_handles;
+	uint32_t eb_handles[3];
+
+	require_pad_to_size(fd, handles);
+
+	loc_handles = calloc(max_tries * 2, sizeof(uint32_t));
+	igt_assert(loc_handles);
+
+	/* Try with passed in handles first. */
+	loc_handles[0] = handles[1];
+	loc_handles[1] = handles[2];
+
+	/* Try to get two buffer object next to each other in GTT space. */
+	for (try = 0, idx = 0; try < max_tries;) {
+		eb_handles[0] = handles[0];
+		eb_handles[1] = loc_handles[idx];
+		eb_handles[2] = loc_handles[idx + 1];
+
+		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
+				offsets) == 0);
+
+		if (offsets[1] > offsets[0]) {
+			distance = offsets[1] - offsets[0];
+			if (distance == PAGE_SIZE)
+				neighbours = true;
+			pad_to_size[0] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
+		} else {
+			distance = offsets[0] - offsets[1];
+			if (distance == PAGE_SIZE)
+				neighbours = true;
+			pad_to_size[1] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
+		}
+
+		if (neighbours)
+			break;
+
+		try++;
+		idx +=2;
+
+		loc_handles[idx] = gem_create(fd, PAGE_SIZE);
+		igt_assert(loc_handles[idx]);
+		loc_handles[idx + 1] = gem_create(fd, PAGE_SIZE);
+		igt_assert(loc_handles[idx + 1]);
+	}
+
+	/* Test can't confidently run. */
+	if (!neighbours)
+		goto cleanup;
+
+	/* Re-exec with padding set. */
+	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
+
+	if (offsets[1] > offsets[0])
+		distance = offsets[1] - offsets[0];
+	else
+		distance = offsets[0] - offsets[1];
+
+	/* Check that object are now away from each other. */
+	igt_assert(distance >= 2 * PAGE_SIZE);
+
+cleanup:
+	/* Cleanup our bos. */
+	for (idx = 0; loc_handles[idx] && (idx < try * 2); idx++)
+		gem_close(fd, loc_handles[2 + idx]);
+
+	free(loc_handles);
+
+	igt_require(neighbours);
+}
+
+uint32_t batch[2] = {MI_BATCH_BUFFER_END};
+uint32_t handles[3];
+int fd;
+
+igt_main
+{
+	igt_fixture {
+		fd = drm_open_any();
+
+		handles[0] = gem_create(fd, PAGE_SIZE);
+		gem_write(fd, handles[0], 0, batch, sizeof(batch));
+
+		handles[1] = gem_create(fd, PAGE_SIZE);
+		handles[2] = gem_create(fd, PAGE_SIZE);
+	}
+
+	igt_subtest("pad_to_size")
+		require_pad_to_size(fd, handles);
+
+	igt_subtest("not_aligned")
+		not_aligned(fd, handles);
+
+	igt_subtest("padding")
+		padding(fd, handles);
+
+	igt_fixture {
+		gem_close(fd, handles[0]);
+		gem_close(fd, handles[1]);
+		gem_close(fd, handles[2]);
+
+		close(fd);
+	}
+}
-- 
2.3.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v2] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 11:21 ` [RFC i-g-t v2] " Tvrtko Ursulin
@ 2015-04-01 13:06   ` Chris Wilson
  2015-04-01 13:17     ` Chris Wilson
  2015-04-01 13:36     ` Tvrtko Ursulin
  0 siblings, 2 replies; 22+ messages in thread
From: Chris Wilson @ 2015-04-01 13:06 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Apr 01, 2015 at 12:21:14PM +0100, Tvrtko Ursulin wrote:

> +static int
> +exec(int fd, uint32_t handles[3], uint32_t pad_to_size[2], uint64_t *offsets)
> +{
> +	struct drm_i915_gem_execbuffer2 execbuf;
> +	struct local_drm_i915_gem_exec_object2 gem_exec[3];
> +	int ret = 0;
> +
> +	memset(gem_exec, 0, sizeof(gem_exec));
> +
> +	gem_exec[0].handle = handles[1];
> +	gem_exec[0].relocation_count = 0;
> +	gem_exec[0].relocs_ptr = 0;
> +	gem_exec[0].alignment = 0;
> +	gem_exec[0].offset = 0;
Ignore the unused fields (they are explicitly memset(0)) so that we can
focus on the important ones under test.

> +	gem_exec[0].flags = pad_to_size[0] ?
> +			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
> +	gem_exec[0].pad_to_size = pad_to_size[0];
> +

memset(execbuf) and skip boring fields
> +	execbuf.buffers_ptr = (uintptr_t)gem_exec;
> +	execbuf.buffer_count = 3;
> +	execbuf.batch_start_offset = 0;
> +	execbuf.batch_len = 8;
> +	execbuf.flags = I915_EXEC_RENDER;
This can be just I915_EXEC_DEFAULT (i.e. 0);


> +	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
> +			ret = -errno;

> +	if (ret == 0) {
> +		gem_sync(fd, handles[0]);
Not required for this test. However... You probably want to do the
gem_sync() first. (Yes, there is an amusing reason to do :)

> +		if (offsets) {
> +			offsets[0] = gem_exec[0].offset;
> +			offsets[1] = gem_exec[1].offset;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void
> +require_pad_to_size(int fd, uint32_t handles[3])
> +{
> +	igt_require(exec(fd, handles, (uint32_t[2]){ PAGE_SIZE, PAGE_SIZE },
> +			 NULL) == 0);
> +}
> +
> +static void
> +not_aligned(int fd, uint32_t handles[3])
> +{
> +	require_pad_to_size(fd, handles);
> +
> +	igt_require(exec(fd, handles, (uint32_t[2]){1 ,1},
> +			 NULL) == -EINVAL);
> +}
> +
> +static void
> +padding(int fd, uint32_t handles[3])
> +{
> +	uint32_t pad_to_size[2] = {0, 0};
> +	uint64_t offsets[2];
> +	uint32_t distance;
> +	bool neighbours = false;
> +	unsigned int try, idx;
> +	const unsigned int max_tries = 1024; /* Finger in the air. */
> +	uint32_t *loc_handles;
> +	uint32_t eb_handles[3];
> +
> +	require_pad_to_size(fd, handles);
> +
> +	loc_handles = calloc(max_tries * 2, sizeof(uint32_t));
> +	igt_assert(loc_handles);
> +
> +	/* Try with passed in handles first. */
> +	loc_handles[0] = handles[1];
> +	loc_handles[1] = handles[2];
> +
> +	/* Try to get two buffer object next to each other in GTT space. */
/* Try to get two buffer object next to each other in GTT space.
 * We presume that sequential reservations are likely to be aligned and
 * try until we find a pair that is.
 */

> +	for (try = 0, idx = 0; try < max_tries;) {
> +		eb_handles[0] = handles[0];
> +		eb_handles[1] = loc_handles[idx];
> +		eb_handles[2] = loc_handles[idx + 1];
> +
> +		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
> +				offsets) == 0);
> +
> +		if (offsets[1] > offsets[0]) {
> +			distance = offsets[1] - offsets[0];
> +			if (distance == PAGE_SIZE)
> +				neighbours = true;
> +			pad_to_size[0] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
> +		} else {
> +			distance = offsets[0] - offsets[1];
> +			if (distance == PAGE_SIZE)
> +				neighbours = true;
> +			pad_to_size[1] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
> +		}
> +
> +		if (neighbours)
> +			break;
> +
> +		try++;
> +		idx +=2;

Just use idx++ here and allocate a new handle one at a time. Just as
likely to be adjacent to the previous handle as the next one will be to
us. For extra paranoia, you could even try an evict-everything pass :)

Otherwise looks fine.
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v2] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 13:06   ` Chris Wilson
@ 2015-04-01 13:17     ` Chris Wilson
  2015-04-01 13:36     ` Tvrtko Ursulin
  1 sibling, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2015-04-01 13:17 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin

On Wed, Apr 01, 2015 at 02:06:53PM +0100, Chris Wilson wrote:
> Just use idx++ here and allocate a new handle one at a time. Just as
> likely to be adjacent to the previous handle as the next one will be to
> us. For extra paranoia, you could even try an evict-everything pass :)

Which is as easy as igt_drop_caches_set(DROP_ALL);
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v2] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 13:06   ` Chris Wilson
  2015-04-01 13:17     ` Chris Wilson
@ 2015-04-01 13:36     ` Tvrtko Ursulin
  2015-04-01 13:56       ` Chris Wilson
  1 sibling, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-01 13:36 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx, Tvrtko Ursulin


On 04/01/2015 02:06 PM, Chris Wilson wrote:
> On Wed, Apr 01, 2015 at 12:21:14PM +0100, Tvrtko Ursulin wrote:
>> +	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
>> +			ret = -errno;
>
>> +	if (ret == 0) {
>> +		gem_sync(fd, handles[0]);
> Not required for this test. However... You probably want to do the
> gem_sync() first. (Yes, there is an amusing reason to do :)

What reason is that and what do you mean by "first"?

>> +	for (try = 0, idx = 0; try < max_tries;) {
>> +		eb_handles[0] = handles[0];
>> +		eb_handles[1] = loc_handles[idx];
>> +		eb_handles[2] = loc_handles[idx + 1];
>> +
>> +		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
>> +				offsets) == 0);
>> +
>> +		if (offsets[1] > offsets[0]) {
>> +			distance = offsets[1] - offsets[0];
>> +			if (distance == PAGE_SIZE)
>> +				neighbours = true;
>> +			pad_to_size[0] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
>> +		} else {
>> +			distance = offsets[0] - offsets[1];
>> +			if (distance == PAGE_SIZE)
>> +				neighbours = true;
>> +			pad_to_size[1] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
>> +		}
>> +
>> +		if (neighbours)
>> +			break;
>> +
>> +		try++;
>> +		idx +=2;
>
> Just use idx++ here and allocate a new handle one at a time. Just as
> likely to be adjacent to the previous handle as the next one will be to

Ah yes, didn't think of that!

> us. For extra paranoia, you could even try an evict-everything pass :)

You mean if the lightweight approach fails? Ok.

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v2] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 13:36     ` Tvrtko Ursulin
@ 2015-04-01 13:56       ` Chris Wilson
  2015-04-01 14:14         ` Tvrtko Ursulin
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2015-04-01 13:56 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Apr 01, 2015 at 02:36:29PM +0100, Tvrtko Ursulin wrote:
> 
> On 04/01/2015 02:06 PM, Chris Wilson wrote:
> >On Wed, Apr 01, 2015 at 12:21:14PM +0100, Tvrtko Ursulin wrote:
> >>+	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
> >>+			ret = -errno;
> >
> >>+	if (ret == 0) {
> >>+		gem_sync(fd, handles[0]);
> >Not required for this test. However... You probably want to do the
> >gem_sync() first. (Yes, there is an amusing reason to do :)
> 
> What reason is that and what do you mean by "first"?

When calling the test multiple times successive passes will have a busy
batch, which could conceivably cause the relocations to fail, and the
returned offsets to be -1. Calling gem_sync() before execbuf prevents that
slowpath from affecting the test, without people wondering whether
you need to call gem_sync() before the returned array is valid (which is
why I did a double take here trying to work out what you meant the
gem_sync() to do).
 
> >Just use idx++ here and allocate a new handle one at a time. Just as
> >likely to be adjacent to the previous handle as the next one will be to
> 
> Ah yes, didn't think of that!
> 
> >us. For extra paranoia, you could even try an evict-everything pass :)
> 
> You mean if the lightweight approach fails? Ok.

Calling igt_drop_caches_set() is reasonable enough to in the preamble.
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v2] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 13:56       ` Chris Wilson
@ 2015-04-01 14:14         ` Tvrtko Ursulin
  2015-04-01 14:27           ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-01 14:14 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx, Tvrtko Ursulin


On 04/01/2015 02:56 PM, Chris Wilson wrote:
> On Wed, Apr 01, 2015 at 02:36:29PM +0100, Tvrtko Ursulin wrote:
>>
>> On 04/01/2015 02:06 PM, Chris Wilson wrote:
>>> On Wed, Apr 01, 2015 at 12:21:14PM +0100, Tvrtko Ursulin wrote:
>>>> +	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
>>>> +			ret = -errno;
>>>
>>>> +	if (ret == 0) {
>>>> +		gem_sync(fd, handles[0]);
>>> Not required for this test. However... You probably want to do the
>>> gem_sync() first. (Yes, there is an amusing reason to do :)
>>
>> What reason is that and what do you mean by "first"?
>
> When calling the test multiple times successive passes will have a busy
> batch, which could conceivably cause the relocations to fail, and the
> returned offsets to be -1. Calling gem_sync() before execbuf prevents that
> slowpath from affecting the test, without people wondering whether
> you need to call gem_sync() before the returned array is valid (which is
> why I did a double take here trying to work out what you meant the
> gem_sync() to do).

How it may be busy if gem_sync after execbuf is supposed to wait until 
batch has been retired?

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v2] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 14:14         ` Tvrtko Ursulin
@ 2015-04-01 14:27           ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2015-04-01 14:27 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Apr 01, 2015 at 03:14:26PM +0100, Tvrtko Ursulin wrote:
> 
> On 04/01/2015 02:56 PM, Chris Wilson wrote:
> >On Wed, Apr 01, 2015 at 02:36:29PM +0100, Tvrtko Ursulin wrote:
> >>
> >>On 04/01/2015 02:06 PM, Chris Wilson wrote:
> >>>On Wed, Apr 01, 2015 at 12:21:14PM +0100, Tvrtko Ursulin wrote:
> >>>>+	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
> >>>>+			ret = -errno;
> >>>
> >>>>+	if (ret == 0) {
> >>>>+		gem_sync(fd, handles[0]);
> >>>Not required for this test. However... You probably want to do the
> >>>gem_sync() first. (Yes, there is an amusing reason to do :)
> >>
> >>What reason is that and what do you mean by "first"?
> >
> >When calling the test multiple times successive passes will have a busy
> >batch, which could conceivably cause the relocations to fail, and the
> >returned offsets to be -1. Calling gem_sync() before execbuf prevents that
> >slowpath from affecting the test, without people wondering whether
> >you need to call gem_sync() before the returned array is valid (which is
> >why I did a double take here trying to work out what you meant the
> >gem_sync() to do).
> 
> How it may be busy if gem_sync after execbuf is supposed to wait
> until batch has been retired?

It's not. I am trying to say that the placement of gem_sync() here
suggests to me that you thought it was required to get valid offsets
after an execbuf call. But in actualilty you do not need a gem_sync() in
this test at all as you are not doing relocations.
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC i-g-t v3] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:21 [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf Tvrtko Ursulin
  2015-03-25 14:28 ` Chris Wilson
  2015-04-01 11:21 ` [RFC i-g-t v2] " Tvrtko Ursulin
@ 2015-04-01 15:14 ` Tvrtko Ursulin
  2015-04-01 15:42   ` Chris Wilson
  2015-04-02 10:45 ` [RFC i-g-t v4] " Tvrtko Ursulin
  2015-04-02 12:54 ` [RFC i-g-t v5] " Tvrtko Ursulin
  4 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-01 15:14 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.

Similar to some other tests, it uses knowledge of the DRM
allocation policy in order to get two objects mapped adjacent
to each other. It is then possible to verify that the pad to
size flag will move them apart.

v2: Correct commit message. (Chris Wilson)
v3: Changes after code review by Chris Wilson.
     * No need for gem_sync after execbuf.
     * Drop caches before running.
     * Allocate one additional bo per iteration.
     * Don't explicitly set unused execbuf fields to zero.
     * One improved comment.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources       |   1 +
 tests/gem_exec_pad_to_size.c | 252 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 253 insertions(+)
 create mode 100644 tests/gem_exec_pad_to_size.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 0a974a6..5f21728 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -34,6 +34,7 @@ TESTS_progs_M = \
 	gem_exec_bad_domains \
 	gem_exec_faulting_reloc \
 	gem_exec_nop \
+	gem_exec_pad_to_size \
 	gem_exec_params \
 	gem_exec_parse \
 	gem_fenced_exec_thrash \
diff --git a/tests/gem_exec_pad_to_size.c b/tests/gem_exec_pad_to_size.c
new file mode 100644
index 0000000..0017586
--- /dev/null
+++ b/tests/gem_exec_pad_to_size.c
@@ -0,0 +1,252 @@
+/*
+ * Copyright © 2015 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:
+ *    Tvrtko Ursulin <tvrtko.ursulin@intel.com>
+ *
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "igt_core.h"
+#include "drmtest.h"
+#include "intel_reg.h"
+#include "igt_debugfs.h"
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+struct local_drm_i915_gem_exec_object2 {
+	/**
+	 * User's handle for a buffer to be bound into the GTT for this
+	 * operation.
+	 */
+	__u32 handle;
+
+	/** Number of relocations to be performed on this buffer */
+	__u32 relocation_count;
+	/**
+	 * Pointer to array of struct drm_i915_gem_relocation_entry containing
+	 * the relocations to be performed in this buffer.
+	 */
+	__u64 relocs_ptr;
+
+	/** Required alignment in graphics aperture */
+	__u64 alignment;
+
+	/**
+	 * Returned value of the updated offset of the object, for future
+	 * presumed_offset writes.
+	 */
+	__u64 offset;
+
+#define LOCAL_EXEC_OBJECT_NEEDS_FENCE (1<<0)
+#define LOCAL_EXEC_OBJECT_NEEDS_GTT	(1<<1)
+#define LOCAL_EXEC_OBJECT_WRITE	(1<<2)
+#define LOCAL_EXEC_OBJECT_PAD_TO_SIZE (1<<3)
+#define __LOCAL_EXEC_OBJECT_UNKNOWN_FLAGS -(LOCAL_EXEC_OBJECT_PAD_TO_SIZE<<1)
+	__u64 flags;
+
+	__u64 pad_to_size;
+	__u64 rsvd2;
+};
+
+static int
+exec(int fd, uint32_t handles[3], uint32_t pad_to_size[2], uint64_t *offsets)
+{
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct local_drm_i915_gem_exec_object2 gem_exec[3];
+	int ret = 0;
+
+	memset(gem_exec, 0, sizeof(gem_exec));
+
+	gem_exec[0].handle = handles[1];
+	gem_exec[0].flags = pad_to_size[0] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[0].pad_to_size = pad_to_size[0];
+
+	gem_exec[1].handle = handles[2];
+	gem_exec[1].flags = pad_to_size[1] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[1].pad_to_size = pad_to_size[1];
+
+	gem_exec[2].handle = handles[0];
+
+	memset(&execbuf, 0, sizeof(execbuf));
+
+	execbuf.buffers_ptr = (uintptr_t)gem_exec;
+	execbuf.buffer_count = 3;
+	execbuf.batch_len = 8;
+
+	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
+			ret = -errno;
+
+	if (ret == 0 && offsets) {
+		offsets[0] = gem_exec[0].offset;
+		offsets[1] = gem_exec[1].offset;
+	}
+
+	return ret;
+}
+
+static void
+require_pad_to_size(int fd, uint32_t handles[3])
+{
+	igt_require(exec(fd, handles, (uint32_t[2]){ PAGE_SIZE, PAGE_SIZE },
+			 NULL) == 0);
+}
+
+static void
+not_aligned(int fd, uint32_t handles[3])
+{
+	require_pad_to_size(fd, handles);
+
+	igt_require(exec(fd, handles, (uint32_t[2]){1 ,1},
+			 NULL) == -EINVAL);
+}
+
+static void
+padding(int fd, uint32_t handles[3])
+{
+	uint32_t pad_to_size[2] = {0, 0};
+	uint64_t offsets[2];
+	uint32_t distance;
+	bool neighbours = false;
+	unsigned int try, i;
+	const unsigned int max_tries = 4096; /* Finger in the air. */
+	uint32_t *loc_handles;
+	uint32_t eb_handles[3];
+
+	require_pad_to_size(fd, handles);
+
+	igt_drop_caches_set(DROP_ALL);
+
+	loc_handles = calloc(max_tries * 2, sizeof(uint32_t));
+	igt_assert(loc_handles);
+
+	/* Try with passed in handles first. */
+	loc_handles[0] = handles[1];
+	loc_handles[1] = handles[2];
+
+	/* Try to get two buffer object next to each other in GTT space.
+	 * We presume that sequential reservations are likely to be aligned and
+	 * try until we find a pair that is.
+	 */
+	for (try = 0; try < max_tries;) {
+		eb_handles[0] = handles[0];
+		eb_handles[1] = loc_handles[try];
+		eb_handles[2] = loc_handles[try + 1];
+
+		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
+				offsets) == 0);
+
+		if (offsets[1] > offsets[0]) {
+			distance = offsets[1] - offsets[0];
+			if (distance == PAGE_SIZE)
+				neighbours = true;
+			pad_to_size[0] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
+		} else {
+			distance = offsets[0] - offsets[1];
+			if (distance == PAGE_SIZE)
+				neighbours = true;
+			pad_to_size[1] = ALIGN(distance + PAGE_SIZE, PAGE_SIZE);
+		}
+
+		if (neighbours)
+			break;
+
+		try++;
+
+		loc_handles[try + 1] = gem_create(fd, PAGE_SIZE);
+		igt_assert(loc_handles[try + 1]);
+	}
+
+	/* Test can't confidently run. */
+	if (!neighbours)
+		goto cleanup;
+
+	/* Re-exec with padding set. */
+	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
+
+	if (offsets[1] > offsets[0])
+		distance = offsets[1] - offsets[0];
+	else
+		distance = offsets[0] - offsets[1];
+
+	/* Check that object are now away from each other. */
+	igt_assert(distance >= 2 * PAGE_SIZE);
+
+cleanup:
+	/* Cleanup our bos. */
+	for (i = 0;  i < try; i++)
+		gem_close(fd, loc_handles[2 + i]);
+
+	free(loc_handles);
+
+	igt_require(neighbours);
+}
+
+uint32_t batch[2] = {MI_BATCH_BUFFER_END};
+uint32_t handles[3];
+int fd;
+
+igt_main
+{
+	igt_fixture {
+		fd = drm_open_any();
+
+		handles[0] = gem_create(fd, PAGE_SIZE);
+		gem_write(fd, handles[0], 0, batch, sizeof(batch));
+
+		handles[1] = gem_create(fd, PAGE_SIZE);
+		handles[2] = gem_create(fd, PAGE_SIZE);
+	}
+
+	igt_subtest("pad_to_size")
+		require_pad_to_size(fd, handles);
+
+	igt_subtest("not_aligned")
+		not_aligned(fd, handles);
+
+	igt_subtest("padding")
+		padding(fd, handles);
+
+	igt_fixture {
+		gem_close(fd, handles[0]);
+		gem_close(fd, handles[1]);
+		gem_close(fd, handles[2]);
+
+		close(fd);
+	}
+}
-- 
2.3.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v3] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 15:14 ` [RFC i-g-t v3] " Tvrtko Ursulin
@ 2015-04-01 15:42   ` Chris Wilson
  2015-04-01 16:07     ` Tvrtko Ursulin
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2015-04-01 15:42 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Apr 01, 2015 at 04:14:52PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.
> 
> Similar to some other tests, it uses knowledge of the DRM
> allocation policy in order to get two objects mapped adjacent
> to each other. It is then possible to verify that the pad to
> size flag will move them apart.
> 
> v2: Correct commit message. (Chris Wilson)
> v3: Changes after code review by Chris Wilson.
>      * No need for gem_sync after execbuf.
>      * Drop caches before running.
>      * Allocate one additional bo per iteration.
>      * Don't explicitly set unused execbuf fields to zero.
>      * One improved comment.

Ah, sorry to be a nuisance, I'm getting to the actual test finally!

> +	/* Re-exec with padding set. */
> +	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);

The crux of the test is that we generate two objects such that

B_offset = A_offset + A_size

and then tell the kernel that A is actually 2*size (A_pad_to_size)

> +	if (offsets[1] > offsets[0])
> +		distance = offsets[1] - offsets[0];
> +	else
> +		distance = offsets[0] - offsets[1];

The assertion I feel should only be that

B_offset + B_size <= A_offset && B_offset >= A_offset + A_pad_to_size

i.e. that they are now disjoint.

Your test is valid nevertheless, it is the ordering of the objects that
is confusing.

Hmm, can you loop until B_offset == A_offset + A_size such that we don't
have the confusion with order? And even assert that A_offset is
unchanged (though that smells like a little to much internal knowledge
leaking through, it is a desirable property of the allocator though - no
unnecessarily eviction) afterwards.

Do you agree that losing the handling of negative distances will make
the test simpler to understand (at the expense of doing more work in the
setup)?
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v3] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 15:42   ` Chris Wilson
@ 2015-04-01 16:07     ` Tvrtko Ursulin
  2015-04-01 16:31       ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-01 16:07 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx, Tvrtko Ursulin


On 04/01/2015 04:42 PM, Chris Wilson wrote:
> On Wed, Apr 01, 2015 at 04:14:52PM +0100, Tvrtko Ursulin wrote:
>> +	/* Re-exec with padding set. */
>> +	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
>
> The crux of the test is that we generate two objects such that
>
> B_offset = A_offset + A_size
>
> and then tell the kernel that A is actually 2*size (A_pad_to_size)
>
>> +	if (offsets[1] > offsets[0])
>> +		distance = offsets[1] - offsets[0];
>> +	else
>> +		distance = offsets[0] - offsets[1];
>
> The assertion I feel should only be that
>
> B_offset + B_size <= A_offset && B_offset >= A_offset + A_pad_to_size

I don't get this. B starts after A + padding, but B ends before A?

> i.e. that they are now disjoint.
>
> Your test is valid nevertheless, it is the ordering of the objects that
> is confusing.
>
> Hmm, can you loop until B_offset == A_offset + A_size such that we don't
> have the confusion with order? And even assert that A_offset is
> unchanged (though that smells like a little to much internal knowledge
> leaking through, it is a desirable property of the allocator though - no
> unnecessarily eviction) afterwards.
>
> Do you agree that losing the handling of negative distances will make
> the test simpler to understand (at the expense of doing more work in the
> setup)?

I thought my test logic is pretty straightforward:

1. Find two objects next to each other.
2. Add padding on the "lower" (addressed) object.
3. Ensure objects are now apart at least what the padding is.

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v3] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 16:07     ` Tvrtko Ursulin
@ 2015-04-01 16:31       ` Chris Wilson
  2015-04-01 16:39         ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2015-04-01 16:31 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Wed, Apr 01, 2015 at 05:07:25PM +0100, Tvrtko Ursulin wrote:
> 
> On 04/01/2015 04:42 PM, Chris Wilson wrote:
> >On Wed, Apr 01, 2015 at 04:14:52PM +0100, Tvrtko Ursulin wrote:
> >>+	/* Re-exec with padding set. */
> >>+	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
> >
> >The crux of the test is that we generate two objects such that
> >
> >B_offset = A_offset + A_size
> >
> >and then tell the kernel that A is actually 2*size (A_pad_to_size)
> >
> >>+	if (offsets[1] > offsets[0])
> >>+		distance = offsets[1] - offsets[0];
> >>+	else
> >>+		distance = offsets[0] - offsets[1];
> >
> >The assertion I feel should only be that
> >
> >B_offset + B_size <= A_offset && B_offset >= A_offset + A_pad_to_size
> 
> I don't get this. B starts after A + padding, but B ends before A?

s/&&/||/ 

> >i.e. that they are now disjoint.
> >
> >Your test is valid nevertheless, it is the ordering of the objects that
> >is confusing.
> >
> >Hmm, can you loop until B_offset == A_offset + A_size such that we don't
> >have the confusion with order? And even assert that A_offset is
> >unchanged (though that smells like a little to much internal knowledge
> >leaking through, it is a desirable property of the allocator though - no
> >unnecessarily eviction) afterwards.
> >
> >Do you agree that losing the handling of negative distances will make
> >the test simpler to understand (at the expense of doing more work in the
> >setup)?
> 
> I thought my test logic is pretty straightforward:
> 
> 1. Find two objects next to each other.
> 2. Add padding on the "lower" (addressed) object.
> 3. Ensure objects are now apart at least what the padding is.

What should happen is:

Initial layout:

|    ||aa||bb|
batch A   B

(B_offset == A_offset + PAGE)

We then submit an execbuf with A_pad_to_size = 2*PAGE, the kernel will
unbind A, keep B, then bind A (given an empty GTT):

|    |xxxx|bb||aaaaaa|
batch      B  A

but the distance here is just 1 PAGE. Then if B_pad_to_size is set to
2*PAGE:

|    |xxxxxxxx|aaaaaa||bbbbbb|
batch         A       B

If both are initially set to pad_to_size = 2*PAGE, we should see something
like:

|    ||aaaaaa||bbbbbb|
batch A       B

Given that the assertion should just be that the new objects do not
overlap assuming their size is now pad_to_size.
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v3] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 16:31       ` Chris Wilson
@ 2015-04-01 16:39         ` Chris Wilson
  2015-04-02 10:47           ` Tvrtko Ursulin
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2015-04-01 16:39 UTC (permalink / raw)
  To: Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin

On Wed, Apr 01, 2015 at 05:31:16PM +0100, Chris Wilson wrote:
> On Wed, Apr 01, 2015 at 05:07:25PM +0100, Tvrtko Ursulin wrote:
> > 
> > On 04/01/2015 04:42 PM, Chris Wilson wrote:
> > >On Wed, Apr 01, 2015 at 04:14:52PM +0100, Tvrtko Ursulin wrote:
> > >>+	/* Re-exec with padding set. */
> > >>+	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
> > >
> > >The crux of the test is that we generate two objects such that
> > >
> > >B_offset = A_offset + A_size
> > >
> > >and then tell the kernel that A is actually 2*size (A_pad_to_size)
> > >
> > >>+	if (offsets[1] > offsets[0])
> > >>+		distance = offsets[1] - offsets[0];
> > >>+	else
> > >>+		distance = offsets[0] - offsets[1];
> > >
> > >The assertion I feel should only be that
> > >
> > >B_offset + B_size <= A_offset && B_offset >= A_offset + A_pad_to_size
> > 
> > I don't get this. B starts after A + padding, but B ends before A?
> 
> s/&&/||/ 

Sorry &&. Gah, must be time for a coffee break.

The assertion is that the objects do not overlap based on the pad_to_size we
expect the kernel to apply, rather than their natural size. If the
kernel doesn't move the objects, they would it would fail.
-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC i-g-t v4] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:21 [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf Tvrtko Ursulin
                   ` (2 preceding siblings ...)
  2015-04-01 15:14 ` [RFC i-g-t v3] " Tvrtko Ursulin
@ 2015-04-02 10:45 ` Tvrtko Ursulin
  2015-04-02 11:02   ` Chris Wilson
  2015-04-02 12:54 ` [RFC i-g-t v5] " Tvrtko Ursulin
  4 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-02 10:45 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.

Similar to some other tests, it uses knowledge of the DRM
allocation policy in order to get two objects mapped adjacent
to each other. It is then possible to verify that the pad to
size flag will move them apart.

v2: Correct commit message. (Chris Wilson)
v3: Changes after code review by Chris Wilson.
     * No need for gem_sync after execbuf.
     * Drop caches before running.
     * Allocate one additional bo per iteration.
     * Don't explicitly set unused execbuf fields to zero.
     * One improved comment.
v4: Require simpler object ordering and fixed overlap test. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources       |   1 +
 tests/gem_exec_pad_to_size.c | 236 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 237 insertions(+)
 create mode 100644 tests/gem_exec_pad_to_size.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 0a974a6..5f21728 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -34,6 +34,7 @@ TESTS_progs_M = \
 	gem_exec_bad_domains \
 	gem_exec_faulting_reloc \
 	gem_exec_nop \
+	gem_exec_pad_to_size \
 	gem_exec_params \
 	gem_exec_parse \
 	gem_fenced_exec_thrash \
diff --git a/tests/gem_exec_pad_to_size.c b/tests/gem_exec_pad_to_size.c
new file mode 100644
index 0000000..db56d15
--- /dev/null
+++ b/tests/gem_exec_pad_to_size.c
@@ -0,0 +1,236 @@
+/*
+ * Copyright © 2015 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:
+ *    Tvrtko Ursulin <tvrtko.ursulin@intel.com>
+ *
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "igt_core.h"
+#include "drmtest.h"
+#include "intel_reg.h"
+#include "igt_debugfs.h"
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+struct local_drm_i915_gem_exec_object2 {
+	/**
+	 * User's handle for a buffer to be bound into the GTT for this
+	 * operation.
+	 */
+	__u32 handle;
+
+	/** Number of relocations to be performed on this buffer */
+	__u32 relocation_count;
+	/**
+	 * Pointer to array of struct drm_i915_gem_relocation_entry containing
+	 * the relocations to be performed in this buffer.
+	 */
+	__u64 relocs_ptr;
+
+	/** Required alignment in graphics aperture */
+	__u64 alignment;
+
+	/**
+	 * Returned value of the updated offset of the object, for future
+	 * presumed_offset writes.
+	 */
+	__u64 offset;
+
+#define LOCAL_EXEC_OBJECT_NEEDS_FENCE (1<<0)
+#define LOCAL_EXEC_OBJECT_NEEDS_GTT	(1<<1)
+#define LOCAL_EXEC_OBJECT_WRITE	(1<<2)
+#define LOCAL_EXEC_OBJECT_PAD_TO_SIZE (1<<3)
+#define __LOCAL_EXEC_OBJECT_UNKNOWN_FLAGS -(LOCAL_EXEC_OBJECT_PAD_TO_SIZE<<1)
+	__u64 flags;
+
+	__u64 pad_to_size;
+	__u64 rsvd2;
+};
+
+static int
+exec(int fd, uint32_t handles[3], uint32_t pad_to_size[2], uint64_t *offsets)
+{
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct local_drm_i915_gem_exec_object2 gem_exec[3];
+	int ret = 0;
+
+	memset(gem_exec, 0, sizeof(gem_exec));
+
+	gem_exec[0].handle = handles[1];
+	gem_exec[0].flags = pad_to_size[0] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[0].pad_to_size = pad_to_size[0];
+
+	gem_exec[1].handle = handles[2];
+	gem_exec[1].flags = pad_to_size[1] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[1].pad_to_size = pad_to_size[1];
+
+	gem_exec[2].handle = handles[0];
+
+	memset(&execbuf, 0, sizeof(execbuf));
+
+	execbuf.buffers_ptr = (uintptr_t)gem_exec;
+	execbuf.buffer_count = 3;
+	execbuf.batch_len = 8;
+
+	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
+			ret = -errno;
+
+	if (ret == 0 && offsets) {
+		offsets[0] = gem_exec[0].offset;
+		offsets[1] = gem_exec[1].offset;
+	}
+
+	return ret;
+}
+
+static void
+require_pad_to_size(int fd, uint32_t handles[3])
+{
+	igt_require(exec(fd, handles, (uint32_t[2]){ PAGE_SIZE, PAGE_SIZE },
+			 NULL) == 0);
+}
+
+static void
+not_aligned(int fd, uint32_t handles[3])
+{
+	require_pad_to_size(fd, handles);
+
+	igt_require(exec(fd, handles, (uint32_t[2]){1 ,1},
+			 NULL) == -EINVAL);
+}
+
+static void
+padding(int fd, uint32_t handles[3])
+{
+	uint32_t pad_to_size[2] = {0, 0};
+	uint64_t offsets[2];
+	bool neighbours = false;
+	unsigned int try, i;
+	const unsigned int max_tries = 4096; /* Finger in the air. */
+	uint32_t *loc_handles;
+	uint32_t eb_handles[3];
+
+	require_pad_to_size(fd, handles);
+
+	igt_drop_caches_set(DROP_ALL);
+
+	loc_handles = calloc(max_tries * 2, sizeof(uint32_t));
+	igt_assert(loc_handles);
+
+	/* Try with passed in handles first. */
+	loc_handles[0] = handles[1];
+	loc_handles[1] = handles[2];
+
+	/* Try to get two buffer object next to each other in GTT space.
+	 * We presume that sequential reservations are likely to be aligned and
+	 * try until we find a pair that is.
+	 */
+	for (try = 0; try < max_tries;) {
+		eb_handles[0] = handles[0];
+		eb_handles[1] = loc_handles[try];
+		eb_handles[2] = loc_handles[try + 1];
+
+		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
+				offsets) == 0);
+
+		if ((offsets[1] - offsets[0]) == PAGE_SIZE) {
+			neighbours = true;
+			pad_to_size[0] = 2 * PAGE_SIZE;
+			break;
+		}
+
+		try++;
+
+		loc_handles[try + 1] = gem_create(fd, PAGE_SIZE);
+		igt_assert(loc_handles[try + 1]);
+	}
+
+	/* Otherwise test can't confidently run. */
+	if (neighbours) {
+		/* Re-exec with padding set. */
+		igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
+
+		/* Check that objects with padding do not overlap. */
+		igt_assert(offsets[0] >= offsets[1] + PAGE_SIZE ||
+			offsets[0] + 2 * PAGE_SIZE <= offsets[1]);
+	}
+
+	/* Cleanup our bos. */
+	for (i = 0;  i < try; i++)
+		gem_close(fd, loc_handles[2 + i]);
+
+	free(loc_handles);
+
+	igt_require(neighbours);
+}
+
+uint32_t batch[2] = {MI_BATCH_BUFFER_END};
+uint32_t handles[3];
+int fd;
+
+igt_main
+{
+	igt_fixture {
+		fd = drm_open_any();
+
+		handles[0] = gem_create(fd, PAGE_SIZE);
+		gem_write(fd, handles[0], 0, batch, sizeof(batch));
+
+		handles[1] = gem_create(fd, PAGE_SIZE);
+		handles[2] = gem_create(fd, PAGE_SIZE);
+	}
+
+	igt_subtest("pad_to_size")
+		require_pad_to_size(fd, handles);
+
+	igt_subtest("not_aligned")
+		not_aligned(fd, handles);
+
+	igt_subtest("padding")
+		padding(fd, handles);
+
+	igt_fixture {
+		gem_close(fd, handles[0]);
+		gem_close(fd, handles[1]);
+		gem_close(fd, handles[2]);
+
+		close(fd);
+	}
+}
-- 
2.3.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v3] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-01 16:39         ` Chris Wilson
@ 2015-04-02 10:47           ` Tvrtko Ursulin
  0 siblings, 0 replies; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-02 10:47 UTC (permalink / raw)
  To: Chris Wilson, Intel-gfx, Tvrtko Ursulin


On 04/01/2015 05:39 PM, Chris Wilson wrote:
> On Wed, Apr 01, 2015 at 05:31:16PM +0100, Chris Wilson wrote:
>> On Wed, Apr 01, 2015 at 05:07:25PM +0100, Tvrtko Ursulin wrote:
>>>
>>> On 04/01/2015 04:42 PM, Chris Wilson wrote:
>>>> On Wed, Apr 01, 2015 at 04:14:52PM +0100, Tvrtko Ursulin wrote:
>>>>> +	/* Re-exec with padding set. */
>>>>> +	igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
>>>>
>>>> The crux of the test is that we generate two objects such that
>>>>
>>>> B_offset = A_offset + A_size
>>>>
>>>> and then tell the kernel that A is actually 2*size (A_pad_to_size)
>>>>
>>>>> +	if (offsets[1] > offsets[0])
>>>>> +		distance = offsets[1] - offsets[0];
>>>>> +	else
>>>>> +		distance = offsets[0] - offsets[1];
>>>>
>>>> The assertion I feel should only be that
>>>>
>>>> B_offset + B_size <= A_offset && B_offset >= A_offset + A_pad_to_size
>>>
>>> I don't get this. B starts after A + padding, but B ends before A?
>>
>> s/&&/||/
>
> Sorry &&. Gah, must be time for a coffee break.
>
> The assertion is that the objects do not overlap based on the pad_to_size we
> expect the kernel to apply, rather than their natural size. If the
> kernel doesn't move the objects, they would it would fail.

You know all this time I didn't realize you were saying me check was 
broken, just that it was confusing. :)

Regards,

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v4] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-02 10:45 ` [RFC i-g-t v4] " Tvrtko Ursulin
@ 2015-04-02 11:02   ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2015-04-02 11:02 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel-gfx

On Thu, Apr 02, 2015 at 11:45:59AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.
> 
> Similar to some other tests, it uses knowledge of the DRM
> allocation policy in order to get two objects mapped adjacent
> to each other. It is then possible to verify that the pad to
> size flag will move them apart.
> 
> v2: Correct commit message. (Chris Wilson)
> v3: Changes after code review by Chris Wilson.
>      * No need for gem_sync after execbuf.
>      * Drop caches before running.
>      * Allocate one additional bo per iteration.
>      * Don't explicitly set unused execbuf fields to zero.
>      * One improved comment.
> v4: Require simpler object ordering and fixed overlap test. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

Thanks, that reads really well.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Hah, just thought of one minor tweak I would do though:

		if ((offsets[1] - offsets[0]) == PAGE_SIZE) {
			neighbours = true;
			break;
		}
		....
	}

	/* Otherwise test can't confidently run. */
	if (neighbours) {
		/* Check the object don't move by themselves */
		igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
		igt_assert(offsets[1] - offset[0] == PAGE_SIZE);

		/* Then re-exec with padding set, and now they should move. */
		pad_to_size[0] = 2*PAGE_SIZE;
		igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);

		/* Check that objects with padding do not overlap. */
		igt_assert(offsets[0] >= offsets[1] + PAGE_SIZE ||
			   offsets[0] + 2 * PAGE_SIZE <= offsets[1]);
	}

-Chris

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC i-g-t v5] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-03-25 14:21 [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf Tvrtko Ursulin
                   ` (3 preceding siblings ...)
  2015-04-02 10:45 ` [RFC i-g-t v4] " Tvrtko Ursulin
@ 2015-04-02 12:54 ` Tvrtko Ursulin
  2015-04-02 16:09   ` Thomas Wood
  4 siblings, 1 reply; 22+ messages in thread
From: Tvrtko Ursulin @ 2015-04-02 12:54 UTC (permalink / raw)
  To: Intel-gfx

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.

Similar to some other tests, it uses knowledge of the DRM
allocation policy in order to get two objects mapped adjacent
to each other. It is then possible to verify that the pad to
size flag will move them apart.

v2: Correct commit message. (Chris Wilson)
v3: Changes after code review by Chris Wilson.
     * No need for gem_sync after execbuf.
     * Drop caches before running.
     * Allocate one additional bo per iteration.
     * Don't explicitly set unused execbuf fields to zero.
     * One improved comment.
v4: Require simpler object ordering and fixed overlap test. (Chris Wilson)
v5: Check unpadded offsets once more before padding. (Chris Wilson)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 tests/Makefile.sources       |   1 +
 tests/gem_exec_pad_to_size.c | 240 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 241 insertions(+)
 create mode 100644 tests/gem_exec_pad_to_size.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 0a974a6..5f21728 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -34,6 +34,7 @@ TESTS_progs_M = \
 	gem_exec_bad_domains \
 	gem_exec_faulting_reloc \
 	gem_exec_nop \
+	gem_exec_pad_to_size \
 	gem_exec_params \
 	gem_exec_parse \
 	gem_fenced_exec_thrash \
diff --git a/tests/gem_exec_pad_to_size.c b/tests/gem_exec_pad_to_size.c
new file mode 100644
index 0000000..87afe40
--- /dev/null
+++ b/tests/gem_exec_pad_to_size.c
@@ -0,0 +1,240 @@
+/*
+ * Copyright © 2015 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:
+ *    Tvrtko Ursulin <tvrtko.ursulin@intel.com>
+ *
+ */
+
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <errno.h>
+#include <sys/stat.h>
+#include <sys/ioctl.h>
+#include <sys/time.h>
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "igt_core.h"
+#include "drmtest.h"
+#include "intel_reg.h"
+#include "igt_debugfs.h"
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+struct local_drm_i915_gem_exec_object2 {
+	/**
+	 * User's handle for a buffer to be bound into the GTT for this
+	 * operation.
+	 */
+	__u32 handle;
+
+	/** Number of relocations to be performed on this buffer */
+	__u32 relocation_count;
+	/**
+	 * Pointer to array of struct drm_i915_gem_relocation_entry containing
+	 * the relocations to be performed in this buffer.
+	 */
+	__u64 relocs_ptr;
+
+	/** Required alignment in graphics aperture */
+	__u64 alignment;
+
+	/**
+	 * Returned value of the updated offset of the object, for future
+	 * presumed_offset writes.
+	 */
+	__u64 offset;
+
+#define LOCAL_EXEC_OBJECT_NEEDS_FENCE (1<<0)
+#define LOCAL_EXEC_OBJECT_NEEDS_GTT	(1<<1)
+#define LOCAL_EXEC_OBJECT_WRITE	(1<<2)
+#define LOCAL_EXEC_OBJECT_PAD_TO_SIZE (1<<3)
+#define __LOCAL_EXEC_OBJECT_UNKNOWN_FLAGS -(LOCAL_EXEC_OBJECT_PAD_TO_SIZE<<1)
+	__u64 flags;
+
+	__u64 pad_to_size;
+	__u64 rsvd2;
+};
+
+static int
+exec(int fd, uint32_t handles[3], uint32_t pad_to_size[2], uint64_t *offsets)
+{
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct local_drm_i915_gem_exec_object2 gem_exec[3];
+	int ret = 0;
+
+	memset(gem_exec, 0, sizeof(gem_exec));
+
+	gem_exec[0].handle = handles[1];
+	gem_exec[0].flags = pad_to_size[0] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[0].pad_to_size = pad_to_size[0];
+
+	gem_exec[1].handle = handles[2];
+	gem_exec[1].flags = pad_to_size[1] ?
+			    LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
+	gem_exec[1].pad_to_size = pad_to_size[1];
+
+	gem_exec[2].handle = handles[0];
+
+	memset(&execbuf, 0, sizeof(execbuf));
+
+	execbuf.buffers_ptr = (uintptr_t)gem_exec;
+	execbuf.buffer_count = 3;
+	execbuf.batch_len = 8;
+
+	if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
+			ret = -errno;
+
+	if (ret == 0 && offsets) {
+		offsets[0] = gem_exec[0].offset;
+		offsets[1] = gem_exec[1].offset;
+	}
+
+	return ret;
+}
+
+static void
+require_pad_to_size(int fd, uint32_t handles[3])
+{
+	igt_require(exec(fd, handles, (uint32_t[2]){ PAGE_SIZE, PAGE_SIZE },
+			 NULL) == 0);
+}
+
+static void
+not_aligned(int fd, uint32_t handles[3])
+{
+	require_pad_to_size(fd, handles);
+
+	igt_require(exec(fd, handles, (uint32_t[2]){1 ,1},
+			 NULL) == -EINVAL);
+}
+
+static void
+padding(int fd, uint32_t handles[3])
+{
+	uint32_t pad_to_size[2] = {0, 0};
+	uint64_t offsets[2];
+	bool neighbours = false;
+	unsigned int try, i;
+	const unsigned int max_tries = 4096; /* Finger in the air. */
+	uint32_t *loc_handles;
+	uint32_t eb_handles[3];
+
+	require_pad_to_size(fd, handles);
+
+	igt_drop_caches_set(DROP_ALL);
+
+	loc_handles = calloc(max_tries * 2, sizeof(uint32_t));
+	igt_assert(loc_handles);
+
+	/* Try with passed in handles first. */
+	loc_handles[0] = handles[1];
+	loc_handles[1] = handles[2];
+
+	/* Try to get two buffer object next to each other in GTT space.
+	 * We presume that sequential reservations are likely to be aligned and
+	 * try until we find a pair that is.
+	 */
+	for (try = 0; try < max_tries;) {
+		eb_handles[0] = handles[0];
+		eb_handles[1] = loc_handles[try];
+		eb_handles[2] = loc_handles[try + 1];
+
+		igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
+				offsets) == 0);
+
+		if ((offsets[1] - offsets[0]) == PAGE_SIZE) {
+			neighbours = true;
+			break;
+		}
+
+		try++;
+
+		loc_handles[try + 1] = gem_create(fd, PAGE_SIZE);
+		igt_assert(loc_handles[try + 1]);
+	}
+
+	/* Otherwise test can't confidently run. */
+	if (neighbours) {
+		/* Check the object don't move by themselves */
+		igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
+		igt_assert((offsets[1] - offsets[0]) == PAGE_SIZE);
+
+		/* Re-exec with padding set. */
+		pad_to_size[0] = 2 * PAGE_SIZE;
+		igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
+
+		/* Check that objects with padding do not overlap. */
+		igt_assert(offsets[0] >= offsets[1] + PAGE_SIZE ||
+			   offsets[0] + 2 * PAGE_SIZE <= offsets[1]);
+	}
+
+	/* Cleanup our bos. */
+	for (i = 0;  i < try; i++)
+		gem_close(fd, loc_handles[2 + i]);
+
+	free(loc_handles);
+
+	igt_require(neighbours);
+}
+
+uint32_t batch[2] = {MI_BATCH_BUFFER_END};
+uint32_t handles[3];
+int fd;
+
+igt_main
+{
+	igt_fixture {
+		fd = drm_open_any();
+
+		handles[0] = gem_create(fd, PAGE_SIZE);
+		gem_write(fd, handles[0], 0, batch, sizeof(batch));
+
+		handles[1] = gem_create(fd, PAGE_SIZE);
+		handles[2] = gem_create(fd, PAGE_SIZE);
+	}
+
+	igt_subtest("pad_to_size")
+		require_pad_to_size(fd, handles);
+
+	igt_subtest("not_aligned")
+		not_aligned(fd, handles);
+
+	igt_subtest("padding")
+		padding(fd, handles);
+
+	igt_fixture {
+		gem_close(fd, handles[0]);
+		gem_close(fd, handles[1]);
+		gem_close(fd, handles[2]);
+
+		close(fd);
+	}
+}
-- 
2.3.2

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC i-g-t v5] tests/gem_exec_pad_to_size: Test object padding at execbuf
  2015-04-02 12:54 ` [RFC i-g-t v5] " Tvrtko Ursulin
@ 2015-04-02 16:09   ` Thomas Wood
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Wood @ 2015-04-02 16:09 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: Intel Graphics Development

On 2 April 2015 at 13:54, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> This tests the new EXEC_OBJECT_PAD_TO_SIZE exec_object2 flag.

Just two things from an i-g-t perspective: the new binary needs adding
to .gitignore and it would be good to include a short description of
the test using the IGT_TEST_DESCRIPTION macro.


>
> Similar to some other tests, it uses knowledge of the DRM
> allocation policy in order to get two objects mapped adjacent
> to each other. It is then possible to verify that the pad to
> size flag will move them apart.
>
> v2: Correct commit message. (Chris Wilson)
> v3: Changes after code review by Chris Wilson.
>      * No need for gem_sync after execbuf.
>      * Drop caches before running.
>      * Allocate one additional bo per iteration.
>      * Don't explicitly set unused execbuf fields to zero.
>      * One improved comment.
> v4: Require simpler object ordering and fixed overlap test. (Chris Wilson)
> v5: Check unpadded offsets once more before padding. (Chris Wilson)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  tests/Makefile.sources       |   1 +
>  tests/gem_exec_pad_to_size.c | 240 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 241 insertions(+)
>  create mode 100644 tests/gem_exec_pad_to_size.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 0a974a6..5f21728 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -34,6 +34,7 @@ TESTS_progs_M = \
>         gem_exec_bad_domains \
>         gem_exec_faulting_reloc \
>         gem_exec_nop \
> +       gem_exec_pad_to_size \
>         gem_exec_params \
>         gem_exec_parse \
>         gem_fenced_exec_thrash \
> diff --git a/tests/gem_exec_pad_to_size.c b/tests/gem_exec_pad_to_size.c
> new file mode 100644
> index 0000000..87afe40
> --- /dev/null
> +++ b/tests/gem_exec_pad_to_size.c
> @@ -0,0 +1,240 @@
> +/*
> + * Copyright © 2015 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:
> + *    Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> + *
> + */
> +
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/ioctl.h>
> +#include <sys/time.h>
> +#include "drm.h"
> +#include "ioctl_wrappers.h"
> +#include "igt_core.h"
> +#include "drmtest.h"
> +#include "intel_reg.h"
> +#include "igt_debugfs.h"
> +
> +#ifndef PAGE_SIZE
> +#define PAGE_SIZE 4096
> +#endif
> +
> +struct local_drm_i915_gem_exec_object2 {
> +       /**
> +        * User's handle for a buffer to be bound into the GTT for this
> +        * operation.
> +        */
> +       __u32 handle;
> +
> +       /** Number of relocations to be performed on this buffer */
> +       __u32 relocation_count;
> +       /**
> +        * Pointer to array of struct drm_i915_gem_relocation_entry containing
> +        * the relocations to be performed in this buffer.
> +        */
> +       __u64 relocs_ptr;
> +
> +       /** Required alignment in graphics aperture */
> +       __u64 alignment;
> +
> +       /**
> +        * Returned value of the updated offset of the object, for future
> +        * presumed_offset writes.
> +        */
> +       __u64 offset;
> +
> +#define LOCAL_EXEC_OBJECT_NEEDS_FENCE (1<<0)
> +#define LOCAL_EXEC_OBJECT_NEEDS_GTT    (1<<1)
> +#define LOCAL_EXEC_OBJECT_WRITE        (1<<2)
> +#define LOCAL_EXEC_OBJECT_PAD_TO_SIZE (1<<3)
> +#define __LOCAL_EXEC_OBJECT_UNKNOWN_FLAGS -(LOCAL_EXEC_OBJECT_PAD_TO_SIZE<<1)
> +       __u64 flags;
> +
> +       __u64 pad_to_size;
> +       __u64 rsvd2;
> +};
> +
> +static int
> +exec(int fd, uint32_t handles[3], uint32_t pad_to_size[2], uint64_t *offsets)
> +{
> +       struct drm_i915_gem_execbuffer2 execbuf;
> +       struct local_drm_i915_gem_exec_object2 gem_exec[3];
> +       int ret = 0;
> +
> +       memset(gem_exec, 0, sizeof(gem_exec));
> +
> +       gem_exec[0].handle = handles[1];
> +       gem_exec[0].flags = pad_to_size[0] ?
> +                           LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
> +       gem_exec[0].pad_to_size = pad_to_size[0];
> +
> +       gem_exec[1].handle = handles[2];
> +       gem_exec[1].flags = pad_to_size[1] ?
> +                           LOCAL_EXEC_OBJECT_PAD_TO_SIZE : 0;
> +       gem_exec[1].pad_to_size = pad_to_size[1];
> +
> +       gem_exec[2].handle = handles[0];
> +
> +       memset(&execbuf, 0, sizeof(execbuf));
> +
> +       execbuf.buffers_ptr = (uintptr_t)gem_exec;
> +       execbuf.buffer_count = 3;
> +       execbuf.batch_len = 8;
> +
> +       if (drmIoctl(fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf))
> +                       ret = -errno;
> +
> +       if (ret == 0 && offsets) {
> +               offsets[0] = gem_exec[0].offset;
> +               offsets[1] = gem_exec[1].offset;
> +       }
> +
> +       return ret;
> +}
> +
> +static void
> +require_pad_to_size(int fd, uint32_t handles[3])
> +{
> +       igt_require(exec(fd, handles, (uint32_t[2]){ PAGE_SIZE, PAGE_SIZE },
> +                        NULL) == 0);
> +}
> +
> +static void
> +not_aligned(int fd, uint32_t handles[3])
> +{
> +       require_pad_to_size(fd, handles);
> +
> +       igt_require(exec(fd, handles, (uint32_t[2]){1 ,1},
> +                        NULL) == -EINVAL);
> +}
> +
> +static void
> +padding(int fd, uint32_t handles[3])
> +{
> +       uint32_t pad_to_size[2] = {0, 0};
> +       uint64_t offsets[2];
> +       bool neighbours = false;
> +       unsigned int try, i;
> +       const unsigned int max_tries = 4096; /* Finger in the air. */
> +       uint32_t *loc_handles;
> +       uint32_t eb_handles[3];
> +
> +       require_pad_to_size(fd, handles);
> +
> +       igt_drop_caches_set(DROP_ALL);
> +
> +       loc_handles = calloc(max_tries * 2, sizeof(uint32_t));
> +       igt_assert(loc_handles);
> +
> +       /* Try with passed in handles first. */
> +       loc_handles[0] = handles[1];
> +       loc_handles[1] = handles[2];
> +
> +       /* Try to get two buffer object next to each other in GTT space.
> +        * We presume that sequential reservations are likely to be aligned and
> +        * try until we find a pair that is.
> +        */
> +       for (try = 0; try < max_tries;) {
> +               eb_handles[0] = handles[0];
> +               eb_handles[1] = loc_handles[try];
> +               eb_handles[2] = loc_handles[try + 1];
> +
> +               igt_assert(exec(fd, eb_handles, (uint32_t[2]){0, 0},
> +                               offsets) == 0);
> +
> +               if ((offsets[1] - offsets[0]) == PAGE_SIZE) {
> +                       neighbours = true;
> +                       break;
> +               }
> +
> +               try++;
> +
> +               loc_handles[try + 1] = gem_create(fd, PAGE_SIZE);
> +               igt_assert(loc_handles[try + 1]);
> +       }
> +
> +       /* Otherwise test can't confidently run. */
> +       if (neighbours) {
> +               /* Check the object don't move by themselves */
> +               igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
> +               igt_assert((offsets[1] - offsets[0]) == PAGE_SIZE);
> +
> +               /* Re-exec with padding set. */
> +               pad_to_size[0] = 2 * PAGE_SIZE;
> +               igt_assert(exec(fd, eb_handles, pad_to_size, offsets) == 0);
> +
> +               /* Check that objects with padding do not overlap. */
> +               igt_assert(offsets[0] >= offsets[1] + PAGE_SIZE ||
> +                          offsets[0] + 2 * PAGE_SIZE <= offsets[1]);
> +       }
> +
> +       /* Cleanup our bos. */
> +       for (i = 0;  i < try; i++)
> +               gem_close(fd, loc_handles[2 + i]);
> +
> +       free(loc_handles);
> +
> +       igt_require(neighbours);
> +}
> +
> +uint32_t batch[2] = {MI_BATCH_BUFFER_END};
> +uint32_t handles[3];
> +int fd;
> +
> +igt_main
> +{
> +       igt_fixture {
> +               fd = drm_open_any();
> +
> +               handles[0] = gem_create(fd, PAGE_SIZE);
> +               gem_write(fd, handles[0], 0, batch, sizeof(batch));
> +
> +               handles[1] = gem_create(fd, PAGE_SIZE);
> +               handles[2] = gem_create(fd, PAGE_SIZE);
> +       }
> +
> +       igt_subtest("pad_to_size")
> +               require_pad_to_size(fd, handles);
> +
> +       igt_subtest("not_aligned")
> +               not_aligned(fd, handles);
> +
> +       igt_subtest("padding")
> +               padding(fd, handles);
> +
> +       igt_fixture {
> +               gem_close(fd, handles[0]);
> +               gem_close(fd, handles[1]);
> +               gem_close(fd, handles[2]);
> +
> +               close(fd);
> +       }
> +}
> --
> 2.3.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-04-02 16:10 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-25 14:21 [RFC i-g-t] tests/gem_exec_pad_to_size: Test object padding at execbuf Tvrtko Ursulin
2015-03-25 14:28 ` Chris Wilson
2015-03-25 14:32   ` Tvrtko Ursulin
2015-03-25 14:41     ` Chris Wilson
2015-03-25 14:57       ` Tvrtko Ursulin
2015-04-01 11:21 ` [RFC i-g-t v2] " Tvrtko Ursulin
2015-04-01 13:06   ` Chris Wilson
2015-04-01 13:17     ` Chris Wilson
2015-04-01 13:36     ` Tvrtko Ursulin
2015-04-01 13:56       ` Chris Wilson
2015-04-01 14:14         ` Tvrtko Ursulin
2015-04-01 14:27           ` Chris Wilson
2015-04-01 15:14 ` [RFC i-g-t v3] " Tvrtko Ursulin
2015-04-01 15:42   ` Chris Wilson
2015-04-01 16:07     ` Tvrtko Ursulin
2015-04-01 16:31       ` Chris Wilson
2015-04-01 16:39         ` Chris Wilson
2015-04-02 10:47           ` Tvrtko Ursulin
2015-04-02 10:45 ` [RFC i-g-t v4] " Tvrtko Ursulin
2015-04-02 11:02   ` Chris Wilson
2015-04-02 12:54 ` [RFC i-g-t v5] " Tvrtko Ursulin
2015-04-02 16:09   ` Thomas Wood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.