From aa58ff191f428c5deb977523351b171148d9167a Mon Sep 17 00:00:00 2001 From: Pierre-Eric Pelloux-Prayer Date: Tue, 14 Jun 2022 17:27:47 +0200 Subject: [PATCH] Revert "winsys/amdgpu: use AMDGPU_IB_FLAG_PREAMBLE for the CS preamble on gfx10+" MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This reverts commit 8edafaa25c5d649af6c016a61383d784a1ebb078. This fixes hangs on Navi21. Reviewed-by: Marek Olšák Part-of: --- src/gallium/drivers/radeonsi/si_cp_reg_shadowing.c | 4 +-- src/gallium/drivers/radeonsi/si_gfx_cs.c | 2 +- src/gallium/include/winsys/radeon_winsys.h | 19 ++++++---- src/gallium/winsys/amdgpu/drm/amdgpu_cs.c | 42 ++++++++-------------- src/gallium/winsys/radeon/drm/radeon_drm_cs.c | 8 ++--- 5 files changed, 31 insertions(+), 44 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_cp_reg_shadowing.c b/src/gallium/drivers/radeonsi/si_cp_reg_shadowing.c index 94d3e84..71b83d3 100644 --- a/src/gallium/drivers/radeonsi/si_cp_reg_shadowing.c +++ b/src/gallium/drivers/radeonsi/si_cp_reg_shadowing.c @@ -196,8 +196,8 @@ void si_init_cp_reg_shadowing(struct si_context *sctx) /* Setup preemption. The shadowing preamble will be executed as a preamble IB, * which will load register values from memory on a context switch. */ - sctx->ws->cs_set_preamble(&sctx->gfx_cs, shadowing_preamble->pm4, shadowing_preamble->ndw, - false, true); + sctx->ws->cs_setup_preemption(&sctx->gfx_cs, shadowing_preamble->pm4, + shadowing_preamble->ndw); si_pm4_free_state(sctx, shadowing_preamble, ~0); } } diff --git a/src/gallium/drivers/radeonsi/si_gfx_cs.c b/src/gallium/drivers/radeonsi/si_gfx_cs.c index ff2bb83..fff674d 100644 --- a/src/gallium/drivers/radeonsi/si_gfx_cs.c +++ b/src/gallium/drivers/radeonsi/si_gfx_cs.c @@ -443,7 +443,7 @@ void si_begin_new_gfx_cs(struct si_context *ctx, bool first_cs) struct si_pm4_state *preamble = is_secure ? ctx->cs_preamble_state_tmz : ctx->cs_preamble_state; ctx->ws->cs_set_preamble(&ctx->gfx_cs, preamble->pm4, preamble->ndw, - preamble != ctx->last_preamble, false); + preamble != ctx->last_preamble); ctx->last_preamble = preamble; } diff --git a/src/gallium/include/winsys/radeon_winsys.h b/src/gallium/include/winsys/radeon_winsys.h index b3b77df..2c9169d 100644 --- a/src/gallium/include/winsys/radeon_winsys.h +++ b/src/gallium/include/winsys/radeon_winsys.h @@ -510,18 +510,23 @@ struct radeon_winsys { * the command buffer. If the winsys doesn't support preambles, the packets are inserted * into the command buffer. * - * If preemption is enabled, the preamble is also executed when an IB is resumed, which can - * happen in the middle of it. - * * \param cs Command stream * \param preamble_ib Preamble IB for the context. * \param preamble_num_dw Number of dwords in the preamble IB. * \param preamble_changed Whether the preamble changed or is the same as the last one. - * \param enable_preemption If this is true, it also enables preemption. */ - bool (*cs_set_preamble)(struct radeon_cmdbuf *cs, const uint32_t *preamble_ib, - unsigned preamble_num_dw, bool preamble_changed, - bool enable_preemption); + void (*cs_set_preamble)(struct radeon_cmdbuf *cs, const uint32_t *preamble_ib, + unsigned preamble_num_dw, bool preamble_changed); + + /** + * Set up and enable mid command buffer preemption for the command stream. + * + * \param cs Command stream + * \param preamble_ib Non-preemptible preamble IB for the context. + * \param preamble_num_dw Number of dwords in the preamble IB. + */ + bool (*cs_setup_preemption)(struct radeon_cmdbuf *cs, const uint32_t *preamble_ib, + unsigned preamble_num_dw); /** * Destroy a command stream. diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c index 2b03a62..ec6aa40 100644 --- a/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_cs.c @@ -1027,9 +1027,16 @@ amdgpu_cs_create(struct radeon_cmdbuf *rcs, return true; } -static bool amdgpu_cs_set_preamble(struct radeon_cmdbuf *rcs, const uint32_t *preamble_ib, - unsigned preamble_num_dw, bool preamble_changed, - bool enable_preemption) +static void amdgpu_cs_set_preamble(struct radeon_cmdbuf *cs, const uint32_t *preamble_ib, + unsigned preamble_num_dw, bool preamble_changed) +{ + /* TODO: implement this properly */ + radeon_emit_array(cs, preamble_ib, preamble_num_dw); +} + +static bool +amdgpu_cs_setup_preemption(struct radeon_cmdbuf *rcs, const uint32_t *preamble_ib, + unsigned preamble_num_dw) { struct amdgpu_cs *cs = amdgpu_cs(rcs); struct amdgpu_winsys *ws = cs->ws; @@ -1038,23 +1045,6 @@ static bool amdgpu_cs_set_preamble(struct radeon_cmdbuf *rcs, const uint32_t *pr struct pb_buffer *preamble_bo; uint32_t *map; - assert(preamble_ib); - /* The preamble can be set only once for preemption. */ - assert(!enable_preemption || !cs->preamble_ib_bo); - - /* The preamble IB causes GPU hangs on Stoney. To be safe, don't use the preamble IB on - * chips older than gfx10, and instead paste the preamble into the main command buffer. - */ - if (ws->info.gfx_level < GFX10) { - radeon_emit_array(rcs, preamble_ib, preamble_num_dw); - return true; - } - - if (!preamble_changed && !enable_preemption) { - assert(cs->preamble_ib_bo); /* we shouldn't get no-change calls with no preamble */ - return true; - } - /* Create the preamble IB buffer. */ preamble_bo = amdgpu_bo_create(ws, size, ws->info.ib_alignment, RADEON_DOMAIN_VRAM, @@ -1080,20 +1070,15 @@ static bool amdgpu_cs_set_preamble(struct radeon_cmdbuf *rcs, const uint32_t *pr map[preamble_num_dw++] = PKT3_NOP_PAD; amdgpu_bo_unmap(&ws->dummy_ws.base, preamble_bo); - /* Wait until the CS job finishes, so that we don't mess up IB_PREAMBLE while the IB is being - * submitted. - */ - amdgpu_cs_sync_flush(rcs); - for (unsigned i = 0; i < 2; i++) { csc[i]->ib[IB_PREAMBLE].va_start = amdgpu_winsys_bo(preamble_bo)->va; csc[i]->ib[IB_PREAMBLE].ib_bytes = preamble_num_dw * 4; - if (enable_preemption) - csc[i]->ib[IB_MAIN].flags |= AMDGPU_IB_FLAG_PREEMPT; + csc[i]->ib[IB_MAIN].flags |= AMDGPU_IB_FLAG_PREEMPT; } - radeon_bo_reference(&ws->dummy_ws.base, &cs->preamble_ib_bo, preamble_bo); + assert(!cs->preamble_ib_bo); + cs->preamble_ib_bo = preamble_bo; amdgpu_cs_add_buffer(rcs, cs->preamble_ib_bo, RADEON_USAGE_READ | RADEON_PRIO_IB, 0); @@ -1868,6 +1853,7 @@ void amdgpu_cs_init_functions(struct amdgpu_screen_winsys *ws) ws->base.ctx_query_reset_status = amdgpu_ctx_query_reset_status; ws->base.cs_create = amdgpu_cs_create; ws->base.cs_set_preamble = amdgpu_cs_set_preamble; + ws->base.cs_setup_preemption = amdgpu_cs_setup_preemption; ws->base.cs_destroy = amdgpu_cs_destroy; ws->base.cs_add_buffer = amdgpu_cs_add_buffer; ws->base.cs_validate = amdgpu_cs_validate; diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c index 45c8d09..51864a3 100644 --- a/src/gallium/winsys/radeon/drm/radeon_drm_cs.c +++ b/src/gallium/winsys/radeon/drm/radeon_drm_cs.c @@ -216,15 +216,11 @@ radeon_drm_cs_create(struct radeon_cmdbuf *rcs, return true; } -static bool radeon_drm_cs_set_preamble(struct radeon_cmdbuf *cs, const uint32_t *preamble_ib, - unsigned preamble_num_dw, bool preamble_changed, - bool enable_preemption) +static void radeon_drm_cs_set_preamble(struct radeon_cmdbuf *cs, const uint32_t *preamble_ib, + unsigned preamble_num_dw, bool preamble_changed) { - assert(!enable_preemption); - /* The radeon kernel driver doesn't support preambles. */ radeon_emit_array(cs, preamble_ib, preamble_num_dw); - return true; } int radeon_lookup_buffer(struct radeon_cs_context *csc, struct radeon_bo *bo) -- 2.7.4