[RFC,i-g-t,2/3] igt/gem_wait: Use new igt_dummyload api
diff mbox

Message ID 20161012115954.16803-3-abdiel.janulgue@linux.intel.com
State New
Headers show

Commit Message

Abdiel Janulgue Oct. 12, 2016, 11:59 a.m. UTC
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 tests/gem_wait.c | 77 +++++++++-----------------------------------------------
 1 file changed, 12 insertions(+), 65 deletions(-)

Comments

Chris Wilson Oct. 12, 2016, 12:07 p.m. UTC | #1
On Wed, Oct 12, 2016 at 02:59:53PM +0300, Abdiel Janulgue wrote:
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> ---
>  tests/gem_wait.c | 77 +++++++++-----------------------------------------------
>  1 file changed, 12 insertions(+), 65 deletions(-)

We can do so much better than a dummy load here. We can precisely
control how long we want the object to be busy by using a recursive
batch buffer (and terminating that batch at the exact moment we require).
-Chris
Abdiel Janulgue Oct. 13, 2016, 9:31 a.m. UTC | #2
On 10/12/2016 03:07 PM, Chris Wilson wrote:
> On Wed, Oct 12, 2016 at 02:59:53PM +0300, Abdiel Janulgue wrote:
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>> ---
>>  tests/gem_wait.c | 77 +++++++++-----------------------------------------------
>>  1 file changed, 12 insertions(+), 65 deletions(-)
> 
> We can do so much better than a dummy load here. We can precisely
> control how long we want the object to be busy by using a recursive
> batch buffer (and terminating that batch at the exact moment we require).
> -Chris
> 
Hi Chris, I see you've posted a better solution to gem_wait. I could
drop this one and defer to yours instead. So for now, igt_dummyload has
dropped to only 1 customer at the moment: kms_flip.  Let me know whether
it's possible to upstream this dummyload api.

-Abdiel
Chris Wilson Oct. 13, 2016, 9:49 a.m. UTC | #3
On Thu, Oct 13, 2016 at 12:31:13PM +0300, Abdiel Janulgue wrote:
> 
> 
> On 10/12/2016 03:07 PM, Chris Wilson wrote:
> > On Wed, Oct 12, 2016 at 02:59:53PM +0300, Abdiel Janulgue wrote:
> >> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> >> ---
> >>  tests/gem_wait.c | 77 +++++++++-----------------------------------------------
> >>  1 file changed, 12 insertions(+), 65 deletions(-)
> > 
> > We can do so much better than a dummy load here. We can precisely
> > control how long we want the object to be busy by using a recursive
> > batch buffer (and terminating that batch at the exact moment we require).
> > -Chris
> > 
> Hi Chris, I see you've posted a better solution to gem_wait. I could
> drop this one and defer to yours instead. So for now, igt_dummyload has
> dropped to only 1 customer at the moment: kms_flip.  Let me know whether
> it's possible to upstream this dummyload api.

kms_flip would probably be better with a specific load rather than a
dummy as well. The challenge is whether the flip works given various
input states of the framebuffers, and the more control we have over
those inputs the better.
-Chris
Daniel Vetter Oct. 13, 2016, 3:17 p.m. UTC | #4
On Thu, Oct 13, 2016 at 10:49:39AM +0100, Chris Wilson wrote:
> On Thu, Oct 13, 2016 at 12:31:13PM +0300, Abdiel Janulgue wrote:
> > 
> > 
> > On 10/12/2016 03:07 PM, Chris Wilson wrote:
> > > On Wed, Oct 12, 2016 at 02:59:53PM +0300, Abdiel Janulgue wrote:
> > >> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > >> ---
> > >>  tests/gem_wait.c | 77 +++++++++-----------------------------------------------
> > >>  1 file changed, 12 insertions(+), 65 deletions(-)
> > > 
> > > We can do so much better than a dummy load here. We can precisely
> > > control how long we want the object to be busy by using a recursive
> > > batch buffer (and terminating that batch at the exact moment we require).
> > > -Chris
> > > 
> > Hi Chris, I see you've posted a better solution to gem_wait. I could
> > drop this one and defer to yours instead. So for now, igt_dummyload has
> > dropped to only 1 customer at the moment: kms_flip.  Let me know whether
> > it's possible to upstream this dummyload api.
> 
> kms_flip would probably be better with a specific load rather than a
> dummy as well. The challenge is whether the flip works given various
> input states of the framebuffers, and the more control we have over
> those inputs the better.

Oh yeah, this is a pretty sweet idea with a spinning batch that we
manually terminate. Assuming it works everywhere I think this is a much
better approach, and by submitting different workloads we can always put
the delay workload exactly where we want.

Abdiel, can you pls update the JIRA to instead extract Chris' trick into a
library (pretty sure Chris can help with bikeshedding the interface to
make it all pretty) and then roll that one out? Being able to control the
time used by delay tasks down to jiffies is real sweet.

Also there might be some space to reuse this with Mika's hangcheck stuff,
not sure.
-Daniel
Abdiel Janulgue Oct. 14, 2016, 10:17 a.m. UTC | #5
On 10/13/2016 06:17 PM, Daniel Vetter wrote:
> On Thu, Oct 13, 2016 at 10:49:39AM +0100, Chris Wilson wrote:
>> On Thu, Oct 13, 2016 at 12:31:13PM +0300, Abdiel Janulgue wrote:
>>>
>>>
>>> On 10/12/2016 03:07 PM, Chris Wilson wrote:
>>>> On Wed, Oct 12, 2016 at 02:59:53PM +0300, Abdiel Janulgue wrote:
>>>>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>>>>> ---
>>>>>  tests/gem_wait.c | 77 +++++++++-----------------------------------------------
>>>>>  1 file changed, 12 insertions(+), 65 deletions(-)
>>>>
>>>> We can do so much better than a dummy load here. We can precisely
>>>> control how long we want the object to be busy by using a recursive
>>>> batch buffer (and terminating that batch at the exact moment we require).
>>>> -Chris
>>>>
>>> Hi Chris, I see you've posted a better solution to gem_wait. I could
>>> drop this one and defer to yours instead. So for now, igt_dummyload has
>>> dropped to only 1 customer at the moment: kms_flip.  Let me know whether
>>> it's possible to upstream this dummyload api.
>>
>> kms_flip would probably be better with a specific load rather than a
>> dummy as well. The challenge is whether the flip works given various
>> input states of the framebuffers, and the more control we have over
>> those inputs the better.
> 
> Oh yeah, this is a pretty sweet idea with a spinning batch that we
> manually terminate. Assuming it works everywhere I think this is a much
> better approach, and by submitting different workloads we can always put
> the delay workload exactly where we want.
> 
> Abdiel, can you pls update the JIRA to instead extract Chris' trick into a
> library (pretty sure Chris can help with bikeshedding the interface to
> make it all pretty) and then roll that one out? Being able to control the
> time used by delay tasks down to jiffies is real sweet.

Ok I'll try that. Thanks!

-Abdiel

> 
> Also there might be some space to reuse this with Mika's hangcheck stuff,
> not sure.
> -Daniel
>

Patch
diff mbox

diff --git a/tests/gem_wait.c b/tests/gem_wait.c
index 461efdb..24a5f5e 100644
--- a/tests/gem_wait.c
+++ b/tests/gem_wait.c
@@ -54,36 +54,6 @@ 
 #define BUF_PAGES ((8<<20)>>12)
 drm_intel_bo *dst, *dst2;
 
-/* returns time diff in milliseconds */
-static int64_t
-do_time_diff(struct timespec *end, struct timespec *start)
-{
-	int64_t ret;
-	ret = (MSEC_PER_SEC * difftime(end->tv_sec, start->tv_sec)) +
-	      ((end->tv_nsec/NSEC_PER_MSEC) - (start->tv_nsec/NSEC_PER_MSEC));
-	return ret;
-}
-
-static void blt_color_fill(struct intel_batchbuffer *batch,
-			   drm_intel_bo *buf,
-			   const unsigned int pages)
-{
-	const unsigned short height = pages/4;
-	const unsigned short width =  4096;
-
-	COLOR_BLIT_COPY_BATCH_START(COLOR_BLT_WRITE_ALPHA |
-				    XY_COLOR_BLT_WRITE_RGB);
-	OUT_BATCH((3 << 24)	| /* 32 Bit Color */
-		  (0xF0 << 16)	| /* Raster OP copy background register */
-		  0);		  /* Dest pitch is 0 */
-	OUT_BATCH(0);
-	OUT_BATCH(width << 16	|
-		  height);
-	OUT_RELOC_FENCED(buf, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, 0);
-	OUT_BATCH(rand()); /* random pattern */
-	ADVANCE_BATCH();
-}
-
 static void render_timeout(int fd)
 {
 	drm_intel_bufmgr *bufmgr;
@@ -91,10 +61,11 @@  static void render_timeout(int fd)
 	int64_t timeout = ENOUGH_WORK_IN_SECONDS * NSEC_PER_SEC;
 	int64_t negative_timeout = -1;
 	int ret;
+	const unsigned short height = BUF_PAGES/4;
+	const unsigned short width =  4096;
 	const bool do_signals = true; /* signals will seem to make the operation
 				       * use less process CPU time */
-	bool done = false;
-	int i, iter = 1;
+	int iter = 1;
 
 	igt_skip_on_simulation();
 
@@ -112,30 +83,9 @@  static void render_timeout(int fd)
 	/* Figure out a rough number of fills required to consume 1 second of
 	 * GPU work.
 	 */
-	do {
-		struct timespec start, end;
-		long diff;
-
-#ifndef CLOCK_MONOTONIC_RAW
-#define CLOCK_MONOTONIC_RAW CLOCK_MONOTONIC
-#endif
-
-		igt_assert(clock_gettime(CLOCK_MONOTONIC_RAW, &start) == 0);
-		for (i = 0; i < iter; i++)
-			blt_color_fill(batch, dst, BUF_PAGES);
-		intel_batchbuffer_flush(batch);
-		drm_intel_bo_wait_rendering(dst);
-		igt_assert(clock_gettime(CLOCK_MONOTONIC_RAW, &end) == 0);
-
-		diff = do_time_diff(&end, &start);
-		igt_assert(diff >= 0);
-
-		if ((diff / MSEC_PER_SEC) > ENOUGH_WORK_IN_SECONDS)
-			done = true;
-		else
-			iter <<= 1;
-	} while (!done && iter < 1000000);
-
+	iter = igt_calibrate_dummy_load(bufmgr, batch, fd,
+					dst->handle, 0,
+					width, height, 1, IGT_DUMMY_BLIT_FILL);
 	igt_assert_lt(iter, 1000000);
 
 	igt_debug("%d iters is enough work\n", iter);
@@ -146,10 +96,9 @@  static void render_timeout(int fd)
 	/* We should be able to do half as much work in the same amount of time,
 	 * but because we might schedule almost twice as much as required, we
 	 * might accidentally time out. Hence add some fudge. */
-	for (i = 0; i < iter/3; i++)
-		blt_color_fill(batch, dst2, BUF_PAGES);
 
-	intel_batchbuffer_flush(batch);
+	igt_emit_dummy_load(bufmgr, batch, fd, dst2->handle, 0,
+			    width, height, iter/3, 0, IGT_DUMMY_BLIT_FILL);
 	igt_assert(gem_bo_busy(fd, dst2->handle) == true);
 
 	igt_assert_eq(gem_wait(fd, dst2->handle, &timeout), 0);
@@ -168,10 +117,9 @@  static void render_timeout(int fd)
 	/* Now check that we correctly time out, twice the auto-tune load should
 	 * be good enough. */
 	timeout = ENOUGH_WORK_IN_SECONDS * NSEC_PER_SEC;
-	for (i = 0; i < iter*2; i++)
-		blt_color_fill(batch, dst2, BUF_PAGES);
 
-	intel_batchbuffer_flush(batch);
+	igt_emit_dummy_load(bufmgr, batch, fd, dst2->handle, 0,
+			    width, height, iter*2, 0, IGT_DUMMY_BLIT_FILL);
 
 	ret = gem_wait(fd, dst2->handle, &timeout);
 	igt_assert_eq(ret, -ETIME);
@@ -186,10 +134,9 @@  static void render_timeout(int fd)
 
 	/* Now check that we can pass negative (infinite) timeouts. */
 	negative_timeout = -1;
-	for (i = 0; i < iter; i++)
-		blt_color_fill(batch, dst2, BUF_PAGES);
 
-	intel_batchbuffer_flush(batch);
+	igt_emit_dummy_load(bufmgr, batch, fd, dst2->handle, 0,
+			    width, height, iter, 0, IGT_DUMMY_BLIT_FILL);
 
 	igt_assert_eq(gem_wait(fd, dst2->handle, &negative_timeout), 0);
 	igt_assert_eq(negative_timeout, -1); /* infinity always remains */