[PATCH 3/3] drm/amdgpu: Move to gtt before cpu accesses dma buf.

[Christian König]

Am 04.01.2018 um 22:12 schrieb Samuel Li:
> To improve cpu read performance. This is implemented for APUs currently.
>
> v2: Adapt to change https://lists.freedesktop.org/archives/amd-gfx/2017-October/015174.html
> v3: Adapt to change "forward begin_cpu_access callback to drivers"
> v4: Instead of v3, reuse drm_gem dmabuf_ops here. Also some minor fixes as suggested.
>
> Change-Id: I7a583e23a9ee706e0edd2a46f4e4186a609368e3
>
> Signed-off-by: Samuel Li <Samuel.Li at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  2 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c | 67 +++++++++++++++++++++++++++++++
>   3 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index f8657c3..193db70 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -417,6 +417,8 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev,
>   struct dma_buf *amdgpu_gem_prime_export(struct drm_device *dev,
>   					struct drm_gem_object *gobj,
>   					int flags);
> +struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
> +					    struct dma_buf *dma_buf);
>   int amdgpu_gem_prime_pin(struct drm_gem_object *obj);
>   void amdgpu_gem_prime_unpin(struct drm_gem_object *obj);
>   struct reservation_object *amdgpu_gem_prime_res_obj(struct drm_gem_object *);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 31383e0..df30b08 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -868,7 +868,7 @@ static struct drm_driver kms_driver = {
>   	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>   	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>   	.gem_prime_export = amdgpu_gem_prime_export,
> -	.gem_prime_import = drm_gem_prime_import,
> +	.gem_prime_import = amdgpu_gem_prime_import,
>   	.gem_prime_pin = amdgpu_gem_prime_pin,
>   	.gem_prime_unpin = amdgpu_gem_prime_unpin,
>   	.gem_prime_res_obj = amdgpu_gem_prime_res_obj,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> index ae9c106..283b523 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_prime.c
> @@ -26,6 +26,7 @@
>   #include <drm/drmP.h>
>   
>   #include "amdgpu.h"
> +#include "amdgpu_display.h"
>   #include <drm/amdgpu_drm.h>
>   #include <linux/dma-buf.h>
>   
> @@ -164,6 +165,50 @@ struct reservation_object *amdgpu_gem_prime_res_obj(struct drm_gem_object *obj)
>   	return bo->tbo.resv;
>   }
>   
> +static int amdgpu_gem_begin_cpu_access(struct dma_buf *dma_buf,
> +				       enum dma_data_direction direction)
> +{
> +	struct amdgpu_bo *bo = gem_to_amdgpu_bo(dma_buf->priv);
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	struct ttm_operation_ctx ctx = { true, false };
> +	u32 domain = amdgpu_framebuffer_domains(adev);
> +	int ret = 0;

Style nit pick: Drop the initializer and make that the last variable 
declared.

Apart from that the patch is Reviewed-by: Christian König 
<christian.koenig at amd.com>.

Regards,
Christian.

> +	bool reads = (direction == DMA_BIDIRECTIONAL ||
> +		      direction == DMA_FROM_DEVICE);
> +
> +	if (!reads || !(domain & AMDGPU_GEM_DOMAIN_GTT))
> +		return 0;
> +
> +	/* move to gtt */
> +	ret = amdgpu_bo_reserve(bo, false);
> +	if (unlikely(ret != 0))
> +		return ret;
> +
> +	if (!bo->pin_count && (bo->allowed_domains & AMDGPU_GEM_DOMAIN_GTT)) {
> +		amdgpu_ttm_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
> +		ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> +	}
> +
> +	amdgpu_bo_unreserve(bo);
> +	return ret;
> +}
> +
> +static const struct dma_buf_ops amdgpu_dmabuf_ops = {
> +	.attach = drm_gem_map_attach,
> +	.detach = drm_gem_map_detach,
> +	.map_dma_buf = drm_gem_map_dma_buf,
> +	.unmap_dma_buf = drm_gem_unmap_dma_buf,
> +	.release = drm_gem_dmabuf_release,
> +	.begin_cpu_access = amdgpu_gem_begin_cpu_access,
> +	.map = drm_gem_dmabuf_kmap,
> +	.map_atomic = drm_gem_dmabuf_kmap_atomic,
> +	.unmap = drm_gem_dmabuf_kunmap,
> +	.unmap_atomic = drm_gem_dmabuf_kunmap_atomic,
> +	.mmap = drm_gem_dmabuf_mmap,
> +	.vmap = drm_gem_dmabuf_vmap,
> +	.vunmap = drm_gem_dmabuf_vunmap,
> +};
> +
>   struct dma_buf *amdgpu_gem_prime_export(struct drm_device *dev,
>   					struct drm_gem_object *gobj,
>   					int flags)
> @@ -178,5 +223,27 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_device *dev,
>   	buf = drm_gem_prime_export(dev, gobj, flags);
>   	if (!IS_ERR(buf))
>   		buf->file->f_mapping = dev->anon_inode->i_mapping;
> +	buf->ops = &amdgpu_dmabuf_ops;
> +
>   	return buf;
>   }
> +
> +struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
> +					    struct dma_buf *dma_buf)
> +{
> +	struct drm_gem_object *obj;
> +
> +	if (dma_buf->ops == &amdgpu_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_get(obj);
> +			return obj;
> +		}
> +	}
> +
> +	return drm_gem_prime_import(dev, dma_buf);
> +}