From: Alex Deucher <alexander.deucher@amd.com>
To: <amd-gfx@lists.freedesktop.org>, <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>,
Jesse Zhang <jesse.zhang@amd.com>
Subject: [PATCH 12/12] drm/amdgpu: rework ring reset backup and reemit v8
Date: Thu, 29 Jan 2026 15:37:31 -0500 [thread overview]
Message-ID: <20260129203731.21506-13-alexander.deucher@amd.com> (raw)
In-Reply-To: <20260129203731.21506-1-alexander.deucher@amd.com>
Store the start wptr and ib size in the IB fence. On queue
reset, save the ring contents of all IBs.
For reemit, reemit the entire IB state for non-guilty contexts.
For guilty contexts, replace the IB submission with nops, but reemit
the rest. Split the reemit per fence and when we reemit, update the
wptr with the new values from reemit. This allows us to reemit jobs
repeatedly as the wptrs get properly updated each time.
v2: further simplify the logic
v3: reemit vm state, not just vm fence
v4: just nop the IB and possibly the VM portion of the submission
v5: simplify the vm fence check
v6: split the vm and ib fences
v7: fix commit message
v8: use wptr rather than count_dw to calculate offsets
Reviewed-by: Jesse Zhang <jesse.zhang@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 98 +++++++++--------------
drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 25 ++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 46 +++--------
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 20 ++---
4 files changed, 72 insertions(+), 117 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 3c38bf58fd3f2..941aa442fa133 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -89,16 +89,6 @@ static u32 amdgpu_fence_read(struct amdgpu_ring *ring)
return seq;
}
-static void amdgpu_fence_save_fence_wptr_start(struct amdgpu_fence *af)
-{
- af->fence_wptr_start = af->ring->wptr;
-}
-
-static void amdgpu_fence_save_fence_wptr_end(struct amdgpu_fence *af)
-{
- af->fence_wptr_end = af->ring->wptr;
-}
-
/**
* amdgpu_fence_emit - emit a fence on the requested ring
*
@@ -126,11 +116,9 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct amdgpu_fence *af,
&ring->fence_drv.lock,
adev->fence_context + ring->idx, seq);
- amdgpu_fence_save_fence_wptr_start(af);
amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
seq, flags | AMDGPU_FENCE_FLAG_INT);
- amdgpu_fence_save_fence_wptr_end(af);
- amdgpu_fence_save_wptr(af);
+
pm_runtime_get_noresume(adev_to_drm(adev)->dev);
ptr = &ring->fence_drv.fences[seq & ring->fence_drv.num_fences_mask];
if (unlikely(rcu_dereference_protected(*ptr, 1))) {
@@ -241,7 +229,6 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
do {
struct dma_fence *fence, **ptr;
- struct amdgpu_fence *am_fence;
++last_seq;
last_seq &= drv->num_fences_mask;
@@ -254,12 +241,6 @@ bool amdgpu_fence_process(struct amdgpu_ring *ring)
if (!fence)
continue;
- /* Save the wptr in the fence driver so we know what the last processed
- * wptr was. This is required for re-emitting the ring state for
- * queues that are reset but are not guilty and thus have no guilty fence.
- */
- am_fence = container_of(fence, struct amdgpu_fence, base);
- drv->signalled_wptr = am_fence->wptr;
dma_fence_signal(fence);
dma_fence_put(fence);
pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
@@ -726,25 +707,28 @@ void amdgpu_fence_driver_force_completion(struct amdgpu_ring *ring,
*/
/**
- * amdgpu_fence_driver_update_timedout_fence_state - Update fence state and set errors
+ * amdgpu_ring_set_fence_errors_and_reemit - Set dma_fence errors and reemit
*
- * @af: fence of the ring to update
+ * @guilty_fence: fence of the ring to update
*
*/
-void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
+void amdgpu_ring_set_fence_errors_and_reemit(struct amdgpu_ring *ring,
+ struct amdgpu_fence *guilty_fence)
{
struct dma_fence *unprocessed;
struct dma_fence __rcu **ptr;
struct amdgpu_fence *fence;
- struct amdgpu_ring *ring = af->ring;
unsigned long flags;
u32 seq, last_seq;
- bool reemitted = false;
+ unsigned int i;
+ bool is_guilty_fence;
+ bool is_guilty_context;
last_seq = amdgpu_fence_read(ring) & ring->fence_drv.num_fences_mask;
seq = ring->fence_drv.sync_seq & ring->fence_drv.num_fences_mask;
- /* mark all fences from the guilty context with an error */
+ ring->reemit = true;
+ amdgpu_ring_alloc(ring, ring->ring_backup_entries_to_copy);
spin_lock_irqsave(&ring->fence_drv.lock, flags);
do {
last_seq++;
@@ -756,39 +740,45 @@ void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af)
if (unprocessed && !dma_fence_is_signaled_locked(unprocessed)) {
fence = container_of(unprocessed, struct amdgpu_fence, base);
+ is_guilty_fence = fence == guilty_fence;
+ is_guilty_context = fence->context == guilty_fence->context;
- if (fence->reemitted > 1)
- reemitted = true;
- else if (fence == af)
+ /* mark all fences from the guilty context with an error */
+ if (is_guilty_fence)
dma_fence_set_error(&fence->base, -ETIME);
- else if (fence->context == af->context)
+ else if (is_guilty_context)
dma_fence_set_error(&fence->base, -ECANCELED);
+
+ /* reemit the packet stream and update wptrs */
+ fence->ib_wptr = ring->wptr;
+ for (i = 0; i < fence->ib_dw_size; i++) {
+ /* Skip the IB(s) for the guilty context. */
+ if (is_guilty_context &&
+ i >= fence->skip_ib_dw_start_offset &&
+ i < fence->skip_ib_dw_end_offset)
+ amdgpu_ring_write(ring, ring->funcs->nop);
+ else
+ amdgpu_ring_write(ring,
+ ring->ring_backup[fence->backup_idx + i]);
+ }
}
rcu_read_unlock();
} while (last_seq != seq);
spin_unlock_irqrestore(&ring->fence_drv.lock, flags);
-
- if (reemitted) {
- /* if we've already reemitted once then just cancel everything */
- amdgpu_fence_driver_force_completion(af->ring, &af->base);
- af->ring->ring_backup_entries_to_copy = 0;
- }
-}
-
-void amdgpu_fence_save_wptr(struct amdgpu_fence *af)
-{
- af->wptr = af->ring->wptr;
+ amdgpu_ring_commit(ring);
+ ring->reemit = false;
}
static void amdgpu_ring_backup_unprocessed_command(struct amdgpu_ring *ring,
- u64 start_wptr, u64 end_wptr)
+ struct amdgpu_fence *af)
{
- unsigned int first_idx = start_wptr & ring->buf_mask;
- unsigned int last_idx = end_wptr & ring->buf_mask;
+ unsigned int first_idx = af->ib_wptr & ring->buf_mask;
+ unsigned int dw_size = af->ib_dw_size;
unsigned int i;
+ af->backup_idx = ring->ring_backup_entries_to_copy;
/* Backup the contents of the ring buffer. */
- for (i = first_idx; i != last_idx; ++i, i &= ring->buf_mask)
+ for (i = first_idx; dw_size > 0; ++i, i &= ring->buf_mask, --dw_size)
ring->ring_backup[ring->ring_backup_entries_to_copy++] = ring->ring[i];
}
@@ -798,12 +788,10 @@ void amdgpu_ring_backup_unprocessed_commands(struct amdgpu_ring *ring,
struct dma_fence *unprocessed;
struct dma_fence __rcu **ptr;
struct amdgpu_fence *fence;
- u64 wptr;
u32 seq, last_seq;
last_seq = amdgpu_fence_read(ring) & ring->fence_drv.num_fences_mask;
seq = ring->fence_drv.sync_seq & ring->fence_drv.num_fences_mask;
- wptr = ring->fence_drv.signalled_wptr;
ring->ring_backup_entries_to_copy = 0;
do {
@@ -817,21 +805,7 @@ void amdgpu_ring_backup_unprocessed_commands(struct amdgpu_ring *ring,
if (unprocessed && !dma_fence_is_signaled(unprocessed)) {
fence = container_of(unprocessed, struct amdgpu_fence, base);
- /* save everything if the ring is not guilty, otherwise
- * just save the content from other contexts.
- */
- if (!fence->reemitted &&
- (!guilty_fence || (fence->context != guilty_fence->context))) {
- amdgpu_ring_backup_unprocessed_command(ring, wptr,
- fence->wptr);
- } else if (!fence->reemitted) {
- /* always save the fence */
- amdgpu_ring_backup_unprocessed_command(ring,
- fence->fence_wptr_start,
- fence->fence_wptr_end);
- }
- wptr = fence->wptr;
- fence->reemitted++;
+ amdgpu_ring_backup_unprocessed_command(ring, fence);
}
rcu_read_unlock();
} while (last_seq != seq);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 0932a093dee3d..f4d090bc4784c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -126,6 +126,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct amdgpu_job *job,
struct amdgpu_ib *ib;
struct dma_fence *tmp = NULL;
struct amdgpu_fence *af;
+ struct amdgpu_fence *vm_af;
bool need_ctx_switch;
uint64_t fence_ctx;
uint32_t status = 0, alloc_size;
@@ -146,13 +147,14 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct amdgpu_job *job,
fence_ctx = job->base.s_fence ?
job->base.s_fence->finished.context : 0;
af = job->hw_fence;
+ vm_af = job->hw_vm_fence;
/* Save the context of the job for reset handling.
* The driver needs this so it can skip the ring
* contents for guilty contexts.
*/
af->context = fence_ctx;
/* the vm fence is also part of the job's context */
- job->hw_vm_fence->context = fence_ctx;
+ vm_af->context = fence_ctx;
if (!ring->sched.ready) {
dev_err(adev->dev, "couldn't schedule ib on ring <%s>\n", ring->name);
@@ -192,12 +194,18 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct amdgpu_job *job,
dma_fence_put(tmp);
}
+ /* VM sequence */
+ vm_af->ib_wptr = ring->wptr;
r = amdgpu_vm_flush(ring, job, need_pipe_sync);
if (r) {
amdgpu_ring_undo(ring);
return r;
}
+ vm_af->ib_dw_size =
+ amdgpu_ring_get_dw_distance(ring, vm_af->ib_wptr, ring->wptr);
+ /* IB sequence */
+ af->ib_wptr = ring->wptr;
amdgpu_ring_ib_begin(ring);
if (ring->funcs->insert_start)
@@ -218,6 +226,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct amdgpu_job *job,
cond_exec = amdgpu_ring_init_cond_exec(ring,
ring->cond_exe_gpu_addr);
+ /* Skip the IB for guilty contexts */
+ af->skip_ib_dw_start_offset =
+ amdgpu_ring_get_dw_distance(ring, af->ib_wptr, ring->wptr);
amdgpu_device_flush_hdp(adev, ring);
if (need_ctx_switch)
@@ -256,6 +267,9 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct amdgpu_job *job,
amdgpu_ring_emit_frame_cntl(ring, false, secure);
amdgpu_device_invalidate_hdp(adev, ring);
+ /* Skip the IB for guilty contexts */
+ af->skip_ib_dw_end_offset =
+ amdgpu_ring_get_dw_distance(ring, af->ib_wptr, ring->wptr);
if (ib->flags & AMDGPU_IB_FLAG_TC_WB_NOT_INVALIDATE)
fence_flags |= AMDGPU_FENCE_FLAG_TC_WB_ONLY;
@@ -298,13 +312,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, struct amdgpu_job *job,
ring->funcs->emit_wave_limit(ring, false);
amdgpu_ring_ib_end(ring);
- /* Save the wptr associated with this fence.
- * This must be last for resets to work properly
- * as we need to save the wptr associated with this
- * fence so we know what rings contents to backup
- * after we reset the queue.
- */
- amdgpu_fence_save_wptr(af);
+
+ af->ib_dw_size = amdgpu_ring_get_dw_distance(ring, af->ib_wptr, ring->wptr);
amdgpu_ring_commit(ring);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 129ad51386535..83750ab4e81b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -90,10 +90,13 @@ int amdgpu_ring_alloc(struct amdgpu_ring *ring, unsigned int ndw)
ndw = (ndw + ring->funcs->align_mask) & ~ring->funcs->align_mask;
/* Make sure we aren't trying to allocate more space
- * than the maximum for one submission
+ * than the maximum for one submission. Skip for reemit
+ * since we may be reemitting several submissions.
*/
- if (WARN_ON_ONCE(ndw > ring->max_dw))
- return -ENOMEM;
+ if (!ring->reemit) {
+ if (WARN_ON_ONCE(ndw > ring->max_dw))
+ return -ENOMEM;
+ }
ring->count_dw = ndw;
ring->wptr_old = ring->wptr;
@@ -104,29 +107,6 @@ int amdgpu_ring_alloc(struct amdgpu_ring *ring, unsigned int ndw)
return 0;
}
-/**
- * amdgpu_ring_alloc_reemit - allocate space on the ring buffer for reemit
- *
- * @ring: amdgpu_ring structure holding ring information
- * @ndw: number of dwords to allocate in the ring buffer
- *
- * Allocate @ndw dwords in the ring buffer (all asics).
- * doesn't check the max_dw limit as we may be reemitting
- * several submissions.
- */
-static void amdgpu_ring_alloc_reemit(struct amdgpu_ring *ring, unsigned int ndw)
-{
- /* Align requested size with padding so unlock_commit can
- * pad safely */
- ndw = (ndw + ring->funcs->align_mask) & ~ring->funcs->align_mask;
-
- ring->count_dw = ndw;
- ring->wptr_old = ring->wptr;
-
- if (ring->funcs->begin_use)
- ring->funcs->begin_use(ring);
-}
-
/**
* amdgpu_ring_insert_nop - insert NOP packets
*
@@ -875,7 +855,6 @@ void amdgpu_ring_reset_helper_begin(struct amdgpu_ring *ring,
int amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring,
struct amdgpu_fence *guilty_fence)
{
- unsigned int i;
int r;
/* verify that the ring is functional */
@@ -883,16 +862,9 @@ int amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring,
if (r)
return r;
- /* set an error on all fences from the context */
- if (guilty_fence)
- amdgpu_fence_driver_update_timedout_fence_state(guilty_fence);
- /* Re-emit the non-guilty commands */
- if (ring->ring_backup_entries_to_copy) {
- amdgpu_ring_alloc_reemit(ring, ring->ring_backup_entries_to_copy);
- for (i = 0; i < ring->ring_backup_entries_to_copy; i++)
- amdgpu_ring_write(ring, ring->ring_backup[i]);
- amdgpu_ring_commit(ring);
- }
+ /* set an error on all fences from the context and reemit */
+ amdgpu_ring_set_fence_errors_and_reemit(ring, guilty_fence);
+
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index e196ffbdd1f3a..6da7eb49d30a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -121,7 +121,6 @@ struct amdgpu_fence_driver {
/* sync_seq is protected by ring emission lock */
uint32_t sync_seq;
atomic_t last_seq;
- u64 signalled_wptr;
bool initialized;
struct amdgpu_irq_src *irq_src;
unsigned irq_type;
@@ -146,15 +145,15 @@ struct amdgpu_fence {
struct amdgpu_ring *ring;
ktime_t start_timestamp;
- /* wptr for the total submission for resets */
- u64 wptr;
+ /* location and size of the IB */
+ u64 ib_wptr;
+ unsigned int ib_dw_size;
+ unsigned int skip_ib_dw_start_offset;
+ unsigned int skip_ib_dw_end_offset;
/* fence context for resets */
u64 context;
- /* has this fence been reemitted */
- unsigned int reemitted;
- /* wptr for the fence for the submission */
- u64 fence_wptr_start;
- u64 fence_wptr_end;
+ /* idx for ring backups */
+ unsigned int backup_idx;
};
extern const struct drm_sched_backend_ops amdgpu_sched_ops;
@@ -162,8 +161,8 @@ extern const struct drm_sched_backend_ops amdgpu_sched_ops;
void amdgpu_fence_driver_set_error(struct amdgpu_ring *ring, int error);
void amdgpu_fence_driver_force_completion(struct amdgpu_ring *ring,
struct dma_fence *timedout_fence);
-void amdgpu_fence_driver_update_timedout_fence_state(struct amdgpu_fence *af);
-void amdgpu_fence_save_wptr(struct amdgpu_fence *af);
+void amdgpu_ring_set_fence_errors_and_reemit(struct amdgpu_ring *ring,
+ struct amdgpu_fence *guilty_fence);
int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring);
int amdgpu_fence_driver_start_ring(struct amdgpu_ring *ring,
@@ -314,6 +313,7 @@ struct amdgpu_ring {
/* backups for resets */
uint32_t *ring_backup;
unsigned int ring_backup_entries_to_copy;
+ bool reemit;
unsigned rptr_offs;
u64 rptr_gpu_addr;
u32 *rptr_cpu_addr;
--
2.52.0
next prev parent reply other threads:[~2026-01-29 20:38 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-29 20:37 [PATCH 00/12] Improvements for IB handling V7 Alex Deucher
2026-01-29 20:37 ` [PATCH 01/12] drm/amdgpu: re-add the bad job to the pending list for ring resets Alex Deucher
2026-01-30 15:30 ` Christian König
2026-01-30 15:43 ` Philipp Stanner
2026-01-30 16:25 ` Alex Deucher
2026-01-29 20:37 ` [PATCH 02/12] drm/amdgpu/job: use GFP_ATOMIC while in gpu reset Alex Deucher
2026-01-29 20:37 ` [PATCH 03/12] drm/amdgpu: switch all IPs to using job for IBs Alex Deucher
2026-01-29 20:37 ` [PATCH 04/12] drm/amdgpu: require a job to schedule an IB Alex Deucher
2026-01-29 20:37 ` [PATCH 05/12] drm/amdgpu: don't call drm_sched_stop/start() in asic reset Alex Deucher
2026-01-29 20:37 ` [PATCH 06/12] drm/amdgpu/cs: return -ETIME for guilty contexts Alex Deucher
2026-01-29 20:37 ` [PATCH 07/12] drm/amdgpu: plumb timedout fence through to force completion Alex Deucher
2026-01-29 20:37 ` [PATCH 08/12] drm/amdgpu: simplify VCN reset helper Alex Deucher
2026-01-29 20:37 ` [PATCH 09/12] drm/amdgpu: Call drm_sched_increase_karma() for ring resets Alex Deucher
2026-01-29 20:37 ` [PATCH 10/12] drm/amdgpu: reorder IB schedule sequence Alex Deucher
2026-01-29 20:37 ` [PATCH 11/12] drm/amdgpu: add a helper to calculate ring distance Alex Deucher
2026-01-29 20:37 ` Alex Deucher [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-01-30 17:30 [PATCH 00/12] Improvements for IB handling V8 Alex Deucher
2026-01-30 17:30 ` [PATCH 12/12] drm/amdgpu: rework ring reset backup and reemit v8 Alex Deucher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260129203731.21506-13-alexander.deucher@amd.com \
--to=alexander.deucher@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=christian.koenig@amd.com \
--cc=jesse.zhang@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox