Module: Mesa Branch: staging/20.3 Commit: 7841689d0cfe739d7d4f3c166d70146e187b4e2d URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=7841689d0cfe739d7d4f3c166d70146e187b4e2d
Author: Pierre-Eric Pelloux-Prayer <[email protected]> Date: Tue Jan 26 17:46:44 2021 +0100 radeonsi: fix read from compute / write from draw sync A compute dispatch should see the result of a previous draw command. radeonsi was missing this implicit sync, causing rendering artifacts: the compute shader was reading from a texture still being written to by the previous draw. Framebuffer BOs are marked with RADEON_USAGE_NEEDS_IMPLICIT_SYNC, so compute jobs will sync. v2: use RADEON_USAGE_NEEDS_IMPLICIT_SYNC v3: unconditionally make CB coherent after a flush (cherry-picked from 21.1 backport c52f98e6eac) Reviewed-by: Zoltán Böszörményi <[email protected]> (v3) Reviewed-by: Marek Olšák <[email protected]> (v3) Cc: mesa-stable Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4032 Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/2878 Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/1336 Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9677> --- src/gallium/drivers/radeon/radeon_winsys.h | 7 ++++- src/gallium/drivers/radeonsi/si_compute.c | 46 ++++++++++++++++++++++++++++++ src/gallium/drivers/radeonsi/si_gfx_cs.c | 7 +++++ src/gallium/drivers/radeonsi/si_pipe.h | 3 ++ src/gallium/drivers/radeonsi/si_state.c | 5 ++-- 5 files changed, 65 insertions(+), 3 deletions(-) diff --git a/src/gallium/drivers/radeon/radeon_winsys.h b/src/gallium/drivers/radeon/radeon_winsys.h index ae124bdb968..965e04e4470 100644 --- a/src/gallium/drivers/radeon/radeon_winsys.h +++ b/src/gallium/drivers/radeon/radeon_winsys.h @@ -98,7 +98,12 @@ enum radeon_bo_usage /* The winsys ensures that the CS submission will be scheduled after * previously flushed CSs referencing this BO in a conflicting way. */ - RADEON_USAGE_SYNCHRONIZED = 8 + RADEON_USAGE_SYNCHRONIZED = 8, + + /* When used, an implicit sync is done to make sure a compute shader + * will read the written values from a previous draw. + */ + RADEON_USAGE_NEEDS_IMPLICIT_SYNC = 16, }; enum radeon_map_flags diff --git a/src/gallium/drivers/radeonsi/si_compute.c b/src/gallium/drivers/radeonsi/si_compute.c index 791f438f161..e58958ab211 100644 --- a/src/gallium/drivers/radeonsi/si_compute.c +++ b/src/gallium/drivers/radeonsi/si_compute.c @@ -795,6 +795,47 @@ static void si_emit_dispatch_packets(struct si_context *sctx, const struct pipe_ } } +static bool si_check_needs_implicit_sync(struct si_context *sctx) +{ + /* If the compute shader is going to read from a texture/image written by a + * previous draw, we must wait for its completion before continuing. + * Buffers and image stores (from the draw) are not taken into consideration + * because that's the app responsibility. + * + * The OpenGL 4.6 spec says: + * + * buffer object and texture stores performed by shaders are not + * automatically synchronized + */ + struct si_shader_info *info = &sctx->cs_shader_state.program->sel.info; + struct si_samplers *samplers = &sctx->samplers[PIPE_SHADER_COMPUTE]; + unsigned mask = samplers->enabled_mask & info->base.textures_used; + + while (mask) { + int i = u_bit_scan(&mask); + struct si_sampler_view *sview = (struct si_sampler_view *)samplers->views[i]; + + struct si_resource *res = si_resource(sview->base.texture); + if (sctx->ws->cs_is_buffer_referenced(sctx->gfx_cs, res->buf, + RADEON_USAGE_NEEDS_IMPLICIT_SYNC)) + return true; + } + + struct si_images *images = &sctx->images[PIPE_SHADER_COMPUTE]; + mask = u_bit_consecutive(0, info->base.num_images); + + while (mask) { + int i = u_bit_scan(&mask); + struct pipe_image_view *sview = &images->views[i]; + + struct si_resource *res = si_resource(sview->resource); + if (sctx->ws->cs_is_buffer_referenced(sctx->gfx_cs, res->buf, + RADEON_USAGE_NEEDS_IMPLICIT_SYNC)) + return true; + } + return false; +} + static void si_launch_grid(struct pipe_context *ctx, const struct pipe_grid_info *info) { struct si_context *sctx = (struct si_context *)ctx; @@ -821,6 +862,11 @@ static void si_launch_grid(struct pipe_context *ctx, const struct pipe_grid_info if (sctx->last_num_draw_calls != sctx->num_draw_calls) { si_update_fb_dirtiness_after_rendering(sctx); sctx->last_num_draw_calls = sctx->num_draw_calls; + + if (sctx->force_cb_shader_coherent || si_check_needs_implicit_sync(sctx)) + si_make_CB_shader_coherent(sctx, 0, + sctx->framebuffer.CB_has_shader_readable_metadata, + sctx->framebuffer.all_DCC_pipe_aligned); } si_decompress_textures(sctx, 1 << PIPE_SHADER_COMPUTE); diff --git a/src/gallium/drivers/radeonsi/si_gfx_cs.c b/src/gallium/drivers/radeonsi/si_gfx_cs.c index e7c169e7408..3ba46c87f13 100644 --- a/src/gallium/drivers/radeonsi/si_gfx_cs.c +++ b/src/gallium/drivers/radeonsi/si_gfx_cs.c @@ -588,4 +588,11 @@ void si_begin_new_gfx_cs(struct si_context *ctx, bool first_cs) ctx->index_ring_base = ctx->index_ring_size_per_ib; ctx->index_ring_offset = 0; + + /* All buffer references are removed on a flush, so si_check_needs_implicit_sync + * cannot determine if si_make_CB_shader_coherent() needs to be called. + * ctx->force_cb_shader_coherent will be cleared by the first call to + * si_make_CB_shader_coherent. + */ + ctx->force_cb_shader_coherent = true; } diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h index b36d695727d..3e5e6231a4e 100644 --- a/src/gallium/drivers/radeonsi/si_pipe.h +++ b/src/gallium/drivers/radeonsi/si_pipe.h @@ -1257,6 +1257,8 @@ struct si_context { struct list_head shader_query_buffers; unsigned num_active_shader_queries; + bool force_cb_shader_coherent; + /* Statistics gathering for the DCC enablement heuristic. It can't be * in si_texture because si_texture can be shared by multiple * contexts. This is for back buffers only. We shouldn't get too many @@ -1711,6 +1713,7 @@ static inline void si_make_CB_shader_coherent(struct si_context *sctx, unsigned bool shaders_read_metadata, bool dcc_pipe_aligned) { sctx->flags |= SI_CONTEXT_FLUSH_AND_INV_CB | SI_CONTEXT_INV_VCACHE; + sctx->force_cb_shader_coherent = false; if (sctx->chip_class >= GFX10) { if (sctx->screen->info.tcc_harvested) diff --git a/src/gallium/drivers/radeonsi/si_state.c b/src/gallium/drivers/radeonsi/si_state.c index 6f018007ea7..d15e1c8f53f 100644 --- a/src/gallium/drivers/radeonsi/si_state.c +++ b/src/gallium/drivers/radeonsi/si_state.c @@ -2910,7 +2910,7 @@ static void si_emit_framebuffer_state(struct si_context *sctx) tex = (struct si_texture *)cb->base.texture; radeon_add_to_buffer_list( - sctx, sctx->gfx_cs, &tex->buffer, RADEON_USAGE_READWRITE, + sctx, sctx->gfx_cs, &tex->buffer, RADEON_USAGE_READWRITE | RADEON_USAGE_NEEDS_IMPLICIT_SYNC, tex->buffer.b.b.nr_samples > 1 ? RADEON_PRIO_COLOR_BUFFER_MSAA : RADEON_PRIO_COLOR_BUFFER); if (tex->cmask_buffer && tex->cmask_buffer != &tex->buffer) { @@ -2920,7 +2920,8 @@ static void si_emit_framebuffer_state(struct si_context *sctx) if (tex->dcc_separate_buffer) radeon_add_to_buffer_list(sctx, sctx->gfx_cs, tex->dcc_separate_buffer, - RADEON_USAGE_READWRITE, RADEON_PRIO_SEPARATE_META); + RADEON_USAGE_READWRITE | RADEON_USAGE_NEEDS_IMPLICIT_SYNC, + RADEON_PRIO_SEPARATE_META); /* Compute mutable surface parameters. */ cb_color_base = tex->buffer.gpu_address >> 8; _______________________________________________ mesa-commit mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-commit
