Re: [PATCH v2 3/6] drm/gem: add drm_gem_prime_dma_buf_to_gem() self-import helper

[Matthew Brost <matthew.brost@intel.com>]

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?

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

Matt

> Best regards
> Thomas
> 
> > 
> > Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> > 
> > > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
> > > ---
> > >   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)
> 
>