[Mesa-dev] [PATCH v4 1/2] i965/batch: avoid reverting batch buffer if saved state is an empty

Mon Nov 5 08:42:16 UTC 2018

On 2018-11-05 00:11:52, andrey simiklit wrote:
> Hello,
> 
> I tested this patch few times and have the following results:
> https://mesa-ci.01.org/global_logic/builds/38/group/63a9f0ea7bb98050796b649e85481845
> https://mesa-ci.01.org/global_logic/builds/41/group/63a9f0ea7bb98050796b649e85481845
> https://mesa-ci.01.org/global_logic/builds/43/group/63a9f0ea7bb98050796b649e85481845
> 
> All test runs are finished with the following issue on G965:
> 
> https://mesa-ci.01.org/global_logic/builds/38/results/46496330
> ext_framebuffer_multisample-accuracy all_samples srgb small depthstencil
> linear -auto -fbo
> ....
> ERROR: This test passed when it expected failure
> 
> Note: Also I checked latest mesa master few times and there was no
> regression:
> 
> https://mesa-ci.01.org/global_logic/builds/42/group/63a9f0ea7bb98050796b649e85481845
> 

It also looks like the tex3d-maxsize test is back to failing after a
few seconds on g965, rather than running forever. (So, #108630 is
fixed by this version.)

https://mesa-ci.01.org/global_logic/builds/38/results/46492597

-Jordan

> 
> On Mon, Nov 5, 2018 at 9:48 AM <asimiklit.work at gmail.com> wrote:
> 
> > From: Andrii Simiklit <andrii.simiklit at globallogic.com>
> >
> > There's no point reverting to the last saved point if that save point is
> > the empty batch, we will just repeat ourselves.
> >
> > v2: Merge with new commits, changes was minimized, added the 'fixes' tag
> > v3: Added in to patch series
> > v4: Fixed the regression which was introduced by this patch
> >     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108630
> >     Reported-by:  Mark Janes <mark.a.janes at intel.com>
> >     The solution provided by: Jordan Justen <jordan.l.justen at intel.com>
> >
> > CC: Chris Wilson <chris at chris-wilson.co.uk>
> > Fixes: 3faf56ffbdeb "intel: Add an interface for saving/restoring
> >                      the batchbuffer state."
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107626
> > Signed-off-by: Andrii Simiklit <andrii.simiklit at globallogic.com>
> > ---
> >  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   | 1 +
> >  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 7 +++++++
> >  src/mesa/drivers/dri/i965/intel_batchbuffer.h | 1 +
> >  5 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_compute.c
> > b/src/mesa/drivers/dri/i965/brw_compute.c
> > index de08fc3ac1..5c8e3a5d4d 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 8536c04010..19ee3962d7 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 34bfcad03e..a62b88e166 100644
> > --- a/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> > +++ b/src/mesa/drivers/dri/i965/genX_blorp_exec.c
> > @@ -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 8b769eaf53..6207de5a06 100644
> > --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> > @@ -301,6 +301,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)
> >  {
> > diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> > b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
> > index 0632142cd3..91720dad5b 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,
> > --
> > 2.17.1
> >
> >