All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
@ 2014-10-15 21:52 bradley.d.volkin
  2014-10-15 21:52 ` [PATCH 2/2] tests/gem_exec_parse: test for chained batch buffers bradley.d.volkin
  2014-10-21 15:26 ` [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page Daniel Vetter
  0 siblings, 2 replies; 7+ messages in thread
From: bradley.d.volkin @ 2014-10-15 21:52 UTC (permalink / raw)
  To: intel-gfx

From: Brad Volkin <bradley.d.volkin@intel.com>

The size of the batch buffer passed to the kernel is significantly
larger than the size of the batch buffer passed to the function. A
proposed optimization as part of the batch copy kernel series is to
use batch_len for the copy and parse operations, which leads to a
false "batch without MI_BATCH_BUFFER_END" failure for this test.

Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
---
 tests/gem_exec_parse.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 05f271c..568bd4a 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -144,9 +144,10 @@ static void exec_split_batch(int fd, uint32_t *cmds,
 	struct drm_i915_gem_exec_object2 objs[1];
 	uint32_t cmd_bo;
 	uint32_t noop[1024] = { 0 };
+	const int alloc_size = 4096 * 2;
 
 	// Allocate and fill a 2-page batch with noops
-	cmd_bo = gem_create(fd, 4096 * 2);
+	cmd_bo = gem_create(fd, alloc_size);
 	gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
 	gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
 
@@ -167,7 +168,7 @@ static void exec_split_batch(int fd, uint32_t *cmds,
 	execbuf.buffers_ptr = (uintptr_t)objs;
 	execbuf.buffer_count = 1;
 	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = size;
+	execbuf.batch_len = alloc_size;
 	execbuf.cliprects_ptr = 0;
 	execbuf.num_cliprects = 0;
 	execbuf.DR1 = 0;
-- 
1.9.1

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

* [PATCH 2/2] tests/gem_exec_parse: test for chained batch buffers
  2014-10-15 21:52 [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page bradley.d.volkin
@ 2014-10-15 21:52 ` bradley.d.volkin
  2014-10-21 15:26 ` [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page Daniel Vetter
  1 sibling, 0 replies; 7+ messages in thread
From: bradley.d.volkin @ 2014-10-15 21:52 UTC (permalink / raw)
  To: intel-gfx

From: Brad Volkin <bradley.d.volkin@intel.com>

libva makes extensive use of chained batch buffers. The batch
buffer copy portion of the command parser has the potential to
break chained batches, so add a simple test to make sure that
doesn't happen.

Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
---
 lib/intel_reg.h        |   1 +
 tests/gem_exec_parse.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/lib/intel_reg.h b/lib/intel_reg.h
index f0fc5fd..fcc9d7c 100644
--- a/lib/intel_reg.h
+++ b/lib/intel_reg.h
@@ -2571,6 +2571,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #define MI_BATCH_BUFFER_END	(0xA << 23)
 #define MI_BATCH_NON_SECURE		(1)
 #define MI_BATCH_NON_SECURE_I965	(1 << 8)
+#define MI_BATCH_NON_SECURE_HSW		(1<<13) /* Additional bit for RCS */
 
 #define MAX_DISPLAY_PIPES	2
 
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 568bd4a..3ff6a66 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -183,6 +183,96 @@ static void exec_split_batch(int fd, uint32_t *cmds,
 	gem_close(fd, cmd_bo);
 }
 
+static void exec_batch_chained(int fd, uint32_t cmd_bo, uint32_t *cmds,
+			       int size, int patch_offset,
+			       uint64_t expected_value)
+{
+	struct drm_i915_gem_execbuffer2 execbuf;
+	struct drm_i915_gem_exec_object2 objs[3];
+	struct drm_i915_gem_relocation_entry reloc;
+	struct drm_i915_gem_relocation_entry first_level_reloc;
+
+	uint32_t target_bo = gem_create(fd, 4096);
+	uint32_t first_level_bo = gem_create(fd, 4096);
+	uint64_t actual_value = 0;
+
+	static uint32_t first_level_cmds[] = {
+		MI_BATCH_BUFFER_START | MI_BATCH_NON_SECURE_I965,
+		0,
+		MI_BATCH_BUFFER_END,
+		0,
+	};
+
+	if (IS_HASWELL(intel_get_drm_devid(fd)))
+		first_level_cmds[0] |= MI_BATCH_NON_SECURE_HSW;
+
+	gem_write(fd, first_level_bo, 0,
+		  first_level_cmds, sizeof(first_level_cmds));
+	gem_write(fd, cmd_bo, 0, cmds, size);
+
+	reloc.offset = patch_offset;
+	reloc.delta = 0;
+	reloc.target_handle = target_bo;
+	reloc.read_domains = I915_GEM_DOMAIN_RENDER;
+	reloc.write_domain = I915_GEM_DOMAIN_RENDER;
+	reloc.presumed_offset = 0;
+
+	first_level_reloc.offset = 4;
+	first_level_reloc.delta = 0;
+	first_level_reloc.target_handle = cmd_bo;
+	first_level_reloc.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+	first_level_reloc.write_domain = 0;
+	first_level_reloc.presumed_offset = 0;
+
+	objs[0].handle = target_bo;
+	objs[0].relocation_count = 0;
+	objs[0].relocs_ptr = 0;
+	objs[0].alignment = 0;
+	objs[0].offset = 0;
+	objs[0].flags = 0;
+	objs[0].rsvd1 = 0;
+	objs[0].rsvd2 = 0;
+
+	objs[1].handle = cmd_bo;
+	objs[1].relocation_count = 1;
+	objs[1].relocs_ptr = (uintptr_t)&reloc;
+	objs[1].alignment = 0;
+	objs[1].offset = 0;
+	objs[1].flags = 0;
+	objs[1].rsvd1 = 0;
+	objs[1].rsvd2 = 0;
+
+	objs[2].handle = first_level_bo;
+	objs[2].relocation_count = 1;
+	objs[2].relocs_ptr = (uintptr_t)&first_level_reloc;
+	objs[2].alignment = 0;
+	objs[2].offset = 0;
+	objs[2].flags = 0;
+	objs[2].rsvd1 = 0;
+	objs[2].rsvd2 = 0;
+
+	execbuf.buffers_ptr = (uintptr_t)objs;
+	execbuf.buffer_count = 3;
+	execbuf.batch_start_offset = 0;
+	execbuf.batch_len = sizeof(first_level_cmds);
+	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;
+
+	gem_execbuf(fd, &execbuf);
+	gem_sync(fd, cmd_bo);
+
+	gem_read(fd,target_bo, 0, &actual_value, sizeof(actual_value));
+	igt_assert_eq(expected_value, actual_value);
+
+	gem_close(fd, first_level_bo);
+	gem_close(fd, target_bo);
+}
+
 uint32_t handle;
 int fd;
 
@@ -366,6 +456,21 @@ igt_main
 			   -EINVAL);
 	}
 
+	igt_subtest("chained-batch") {
+		uint32_t pc[] = {
+			GFX_OP_PIPE_CONTROL,
+			PIPE_CONTROL_QW_WRITE,
+			0, // To be patched
+			0x12000000,
+			0,
+			MI_BATCH_BUFFER_END,
+		};
+		exec_batch_chained(fd, handle,
+				   pc, sizeof(pc),
+				   8, // patch offset,
+				   0x12000000);
+	}
+
 	igt_fixture {
 		gem_close(fd, handle);
 
-- 
1.9.1

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

* Re: [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
  2014-10-15 21:52 [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page bradley.d.volkin
  2014-10-15 21:52 ` [PATCH 2/2] tests/gem_exec_parse: test for chained batch buffers bradley.d.volkin
@ 2014-10-21 15:26 ` Daniel Vetter
  2014-10-22 15:29   ` Volkin, Bradley D
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2014-10-21 15:26 UTC (permalink / raw)
  To: bradley.d.volkin; +Cc: intel-gfx

On Wed, Oct 15, 2014 at 02:52:41PM -0700, bradley.d.volkin@intel.com wrote:
> From: Brad Volkin <bradley.d.volkin@intel.com>
> 
> The size of the batch buffer passed to the kernel is significantly
> larger than the size of the batch buffer passed to the function. A
> proposed optimization as part of the batch copy kernel series is to
> use batch_len for the copy and parse operations, which leads to a
> false "batch without MI_BATCH_BUFFER_END" failure for this test.
> 
> Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> ---
>  tests/gem_exec_parse.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
> index 05f271c..568bd4a 100644
> --- a/tests/gem_exec_parse.c
> +++ b/tests/gem_exec_parse.c
> @@ -144,9 +144,10 @@ static void exec_split_batch(int fd, uint32_t *cmds,
>  	struct drm_i915_gem_exec_object2 objs[1];
>  	uint32_t cmd_bo;
>  	uint32_t noop[1024] = { 0 };
> +	const int alloc_size = 4096 * 2;
>  
>  	// Allocate and fill a 2-page batch with noops
> -	cmd_bo = gem_create(fd, 4096 * 2);
> +	cmd_bo = gem_create(fd, alloc_size);
>  	gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
>  	gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
>  
> @@ -167,7 +168,7 @@ static void exec_split_batch(int fd, uint32_t *cmds,
>  	execbuf.buffers_ptr = (uintptr_t)objs;
>  	execbuf.buffer_count = 1;
>  	execbuf.batch_start_offset = 0;

Shouldn't we simply adjust batch_start_offset to be 4096-4, i.e. where
we've placed the batch? Would have the benifit of also testing offset
batches ;-)
-Daniel

> -	execbuf.batch_len = size;
> +	execbuf.batch_len = alloc_size;
>  	execbuf.cliprects_ptr = 0;
>  	execbuf.num_cliprects = 0;
>  	execbuf.DR1 = 0;
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
  2014-10-21 15:26 ` [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page Daniel Vetter
@ 2014-10-22 15:29   ` Volkin, Bradley D
  0 siblings, 0 replies; 7+ messages in thread
From: Volkin, Bradley D @ 2014-10-22 15:29 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Tue, Oct 21, 2014 at 08:26:05AM -0700, Daniel Vetter wrote:
> On Wed, Oct 15, 2014 at 02:52:41PM -0700, bradley.d.volkin@intel.com wrote:
> > From: Brad Volkin <bradley.d.volkin@intel.com>
> > 
> > The size of the batch buffer passed to the kernel is significantly
> > larger than the size of the batch buffer passed to the function. A
> > proposed optimization as part of the batch copy kernel series is to
> > use batch_len for the copy and parse operations, which leads to a
> > false "batch without MI_BATCH_BUFFER_END" failure for this test.
> > 
> > Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> > ---
> >  tests/gem_exec_parse.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
> > index 05f271c..568bd4a 100644
> > --- a/tests/gem_exec_parse.c
> > +++ b/tests/gem_exec_parse.c
> > @@ -144,9 +144,10 @@ static void exec_split_batch(int fd, uint32_t *cmds,
> >  	struct drm_i915_gem_exec_object2 objs[1];
> >  	uint32_t cmd_bo;
> >  	uint32_t noop[1024] = { 0 };
> > +	const int alloc_size = 4096 * 2;
> >  
> >  	// Allocate and fill a 2-page batch with noops
> > -	cmd_bo = gem_create(fd, 4096 * 2);
> > +	cmd_bo = gem_create(fd, alloc_size);
> >  	gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
> >  	gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
> >  
> > @@ -167,7 +168,7 @@ static void exec_split_batch(int fd, uint32_t *cmds,
> >  	execbuf.buffers_ptr = (uintptr_t)objs;
> >  	execbuf.buffer_count = 1;
> >  	execbuf.batch_start_offset = 0;
> 
> Shouldn't we simply adjust batch_start_offset to be 4096-4, i.e. where
> we've placed the batch? Would have the benifit of also testing offset
> batches ;-)
> -Daniel

Good suggestion. I'll go with that approach instead.

Brad

> 
> > -	execbuf.batch_len = size;
> > +	execbuf.batch_len = alloc_size;
> >  	execbuf.cliprects_ptr = 0;
> >  	execbuf.num_cliprects = 0;
> >  	execbuf.DR1 = 0;
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
  2014-11-06 22:39 ` Volkin, Bradley D
@ 2014-11-07  9:42   ` Daniel Vetter
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2014-11-07  9:42 UTC (permalink / raw)
  To: Volkin, Bradley D; +Cc: intel-gfx

On Thu, Nov 06, 2014 at 02:39:53PM -0800, Volkin, Bradley D wrote:
> Ping on this series. They're related to the batch copy series, but
> the changes are valid and tests should still pass even without the
> kernel changes being merged.

Merged - I kinda forgotten that you don't have commit access.
> 
> On Mon, Nov 03, 2014 at 11:18:59AM -0800, Volkin, Bradley D wrote:
> > From: Brad Volkin <bradley.d.volkin@intel.com>
> > 
> > The size of the batch buffer passed to the kernel is significantly
> > larger than the size of the batch buffer passed to the function. A
> > proposed optimization as part of the batch copy kernel series is to
> > use batch_len for the copy and parse operations, which leads to a
> > false "batch without MI_BATCH_BUFFER_END" failure for this test.
> > 
> > To fix this, modify the test to set batch_start_offset and batch_len
> > such that they define the range of actual commands in the batch,
> > including a few of the surrounding nops for alignment purposes.
> > 
> > v2: update batch_start_offset as well
> > 
> > Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> > ---
> >  tests/gem_exec_parse.c | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
> > index 1dc9103..e48b83a 100644
> > --- a/tests/gem_exec_parse.c
> > +++ b/tests/gem_exec_parse.c
> > @@ -144,16 +144,18 @@ static void exec_split_batch(int fd, uint32_t *cmds,
> >  	struct drm_i915_gem_exec_object2 objs[1];
> >  	uint32_t cmd_bo;
> >  	uint32_t noop[1024] = { 0 };
> > +	const int alloc_size = 4096 * 2;
> > +	const int actual_start_offset = 4096-sizeof(uint32_t);
> >  
> >  	// Allocate and fill a 2-page batch with noops
> > -	cmd_bo = gem_create(fd, 4096 * 2);
> > +	cmd_bo = gem_create(fd, alloc_size);
> >  	gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
> >  	gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
> >  
> >  	// Write the provided commands such that the first dword
> >  	// of the command buffer is the last dword of the first
> >  	// page (i.e. the command is split across the two pages).
> > -	gem_write(fd, cmd_bo, 4096-sizeof(uint32_t), cmds, size);
> > +	gem_write(fd, cmd_bo, actual_start_offset, cmds, size);
> >  
> >  	objs[0].handle = cmd_bo;
> >  	objs[0].relocation_count = 0;
> > @@ -166,8 +168,14 @@ static void exec_split_batch(int fd, uint32_t *cmds,
> >  
> >  	execbuf.buffers_ptr = (uintptr_t)objs;
> >  	execbuf.buffer_count = 1;
> > -	execbuf.batch_start_offset = 0;
> > -	execbuf.batch_len = size;
> > +	// NB: We want batch_start_offset and batch_len to point to the block
> > +	// of the actual commands (i.e. at the last dword of the first page),
> > +	// but have to adjust both the start offset and length to meet the
> > +	// kernel driver's requirements on the alignment of those fields.

Also pushed a patch on top to change all the comments to C-style, just for
my OCD ;-)
-Daniel

> > +	execbuf.batch_start_offset = actual_start_offset & ~0x7;
> > +	execbuf.batch_len =
> > +		ALIGN(size + actual_start_offset - execbuf.batch_start_offset,
> > +		      0x8);
> >  	execbuf.cliprects_ptr = 0;
> >  	execbuf.num_cliprects = 0;
> >  	execbuf.DR1 = 0;
> > -- 
> > 1.9.1
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
  2014-11-03 19:18 bradley.d.volkin
@ 2014-11-06 22:39 ` Volkin, Bradley D
  2014-11-07  9:42   ` Daniel Vetter
  0 siblings, 1 reply; 7+ messages in thread
From: Volkin, Bradley D @ 2014-11-06 22:39 UTC (permalink / raw)
  To: intel-gfx

Ping on this series. They're related to the batch copy series, but
the changes are valid and tests should still pass even without the
kernel changes being merged.

On Mon, Nov 03, 2014 at 11:18:59AM -0800, Volkin, Bradley D wrote:
> From: Brad Volkin <bradley.d.volkin@intel.com>
> 
> The size of the batch buffer passed to the kernel is significantly
> larger than the size of the batch buffer passed to the function. A
> proposed optimization as part of the batch copy kernel series is to
> use batch_len for the copy and parse operations, which leads to a
> false "batch without MI_BATCH_BUFFER_END" failure for this test.
> 
> To fix this, modify the test to set batch_start_offset and batch_len
> such that they define the range of actual commands in the batch,
> including a few of the surrounding nops for alignment purposes.
> 
> v2: update batch_start_offset as well
> 
> Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> ---
>  tests/gem_exec_parse.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
> index 1dc9103..e48b83a 100644
> --- a/tests/gem_exec_parse.c
> +++ b/tests/gem_exec_parse.c
> @@ -144,16 +144,18 @@ static void exec_split_batch(int fd, uint32_t *cmds,
>  	struct drm_i915_gem_exec_object2 objs[1];
>  	uint32_t cmd_bo;
>  	uint32_t noop[1024] = { 0 };
> +	const int alloc_size = 4096 * 2;
> +	const int actual_start_offset = 4096-sizeof(uint32_t);
>  
>  	// Allocate and fill a 2-page batch with noops
> -	cmd_bo = gem_create(fd, 4096 * 2);
> +	cmd_bo = gem_create(fd, alloc_size);
>  	gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
>  	gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
>  
>  	// Write the provided commands such that the first dword
>  	// of the command buffer is the last dword of the first
>  	// page (i.e. the command is split across the two pages).
> -	gem_write(fd, cmd_bo, 4096-sizeof(uint32_t), cmds, size);
> +	gem_write(fd, cmd_bo, actual_start_offset, cmds, size);
>  
>  	objs[0].handle = cmd_bo;
>  	objs[0].relocation_count = 0;
> @@ -166,8 +168,14 @@ static void exec_split_batch(int fd, uint32_t *cmds,
>  
>  	execbuf.buffers_ptr = (uintptr_t)objs;
>  	execbuf.buffer_count = 1;
> -	execbuf.batch_start_offset = 0;
> -	execbuf.batch_len = size;
> +	// NB: We want batch_start_offset and batch_len to point to the block
> +	// of the actual commands (i.e. at the last dword of the first page),
> +	// but have to adjust both the start offset and length to meet the
> +	// kernel driver's requirements on the alignment of those fields.
> +	execbuf.batch_start_offset = actual_start_offset & ~0x7;
> +	execbuf.batch_len =
> +		ALIGN(size + actual_start_offset - execbuf.batch_start_offset,
> +		      0x8);
>  	execbuf.cliprects_ptr = 0;
>  	execbuf.num_cliprects = 0;
>  	execbuf.DR1 = 0;
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page
@ 2014-11-03 19:18 bradley.d.volkin
  2014-11-06 22:39 ` Volkin, Bradley D
  0 siblings, 1 reply; 7+ messages in thread
From: bradley.d.volkin @ 2014-11-03 19:18 UTC (permalink / raw)
  To: intel-gfx

From: Brad Volkin <bradley.d.volkin@intel.com>

The size of the batch buffer passed to the kernel is significantly
larger than the size of the batch buffer passed to the function. A
proposed optimization as part of the batch copy kernel series is to
use batch_len for the copy and parse operations, which leads to a
false "batch without MI_BATCH_BUFFER_END" failure for this test.

To fix this, modify the test to set batch_start_offset and batch_len
such that they define the range of actual commands in the batch,
including a few of the surrounding nops for alignment purposes.

v2: update batch_start_offset as well

Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
---
 tests/gem_exec_parse.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 1dc9103..e48b83a 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -144,16 +144,18 @@ static void exec_split_batch(int fd, uint32_t *cmds,
 	struct drm_i915_gem_exec_object2 objs[1];
 	uint32_t cmd_bo;
 	uint32_t noop[1024] = { 0 };
+	const int alloc_size = 4096 * 2;
+	const int actual_start_offset = 4096-sizeof(uint32_t);
 
 	// Allocate and fill a 2-page batch with noops
-	cmd_bo = gem_create(fd, 4096 * 2);
+	cmd_bo = gem_create(fd, alloc_size);
 	gem_write(fd, cmd_bo, 0, noop, sizeof(noop));
 	gem_write(fd, cmd_bo, 4096, noop, sizeof(noop));
 
 	// Write the provided commands such that the first dword
 	// of the command buffer is the last dword of the first
 	// page (i.e. the command is split across the two pages).
-	gem_write(fd, cmd_bo, 4096-sizeof(uint32_t), cmds, size);
+	gem_write(fd, cmd_bo, actual_start_offset, cmds, size);
 
 	objs[0].handle = cmd_bo;
 	objs[0].relocation_count = 0;
@@ -166,8 +168,14 @@ static void exec_split_batch(int fd, uint32_t *cmds,
 
 	execbuf.buffers_ptr = (uintptr_t)objs;
 	execbuf.buffer_count = 1;
-	execbuf.batch_start_offset = 0;
-	execbuf.batch_len = size;
+	// NB: We want batch_start_offset and batch_len to point to the block
+	// of the actual commands (i.e. at the last dword of the first page),
+	// but have to adjust both the start offset and length to meet the
+	// kernel driver's requirements on the alignment of those fields.
+	execbuf.batch_start_offset = actual_start_offset & ~0x7;
+	execbuf.batch_len =
+		ALIGN(size + actual_start_offset - execbuf.batch_start_offset,
+		      0x8);
 	execbuf.cliprects_ptr = 0;
 	execbuf.num_cliprects = 0;
 	execbuf.DR1 = 0;
-- 
1.9.1

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

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

end of thread, other threads:[~2014-11-07  9:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-15 21:52 [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page bradley.d.volkin
2014-10-15 21:52 ` [PATCH 2/2] tests/gem_exec_parse: test for chained batch buffers bradley.d.volkin
2014-10-21 15:26 ` [PATCH 1/2] tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page Daniel Vetter
2014-10-22 15:29   ` Volkin, Bradley D
2014-11-03 19:18 bradley.d.volkin
2014-11-06 22:39 ` Volkin, Bradley D
2014-11-07  9:42   ` Daniel Vetter

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.