Re: [PATCH v2 2/8] drm/etnaviv: split out cmdbuf mapping into address space

On Fri, 2019-07-05 at 19:17 +0200, Lucas Stach wrote:

This allows to decouple the cmdbuf suballocator create and mapping the region into the GPU address space. Allowing multiple AS to share a single cmdbuf suballoc.

Signed-off-by: Lucas Stach l.stach@pengutronix.de

drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 23 ++++---- drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 35 ++++++------ drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h | 11 +++- drivers/gpu/drm/etnaviv/etnaviv_dump.c | 6 +- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 19 +++++-- drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 3 +- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 70 +++++++++++++++--------- drivers/gpu/drm/etnaviv/etnaviv_mmu.h | 12 ++-- 8 files changed, 114 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c index fe0d2d67007d..6400a88cd778 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c @@ -118,7 +118,8 @@ static void etnaviv_buffer_dump(struct etnaviv_gpu *gpu, u32 *ptr = buf->vaddr + off;

dev_info(gpu->dev, "virt %p phys 0x%08x free 0x%08x\n",

• 	ptr, etnaviv_cmdbuf_get_va(buf) + off, size - len * 4 - off);
  

• 	ptr, etnaviv_cmdbuf_get_va(buf, &gpu->cmdbuf_mapping) +
  

• 	off, size - len * 4 - off);
  
  

  print_hex_dump(KERN_INFO, "cmd ", DUMP_PREFIX_OFFSET, 16, 4, ptr, len * 4, 0);

@@ -151,7 +152,8 @@ static u32 etnaviv_buffer_reserve(struct etnaviv_gpu *gpu, if (buffer->user_size + cmd_dwords * sizeof(u64) > buffer->size) buffer->user_size = 0;

• return etnaviv_cmdbuf_get_va(buffer) + buffer->user_size;

• return etnaviv_cmdbuf_get_va(buffer, &gpu->cmdbuf_mapping) +

•       buffer->user_size;
  

}

u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu) @@ -164,8 +166,8 @@ u16 etnaviv_buffer_init(struct etnaviv_gpu *gpu) buffer->user_size = 0;

CMD_WAIT(buffer);

• CMD_LINK(buffer, 2, etnaviv_cmdbuf_get_va(buffer) +

•  buffer->user_size - 4);
  

• CMD_LINK(buffer, 2, etnaviv_cmdbuf_get_va(buffer, &gpu->cmdbuf_mapping)

•  + buffer->user_size - 4);
  
  

  return buffer->user_size / 8;

} @@ -291,8 +293,8 @@ void etnaviv_sync_point_queue(struct etnaviv_gpu *gpu, unsigned int event)

/* Append waitlink */ CMD_WAIT(buffer);

• CMD_LINK(buffer, 2, etnaviv_cmdbuf_get_va(buffer) +

• 	    buffer->user_size - 4);
  

• CMD_LINK(buffer, 2, etnaviv_cmdbuf_get_va(buffer, &gpu->cmdbuf_mapping)

•  + buffer->user_size - 4);
  
  

  /*

  • Kick off the 'sync point' command by replacing the previous

@@ -319,7 +321,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state, if (drm_debug & DRM_UT_DRIVER) etnaviv_buffer_dump(gpu, buffer, 0, 0x50);

• link_target = etnaviv_cmdbuf_get_va(cmdbuf);

• link_target = etnaviv_cmdbuf_get_va(cmdbuf, &gpu->cmdbuf_mapping); link_dwords = cmdbuf->size / 8;

  /*

@@ -412,12 +414,13 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state, CMD_LOAD_STATE(buffer, VIVS_GL_EVENT, VIVS_GL_EVENT_EVENT_ID(event) | VIVS_GL_EVENT_FROM_PE); CMD_WAIT(buffer);

• CMD_LINK(buffer, 2, etnaviv_cmdbuf_get_va(buffer) +

• 	    buffer->user_size - 4);
  

• CMD_LINK(buffer, 2, etnaviv_cmdbuf_get_va(buffer, &gpu->cmdbuf_mapping)

•  + buffer->user_size - 4);
  
  

  if (drm_debug & DRM_UT_DRIVER) pr_info("stream link to 0x%08x @ 0x%08x %p\n",

• 	return_target, etnaviv_cmdbuf_get_va(cmdbuf),
  

• 	return_target,
  

• 	etnaviv_cmdbuf_get_va(cmdbuf, &gpu->cmdbuf_mapping),
  cmdbuf->vaddr);
  
  

  if (drm_debug & DRM_UT_DRIVER) {

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c index 7b77992f31c4..8915d9d056a6 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c @@ -8,6 +8,7 @@ #include <drm/drm_mm.h>

#include "etnaviv_cmdbuf.h" +#include "etnaviv_gem.h" #include "etnaviv_gpu.h" #include "etnaviv_mmu.h"

@@ -21,10 +22,6 @@ struct etnaviv_cmdbuf_suballoc { void *vaddr; dma_addr_t paddr;

• /* GPU mapping */
• u32 iova;
• struct drm_mm_node vram_node; /* only used on MMUv2 */
• /* allocation management */ struct mutex lock; DECLARE_BITMAP(granule_map, SUBALLOC_GRANULES);

@@ -53,26 +50,31 @@ etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu) goto free_suballoc; }

• ret = etnaviv_iommu_get_suballoc_va(gpu, suballoc->paddr,

• 			    &suballoc->vram_node, SUBALLOC_SIZE,
  

• 			    &suballoc->iova);
  

• if (ret)

• goto free_dma;
  

• return suballoc;

-free_dma:

• dma_free_wc(gpu->dev, SUBALLOC_SIZE, suballoc->vaddr, suballoc->paddr);

free_suballoc: kfree(suballoc);

return ERR_PTR(ret); }

+int etnaviv_cmdbuf_suballoc_map(struct etnaviv_cmdbuf_suballoc *suballoc,

• 		struct etnaviv_iommu *mmu,
  

• 		struct etnaviv_vram_mapping *mapping,
  

• 		u32 memory_base)
  

+{

• return etnaviv_iommu_get_suballoc_va(mmu, mapping, memory_base,

• 			     suballoc->paddr, SUBALLOC_SIZE);
  

+}

+void etnaviv_cmdbuf_suballoc_unmap(struct etnaviv_iommu *mmu,

• 		   struct etnaviv_vram_mapping *mapping)
  

+{

• etnaviv_iommu_put_suballoc_va(mmu, mapping);

+}

void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc) {

• etnaviv_iommu_put_suballoc_va(suballoc->gpu, &suballoc->vram_node,

• 		      SUBALLOC_SIZE, suballoc->iova);
  

  dma_free_wc(suballoc->gpu->dev, SUBALLOC_SIZE, suballoc->vaddr, suballoc->paddr); kfree(suballoc);

@@ -126,9 +128,10 @@ void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf) wake_up_all(&suballoc->free_event); }

-u32 etnaviv_cmdbuf_get_va(struct etnaviv_cmdbuf *buf) +u32 etnaviv_cmdbuf_get_va(struct etnaviv_cmdbuf *buf,

• 	  struct etnaviv_vram_mapping *mapping)
  

{

• return buf->suballoc->iova + buf->suballoc_offset;

• return mapping->iova + buf->suballoc_offset;

}

dma_addr_t etnaviv_cmdbuf_get_pa(struct etnaviv_cmdbuf *buf) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h index 49908797456e..11d95f05c017 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.h @@ -9,6 +9,8 @@ #include <linux/types.h>

struct etnaviv_gpu; +struct etnaviv_iommu; +struct etnaviv_vram_mapping; struct etnaviv_cmdbuf_suballoc;

struct etnaviv_cmdbuf { @@ -24,13 +26,20 @@ struct etnaviv_cmdbuf { struct etnaviv_cmdbuf_suballoc * etnaviv_cmdbuf_suballoc_new(struct etnaviv_gpu * gpu); void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc); +int etnaviv_cmdbuf_suballoc_map(struct etnaviv_cmdbuf_suballoc *suballoc,

• 		struct etnaviv_iommu *mmu,
  

• 		struct etnaviv_vram_mapping *mapping,
  

• 		u32 memory_base);
  

+void etnaviv_cmdbuf_suballoc_unmap(struct etnaviv_iommu *mmu,

• 		   struct etnaviv_vram_mapping *mapping);
  
  
  

int etnaviv_cmdbuf_init(struct etnaviv_cmdbuf_suballoc *suballoc, struct etnaviv_cmdbuf *cmdbuf, u32 size); void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf);

-u32 etnaviv_cmdbuf_get_va(struct etnaviv_cmdbuf *buf); +u32 etnaviv_cmdbuf_get_va(struct etnaviv_cmdbuf *buf,

• 	  struct etnaviv_vram_mapping *mapping);
  

dma_addr_t etnaviv_cmdbuf_get_pa(struct etnaviv_cmdbuf *buf);

#endif /* __ETNAVIV_CMDBUF_H__ */ diff --git a/drivers/gpu/drm/etnaviv/etnaviv_dump.c b/drivers/gpu/drm/etnaviv/etnaviv_dump.c index 0aa8cde68593..13a63d9dcf54 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_dump.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_dump.c @@ -173,11 +173,13 @@ void etnaviv_core_dump(struct etnaviv_gem_submit *submit)

etnaviv_core_dump_mem(&iter, ETDUMP_BUF_RING, gpu->buffer.vaddr, gpu->buffer.size,

• 	      etnaviv_cmdbuf_get_va(&gpu->buffer));
  

• 	      etnaviv_cmdbuf_get_va(&gpu->buffer,
  

• 				    &gpu->cmdbuf_mapping));
  
  

  etnaviv_core_dump_mem(&iter, ETDUMP_BUF_CMD, submit->cmdbuf.vaddr, submit->cmdbuf.size,

• 	      etnaviv_cmdbuf_get_va(&submit->cmdbuf));
  

• 	      etnaviv_cmdbuf_get_va(&submit->cmdbuf,
  

• 				    &gpu->cmdbuf_mapping));
  
  

  /* Reserve space for the bomap */ if (n_bomap_pages) {

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index e84a0ed904aa..62a38a63e4eb 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -687,8 +687,8 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu) prefetch = etnaviv_buffer_init(gpu);

gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);

• etnaviv_gpu_start_fe(gpu, etnaviv_cmdbuf_get_va(&gpu->buffer),

• 	     prefetch);
  

• etnaviv_gpu_start_fe(gpu, etnaviv_cmdbuf_get_va(&gpu->buffer,

• 	     &gpu->cmdbuf_mapping), prefetch);
  

}

int etnaviv_gpu_init(struct etnaviv_gpu *gpu) @@ -767,16 +767,24 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu) goto destroy_iommu; }

• ret = etnaviv_cmdbuf_suballoc_map(gpu->cmdbuf_suballoc, gpu->mmu,

• 			  &gpu->cmdbuf_mapping,
  

• 			  gpu->memory_base);
  

• if (ret) {

• dev_err(gpu->dev, "failed to map cmdbuf suballoc\n");
  

• goto destroy_suballoc;
  

• }
• /* Create buffer: */ ret = etnaviv_cmdbuf_init(gpu->cmdbuf_suballoc, &gpu->buffer, PAGE_SIZE); if (ret) { dev_err(gpu->dev, "could not create command buffer\n");

• goto destroy_suballoc;
  

• goto unmap_suballoc;
  

  }

  if (gpu->mmu->version == ETNAVIV_IOMMU_V1 &&

•    etnaviv_cmdbuf_get_va(&gpu->buffer) > 0x80000000) {
  

•    etnaviv_cmdbuf_get_va(&gpu->buffer, &gpu->cmdbuf_mapping) > 0x80000000) {
  

  ret = -EINVAL; dev_err(gpu->dev, "command buffer outside valid memory window\n");

@@ -805,6 +813,8 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)

free_buffer: etnaviv_cmdbuf_free(&gpu->buffer); +unmap_suballoc:

• etnaviv_cmdbuf_suballoc_unmap(gpu->mmu, &gpu->cmdbuf_mapping);

destroy_suballoc: etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); destroy_iommu: @@ -1681,6 +1691,7 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master,

if (gpu->initialized) { etnaviv_cmdbuf_free(&gpu->buffer);

• etnaviv_cmdbuf_suballoc_unmap(gpu->mmu, &gpu->cmdbuf_mapping);
  

  etnaviv_cmdbuf_suballoc_destroy(gpu->cmdbuf_suballoc); etnaviv_iommu_destroy(gpu->mmu); gpu->initialized = false;

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h index b06c7c98d522..6a6add350d2d 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h @@ -7,6 +7,7 @@ #define __ETNAVIV_GPU_H__

#include "etnaviv_cmdbuf.h" +#include "etnaviv_gem.h" #include "etnaviv_drv.h"

struct etnaviv_gem_submit; @@ -84,7 +85,6 @@ struct etnaviv_event { };

struct etnaviv_cmdbuf_suballoc; -struct etnaviv_cmdbuf; struct regulator; struct clk;

@@ -102,6 +102,7 @@ struct etnaviv_gpu { bool initialized;

/* 'ring'-buffer: */

• struct etnaviv_vram_mapping cmdbuf_mapping; struct etnaviv_cmdbuf buffer; int exec_state;

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c index 731275999a57..dd81376724d7 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -334,52 +334,72 @@ void etnaviv_iommu_restore(struct etnaviv_gpu *gpu) etnaviv_iommuv2_restore(gpu); }

-int etnaviv_iommu_get_suballoc_va(struct etnaviv_gpu *gpu, dma_addr_t paddr,

• 		  struct drm_mm_node *vram_node, size_t size,
  

• 		  u32 *iova)
  

+int etnaviv_iommu_get_suballoc_va(struct etnaviv_iommu *mmu,

• 		  struct etnaviv_vram_mapping *mapping,
  

• 		  u32 memory_base, dma_addr_t paddr,
  

• 		  size_t size)
  

{

• struct etnaviv_iommu *mmu = gpu->mmu;

• mutex_lock(&mmu->lock);

• /*

• * For MMUv1 we don't add the suballoc region to the pagetables, as
  

• * those GPUs can only work with cmdbufs accessed through the linear
  

• * window. Instead we manufacture a mapping to make it look uniform
  

• * to the upper layers.
  

• */
  

  if (mmu->version == ETNAVIV_IOMMU_V1) {

• *iova = paddr - gpu->memory_base;
  

• return 0;
  

• mapping->iova = paddr - memory_base;
  

• list_add_tail(&mapping->mmu_node, &mmu->mappings);
  

  } else {

[...]

• list_add_tail(&mapping->mmu_node, &mmu->mappings);
  

This is the same in both branches and could be moved below.

• mmu->need_flush = true;
  

  }
• mapping->use = 1;
• mutex_unlock(&mmu->lock);
• return 0;

}

[...]

Reviewed-by: Philipp Zabel p.zabel@pengutronix.de

regards Philipp