Mesa (main): crocus: Delete the MI_COPY_MEM_MEM resource_copy_region implementation.

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 7681500ead4f085d75b0b2f39b48d1f9dc637dc9
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=7681500ead4f085d75b0b2f39b48d1f9dc637dc9

Author: Dave Airlie <airlied at redhat.com>
Date:   Wed Sep 15 14:26:36 2021 +1000

crocus: Delete the MI_COPY_MEM_MEM resource_copy_region implementation.

(ported from iris - airlied)

The MI_COPY_MEM_MEM version of resource_copy_region has known bugs:

    It's failing to set valid_buffer_range correctly
    It's missing iris_emit_buffer_barrier_for() for the source/destination, so there may be missing flushes.
    There are some bad interactions with the tile cache and VF using L3.

Even with those fixed, if you expand the "no more than 16 bytes" restriction to allow copies up to 1024 bytes, then it starts failing Piglit tests on Icelake.

We could probably fix this. However, I had originally only measured a 0.689096% +/- 0.473968% (n=4) speedup in Shadow of Mordor's OpenGL port, which is already fairly small, especially before adding missing flushes. Further, some of that likely came from not switching between render and compute...which we'll soon be able to avoid thanks to BLOCS.

Folks were also worried that MI_COPY_MEM_MEM can't be pipelined, and that stalling the command streamer may actually slow things down, especially as the GPUs become more powerful. We aren't really sure about this, but it's another concern.

So, let's just get rid of this optimization. It seemed like a good idea at the time, but it's just causing issues for very little gain.

Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/13374>

---

 src/gallium/drivers/crocus/crocus_blit.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/src/gallium/drivers/crocus/crocus_blit.c b/src/gallium/drivers/crocus/crocus_blit.c
index 7620efc932b..2fad9a30aaf 100644
--- a/src/gallium/drivers/crocus/crocus_blit.c
+++ b/src/gallium/drivers/crocus/crocus_blit.c
@@ -754,20 +754,6 @@ crocus_copy_region(struct blorp_context *blorp,
    tex_cache_flush_hack(batch, ISL_FORMAT_UNSUPPORTED, src_res->surf.format);
 }
 
-static struct crocus_batch *
-get_preferred_batch(struct crocus_context *ice, struct crocus_bo *bo)
-{
-   /* If the compute batch is already using this buffer, we'd prefer to
-    * continue queueing in the compute batch.
-    */
-   if (crocus_batch_references(&ice->batches[CROCUS_BATCH_COMPUTE], bo))
-      return &ice->batches[CROCUS_BATCH_COMPUTE];
-
-   /* Otherwise default to the render batch. */
-   return &ice->batches[CROCUS_BATCH_RENDER];
-}
-
-
 /**
  * The pipe->resource_copy_region() driver hook.
  *
@@ -795,21 +781,6 @@ crocus_resource_copy_region(struct pipe_context *ctx,
    if (crocus_resource_unfinished_aux_import(dst))
       crocus_resource_finish_aux_import(ctx->screen, dst);
 
-   /* Use MI_COPY_MEM_MEM for tiny (<= 16 byte, % 4) buffer copies. */
-   if (p_src->target == PIPE_BUFFER && p_dst->target == PIPE_BUFFER &&
-       (src_box->width % 4 == 0) && src_box->width <= 16 &&
-       screen->vtbl.copy_mem_mem) {
-      struct crocus_bo *dst_bo = crocus_resource_bo(p_dst);
-      batch = get_preferred_batch(ice, dst_bo);
-      crocus_batch_maybe_flush(batch, 24 + 5 * (src_box->width / 4));
-      crocus_emit_pipe_control_flush(batch,
-                                     "stall for MI_COPY_MEM_MEM copy_region",
-                                     PIPE_CONTROL_CS_STALL);
-      screen->vtbl.copy_mem_mem(batch, dst_bo, dstx, crocus_resource_bo(p_src),
-                                src_box->x, src_box->width);
-      return;
-   }
-
    if (devinfo->ver < 6 && util_format_is_depth_or_stencil(p_dst->format)) {
       util_resource_copy_region(ctx, p_dst, dst_level, dstx, dsty, dstz,
                                 p_src, src_level, src_box);