Mesa (main): Revert "winsys/amdgpu: use AMDGPU_IB_FLAG_PREAMBLE for the CS preamble on gfx10+"
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Wed Jun 15 11:02:19 UTC 2022
Module: Mesa
Branch: main
Commit: aa58ff191f428c5deb977523351b171148d9167a
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=aa58ff191f428c5deb977523351b171148d9167a
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
Date: Tue Jun 14 17:27:47 2022 +0200
Revert "winsys/amdgpu: use AMDGPU_IB_FLAG_PREAMBLE for the CS preamble on gfx10+"
This reverts commit 8edafaa25c5d649af6c016a61383d784a1ebb078.
This fixes hangs on Navi21.
Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/17032>
---
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 94d3e84687b..71b83d3223c 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 ff2bb834324..fff674d6997 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 b3b77df5262..2c9169dbfcc 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 2b03a62ed00..ec6aa40d2c9 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 45c8d091d84..51864a31ae5 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)
More information about the mesa-commit
mailing list