[PATCH 13/14] drm/exynos: use prime helpers

Inki Dae inki.dae at samsung.com
Sat Aug 15 22:26:31 PDT 2015


On 2015년 07월 28일 17:53, Joonyoung Shim wrote:
> The dma-buf codes of exynos drm is almost same with prime helpers. A
> difference is that consider DMA_NONE when import dma-buf, but it's wrong
> and we don't consider it any more, so we can use prime interface.

I think it's good idea to use prime helpers. However, I'm not sure of
there really is no any corner case the prime helper doesn't consider for
Vendor SoC yet. So let's have more reviews about this.

Thanks,
Inki Dae

> 
> Signed-off-by: Joonyoung Shim <jy0922.shim at samsung.com>
> ---
>  drivers/gpu/drm/exynos/Makefile            |   2 +-
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 289 -----------------------------
>  drivers/gpu/drm/exynos/exynos_drm_dmabuf.h |  20 --
>  drivers/gpu/drm/exynos/exynos_drm_drv.c    |   9 +-
>  drivers/gpu/drm/exynos/exynos_drm_gem.c    |  79 ++++++++
>  drivers/gpu/drm/exynos/exynos_drm_gem.h    |   9 +
>  6 files changed, 95 insertions(+), 313 deletions(-)
>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
> 
> diff --git a/drivers/gpu/drm/exynos/Makefile b/drivers/gpu/drm/exynos/Makefile
> index 7de0b10..95d0306 100644
> --- a/drivers/gpu/drm/exynos/Makefile
> +++ b/drivers/gpu/drm/exynos/Makefile
> @@ -6,7 +6,7 @@ ccflags-y := -Iinclude/drm -Idrivers/gpu/drm/exynos
>  exynosdrm-y := exynos_drm_drv.o exynos_drm_encoder.o \
>  		exynos_drm_crtc.o exynos_drm_fbdev.o exynos_drm_fb.o \
>  		exynos_drm_buf.o exynos_drm_gem.o exynos_drm_core.o \
> -		exynos_drm_plane.o exynos_drm_dmabuf.o
> +		exynos_drm_plane.o
>  
>  exynosdrm-$(CONFIG_DRM_EXYNOS_IOMMU) += exynos_drm_iommu.o
>  exynosdrm-$(CONFIG_DRM_EXYNOS_FIMD)	+= exynos_drm_fimd.o
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> deleted file mode 100644
> index 619ecdd..0000000
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> +++ /dev/null
> @@ -1,289 +0,0 @@
> -/* exynos_drm_dmabuf.c
> - *
> - * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> - * Author: Inki Dae <inki.dae at samsung.com>
> - *
> - * This program is free software; you can redistribute  it and/or modify it
> - * under  the terms of  the GNU General  Public License as published by the
> - * Free Software Foundation;  either version 2 of the  License, or (at your
> - * option) any later version.
> - */
> -
> -#include <drm/drmP.h>
> -#include <drm/exynos_drm.h>
> -#include "exynos_drm_dmabuf.h"
> -#include "exynos_drm_drv.h"
> -#include "exynos_drm_gem.h"
> -
> -#include <linux/dma-buf.h>
> -
> -struct exynos_drm_dmabuf_attachment {
> -	struct sg_table *sgt;
> -	enum dma_data_direction dir;
> -	bool is_mapped;
> -};
> -
> -static struct exynos_drm_gem_obj *dma_buf_to_obj(struct dma_buf *buf)
> -{
> -	return to_exynos_gem_obj(buf->priv);
> -}
> -
> -static int exynos_gem_attach_dma_buf(struct dma_buf *dmabuf,
> -					struct device *dev,
> -					struct dma_buf_attachment *attach)
> -{
> -	struct exynos_drm_dmabuf_attachment *exynos_attach;
> -
> -	exynos_attach = kzalloc(sizeof(*exynos_attach), GFP_KERNEL);
> -	if (!exynos_attach)
> -		return -ENOMEM;
> -
> -	exynos_attach->dir = DMA_NONE;
> -	attach->priv = exynos_attach;
> -
> -	return 0;
> -}
> -
> -static void exynos_gem_detach_dma_buf(struct dma_buf *dmabuf,
> -					struct dma_buf_attachment *attach)
> -{
> -	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
> -	struct sg_table *sgt;
> -
> -	if (!exynos_attach)
> -		return;
> -
> -	sgt = exynos_attach->sgt;
> -	if (sgt) {
> -		if (exynos_attach->dir != DMA_NONE)
> -			dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents,
> -					exynos_attach->dir);
> -		sg_free_table(sgt);
> -	}
> -
> -	kfree(sgt);
> -	kfree(exynos_attach);
> -	attach->priv = NULL;
> -}
> -
> -static struct sg_table *
> -		exynos_gem_map_dma_buf(struct dma_buf_attachment *attach,
> -					enum dma_data_direction dir)
> -{
> -	struct exynos_drm_dmabuf_attachment *exynos_attach = attach->priv;
> -	struct exynos_drm_gem_obj *gem_obj = dma_buf_to_obj(attach->dmabuf);
> -	struct exynos_drm_gem_buf *buf;
> -	struct sg_table *sgt;
> -	int npages;
> -
> -	/* just return current sgt if already requested. */
> -	if (exynos_attach->dir == dir && exynos_attach->is_mapped)
> -		return exynos_attach->sgt;
> -
> -	buf = gem_obj->buffer;
> -	if (!buf) {
> -		DRM_ERROR("buffer is null.\n");
> -		return ERR_PTR(-ENOMEM);
> -	}
> -
> -	npages = buf->size >> PAGE_SHIFT;
> -
> -	sgt = drm_prime_pages_to_sg(buf->pages, npages);
> -	if (IS_ERR(sgt))
> -		goto err;
> -
> -	if (dir != DMA_NONE) {
> -		if (!dma_map_sg(attach->dev, sgt->sgl, sgt->orig_nents, dir)) {
> -			DRM_ERROR("failed to map sgl with iommu.\n");
> -			sg_free_table(sgt);
> -			sgt = ERR_PTR(-EIO);
> -			goto err;
> -		}
> -	}
> -
> -	exynos_attach->is_mapped = true;
> -	exynos_attach->sgt = sgt;
> -	exynos_attach->dir = dir;
> -	attach->priv = exynos_attach;
> -
> -	DRM_DEBUG_PRIME("buffer size = 0x%lx\n", buf->size);
> -
> -err:
> -	return sgt;
> -}
> -
> -static void exynos_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
> -						struct sg_table *sgt,
> -						enum dma_data_direction dir)
> -{
> -	/* Nothing to do. */
> -}
> -
> -static void *exynos_gem_dmabuf_kmap_atomic(struct dma_buf *dma_buf,
> -						unsigned long page_num)
> -{
> -	/* TODO */
> -
> -	return NULL;
> -}
> -
> -static void exynos_gem_dmabuf_kunmap_atomic(struct dma_buf *dma_buf,
> -						unsigned long page_num,
> -						void *addr)
> -{
> -	/* TODO */
> -}
> -
> -static void *exynos_gem_dmabuf_kmap(struct dma_buf *dma_buf,
> -					unsigned long page_num)
> -{
> -	/* TODO */
> -
> -	return NULL;
> -}
> -
> -static void exynos_gem_dmabuf_kunmap(struct dma_buf *dma_buf,
> -					unsigned long page_num, void *addr)
> -{
> -	/* TODO */
> -}
> -
> -static int exynos_gem_dmabuf_mmap(struct dma_buf *dma_buf,
> -	struct vm_area_struct *vma)
> -{
> -	return -ENOTTY;
> -}
> -
> -static struct dma_buf_ops exynos_dmabuf_ops = {
> -	.attach			= exynos_gem_attach_dma_buf,
> -	.detach			= exynos_gem_detach_dma_buf,
> -	.map_dma_buf		= exynos_gem_map_dma_buf,
> -	.unmap_dma_buf		= exynos_gem_unmap_dma_buf,
> -	.kmap			= exynos_gem_dmabuf_kmap,
> -	.kmap_atomic		= exynos_gem_dmabuf_kmap_atomic,
> -	.kunmap			= exynos_gem_dmabuf_kunmap,
> -	.kunmap_atomic		= exynos_gem_dmabuf_kunmap_atomic,
> -	.mmap			= exynos_gem_dmabuf_mmap,
> -	.release		= drm_gem_dmabuf_release,
> -};
> -
> -struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
> -				struct drm_gem_object *obj, int flags)
> -{
> -	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
> -	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> -
> -	exp_info.ops = &exynos_dmabuf_ops;
> -	exp_info.size = exynos_gem_obj->base.size;
> -	exp_info.flags = flags;
> -	exp_info.priv = obj;
> -
> -	return dma_buf_export(&exp_info);
> -}
> -
> -struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
> -				struct dma_buf *dma_buf)
> -{
> -	struct dma_buf_attachment *attach;
> -	struct sg_table *sgt;
> -	struct scatterlist *sgl;
> -	struct exynos_drm_gem_obj *exynos_gem_obj;
> -	struct exynos_drm_gem_buf *buffer;
> -	int npages;
> -	int ret;
> -
> -	/* is this one of own objects? */
> -	if (dma_buf->ops == &exynos_dmabuf_ops) {
> -		struct drm_gem_object *obj;
> -
> -		obj = dma_buf->priv;
> -
> -		/* is it from our device? */
> -		if (obj->dev == drm_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;
> -		}
> -	}
> -
> -	attach = dma_buf_attach(dma_buf, drm_dev->dev);
> -	if (IS_ERR(attach))
> -		return ERR_PTR(-EINVAL);
> -
> -	get_dma_buf(dma_buf);
> -
> -	sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> -	if (IS_ERR(sgt)) {
> -		ret = PTR_ERR(sgt);
> -		goto err_buf_detach;
> -	}
> -
> -	buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
> -	if (!buffer) {
> -		ret = -ENOMEM;
> -		goto err_unmap_attach;
> -	}
> -
> -	exynos_gem_obj = exynos_drm_gem_init(drm_dev, dma_buf->size);
> -	if (!exynos_gem_obj) {
> -		ret = -ENOMEM;
> -		goto err_free_buffer;
> -	}
> -
> -	sgl = sgt->sgl;
> -
> -	buffer->size = dma_buf->size;
> -	buffer->dma_addr = sg_dma_address(sgl);
> -
> -	npages = dma_buf->size >> PAGE_SHIFT;
> -	buffer->pages = drm_malloc_ab(npages, sizeof(struct page *));
> -	if (!buffer->pages) {
> -		ret = -ENOMEM;
> -		goto err_free_gem;
> -	}
> -
> -	ret = drm_prime_sg_to_page_addr_arrays(sgt, buffer->pages, NULL,
> -			npages);
> -	if (ret < 0) {
> -		drm_free_large(buffer->pages);
> -		goto err_free_gem;
> -	}
> -
> -	if (sgt->nents == 1) {
> -		/* always physically continuous memory if sgt->nents is 1. */
> -		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
> -	} else {
> -		/*
> -		 * this case could be CONTIG or NONCONTIG type but for now
> -		 * sets NONCONTIG.
> -		 * TODO. we have to find a way that exporter can notify
> -		 * the type of its own buffer to importer.
> -		 */
> -		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
> -	}
> -
> -	exynos_gem_obj->buffer = buffer;
> -	exynos_gem_obj->base.import_attach = attach;
> -
> -	DRM_DEBUG_PRIME("dma_addr = %pad, size = 0x%lx\n", &buffer->dma_addr,
> -								buffer->size);
> -
> -	return &exynos_gem_obj->base;
> -
> -err_free_gem:
> -	drm_gem_object_release(&exynos_gem_obj->base);
> -	kfree(exynos_gem_obj);
> -err_free_buffer:
> -	kfree(buffer);
> -	buffer = NULL;
> -err_unmap_attach:
> -	dma_buf_unmap_attachment(attach, sgt, DMA_BIDIRECTIONAL);
> -err_buf_detach:
> -	dma_buf_detach(dma_buf, attach);
> -	dma_buf_put(dma_buf);
> -
> -	return ERR_PTR(ret);
> -}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
> deleted file mode 100644
> index 886de9f..0000000
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* exynos_drm_dmabuf.h
> - *
> - * Copyright (c) 2012 Samsung Electronics Co., Ltd.
> - * Author: Inki Dae <inki.dae at samsung.com>
> - *
> - * This program is free software; you can redistribute  it and/or modify it
> - * under  the terms of  the GNU General  Public License as published by the
> - * Free Software Foundation;  either version 2 of the  License, or (at your
> - * option) any later version.
> - */
> -
> -#ifndef _EXYNOS_DRM_DMABUF_H_
> -#define _EXYNOS_DRM_DMABUF_H_
> -
> -struct dma_buf *exynos_dmabuf_prime_export(struct drm_device *drm_dev,
> -				struct drm_gem_object *obj, int flags);
> -
> -struct drm_gem_object *exynos_dmabuf_prime_import(struct drm_device *drm_dev,
> -						struct dma_buf *dma_buf);
> -#endif
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index f1d6966..e9ddaa2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -27,7 +27,6 @@
>  #include "exynos_drm_gem.h"
>  #include "exynos_drm_plane.h"
>  #include "exynos_drm_vidi.h"
> -#include "exynos_drm_dmabuf.h"
>  #include "exynos_drm_g2d.h"
>  #include "exynos_drm_ipp.h"
>  #include "exynos_drm_iommu.h"
> @@ -297,8 +296,12 @@ static struct drm_driver exynos_drm_driver = {
>  	.dumb_destroy		= drm_gem_dumb_destroy,
>  	.prime_handle_to_fd	= drm_gem_prime_handle_to_fd,
>  	.prime_fd_to_handle	= drm_gem_prime_fd_to_handle,
> -	.gem_prime_export	= exynos_dmabuf_prime_export,
> -	.gem_prime_import	= exynos_dmabuf_prime_import,
> +	.gem_prime_export	= drm_gem_prime_export,
> +	.gem_prime_import	= drm_gem_prime_import,
> +	.gem_prime_get_sg_table	= exynos_drm_gem_prime_get_sg_table,
> +	.gem_prime_import_sg_table	= exynos_drm_gem_prime_import_sg_table,
> +	.gem_prime_vmap		= exynos_drm_gem_prime_vmap,
> +	.gem_prime_vunmap	= exynos_drm_gem_prime_vunmap,
>  	.ioctls			= exynos_ioctls,
>  	.num_ioctls		= ARRAY_SIZE(exynos_ioctls),
>  	.fops			= &exynos_drm_driver_fops,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> index 9df100f..7d4bf9d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
> @@ -13,6 +13,7 @@
>  #include <drm/drm_vma_manager.h>
>  
>  #include <linux/shmem_fs.h>
> +#include <linux/dma-buf.h>
>  #include <drm/exynos_drm.h>
>  
>  #include "exynos_drm_drv.h"
> @@ -580,3 +581,81 @@ err_close_vm:
>  
>  	return ret;
>  }
> +
> +/* low-level interface prime helpers */
> +struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj)
> +{
> +	struct exynos_drm_gem_obj *exynos_gem_obj = to_exynos_gem_obj(obj);
> +	struct exynos_drm_gem_buf *buf = exynos_gem_obj->buffer;
> +	int npages;
> +
> +	npages = buf->size >> PAGE_SHIFT;
> +
> +	return drm_prime_pages_to_sg(buf->pages, npages);
> +}
> +
> +struct drm_gem_object *
> +exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> +				     struct dma_buf_attachment *attach,
> +				     struct sg_table *sgt)
> +{
> +	struct exynos_drm_gem_obj *exynos_gem_obj;
> +	struct exynos_drm_gem_buf *buf;
> +	int npages;
> +	int ret;
> +
> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	buf->size = attach->dmabuf->size;
> +	buf->dma_addr = sg_dma_address(sgt->sgl);
> +
> +	npages = buf->size >> PAGE_SHIFT;
> +	buf->pages = drm_malloc_ab(npages, sizeof(struct page *));
> +	if (!buf->pages) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	ret = drm_prime_sg_to_page_addr_arrays(sgt, buf->pages, NULL, npages);
> +	if (ret < 0)
> +		goto err_free_large;
> +
> +	exynos_gem_obj = exynos_drm_gem_init(dev, buf->size);
> +	if (IS_ERR(exynos_gem_obj)) {
> +		ret = PTR_ERR(exynos_gem_obj);
> +		goto err;
> +	}
> +
> +	if (sgt->nents == 1) {
> +		/* always physically continuous memory if sgt->nents is 1. */
> +		exynos_gem_obj->flags |= EXYNOS_BO_CONTIG;
> +	} else {
> +		/*
> +		 * this case could be CONTIG or NONCONTIG type but for now
> +		 * sets NONCONTIG.
> +		 * TODO. we have to find a way that exporter can notify
> +		 * the type of its own buffer to importer.
> +		 */
> +		exynos_gem_obj->flags |= EXYNOS_BO_NONCONTIG;
> +	}
> +
> +	return &exynos_gem_obj->base;
> +
> +err_free_large:
> +	drm_free_large(buf->pages);
> +err:
> +	kfree(buf);
> +	return ERR_PTR(ret);
> +}
> +
> +void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj)
> +{
> +	return NULL;
> +}
> +
> +void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
> +{
> +	/* Nothing to do */
> +}
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index 49b5ef1..5e20da6 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -168,4 +168,13 @@ void exynos_gem_unmap_sgt_from_dma(struct drm_device *drm_dev,
>  				struct sg_table *sgt,
>  				enum dma_data_direction dir);
>  
> +/* low-level interface prime helpers */
> +struct sg_table *exynos_drm_gem_prime_get_sg_table(struct drm_gem_object *obj);
> +struct drm_gem_object *
> +exynos_drm_gem_prime_import_sg_table(struct drm_device *dev,
> +				     struct dma_buf_attachment *attach,
> +				     struct sg_table *sgt);
> +void *exynos_drm_gem_prime_vmap(struct drm_gem_object *obj);
> +void exynos_drm_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr);
> +
>  #endif
> 



More information about the dri-devel mailing list