[RFC PATCH 0/6] do not store GPU address in TTM

[RFC PATCH 0/6] do not store GPU address in TTM

[Nirmoy Das]

With this patch series I am trying to remove GPU address dependency in
TTM and moving GPU address calculation to individual drm drivers.
This is required[1] to continue the work started by Brian Welty to create
struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory
limits.


I have only manage to test amdgpu driver as I only have GPU for that.
I might be doing something really stupid while calculeting gpu offset for
some of the drivers so please be patient and let me know how can I improve
that.

[1] https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg272238.html

Nirmoy Das (6):
  drm/amdgpu: move ttm bo->offset to amdgpu_bo
  drm/radeon: don't use ttm bo->offset
  drm/vmwgfx: don't use ttm bo->offset
  drm/nouveau: don't use ttm bo->offset
  drm/qxl: don't use ttm bo->offset
  drm/ttm: do not keep GPU dependent addresses

 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 ++++++++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 29 ++++++++++++++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 +--
 drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  6 ++---
 drivers/gpu/drm/nouveau/dispnv04/disp.c     |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 ++---
 drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_abi16.c     |  8 +++---
 drivers/gpu/drm/nouveau/nouveau_bo.c        |  1 +
 drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
 drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++----
 drivers/gpu/drm/qxl/qxl_drv.h               |  6 ++---
 drivers/gpu/drm/qxl/qxl_kms.c               |  3 +++
 drivers/gpu/drm/qxl/qxl_object.h            |  5 ----
 drivers/gpu/drm/qxl/qxl_ttm.c               |  9 -------
 drivers/gpu/drm/radeon/radeon.h             |  1 +
 drivers/gpu/drm/radeon/radeon_object.h      | 16 +++++++++++-
 drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +--
 drivers/gpu/drm/ttm/ttm_bo.c                |  7 -----
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c          |  4 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c     |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c        |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  2 --
 include/drm/ttm/ttm_bo_api.h                |  2 --
 include/drm/ttm/ttm_bo_driver.h             |  1 -
 33 files changed, 99 insertions(+), 72 deletions(-)

--
2.25.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 1/6] drm/amdgpu: move ttm bo->offset to amdgpu_bo

[Nirmoy Das]

GPU address should belong to driver not in memory management.
This patch moves ttm bo.offset and gpu_offset calculation to amdgpu driver.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 ++++++++++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 29 ++++++++++++++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 +--
 5 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6f60a581e3ba..1b1c393587a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -917,7 +917,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
 		bo->pin_count++;
 
 		if (max_offset != 0) {
-			u64 domain_start = bo->tbo.bdev->man[mem_type].gpu_offset;
+			u64 domain_start = amdgpu_ttm_domain_start(adev, mem_type);
 			WARN_ON_ONCE(max_offset <
 				     (amdgpu_bo_gpu_offset(bo) - domain_start));
 		}
@@ -1467,7 +1467,25 @@ u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo)
 	WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM &&
 		     !(bo->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS));
 
-	return amdgpu_gmc_sign_extend(bo->tbo.offset);
+	return amdgpu_bo_gpu_offset_no_check(bo);
+}
+
+/**
+ * amdgpu_bo_gpu_offset_no_check - return GPU offset of bo
+ * @bo:	amdgpu object for which we query the offset
+ *
+ * Returns:
+ * current GPU offset of the object without raising warnings.
+ */
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
+{
+	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+	uint64_t offset;
+
+        offset = (bo->tbo.mem.start << PAGE_SHIFT) +
+		 amdgpu_ttm_domain_start(adev, bo->tbo.mem.mem_type);
+
+	return amdgpu_gmc_sign_extend(offset);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 96d805889e8d..9075ef20ce02 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -288,6 +288,7 @@ int amdgpu_bo_sync_wait_resv(struct amdgpu_device *adev, struct dma_resv *resv,
 			     bool intr);
 int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
 u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo);
+u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
 int amdgpu_bo_validate(struct amdgpu_bo *bo);
 int amdgpu_bo_restore_shadow(struct amdgpu_bo *shadow,
 			     struct dma_fence **fence);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 2c1d1eb1a7e1..4bb02d787945 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -103,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_TT:
 		/* GTT memory  */
 		man->func = &amdgpu_gtt_mgr_func;
-		man->gpu_offset = adev->gmc.gart_start;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -111,7 +110,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &amdgpu_vram_mgr_func;
-		man->gpu_offset = adev->gmc.vram_start;
 		man->flags = TTM_MEMTYPE_FLAG_FIXED |
 			     TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
@@ -122,7 +120,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case AMDGPU_PL_OA:
 		/* On-chip GDS memory*/
 		man->func = &ttm_bo_manager_func;
-		man->gpu_offset = 0;
 		man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
 		man->available_caching = TTM_PL_FLAG_UNCACHED;
 		man->default_caching = TTM_PL_FLAG_UNCACHED;
@@ -270,7 +267,7 @@ static uint64_t amdgpu_mm_node_addr(struct ttm_buffer_object *bo,
 
 	if (mm_node->start != AMDGPU_BO_INVALID_OFFSET) {
 		addr = mm_node->start << PAGE_SHIFT;
-		addr += bo->bdev->man[mem->mem_type].gpu_offset;
+		addr += amdgpu_ttm_domain_start(amdgpu_ttm_adev(bo->bdev), mem->mem_type);
 	}
 	return addr;
 }
@@ -757,6 +754,27 @@ static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
 		(offset >> PAGE_SHIFT);
 }
 
+/**
+ * amdgpu_ttm_domain_start - Returns GPU start address
+ * @adev: amdgpu device object
+ * @type: type of the memory
+ *
+ * Returns:
+ * GPU start address of a memory domain
+ */
+
+uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type)
+{
+	switch(type) {
+	case TTM_PL_TT:
+		return adev->gmc.gart_start;
+	case TTM_PL_VRAM:
+		return adev->gmc.vram_start;
+	}
+
+	return 0;
+}
+
 /*
  * TTM backend functions.
  */
@@ -1136,9 +1154,6 @@ int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 		bo->mem = tmp;
 	}
 
-	bo->offset = (bo->mem.start << PAGE_SHIFT) +
-		bo->bdev->man[bo->mem.mem_type].gpu_offset;
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index f1ebd424510c..aed364d71d58 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -102,6 +102,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
 int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
 int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
+uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type);
 
 #if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR)
 int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index 4cc7881f438c..3c02647e6848 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -140,7 +140,7 @@ static void amdgpu_vm_sdma_copy_ptes(struct amdgpu_vm_update_params *p,
 
 	src += p->num_dw_left * 4;
 
-	pe += amdgpu_gmc_sign_extend(bo->tbo.offset);
+	pe += amdgpu_bo_gpu_offset_no_check(bo);
 	trace_amdgpu_vm_copy_ptes(pe, src, count, p->direct);
 
 	amdgpu_vm_copy_pte(p->adev, ib, pe, src, count);
@@ -167,7 +167,7 @@ static void amdgpu_vm_sdma_set_ptes(struct amdgpu_vm_update_params *p,
 {
 	struct amdgpu_ib *ib = p->job->ibs;
 
-	pe += amdgpu_gmc_sign_extend(bo->tbo.offset);
+	pe += amdgpu_bo_gpu_offset_no_check(bo);
 	trace_amdgpu_vm_set_ptes(pe, addr, count, incr, flags, p->direct);
 	if (count < 3) {
 		amdgpu_vm_write_pte(p->adev, ib, pe, addr | flags,
-- 
2.25.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset

[Nirmoy Das]

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/radeon/radeon.h        |  1 +
 drivers/gpu/drm/radeon/radeon_object.h | 16 +++++++++++++++-
 drivers/gpu/drm/radeon/radeon_ttm.c    |  4 +---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index d59b004f6695..97cfcc2870af 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2823,6 +2823,7 @@ extern void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size
 extern void radeon_program_register_sequence(struct radeon_device *rdev,
 					     const u32 *registers,
 					     const u32 array_size);
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
 
 /*
  * vm
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index d23f2ed4126e..4d37571c7ff5 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo)
  */
 static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
 {
-	return bo->tbo.offset;
+	struct radeon_device *rdev;
+	u64 start = 0;
+
+	rdev = radeon_get_rdev(bo->tbo.bdev);
+
+	switch(bo->tbo.mem.mem_type) {
+	case TTM_PL_TT:
+		start = rdev->mc.gtt_start;
+		break;
+	case TTM_PL_VRAM:
+		start = rdev->mc.vram_start;
+		break;
+	}
+
+	return (bo->tbo.mem.start << PAGE_SHIFT) + start;
 }
 
 static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 098bc9f40b98..b10654494262 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -56,7 +56,7 @@
 static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
 static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
 
-static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
+struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
 {
 	struct radeon_mman *mman;
 	struct radeon_device *rdev;
@@ -87,7 +87,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		break;
 	case TTM_PL_TT:
 		man->func = &ttm_bo_manager_func;
-		man->gpu_offset = rdev->mc.gtt_start;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
@@ -109,7 +108,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &ttm_bo_manager_func;
-		man->gpu_offset = rdev->mc.vram_start;
 		man->flags = TTM_MEMTYPE_FLAG_FIXED |
 			     TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
-- 
2.25.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 3/6] drm/vmwgfx: don't use ttm bo->offset

[Nirmoy Das]

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c         | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c    | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c       | 2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 74016a08d118..dd9fd609d37c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -258,7 +258,7 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private *dev_priv,
 		ret = ttm_bo_validate(bo, &placement, &ctx);
 
 	/* For some reason we didn't end up at the start of vram */
-	WARN_ON(ret == 0 && bo->offset != 0);
+	WARN_ON(ret == 0 && (bo->mem.start << PAGE_SHIFT) != 0);
 	if (!ret)
 		vmw_bo_pin_reserved(buf, true);
 
@@ -317,7 +317,7 @@ void vmw_bo_get_guest_ptr(const struct ttm_buffer_object *bo,
 {
 	if (bo->mem.mem_type == TTM_PL_VRAM) {
 		ptr->gmrId = SVGA_GMR_FRAMEBUFFER;
-		ptr->offset = bo->offset;
+		ptr->offset = bo->mem.start << PAGE_SHIFT;
 	} else {
 		ptr->gmrId = bo->mem.start;
 		ptr->offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index ff86d49dc5e8..e8a3351f35cf 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3305,7 +3305,7 @@ static void vmw_apply_relocations(struct vmw_sw_context *sw_context)
 		bo = &reloc->vbo->base;
 		switch (bo->mem.mem_type) {
 		case TTM_PL_VRAM:
-			reloc->location->offset += bo->offset;
+			reloc->location->offset += bo->mem.start << PAGE_SHIFT;
 			reloc->location->gmrId = SVGA_GMR_FRAMEBUFFER;
 			break;
 		case VMW_PL_GMR:
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
index e5252ef3812f..1cdc445b24c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
@@ -612,7 +612,7 @@ static int vmw_fifo_emit_dummy_legacy_query(struct vmw_private *dev_priv,
 
 	if (bo->mem.mem_type == TTM_PL_VRAM) {
 		cmd->body.guestResult.gmrId = SVGA_GMR_FRAMEBUFFER;
-		cmd->body.guestResult.offset = bo->offset;
+		cmd->body.guestResult.offset = bo->mem.start << PAGE_SHIFT;
 	} else {
 		cmd->body.guestResult.gmrId = bo->mem.start;
 		cmd->body.guestResult.offset = 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index d8ea3dd10af0..1e69c013b47f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -755,7 +755,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &ttm_bo_manager_func;
-		man->gpu_offset = 0;
 		man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
@@ -768,7 +767,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		 *  slots as well as the bo size.
 		 */
 		man->func = &vmw_gmrid_manager_func;
-		man->gpu_offset = 0;
 		man->flags = TTM_MEMTYPE_FLAG_CMA | TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-- 
2.25.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 4/6] drm/nouveau: don't use ttm bo->offset

[Nirmoy Das]

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  6 +++---
 drivers/gpu/drm/nouveau/dispnv04/disp.c     |  2 +-
 drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 +++---
 drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  2 +-
 drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
 drivers/gpu/drm/nouveau/nouveau_abi16.c     |  8 ++++----
 drivers/gpu/drm/nouveau/nouveau_bo.c        |  1 +
 drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
 drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++++-----
 14 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 37c50ea8f847..18a06cf03fa1 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -845,7 +845,7 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
 		fb = nouveau_framebuffer(crtc->primary->fb);
 	}
 
-	nv_crtc->fb.offset = fb->nvbo->bo.offset;
+	nv_crtc->fb.offset = fb->nvbo->offset;
 
 	if (nv_crtc->lut.depth != drm_fb->format->depth) {
 		nv_crtc->lut.depth = drm_fb->format->depth;
@@ -1013,7 +1013,7 @@ nv04_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file_priv,
 		nv04_cursor_upload(dev, cursor, nv_crtc->cursor.nvbo);
 
 	nouveau_bo_unmap(cursor);
-	nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->bo.offset;
+	nv_crtc->cursor.offset = nv_crtc->cursor.nvbo->offset;
 	nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.offset);
 	nv_crtc->cursor.show(nv_crtc, true);
 out:
@@ -1191,7 +1191,7 @@ nv04_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
 	/* Initialize a page flip struct */
 	*s = (struct nv04_page_flip_state)
 		{ { }, event, crtc, fb->format->cpp[0] * 8, fb->pitches[0],
-		  new_bo->bo.offset };
+		  new_bo->offset };
 
 	/* Keep vblanks on during flip, for the target crtc of this flip */
 	drm_crtc_vblank_get(crtc);
diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c b/drivers/gpu/drm/nouveau/dispnv04/disp.c
index 44ee82d0c9b6..89a4ddfcc55f 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
@@ -151,7 +151,7 @@ nv04_display_init(struct drm_device *dev, bool resume, bool runtime)
 			continue;
 
 		if (nv_crtc->cursor.set_offset)
-			nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.nvbo->bo.offset);
+			nv_crtc->cursor.set_offset(nv_crtc, nv_crtc->cursor.nvbo->offset);
 		nv_crtc->cursor.set_pos(nv_crtc, nv_crtc->cursor_saved_x,
 						 nv_crtc->cursor_saved_y);
 	}
diff --git a/drivers/gpu/drm/nouveau/dispnv04/overlay.c b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
index a3a0a73ae8ab..9529bd9053e7 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/overlay.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/overlay.c
@@ -150,7 +150,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	nvif_mask(dev, NV_PCRTC_ENGINE_CTRL + soff2, NV_CRTC_FSEL_OVERLAY, 0);
 
 	nvif_wr32(dev, NV_PVIDEO_BASE(flip), 0);
-	nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->bo.offset);
+	nvif_wr32(dev, NV_PVIDEO_OFFSET_BUFF(flip), nv_fb->nvbo->offset);
 	nvif_wr32(dev, NV_PVIDEO_SIZE_IN(flip), src_h << 16 | src_w);
 	nvif_wr32(dev, NV_PVIDEO_POINT_IN(flip), src_y << 16 | src_x);
 	nvif_wr32(dev, NV_PVIDEO_DS_DX(flip), (src_w << 20) / crtc_w);
@@ -172,7 +172,7 @@ nv10_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 	if (format & NV_PVIDEO_FORMAT_PLANAR) {
 		nvif_wr32(dev, NV_PVIDEO_UVPLANE_BASE(flip), 0);
 		nvif_wr32(dev, NV_PVIDEO_UVPLANE_OFFSET_BUFF(flip),
-			nv_fb->nvbo->bo.offset + fb->offsets[1]);
+			nv_fb->nvbo->offset + fb->offsets[1]);
 	}
 	nvif_wr32(dev, NV_PVIDEO_FORMAT(flip), format | fb->pitches[0]);
 	nvif_wr32(dev, NV_PVIDEO_STOP, 0);
@@ -396,7 +396,7 @@ nv04_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 
 	for (i = 0; i < 2; i++) {
 		nvif_wr32(dev, NV_PVIDEO_BUFF0_START_ADDRESS + 4 * i,
-			  nv_fb->nvbo->bo.offset);
+			  nv_fb->nvbo->offset);
 		nvif_wr32(dev, NV_PVIDEO_BUFF0_PITCH_LENGTH + 4 * i,
 			  fb->pitches[0]);
 		nvif_wr32(dev, NV_PVIDEO_BUFF0_OFFSET + 4 * i, 0);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/base507c.c b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
index 00a85f1e1a4a..67829f04b2c7 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/base507c.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
@@ -274,7 +274,7 @@ base507c_new_(const struct nv50_wndw_func *func, const u32 *format,
 
 	ret = nv50_dmac_create(&drm->client.device, &disp->disp->object,
 			       &oclass, head, &args, sizeof(args),
-			       disp->sync->bo.offset, &wndw->wndw);
+			       disp->sync->offset, &wndw->wndw);
 	if (ret) {
 		NV_ERROR(drm, "base%04x allocation failed: %d\n", oclass, ret);
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/core507d.c b/drivers/gpu/drm/nouveau/dispnv50/core507d.c
index e7fcfa6e6467..793dcb2ea196 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/core507d.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/core507d.c
@@ -99,7 +99,7 @@ core507d_new_(const struct nv50_core_func *func, struct nouveau_drm *drm,
 
 	ret = nv50_dmac_create(&drm->client.device, &disp->disp->object,
 			       &oclass, 0, &args, sizeof(args),
-			       disp->sync->bo.offset, &core->chan);
+			       disp->sync->offset, &core->chan);
 	if (ret) {
 		NV_ERROR(drm, "core%04x allocation failed: %d\n", oclass, ret);
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c b/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
index 8ccd96113bad..4cce1078140a 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
@@ -186,7 +186,7 @@ ovly507e_new_(const struct nv50_wndw_func *func, const u32 *format,
 
 	ret = nv50_dmac_create(&drm->client.device, &disp->disp->object,
 			       &oclass, 0, &args, sizeof(args),
-			       disp->sync->bo.offset, &wndw->wndw);
+			       disp->sync->offset, &wndw->wndw);
 	if (ret) {
 		NV_ERROR(drm, "ovly%04x allocation failed: %d\n", oclass, ret);
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 5193b6257061..00c921f09eaf 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -502,7 +502,7 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state)
 	}
 
 	asyw->state.fence = dma_resv_get_excl_rcu(fb->nvbo->bo.base.resv);
-	asyw->image.offset[0] = fb->nvbo->bo.offset;
+	asyw->image.offset[0] = fb->nvbo->offset;
 
 	if (wndw->func->prepare) {
 		asyh = nv50_head_atom_get(asyw->state.state, asyw->state.crtc);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
index 0f9402162bde..5e47f3065b48 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
@@ -293,7 +293,7 @@ wndwc37e_new_(const struct nv50_wndw_func *func, struct nouveau_drm *drm,
 
 	ret = nv50_dmac_create(&drm->client.device, &disp->disp->object,
 			       &oclass, 0, &args, sizeof(args),
-			       disp->sync->bo.offset, &wndw->wndw);
+			       disp->sync->offset, &wndw->wndw);
 	if (ret) {
 		NV_ERROR(drm, "qndw%04x allocation failed: %d\n", oclass, ret);
 		return ret;
diff --git a/drivers/gpu/drm/nouveau/nouveau_abi16.c b/drivers/gpu/drm/nouveau/nouveau_abi16.c
index e2bae1424502..c32a8ca67f82 100644
--- a/drivers/gpu/drm/nouveau/nouveau_abi16.c
+++ b/drivers/gpu/drm/nouveau/nouveau_abi16.c
@@ -558,13 +558,13 @@ nouveau_abi16_ioctl_notifierobj_alloc(ABI16_IOCTL_ARGS)
 	if (drm->agp.bridge) {
 		args.target = NV_DMA_V0_TARGET_AGP;
 		args.access = NV_DMA_V0_ACCESS_RDWR;
-		args.start += drm->agp.base + chan->ntfy->bo.offset;
-		args.limit += drm->agp.base + chan->ntfy->bo.offset;
+		args.start += drm->agp.base + chan->ntfy->offset;
+		args.limit += drm->agp.base + chan->ntfy->offset;
 	} else {
 		args.target = NV_DMA_V0_TARGET_VM;
 		args.access = NV_DMA_V0_ACCESS_RDWR;
-		args.start += chan->ntfy->bo.offset;
-		args.limit += chan->ntfy->bo.offset;
+		args.start += chan->ntfy->offset;
+		args.limit += chan->ntfy->offset;
 	}
 
 	client->route = NVDRM_OBJECT_ABI16;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index f8015e0318d7..30238fa60579 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1317,6 +1317,7 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
 		list_for_each_entry(vma, &nvbo->vma_list, head) {
 			nouveau_vma_map(vma, mem);
 		}
+		nvbo->offset = (new_reg->start << PAGE_SHIFT);
 	} else {
 		list_for_each_entry(vma, &nvbo->vma_list, head) {
 			WARN_ON(ttm_bo_wait(bo, false, false));
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h b/drivers/gpu/drm/nouveau/nouveau_bo.h
index 38f9d8350963..e944b4aa5547 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -24,6 +24,9 @@ struct nouveau_bo {
 	int pbbo_index;
 	bool validate_mapped;
 
+	/* GPU address space is independent of CPU word size */
+	uint64_t offset;
+
 	struct list_head vma_list;
 
 	unsigned contig:1;
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/nouveau/nouveau_chan.c
index 282fd90b65e1..283396b6edab 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -160,7 +160,7 @@ nouveau_channel_prep(struct nouveau_drm *drm, struct nvif_device *device,
 	 * pushbuf lives in, this is because the GEM code requires that
 	 * we be able to call out to other (indirect) push buffers
 	 */
-	chan->push.addr = chan->push.buffer->bo.offset;
+	chan->push.addr = chan->push.buffer->offset;
 
 	if (device->info.family >= NV_DEVICE_INFO_V0_TESLA) {
 		ret = nouveau_vma_new(chan->push.buffer, chan->vmm,
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index f439f0a5b43a..78b8ec667d72 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -393,7 +393,7 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
 
 	/* To allow resizeing without swapping buffers */
 	NV_INFO(drm, "allocated %dx%d fb: 0x%llx, bo %p\n",
-		fb->base.width, fb->base.height, fb->nvbo->bo.offset, nvbo);
+		fb->base.width, fb->base.height, fb->nvbo->offset, nvbo);
 
 	vga_switcheroo_client_fb_set(dev->pdev, info);
 	return 0;
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index 1324c19f4e5c..00018d3f6213 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -232,7 +232,7 @@ nouveau_gem_info(struct drm_file *file_priv, struct drm_gem_object *gem,
 		rep->domain = NOUVEAU_GEM_DOMAIN_GART;
 	else
 		rep->domain = NOUVEAU_GEM_DOMAIN_VRAM;
-	rep->offset = nvbo->bo.offset;
+	rep->offset = nvbo->offset;
 	if (vmm->vmm.object.oclass >= NVIF_CLASS_VMM_NV50) {
 		vma = nouveau_vma_find(nvbo, vmm);
 		if (!vma)
@@ -519,7 +519,7 @@ validate_list(struct nouveau_channel *chan, struct nouveau_cli *cli,
 		}
 
 		if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) {
-			if (nvbo->bo.offset == b->presumed.offset &&
+			if (nvbo->offset == b->presumed.offset &&
 			    ((nvbo->bo.mem.mem_type == TTM_PL_VRAM &&
 			      b->presumed.domain & NOUVEAU_GEM_DOMAIN_VRAM) ||
 			     (nvbo->bo.mem.mem_type == TTM_PL_TT &&
@@ -530,7 +530,7 @@ validate_list(struct nouveau_channel *chan, struct nouveau_cli *cli,
 				b->presumed.domain = NOUVEAU_GEM_DOMAIN_GART;
 			else
 				b->presumed.domain = NOUVEAU_GEM_DOMAIN_VRAM;
-			b->presumed.offset = nvbo->bo.offset;
+			b->presumed.offset = nvbo->offset;
 			b->presumed.valid = 0;
 			relocs++;
 
@@ -798,7 +798,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
 			struct nouveau_bo *nvbo = (void *)(unsigned long)
 				bo[push[i].bo_index].user_priv;
 
-			OUT_RING(chan, (nvbo->bo.offset + push[i].offset) | 2);
+			OUT_RING(chan, (nvbo->offset + push[i].offset) | 2);
 			OUT_RING(chan, 0);
 		}
 	} else {
@@ -833,7 +833,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
 			}
 
 			OUT_RING(chan, 0x20000000 |
-				      (nvbo->bo.offset + push[i].offset));
+				      (nvbo->offset + push[i].offset));
 			OUT_RING(chan, 0);
 			for (j = 0; j < NOUVEAU_DMA_SKIPS; j++)
 				OUT_RING(chan, 0);
-- 
2.25.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset

[Nirmoy Das]

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/qxl/qxl_drv.h    | 6 ++----
 drivers/gpu/drm/qxl/qxl_kms.c    | 3 +++
 drivers/gpu/drm/qxl/qxl_object.h | 5 -----
 drivers/gpu/drm/qxl/qxl_ttm.c    | 9 ---------
 4 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 27e45a2d6b52..9a76a2a0283d 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
 		(bo->tbo.mem.mem_type == TTM_PL_VRAM)
 		? &qdev->main_slot : &qdev->surfaces_slot;
 
-	WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
-
-	/* TODO - need to hold one of the locks to read tbo.offset */
-	return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
+	return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) +
+				  slot->gpu_offset + offset);
 }
 
 /* qxl_display.c */
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 611cbe7aee69..937cac9ba384 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -71,11 +71,14 @@ static void setup_slot(struct qxl_device *qdev,
 		       unsigned long size)
 {
 	uint64_t high_bits;
+	unsigned int gpu_offset_shift =
+		64 - (qdev->rom->slot_gen_bits + qdev->rom->slot_id_bits + 8);
 
 	slot->index = slot_index;
 	slot->name = slot_name;
 	slot->start_phys_addr = start_phys_addr;
 	slot->size = size;
+	slot->gpu_offset = (uint64_t)slot_index << gpu_offset_shift;
 
 	setup_hw_slot(qdev, slot);
 
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 8ae54ba7857c..21fa81048f4f 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -48,11 +48,6 @@ static inline void qxl_bo_unreserve(struct qxl_bo *bo)
 	ttm_bo_unreserve(&bo->tbo);
 }
 
-static inline u64 qxl_bo_gpu_offset(struct qxl_bo *bo)
-{
-	return bo->tbo.offset;
-}
-
 static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
 {
 	return bo->tbo.num_pages << PAGE_SHIFT;
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 16a5e903533d..2a43d0ef9ba1 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -56,11 +56,6 @@ static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
 static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 			     struct ttm_mem_type_manager *man)
 {
-	struct qxl_device *qdev = qxl_get_qdev(bdev);
-	unsigned int gpu_offset_shift =
-		64 - (qdev->rom->slot_gen_bits + qdev->rom->slot_id_bits + 8);
-	struct qxl_memslot *slot;
-
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
@@ -71,11 +66,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 	case TTM_PL_PRIV:
 		/* "On-card" video ram */
-		slot = (type == TTM_PL_VRAM) ?
-			&qdev->main_slot : &qdev->surfaces_slot;
-		slot->gpu_offset = (uint64_t)type << gpu_offset_shift;
 		man->func = &ttm_bo_manager_func;
-		man->gpu_offset = slot->gpu_offset;
 		man->flags = TTM_MEMTYPE_FLAG_FIXED |
 			     TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_MASK_CACHING;
-- 
2.25.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 6/6] drm/ttm: do not keep GPU dependent addresses

[Nirmoy Das]

GPU address handling is device specific and should be handle by its device
driver.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c    | 7 -------
 include/drm/ttm/ttm_bo_api.h    | 2 --
 include/drm/ttm/ttm_bo_driver.h | 1 -
 3 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 229205e499db..2ccfebc3c9a2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -85,7 +85,6 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
 	drm_printf(p, "    has_type: %d\n", man->has_type);
 	drm_printf(p, "    use_type: %d\n", man->use_type);
 	drm_printf(p, "    flags: 0x%08X\n", man->flags);
-	drm_printf(p, "    gpu_offset: 0x%08llX\n", man->gpu_offset);
 	drm_printf(p, "    size: %llu\n", man->size);
 	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
 	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
@@ -382,12 +381,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 		bo->evicted = false;
 	}
 
-	if (bo->mem.mm_node)
-		bo->offset = (bo->mem.start << PAGE_SHIFT) +
-		    bdev->man[bo->mem.mem_type].gpu_offset;
-	else
-		bo->offset = 0;
-
 	ctx->bytes_moved += bo->num_pages << PAGE_SHIFT;
 	return 0;
 
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 65e399d280f7..3cf8bb82c899 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -219,8 +219,6 @@ struct ttm_buffer_object {
 	 * either of these locks held.
 	 */
 
-	uint64_t offset; /* GPU address space is independent of CPU word size */
-
 	struct sg_table *sg;
 
 	struct mutex wu_mutex;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index cac7a8a0825a..302b0aaf8d13 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -177,7 +177,6 @@ struct ttm_mem_type_manager {
 	bool has_type;
 	bool use_type;
 	uint32_t flags;
-	uint64_t gpu_offset; /* GPU address space is independent of CPU word size */
 	uint64_t size;
 	uint32_t available_caching;
 	uint32_t default_caching;
-- 
2.25.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset

[Christian König]

Looks like most of the patch is missing?

We should have quite a number of uses of the BO offset in radeon or do 
all of those go through radeon_bo_gpu_offset?

If yes then the change is much smaller than I thought i needs to be.

Christian.

Am 13.02.20 um 13:01 schrieb Nirmoy Das:
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/radeon/radeon.h        |  1 +
>   drivers/gpu/drm/radeon/radeon_object.h | 16 +++++++++++++++-
>   drivers/gpu/drm/radeon/radeon_ttm.c    |  4 +---
>   3 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index d59b004f6695..97cfcc2870af 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2823,6 +2823,7 @@ extern void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size
>   extern void radeon_program_register_sequence(struct radeon_device *rdev,
>   					     const u32 *registers,
>   					     const u32 array_size);
> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev);
>   
>   /*
>    * vm
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
> index d23f2ed4126e..4d37571c7ff5 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -90,7 +90,21 @@ static inline void radeon_bo_unreserve(struct radeon_bo *bo)
>    */
>   static inline u64 radeon_bo_gpu_offset(struct radeon_bo *bo)
>   {
> -	return bo->tbo.offset;
> +	struct radeon_device *rdev;
> +	u64 start = 0;
> +
> +	rdev = radeon_get_rdev(bo->tbo.bdev);
> +
> +	switch(bo->tbo.mem.mem_type) {
> +	case TTM_PL_TT:
> +		start = rdev->mc.gtt_start;
> +		break;
> +	case TTM_PL_VRAM:
> +		start = rdev->mc.vram_start;
> +		break;
> +	}
> +
> +	return (bo->tbo.mem.start << PAGE_SHIFT) + start;
>   }
>   
>   static inline unsigned long radeon_bo_size(struct radeon_bo *bo)
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 098bc9f40b98..b10654494262 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -56,7 +56,7 @@
>   static int radeon_ttm_debugfs_init(struct radeon_device *rdev);
>   static void radeon_ttm_debugfs_fini(struct radeon_device *rdev);
>   
> -static struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
> +struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
>   {
>   	struct radeon_mman *mman;
>   	struct radeon_device *rdev;
> @@ -87,7 +87,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   		break;
>   	case TTM_PL_TT:
>   		man->func = &ttm_bo_manager_func;
> -		man->gpu_offset = rdev->mc.gtt_start;
>   		man->available_caching = TTM_PL_MASK_CACHING;
>   		man->default_caching = TTM_PL_FLAG_CACHED;
>   		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
> @@ -109,7 +108,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   	case TTM_PL_VRAM:
>   		/* "On-card" video ram */
>   		man->func = &ttm_bo_manager_func;
> -		man->gpu_offset = rdev->mc.vram_start;
>   		man->flags = TTM_MEMTYPE_FLAG_FIXED |
>   			     TTM_MEMTYPE_FLAG_MAPPABLE;
>   		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 0/6] do not store GPU address in TTM

[Christian König]

Am 13.02.20 um 13:01 schrieb Nirmoy Das:
> With this patch series I am trying to remove GPU address dependency in
> TTM and moving GPU address calculation to individual drm drivers.
> This is required[1] to continue the work started by Brian Welty to create
> struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory
> limits.

Nice work. I wouldn't say that it is necessary for [1], but it is 
certainly nice to have that cleaned up.

Christian.

>
>
> I have only manage to test amdgpu driver as I only have GPU for that.
> I might be doing something really stupid while calculeting gpu offset for
> some of the drivers so please be patient and let me know how can I improve
> that.
>
> [1] https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg272238.html
>
> Nirmoy Das (6):
>    drm/amdgpu: move ttm bo->offset to amdgpu_bo
>    drm/radeon: don't use ttm bo->offset
>    drm/vmwgfx: don't use ttm bo->offset
>    drm/nouveau: don't use ttm bo->offset
>    drm/qxl: don't use ttm bo->offset
>    drm/ttm: do not keep GPU dependent addresses
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 ++++++++++++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 29 ++++++++++++++++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 +--
>   drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  6 ++---
>   drivers/gpu/drm/nouveau/dispnv04/disp.c     |  2 +-
>   drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 ++---
>   drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
>   drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
>   drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
>   drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  2 +-
>   drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_abi16.c     |  8 +++---
>   drivers/gpu/drm/nouveau/nouveau_bo.c        |  1 +
>   drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
>   drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++----
>   drivers/gpu/drm/qxl/qxl_drv.h               |  6 ++---
>   drivers/gpu/drm/qxl/qxl_kms.c               |  3 +++
>   drivers/gpu/drm/qxl/qxl_object.h            |  5 ----
>   drivers/gpu/drm/qxl/qxl_ttm.c               |  9 -------
>   drivers/gpu/drm/radeon/radeon.h             |  1 +
>   drivers/gpu/drm/radeon/radeon_object.h      | 16 +++++++++++-
>   drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +--
>   drivers/gpu/drm/ttm/ttm_bo.c                |  7 -----
>   drivers/gpu/drm/vmwgfx/vmwgfx_bo.c          |  4 +--
>   drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c     |  2 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c        |  2 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  2 --
>   include/drm/ttm/ttm_bo_api.h                |  2 --
>   include/drm/ttm/ttm_bo_driver.h             |  1 -
>   33 files changed, 99 insertions(+), 72 deletions(-)
>
> --
> 2.25.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset

[Nirmoy]

On 2/13/20 1:18 PM, Christian König wrote:
> Looks like most of the patch is missing?
>
> We should have quite a number of uses of the BO offset in radeon or do 
> all of those go through radeon_bo_gpu_offset?
Compilation worked so I think all those(bo->offset) accesses went 
through radeon_bo_gpu_offset.
>
> If yes then the change is much smaller than I thought i needs to be.
>
> Christian.
>
Regards,

Nirmoy

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset

[Christian König]

Am 13.02.20 um 13:31 schrieb Nirmoy:
>
> On 2/13/20 1:18 PM, Christian König wrote:
>> Looks like most of the patch is missing?
>>
>> We should have quite a number of uses of the BO offset in radeon or 
>> do all of those go through radeon_bo_gpu_offset?
> Compilation worked so I think all those(bo->offset) accesses went 
> through radeon_bo_gpu_offset.

Cool, than that is a lot easier to implement than I thought it would be.

I assume you don't have Radeon hardware lying around to test this?

If you can you give me a branch on the AMD (or public) servers I can 
give it at least a quick round of testing.

Christian.

>>
>> If yes then the change is much smaller than I thought i needs to be.
>>
>> Christian.
>>
> Regards,
>
> Nirmoy
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset

[Nirmoy]

On 2/13/20 2:00 PM, Christian König wrote:
> Am 13.02.20 um 13:31 schrieb Nirmoy:
>>
>> On 2/13/20 1:18 PM, Christian König wrote:
>>> Looks like most of the patch is missing?
>>>
>>> We should have quite a number of uses of the BO offset in radeon or 
>>> do all of those go through radeon_bo_gpu_offset?
>> Compilation worked so I think all those(bo->offset) accesses went 
>> through radeon_bo_gpu_offset.
>
> Cool, than that is a lot easier to implement than I thought it would be.
>
> I assume you don't have Radeon hardware lying around to test this?
>
> If you can you give me a branch on the AMD (or public) servers I can 
> give it at least a quick round of testing.

huh it seems I have to rebase it a bit for drm/drm-next as it is based 
on our internal branch :/

You can access the branch at 
http://gitlab1.amd.com/nirmodas/linux/tree/ttm_clean.1

I will rebase this for drm-next in next version of the patch series.

>
> Christian.
>
>>>
>>> If yes then the change is much smaller than I thought i needs to be.
>>>
>>> Christian.
>>>
>> Regards,
>>
>> Nirmoy
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CNirmoy.Das%40amd.com%7C60aa17c05a2f474663f008d7b084b550%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171956342728389&sdata=I2n0yMXK5CtrG5tY9Q47igxJv9Onb%2FA7PBeLwrpPb54%3D&reserved=0 
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset

[Nirmoy]

On 2/13/20 2:00 PM, Christian König wrote:
> Cool, than that is a lot easier to implement than I thought it would be.
>
> I assume you don't have Radeon hardware lying around to test this?

Yes this would be nice I don't have a  Radeon hw with me.

>
> If you can you give me a branch on the AMD (or public) servers I can 
> give it at least a quick round of testing.


Thanks,

Nirmoy


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset

[Gerd Hoffmann]

> @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
>  		(bo->tbo.mem.mem_type == TTM_PL_VRAM)
>  		? &qdev->main_slot : &qdev->surfaces_slot;
>  
> -	WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
> -
> -	/* TODO - need to hold one of the locks to read tbo.offset */
> -	return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
> +	return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) +
> +				  slot->gpu_offset + offset);
>  }

--verbose please.

I don't get the logic behind this change.

The other chunks look sane, calculating slot->gpu_offset
in setup_slot() certainly makes sense.

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 0/6] do not store GPU address in TTM

[Huang Rui]

On Thu, Feb 13, 2020 at 01:01:57PM +0100, Nirmoy Das wrote:
> With this patch series I am trying to remove GPU address dependency in
> TTM and moving GPU address calculation to individual drm drivers.
> This is required[1] to continue the work started by Brian Welty to create
> struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory
> limits.
> 
> 
> I have only manage to test amdgpu driver as I only have GPU for that.
> I might be doing something really stupid while calculeting gpu offset for
> some of the drivers so please be patient and let me know how can I improve
> that.
> 
> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.mail-archive.com%2Fdri-devel%40lists.freedesktop.org%2Fmsg272238.html&data=02%7C01%7Cray.huang%40amd.com%7Cad8c8464b13e4764556008d7b07c3344%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171919805856109&sdata=zlA%2FHePGKcILKg7Ezc9CGc%2FWXJkRa5xmrBznvJcAomk%3D&reserved=0

Looks good for me as well for amd part.

Acked-by: Huang Rui <ray.huang@amd.com>

> 
> Nirmoy Das (6):
>   drm/amdgpu: move ttm bo->offset to amdgpu_bo
>   drm/radeon: don't use ttm bo->offset
>   drm/vmwgfx: don't use ttm bo->offset
>   drm/nouveau: don't use ttm bo->offset
>   drm/qxl: don't use ttm bo->offset
>   drm/ttm: do not keep GPU dependent addresses
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 ++++++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 29 ++++++++++++++++-----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 +--
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  6 ++---
>  drivers/gpu/drm/nouveau/dispnv04/disp.c     |  2 +-
>  drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 ++---
>  drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_abi16.c     |  8 +++---
>  drivers/gpu/drm/nouveau/nouveau_bo.c        |  1 +
>  drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
>  drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++----
>  drivers/gpu/drm/qxl/qxl_drv.h               |  6 ++---
>  drivers/gpu/drm/qxl/qxl_kms.c               |  3 +++
>  drivers/gpu/drm/qxl/qxl_object.h            |  5 ----
>  drivers/gpu/drm/qxl/qxl_ttm.c               |  9 -------
>  drivers/gpu/drm/radeon/radeon.h             |  1 +
>  drivers/gpu/drm/radeon/radeon_object.h      | 16 +++++++++++-
>  drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +--
>  drivers/gpu/drm/ttm/ttm_bo.c                |  7 -----
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c          |  4 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c     |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c        |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  2 --
>  include/drm/ttm/ttm_bo_api.h                |  2 --
>  include/drm/ttm/ttm_bo_driver.h             |  1 -
>  33 files changed, 99 insertions(+), 72 deletions(-)
> 
> --
> 2.25.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Cray.huang%40amd.com%7Cad8c8464b13e4764556008d7b07c3344%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171919805856109&amp;sdata=lnJkwlCEbUmtsBhBY94rB3hRgaYg4ENQ0DNTXxxwPL4%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset

[Nirmoy]

On 2/13/20 3:30 PM, Gerd Hoffmann wrote:
>> @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
>>   		(bo->tbo.mem.mem_type == TTM_PL_VRAM)
>>   		? &qdev->main_slot : &qdev->surfaces_slot;
>>   
>> -	WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
>> -
>> -	/* TODO - need to hold one of the locks to read tbo.offset */
>> -	return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
>> +	return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) +
>> +				  slot->gpu_offset + offset);
>>   }
> --verbose please.
>
> I don't get the logic behind this change.

Hi Gerd,

I borrowed the logic for removed ttm part

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 229205e499db..2ccfebc3c9a2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -382,12 +381,6 @@ static int ttm_bo_handle_move_mem(struct 
ttm_buffer_object *bo,
                 bo->evicted = false;
         }

-       if (bo->mem.mm_node)
-               bo->offset = (bo->mem.start << PAGE_SHIFT) +
-                   bdev->man[bo->mem.mem_type].gpu_offset;
-       else
-               bo->offset = 0;
-


My assumption is

  (bo->tbo.offset - slot->gpu_offset + offset) == (bo->tbo.mem.start << PAGE_SHIFT) + bdev->man[bo->mem.mem_type].gpu_offset - slot->gpu_offset + offset)

-> == (bo->tbo.mem.start << PAGE_SHIFT) + offset

and we loose  slot->gpu_offset so I thought it should be

((bo->tbo.mem.start << PAGE_SHIFT) + slot->gpu_offset + offset);

Can you please suggest me how to calculate the offset  correctly here.


Regards,

Nirmo

>
> The other chunks look sane, calculating slot->gpu_offset
> in setup_slot() certainly makes sense.
>
> cheers,
>    Gerd
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset

[Christian König]

Am 13.02.20 um 15:30 schrieb Gerd Hoffmann:
>> @@ -311,10 +311,8 @@ qxl_bo_physical_address(struct qxl_device *qdev, struct qxl_bo *bo,
>>   		(bo->tbo.mem.mem_type == TTM_PL_VRAM)
>>   		? &qdev->main_slot : &qdev->surfaces_slot;
>>   
>> -	WARN_ON_ONCE((bo->tbo.offset & slot->gpu_offset) != slot->gpu_offset);
>> -
>> -	/* TODO - need to hold one of the locks to read tbo.offset */
>> -	return slot->high_bits | (bo->tbo.offset - slot->gpu_offset + offset);
>> +	return slot->high_bits | ((bo->tbo.mem.start << PAGE_SHIFT) +
>> +				  slot->gpu_offset + offset);
>>   }
> --verbose please.

The warning and the TODO should probably stay, don't they?

> I don't get the logic behind this change.

We ran into the problem that the whole offset handling in TTM is 
actually completely hardware specific.

E.g. Some need pfn, some need byte offsets, some need 32bit, some 64bit 
and some don't have a consistent offset at all (e.g. amdgpu for exmaple).

So we try to move that back into the drivers to concentrate on memory 
management in TTM.

>
> The other chunks look sane, calculating slot->gpu_offset
> in setup_slot() certainly makes sense.

Thanks for the review,
Christian.

>
> cheers,
>    Gerd
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset

[Gerd Hoffmann]

> -       if (bo->mem.mm_node)
> -               bo->offset = (bo->mem.start << PAGE_SHIFT) +
> -                   bdev->man[bo->mem.mem_type].gpu_offset;
> -       else
> -               bo->offset = 0;
> -
> 
> 
> My assumption is
> 
>  (bo->tbo.offset - slot->gpu_offset + offset) == (bo->tbo.mem.start << PAGE_SHIFT) + bdev->man[bo->mem.mem_type].gpu_offset - slot->gpu_offset + offset)
> 
> -> == (bo->tbo.mem.start << PAGE_SHIFT) + offset

That looks better.

> and we loose  slot->gpu_offset so I thought it should be
> 
> ((bo->tbo.mem.start << PAGE_SHIFT) + slot->gpu_offset + offset);

No.

The addressing scheme used by qxl is the slot in the high bits and the
offset within the slot in the low bits.  The qxl device has two pci
memory bars, the driver creates a slot for each of them, for ttm they
are VRAM and PRIV.

So maybe we don't need gpu_offset at all.  Not fully sure how driver and
ttm interact here.

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 0/6] do not store GPU address in TTM

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3629 bytes --]

Hi

Am 13.02.20 um 13:01 schrieb Nirmoy Das:
> With this patch series I am trying to remove GPU address dependency in
> TTM and moving GPU address calculation to individual drm drivers.
> This is required[1] to continue the work started by Brian Welty to create
> struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory
> limits.
> 
> 
> I have only manage to test amdgpu driver as I only have GPU for that.
> I might be doing something really stupid while calculeting gpu offset for
> some of the drivers so please be patient and let me know how can I improve
> that.
> 
> [1] https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg272238.html
> 
> Nirmoy Das (6):
>   drm/amdgpu: move ttm bo->offset to amdgpu_bo
>   drm/radeon: don't use ttm bo->offset
>   drm/vmwgfx: don't use ttm bo->offset
>   drm/nouveau: don't use ttm bo->offset
>   drm/qxl: don't use ttm bo->offset
>   drm/ttm: do not keep GPU dependent addresses

I think VRAM helpers are missing from this patchset. [1] You can use
bochs in qemu to test them.

Best regards
Thomas

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_gem_vram_helper.c?h=v5.6-rc1#n217

> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 ++++++++++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 29 ++++++++++++++++-----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 +--
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  6 ++---
>  drivers/gpu/drm/nouveau/dispnv04/disp.c     |  2 +-
>  drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 ++---
>  drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_abi16.c     |  8 +++---
>  drivers/gpu/drm/nouveau/nouveau_bo.c        |  1 +
>  drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
>  drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++----
>  drivers/gpu/drm/qxl/qxl_drv.h               |  6 ++---
>  drivers/gpu/drm/qxl/qxl_kms.c               |  3 +++
>  drivers/gpu/drm/qxl/qxl_object.h            |  5 ----
>  drivers/gpu/drm/qxl/qxl_ttm.c               |  9 -------
>  drivers/gpu/drm/radeon/radeon.h             |  1 +
>  drivers/gpu/drm/radeon/radeon_object.h      | 16 +++++++++++-
>  drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +--
>  drivers/gpu/drm/ttm/ttm_bo.c                |  7 -----
>  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c          |  4 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c     |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c        |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  2 --
>  include/drm/ttm/ttm_bo_api.h                |  2 --
>  include/drm/ttm/ttm_bo_driver.h             |  1 -
>  33 files changed, 99 insertions(+), 72 deletions(-)
> 
> --
> 2.25.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 0/6] do not store GPU address in TTM

[Nirmoy]

On 2/14/20 10:19 AM, Thomas Zimmermann wrote:
> Hi
>
> Am 13.02.20 um 13:01 schrieb Nirmoy Das:
>> With this patch series I am trying to remove GPU address dependency in
>> TTM and moving GPU address calculation to individual drm drivers.
>> This is required[1] to continue the work started by Brian Welty to create
>> struct drm_mem_region which can be leverage by DRM cgroup controller to manage memory
>> limits.
>>
>>
>> I have only manage to test amdgpu driver as I only have GPU for that.
>> I might be doing something really stupid while calculeting gpu offset for
>> some of the drivers so please be patient and let me know how can I improve
>> that.
>>
>> [1] https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg272238.html
>>
>> Nirmoy Das (6):
>>    drm/amdgpu: move ttm bo->offset to amdgpu_bo
>>    drm/radeon: don't use ttm bo->offset
>>    drm/vmwgfx: don't use ttm bo->offset
>>    drm/nouveau: don't use ttm bo->offset
>>    drm/qxl: don't use ttm bo->offset
>>    drm/ttm: do not keep GPU dependent addresses
> I think VRAM helpers are missing from this patchset. [1] You can use
> bochs in qemu to test them.
Thanks Thomas. I missed that, I will add that into next revision.
>
> Best regards
> Thomas
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_gem_vram_helper.c?h=v5.6-rc1#n217
>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 22 ++++++++++++++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 29 ++++++++++++++++-----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 +--
>>   drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  6 ++---
>>   drivers/gpu/drm/nouveau/dispnv04/disp.c     |  2 +-
>>   drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 ++---
>>   drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
>>   drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
>>   drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
>>   drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  2 +-
>>   drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
>>   drivers/gpu/drm/nouveau/nouveau_abi16.c     |  8 +++---
>>   drivers/gpu/drm/nouveau/nouveau_bo.c        |  1 +
>>   drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
>>   drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
>>   drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
>>   drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++----
>>   drivers/gpu/drm/qxl/qxl_drv.h               |  6 ++---
>>   drivers/gpu/drm/qxl/qxl_kms.c               |  3 +++
>>   drivers/gpu/drm/qxl/qxl_object.h            |  5 ----
>>   drivers/gpu/drm/qxl/qxl_ttm.c               |  9 -------
>>   drivers/gpu/drm/radeon/radeon.h             |  1 +
>>   drivers/gpu/drm/radeon/radeon_object.h      | 16 +++++++++++-
>>   drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +--
>>   drivers/gpu/drm/ttm/ttm_bo.c                |  7 -----
>>   drivers/gpu/drm/vmwgfx/vmwgfx_bo.c          |  4 +--
>>   drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c     |  2 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c        |  2 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  2 --
>>   include/drm/ttm/ttm_bo_api.h                |  2 --
>>   include/drm/ttm/ttm_bo_driver.h             |  1 -
>>   33 files changed, 99 insertions(+), 72 deletions(-)
>>
>> --
>> 2.25.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 5/6] drm/qxl: don't use ttm bo->offset

[Nirmoy]

On 2/14/20 10:08 AM, Gerd Hoffmann wrote:
>> -       if (bo->mem.mm_node)
>> -               bo->offset = (bo->mem.start << PAGE_SHIFT) +
>> -                   bdev->man[bo->mem.mem_type].gpu_offset;
>> -       else
>> -               bo->offset = 0;
>> -
>>
>>
>> My assumption is
>>
>>   (bo->tbo.offset - slot->gpu_offset + offset) == (bo->tbo.mem.start << PAGE_SHIFT) + bdev->man[bo->mem.mem_type].gpu_offset - slot->gpu_offset + offset)
>>
>> -> == (bo->tbo.mem.start << PAGE_SHIFT) + offset
> That looks better.
Thanks, I will use that.
>
>> and we loose  slot->gpu_offset so I thought it should be
>>
>> ((bo->tbo.mem.start << PAGE_SHIFT) + slot->gpu_offset + offset);
> No.

Yes  this doesn't work, qemu throws bunch of warnings. (tested without 
GUI, modprobe qxl)

[   10.691506] [drm] Device Version 0.0
[   10.691618] [drm] Compression level 0 log level 0
[   10.691759] [drm] 12286 io pages at offset 0x1000000
[   10.691897] [drm] 16777216 byte draw area at offset 0x0
[   10.692043] [drm] RAM header offset: 0x3ffe000
[   10.694823] [TTM] Zone  kernel: Available graphics memory: 240756 KiB
[   10.695055] [TTM] Initializing pool allocator
[   10.695294] [TTM] Initializing DMA pool allocator
[   10.695807] [drm] qxl: 16M of VRAM memory size
[   10.695933] [drm] qxl: 63M of IO pages memory ready (VRAM domain)
[   10.696093] [drm] qxl: 64M of Surface memory size
[   10.699969] [drm] slot 0 (main): base 0xf4000000, size 0x03ffe000, 
gpu_offset 0x0
[   10.700319] [drm] slot 1 (surfaces): base 0xf8000000, size 
0x04000000, gpu_offset 0x10000000000
[   10.707842] [drm] Initialized qxl 0.1.0 20120117 for 0000:00:02.0 on 
minor 0
id 0, group 0, virt start 0, virt end ffffffffffffffff, generation 0, 
delta 0
id 1, group 1, virt start 7f329f400000, virt end 7f32a33fe000, 
generation 0, delta 7f329f400000
id 2, group 1, virt start 7f329b000000, virt end 7f329f000000, 
generation 0, delta 7f329b000000
qemu-system-x86_64: warning: Spice: memslot.c:64:memslot_validate_virt: 
virtual address out of range    virt=0x80329b300000+0x4000 slot_id=2 
group_id=1
     slot=0x7f329b000000-0x7f329f000000 delta=0x7f329b000000
qemu-system-x86_64: warning: Spice: 
display-channel.c:2437:display_channel_validate_surface: canvas address 
is 0x7f32d989eb18 for 0 (and is NULL)
qemu-system-x86_64: warning: Spice: 
display-channel.c:2439:display_channel_validate_surface: failed on 0
qemu-system-x86_64: warning: Spice: 
display-channel.c:2437:display_channel_validate_surface: canvas address 
is 0x7f32d989eb18 for 0 (and is NULL)
qemu-system-x86_64: warning: Spice: 
display-channel.c:2439:display_channel_validate_surface: failed on 0
qemu-system-x86_64: warning: Spice: 
display-channel.c:2437:display_channel_validate_surface: canvas address 
is 0x7f32d989eb18 for 0 (and is NULL)
qemu-system-x86_64: warning: Spice: 
display-channel.c:2439:display_channel_validate_surface: failed on 0
qemu-system-x86_64: warning: Spice: 
red-worker.c:468:destroy_primary_surface: double destroy of primary surface
id 0, group 0, virt start 0, virt end ffffffffffffffff, generation 0, 
delta 0
id 1, group 1, virt start 7f329f400000, virt end 7f32a33fe000, 
generation 0, delta 7f329f400000
id 2, group 1, virt start 7f329b000000, virt end 7f329f000000, 
generation 0, delta 7f329b000000
qemu-system-x86_64: warning: Spice: memslot.c:64:memslot_validate_virt: 
virtual address out of range    virt=0x80329b304000+0x300000 slot_id=2 
group_id=1
     slot=0x7f329b000000-0x7f329f000000 delta=0x7f329b000000
qemu-system-x86_64: warning: Spice: 
display-channel.c:2437:display_channel_validate_surface: canvas address 
is 0x7f32d989eb18 for 0 (and is NULL)
qemu-system-x86_64: warning: Spice: 
display-channel.c:2439:display_channel_validate_surface: failed on 0
[   10.723939] fbcon: qxldrmfb (fb0) is primary device
[   10.749245] Console: switching to colour frame buffer device 128x48
[   10.775038] qxl 0000:00:02.0: fb0: qxldrmfb frame buffer device
qemu-system-x86_64: warning: Spice: 
display-channel.c:2437:display_channel_validate_surface: canvas address 
is 0x7f32d989eb18 for 0 (and is NULL)
qemu-system-x86_64: warning: Spice: 
display-channel.c:2439:display_channel_validate_surface: failed on 0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 2/6] drm/radeon: don't use ttm bo->offset

[Christian König]

Am 13.02.20 um 14:28 schrieb Nirmoy:
>
> On 2/13/20 2:00 PM, Christian König wrote:
>> Am 13.02.20 um 13:31 schrieb Nirmoy:
>>>
>>> On 2/13/20 1:18 PM, Christian König wrote:
>>>> Looks like most of the patch is missing?
>>>>
>>>> We should have quite a number of uses of the BO offset in radeon or 
>>>> do all of those go through radeon_bo_gpu_offset?
>>> Compilation worked so I think all those(bo->offset) accesses went 
>>> through radeon_bo_gpu_offset.
>>
>> Cool, than that is a lot easier to implement than I thought it would be.
>>
>> I assume you don't have Radeon hardware lying around to test this?
>>
>> If you can you give me a branch on the AMD (or public) servers I can 
>> give it at least a quick round of testing.
>
> huh it seems I have to rebase it a bit for drm/drm-next as it is based 
> on our internal branch :/
>
> You can access the branch at 
> http://gitlab1.amd.com/nirmodas/linux/tree/ttm_clean.1
>
> I will rebase this for drm-next in next version of the patch series.

Actually you should probably base it on drm-misc-next instead.

And additional to the already noted VRAM helpers the bochs driver fails 
to compile on your branch.

The good news is that my old HD5670 still works with the radeon patch 
applied. So this patch is Reviewed-and-tested-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

>
>>
>> Christian.
>>
>>>>
>>>> If yes then the change is much smaller than I thought i needs to be.
>>>>
>>>> Christian.
>>>>
>>> Regards,
>>>
>>> Nirmoy
>>>
>>> _______________________________________________
>>> amd-gfx mailing list
>>> amd-gfx@lists.freedesktop.org
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CNirmoy.Das%40amd.com%7C60aa17c05a2f474663f008d7b084b550%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171956342728389&amp;sdata=I2n0yMXK5CtrG5tY9Q47igxJv9Onb%2FA7PBeLwrpPb54%3D&amp;reserved=0 
>>>
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel