mbox series

[0/7] Rust abstractions for shmem-backed GEM objects

Message ID 20250318-drm-gem-shmem-v1-0-64b96511a84f@collabora.com
Headers show
Series Rust abstractions for shmem-backed GEM objects | expand

Message

Daniel Almeida March 18, 2025, 7:22 p.m. UTC
Hi all,

This series picks up the work carried out by the Asahi project for
shmem-backed GEM objects. This initial version is meant to kickstart the
discussion on this topic, as the bindings will be clearly needed by Tyr
and other drivers.

It has been tested on both AGX and Tyr successfully.

I did provide a minor fix for a missing #include, but I did not touch
this code otherwise so far. Even the rebase was done by Janne Grunnau.

Applies on top of

commit 0722a3f4f15545a4a25fd124b6955a5b6498e23a
Author: Danilo Krummrich <dakr@kernel.org>
Date:   Tue Oct 15 17:19:27 2024 +0200

    nova: add initial driver stub
    

---
Asahi Lina (7):
      drm/shmem-helper: Add lockdep asserts to vmap/vunmap
      drm/gem-shmem: Export VM ops functions
      rust: helpers: Add bindings/wrappers for dma_resv_lock
      rust: drm: gem: shmem: Add DRM shmem helper abstraction
      drm/gem: Add a flag to control whether objects can be exported
      rust: drm: gem: Add set_exportable() method
      rust: drm: gem: shmem: Add share_dma_resv() function

 drivers/gpu/drm/drm_gem.c              |   1 +
 drivers/gpu/drm/drm_gem_shmem_helper.c |  13 +-
 drivers/gpu/drm/drm_prime.c            |   5 +
 include/drm/drm_gem.h                  |   8 +
 include/drm/drm_gem_shmem_helper.h     |   3 +
 rust/bindings/bindings_helper.h        |   4 +
 rust/helpers/dma-resv.c                |  13 +
 rust/helpers/drm.c                     |  46 ++++
 rust/helpers/helpers.c                 |   2 +
 rust/helpers/scatterlist.c             |  13 +
 rust/kernel/drm/gem/mod.rs             |  15 ++
 rust/kernel/drm/gem/shmem.rs           | 457 +++++++++++++++++++++++++++++++++
 12 files changed, 577 insertions(+), 3 deletions(-)
---
base-commit: 0722a3f4f15545a4a25fd124b6955a5b6498e23a
change-id: 20250318-drm-gem-shmem-8bb647b66b1c

Best regards,

Comments

Christian König March 19, 2025, 7:49 a.m. UTC | #1
Am 18.03.25 um 20:22 schrieb Daniel Almeida:
> From: Asahi Lina <lina@asahilina.net>
>
> Since commit 21aa27ddc582 ("drm/shmem-helper: Switch to reservation
> lock"), the drm_gem_shmem_vmap and drm_gem_shmem_vunmap functions
> require that the caller holds the DMA reservation lock for the object.
> Add lockdep assertions to help validate this.
>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>

Oh, yeah that is certainly a good idea.

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>  drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 5ab351409312b5a0de542df2b636278d6186cb7b..ec89e9499f5f02a2a35713669bf649dd2abb9938 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -338,6 +338,8 @@ int drm_gem_shmem_vmap(struct drm_gem_shmem_object *shmem,
>  	struct drm_gem_object *obj = &shmem->base;
>  	int ret = 0;
>  
> +	dma_resv_assert_held(obj->resv);
> +
>  	if (obj->import_attach) {
>  		ret = dma_buf_vmap(obj->import_attach->dmabuf, map);
>  		if (!ret) {
> @@ -404,6 +406,8 @@ void drm_gem_shmem_vunmap(struct drm_gem_shmem_object *shmem,
>  {
>  	struct drm_gem_object *obj = &shmem->base;
>  
> +	dma_resv_assert_held(obj->resv);
> +
>  	if (obj->import_attach) {
>  		dma_buf_vunmap(obj->import_attach->dmabuf, map);
>  	} else {
>
Christian König March 19, 2025, 8:04 a.m. UTC | #2
Am 18.03.25 um 20:22 schrieb Daniel Almeida:
> From: Asahi Lina <lina@asahilina.net>
>
> Drivers may want to support driver-private objects, which cannot be
> shared. This allows them to share a single lock and enables other
> optimizations.
>
> Add an `exportable` field to drm_gem_object, which blocks PRIME export
> if set to false. It is initialized to true in
> drm_gem_private_object_init.

We already have a method for doing that which is used by almost all drivers (except for lsdc).

Basically you just create a function which checks the per-requisites if a buffer can be exported before calling drm_gem_prime_export() and installs that as .export callback into the drm_gem_object_funcs.

See amdgpu_gem_prime_export() for a simpler example.

Regards,
Christian.

>
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> ---
>  drivers/gpu/drm/drm_gem.c   | 1 +
>  drivers/gpu/drm/drm_prime.c | 5 +++++
>  include/drm/drm_gem.h       | 8 ++++++++
>  3 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index ee811764c3df4b4e9c377a66afd4967512ba2001..8f998fe6beecd285ce3e2d5badfa95eb7d7bd548 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -195,6 +195,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
>  
>  	drm_vma_node_reset(&obj->vma_node);
>  	INIT_LIST_HEAD(&obj->lru_node);
> +	obj->exportable = true;
>  }
>  EXPORT_SYMBOL(drm_gem_private_object_init);
>  
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 32a8781cfd67b82ece7b7b94625715171bb41917..20aa350280abe9a6ed6742e131ff50c65bc9dfa9 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -387,6 +387,11 @@ static struct dma_buf *export_and_register_object(struct drm_device *dev,
>  		return dmabuf;
>  	}
>  
> +	if (!obj->exportable) {
> +		dmabuf = ERR_PTR(-EINVAL);
> +		return dmabuf;
> +	}
> +
>  	if (obj->funcs && obj->funcs->export)
>  		dmabuf = obj->funcs->export(obj, flags);
>  	else
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index fdae947682cd0b7b06db5e35e120f049a0f30179..f700e4996eccb92597cca6b8c3df8e35b864c1e1 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -432,6 +432,14 @@ struct drm_gem_object {
>  	 * The current LRU list that the GEM object is on.
>  	 */
>  	struct drm_gem_lru *lru;
> +
> +	/**
> +	 * @exportable:
> +	 *
> +	 * Whether this GEM object can be exported via the drm_gem_object_funcs->export
> +	 * callback. Defaults to true.
> +	 */
> +	bool exportable;
>  };
>  
>  /**
>