From 82f36cd8e3a5d15857e73130d6a1cca95db55aa9 Mon Sep 17 00:00:00 2001 From: Christoph Pillmayer Date: Wed, 14 May 2025 12:17:04 +0000 Subject: [PATCH] panfrost: Make ls tracker mandatory in cs_builder This commit moves cs_load_store_tracker from the cs_builder_conf to cs_builder and makes it no longer optional to supply. Reviewed-by: Boris Brezillon Reviewed-by: Lars-Ivar Hesselberg Simonsen Part-of: --- src/gallium/drivers/panfrost/pan_csf.c | 12 +-- src/gallium/drivers/panfrost/pan_csf.h | 3 - src/panfrost/genxml/cs_builder.h | 85 +++++++++---------- src/panfrost/vulkan/csf/panvk_cmd_buffer.h | 2 - src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c | 7 +- .../vulkan/csf/panvk_vX_exception_handler.c | 1 + src/panfrost/vulkan/csf/panvk_vX_queue.c | 1 + src/panfrost/vulkan/csf/panvk_vX_utrace.c | 1 + 8 files changed, 46 insertions(+), 66 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_csf.c b/src/gallium/drivers/panfrost/pan_csf.c index b40b18a7bee..1d24833f78d 100644 --- a/src/gallium/drivers/panfrost/pan_csf.c +++ b/src/gallium/drivers/panfrost/pan_csf.c @@ -118,6 +118,7 @@ csf_oom_handler_init(struct panfrost_context *ctx) .nr_registers = csif_info->cs_reg_count, .nr_kernel_registers = MAX2(csif_info->unpreserved_cs_reg_count, 4), .reg_perm = (dev->debug & PAN_DBG_CS) ? csf_reg_perm_cb : NULL, + .ls_sb_slot = 0, }; cs_builder_init(&b, &conf, queue); @@ -225,7 +226,6 @@ void GENX(csf_cleanup_batch)(struct panfrost_batch *batch) { free(batch->csf.cs.builder); - free(batch->csf.cs.ls_tracker); panfrost_pool_cleanup(&batch->csf.cs_chunk_pool); } @@ -248,13 +248,6 @@ GENX(csf_init_batch)(struct panfrost_batch *batch) "CS chunk pool", false, true)) return -1; - if (dev->debug & PAN_DBG_CS) { - /* Load/store tracker if extra checks are enabled. */ - batch->csf.cs.ls_tracker = - calloc(1, sizeof(struct cs_load_store_tracker)); - batch->csf.cs.ls_tracker->sb_slot = 0; - } - /* Allocate and bind the command queue */ struct cs_buffer queue = csf_alloc_cs_buffer(batch); if (!queue.gpu) @@ -268,8 +261,8 @@ GENX(csf_init_batch)(struct panfrost_batch *batch) .nr_kernel_registers = MAX2(csif_info->unpreserved_cs_reg_count, 4), .alloc_buffer = csf_alloc_cs_buffer, .cookie = batch, - .ls_tracker = batch->csf.cs.ls_tracker, .reg_perm = (dev->debug & PAN_DBG_CS) ? csf_reg_perm_cb : NULL, + .ls_sb_slot = 0, }; /* Setup the queue builder */ @@ -1535,6 +1528,7 @@ GENX(csf_init_context)(struct panfrost_context *ctx) const struct cs_builder_conf bconf = { .nr_registers = csif_info->cs_reg_count, .nr_kernel_registers = MAX2(csif_info->unpreserved_cs_reg_count, 4), + .ls_sb_slot = 0, }; struct cs_builder b; cs_builder_init(&b, &bconf, init_buffer); diff --git a/src/gallium/drivers/panfrost/pan_csf.h b/src/gallium/drivers/panfrost/pan_csf.h index 16d343bae84..2fe417ef4d6 100644 --- a/src/gallium/drivers/panfrost/pan_csf.h +++ b/src/gallium/drivers/panfrost/pan_csf.h @@ -68,9 +68,6 @@ struct panfrost_csf_batch { /* CS state, written through the CS, and checked when PAN_MESA_DEBUG=sync. */ struct panfrost_ptr state; - - /* CS load/store tracker if extra checks are enabled. */ - struct cs_load_store_tracker *ls_tracker; } cs; /* Pool used to allocate CS chunks. */ diff --git a/src/panfrost/genxml/cs_builder.h b/src/panfrost/genxml/cs_builder.h index 22a4d133b05..c962822411b 100644 --- a/src/panfrost/genxml/cs_builder.h +++ b/src/panfrost/genxml/cs_builder.h @@ -77,7 +77,6 @@ struct cs_buffer { struct cs_load_store_tracker { BITSET_DECLARE(pending_loads, 256); BITSET_DECLARE(pending_stores, 256); - uint8_t sb_slot; }; /** @@ -109,9 +108,6 @@ struct cs_builder_conf { /* CS buffer allocator */ struct cs_buffer (*alloc_buffer)(void *cookie); - /* Optional load/store tracker. */ - struct cs_load_store_tracker *ls_tracker; - /* Optional dirty registers tracker. */ struct cs_dirty_tracker *dirty_tracker; @@ -120,6 +116,9 @@ struct cs_builder_conf { /* Cookie passed back to alloc_buffer() */ void *cookie; + + /* SB slot used for load/store instructions. */ + uint8_t ls_sb_slot; }; /* The CS is formed of one or more CS chunks linked with JUMP instructions. @@ -185,6 +184,11 @@ struct cs_builder { /* Current CS chunk. */ struct cs_chunk cur_chunk; + /* Current load/store tracker. */ + struct cs_load_store_tracker *cur_ls_tracker; + + struct cs_load_store_tracker root_ls_tracker; + /* Temporary storage for inner blocks that need to be built * and copied in one monolithic sequence of instructions with no * jump in the middle. @@ -216,8 +220,11 @@ cs_builder_init(struct cs_builder *b, const struct cs_builder_conf *conf, .conf = *conf, .root_chunk.buffer = root_buffer, .cur_chunk.buffer = root_buffer, + .cur_ls_tracker = &b->root_ls_tracker, }; + *b->cur_ls_tracker = (struct cs_load_store_tracker){0}; + /* We need at least 3 registers for CS chunk linking. Assume the kernel needs * at least that too. */ @@ -329,14 +336,12 @@ cs_src_tuple(struct cs_builder *b, struct cs_index src, ASSERTED unsigned count, } } - struct cs_load_store_tracker *ls_tracker = b->conf.ls_tracker; + struct cs_load_store_tracker *ls_tracker = b->cur_ls_tracker; - if (unlikely(ls_tracker)) { - for (unsigned i = reg; i < reg + count; i++) { - if ((mask & BITFIELD_BIT(i - reg)) && - BITSET_TEST(ls_tracker->pending_loads, i)) - assert(!"register used as a source before flushing loads\n"); - } + for (unsigned i = reg; i < reg + count; i++) { + if ((mask & BITFIELD_BIT(i - reg)) && + BITSET_TEST(ls_tracker->pending_loads, i)) + assert(!"register used as a source before flushing loads\n"); } return reg; @@ -369,15 +374,12 @@ cs_dst_tuple(struct cs_builder *b, struct cs_index dst, ASSERTED unsigned count, } } - struct cs_load_store_tracker *ls_tracker = b->conf.ls_tracker; + struct cs_load_store_tracker *ls_tracker = b->cur_ls_tracker; - if (unlikely(ls_tracker)) { - for (unsigned i = reg; i < reg + count; i++) { - if ((mask & BITFIELD_BIT(i - reg)) && - BITSET_TEST(ls_tracker->pending_stores, i)) - assert( - !"register reused as a destination before flushing stores\n"); - } + for (unsigned i = reg; i < reg + count; i++) { + if ((mask & BITFIELD_BIT(i - reg)) && + BITSET_TEST(ls_tracker->pending_stores, i)) + assert(!"register reused as a destination before flushing stores\n"); } if (unlikely(b->conf.dirty_tracker)) { @@ -995,13 +997,10 @@ struct cs_loop { static inline void cs_loop_diverge_ls_update(struct cs_builder *b, struct cs_loop *loop) { - if (likely(!b->conf.ls_tracker)) - return; - if (!loop->orig_ls_state) { - loop->orig_ls_state = b->conf.ls_tracker; + loop->orig_ls_state = b->cur_ls_tracker; loop->ls_state = *loop->orig_ls_state; - b->conf.ls_tracker = &loop->ls_state; + b->cur_ls_tracker = &loop->ls_state; } else { BITSET_OR(loop->orig_ls_state->pending_loads, loop->orig_ls_state->pending_loads, @@ -1079,7 +1078,7 @@ cs_while_end(struct cs_builder *b, struct cs_loop *loop) BITSET_OR(loop->orig_ls_state->pending_stores, loop->orig_ls_state->pending_stores, loop->ls_state.pending_stores); - b->conf.ls_tracker = loop->orig_ls_state; + b->cur_ls_tracker = loop->orig_ls_state; } } @@ -1112,7 +1111,8 @@ cs_move64_to(struct cs_builder *b, struct cs_index dest, uint64_t imm) static inline void cs_wait_slots(struct cs_builder *b, unsigned wait_mask) { - struct cs_load_store_tracker *ls_tracker = b->conf.ls_tracker; + struct cs_load_store_tracker *ls_tracker = b->cur_ls_tracker; + assert(ls_tracker != NULL); cs_emit(b, WAIT, I) { I.wait_mask = wait_mask; @@ -1121,8 +1121,7 @@ cs_wait_slots(struct cs_builder *b, unsigned wait_mask) /* We don't do advanced tracking of cs_defer(), and assume that * load/store will be flushed with an explicit wait on the load/store * scoreboard. */ - if (unlikely(ls_tracker) && - (wait_mask & BITFIELD_BIT(ls_tracker->sb_slot))) { + if (wait_mask & BITFIELD_BIT(b->conf.ls_sb_slot)) { BITSET_CLEAR_RANGE(ls_tracker->pending_loads, 0, 255); BITSET_CLEAR_RANGE(ls_tracker->pending_stores, 0, 255); } @@ -1322,11 +1321,9 @@ cs_load_to(struct cs_builder *b, struct cs_index dest, struct cs_index address, I.offset = offset; } - if (unlikely(b->conf.ls_tracker)) { - for (unsigned i = 0; i < count; i++) { - if (mask & BITFIELD_BIT(i)) - BITSET_SET(b->conf.ls_tracker->pending_loads, base_reg + i); - } + for (unsigned i = 0; i < count; i++) { + if (mask & BITFIELD_BIT(i)) + BITSET_SET(b->cur_ls_tracker->pending_loads, base_reg + i); } } @@ -1358,11 +1355,9 @@ cs_store(struct cs_builder *b, struct cs_index data, struct cs_index address, I.offset = offset; } - if (unlikely(b->conf.ls_tracker)) { - for (unsigned i = 0; i < count; i++) { - if (mask & BITFIELD_BIT(i)) - BITSET_SET(b->conf.ls_tracker->pending_stores, base_reg + i); - } + for (unsigned i = 0; i < count; i++) { + if (mask & BITFIELD_BIT(i)) + BITSET_SET(b->cur_ls_tracker->pending_stores, base_reg + i); } } @@ -1398,8 +1393,7 @@ cs_set_scoreboard_entry(struct cs_builder *b, unsigned ep, unsigned other) /* We assume the load/store scoreboard entry is static to keep things * simple. */ - if (unlikely(b->conf.ls_tracker)) - assert(b->conf.ls_tracker->sb_slot == other); + assert(b->conf.ls_sb_slot == other); } #else static inline void @@ -1413,9 +1407,8 @@ cs_set_state_imm32(struct cs_builder *b, enum mali_cs_set_state_type state, /* We assume the load/store scoreboard entry is static to keep things * simple. */ - if (state == MALI_CS_SET_STATE_TYPE_SB_SEL_OTHER && - unlikely(b->conf.ls_tracker)) - assert(b->conf.ls_tracker->sb_slot == value); + if (state == MALI_CS_SET_STATE_TYPE_SB_SEL_OTHER) + assert(b->conf.ls_sb_slot == value); } #endif @@ -1651,7 +1644,7 @@ cs_match_start(struct cs_builder *b, struct cs_match *match, *match = (struct cs_match){ .val = val, .scratch_reg = scratch_reg, - .orig_ls_state = b->conf.ls_tracker, + .orig_ls_state = b->cur_ls_tracker, }; cs_block_start(b, &match->block); @@ -1666,7 +1659,7 @@ cs_match_case_ls_set(struct cs_builder *b, struct cs_match *match) { if (unlikely(match->orig_ls_state)) { match->case_ls_state = *match->orig_ls_state; - b->conf.ls_tracker = &match->case_ls_state; + b->cur_ls_tracker = &match->case_ls_state; } } @@ -1749,7 +1742,7 @@ cs_match_end(struct cs_builder *b, struct cs_match *match) *match->orig_ls_state = match->ls_state; } - b->conf.ls_tracker = match->orig_ls_state; + b->cur_ls_tracker = match->orig_ls_state; } cs_set_label(b, &match->next_case_label); diff --git a/src/panfrost/vulkan/csf/panvk_cmd_buffer.h b/src/panfrost/vulkan/csf/panvk_cmd_buffer.h index 434ef846e37..e2dddf62144 100644 --- a/src/panfrost/vulkan/csf/panvk_cmd_buffer.h +++ b/src/panfrost/vulkan/csf/panvk_cmd_buffer.h @@ -232,8 +232,6 @@ struct panvk_cs_reg_upd_context { struct panvk_cs_state { struct cs_builder builder; - struct cs_load_store_tracker ls_tracker; - /* Used to debug register writes in invalid contexts. */ struct { struct panvk_cs_reg_upd_context *upd_ctx_stack; diff --git a/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c b/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c index 571561c7459..1efe2ea35f1 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c +++ b/src/panfrost/vulkan/csf/panvk_vX_cmd_buffer.c @@ -703,15 +703,10 @@ init_cs_builders(struct panvk_cmd_buffer *cmdbuf) .nr_kernel_registers = MAX2(csif_info->unpreserved_cs_reg_count, 4), .alloc_buffer = alloc_cs_buffer, .cookie = cmdbuf, + .ls_sb_slot = SB_ID(LS), }; if (instance->debug_flags & PANVK_DEBUG_CS) { - cmdbuf->state.cs[i].ls_tracker = (struct cs_load_store_tracker){ - .sb_slot = SB_ID(LS), - }; - - conf.ls_tracker = &cmdbuf->state.cs[i].ls_tracker; - cmdbuf->state.cs[i].reg_access.upd_ctx_stack = NULL; cmdbuf->state.cs[i].reg_access.base_perm = base_reg_perms[i]; conf.reg_perm = cs_reg_perm; diff --git a/src/panfrost/vulkan/csf/panvk_vX_exception_handler.c b/src/panfrost/vulkan/csf/panvk_vX_exception_handler.c index 2baf0016591..8cf19875ff6 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_exception_handler.c +++ b/src/panfrost/vulkan/csf/panvk_vX_exception_handler.c @@ -40,6 +40,7 @@ generate_tiler_oom_handler(struct panvk_device *dev, .nr_registers = csif_info->cs_reg_count, .nr_kernel_registers = MAX2(csif_info->unpreserved_cs_reg_count, 4), .reg_perm = tiler_oom_reg_perm_cb, + .ls_sb_slot = SB_ID(LS), }; cs_builder_init(&b, &conf, handler_mem); diff --git a/src/panfrost/vulkan/csf/panvk_vX_queue.c b/src/panfrost/vulkan/csf/panvk_vX_queue.c index 3c39caeed2e..7f02bc1a520 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_queue.c +++ b/src/panfrost/vulkan/csf/panvk_vX_queue.c @@ -400,6 +400,7 @@ init_subqueue(struct panvk_queue *queue, enum panvk_subqueue_id subqueue) const struct cs_builder_conf conf = { .nr_registers = csif_info->cs_reg_count, .nr_kernel_registers = MAX2(csif_info->unpreserved_cs_reg_count, 4), + .ls_sb_slot = SB_ID(LS), }; struct cs_builder b; diff --git a/src/panfrost/vulkan/csf/panvk_vX_utrace.c b/src/panfrost/vulkan/csf/panvk_vX_utrace.c index 5d38534c6a3..96308efc042 100644 --- a/src/panfrost/vulkan/csf/panvk_vX_utrace.c +++ b/src/panfrost/vulkan/csf/panvk_vX_utrace.c @@ -157,6 +157,7 @@ panvk_per_arch(utrace_clone_init_builder)(struct cs_builder *b, .nr_kernel_registers = MAX2(csif_info->unpreserved_cs_reg_count, 4), .alloc_buffer = alloc_clone_buffer, .cookie = pool, + .ls_sb_slot = SB_ID(LS), }; cs_builder_init(b, &builder_conf, (struct cs_buffer){0}); }