[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250814-gpuva-mutex-in-gem-v1-1-e202cbfe6d77@google.com>
Date: Thu, 14 Aug 2025 13:53:14 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Danilo Krummrich <dakr@...nel.org>, Boris Brezillon <boris.brezillon@...labora.com>,
Daniel Almeida <daniel.almeida@...labora.com>, Steven Price <steven.price@....com>,
Liviu Dudau <liviu.dudau@....com>, Rob Clark <robin.clark@....qualcomm.com>,
Rob Herring <robh@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org, Alice Ryhl <aliceryhl@...gle.com>
Subject: [PATCH 1/2] drm_gem: add mutex to drm_gem_object.gpuva
There are two main ways that GPUVM might be used:
* staged mode, where VM_BIND ioctls update the GPUVM immediately so that
the GPUVM reflects the state of the VM *including* staged changes that
are not yet applied to the GPU's virtual address space.
* immediate mode, where the GPUVM state is updated during run_job(),
i.e., in the DMA fence signalling critical path, to ensure that the
GPUVM and the GPU's virtual address space has the same state at all
times.
Currently, only Panthor uses GPUVM in immediate mode, but the Rust
drivers Tyr and Nova will also use GPUVM in immediate mode, so it is
worth to support both staged and immediate mode well in GPUVM. To use
immediate mode, the GEMs gpuva list must be modified during the fence
signalling path, which means that it must be protected by a lock that is
fence signalling safe.
For this reason, a mutex is added to struct drm_gem_object that is
intended to achieve this purpose. Adding it directly in the GEM object
both makes it easier to use GPUVM in immediate mode, but also makes it
possible to take the gpuva lock from core drm code.
As a follow-up, another change that should probably be made to support
immediate mode is a mechanism to postpone cleanup of vm_bo objects, as
dropping a vm_bo object in the fence signalling path is problematic for
two reasons:
* When using DRM_GPUVM_RESV_PROTECTED, you cannot remove the vm_bo from
the extobj/evicted lists during the fence signalling path.
* Dropping a vm_bo could lead to the GEM object getting destroyed.
The requirement that GEM object cleanup is fence signalling safe is
dubious and likely to be violated in practice.
Panthor already has its own custom implementation of postponing vm_bo
cleanup.
Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
---
drivers/gpu/drm/drm_gem.c | 2 ++
include/drm/drm_gem.h | 4 +++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 6a44351e58b7741c358406c8a576b6660b5ca904..24c109ab3fadd5af2e5d9de3fe330b105217a9ce 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -187,6 +187,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
kref_init(&obj->refcount);
obj->handle_count = 0;
obj->size = size;
+ mutex_init(&obj->gpuva.lock);
dma_resv_init(&obj->_resv);
if (!obj->resv)
obj->resv = &obj->_resv;
@@ -1057,6 +1058,7 @@ drm_gem_object_free(struct kref *kref)
if (WARN_ON(!obj->funcs->free))
return;
+ mutex_destroy(&obj->gpuva.lock);
obj->funcs->free(obj);
}
EXPORT_SYMBOL(drm_gem_object_free);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index d3a7b43e2c637b164eba5af7cc2fc8ef09d4f0a4..5934d8dc267a65aaf62d2d025869221cd110b325 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -403,11 +403,13 @@ struct drm_gem_object {
* Provides the list of GPU VAs attached to this GEM object.
*
* Drivers should lock list accesses with the GEMs &dma_resv lock
- * (&drm_gem_object.resv) or a custom lock if one is provided.
+ * (&drm_gem_object.resv) or a custom lock if one is provided. The
+ * mutex inside this struct may be used as the custom lock.
*/
struct {
struct list_head list;
+ struct mutex lock;
#ifdef CONFIG_LOCKDEP
struct lockdep_map *lock_dep_map;
#endif
--
2.51.0.rc0.215.g125493bb4a-goog
Powered by blists - more mailing lists