[2/2] i965: add missing rollback of a xfb zero_offsets flag

Submitted by andrey simiklit on Nov. 19, 2018, 9:19 p.m.

Details

Message ID 20181119211938.31394-2-asimiklit.work@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

andrey simiklit Nov. 19, 2018, 9:19 p.m.
From: Andrii Simiklit <andrii.simiklit@globallogic.com>

This patch is needed to avoid incorrect value of StreamOffset flag
in 3DSTATE_SO_BUFFER command after rollback operation.

To be able to test easily this issue
the following workaround is necessary:
-if (!brw_batch_has_aperture_space(brw, 0)) {
by:
+if (true) {
in brw_draw_single_prim function.

This workaround forces a rollback for each batch.
The "gl-3.2-adj-prims pv-first -fbo" piglit test with this workaround failed
(when URB requirements was fixed this test started to fail instead of a hang).
Investigation of this failure helped to find that some batches have a different
Stream Offset in the 3D STATE_SO_BUFFER command in compare to batches which were
generated without this workaround.

Reported-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
---
 src/mesa/drivers/dri/i965/brw_context.h       |  3 +++
 src/mesa/drivers/dri/i965/intel_batchbuffer.c | 10 ++++++++++
 2 files changed, 13 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index 4e1682ba5c..86c9bf0a5c 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -521,6 +521,9 @@  struct intel_batchbuffer {
       int batch_reloc_count;
       int state_reloc_count;
       int exec_count;
+      struct {
+         bool zero_offsets;
+      } xfb;
       struct {
          GLuint vsize;
          GLuint gsize;
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index bae9f2ffed..85e5f64123 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -295,10 +295,15 @@  intel_batchbuffer_reset_and_clear_render_cache(struct brw_context *brw)
 void
 intel_batchbuffer_save_state(struct brw_context *brw)
 {
+   struct brw_transform_feedback_object *brw_xfb_obj =
+                   (struct brw_transform_feedback_object *)
+                        brw->ctx.TransformFeedback.CurrentObject;
+
    brw->batch.saved.map_next = brw->batch.map_next;
    brw->batch.saved.batch_reloc_count = brw->batch.batch_relocs.reloc_count;
    brw->batch.saved.state_reloc_count = brw->batch.state_relocs.reloc_count;
    brw->batch.saved.exec_count = brw->batch.exec_count;
+   brw->batch.saved.xfb.zero_offsets = brw_xfb_obj->zero_offsets;
    brw->batch.saved.urb.vsize = brw->urb.vsize;
    brw->batch.saved.urb.gs_present = brw->urb.gs_present;
    brw->batch.saved.urb.gsize = brw->urb.gsize;
@@ -310,6 +315,10 @@  intel_batchbuffer_save_state(struct brw_context *brw)
 void
 intel_batchbuffer_reset_to_saved(struct brw_context *brw)
 {
+   struct brw_transform_feedback_object *brw_xfb_obj =
+                   (struct brw_transform_feedback_object *)
+                        brw->ctx.TransformFeedback.CurrentObject;
+
    for (int i = brw->batch.saved.exec_count;
         i < brw->batch.exec_count; i++) {
       brw_bo_unreference(brw->batch.exec_bos[i]);
@@ -319,6 +328,7 @@  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;
+   brw_xfb_obj->zero_offsets = brw->batch.saved.xfb.zero_offsets;
    brw->urb.vsize = brw->batch.saved.urb.vsize;
    brw->urb.gs_present = brw->batch.saved.urb.gs_present;
    brw->urb.gsize = brw->batch.saved.urb.gsize;

Comments

Hi all,

Could somebody take a look at this patch?

Regards,
Andrii.

On Mon, Nov 19, 2018 at 11:19 PM <asimiklit.work@gmail.com> wrote:

> From: Andrii Simiklit <andrii.simiklit@globallogic.com>
>
> This patch is needed to avoid incorrect value of StreamOffset flag
> in 3DSTATE_SO_BUFFER command after rollback operation.
>
> To be able to test easily this issue
> the following workaround is necessary:
> -if (!brw_batch_has_aperture_space(brw, 0)) {
> by:
> +if (true) {
> in brw_draw_single_prim function.
>
> This workaround forces a rollback for each batch.
> The "gl-3.2-adj-prims pv-first -fbo" piglit test with this workaround
> failed
> (when URB requirements was fixed this test started to fail instead of a
> hang).
> Investigation of this failure helped to find that some batches have a
> different
> Stream Offset in the 3D STATE_SO_BUFFER command in compare to batches
> which were
> generated without this workaround.
>
> Reported-by: Kenneth Graunke <kenneth@whitecape.org>
> Signed-off-by: Andrii Simiklit <andrii.simiklit@globallogic.com>
> ---
>  src/mesa/drivers/dri/i965/brw_context.h       |  3 +++
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 10 ++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> b/src/mesa/drivers/dri/i965/brw_context.h
> index 4e1682ba5c..86c9bf0a5c 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -521,6 +521,9 @@ struct intel_batchbuffer {
>        int batch_reloc_count;
>        int state_reloc_count;
>        int exec_count;
> +      struct {
> +         bool zero_offsets;
> +      } xfb;
>        struct {
>           GLuint vsize;
>           GLuint gsize;
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index bae9f2ffed..85e5f64123 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -295,10 +295,15 @@
> intel_batchbuffer_reset_and_clear_render_cache(struct brw_context *brw)
>  void
>  intel_batchbuffer_save_state(struct brw_context *brw)
>  {
> +   struct brw_transform_feedback_object *brw_xfb_obj =
> +                   (struct brw_transform_feedback_object *)
> +                        brw->ctx.TransformFeedback.CurrentObject;
> +
>     brw->batch.saved.map_next = brw->batch.map_next;
>     brw->batch.saved.batch_reloc_count =
> brw->batch.batch_relocs.reloc_count;
>     brw->batch.saved.state_reloc_count =
> brw->batch.state_relocs.reloc_count;
>     brw->batch.saved.exec_count = brw->batch.exec_count;
> +   brw->batch.saved.xfb.zero_offsets = brw_xfb_obj->zero_offsets;
>     brw->batch.saved.urb.vsize = brw->urb.vsize;
>     brw->batch.saved.urb.gs_present = brw->urb.gs_present;
>     brw->batch.saved.urb.gsize = brw->urb.gsize;
> @@ -310,6 +315,10 @@ intel_batchbuffer_save_state(struct brw_context *brw)
>  void
>  intel_batchbuffer_reset_to_saved(struct brw_context *brw)
>  {
> +   struct brw_transform_feedback_object *brw_xfb_obj =
> +                   (struct brw_transform_feedback_object *)
> +                        brw->ctx.TransformFeedback.CurrentObject;
> +
>     for (int i = brw->batch.saved.exec_count;
>          i < brw->batch.exec_count; i++) {
>        brw_bo_unreference(brw->batch.exec_bos[i]);
> @@ -319,6 +328,7 @@ 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;
> +   brw_xfb_obj->zero_offsets = brw->batch.saved.xfb.zero_offsets;
>     brw->urb.vsize = brw->batch.saved.urb.vsize;
>     brw->urb.gs_present = brw->batch.saved.urb.gs_present;
>     brw->urb.gsize = brw->batch.saved.urb.gsize;
> --
> 2.17.1
>
>