Mesa (master): i965: Share the flush for brw_blorp_miptree_download into a pbo

[Kenneth Graunke]

Module: Mesa
Branch: master
Commit: c866e0b3ca563de579d0231278239aa6427c9ddf
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=c866e0b3ca563de579d0231278239aa6427c9ddf

Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Wed Oct 11 21:43:45 2017 +0100

i965: Share the flush for brw_blorp_miptree_download into a pbo

As all users of brw_blorp_miptree_download() must emit a full pipeline
and cache flush when targetting a user PBO (as that PBO may then be
subsequently bound or *be* bound anywhere and outside of the driver
dirty tracking) move that flush into brw_blorp_miptree_download()
itself.

v2 (Ken): Rebase without userptr stuff so it can land sooner.

Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

---

 src/mesa/drivers/dri/i965/brw_blorp.c        | 22 ++++++++++++++++++++++
 src/mesa/drivers/dri/i965/intel_pixel_read.c | 24 +-----------------------
 src/mesa/drivers/dri/i965/intel_tex_image.c  |  9 +--------
 3 files changed, 24 insertions(+), 31 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c
index eec2b14174..ed4f9870f2 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp.c
+++ b/src/mesa/drivers/dri/i965/brw_blorp.c
@@ -1094,6 +1094,28 @@ brw_blorp_download_miptree(struct brw_context *brw,
 
    result = true;
 
+   /* As we implement PBO transfers by binding the user-provided BO as a
+    * fake framebuffer and rendering to it.  This breaks the invariant of the
+    * GL that nothing is able to render to a BO, causing nondeterministic
+    * corruption issues because the render cache is not coherent with a
+    * number of other caches that the BO could potentially be bound to
+    * afterwards.
+    *
+    * This could be solved in the same way that we guarantee texture
+    * coherency after a texture is attached to a framebuffer and
+    * rendered to, but that would involve checking *all* BOs bound to
+    * the pipeline for the case we need to emit a cache flush due to
+    * previous rendering to any of them -- Including vertex, index,
+    * uniform, atomic counter, shader image, transform feedback,
+    * indirect draw buffers, etc.
+    *
+    * That would increase the per-draw call overhead even though it's
+    * very unlikely that any of the BOs bound to the pipeline has been
+    * rendered to via a PBO at any point, so it seems better to just
+    * flush here unconditionally.
+    */
+   brw_emit_mi_flush(brw);
+
 err:
    brw_bo_unreference(dst_bo);
 
diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c b/src/mesa/drivers/dri/i965/intel_pixel_read.c
index 6aa9b53464..4528d6d265 100644
--- a/src/mesa/drivers/dri/i965/intel_pixel_read.c
+++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c
@@ -275,30 +275,8 @@ intelReadPixels(struct gl_context * ctx,
 
    if (_mesa_is_bufferobj(pack->BufferObj)) {
       if (intel_readpixels_blorp(ctx, x, y, width, height,
-                                 format, type, pixels, pack)) {
-         /* intel_readpixels_blorp() implements PBO transfers by
-          * binding the user-provided BO as a fake framebuffer and rendering
-          * to it.  This breaks the invariant of the GL that nothing is able
-          * to render to a BO, causing nondeterministic corruption issues
-          * because the render cache is not coherent with a number of other
-          * caches that the BO could potentially be bound to afterwards.
-          *
-          * This could be solved in the same way that we guarantee texture
-          * coherency after a texture is attached to a framebuffer and
-          * rendered to, but that would involve checking *all* BOs bound to
-          * the pipeline for the case we need to emit a cache flush due to
-          * previous rendering to any of them -- Including vertex, index,
-          * uniform, atomic counter, shader image, transform feedback,
-          * indirect draw buffers, etc.
-          *
-          * That would increase the per-draw call overhead even though it's
-          * very unlikely that any of the BOs bound to the pipeline has been
-          * rendered to via a PBO at any point, so it seems better to just
-          * flush here unconditionally.
-          */
-         brw_emit_mi_flush(brw);
+                                 format, type, pixels, pack))
          return;
-      }
 
       perf_debug("%s: fallback to CPU mapping in PBO case\n", __func__);
    }
diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
index e4d3f12038..5396e0a43b 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -747,15 +747,8 @@ intel_get_tex_sub_image(struct gl_context *ctx,
       if (intel_gettexsubimage_blorp(brw, texImage,
                                      xoffset, yoffset, zoffset,
                                      width, height, depth, format, type,
-                                     pixels, &ctx->Pack)) {
-         /* Flush to guarantee coherency between the render cache and other
-          * caches the PBO could potentially be bound to after this point.
-          * See the related comment in intelReadPixels() for a more detailed
-          * explanation.
-          */
-         brw_emit_mi_flush(brw);
+                                     pixels, &ctx->Pack))
          return;
-      }
 
       perf_debug("%s: fallback to CPU mapping in PBO case\n", __func__);
    }