From de4afae42a4d25479990b601083faf6ce0bb542f Mon Sep 17 00:00:00 2001 From: Nanley Chery Date: Mon, 13 Feb 2023 17:52:58 -0800 Subject: [PATCH] iris: Flush caches for aux-mode changes more often Memory accesses can get corrupted when there's a disagreement between: * the aux-mode of existing cache lines for a surface and * the aux-usage in that surface's RENDER_SURFACE_STATE object We have already prevented hardware from seeing this conflict for rendering operations, but due to how the L3 is shared among multiple clients in gfx12 (e.g., sampler engine, render engine, etc.), we need to expand the scope of the existing solution. Now, before any access of a compressible resource, we make sure to flush the prior aux-mode from the caches. The majority of changes here refactor things for use in a new function, flush_previous_aux_mode. The remaining change calls that function from within iris_resource_prepare_access. Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/6558 Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7625 Cc: 23.0 Reviewed-by: Kenneth Graunke Part-of: (cherry picked from commit 7c367bef0d593962a613dfb7de4d678f94574d36) --- .pick_status.json | 2 +- src/gallium/drivers/iris/iris_batch.c | 4 ++-- src/gallium/drivers/iris/iris_batch.h | 14 +++++------ src/gallium/drivers/iris/iris_resolve.c | 31 ++++++++++++++++--------- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index f65d702d49f..757ac98af97 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -508,7 +508,7 @@ "description": "iris: Flush caches for aux-mode changes more often", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null }, diff --git a/src/gallium/drivers/iris/iris_batch.c b/src/gallium/drivers/iris/iris_batch.c index 054c27e0797..c7a08a0e1f5 100644 --- a/src/gallium/drivers/iris/iris_batch.c +++ b/src/gallium/drivers/iris/iris_batch.c @@ -215,7 +215,7 @@ iris_init_batch(struct iris_context *ice, batch->bos_written = rzalloc_array(NULL, BITSET_WORD, BITSET_WORDS(batch->exec_array_size)); - batch->cache.render = _mesa_hash_table_create(NULL, _mesa_hash_pointer, + batch->bo_aux_modes = _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); batch->num_other_batches = 0; @@ -596,7 +596,7 @@ iris_batch_free(const struct iris_context *ice, struct iris_batch *batch) u_trace_fini(&batch->trace); - _mesa_hash_table_destroy(batch->cache.render, NULL); + _mesa_hash_table_destroy(batch->bo_aux_modes, NULL); if (INTEL_DEBUG(DEBUG_ANY)) intel_batch_decode_ctx_finish(&batch->decoder); diff --git a/src/gallium/drivers/iris/iris_batch.h b/src/gallium/drivers/iris/iris_batch.h index d9c77cfae95..cf067e48acc 100644 --- a/src/gallium/drivers/iris/iris_batch.h +++ b/src/gallium/drivers/iris/iris_batch.h @@ -137,14 +137,12 @@ struct iris_batch { struct iris_batch *other_batches[IRIS_BATCH_COUNT - 1]; unsigned num_other_batches; - struct { - /** - * Set of struct brw_bo * that have been rendered to within this - * batchbuffer and would need flushing before being used from another - * cache domain that isn't coherent with it (i.e. the sampler). - */ - struct hash_table *render; - } cache; + /** + * Table containing struct iris_bo * that have been accessed within this + * batchbuffer and would need flushing before being used with a different + * aux mode. + */ + struct hash_table *bo_aux_modes; struct intel_batch_decode_ctx decoder; struct hash_table_u64 *state_sizes; diff --git a/src/gallium/drivers/iris/iris_resolve.c b/src/gallium/drivers/iris/iris_resolve.c index bf54633af9d..41fefdb3321 100644 --- a/src/gallium/drivers/iris/iris_resolve.c +++ b/src/gallium/drivers/iris/iris_resolve.c @@ -387,16 +387,14 @@ iris_postdraw_update_resolve_tracking(struct iris_context *ice) } } -void -iris_cache_flush_for_render(struct iris_batch *batch, - struct iris_bo *bo, - enum isl_aux_usage aux_usage) +static void +flush_previous_aux_mode(struct iris_batch *batch, + const struct iris_bo *bo, + enum isl_aux_usage aux_usage) { - iris_emit_buffer_barrier_for(batch, bo, IRIS_DOMAIN_RENDER_WRITE); - - /* Check to see if this bo has been used by a previous rendering operation - * but with a different aux usage. If it has, flush the render cache so we - * ensure that it's only in there with one aux usage at a time. + /* Check to see if this BO has been put into caches by a previous operation + * but with a different aux usage. If it has, flush those caches to ensure + * that it's only in there with one aux usage at a time. * * Even though it's not obvious, this could easily happen in practice. * Suppose a client is blending on a surface with sRGB encode enabled on @@ -416,9 +414,9 @@ iris_cache_flush_for_render(struct iris_batch *batch, */ void *v_aux_usage = (void *) (uintptr_t) aux_usage; struct hash_entry *entry = - _mesa_hash_table_search_pre_hashed(batch->cache.render, bo->hash, bo); + _mesa_hash_table_search_pre_hashed(batch->bo_aux_modes, bo->hash, bo); if (!entry) { - _mesa_hash_table_insert_pre_hashed(batch->cache.render, bo->hash, bo, + _mesa_hash_table_insert_pre_hashed(batch->bo_aux_modes, bo->hash, bo, v_aux_usage); } else if (entry->data != v_aux_usage) { iris_emit_pipe_control_flush(batch, @@ -430,6 +428,15 @@ iris_cache_flush_for_render(struct iris_batch *batch, } } +void +iris_cache_flush_for_render(struct iris_batch *batch, + struct iris_bo *bo, + enum isl_aux_usage aux_usage) +{ + iris_emit_buffer_barrier_for(batch, bo, IRIS_DOMAIN_RENDER_WRITE); + flush_previous_aux_mode(batch, bo, aux_usage); +} + static void flush_ubos(struct iris_batch *batch, struct iris_shader_state *shs) @@ -865,6 +872,8 @@ iris_resource_prepare_access(struct iris_context *ice, iris_resource_set_aux_state(ice, res, level, layer, 1, new_state); } } + + flush_previous_aux_mode(batch, res->bo, aux_usage); } void