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

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

+}