From: "David (Ming Qiang) Wu" <David.Wu3@amd.com>
To: <amd-gfx@lists.freedesktop.org>
Cc: <Christian.Koenig@amd.com>
Subject: [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes
Date: Tue, 4 Aug 2026 11:26:35 -0400 [thread overview]
Message-ID: <20260804152635.899680-1-David.Wu3@amd.com> (raw)
On hardware where EFC (Encode Frame Copy) is unavailable or the source
frame format requires conversion, the VA-API frontend falls back to a
GFX/compute shader blit (vl_compositor) to perform color space conversion
before VCN encode. That blit goes through the GFX/compute ring and bumps
the VM's shared tlb_seq. Every subsequent VCN encode job then finds
flushed_updates < tlb_seq and forces a full MMHUB TLB flush, even though
no MMHUB-relevant mappings changed. On multi-instance VCN hardware the
same issue is compounded by the dedicated PIPE_CONTEXT_COMPUTE_ONLY
scheduling context Mesa creates.
Fix this by splitting the single tlb_seq into two counters:
tlb_seq - bumped only when mappings accessed by GFXHUB change
tlb_seq_mm - bumped only when mappings accessed by MMHUB change
To know which hubs are relevant for a given PT update, add a vmhub_mask
parameter to amdgpu_vm_update_range(). The mask is derived from the set
of rings in each command submission (collected in amdgpu_cs.c and stored
in bo_va->vmhub_mask / mapping->vmhub_mask). When vmhub_mask is 0 (BO
has never been submitted), both counters are bumped conservatively.
PDE updates and XGMI/GFX8 legacy paths bump both counters since they
affect all hubs. The fault handler uses GENMASK(AMDGPU_MAX_VMHUBS-1,0)
because the faulting hub is unknown at that point.
MMHUB VMID allocation (amdgpu_ids.c) now compares flushed_updates against
tlb_seq_mm instead of tlb_seq, so VCN VMIDs no longer force a TLB flush
when only GFX/compute page tables changed.
Signed-off-by: David (Ming Qiang) Wu <David.Wu3@amd.com>
Assisted-by: Claude:Claude-Sonnet-4-6
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 37 ++++++++----
drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 9 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 5 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 65 +++++++++++++++++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 32 ++++++++++-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 5 +-
6 files changed, 129 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index afc1c631d55a..33c1e9341774 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1136,19 +1136,34 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
* with p->ticket. But removing it caused test regressions, so I'm
* leaving it here for now.
*/
- amdgpu_bo_list_for_each_entry(e, p->bo_list) {
- bo_va = e->bo_va;
- if (bo_va == NULL)
- continue;
+ /* Collect vmhub bitmask for all rings in this submission */
+ {
+ uint16_t sub_vmhub_mask = 0;
- r = amdgpu_vm_bo_update(adev, bo_va, false);
- if (r)
- return r;
+ for (i = 0; i < p->gang_size; ++i) {
+ struct amdgpu_ring *ring = amdgpu_job_ring(p->jobs[i]);
- r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
- GFP_KERNEL);
- if (r)
- return r;
+ if (ring)
+ sub_vmhub_mask |= BIT(ring->vm_hub);
+ }
+
+ amdgpu_bo_list_for_each_entry(e, p->bo_list) {
+ bo_va = e->bo_va;
+ if (bo_va == NULL)
+ continue;
+
+ /* Track which hubs have accessed this BO for per-hub tlb_seq */
+ bo_va->vmhub_mask |= sub_vmhub_mask;
+
+ r = amdgpu_vm_bo_update(adev, bo_va, false);
+ if (r)
+ return r;
+
+ r = amdgpu_sync_fence(&p->sync, bo_va->last_pt_update,
+ GFP_KERNEL);
+ if (r)
+ return r;
+ }
}
r = amdgpu_vm_handle_moved(adev, vm, &p->exec.ticket);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index 8a2d64f0ebc3..9183569d340a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -331,7 +331,8 @@ static int amdgpu_vmid_grab_reserved(struct amdgpu_vm *vm,
unsigned vmhub = ring->vm_hub;
uint64_t fence_context = adev->fence_context + ring->idx;
bool needs_flush = vm->use_cpu_for_update;
- uint64_t updates = amdgpu_vm_tlb_seq(vm);
+ uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
+ amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
int r;
*id = vm->reserved_vmid[vmhub];
@@ -395,7 +396,8 @@ static int amdgpu_vmid_grab_used(struct amdgpu_vm *vm,
unsigned vmhub = ring->vm_hub;
struct amdgpu_vmid_mgr *id_mgr = &adev->vm_manager.id_mgr[vmhub];
uint64_t fence_context = adev->fence_context + ring->idx;
- uint64_t updates = amdgpu_vm_tlb_seq(vm);
+ uint64_t updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
+ amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
int r;
job->vm_needs_flush = vm->use_cpu_for_update;
@@ -492,7 +494,8 @@ int amdgpu_vmid_grab(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
job->gds_switch_needed = amdgpu_vmid_gds_switch_needed(id, job);
if (job->vm_needs_flush) {
- id->flushed_updates = amdgpu_vm_tlb_seq(vm);
+ id->flushed_updates = AMDGPU_IS_GFXHUB(ring->vm_hub) ?
+ amdgpu_vm_tlb_seq(vm) : amdgpu_vm_tlb_seq_mm(vm);
dma_fence_put(id->last_flush);
id->last_flush = NULL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index ff11a0903499..1d6373f1d985 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -70,6 +70,8 @@ struct amdgpu_bo_va_mapping {
uint64_t __subtree_last;
uint64_t offset;
uint32_t flags;
+ /* bitmask of VMHUB indices whose TLBs have cached this mapping's PTEs */
+ uint16_t vmhub_mask;
};
/* User space allocated BO in a VM */
@@ -91,6 +93,9 @@ struct amdgpu_bo_va {
bool is_xgmi;
+ /* bitmask of VMHUB indices that have ever submitted jobs referencing this BO */
+ uint16_t vmhub_mask;
+
/*
* protected by vm reservation lock
* if non-zero, cannot unmap from GPU because user queues may still access it
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index aac8ace9d7a6..b05befa1d5c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -121,6 +121,11 @@ struct amdgpu_vm_tlb_seq_struct {
*/
struct amdgpu_vm *vm;
+ /**
+ * @vmhub_mask: which hub counters to increment (bitmask of VMHUB indices)
+ */
+ uint16_t vmhub_mask;
+
/**
* @cb: callback
*/
@@ -1008,8 +1013,10 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
if (r)
goto error;
- if (flush_tlb_needed)
+ if (flush_tlb_needed) {
atomic64_inc(&vm->tlb_seq);
+ atomic64_inc(&vm->tlb_seq_mm);
+ }
list_for_each_entry_safe(entry, tmp, &vm->kernel.needs_update,
vm_status)
@@ -1031,9 +1038,25 @@ static void amdgpu_vm_tlb_seq_cb(struct dma_fence *fence,
struct dma_fence_cb *cb)
{
struct amdgpu_vm_tlb_seq_struct *tlb_cb;
+ bool any_gfx, any_mm;
+ unsigned int i;
tlb_cb = container_of(cb, typeof(*tlb_cb), cb);
- atomic64_inc(&tlb_cb->vm->tlb_seq);
+
+ any_gfx = any_mm = false;
+ for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+ if (!(tlb_cb->vmhub_mask & BIT(i)))
+ continue;
+ if (AMDGPU_IS_GFXHUB(i))
+ any_gfx = true;
+ else
+ any_mm = true;
+ }
+ if (any_gfx)
+ atomic64_inc(&tlb_cb->vm->tlb_seq);
+ if (any_mm)
+ atomic64_inc(&tlb_cb->vm->tlb_seq_mm);
+
kfree(tlb_cb);
}
@@ -1054,6 +1077,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
struct amdgpu_vm *vm = params->vm;
tlb_cb->vm = vm;
+ tlb_cb->vmhub_mask = params->vmhub_mask;
if (!fence || !*fence) {
amdgpu_vm_tlb_seq_cb(NULL, &tlb_cb->cb);
return;
@@ -1106,7 +1130,8 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
*/
int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
bool immediate, bool unlocked, bool flush_tlb,
- bool allow_override, struct amdgpu_sync *sync,
+ bool allow_override, uint16_t vmhub_mask,
+ struct amdgpu_sync *sync,
uint64_t start, uint64_t last, uint64_t flags,
uint64_t offset, uint64_t vram_base,
struct ttm_resource *res, dma_addr_t *pages_addr,
@@ -1129,13 +1154,20 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
/* Vega20+XGMI where PTEs get inadvertently cached in L2 texture cache,
* heavy-weight flush TLB unconditionally.
*/
- flush_tlb |= adev->gmc.xgmi.num_physical_nodes &&
- amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0);
+ if (adev->gmc.xgmi.num_physical_nodes &&
+ amdgpu_ip_version(adev, GC_HWIP, 0) == IP_VERSION(9, 4, 0)) {
+ flush_tlb = true;
+ /* XGMI: all hubs need flushing */
+ vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
+ }
/*
* On GFX8 and older any 8 PTE block with a valid bit set enters the TLB
*/
- flush_tlb |= amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0);
+ if (amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0)) {
+ flush_tlb = true;
+ vmhub_mask = GENMASK(AMDGPU_MAX_VMHUBS - 1, 0);
+ }
memset(¶ms, 0, sizeof(params));
params.adev = adev;
@@ -1144,6 +1176,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
params.pages_addr = pages_addr;
params.unlocked = unlocked;
params.needs_flush = flush_tlb;
+ params.vmhub_mask = vmhub_mask;
params.override_pte = allow_override && adev->gmc.override_pte;
INIT_LIST_HEAD(¶ms.tlb_flush_waitlist);
@@ -1366,9 +1399,18 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
trace_amdgpu_vm_bo_update(mapping);
+ /*
+ * If vmhub_mask is 0, no ring has submitted a job with this BO
+ * yet, so we don't know which hubs may have cached its PTEs.
+ * Be conservative and bump all counters.
+ */
r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb,
- !uncached, &sync, mapping->start,
- mapping->last, update_flags,
+ !uncached,
+ bo_va->vmhub_mask ?
+ bo_va->vmhub_mask :
+ GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
+ &sync,
+ mapping->start, mapping->last, update_flags,
mapping->offset, vram_base, mem,
pages_addr, last_update);
if (r)
@@ -1572,6 +1614,9 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
list_del(&mapping->list);
r = amdgpu_vm_update_range(adev, vm, false, false, true, false,
+ mapping->vmhub_mask ?
+ mapping->vmhub_mask :
+ GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
&sync, mapping->start, mapping->last,
0, 0, 0, NULL, NULL, &f);
amdgpu_vm_free_mapping(adev, vm, mapping, f);
@@ -1989,6 +2034,8 @@ int amdgpu_vm_bo_unmap(struct amdgpu_device *adev,
list_del(&mapping->list);
amdgpu_vm_it_remove(mapping, &vm->va);
+ /* Copy hub mask before clearing bo_va pointer (freed list loses the link) */
+ mapping->vmhub_mask = bo_va->vmhub_mask;
mapping->bo_va = NULL;
trace_amdgpu_vm_bo_unmap(bo_va, mapping);
@@ -2212,6 +2259,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
list_del(&mapping->list);
amdgpu_vm_it_remove(mapping, &vm->va);
+ mapping->vmhub_mask = bo_va->vmhub_mask;
mapping->bo_va = NULL;
trace_amdgpu_vm_bo_unmap(bo_va, mapping);
list_add(&mapping->list, &vm->freed);
@@ -3048,6 +3096,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
}
r = amdgpu_vm_update_range(adev, vm, true, false, false, false,
+ GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
NULL, addr, addr, flags, value, 0, NULL, NULL, NULL);
if (r)
goto error_unlock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 2f8234560764..deeb43a7ef86 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -313,6 +313,11 @@ struct amdgpu_vm_update_params {
*/
bool needs_flush;
+ /**
+ * @vmhub_mask: bitmask of VMHUB indices whose tlb_seq counters to bump
+ */
+ uint16_t vmhub_mask;
+
/**
* @override_pte: true for memory that is not uncached and gmc override function is
* implemented to allow MTYPE to be overridden for NUMA local memory.
@@ -403,6 +408,8 @@ struct amdgpu_vm {
/* Last finished delayed update */
atomic64_t tlb_seq;
+ /* Separate TLB flush sequence for MMHUB rings (VCN/JPEG/VPE) */
+ atomic64_t tlb_seq_mm;
struct dma_fence *last_tlb_flush;
atomic64_t kfd_last_flushed_seq;
uint64_t tlb_fence_context;
@@ -528,7 +535,8 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
struct amdgpu_vm *vm, struct amdgpu_bo *bo);
int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
bool immediate, bool unlocked, bool flush_tlb,
- bool allow_override, struct amdgpu_sync *sync,
+ bool allow_override, uint16_t vmhub_mask,
+ struct amdgpu_sync *sync,
uint64_t start, uint64_t last, uint64_t flags,
uint64_t offset, uint64_t vram_base,
struct ttm_resource *res, dma_addr_t *pages_addr,
@@ -649,6 +657,28 @@ static inline uint64_t amdgpu_vm_tlb_seq(struct amdgpu_vm *vm)
return atomic64_read(&vm->tlb_seq);
}
+/**
+ * amdgpu_vm_tlb_seq_mm - return MMHUB-specific tlb flush sequence number
+ * @vm: the amdgpu_vm structure to query
+ *
+ * Returns the tlb flush sequence number for MMHUB rings (VCN/JPEG/VPE).
+ * Only bumped when mappings accessed by MMHUB engines change.
+ */
+static inline uint64_t amdgpu_vm_tlb_seq_mm(struct amdgpu_vm *vm)
+{
+ unsigned long flags;
+ spinlock_t *lock;
+
+ rcu_read_lock();
+ lock = dma_fence_spinlock(vm->last_tlb_flush);
+ rcu_read_unlock();
+
+ spin_lock_irqsave(lock, flags);
+ spin_unlock_irqrestore(lock, flags);
+
+ return atomic64_read(&vm->tlb_seq_mm);
+}
+
/*
* vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
* happens while holding this lock anywhere to prevent deadlocks when
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 30ad10bbd47e..fb888f200f70 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1372,7 +1372,9 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
return -EINVAL;
}
- return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL, gpu_start,
+ return amdgpu_vm_update_range(adev, vm, false, true, true, false,
+ GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
+ NULL, gpu_start,
gpu_end, init_pte_value, 0, 0, NULL, NULL,
fence);
}
@@ -1490,6 +1492,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
pte_flags);
r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb, true,
+ GENMASK(AMDGPU_MAX_VMHUBS - 1, 0),
NULL, gpu_start, gpu_end,
pte_flags,
(last_start - prange->start) << PAGE_SHIFT,
--
2.43.0
next reply other threads:[~2026-08-04 15:28 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 15:26 David (Ming Qiang) Wu [this message]
2026-08-05 14:58 ` [PATCH] drm/amdgpu: split tlb_seq into per-VMHUB counters to avoid spurious MMHUB flushes Christian König
2026-08-05 18:21 ` David Wu
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=20260804152635.899680-1-David.Wu3@amd.com \
--to=david.wu3@amd.com \
--cc=Christian.Koenig@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.