batchbuffers failing to execute

batchbuffers failing to execute

[Eric Anholt]

I think I've got a concrete test here for something I've been trying
to track down for a while now: it appears that when the GPU is too
busy, batches get dropped and sometimes the GPU hangs.

This really became clear when I was testing a patch for citybench to
"improve" the swap throttling -- a broken version that I submitted
caused no throttling to occur at all.  The app, which renders a
predefined set of frames as fast as possible, would now frequently
stutter in the middle.  The results were insanely fast (40-60%
better!).  But it didn't make sense for there to be a stutter if the
set of frames rendered is predefined.

So, I wrote this little patch series to try to test my guess that my
batches were getting dropped.  The plan is: Emit a dword write every
(render) batch, and after a while go look and see if they all landed.
Corresponding to the stutters is:

Mesa: Initializing x86-64 optimizations
batch 42 didn't report: 0xd0d0d0d0 instead of 0x0000002a
batch 43 didn't report: 0xd0d0d0d0 instead of 0x0000002b
batch 44 didn't report: 0xd0d0d0d0 instead of 0x0000002c
batch 45 didn't report: 0xd0d0d0d0 instead of 0x0000002d
batch 46 didn't report: 0xd0d0d0d0 instead of 0x0000002e
batch 47 didn't report: 0xd0d0d0d0 instead of 0x0000002f
batch 48 didn't report: 0xd0d0d0d0 instead of 0x00000030
batch 49 didn't report: 0xd0d0d0d0 instead of 0x00000031
batch 50 didn't report: 0xd0d0d0d0 instead of 0x00000032
batch 51 didn't report: 0xd0d0d0d0 instead of 0x00000033
batch 52 didn't report: 0xd0d0d0d0 instead of 0x00000034
batch 53 didn't report: 0xd0d0d0d0 instead of 0x00000035
shutting up

The cool thing is apparently I don't need citybench, either.  This
paste came from vblank_mode=0 glxgears fullscreened on my 1600x900
panel (default size doesn't reproduce the failure).

This is gen7, danvet's kernel de67cba65944f26c0f147035bd62e30c5f456b96
rebased on top of cherry-picks of the 4 ivb workaround patches.  A
revert of a71d8d94525e8fd855c0466fb586ae1cb008f3a2 doesn't help.

I've also run this on a test libdrm that returns NULL pointers on map
failure and asserts that exec and pwrite don't return errors.

[PATCH 1/2] Try to ensure that batches emitted actually complete.

[Eric Anholt]

---
 src/mesa/drivers/dri/i965/brw_state_upload.c   |    2 +
 src/mesa/drivers/dri/intel/intel_batchbuffer.c |   73 ++++++++++++++++++++++++
 src/mesa/drivers/dri/intel/intel_batchbuffer.h |    2 +
 3 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_state_upload.c b/src/mesa/drivers/dri/i965/brw_state_upload.c
index f5e6fdc..4655e75 100644
--- a/src/mesa/drivers/dri/i965/brw_state_upload.c
+++ b/src/mesa/drivers/dri/i965/brw_state_upload.c
@@ -435,6 +435,8 @@ void brw_upload_state(struct brw_context *brw)
    int i;
    static int dirty_count = 0;
 
+   record_batch(intel);
+
    state->mesa |= brw->intel.NewGLState;
    brw->intel.NewGLState = 0;
 
diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.c b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
index d10e008..3ae2515 100644
--- a/src/mesa/drivers/dri/intel/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.c
@@ -51,6 +51,13 @@ static void clear_cache( struct intel_context *intel )
    intel->batch.cached_items = NULL;
 }
 
+static struct {
+   drm_intel_bo *bo;
+   int max_batches;
+   int next_batch;
+   bool needs_record;
+} batch_watch;
+
 void
 intel_batchbuffer_init(struct intel_context *intel)
 {
@@ -65,9 +72,73 @@ intel_batchbuffer_init(struct intel_context *intel)
 						      "pipe_control workaround",
 						      4096, 4096);
    }
+
+   batch_watch.max_batches = 1000;
+   batch_watch.bo = drm_intel_bo_alloc(intel->bufmgr,
+				       "watch",
+				       batch_watch.max_batches * 8,
+				       4096);
+   batch_watch.next_batch = 0;
+
+   drm_intel_bo_map(batch_watch.bo, true);
+   memset(batch_watch.bo->virtual, 0xd0, batch_watch.bo->size);
+   drm_intel_bo_unmap(batch_watch.bo);
 }
 
 void
+record_batch(struct intel_context *intel)
+{
+   if (!batch_watch.needs_record)
+      return;
+
+   batch_watch.needs_record = false;
+
+   assert(batch_watch.next_batch < batch_watch.max_batches);
+
+   BEGIN_BATCH(4);
+   OUT_BATCH(_3DSTATE_PIPE_CONTROL);
+   OUT_BATCH(PIPE_CONTROL_WRITE_IMMEDIATE);
+   OUT_RELOC(batch_watch.bo,
+	     I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
+	     batch_watch.next_batch * 8);
+   OUT_BATCH(batch_watch.next_batch); /* write data */
+   ADVANCE_BATCH();
+
+   batch_watch.next_batch++;
+}
+
+static void
+check_batches(struct intel_context *intel)
+{
+   batch_watch.needs_record = true;
+
+   if (batch_watch.next_batch == batch_watch.max_batches) {
+      int i;
+      int fails = 0;
+      uint32_t *map;
+
+      drm_intel_bo_map(batch_watch.bo, true);
+      map = batch_watch.bo->virtual;
+      for (i = 0; i < batch_watch.next_batch; i ++) {
+	 if (map[i * 2] != i) {
+	    printf("batch %d didn't report: 0x%08x instead of 0x%08x\n",
+		   i, map[i * 2], i);
+
+	    if (fails++ > 10) {
+	       printf("shutting up\n");
+	       break;
+	    }
+	 }
+      }
+
+      memset(batch_watch.bo->virtual, 0xd0, batch_watch.bo->size);
+      batch_watch.next_batch = 0;
+      drm_intel_bo_unmap(batch_watch.bo);
+   }
+}
+
+
+void
 intel_batchbuffer_reset(struct intel_context *intel)
 {
    if (intel->batch.last_bo != NULL) {
@@ -248,6 +319,8 @@ _intel_batchbuffer_flush(struct intel_context *intel,
     */
    intel_batchbuffer_reset(intel);
 
+   check_batches(intel);
+
    return ret;
 }
 
diff --git a/src/mesa/drivers/dri/intel/intel_batchbuffer.h b/src/mesa/drivers/dri/intel/intel_batchbuffer.h
index 751ec99..c36fd0b 100644
--- a/src/mesa/drivers/dri/intel/intel_batchbuffer.h
+++ b/src/mesa/drivers/dri/intel/intel_batchbuffer.h
@@ -132,6 +132,8 @@ intel_batchbuffer_advance(struct intel_context *intel)
 
 void intel_batchbuffer_cached_advance(struct intel_context *intel);
 
+void record_batch(struct intel_context *intel);
+
 /* Here are the crusty old macros, to be removed:
  */
 #define BATCH_LOCALS
-- 
1.7.9

[PATCH 2/2] intel: Disable swap throttling entirely.

[Eric Anholt]

This reveals some major breakage in the kernel.
---
 src/mesa/drivers/dri/intel/intel_context.c |   22 ----------------------
 1 files changed, 0 insertions(+), 22 deletions(-)

diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c
index 377bcbc..573346da23 100644
--- a/src/mesa/drivers/dri/intel/intel_context.c
+++ b/src/mesa/drivers/dri/intel/intel_context.c
@@ -411,28 +411,6 @@ intel_prepare_render(struct intel_context *intel)
     */
    if (intel->is_front_buffer_rendering)
       intel->front_buffer_dirty = true;
-
-   /* Wait for the swapbuffers before the one we just emitted, so we
-    * don't get too many swaps outstanding for apps that are GPU-heavy
-    * but not CPU-heavy.
-    *
-    * We're using intelDRI2Flush (called from the loader before
-    * swapbuffer) and glFlush (for front buffer rendering) as the
-    * indicator that a frame is done and then throttle when we get
-    * here as we prepare to render the next frame.  At this point for
-    * round trips for swap/copy and getting new buffers are done and
-    * we'll spend less time waiting on the GPU.
-    *
-    * Unfortunately, we don't have a handle to the batch containing
-    * the swap, and getting our hands on that doesn't seem worth it,
-    * so we just us the first batch we emitted after the last swap.
-    */
-   if (intel->need_throttle && intel->first_post_swapbuffers_batch) {
-      drm_intel_bo_wait_rendering(intel->first_post_swapbuffers_batch);
-      drm_intel_bo_unreference(intel->first_post_swapbuffers_batch);
-      intel->first_post_swapbuffers_batch = NULL;
-      intel->need_throttle = false;
-   }
 }
 
 static void
-- 
1.7.9

Re: batchbuffers failing to execute

[Chris Wilson]

On Mon, 20 Feb 2012 12:10:49 -0800, Eric Anholt <eric@anholt.net> wrote:
> This is gen7, danvet's kernel de67cba65944f26c0f147035bd62e30c5f456b96
> rebased on top of cherry-picks of the 4 ivb workaround patches.  A
> revert of a71d8d94525e8fd855c0466fb586ae1cb008f3a2 doesn't help.

You need the bug fix that was meant to be applied before that patch
id:1328708054-26350-1-git-send-email-chris@chris-wilson.co.uk.

See also https://bugs.freedesktop.org/show_bug.cgi?id=45492
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: batchbuffers failing to execute

[Daniel Vetter]

On Mon, Feb 20, 2012 at 08:30:39PM +0000, Chris Wilson wrote:
> On Mon, 20 Feb 2012 12:10:49 -0800, Eric Anholt <eric@anholt.net> wrote:
> > This is gen7, danvet's kernel de67cba65944f26c0f147035bd62e30c5f456b96
> > rebased on top of cherry-picks of the 4 ivb workaround patches.  A
> > revert of a71d8d94525e8fd855c0466fb586ae1cb008f3a2 doesn't help.
> 
> You need the bug fix that was meant to be applied before that patch
> id:1328708054-26350-1-git-send-email-chris@chris-wilson.co.uk.
> 
> See also https://bugs.freedesktop.org/show_bug.cgi?id=45492

Meh, stupid me somehow got the idea that we'd check the new request->tail
before the b0rked autoreport_head and hence merging just the new code for
-next would fix this desaster without merging in -fixes. Otherwise I'd
been slightly more clear in my -next announcement ...

/me hides in shame
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48