[Mesa-dev] [PATCH v4 1/2] i965/batch: avoid reverting batch buffer if saved state is an empty
Mark Janes
mark.a.janes at intel.com
Mon Nov 5 16:31:16 UTC 2018
andrey simiklit <asimiklit.work at gmail.com> writes:
> 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
If you test the exact same revision twice in i965 CI, it will simply
publish the results from the previous run. Often, a single machine
fails in a run, and we send the build a second time to re-test only that
machine (all other results are re-used for faster results). We re-use
results based the commits of the source projects.
Instead of alternating two commits through the CI, you should trivially
amend the commit message of the patch you want to test, so it gets a
different sha. This will cause a complete re-test of the patch.
For this test, it may be that you are looking at an intermittent result.
However, it is never a problem to unexpectedly fix a test.
> 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
>
> Regards,
> Andrii.
>
> 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
>>
>>
More information about the mesa-dev
mailing list