Hi

Am 13.10.26 um 20:37 schrieb Matthew Brost:
On Tue, Oct 13, 2026 at 09:17:53AM +0200, Thomas Zimmermann wrote:
Hi

Am 13.10.26 um 03:36 schrieb Matthew Brost:
On Mon, Oct 12, 2026 at 06:06:15PM -0700, Niranjana Vishwanathapura wrote:
drm_gem_is_prime_exported_dma_buf() is a bool predicate that open-codes
the core PRIME self-import check. Its only caller, the shmem importer,
then repeats the dma_buf->priv lookup and the reference get.

Replace it with drm_gem_prime_dma_buf_to_gem(), a thin wrapper around
drm_gem_prime_self_import() that returns the backing GEM object -- with
a reference taken -- for a core-PRIME self-export, or NULL otherwise.
This consolidates self-import handling onto drm_gem_prime_self_import()
and lets drm_gem_shmem_prime_import_no_map() reuse the object directly.

Unlike the removed predicate, the helper takes a reference on the
returned object; the sole caller already wanted the referenced object.

Assisted-by: GitHub-Copilot:claude-opus-4.8
Assisted-by: LLM

I was going to suggest this last rev too as I noticed
drm_gem_shmem_prime_import_no_map called
drm_gem_is_prime_exported_dma_buf which should prefer the new helper but
shmem couldn't look at drm_gem_prime_dmabuf_ops.

This is a good cleanup:
No it's not. Please see my reply to the cover letter.

Ok, I can't say I agree, but I'm not going to die on this hill. Still,
here's my two cents.

I'm not sure I agree that comparing a vfunc is a hack, but let's assume
it is. Is it better to have four drivers plus two cases in DRM core all
open-coding this logic, or to consolidate it in a single place so that
if we find a better approach later, we have a central point where it can
be changed?

The helper leaks the prime implementation to the outside world. That alone is a problem. In practice there's no real guarantee that it works with drivers that do not fully use prime, such as amdgpu.

The importer whats to know if it has exported the dma_buf, but there's no clear unique identifier. So it checks the device pointer stored in priv. Because it cannot know if priv contains a device pointer, it first compares the vtable.  All of that really is a hack. A driver could use different vtables for the exported dma-bufs, or also have multiple memory managers that share the same vtable for exported dma-bufs.  Neither would work with the current workarounds.

What the importer needs a  unique identifier for each dma-buf allocator. That workaround is just the next best thing.

Best regards
Thomas



To me, the answer is obvious, but again, I'm fine with ignoring this.

Matt

Best regards
Thomas

Reviewed-by: Matthew Brost <[email protected]>

Signed-off-by: Niranjana Vishwanathapura <[email protected]>
---
   drivers/gpu/drm/drm_gem_shmem_helper.c | 14 ++++++--------
   drivers/gpu/drm/drm_prime.c            | 26 +++++++++++++++-----------
   include/drm/drm_prime.h                |  4 ++--
   3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index d654a22a30de..a0e161f2333e 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -954,15 +954,13 @@ struct drm_gem_object 
*drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
        size_t size;
        int ret;
-       if (drm_gem_is_prime_exported_dma_buf(dev, dma_buf)) {
-               /*
-                * Importing dmabuf exported from our own gem increases
-                * refcount on gem itself instead of f_count of dmabuf.
-                */
-               obj = dma_buf->priv;
-               drm_gem_object_get(obj);
+       /*
+        * Importing a dmabuf exported from our own gem increases the refcount
+        * on the gem itself instead of the f_count of the dmabuf.
+        */
+       obj = drm_gem_prime_dma_buf_to_gem(dev, dma_buf);
+       if (obj)
                return obj;
-       }
        attach = dma_buf_attach(dma_buf, dev->dev);
        if (IS_ERR(attach))
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 588910c250b8..275c5661528d 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -933,23 +933,27 @@ EXPORT_SYMBOL(drm_gem_prime_export);
   /**
- * drm_gem_is_prime_exported_dma_buf -
- * checks if the DMA-BUF was exported from a GEM object belonging to @dev.
+ * drm_gem_prime_dma_buf_to_gem - GEM object backing a self-exported PRIME 
dma-buf
    * @dev: drm_device to check against
    * @dma_buf: dma-buf object to import
    *
- * Return: true if the DMA-BUF was exported from a GEM object belonging
- * to @dev, false otherwise.
+ * If @dma_buf is a PRIME export of a GEM object that belongs to @dev and uses
+ * the core PRIME dma-buf ops, return that GEM object with an extra reference
+ * taken, so a self-import reuses the GEM object instead of referencing the
+ * dma-buf file. Returns NULL for a foreign dma-buf or one using driver-private
+ * ops; such drivers call drm_gem_prime_self_import() with their own ops.
+ *
+ * The returned object carries a reference the caller must drop with
+ * drm_gem_object_put().
+ *
+ * Return: the referenced GEM object on self-import, or NULL otherwise.
    */
-
-bool drm_gem_is_prime_exported_dma_buf(struct drm_device *dev,
-                                      struct dma_buf *dma_buf)
+struct drm_gem_object *drm_gem_prime_dma_buf_to_gem(struct drm_device *dev,
+                                                   struct dma_buf *dma_buf)
   {
-       struct drm_gem_object *obj = dma_buf->priv;
-
-       return (dma_buf->ops == &drm_gem_prime_dmabuf_ops) && (obj->dev == dev);
+       return drm_gem_prime_self_import(dev, dma_buf, 
&drm_gem_prime_dmabuf_ops);
   }
-EXPORT_SYMBOL(drm_gem_is_prime_exported_dma_buf);
+EXPORT_SYMBOL(drm_gem_prime_dma_buf_to_gem);
   /**
    * drm_gem_prime_import_dev - core implementation of the import callback
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index d378404f8ab4..02665205aed1 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -101,8 +101,8 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object 
*obj,
   unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt);
   /* helper functions for importing */
-bool drm_gem_is_prime_exported_dma_buf(struct drm_device *dev,
-                                      struct dma_buf *dma_buf);
+struct drm_gem_object *drm_gem_prime_dma_buf_to_gem(struct drm_device *dev,
+                                                   struct dma_buf *dma_buf);
   struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
                                                struct dma_buf *dma_buf,
--
2.43.0

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)



--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri, (HRB 36809, AG Nürnberg)


Reply via email to