Hi all,
The bug for this issue was created:
https://bugs.freedesktop.org/show_bug.cgi?id=107626
Regards,
Andrii.
On Mon, Aug 20, 2018 at 5:43 PM, <asimiklit.w...@gmail.com> wrote:
> From: Andrii Simiklit <andrii.simik...@globallogic.com>
>
> If we restore the 'new batch' using 'intel_batchbuffer_reset_to_saved'
> function we must restore the default state of the batch using
> 'brw_new_batch' function because the 'intel_batchbuffer_flush'
> function will not do it for the 'new batch' again.
> At least the following fields of the batch
> 'state_base_address_emitted','aperture_space', 'state_used'
> should be restored to default values to avoid:
> 1. the aperture_space overflow
> 2. the missed STATE_BASE_ADDRESS commad in the batch
> 3. the memory overconsumption of the 'statebuffer'
> due to uncleared 'state_used' field.
> etc.
>
> Signed-off-by: Andrii Simiklit <andrii.simik...@globallogic.com>
> ---
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 104
> +++++++++++++++-----------
> 1 file changed, 59 insertions(+), 45 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 65d2c64..b8c5fb4 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -219,7 +219,7 @@ add_exec_bo(struct intel_batchbuffer *batch, struct
> brw_bo *bo)
> bo->index = batch->exec_count;
> batch->exec_bos[batch->exec_count] = bo;
> batch->aperture_space += bo->size;
> -
> + assert((batch->aperture_space >= 0) && "error 'batch->aperture_space'
> field is overflown!");
> return batch->exec_count++;
> }
>
> @@ -290,6 +290,51 @@ intel_batchbuffer_reset_and_clear_render_cache(struct
> brw_context *brw)
> brw_cache_sets_clear(brw);
> }
>
> +/**
> + * Called when starting a new batch buffer.
> + */
> +static void
> +brw_new_batch(struct brw_context *brw)
> +{
> + /* Unreference any BOs held by the previous batch, and reset counts. */
> + for (int i = 0; i < brw->batch.exec_count; i++) {
> + brw_bo_unreference(brw->batch.exec_bos[i]);
> + brw->batch.exec_bos[i] = NULL;
> + }
> + brw->batch.batch_relocs.reloc_count = 0;
> + brw->batch.state_relocs.reloc_count = 0;
> + brw->batch.exec_count = 0;
> + brw->batch.aperture_space = 0;
> +
> + brw_bo_unreference(brw->batch.state.bo);
> +
> + /* Create a new batchbuffer and reset the associated state: */
> + intel_batchbuffer_reset_and_clear_render_cache(brw);
> +
> + /* If the kernel supports hardware contexts, then most hardware state
> is
> + * preserved between batches; we only need to re-emit state that is
> required
> + * to be in every batch. Otherwise we need to re-emit all the state
> that
> + * would otherwise be stored in the context (which for all intents and
> + * purposes means everything).
> + */
> + if (brw->hw_ctx == 0) {
> + brw->ctx.NewDriverState |= BRW_NEW_CONTEXT;
> + brw_upload_invariant_state(brw);
> + }
> +
> + brw->ctx.NewDriverState |= BRW_NEW_BATCH;
> +
> + brw->ib.index_size = -1;
> +
> + /* We need to periodically reap the shader time results, because
> rollover
> + * happens every few seconds. We also want to see results every once
> in a
> + * while, because many programs won't cleanly destroy our context, so
> the
> + * end-of-run printout may not happen.
> + */
> + if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> + brw_collect_and_report_shader_time(brw);
> +}
> +
> void
> intel_batchbuffer_save_state(struct brw_context *brw)
> {
> @@ -311,6 +356,19 @@ 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)
> + {
> + /**
> + * The 'intel_batchbuffer_flush' function will not call
> + * the 'brw_new_batch' function when the USED_BATCH returns 0.
> + * It may leads to the few following issue:
> + * The 'aperture_space' field can be overflown
> + * The 'statebuffer' buffer contains the big unused space
> + * The STATE_BASE_ADDRESS message is missed
> + * etc
> + **/
> + brw_new_batch(brw);
> + }
> }
>
> void
> @@ -529,50 +587,6 @@ intel_batchbuffer_require_space(struct brw_context
> *brw, GLuint sz)
> }
> }
>
> -/**
> - * Called when starting a new batch buffer.
> - */
> -static void
> -brw_new_batch(struct brw_context *brw)
> -{
> - /* Unreference any BOs held by the previous batch, and reset counts. */
> - for (int i = 0; i < brw->batch.exec_count; i++) {
> - brw_bo_unreference(brw->batch.exec_bos[i]);
> - brw->batch.exec_bos[i] = NULL;
> - }
> - brw->batch.batch_relocs.reloc_count = 0;
> - brw->batch.state_relocs.reloc_count = 0;
> - brw->batch.exec_count = 0;
> - brw->batch.aperture_space = 0;
> -
> - brw_bo_unreference(brw->batch.state.bo);
> -
> - /* Create a new batchbuffer and reset the associated state: */
> - intel_batchbuffer_reset_and_clear_render_cache(brw);
> -
> - /* If the kernel supports hardware contexts, then most hardware state
> is
> - * preserved between batches; we only need to re-emit state that is
> required
> - * to be in every batch. Otherwise we need to re-emit all the state
> that
> - * would otherwise be stored in the context (which for all intents and
> - * purposes means everything).
> - */
> - if (brw->hw_ctx == 0) {
> - brw->ctx.NewDriverState |= BRW_NEW_CONTEXT;
> - brw_upload_invariant_state(brw);
> - }
> -
> - brw->ctx.NewDriverState |= BRW_NEW_BATCH;
> -
> - brw->ib.index_size = -1;
> -
> - /* We need to periodically reap the shader time results, because
> rollover
> - * happens every few seconds. We also want to see results every once
> in a
> - * while, because many programs won't cleanly destroy our context, so
> the
> - * end-of-run printout may not happen.
> - */
> - if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> - brw_collect_and_report_shader_time(brw);
> -}
>
> /**
> * Called from intel_batchbuffer_flush before emitting MI_BATCHBUFFER_END
> and
> --
> 2.7.4
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
