Hi Daniel, On 17-07-12 04:11 AM, Daniel Vetter wrote: > On Wed, Jul 12, 2017 at 01:29:22AM -0400, Felix Kuehling wrote: >> Signed-off-by: Felix Kuehling <Felix.Kuehling@xxxxxxx> >> --- >> drivers/gpu/drm/drm_prime.c | 25 +++++++++++++++++++++++++ >> include/drm/drmP.h | 2 ++ >> 2 files changed, 27 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c >> index 25aa455..b1f8445 100644 >> --- a/drivers/gpu/drm/drm_prime.c >> +++ b/drivers/gpu/drm/drm_prime.c >> @@ -594,6 +594,31 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, >> EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); >> >> /** >> + * drm_gem_prime_dmabuf_to_object - try to cast dmabuf to GEM object >> + * @dma_buf: dma-buf object to cast >> + * @driver: driver that is the expected exporter of the dma-buf >> + * >> + * If @dma_buf represents a GEM object, this function return a pointer >> + * to it. Optionally, if @driver is not NULL, it also checks that the >> + * object was exported by @driver. Otherwise it returns NULL. >> + */ >> +struct drm_gem_object *drm_gem_prime_dmabuf_to_object(struct dma_buf *dma_buf, >> + struct drm_driver *driver) >> +{ >> + struct drm_gem_object *obj; >> + >> + if (dma_buf->ops != &drm_gem_prime_dmabuf_ops) >> + return NULL; >> + >> + obj = dma_buf->priv; >> + if (driver && obj->dev->driver != driver) >> + return NULL; >> + >> + return obj; >> +} >> +EXPORT_SYMBOL(drm_gem_prime_dmabuf_to_object); > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Bonus points bikeshed: Would be sweet to use that helper in drm_prime.c > itself, I found only one place in drm_prime.c where it would make sense. In most cases, the drm_prime_* functions called through drm_gem_prime_dmabuf_ops already know that they're dealing with a GEM object. So the extra checks in the new helper are redundant. In drm_gem_prime_import I could make the following change: --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -634,16 +634,14 @@ struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct drm_gem_object *obj; int ret; - if (dma_buf->ops == &drm_gem_prime_dmabuf_ops) { - obj = dma_buf->priv; - if (obj->dev == dev) { - /* - * Importing dmabuf exported from out own gem increases - * refcount on gem itself instead of f_count of dmabuf. - */ - drm_gem_object_reference(obj); - return obj; - } + obj = drm_gem_prime_dmabuf_to_object(dma_buf, NULL); + if (obj && obj->dev == dev) { + /* + * Importing dmabuf exported from out own gem increases + * refcount on gem itself instead of f_count of dmabuf. + */ + drm_gem_object_reference(obj); + return obj; } if (!dev->driver->gem_prime_import_sg_table) Do you want me to do that in the same commit, or a separate one? > and iirc some drivers could use it too. I guess other drivers would have to wait until Alex pushes this to drm-next. Regards, Felix > -Daniel > >> + >> +/** >> * drm_gem_prime_import - helper library implementation of the import callback >> * @dev: drm_device to import into >> * @dma_buf: dma-buf object to import >> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >> index 6105c05..79c2b23 100644 >> --- a/include/drm/drmP.h >> +++ b/include/drm/drmP.h >> @@ -767,6 +767,8 @@ extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev, >> extern int drm_gem_prime_handle_to_fd(struct drm_device *dev, >> struct drm_file *file_priv, uint32_t handle, uint32_t flags, >> int *prime_fd); >> +extern struct drm_gem_object *drm_gem_prime_dmabuf_to_object( >> + struct dma_buf *dma_buf, struct drm_driver *driver); >> extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, >> struct dma_buf *dma_buf); >> extern int drm_gem_prime_fd_to_handle(struct drm_device *dev, >> -- >> 1.9.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel