Re: [PATCH v2 3/3] dma-buf: Use struct dma_buf_map in dma_buf_vunmap() interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 23, 2020 at 02:32:05PM +0200, Thomas Zimmermann wrote:
> This patch updates dma_buf_vunmap() and dma-buf's vunmap callback to
> use struct dma_buf_map. The interfaces used to receive a buffer address.
> This address is now given in an instance of the structure.
> 
> Users of the functions are updated accordingly. This is only an interface
> change. It is currently expected that dma-buf memory can be accessed with
> system memory load/store operations.
> 
> v2:
> 	* include dma-buf-heaps and i915 selftests (kernel test robot)
> 	* initialize cma_obj before using it in drm_gem_cma_free_object()
> 	  (kernel test robot)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> Acked-by: Sumit Semwal <sumit.semwal@xxxxxxxxxx>

Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx>

> ---
>  drivers/dma-buf/dma-buf.c                     |  8 ++---
>  drivers/dma-buf/heaps/heap-helpers.c          |  2 +-
>  drivers/gpu/drm/drm_gem_cma_helper.c          |  9 +++---
>  drivers/gpu/drm/drm_gem_shmem_helper.c        |  3 +-
>  drivers/gpu/drm/drm_prime.c                   |  6 ++--
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c   |  5 +--
>  drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c    |  2 +-
>  .../drm/i915/gem/selftests/i915_gem_dmabuf.c  |  6 ++--
>  .../gpu/drm/i915/gem/selftests/mock_dmabuf.c  |  4 +--
>  drivers/gpu/drm/tegra/gem.c                   |  5 +--
>  .../common/videobuf2/videobuf2-dma-contig.c   |  3 +-
>  .../media/common/videobuf2/videobuf2-dma-sg.c |  3 +-
>  .../common/videobuf2/videobuf2-vmalloc.c      |  6 ++--
>  include/drm/drm_prime.h                       |  2 +-
>  include/linux/dma-buf-map.h                   | 32 +++++++++++++++++--
>  include/linux/dma-buf.h                       |  4 +--
>  16 files changed, 66 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index 61bd24d21b38..a6ba4d598f0e 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -1236,21 +1236,21 @@ EXPORT_SYMBOL_GPL(dma_buf_vmap);
>  /**
>   * dma_buf_vunmap - Unmap a vmap obtained by dma_buf_vmap.
>   * @dmabuf:	[in]	buffer to vunmap
> - * @vaddr:	[in]	vmap to vunmap
> + * @map:	[in]	vmap pointer to vunmap
>   */
> -void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
> +void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map)
>  {
>  	if (WARN_ON(!dmabuf))
>  		return;
>  
>  	BUG_ON(dma_buf_map_is_null(&dmabuf->vmap_ptr));
>  	BUG_ON(dmabuf->vmapping_counter == 0);
> -	BUG_ON(!dma_buf_map_is_vaddr(&dmabuf->vmap_ptr, vaddr));
> +	BUG_ON(!dma_buf_map_is_equal(&dmabuf->vmap_ptr, map));
>  
>  	mutex_lock(&dmabuf->lock);
>  	if (--dmabuf->vmapping_counter == 0) {
>  		if (dmabuf->ops->vunmap)
> -			dmabuf->ops->vunmap(dmabuf, vaddr);
> +			dmabuf->ops->vunmap(dmabuf, map);
>  		dma_buf_map_clear(&dmabuf->vmap_ptr);
>  	}
>  	mutex_unlock(&dmabuf->lock);
> diff --git a/drivers/dma-buf/heaps/heap-helpers.c b/drivers/dma-buf/heaps/heap-helpers.c
> index aeb9e100f339..fcf4ce3e2cbb 100644
> --- a/drivers/dma-buf/heaps/heap-helpers.c
> +++ b/drivers/dma-buf/heaps/heap-helpers.c
> @@ -251,7 +251,7 @@ static int dma_heap_dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map
>  	return 0;
>  }
>  
> -static void dma_heap_dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
> +static void dma_heap_dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map)
>  {
>  	struct heap_helper_buffer *buffer = dmabuf->priv;
>  
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 1ece73fd3fe9..1059acdde519 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -175,13 +175,12 @@ drm_gem_cma_create_with_handle(struct drm_file *file_priv,
>   */
>  void drm_gem_cma_free_object(struct drm_gem_object *gem_obj)
>  {
> -	struct drm_gem_cma_object *cma_obj;
> -
> -	cma_obj = to_drm_gem_cma_obj(gem_obj);
> +	struct drm_gem_cma_object *cma_obj = to_drm_gem_cma_obj(gem_obj);
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(cma_obj->vaddr);
>  
>  	if (gem_obj->import_attach) {
>  		if (cma_obj->vaddr)
> -			dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr);
> +			dma_buf_vunmap(gem_obj->import_attach->dmabuf, &map);
>  		drm_prime_gem_destroy(gem_obj, cma_obj->sgt);
>  	} else if (cma_obj->vaddr) {
>  		dma_free_wc(gem_obj->dev->dev, cma_obj->base.size,
> @@ -628,7 +627,7 @@ drm_gem_cma_prime_import_sg_table_vmap(struct drm_device *dev,
>  
>  	obj = drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
>  	if (IS_ERR(obj)) {
> -		dma_buf_vunmap(attach->dmabuf, map.vaddr);
> +		dma_buf_vunmap(attach->dmabuf, &map);
>  		return obj;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 6328cfbb828e..fb11df7aced5 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -337,6 +337,7 @@ EXPORT_SYMBOL(drm_gem_shmem_vmap);
>  static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem)
>  {
>  	struct drm_gem_object *obj = &shmem->base;
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(shmem->vaddr);
>  
>  	if (WARN_ON_ONCE(!shmem->vmap_use_count))
>  		return;
> @@ -345,7 +346,7 @@ static void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem)
>  		return;
>  
>  	if (obj->import_attach)
> -		dma_buf_vunmap(obj->import_attach->dmabuf, shmem->vaddr);
> +		dma_buf_vunmap(obj->import_attach->dmabuf, &map);
>  	else
>  		vunmap(shmem->vaddr);
>  
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 2b3fd01867e4..dfa774c303b6 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -687,16 +687,16 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vmap);
>  /**
>   * drm_gem_dmabuf_vunmap - dma_buf vunmap implementation for GEM
>   * @dma_buf: buffer to be unmapped
> - * @vaddr: the virtual address of the buffer
> + * @map: the virtual address of the buffer
>   *
>   * Releases a kernel virtual mapping. This can be used as the
>   * &dma_buf_ops.vunmap callback. Calls into &drm_gem_object_funcs.vunmap for device specific handling.
>   */
> -void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
> +void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, struct dma_buf_map *map)
>  {
>  	struct drm_gem_object *obj = dma_buf->priv;
>  
> -	drm_gem_vunmap(obj, vaddr);
> +	drm_gem_vunmap(obj, map->vaddr);
>  }
>  EXPORT_SYMBOL(drm_gem_dmabuf_vunmap);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> index 80a9fc143bbb..135fbff6fecf 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
> @@ -70,9 +70,10 @@ void etnaviv_gem_prime_unpin(struct drm_gem_object *obj)
>  
>  static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj)
>  {
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(etnaviv_obj->vaddr);
> +
>  	if (etnaviv_obj->vaddr)
> -		dma_buf_vunmap(etnaviv_obj->base.import_attach->dmabuf,
> -			       etnaviv_obj->vaddr);
> +		dma_buf_vunmap(etnaviv_obj->base.import_attach->dmabuf, &map);
>  
>  	/* Don't drop the pages for imported dmabuf, as they are not
>  	 * ours, just free the array we allocated:
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> index 6ee8f2cfd8c1..0dd477e56573 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
> @@ -91,7 +91,7 @@ static int i915_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map
>  	return 0;
>  }
>  
> -static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
> +static void i915_gem_dmabuf_vunmap(struct dma_buf *dma_buf, struct dma_buf_map *map)
>  {
>  	struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
>  
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
> index f79ebc5329b7..0b4d19729e1f 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_dmabuf.c
> @@ -152,7 +152,7 @@ static int igt_dmabuf_import(void *arg)
>  
>  	err = 0;
>  out_dma_map:
> -	dma_buf_vunmap(dmabuf, dma_map);
> +	dma_buf_vunmap(dmabuf, &map);
>  out_obj:
>  	i915_gem_object_put(obj);
>  out_dmabuf:
> @@ -182,7 +182,7 @@ static int igt_dmabuf_import_ownership(void *arg)
>  	}
>  
>  	memset(ptr, 0xc5, PAGE_SIZE);
> -	dma_buf_vunmap(dmabuf, ptr);
> +	dma_buf_vunmap(dmabuf, &map);
>  
>  	obj = to_intel_bo(i915_gem_prime_import(&i915->drm, dmabuf));
>  	if (IS_ERR(obj)) {
> @@ -250,7 +250,7 @@ static int igt_dmabuf_export_vmap(void *arg)
>  	memset(ptr, 0xc5, dmabuf->size);
>  
>  	err = 0;
> -	dma_buf_vunmap(dmabuf, ptr);
> +	dma_buf_vunmap(dmabuf, &map);
>  out:
>  	dma_buf_put(dmabuf);
>  	return err;
> diff --git a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
> index becd9fb95d58..2855d11c7a51 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/mock_dmabuf.c
> @@ -74,11 +74,11 @@ static int mock_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map)
>  	return 0;
>  }
>  
> -static void mock_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr)
> +static void mock_dmabuf_vunmap(struct dma_buf *dma_buf, struct dma_buf_map *map)
>  {
>  	struct mock_dmabuf *mock = to_mock(dma_buf);
>  
> -	vm_unmap_ram(vaddr, mock->npages);
> +	vm_unmap_ram(map->vaddr, mock->npages);
>  }
>  
>  static int mock_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 6f04d7855f95..8637bdff189c 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -149,11 +149,12 @@ static void *tegra_bo_mmap(struct host1x_bo *bo)
>  static void tegra_bo_munmap(struct host1x_bo *bo, void *addr)
>  {
>  	struct tegra_bo *obj = host1x_to_tegra_bo(bo);
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(addr);
>  
>  	if (obj->vaddr)
>  		return;
>  	else if (obj->gem.import_attach)
> -		dma_buf_vunmap(obj->gem.import_attach->dmabuf, addr);
> +		dma_buf_vunmap(obj->gem.import_attach->dmabuf, &map);
>  	else
>  		vunmap(addr);
>  }
> @@ -648,7 +649,7 @@ static int tegra_gem_prime_vmap(struct dma_buf *buf, struct dma_buf_map *map)
>  	return 0;
>  }
>  
> -static void tegra_gem_prime_vunmap(struct dma_buf *buf, void *vaddr)
> +static void tegra_gem_prime_vunmap(struct dma_buf *buf, struct dma_buf_map *map)
>  {
>  }
>  
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> index 11428287bdf3..a1eb8279b113 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
> @@ -648,6 +648,7 @@ static void vb2_dc_unmap_dmabuf(void *mem_priv)
>  {
>  	struct vb2_dc_buf *buf = mem_priv;
>  	struct sg_table *sgt = buf->dma_sgt;
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buf->vaddr);
>  
>  	if (WARN_ON(!buf->db_attach)) {
>  		pr_err("trying to unpin a not attached buffer\n");
> @@ -660,7 +661,7 @@ static void vb2_dc_unmap_dmabuf(void *mem_priv)
>  	}
>  
>  	if (buf->vaddr) {
> -		dma_buf_vunmap(buf->db_attach->dmabuf, buf->vaddr);
> +		dma_buf_vunmap(buf->db_attach->dmabuf, &map);
>  		buf->vaddr = NULL;
>  	}
>  	dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index c51170e9c1b9..d5157e903e27 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -580,6 +580,7 @@ static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = mem_priv;
>  	struct sg_table *sgt = buf->dma_sgt;
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buf->vaddr);
>  
>  	if (WARN_ON(!buf->db_attach)) {
>  		pr_err("trying to unpin a not attached buffer\n");
> @@ -592,7 +593,7 @@ static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
>  	}
>  
>  	if (buf->vaddr) {
> -		dma_buf_vunmap(buf->db_attach->dmabuf, buf->vaddr);
> +		dma_buf_vunmap(buf->db_attach->dmabuf, &map);
>  		buf->vaddr = NULL;
>  	}
>  	dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> index 7b68e2379c65..11ba0eb1315b 100644
> --- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> +++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
> @@ -390,17 +390,19 @@ static int vb2_vmalloc_map_dmabuf(void *mem_priv)
>  static void vb2_vmalloc_unmap_dmabuf(void *mem_priv)
>  {
>  	struct vb2_vmalloc_buf *buf = mem_priv;
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buf->vaddr);
>  
> -	dma_buf_vunmap(buf->dbuf, buf->vaddr);
> +	dma_buf_vunmap(buf->dbuf, &map);
>  	buf->vaddr = NULL;
>  }
>  
>  static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
>  {
>  	struct vb2_vmalloc_buf *buf = mem_priv;
> +	struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buf->vaddr);
>  
>  	if (buf->vaddr)
> -		dma_buf_vunmap(buf->dbuf, buf->vaddr);
> +		dma_buf_vunmap(buf->dbuf, &map);
>  
>  	kfree(buf);
>  }
> diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
> index 5125f84c28f6..0991a47a1567 100644
> --- a/include/drm/drm_prime.h
> +++ b/include/drm/drm_prime.h
> @@ -84,7 +84,7 @@ void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
>  			   struct sg_table *sgt,
>  			   enum dma_data_direction dir);
>  int drm_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map);
> -void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void *vaddr);
> +void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, struct dma_buf_map *map);
>  
>  int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
>  int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma);
> diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> index 6b4f6e0e8b5d..303e1363b221 100644
> --- a/include/linux/dma-buf-map.h
> +++ b/include/linux/dma-buf-map.h
> @@ -28,6 +28,16 @@ struct dma_buf_map {
>  	bool is_iomem;
>  };
>  
> +/**
> + * DMA_BUF_MAP_INIT_VADDR - Initializes struct dma_buf_map to an address in system memory
> + * @vaddr:	A system-memory address
> + */
> +#define DMA_BUF_MAP_INIT_VADDR(vaddr_) \
> +	{ \
> +		.vaddr = (vaddr_), \
> +		.is_iomem = false, \
> +	}
> +
>  /**
>   * dma_buf_map_set_vaddr - Sets a dma-buf mapping structure to an address in system memory
>   * @map:	The dma-buf mapping structure
> @@ -41,10 +51,26 @@ static inline void dma_buf_map_set_vaddr(struct dma_buf_map *map, void *vaddr)
>  	map->is_iomem = false;
>  }
>  
> -/* API transition helper */
> -static inline bool dma_buf_map_is_vaddr(const struct dma_buf_map *map, const void *vaddr)
> +/**
> + * dma_buf_map_is_equal - Compares two dma-buf mapping structures for equality
> + * @lhs:	The dma-buf mapping structure
> + * @rhs:	A dma-buf mapping structure to compare with
> + *
> + * Two dma-buf mapping structures are equal if they both refer to the same type of memory
> + * and to the same address within that memory.
> + *
> + * Returns:
> + * True is both structures are equal, or false otherwise.
> + */
> +static inline bool dma_buf_map_is_equal(const struct dma_buf_map *lhs,
> +					const struct dma_buf_map *rhs)
>  {
> -	return !map->is_iomem && (map->vaddr == vaddr);
> +	if (lhs->is_iomem != rhs->is_iomem)
> +		return false;
> +	else if (lhs->is_iomem)
> +		return lhs->vaddr_iomem == rhs->vaddr_iomem;
> +	else
> +		return lhs->vaddr == rhs->vaddr;
>  }
>  
>  /**
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 7237997cfa38..cf77cc15f4ba 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -267,7 +267,7 @@ struct dma_buf_ops {
>  	int (*mmap)(struct dma_buf *, struct vm_area_struct *vma);
>  
>  	int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map);
> -	void (*vunmap)(struct dma_buf *, void *vaddr);
> +	void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map *map);
>  };
>  
>  /**
> @@ -504,5 +504,5 @@ int dma_buf_end_cpu_access(struct dma_buf *dma_buf,
>  int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *,
>  		 unsigned long);
>  int dma_buf_vmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
> -void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr);
> +void dma_buf_vunmap(struct dma_buf *dmabuf, struct dma_buf_map *map);
>  #endif /* __DMA_BUF_H__ */
> -- 
> 2.28.0
> 

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



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux