[1/6] drm: Add helper to cast DMA-buf to GEM object

Commit Message

Felix Kuehling July 12, 2017, 9:44 p.m. UTC

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 <[e-mail]>
>> ---
>>  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 <[e-mail]>
>
> 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:


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
>> [e-mail]
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Comments

Daniel Vetter July 13, 2017, 6:01 a.m. UTC | #1

On Wed, Jul 12, 2017 at 11:44 PM, Felix Kuehling <[e-mail]> wrote:
>
> --- 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?

I'd squash it into the one commit you have already.
-Daniel

Patch

--- 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)