On Thu, 27 Nov 2025 09:42:52 +0100 Thomas Zimmermann <[email protected]> wrote:
> Hi > > Am 27.11.25 um 09:34 schrieb Thomas Zimmermann: > > 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. 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 <[email protected]> > >> --- > >> 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. > > > > 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; > > On a second thought, we probably cannot be sure that dma_buf->priv > really is a GEM object until we tested the ops field. :/ IIRC that's > why the ops test goes first and the test for obj->dev goes second. So > neither solution works. Hm, What do you mean by neither solution works? The original proposal never dereferences obj until it's sure it's a drm_gem_object, that's what dma_buf->ops == drm_gem_prime_get_dma_buf_ops(dev) is for.
