Hi Andrii,

Although I think what these patches do makes sense, I think it's missing the bigger picture. There is a lot more state that gets lost if we have to revert all of the emitted commands. A quick look at brw_upload_pipeline_state() shows all of the programs could be invalid as well, or the number of samples, etc...

To me it seems like we need a much larger state save/restore.

Ken: what do you think?

Cheers,

-
Lionel

On 19/11/2018 21:19, asimiklit.w...@gmail.com wrote:
From: Andrii Simiklit <andrii.simik...@globallogic.com>

This patch is needed to avoid missing 3DSTATE_URB_* commands in a batch after
rollback operation.

To be able to test easily this issue
the following workaround is necessary:
-if (!brw_batch_has_aperture_space(brw, 0)) {
by:
+if (true) {
in brw_draw_single_prim function.

This workaround forces a rollback for each batch.
The "gl-3.2-adj-prims pv-first -auto -fbo" piglit test with this workaround
led to a hung. The hung investigation helped to find that some batches
didn't have the 3DSTATE_URB_* commands in compare to batches which were
generated without this workaround.

Reported-by: Kenneth Graunke <kenn...@whitecape.org>
Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com>
---
  src/mesa/drivers/dri/i965/brw_context.h       |  8 ++++++++
  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 12 ++++++++++++
  2 files changed, 20 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 7fd15669eb..4e1682ba5c 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -521,6 +521,14 @@ struct intel_batchbuffer {
        int batch_reloc_count;
        int state_reloc_count;
        int exec_count;
+      struct {
+         GLuint vsize;
+         GLuint gsize;
+         GLuint hsize;
+         GLuint dsize;
+         bool gs_present;
+         bool tess_present;
+      } urb;
     } saved;
/** Map from batch offset to brw_state_batch data (with DEBUG_BATCH) */
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 353fcba18f..bae9f2ffed 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -299,6 +299,12 @@ intel_batchbuffer_save_state(struct brw_context *brw)
     brw->batch.saved.batch_reloc_count = brw->batch.batch_relocs.reloc_count;
     brw->batch.saved.state_reloc_count = brw->batch.state_relocs.reloc_count;
     brw->batch.saved.exec_count = brw->batch.exec_count;
+   brw->batch.saved.urb.vsize = brw->urb.vsize;
+   brw->batch.saved.urb.gs_present = brw->urb.gs_present;
+   brw->batch.saved.urb.gsize = brw->urb.gsize;
+   brw->batch.saved.urb.tess_present = brw->urb.tess_present;
+   brw->batch.saved.urb.hsize = brw->urb.hsize;
+   brw->batch.saved.urb.dsize = brw->urb.dsize;
  }
void
@@ -313,6 +319,12 @@ intel_batchbuffer_reset_to_saved(struct brw_context *brw)
     brw->batch.exec_count = brw->batch.saved.exec_count;
brw->batch.map_next = brw->batch.saved.map_next;
+   brw->urb.vsize = brw->batch.saved.urb.vsize;
+   brw->urb.gs_present = brw->batch.saved.urb.gs_present;
+   brw->urb.gsize = brw->batch.saved.urb.gsize;
+   brw->urb.tess_present = brw->batch.saved.urb.tess_present;
+   brw->urb.hsize = brw->batch.saved.urb.hsize;
+   brw->urb.dsize = brw->batch.saved.urb.dsize;
     if (USED_BATCH(brw->batch) == 0)
        brw_new_batch(brw);
  }


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

Reply via email to