<div dir="ltr"><div dir="ltr"><div>Hello,</div><div> </div>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? <div><div><div> </div><div>Thanks,</div><div>Andrii. </div> <div class="gmail_quote"><div dir="ltr">On Mon, Nov 5, 2018 at 10:42 AM Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2018-11-05 00:11:52, andrey simiklit wrote: > Hello, > > I tested this patch few times and have the following results: > <a href="https://mesa-ci.01.org/global_logic/builds/38/group/63a9f0ea7bb98050796b649e85481845" rel="noreferrer" target="_blank">https://mesa-ci.01.org/global_logic/builds/38/group/63a9f0ea7bb98050796b649e85481845</a> > <a href="https://mesa-ci.01.org/global_logic/builds/41/group/63a9f0ea7bb98050796b649e85481845" rel="noreferrer" target="_blank">https://mesa-ci.01.org/global_logic/builds/41/group/63a9f0ea7bb98050796b649e85481845</a> > <a href="https://mesa-ci.01.org/global_logic/builds/43/group/63a9f0ea7bb98050796b649e85481845" rel="noreferrer" target="_blank">https://mesa-ci.01.org/global_logic/builds/43/group/63a9f0ea7bb98050796b649e85481845</a> > > All test runs are finished with the following issue on G965: > > <a href="https://mesa-ci.01.org/global_logic/builds/38/results/46496330" rel="noreferrer" target="_blank">https://mesa-ci.01.org/global_logic/builds/38/results/46496330</a> > 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: > > <a href="https://mesa-ci.01.org/global_logic/builds/42/group/63a9f0ea7bb98050796b649e85481845" rel="noreferrer" target="_blank">https://mesa-ci.01.org/global_logic/builds/42/group/63a9f0ea7bb98050796b649e85481845</a> > 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.) <a href="https://mesa-ci.01.org/global_logic/builds/38/results/46492597" rel="noreferrer" target="_blank">https://mesa-ci.01.org/global_logic/builds/38/results/46492597</a> -Jordan > > On Mon, Nov 5, 2018 at 9:48 AM <<a href="mailto:asimiklit.work@gmail.com" target="_blank">asimiklit.work@gmail.com</a>> wrote: > > > From: Andrii Simiklit <<a href="mailto:andrii.simiklit@globallogic.com" target="_blank">andrii.simiklit@globallogic.com</a>> > > > > 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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=108630" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=108630</a> > > Reported-by: Mark Janes <<a href="mailto:mark.a.janes@intel.com" target="_blank">mark.a.janes@intel.com</a>> > > The solution provided by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>> > > > > CC: Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk" target="_blank">chris@chris-wilson.co.uk</a>> > > Fixes: 3faf56ffbdeb "intel: Add an interface for saving/restoring > > the batchbuffer state." > > Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=107626" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=107626</a> > > Signed-off-by: Andrii Simiklit <<a href="mailto:andrii.simiklit@globallogic.com" target="_blank">andrii.simiklit@globallogic.com</a>> > > --- > > 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 > > > > </blockquote></div></div></div></div></div>