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

andrey simiklit asimiklit.work at gmail.com
Thu Nov 8 09:35:09 UTC 2018


Hello,

I don't know how we usually do in such cases.
Could I just restore back the 'review-by' tags here due to trivial changes
or this patch should pass review process once again?

Thanks,
Andrii.

On Mon, Nov 5, 2018 at 10:42 AM Jordan Justen <jordan.l.justen at intel.com>
wrote:

> 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
> > >
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181108/b6ed3c36/attachment.html>


More information about the mesa-dev mailing list