Re: [PATCH v6 01/16] drm/prime: Simplify life of drivers needing custom dma_buf_ops

[Boris Brezillon <boris.brezillon@collabora.com>]

On Thu, 27 Nov 2025 09:34:43 +0100
Thomas Zimmermann <tzimmermann@suse.de> wrote:

> Hi
> 
> Am 26.11.25 um 13:44 schrieb Boris Brezillon:
> > drm_gem_is_prime_exported_dma_buf() checks the dma_buf->ops against
> > drm_gem_prime_dmabuf_ops, which makes it impossible to use if the
> > driver implements custom dma_buf_ops. Instead of duplicating a bunch
> > of helpers to work around it, let's provide a way for drivers to
> > expose their custom dma_buf_ops so the core prime helpers can rely on
> > that instead of hardcoding &drm_gem_prime_dmabuf_ops.  
> 
> This can't go in as-is. I've spent an awful amount of patches on 
> removing buffer callbacks from struct drm_driver.

First, it's no longer a callback, but a pointer to dma_buf_ops
(joking of course, I get what you mean :P).

> Let's please not go 
> back to that.
> 
> >
> > v5:
> > - New patch
> >
> > v6:
> > - Pass custom dma_buf_ops directly instead of through a getter
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >   drivers/gpu/drm/drm_prime.c | 10 ++++++++--
> >   include/drm/drm_drv.h       |  8 ++++++++
> >   2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > index 21809a82187b..86fd95f0c105 100644
> > --- a/drivers/gpu/drm/drm_prime.c
> > +++ b/drivers/gpu/drm/drm_prime.c
> > @@ -904,6 +904,12 @@ unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt)
> >   }
> >   EXPORT_SYMBOL(drm_prime_get_contiguous_size);
> >   
> > +static const struct dma_buf_ops *
> > +drm_gem_prime_get_dma_buf_ops(struct drm_device *dev)
> > +{
> > +	return dev->driver->dma_buf_ops ?: &drm_gem_prime_dmabuf_ops;
> > +}
> > +
> >   /**
> >    * drm_gem_prime_export - helper library implementation of the export callback
> >    * @obj: GEM object to export
> > @@ -920,7 +926,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
> >   	struct dma_buf_export_info exp_info = {
> >   		.exp_name = KBUILD_MODNAME, /* white lie for debug */
> >   		.owner = dev->driver->fops->owner,
> > -		.ops = &drm_gem_prime_dmabuf_ops,
> > +		.ops = drm_gem_prime_get_dma_buf_ops(dev),  
> 
> Rather provide a new function drm_gem_prime_export_with_ops() that takes 
> an additional dma_ops instance. The current drm_gem_prime_export() would 
> call it with &drm_gem_prime_dmabuf_ops.
> 
> If this really does not work, you could add a pointer to dma_buf_ops to 
> drm_gem_object_funcs and fetch that from drm_gem_prime_export(). We 
> already vm_ops there.

It doesn't work because we need an ops to test against when a dma_buf
is imported, and that information has to be available at the
drm_{device,driver} level.

> 
> Other drivers, such as amdgpu, would also benefit from such a change
> 
> >   		.size = obj->size,
> >   		.flags = flags,
> >   		.priv = obj,
> > @@ -947,7 +953,7 @@ bool drm_gem_is_prime_exported_dma_buf(struct drm_device *dev,
> >   {
> >   	struct drm_gem_object *obj = dma_buf->priv;
> >   
> > -	return (dma_buf->ops == &drm_gem_prime_dmabuf_ops) && (obj->dev == dev);
> > +	return dma_buf->ops == drm_gem_prime_get_dma_buf_ops(dev) && obj->dev == dev;  
> 
> This is a bit more complicated and the test has been a pain point 
> before. For this case, I think we should add a GEM callback for this
> 
> struct drm_gem_object_funcs {
>      bool (*exported_by)(struct drm_gem_object *obj, struct drm_device *dev)
> }
> 
> next to the existing export callback.

Nope, because dma_buf::ops needs to be tested against the driver
dma_buf_ops to determine if it's a drm_gem_object that's stored in
dma_buf::priv. If we add such a callback, it has to be at the
drm_driver level.