[PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

basic-allocations was written to demonstrate a flaw in our continual
reallocation of cmdparser shadow bo, largely fixed by keeping a small
cache of bo of different lengths (to speed up the search for the correct
sized bo). We only care enough to exercise the slowdown by submitting
lots of execbufs, and can see the effect of bo caching on the rate, so
replace the fixed number of iterations with a timeout and count how many
batches we could submit instead.

Similarly, we now do not need to wait for all of our queue to complete
as we can tell the kernel to drop the queue instead.

References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/gem_exec_parse.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
index b653b1bdc..62e8d0a51 100644
--- a/tests/i915/gem_exec_parse.c
+++ b/tests/i915/gem_exec_parse.c
@@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
 
 static void test_allocations(int fd)
 {
-	uint32_t bbe = MI_BATCH_BUFFER_END;
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_exec_object2 obj[17];
-	int i, j;
+	unsigned long count;
 
 	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
 
 	memset(obj, 0, sizeof(obj));
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		uint64_t size = 1ull << (12 + i);
 
 		obj[i].handle = gem_create(fd, size);
@@ -322,17 +322,21 @@ static void test_allocations(int fd)
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffer_count = 1;
-	for (j = 0; j < 16384; j++) {
-		igt_progress("allocations ", j, 16384);
-		i = rand() % ARRAY_SIZE(obj);
+
+	count = 0;
+	igt_until_timeout(20) {
+		int i = rand() % ARRAY_SIZE(obj);
 		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
 		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
 		execbuf.batch_start_offset += 64 * (rand() % 64);
 		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
 		gem_execbuf(fd, &execbuf);
+		count++;
 	}
+	igt_info("Submitted %lu execbufs\n", count);
+	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
 
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		gem_sync(fd, obj[i].handle);
 		gem_close(fd, obj[i].handle);
 	}
-- 
2.20.1

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

[igt-dev] [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

basic-allocations was written to demonstrate a flaw in our continual
reallocation of cmdparser shadow bo, largely fixed by keeping a small
cache of bo of different lengths (to speed up the search for the correct
sized bo). We only care enough to exercise the slowdown by submitting
lots of execbufs, and can see the effect of bo caching on the rate, so
replace the fixed number of iterations with a timeout and count how many
batches we could submit instead.

Similarly, we now do not need to wait for all of our queue to complete
as we can tell the kernel to drop the queue instead.

References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 tests/i915/gem_exec_parse.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
index b653b1bdc..62e8d0a51 100644
--- a/tests/i915/gem_exec_parse.c
+++ b/tests/i915/gem_exec_parse.c
@@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
 
 static void test_allocations(int fd)
 {
-	uint32_t bbe = MI_BATCH_BUFFER_END;
+	const uint32_t bbe = MI_BATCH_BUFFER_END;
 	struct drm_i915_gem_execbuffer2 execbuf;
 	struct drm_i915_gem_exec_object2 obj[17];
-	int i, j;
+	unsigned long count;
 
 	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
 
 	memset(obj, 0, sizeof(obj));
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		uint64_t size = 1ull << (12 + i);
 
 		obj[i].handle = gem_create(fd, size);
@@ -322,17 +322,21 @@ static void test_allocations(int fd)
 
 	memset(&execbuf, 0, sizeof(execbuf));
 	execbuf.buffer_count = 1;
-	for (j = 0; j < 16384; j++) {
-		igt_progress("allocations ", j, 16384);
-		i = rand() % ARRAY_SIZE(obj);
+
+	count = 0;
+	igt_until_timeout(20) {
+		int i = rand() % ARRAY_SIZE(obj);
 		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
 		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
 		execbuf.batch_start_offset += 64 * (rand() % 64);
 		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
 		gem_execbuf(fd, &execbuf);
+		count++;
 	}
+	igt_info("Submitted %lu execbufs\n", count);
+	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
 
-	for (i = 0; i < ARRAY_SIZE(obj); i++) {
+	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
 		gem_sync(fd, obj[i].handle);
 		gem_close(fd, obj[i].handle);
 	}
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Patchwork]

== Series Details ==

Series: i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations
URL   : https://patchwork.freedesktop.org/series/56497/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5588 -> IGTPW_2373
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56497/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2373 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-kbl-7500u:       PASS -> DMESG-WARN [fdo#102505] / [fdo#103558] / [fdo#105602]

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#109485]

  * igt@pm_rpm@module-reload:
    - fi-skl-6770hq:      PASS -> FAIL [fdo#108511]

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u3}:        FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102505]: https://bugs.freedesktop.org/show_bug.cgi?id=102505
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109226]: https://bugs.freedesktop.org/show_bug.cgi?id=109226
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (48 -> 42)
------------------------------

  Additional (1): fi-glk-j4005 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4816 -> IGTPW_2373

  CI_DRM_5588: 133ed6b29f62713f2edd95eac6ccc2ae1d6e4d6e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2373: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2373/
  IGT_4816: f62577c85c9ef0539d468d6fad105b706a15139c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2373/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Tvrtko Ursulin]

On 11/02/2019 14:35, Chris Wilson wrote:
> basic-allocations was written to demonstrate a flaw in our continual
> reallocation of cmdparser shadow bo, largely fixed by keeping a small
> cache of bo of different lengths (to speed up the search for the correct
> sized bo). We only care enough to exercise the slowdown by submitting
> lots of execbufs, and can see the effect of bo caching on the rate, so
> replace the fixed number of iterations with a timeout and count how many
> batches we could submit instead.
> 
> Similarly, we now do not need to wait for all of our queue to complete
> as we can tell the kernel to drop the queue instead.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/i915/gem_exec_parse.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> index b653b1bdc..62e8d0a51 100644
> --- a/tests/i915/gem_exec_parse.c
> +++ b/tests/i915/gem_exec_parse.c
> @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
>   
>   static void test_allocations(int fd)
>   {
> -	uint32_t bbe = MI_BATCH_BUFFER_END;
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
>   	struct drm_i915_gem_execbuffer2 execbuf;
>   	struct drm_i915_gem_exec_object2 obj[17];
> -	int i, j;
> +	unsigned long count;
>   
>   	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
>   
>   	memset(obj, 0, sizeof(obj));
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		uint64_t size = 1ull << (12 + i);
>   
>   		obj[i].handle = gem_create(fd, size);
> @@ -322,17 +322,21 @@ static void test_allocations(int fd)
>   
>   	memset(&execbuf, 0, sizeof(execbuf));
>   	execbuf.buffer_count = 1;
> -	for (j = 0; j < 16384; j++) {
> -		igt_progress("allocations ", j, 16384);
> -		i = rand() % ARRAY_SIZE(obj);
> +
> +	count = 0;
> +	igt_until_timeout(20) {
> +		int i = rand() % ARRAY_SIZE(obj);
>   		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
>   		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
>   		execbuf.batch_start_offset += 64 * (rand() % 64);
>   		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
>   		gem_execbuf(fd, &execbuf);
> +		count++;
>   	}
> +	igt_info("Submitted %lu execbufs\n", count);
> +	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */

Downside here is that tests start to exercise a lot more driver paths. 
Or is that an upside? It's confusing these days.

I'd prefer if we just let it run and don't involve wedge/unwedge. Well 
actually... we could modify the submit loop to sync a bit rather than 
build a queue for 20 seconds? Would sync after each execbuf be 
detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth 
and then sync?

Regards,

Tvrtko

>   
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		gem_sync(fd, obj[i].handle);
>   		gem_close(fd, obj[i].handle);
>   	}
> 

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

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Tvrtko Ursulin]

On 11/02/2019 14:35, Chris Wilson wrote:
> basic-allocations was written to demonstrate a flaw in our continual
> reallocation of cmdparser shadow bo, largely fixed by keeping a small
> cache of bo of different lengths (to speed up the search for the correct
> sized bo). We only care enough to exercise the slowdown by submitting
> lots of execbufs, and can see the effect of bo caching on the rate, so
> replace the fixed number of iterations with a timeout and count how many
> batches we could submit instead.
> 
> Similarly, we now do not need to wait for all of our queue to complete
> as we can tell the kernel to drop the queue instead.
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/i915/gem_exec_parse.c | 18 +++++++++++-------
>   1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> index b653b1bdc..62e8d0a51 100644
> --- a/tests/i915/gem_exec_parse.c
> +++ b/tests/i915/gem_exec_parse.c
> @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
>   
>   static void test_allocations(int fd)
>   {
> -	uint32_t bbe = MI_BATCH_BUFFER_END;
> +	const uint32_t bbe = MI_BATCH_BUFFER_END;
>   	struct drm_i915_gem_execbuffer2 execbuf;
>   	struct drm_i915_gem_exec_object2 obj[17];
> -	int i, j;
> +	unsigned long count;
>   
>   	intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
>   
>   	memset(obj, 0, sizeof(obj));
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		uint64_t size = 1ull << (12 + i);
>   
>   		obj[i].handle = gem_create(fd, size);
> @@ -322,17 +322,21 @@ static void test_allocations(int fd)
>   
>   	memset(&execbuf, 0, sizeof(execbuf));
>   	execbuf.buffer_count = 1;
> -	for (j = 0; j < 16384; j++) {
> -		igt_progress("allocations ", j, 16384);
> -		i = rand() % ARRAY_SIZE(obj);
> +
> +	count = 0;
> +	igt_until_timeout(20) {
> +		int i = rand() % ARRAY_SIZE(obj);
>   		execbuf.buffers_ptr = to_user_pointer(&obj[i]);
>   		execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
>   		execbuf.batch_start_offset += 64 * (rand() % 64);
>   		execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
>   		gem_execbuf(fd, &execbuf);
> +		count++;
>   	}
> +	igt_info("Submitted %lu execbufs\n", count);
> +	igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */

Downside here is that tests start to exercise a lot more driver paths. 
Or is that an upside? It's confusing these days.

I'd prefer if we just let it run and don't involve wedge/unwedge. Well 
actually... we could modify the submit loop to sync a bit rather than 
build a queue for 20 seconds? Would sync after each execbuf be 
detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth 
and then sync?

Regards,

Tvrtko

>   
> -	for (i = 0; i < ARRAY_SIZE(obj); i++) {
> +	for (int i = 0; i < ARRAY_SIZE(obj); i++) {
>   		gem_sync(fd, obj[i].handle);
>   		gem_close(fd, obj[i].handle);
>   	}
> 

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-11 17:18:02)
> 
> On 11/02/2019 14:35, Chris Wilson wrote:
> > basic-allocations was written to demonstrate a flaw in our continual
> > reallocation of cmdparser shadow bo, largely fixed by keeping a small
> > cache of bo of different lengths (to speed up the search for the correct
> > sized bo). We only care enough to exercise the slowdown by submitting
> > lots of execbufs, and can see the effect of bo caching on the rate, so
> > replace the fixed number of iterations with a timeout and count how many
> > batches we could submit instead.
> > 
> > Similarly, we now do not need to wait for all of our queue to complete
> > as we can tell the kernel to drop the queue instead.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   tests/i915/gem_exec_parse.c | 18 +++++++++++-------
> >   1 file changed, 11 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> > index b653b1bdc..62e8d0a51 100644
> > --- a/tests/i915/gem_exec_parse.c
> > +++ b/tests/i915/gem_exec_parse.c
> > @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
> >   
> >   static void test_allocations(int fd)
> >   {
> > -     uint32_t bbe = MI_BATCH_BUFFER_END;
> > +     const uint32_t bbe = MI_BATCH_BUFFER_END;
> >       struct drm_i915_gem_execbuffer2 execbuf;
> >       struct drm_i915_gem_exec_object2 obj[17];
> > -     int i, j;
> > +     unsigned long count;
> >   
> >       intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
> >   
> >       memset(obj, 0, sizeof(obj));
> > -     for (i = 0; i < ARRAY_SIZE(obj); i++) {
> > +     for (int i = 0; i < ARRAY_SIZE(obj); i++) {
> >               uint64_t size = 1ull << (12 + i);
> >   
> >               obj[i].handle = gem_create(fd, size);
> > @@ -322,17 +322,21 @@ static void test_allocations(int fd)
> >   
> >       memset(&execbuf, 0, sizeof(execbuf));
> >       execbuf.buffer_count = 1;
> > -     for (j = 0; j < 16384; j++) {
> > -             igt_progress("allocations ", j, 16384);
> > -             i = rand() % ARRAY_SIZE(obj);
> > +
> > +     count = 0;
> > +     igt_until_timeout(20) {
> > +             int i = rand() % ARRAY_SIZE(obj);
> >               execbuf.buffers_ptr = to_user_pointer(&obj[i]);
> >               execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
> >               execbuf.batch_start_offset += 64 * (rand() % 64);
> >               execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
> >               gem_execbuf(fd, &execbuf);
> > +             count++;
> >       }
> > +     igt_info("Submitted %lu execbufs\n", count);
> > +     igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
> 
> Downside here is that tests start to exercise a lot more driver paths. 
> Or is that an upside? It's confusing these days.
> 
> I'd prefer if we just let it run and don't involve wedge/unwedge. Well 
> actually... we could modify the submit loop to sync a bit rather than 
> build a queue for 20 seconds? Would sync after each execbuf be 
> detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth 
> and then sync?

Yes, syncing affects i915_gem_batch_pool.c. The length of the cache
lists is largely determined by the number of batches in flight.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-02-11 17:18:02)
> 
> On 11/02/2019 14:35, Chris Wilson wrote:
> > basic-allocations was written to demonstrate a flaw in our continual
> > reallocation of cmdparser shadow bo, largely fixed by keeping a small
> > cache of bo of different lengths (to speed up the search for the correct
> > sized bo). We only care enough to exercise the slowdown by submitting
> > lots of execbufs, and can see the effect of bo caching on the rate, so
> > replace the fixed number of iterations with a timeout and count how many
> > batches we could submit instead.
> > 
> > Similarly, we now do not need to wait for all of our queue to complete
> > as we can tell the kernel to drop the queue instead.
> > 
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=107936
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > ---
> >   tests/i915/gem_exec_parse.c | 18 +++++++++++-------
> >   1 file changed, 11 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/i915/gem_exec_parse.c b/tests/i915/gem_exec_parse.c
> > index b653b1bdc..62e8d0a51 100644
> > --- a/tests/i915/gem_exec_parse.c
> > +++ b/tests/i915/gem_exec_parse.c
> > @@ -303,15 +303,15 @@ test_lri(int fd, uint32_t handle, struct test_lri *test)
> >   
> >   static void test_allocations(int fd)
> >   {
> > -     uint32_t bbe = MI_BATCH_BUFFER_END;
> > +     const uint32_t bbe = MI_BATCH_BUFFER_END;
> >       struct drm_i915_gem_execbuffer2 execbuf;
> >       struct drm_i915_gem_exec_object2 obj[17];
> > -     int i, j;
> > +     unsigned long count;
> >   
> >       intel_require_memory(2, 1ull<<(12 + ARRAY_SIZE(obj)), CHECK_RAM);
> >   
> >       memset(obj, 0, sizeof(obj));
> > -     for (i = 0; i < ARRAY_SIZE(obj); i++) {
> > +     for (int i = 0; i < ARRAY_SIZE(obj); i++) {
> >               uint64_t size = 1ull << (12 + i);
> >   
> >               obj[i].handle = gem_create(fd, size);
> > @@ -322,17 +322,21 @@ static void test_allocations(int fd)
> >   
> >       memset(&execbuf, 0, sizeof(execbuf));
> >       execbuf.buffer_count = 1;
> > -     for (j = 0; j < 16384; j++) {
> > -             igt_progress("allocations ", j, 16384);
> > -             i = rand() % ARRAY_SIZE(obj);
> > +
> > +     count = 0;
> > +     igt_until_timeout(20) {
> > +             int i = rand() % ARRAY_SIZE(obj);
> >               execbuf.buffers_ptr = to_user_pointer(&obj[i]);
> >               execbuf.batch_start_offset = (rand() % (1ull<<i)) << 12;
> >               execbuf.batch_start_offset += 64 * (rand() % 64);
> >               execbuf.batch_len = (1ull<<(12+i)) - execbuf.batch_start_offset;
> >               gem_execbuf(fd, &execbuf);
> > +             count++;
> >       }
> > +     igt_info("Submitted %lu execbufs\n", count);
> > +     igt_drop_caches_set(fd, DROP_RESET_ACTIVE); /* Cancel the queued work */
> 
> Downside here is that tests start to exercise a lot more driver paths. 
> Or is that an upside? It's confusing these days.
> 
> I'd prefer if we just let it run and don't involve wedge/unwedge. Well 
> actually... we could modify the submit loop to sync a bit rather than 
> build a queue for 20 seconds? Would sync after each execbuf be 
> detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth 
> and then sync?

Yes, syncing affects i915_gem_batch_pool.c. The length of the cache
lists is largely determined by the number of batches in flight.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Patchwork]

== Series Details ==

Series: i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations
URL   : https://patchwork.freedesktop.org/series/56497/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5588_full -> IGTPW_2373_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56497/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2373_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665] +1

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]
    - shard-kbl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-256x256-dpms:
    - shard-kbl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-kbl:          PASS -> FAIL [fdo#109350]
    - shard-apl:          PASS -> FAIL [fdo#109350]

  * igt@kms_flip@plain-flip-ts-check:
    - shard-glk:          PASS -> FAIL [fdo#100368]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-move:
    - shard-kbl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          PASS -> FAIL [fdo#103167] +5

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          PASS -> FAIL [fdo#103167] +4

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-kbl:          PASS -> FAIL [fdo#108145]
    - shard-apl:          PASS -> FAIL [fdo#108145]
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          PASS -> FAIL [fdo#103166]
    - shard-apl:          PASS -> FAIL [fdo#103166] +3
    - shard-kbl:          PASS -> FAIL [fdo#103166]

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  
#### Possible fixes ####

  * igt@i915_suspend@fence-restore-untiled:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          FAIL [fdo#108145] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-random:
    - shard-kbl:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +3

  * igt@kms_cursor_legacy@all-pipes-torture-move:
    - shard-hsw:          DMESG-WARN [fdo#107122] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-kbl:          FAIL [fdo#103166] -> PASS

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          {SKIP} [fdo#109271] -> PASS
    - shard-snb:          {SKIP} [fdo#109271] -> PASS

  
#### Warnings ####

  * igt@i915_suspend@shrink:
    - shard-glk:          INCOMPLETE [fdo#103359] / [fdo#106886] / [k.org#198133] -> DMESG-WARN [fdo#109244]

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-render:
    - shard-snb:          {SKIP} [fdo#109271] -> INCOMPLETE [fdo#105411]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106886]: https://bugs.freedesktop.org/show_bug.cgi?id=106886
  [fdo#107122]: https://bugs.freedesktop.org/show_bug.cgi?id=107122
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109244]: https://bugs.freedesktop.org/show_bug.cgi?id=109244
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4816 -> IGTPW_2373
    * Piglit: piglit_4509 -> None

  CI_DRM_5588: 133ed6b29f62713f2edd95eac6ccc2ae1d6e4d6e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2373: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2373/
  IGT_4816: f62577c85c9ef0539d468d6fad105b706a15139c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2373/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

Quoting Chris Wilson (2019-02-11 17:23:57)
> Quoting Tvrtko Ursulin (2019-02-11 17:18:02)
> > I'd prefer if we just let it run and don't involve wedge/unwedge. Well 
> > actually... we could modify the submit loop to sync a bit rather than 
> > build a queue for 20 seconds? Would sync after each execbuf be 
> > detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth 
> > and then sync?
> 
> Yes, syncing affects i915_gem_batch_pool.c. The length of the cache
> lists is largely determined by the number of batches in flight.

Along those lines, it's probably worthwhile to stress fixed object sizes
as well, and make those batches long last, yet still quick to parse.

It's worth bearing in mind that another user of i915_gem_batch_pool are
GPU relocs, so this issue isn't just limited to gen7-cmdparser.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/gem_exec_parse: Switch to a fixed timeout for basic-allocations

[Chris Wilson]

Quoting Chris Wilson (2019-02-11 17:23:57)
> Quoting Tvrtko Ursulin (2019-02-11 17:18:02)
> > I'd prefer if we just let it run and don't involve wedge/unwedge. Well 
> > actually... we could modify the submit loop to sync a bit rather than 
> > build a queue for 20 seconds? Would sync after each execbuf be 
> > detrimental to test goals? Alternatively submit maybe ARRAY_SIZE worth 
> > and then sync?
> 
> Yes, syncing affects i915_gem_batch_pool.c. The length of the cache
> lists is largely determined by the number of batches in flight.

Along those lines, it's probably worthwhile to stress fixed object sizes
as well, and make those batches long last, yet still quick to parse.

It's worth bearing in mind that another user of i915_gem_batch_pool are
GPU relocs, so this issue isn't just limited to gen7-cmdparser.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev