On Sun, Mar 25, 2018 at 01:34:51PM +0200, Christian König wrote:
> Move the parameters into a structure to make it simpler to extend it in
> follow up patches.
> 
> This also adds the importer private as parameter so that we can directly
> work with a completely filled in attachment structure.
> 
> Signed-off-by: Christian König <christian.koe...@amd.com>
> ---
>  drivers/dma-buf/dma-buf.c                             | 16 +++++++++-------
>  drivers/gpu/drm/armada/armada_gem.c                   |  6 +++++-
>  drivers/gpu/drm/drm_prime.c                           |  6 +++++-
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c                |  6 +++++-
>  drivers/gpu/drm/tegra/gem.c                           |  6 +++++-
>  drivers/gpu/drm/udl/udl_dmabuf.c                      |  6 +++++-
>  drivers/media/common/videobuf2/videobuf2-dma-contig.c |  6 +++++-
>  drivers/media/common/videobuf2/videobuf2-dma-sg.c     |  6 +++++-
>  drivers/staging/media/tegra-vde/tegra-vde.c           |  6 +++++-
>  include/linux/dma-buf.h                               | 19 
> +++++++++++++++++--
>  10 files changed, 66 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index d78d5fc173dc..d2e8ca0d9427 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -534,8 +534,9 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
>  /**
>   * dma_buf_attach - Add the device to dma_buf's attachments list; optionally,
>   * calls attach() of dma_buf_ops to allow device-specific attach 
> functionality
> - * @dmabuf:  [in]    buffer to attach device to.
> - * @dev:     [in]    device to be attached.
> + * @info:    [in]    holds all the attach related information provided
> + *                   by the importer. see &struct dma_buf_attach_info
> + *                   for further details.

Minor bikeshed, but I like to keep the object we operate on explicit, i.e.
leave the dmabuf as the first paramater and only stuff everything else
into the struct. But I don't think there's enough precedence either way.


>   *
>   * Returns struct dma_buf_attachment pointer for this attachment. Attachments
>   * must be cleaned up by calling dma_buf_detach().
> @@ -549,26 +550,27 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
>   * accessible to @dev, and cannot be moved to a more suitable place. This is
>   * indicated with the error code -EBUSY.
>   */
> -struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> -                                       struct device *dev)
> +struct dma_buf_attachment *dma_buf_attach(const struct dma_buf_attach_info 
> *info)
>  {
> +     struct dma_buf *dmabuf = info->dmabuf;
>       struct dma_buf_attachment *attach;
>       int ret;
>  
> -     if (WARN_ON(!dmabuf || !dev))
> +     if (WARN_ON(!dmabuf || !info->dev))
>               return ERR_PTR(-EINVAL);
>  
>       attach = kzalloc(sizeof(*attach), GFP_KERNEL);
>       if (!attach)
>               return ERR_PTR(-ENOMEM);
>  
> -     attach->dev = dev;
> +     attach->dev = info->dev;
>       attach->dmabuf = dmabuf;
> +     attach->priv = info->priv;

The ->priv field is for the exporter, not the importer. See e.g.
drm_gem_map_attach. You can't let the importer set this now too, so needs
to be removed from the info struct.
-Daniel

>  
>       mutex_lock(&dmabuf->lock);
>  
>       if (dmabuf->ops->attach) {
> -             ret = dmabuf->ops->attach(dmabuf, dev, attach);
> +             ret = dmabuf->ops->attach(dmabuf, info->dev, attach);
>               if (ret)
>                       goto err_attach;
>       }
> diff --git a/drivers/gpu/drm/armada/armada_gem.c 
> b/drivers/gpu/drm/armada/armada_gem.c
> index a97f509743a5..f4d1c11f57ea 100644
> --- a/drivers/gpu/drm/armada/armada_gem.c
> +++ b/drivers/gpu/drm/armada/armada_gem.c
> @@ -514,6 +514,10 @@ armada_gem_prime_export(struct drm_device *dev, struct 
> drm_gem_object *obj,
>  struct drm_gem_object *
>  armada_gem_prime_import(struct drm_device *dev, struct dma_buf *buf)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = dev->dev,
> +             .dmabuf = buf
> +     };
>       struct dma_buf_attachment *attach;
>       struct armada_gem_object *dobj;
>  
> @@ -529,7 +533,7 @@ armada_gem_prime_import(struct drm_device *dev, struct 
> dma_buf *buf)
>               }
>       }
>  
> -     attach = dma_buf_attach(buf, dev->dev);
> +     attach = dma_buf_attach(&attach_info);
>       if (IS_ERR(attach))
>               return ERR_CAST(attach);
>  
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 7856a9b3f8a8..4da242de51c2 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -707,6 +707,10 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct 
> drm_device *dev,
>                                           struct dma_buf *dma_buf,
>                                           struct device *attach_dev)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = attach_dev,
> +             .dmabuf = dma_buf
> +     };
>       struct dma_buf_attachment *attach;
>       struct sg_table *sgt;
>       struct drm_gem_object *obj;
> @@ -727,7 +731,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct 
> drm_device *dev,
>       if (!dev->driver->gem_prime_import_sg_table)
>               return ERR_PTR(-EINVAL);
>  
> -     attach = dma_buf_attach(dma_buf, attach_dev);
> +     attach = dma_buf_attach(&attach_info);
>       if (IS_ERR(attach))
>               return ERR_CAST(attach);
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c 
> b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index 864439a214c8..94552ef3e5a7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -288,6 +288,10 @@ static const struct drm_i915_gem_object_ops 
> i915_gem_object_dmabuf_ops = {
>  struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
>                                            struct dma_buf *dma_buf)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = dev->dev,
> +             .dmabuf = dma_buf
> +     };
>       struct dma_buf_attachment *attach;
>       struct drm_i915_gem_object *obj;
>       int ret;
> @@ -306,7 +310,7 @@ struct drm_gem_object *i915_gem_prime_import(struct 
> drm_device *dev,
>       }
>  
>       /* need to attach */
> -     attach = dma_buf_attach(dma_buf, dev->dev);
> +     attach = dma_buf_attach(&attach_info);
>       if (IS_ERR(attach))
>               return ERR_CAST(attach);
>  
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 49b9bf28f872..462a4bac3f82 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -332,6 +332,10 @@ struct tegra_bo *tegra_bo_create_with_handle(struct 
> drm_file *file,
>  static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
>                                       struct dma_buf *buf)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = drm->dev,
> +             .dmabuf = buf
> +     };
>       struct tegra_drm *tegra = drm->dev_private;
>       struct dma_buf_attachment *attach;
>       struct tegra_bo *bo;
> @@ -341,7 +345,7 @@ static struct tegra_bo *tegra_bo_import(struct drm_device 
> *drm,
>       if (IS_ERR(bo))
>               return bo;
>  
> -     attach = dma_buf_attach(buf, drm->dev);
> +     attach = dma_buf_attach(&attach_info);
>       if (IS_ERR(attach)) {
>               err = PTR_ERR(attach);
>               goto free;
> diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c 
> b/drivers/gpu/drm/udl/udl_dmabuf.c
> index 2867ed155ff6..c4db84abe231 100644
> --- a/drivers/gpu/drm/udl/udl_dmabuf.c
> +++ b/drivers/gpu/drm/udl/udl_dmabuf.c
> @@ -243,6 +243,10 @@ static int udl_prime_create(struct drm_device *dev,
>  struct drm_gem_object *udl_gem_prime_import(struct drm_device *dev,
>                               struct dma_buf *dma_buf)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = dev->dev,
> +             .dmabuf = dma_buf
> +     };
>       struct dma_buf_attachment *attach;
>       struct sg_table *sg;
>       struct udl_gem_object *uobj;
> @@ -250,7 +254,7 @@ struct drm_gem_object *udl_gem_prime_import(struct 
> drm_device *dev,
>  
>       /* need to attach */
>       get_device(dev->dev);
> -     attach = dma_buf_attach(dma_buf, dev->dev);
> +     attach = dma_buf_attach(&attach_info);
>       if (IS_ERR(attach)) {
>               put_device(dev->dev);
>               return ERR_CAST(attach);
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c 
> b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index f1178f6f434d..93bd1f40f756 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -677,6 +677,10 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
>  static void *vb2_dc_attach_dmabuf(struct device *dev, struct dma_buf *dbuf,
>       unsigned long size, enum dma_data_direction dma_dir)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = dev,
> +             .dmabuf = dbuf
> +     };
>       struct vb2_dc_buf *buf;
>       struct dma_buf_attachment *dba;
>  
> @@ -692,7 +696,7 @@ static void *vb2_dc_attach_dmabuf(struct device *dev, 
> struct dma_buf *dbuf,
>  
>       buf->dev = dev;
>       /* create attachment for the dmabuf with the user device */
> -     dba = dma_buf_attach(dbuf, buf->dev);
> +     dba = dma_buf_attach(&attach_info);
>       if (IS_ERR(dba)) {
>               pr_err("failed to attach dmabuf\n");
>               kfree(buf);
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c 
> b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index 753ed3138dcc..4e61050ba87f 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -609,6 +609,10 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
>  static void *vb2_dma_sg_attach_dmabuf(struct device *dev, struct dma_buf 
> *dbuf,
>       unsigned long size, enum dma_data_direction dma_dir)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = dev,
> +             .dmabuf = dbuf
> +     };
>       struct vb2_dma_sg_buf *buf;
>       struct dma_buf_attachment *dba;
>  
> @@ -624,7 +628,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct device *dev, 
> struct dma_buf *dbuf,
>  
>       buf->dev = dev;
>       /* create attachment for the dmabuf with the user device */
> -     dba = dma_buf_attach(dbuf, buf->dev);
> +     dba = dma_buf_attach(&attach_info);
>       if (IS_ERR(dba)) {
>               pr_err("failed to attach dmabuf\n");
>               kfree(buf);
> diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
> b/drivers/staging/media/tegra-vde/tegra-vde.c
> index c47659e96089..25d112443b0d 100644
> --- a/drivers/staging/media/tegra-vde/tegra-vde.c
> +++ b/drivers/staging/media/tegra-vde/tegra-vde.c
> @@ -529,6 +529,10 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
>                                  size_t *size,
>                                  enum dma_data_direction dma_dir)
>  {
> +     struct dma_buf_attach_info attach_info = {
> +             .dev = dev,
> +             .dmabuf = dmabuf
> +     };
>       struct dma_buf_attachment *attachment;
>       struct dma_buf *dmabuf;
>       struct sg_table *sgt;
> @@ -547,7 +551,7 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
>               return -EINVAL;
>       }
>  
> -     attachment = dma_buf_attach(dmabuf, dev);
> +     attachment = dma_buf_attach(&attach_info);
>       if (IS_ERR(attachment)) {
>               dev_err(dev, "Failed to attach dmabuf\n");
>               err = PTR_ERR(attachment);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 085db2fee2d7..2c27568d44af 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -362,6 +362,21 @@ struct dma_buf_export_info {
>       struct dma_buf_export_info name = { .exp_name = KBUILD_MODNAME, \
>                                        .owner = THIS_MODULE }
>  
> +/**
> + * struct dma_buf_attach_info - holds information needed to attach to a 
> dma_buf
> + * @dmabuf:  the exported dma_buf
> + * @dev:     the device which wants to import the attachment
> + * @priv:    private data of importer to this attachment
> + *
> + * This structure holds the information required to attach to a buffer. Used
> + * with dma_buf_attach() only.
> + */
> +struct dma_buf_attach_info {
> +     struct dma_buf *dmabuf;
> +     struct device *dev;
> +     void *priv;
> +};
> +
>  /**
>   * get_dma_buf - convenience wrapper for get_file.
>   * @dmabuf:  [in]    pointer to dma_buf
> @@ -376,8 +391,8 @@ static inline void get_dma_buf(struct dma_buf *dmabuf)
>       get_file(dmabuf->file);
>  }
>  
> -struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> -                                                     struct device *dev);
> +struct dma_buf_attachment *
> +dma_buf_attach(const struct dma_buf_attach_info *info);
>  void dma_buf_detach(struct dma_buf *dmabuf,
>                               struct dma_buf_attachment *dmabuf_attach);
>  
> -- 
> 2.14.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Reply via email to