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 at amd.com>
>> ---
>> 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 at ffwll.ch>
>
> 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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
