i965/batch: don't ignore the 'brw_new_batch' call for a 'new batch'

Submitted by andrey simiklit on Sept. 10, 2018, 2:10 p.m.

Details

Message ID CAHfu=gKscOUFECUxCLqOH+vOi3BoRQ7NQKZDhHBnJJR1VKc8Kg@mail.gmail.com
State New
Headers show
Series "i965/batch: don't ignore the 'brw_new_batch' call for a 'new batch'" ( rev: 5 ) in Mesa

Not browsing as part of any series.

Commit Message

andrey simiklit Sept. 10, 2018, 2:10 p.m.
Hello,

Thanks for your reply.
Please find my comments below:

On Mon, Sep 10, 2018 at 2:45 PM Chris Wilson <chris@chris-wilson.co.uk>
wrote:

> Quoting andrey simiklit (2018-08-21 13:00:57)
> > Hi all,
> >
> > The bug for this issue was created:
> > https://bugs.freedesktop.org/show_bug.cgi?id=107626
>
> What about something like
>

Yes I think it is good idea.
One more point, there are multiple places where 'saving/restoring' api is
used and
I guess that each should be fixed.
That is why I added my solution into location where it could fix each of
them.
But your solution will be better for performance I think.
Is it acceptable for you to merge our solutions to:

---
 src/mesa/drivers/dri/i965/brw_compute.c       |  3 ++-
 src/mesa/drivers/dri/i965/brw_draw.c          |  3 ++-
 src/mesa/drivers/dri/i965/genX_blorp_exec.c   |  3 ++-
 src/mesa/drivers/dri/i965/intel_batchbuffer.c | 12 ++++++++++++
 src/mesa/drivers/dri/i965/intel_batchbuffer.h |  1 +
 5 files changed, 19 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_compute.c
b/src/mesa/drivers/dri/i965/brw_compute.c
index de08fc3..5c8e3a5 100644
--- a/src/mesa/drivers/dri/i965/brw_compute.c
+++ b/src/mesa/drivers/dri/i965/brw_compute.c
@@ -167,7 +167,7 @@  static void
 brw_dispatch_compute_common(struct gl_context *ctx)
 {
    struct brw_context *brw = brw_context(ctx);
-   bool fail_next = false;
+   bool fail_next;

    if (!_mesa_check_conditional_render(ctx))
       return;
@@ -185,6 +185,7 @@  brw_dispatch_compute_common(struct gl_context *ctx)
    intel_batchbuffer_require_space(brw, 600);
    brw_require_statebuffer_space(brw, 2500);
    intel_batchbuffer_save_state(brw);
+   fail_next = intel_batchbuffer_saved_state_is_empty(brw);

  retry:
    brw->batch.no_wrap = true;
diff --git a/src/mesa/drivers/dri/i965/brw_draw.c
b/src/mesa/drivers/dri/i965/brw_draw.c
index 8536c04..19ee396 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -885,7 +885,7 @@  brw_draw_single_prim(struct gl_context *ctx,
 {
    struct brw_context *brw = brw_context(ctx);
    const struct gen_device_info *devinfo = &brw->screen->devinfo;
-   bool fail_next = false;
+   bool fail_next;

    /* Flag BRW_NEW_DRAW_CALL on every draw.  This allows us to have
     * atoms that happen on every draw call.
@@ -898,6 +898,7 @@  brw_draw_single_prim(struct gl_context *ctx,
    intel_batchbuffer_require_space(brw, 1500);
    brw_require_statebuffer_space(brw, 2400);
    intel_batchbuffer_save_state(brw);
+   fail_next = intel_batchbuffer_saved_state_is_empty(brw);

    if (brw->num_instances != prim->num_instances ||
        brw->basevertex != prim->basevertex ||
diff --git a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
index 34bfcad..fd9ce93 100644
--- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
+++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
@@ -268,7 +268,7 @@  genX(blorp_exec)(struct blorp_batch *batch,
    assert(batch->blorp->driver_ctx == batch->driver_batch);
    struct brw_context *brw = batch->driver_batch;
    struct gl_context *ctx = &brw->ctx;
-   bool check_aperture_failed_once = false;
+   bool check_aperture_failed_once;

 #if GEN_GEN >= 11
    /* The PIPE_CONTROL command description says:
@@ -309,6 +309,7 @@  retry:
    intel_batchbuffer_require_space(brw, 1400);
    brw_require_statebuffer_space(brw, 600);
    intel_batchbuffer_save_state(brw);
+   check_aperture_failed_once =
intel_batchbuffer_saved_state_is_empty(brw);
    brw->batch.no_wrap = true;

 #if GEN_GEN == 6
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 4363b14..e0b7c94 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -57,6 +57,9 @@  static void
 intel_batchbuffer_reset(struct brw_context *brw);

 static void
+brw_new_batch(struct brw_context *brw);
+
+static void
 dump_validation_list(struct intel_batchbuffer *batch)
 {
    fprintf(stderr, "Validation list (length %d):\n", batch->exec_count);
@@ -299,6 +302,13 @@  intel_batchbuffer_save_state(struct brw_context *brw)
    brw->batch.saved.exec_count = brw->batch.exec_count;
 }

+bool
+intel_batchbuffer_saved_state_is_empty(struct brw_context *brw)
+{
+   struct intel_batchbuffer *batch = &brw->batch;
+   return (batch->saved.map_next == batch->batch.map);
+}
+
 void
 intel_batchbuffer_reset_to_saved(struct brw_context *brw)
 {
@@ -311,6 +321,8 @@  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;
+   if (USED_BATCH(brw->batch) == 0)
+        brw_new_batch(brw);
 }

 void
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
index 0632142..91720da 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
@@ -24,6 +24,7 @@  struct intel_batchbuffer;
 void intel_batchbuffer_init(struct brw_context *brw);
 void intel_batchbuffer_free(struct intel_batchbuffer *batch);
 void intel_batchbuffer_save_state(struct brw_context *brw);
+bool intel_batchbuffer_saved_state_is_empty(struct brw_context *brw);
 void intel_batchbuffer_reset_to_saved(struct brw_context *brw);
 void intel_batchbuffer_require_space(struct brw_context *brw, GLuint sz);
 int _intel_batchbuffer_flush_fence(struct brw_context *brw,

Comments

From: Andrii Simiklit <andrii.simiklit@globallogic.com>

If due to some reasons the last saved batch is empty and we 
restoring batch using this empty batch we have to call 'brw_new_batch' 
to properly reset all fields of it or we can just avoid this restoring 
to prevent incorrect state of the batch.

Andrii Simiklit (2):
  i965/batch: avoid reverting batch buffer if saved state is an empty
  i965/batch: don't ignore the 'brw_new_batch' call for a 'new batch'

 src/mesa/drivers/dri/i965/brw_compute.c       |  3 ++-
 src/mesa/drivers/dri/i965/brw_draw.c          |  3 ++-
 src/mesa/drivers/dri/i965/genX_blorp_exec.c   |  3 ++-
 src/mesa/drivers/dri/i965/intel_batchbuffer.c | 11 +++++++++++
 src/mesa/drivers/dri/i965/intel_batchbuffer.h |  1 +
 5 files changed, 18 insertions(+), 3 deletions(-)