* [PATCH 1/2] drm/imagination: Switch the GPUVM to immediate mode
2026-10-10 17:22 [PATCH 0/2] drm/imagination: Switch the GPUVM to immediate mode Gyeyoung Baek
@ 2026-10-10 17:22 ` Gyeyoung Baek
2026-10-10 17:22 ` [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap Gyeyoung Baek
1 sibling, 0 replies; 3+ messages in thread
From: Gyeyoung Baek @ 2026-10-10 17:22 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Alexandru Dadu, Brajesh Gupta, Frank Binns, Danilo Krummrich,
Alice Ryhl, imagination, dri-devel, linux-kernel, Gyeyoung Baek
Use DRM_GPUVM_IMMEDIATE_MODE so that each GEM's GPUVA list is protected
by its gpuva.lock instead of its dma_resv. This prepares for
asynchronous VM_BIND, whose run_job() will update the GPUVA lists
before signalling its fence. That section imposes three rules:
1. No dma_resv: another thread may hold it while waiting for the job's
fence, directly or through a dependency.
-> Protect the lists with gpuva.lock, and take it around
drm_gpuva_link().
2. No allocation under a lock run_job() takes: reclaim may wait for a
fence that depends on the job.
-> Allocate the vm_bo before taking gpuva.lock:
drm_gpuvm_bo_obtain_locked() becomes drm_gpuvm_bo_create() +
drm_gpuvm_bo_obtain_prealloc().
3. No freeing of GEM objects: that takes their dma_resv, and under
gpuva.lock would free the held lock too.
-> Defer it: drm_gpuva_unlink() and drm_gpuvm_bo_put() become
drm_gpuva_unlink_defer() and drm_gpuvm_bo_put_deferred(), and
drm_gpuvm_bo_deferred_cleanup() frees once the locks are dropped.
This also fixes an unlocked GPUVA list update: when a map fails,
pvr_vm_bind_op_fini() drops the last vm_bo reference without holding
the object's dma_resv.
Failing VM_MAP allocations with fail-nth shows
it on a lockdep kernel (real allocation failures under memory pressure
take the same path):
WARNING: drivers/gpu/drm/drm_gpuvm.c:1661 at drm_gpuvm_bo_put+0x2ec/0x660 [drm_gpuvm]
Call trace:
drm_gpuvm_bo_put
pvr_vm_bind_op_fini
pvr_vm_map
pvr_ioctl_vm_map
Fixes: ff5f643de0bf ("drm/imagination: Add GEM and VM related code")
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
drivers/gpu/drm/imagination/pvr_vm.c | 37 +++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index cbdd15ed74f..3921b765688 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -214,7 +214,7 @@ static int pvr_vm_bind_op_exec(struct pvr_vm_bind_op *bind_op)
static void pvr_vm_bind_op_fini(struct pvr_vm_bind_op *bind_op)
{
- drm_gpuvm_bo_put(bind_op->gpuvm_bo);
+ drm_gpuvm_bo_put_deferred(bind_op->gpuvm_bo);
kfree(bind_op->new_va);
kfree(bind_op->prev_va);
@@ -255,11 +255,11 @@ pvr_vm_bind_op_map_init(struct pvr_vm_bind_op *bind_op,
bind_op->type = PVR_VM_BIND_TYPE_MAP;
- dma_resv_lock(obj->resv, NULL);
- bind_op->gpuvm_bo = drm_gpuvm_bo_obtain_locked(&vm_ctx->gpuvm_mgr, obj);
- dma_resv_unlock(obj->resv);
- if (IS_ERR(bind_op->gpuvm_bo))
- return PTR_ERR(bind_op->gpuvm_bo);
+ bind_op->gpuvm_bo = drm_gpuvm_bo_create(&vm_ctx->gpuvm_mgr, obj);
+ if (!bind_op->gpuvm_bo)
+ return -ENOMEM;
+
+ bind_op->gpuvm_bo = drm_gpuvm_bo_obtain_prealloc(bind_op->gpuvm_bo);
bind_op->new_va = kzalloc_obj(*bind_op->new_va);
bind_op->prev_va = kzalloc_obj(*bind_op->prev_va);
@@ -366,7 +366,11 @@ pvr_vm_gpuva_map(struct drm_gpuva_op *op, void *op_ctx)
return err;
drm_gpuva_map(&ctx->vm_ctx->gpuvm_mgr, &ctx->new_va->base, &op->map);
+
+ mutex_lock(&op->map.gem.obj->gpuva.lock);
drm_gpuva_link(&ctx->new_va->base, ctx->gpuvm_bo);
+ mutex_unlock(&op->map.gem.obj->gpuva.lock);
+
ctx->new_va = NULL;
return 0;
@@ -396,7 +400,7 @@ pvr_vm_gpuva_unmap(struct drm_gpuva_op *op, void *op_ctx)
return err;
drm_gpuva_unmap(&op->unmap);
- drm_gpuva_unlink(op->unmap.va);
+ drm_gpuva_unlink_defer(op->unmap.va);
kfree(to_pvr_vm_gpuva(op->unmap.va));
return 0;
@@ -418,6 +422,7 @@ pvr_vm_gpuva_unmap(struct drm_gpuva_op *op, void *op_ctx)
static int
pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
{
+ struct drm_gem_object *obj = op->remap.unmap->va->gem.obj;
/* The split parts belong to the object of the mapping being split. */
struct drm_gpuvm_bo *vm_bo = op->remap.unmap->va->vm_bo;
struct pvr_vm_bind_op *ctx = op_ctx;
@@ -435,16 +440,20 @@ pvr_vm_gpuva_remap(struct drm_gpuva_op *op, void *op_ctx)
drm_gpuva_remap(&ctx->prev_va->base, &ctx->next_va->base, &op->remap);
if (op->remap.prev) {
+ mutex_lock(&obj->gpuva.lock);
drm_gpuva_link(&ctx->prev_va->base, vm_bo);
+ mutex_unlock(&obj->gpuva.lock);
ctx->prev_va = NULL;
}
if (op->remap.next) {
+ mutex_lock(&obj->gpuva.lock);
drm_gpuva_link(&ctx->next_va->base, vm_bo);
+ mutex_unlock(&obj->gpuva.lock);
ctx->next_va = NULL;
}
- drm_gpuva_unlink(op->remap.unmap->va);
+ drm_gpuva_unlink_defer(op->remap.unmap->va);
kfree(to_pvr_vm_gpuva(op->remap.unmap->va));
return 0;
@@ -588,7 +597,8 @@ pvr_vm_create_context(struct pvr_device *pvr_dev, bool is_userspace_context)
drm_gem_private_object_init(&pvr_dev->base, &vm_ctx->dummy_gem, 0);
drm_gpuvm_init(&vm_ctx->gpuvm_mgr,
is_userspace_context ? "PowerVR-user-VM" : "PowerVR-FW-VM",
- 0, &pvr_dev->base, &vm_ctx->dummy_gem,
+ DRM_GPUVM_IMMEDIATE_MODE, &pvr_dev->base,
+ &vm_ctx->dummy_gem,
0, 1ULL << device_addr_bits, 0, 0, &pvr_vm_gpuva_ops);
mutex_init(&vm_ctx->lock);
@@ -622,6 +632,7 @@ pvr_vm_context_release(struct kref *ref_count)
pvr_fw_object_destroy(vm_ctx->fw_mem_ctx_obj);
pvr_vm_unmap_all(vm_ctx);
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
pvr_mmu_context_destroy(vm_ctx->mmu_ctx);
drm_gem_private_object_fini(&vm_ctx->dummy_gem);
@@ -758,7 +769,9 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
err_cleanup:
mutex_unlock(&vm_ctx->lock);
+
pvr_vm_bind_op_fini(&bind_op);
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
return err;
}
@@ -840,6 +853,8 @@ pvr_vm_unmap_obj(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
err = pvr_vm_unmap_obj_locked(vm_ctx, pvr_obj, device_addr, size);
mutex_unlock(&vm_ctx->lock);
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
+
return err;
}
@@ -874,6 +889,8 @@ pvr_vm_unmap(struct pvr_vm_context *vm_ctx, u64 device_addr, u64 size)
mutex_unlock(&vm_ctx->lock);
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
+
return err;
}
@@ -906,6 +923,8 @@ pvr_vm_unmap_all(struct pvr_vm_context *vm_ctx)
}
mutex_unlock(&vm_ctx->lock);
+
+ drm_gpuvm_bo_deferred_cleanup(&vm_ctx->gpuvm_mgr);
}
/* Static data areas are determined by firmware. */
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 2/2] drm/imagination: Stop locking reservations around map and unmap
2026-10-10 17:22 [PATCH 0/2] drm/imagination: Switch the GPUVM to immediate mode Gyeyoung Baek
2026-10-10 17:22 ` [PATCH 1/2] " Gyeyoung Baek
@ 2026-10-10 17:22 ` Gyeyoung Baek
1 sibling, 0 replies; 3+ messages in thread
From: Gyeyoung Baek @ 2026-10-10 17:22 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: Alexandru Dadu, Brajesh Gupta, Frank Binns, Danilo Krummrich,
Alice Ryhl, imagination, dri-devel, linux-kernel, Gyeyoung Baek
The GPUVA lists are now protected by gpuva.lock instead of the
reservations, so map and unmap no longer need to lock the reservations.
Nothing else on that path relies on them.
Signed-off-by: Gyeyoung Baek <gye976@gmail.com>
---
drivers/gpu/drm/imagination/pvr_vm.c | 43 ------------------------------------
1 file changed, 43 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index 3921b765688..2f1bbcfdb60 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -702,16 +702,6 @@ void pvr_destroy_vm_contexts_for_file(struct pvr_file *pvr_file)
}
}
-static int
-pvr_vm_lock_extra(struct drm_gpuvm_exec *vm_exec)
-{
- struct pvr_vm_bind_op *bind_op = vm_exec->extra.priv;
- struct pvr_gem_object *pvr_obj = bind_op->pvr_obj;
-
- /* Acquire lock on the GEM object being mapped/unmapped. */
- return drm_exec_lock_obj(&vm_exec->exec, gem_from_pvr_gem(pvr_obj));
-}
-
/**
* pvr_vm_map() - Map a section of physical memory into a section of
* device-virtual memory.
@@ -739,15 +729,6 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
u64 pvr_obj_offset, u64 device_addr, u64 size)
{
struct pvr_vm_bind_op bind_op = {0};
- struct drm_gpuvm_exec vm_exec = {
- .vm = &vm_ctx->gpuvm_mgr,
- .flags = DRM_EXEC_INTERRUPTIBLE_WAIT |
- DRM_EXEC_IGNORE_DUPLICATES,
- .extra = {
- .fn = pvr_vm_lock_extra,
- .priv = &bind_op,
- },
- };
int err = pvr_vm_bind_op_map_init(&bind_op, vm_ctx, pvr_obj,
pvr_obj_offset, device_addr,
@@ -759,15 +740,7 @@ pvr_vm_map(struct pvr_vm_context *vm_ctx, struct pvr_gem_object *pvr_obj,
pvr_gem_object_get(pvr_obj);
mutex_lock(&vm_ctx->lock);
- err = drm_gpuvm_exec_lock(&vm_exec);
- if (err)
- goto err_cleanup;
-
err = pvr_vm_bind_op_exec(&bind_op);
-
- drm_gpuvm_exec_unlock(&vm_exec);
-
-err_cleanup:
mutex_unlock(&vm_ctx->lock);
pvr_vm_bind_op_fini(&bind_op);
@@ -800,15 +773,6 @@ pvr_vm_unmap_obj_locked(struct pvr_vm_context *vm_ctx,
u64 device_addr, u64 size)
{
struct pvr_vm_bind_op bind_op = {0};
- struct drm_gpuvm_exec vm_exec = {
- .vm = &vm_ctx->gpuvm_mgr,
- .flags = DRM_EXEC_INTERRUPTIBLE_WAIT |
- DRM_EXEC_IGNORE_DUPLICATES,
- .extra = {
- .fn = pvr_vm_lock_extra,
- .priv = &bind_op,
- },
- };
int err = pvr_vm_bind_op_unmap_init(&bind_op, vm_ctx, pvr_obj,
device_addr, size);
@@ -817,15 +781,8 @@ pvr_vm_unmap_obj_locked(struct pvr_vm_context *vm_ctx,
pvr_gem_object_get(pvr_obj);
- err = drm_gpuvm_exec_lock(&vm_exec);
- if (err)
- goto err_cleanup;
-
err = pvr_vm_bind_op_exec(&bind_op);
- drm_gpuvm_exec_unlock(&vm_exec);
-
-err_cleanup:
pvr_vm_bind_op_fini(&bind_op);
return err;
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread