dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/8] do not store GPU address in TTM
@ 2020-04-01 18:42 Nirmoy Das
  2020-04-01 18:42 ` [PATCH 1/1] drm/amdgpu: move ttm bo->offset to amdgpu_bo Nirmoy Das
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, alexander.deucher, sean,
	christian.koenig, kraxel

With this patch series I am trying to remove GPU address dependency in
TTM and moving GPU address calculation to individual drm drivers. This
cleanup will simplify introduction of drm_mem_region/domain work started
by Brian Welty[1].


It would be nice if someone test this for nouveau. Rest of the drivers
are already tested.

v2:
* set bo->offset = 0 for drm/nouveau if bo->mem.mm_node == NULL

v3:
* catch return value of drm_gem_vram_offset() in drm/bochs
* introduce drm_gem_vram_pg_offset() in vram helper
* improve nbo->offset calculation for nouveau

v4:
* minor coding style fixes in amdgpu and radeon
* remove unnecessary kerneldoc for internal function

v5:
* rebase on top of drm-misc-next
* fix return value of drm_gem_vram_pg_offset()
* add a comment in drm_gem_vram_pg_offset() to clearify why we return 0.

Nirmoy Das (8):
  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 v3
  drm/qxl: don't use ttm bo->offset
  drm/vram-helper: don't use ttm bo->offset v4
  drm/bochs: use drm_gem_vram_offset to get bo offset v2
  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 |  2 +-
 drivers/gpu/drm/bochs/bochs_kms.c           |  7 ++++-
 drivers/gpu/drm/drm_gem_vram_helper.c       | 11 +++++++-
 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        |  8 ++++++
 drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
 drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
 drivers/gpu/drm/nouveau/nouveau_dmem.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               |  5 ++--
 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 -
 36 files changed, 121 insertions(+), 77 deletions(-)

--
2.25.1

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/1] drm/amdgpu: move ttm bo->offset to amdgpu_bo
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  2020-04-01 18:42 ` [PATCH 2/8] drm/radeon: don't use ttm bo->offset Nirmoy Das
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, alexander.deucher, Huang Rui,
	sean, christian.koenig, kraxel

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>
Acked-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Christian König <christian.koenig@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 c687f5415b3f..26993a2b7fbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -918,7 +918,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));
 		}
@@ -1484,7 +1484,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 5e39ecd8cc28..32edd35d2ccf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -282,6 +282,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 c10ae1cdc1b9..fba21938c7cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -96,7 +96,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;
@@ -104,7 +103,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;
@@ -115,7 +113,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;
@@ -263,7 +260,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;
 }
@@ -750,6 +747,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.
  */
@@ -1164,9 +1182,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 6b22dc41ef13..b6086ab21069 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -112,6 +112,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 cf96c335b258..2a7a6f62d627 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -141,7 +141,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);
@@ -168,7 +168,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.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/8] drm/radeon: don't use ttm bo->offset
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
  2020-04-01 18:42 ` [PATCH 1/1] drm/amdgpu: move ttm bo->offset to amdgpu_bo Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  2020-04-01 18:42 ` [PATCH 3/8] drm/vmwgfx: " Nirmoy Das
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, alexander.deucher, sean,
	christian.koenig, kraxel

Calculate GPU offset in radeon_bo_gpu_offset without depending on
bo->offset.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-and-tested-by: Christian König <christian.koenig@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 30e32adc1fc6..b7c3fb2bfb54 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2828,6 +2828,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..60275b822f79 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 badf1b6d1549..1c8303468e8f 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;
@@ -82,7 +82,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;
@@ -104,7 +103,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.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/8] drm/vmwgfx: don't use ttm bo->offset
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
  2020-04-01 18:42 ` [PATCH 1/1] drm/amdgpu: move ttm bo->offset to amdgpu_bo Nirmoy Das
  2020-04-01 18:42 ` [PATCH 2/8] drm/radeon: don't use ttm bo->offset Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  2020-04-01 18:42 ` [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3 Nirmoy Das
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, alexander.deucher, sean,
	christian.koenig, kraxel

Calculate GPU offset within vmwgfx driver itself without depending on
bo->offset.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
Acked-by: Thomas Hellstrom <thellstrom@vmware.com>
Tested-by: Thomas Hellstrom <thellstrom@vmware.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 8b71bf6b58ef..1e59c019affa 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 != 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 367d5b87ee6a..4284c4bd444d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -3696,7 +3696,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 6941689085ed..a95156fc5db7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c
@@ -610,7 +610,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 3f3b2c7a208a..e7134aebeb81 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -750,7 +750,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;
@@ -763,7 +762,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.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
                   ` (2 preceding siblings ...)
  2020-04-01 18:42 ` [PATCH 3/8] drm/vmwgfx: " Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  2020-04-02  7:04   ` Christian König
  2020-04-01 18:42 ` [PATCH 5/8] drm/qxl: don't use ttm bo->offset Nirmoy Das
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, alexander.deucher, sean,
	christian.koenig, kraxel

Store ttm bo->offset in struct nouveau_bo instead.

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        |  8 ++++++++
 drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
 drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
 drivers/gpu/drm/nouveau/nouveau_dmem.c      |  2 +-
 drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++++-----
 15 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 1f08de4241e0..d06a93f2b38a 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 bb737f9281e6..ee0fd817185e 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -511,7 +511,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 b92dc3461bbd..bb84e4d54a33 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
@@ -298,7 +298,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 2b4b21b02e40..67f173c266ad 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1317,6 +1317,14 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
 			nouveau_vma_unmap(vma);
 		}
 	}
+
+	if (new_reg) {
+		if (new_reg->mm_node)
+			nvbo->offset = (new_reg->start << PAGE_SHIFT);
+		else
+			nvbo->offset = 0;
+	}
+
 }
 
 static int
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 d9381a053169..3d71dfcb2fde 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -162,7 +162,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_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 0ad5d87b5a8e..475ed53b99f1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -89,7 +89,7 @@ static unsigned long nouveau_dmem_page_addr(struct page *page)
 	struct nouveau_dmem_chunk *chunk = page->zone_device_data;
 	unsigned long idx = page_to_pfn(page) - chunk->pfn_first;
 
-	return (idx << PAGE_SHIFT) + chunk->bo->bo.offset;
+	return (idx << PAGE_SHIFT) + chunk->bo->offset;
 }
 
 static void nouveau_dmem_page_free(struct page *page)
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 24d543a01f43..1341c6fca3ed 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 f5ece1f94973..cadff37eade8 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)
@@ -516,7 +516,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 &&
@@ -527,7 +527,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++;
 		}
@@ -805,7 +805,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 {
@@ -840,7 +840,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.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/8] drm/qxl: don't use ttm bo->offset
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
                   ` (3 preceding siblings ...)
  2020-04-01 18:42 ` [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3 Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  2020-04-01 18:42 ` [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4 Nirmoy Das
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, alexander.deucher, sean,
	christian.koenig, kraxel

This patch removes slot->gpu_offset which is not required as
VRAM and PRIV slot are in separate PCI bar.

This patch also removes unused qxl_bo_gpu_offset()

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_drv.h    | 6 ++----
 drivers/gpu/drm/qxl/qxl_kms.c    | 5 ++---
 drivers/gpu/drm/qxl/qxl_object.h | 5 -----
 drivers/gpu/drm/qxl/qxl_ttm.c    | 9 ---------
 4 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 435126facc9b..a9c516328271 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -134,7 +134,6 @@ struct qxl_memslot {
 	uint64_t	start_phys_addr;
 	uint64_t	size;
 	uint64_t	high_bits;
-	uint64_t        gpu_offset;
 };
 
 enum {
@@ -308,10 +307,9 @@ 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 bo->tbo.mem.start */
 
-	/* 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) + offset);
 }
 
 /* qxl_display.c */
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 9eed1a375f24..797cfdf94ae9 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -87,11 +87,10 @@ static void setup_slot(struct qxl_device *qdev,
 	high_bits <<= (64 - (qdev->rom->slot_gen_bits + qdev->rom->slot_id_bits));
 	slot->high_bits = high_bits;
 
-	DRM_INFO("slot %d (%s): base 0x%08lx, size 0x%08lx, gpu_offset 0x%lx\n",
+	DRM_INFO("slot %d (%s): base 0x%08lx, size 0x%08lx\n",
 		 slot->index, slot->name,
 		 (unsigned long)slot->start_phys_addr,
-		 (unsigned long)slot->size,
-		 (unsigned long)slot->gpu_offset);
+		 (unsigned long)slot->size);
 }
 
 void qxl_reinit_memslots(struct qxl_device *qdev)
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 93a2eb14844b..1f02691a69c2 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -51,11 +51,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device *bdev)
 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 */
@@ -66,11 +61,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.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
                   ` (4 preceding siblings ...)
  2020-04-01 18:42 ` [PATCH 5/8] drm/qxl: don't use ttm bo->offset Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  2020-04-01 18:42 ` [PATCH 7/8] drm/bochs: use drm_gem_vram_offset to get bo offset v2 Nirmoy Das
  2020-04-01 18:42 ` [PATCH 8/8] drm/ttm: do not keep GPU dependent addresses Nirmoy Das
  7 siblings, 0 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, christian.koenig, kraxel

Calculate GEM VRAM bo's offset within vram-helper without depending on
bo->offset.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index b3201a70cbfc..e768a1e69d0c 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -198,6 +198,15 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_mmap_offset);

+static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
+{
+	/* Keep TTM behavior for now, remove when drivers are audited */
+	if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
+		return 0;
+
+	return gbo->bo.mem.start;
+}
+
 /**
  * drm_gem_vram_offset() - \
 	Returns a GEM VRAM object's offset in video memory
@@ -214,7 +223,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
 {
 	if (WARN_ON_ONCE(!gbo->pin_count))
 		return (s64)-ENODEV;
-	return gbo->bo.offset;
+	return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
 }
 EXPORT_SYMBOL(drm_gem_vram_offset);

--
2.25.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/8] drm/bochs: use drm_gem_vram_offset to get bo offset v2
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
                   ` (5 preceding siblings ...)
  2020-04-01 18:42 ` [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4 Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  2020-04-01 18:42 ` [PATCH 8/8] drm/ttm: do not keep GPU dependent addresses Nirmoy Das
  7 siblings, 0 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, christian.koenig, kraxel

Switch over to GEM VRAM's implementation to retrieve bo->offset.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/bochs/bochs_kms.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 7f4bcfad87e9..25a23e983b7c 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -29,16 +29,21 @@ static void bochs_plane_update(struct bochs_device *bochs,
 			       struct drm_plane_state *state)
 {
 	struct drm_gem_vram_object *gbo;
+	s64 gpu_addr;
 
 	if (!state->fb || !bochs->stride)
 		return;
 
 	gbo = drm_gem_vram_of_gem(state->fb->obj[0]);
+	gpu_addr = drm_gem_vram_offset(gbo);
+	if (WARN_ON_ONCE(gpu_addr < 0))
+		return; /* Bug: we didn't pin the BO to VRAM in prepare_fb. */
+
 	bochs_hw_setbase(bochs,
 			 state->crtc_x,
 			 state->crtc_y,
 			 state->fb->pitches[0],
-			 state->fb->offsets[0] + gbo->bo.offset);
+			 state->fb->offsets[0] + gpu_addr);
 	bochs_hw_setformat(bochs, state->fb->format);
 }
 
-- 
2.25.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/8] drm/ttm: do not keep GPU dependent addresses
  2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
                   ` (6 preceding siblings ...)
  2020-04-01 18:42 ` [PATCH 7/8] drm/bochs: use drm_gem_vram_offset to get bo offset v2 Nirmoy Das
@ 2020-04-01 18:42 ` Nirmoy Das
  7 siblings, 0 replies; 19+ messages in thread
From: Nirmoy Das @ 2020-04-01 18:42 UTC (permalink / raw)
  To: dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, alexander.deucher, sean,
	christian.koenig, kraxel

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

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Christian König <christian.koenig@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 f73b81c2576e..f78cfc76ad78 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);
@@ -343,12 +342,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 moved:
 	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 b9bc1b00142e..d6f39ee5bf5d 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -213,8 +213,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;
 };
 
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index c9e0fd09f4b2..c8ce6c181abe 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.1

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3
  2020-04-01 18:42 ` [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3 Nirmoy Das
@ 2020-04-02  7:04   ` Christian König
  0 siblings, 0 replies; 19+ messages in thread
From: Christian König @ 2020-04-02  7:04 UTC (permalink / raw)
  To: Nirmoy Das, dri-devel
  Cc: thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, kraxel, alexander.deucher, sean,
	bskeggs

Am 01.04.20 um 20:42 schrieb Nirmoy Das:
> Store ttm bo->offset in struct nouveau_bo instead.
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>

Acked-by: Christian König <christian.koenig@amd.com>

It would indeed be better if one of the nouveau maintainer could take a 
look since this is the last driver blocking this change.

On the other hand it is so straight forward that it most likely doesn't 
cause any problems.

As soon as I get an ok I'm going to merge this through drm-misc-next.

Regards,
Christian.

> ---
>   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        |  8 ++++++++
>   drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
>   drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_dmem.c      |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++++-----
>   15 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 1f08de4241e0..d06a93f2b38a 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 bb737f9281e6..ee0fd817185e 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> @@ -511,7 +511,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 b92dc3461bbd..bb84e4d54a33 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c
> @@ -298,7 +298,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 2b4b21b02e40..67f173c266ad 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1317,6 +1317,14 @@ nouveau_bo_move_ntfy(struct ttm_buffer_object *bo, bool evict,
>   			nouveau_vma_unmap(vma);
>   		}
>   	}
> +
> +	if (new_reg) {
> +		if (new_reg->mm_node)
> +			nvbo->offset = (new_reg->start << PAGE_SHIFT);
> +		else
> +			nvbo->offset = 0;
> +	}
> +
>   }
>   
>   static int
> 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 d9381a053169..3d71dfcb2fde 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
> @@ -162,7 +162,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_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> index 0ad5d87b5a8e..475ed53b99f1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
> @@ -89,7 +89,7 @@ static unsigned long nouveau_dmem_page_addr(struct page *page)
>   	struct nouveau_dmem_chunk *chunk = page->zone_device_data;
>   	unsigned long idx = page_to_pfn(page) - chunk->pfn_first;
>   
> -	return (idx << PAGE_SHIFT) + chunk->bo->bo.offset;
> +	return (idx << PAGE_SHIFT) + chunk->bo->offset;
>   }
>   
>   static void nouveau_dmem_page_free(struct page *page)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> index 24d543a01f43..1341c6fca3ed 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 f5ece1f94973..cadff37eade8 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)
> @@ -516,7 +516,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 &&
> @@ -527,7 +527,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++;
>   		}
> @@ -805,7 +805,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 {
> @@ -840,7 +840,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);

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-09-17 12:32         ` Christian König
  2020-09-17 14:57           ` Thomas Zimmermann
@ 2020-09-21 14:26           ` Thomas Zimmermann
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2020-09-21 14:26 UTC (permalink / raw)
  To: Christian König, Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, kraxel


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

Hi

Am 17.09.20 um 14:32 schrieb Christian König:
> Am 17.09.20 um 14:29 schrieb Thomas Zimmermann:
>> Hi Christian
>>
>> Am 17.09.20 um 13:12 schrieb Christian König:
>>> Hi Thomas,
>>>
>>> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
>>>> Hi
>>>>
>>>> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
>>>>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>>>>> bo->offset.
>>>>>
>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> ---
>>>>>    drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>>>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> index 0023ce1d2cf7..ad096600d89f 100644
>>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct
>>>>> drm_gem_vram_object *gbo)
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>>>>    +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>>>>> +{
>>>>> +    /* Keep TTM behavior for now, remove when drivers are audited */
>>>>> +    if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>>>> At this line I got
>>> Sounds like ast forgot to pin the cursor to VRAM.
>>>
>>> If you look at ast_cursor_page_flip(), you see:
>>>>          off = drm_gem_vram_offset(gbo);
>>>>          if (drm_WARN_ON_ONCE(dev, off < 0))
>>>>                  return; /* Bug: we didn't pin the cursor HW BO to
>>>> VRAM. */
>>> The drm_WARN_ON_ONCE() just never triggered before because it checks for
>>> the wrong condition (off < 0).
>> GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV
>> if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere).
>> That's what we're testing here. Two cursor BOs are permanently pinned to
>> the top of VRAM memory by ast_cursor_init(). If perma-pinning in
>> ast_cursor_init() fails, driver initialization should fail entirely.
>>
>> These cursor BOs do some sort of double buffering, On successful
>> initialization, the actual cursor image is later blit-ed into one of the
>> BOs.
>>
>> All this used to work from what I can tell. Is there any chance that the
>> recent TTM refactoring broke this?
> 
> Yeah, certainly possible. If you have time please bisect.

Found it.

https://patchwork.freedesktop.org/patch/391418/

Best regards
Thomas

> 
> Thanks,
> Christian.
> 
>>
>> Best regards
>> Thomas
>>
>>> Regards,
>>> Christian.
>>>
>>>> [  146.133821] ------------[ cut here ]------------
>>>> [  146.133872] WARNING: CPU: 6 PID: 7 at
>>>> drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
>>>> [drm_vram_helper]
>>>> [  146.133880] Modules linked in: fuse(E) af_packet(E)
>>>> ebtable_filter(E)
>>>> ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
>>>> ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
>>>> intel_powerclamp(E) coretemp(E) kv)
>>>> [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
>>>> [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted:
>>>> G            E
>>>>       5.9.0-rc4-1-default+ #492
>>>> [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
>>>> FIRE X2270 M2, BIOS 2.05    07/01/2010
>>>> [  146.134099] Workqueue: events_unbound commit_work
>>>> [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60
>>>> [drm_vram_helper]
>>>> [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
>>>> d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
>>>> c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
>>>> [  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
>>>> [  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
>>>> ffffffffc032323b
>>>> [  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
>>>> ffff888111155278
>>>> [  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
>>>> 0000000000000002
>>>> [  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
>>>> ffff888109091050
>>>> [  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
>>>> 0000000000000000
>>>> [  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
>>>> knlGS:0000000000000000
>>>> [  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
>>>> 00000000000206e0
>>>> [  146.134223] Call Trace:
>>>> [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
>>>> [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
>>>> [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
>>>> [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
>>>> [  146.134357]  commit_tail+0x103/0x1c0
>>>> [  146.134390]  process_one_work+0x56a/0xa60
>>>> [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
>>>> [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
>>>> [  146.134460]  ? __lock_contended+0x4a0/0x4a0
>>>> [  146.134491]  ? worker_thread+0x150/0x620
>>>> [  146.134521]  worker_thread+0x8b/0x620
>>>> [  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
>>>> [  146.134583]  ? process_one_work+0xa60/0xa60
>>>> [  146.134597]  kthread+0x1e4/0x210
>>>> [  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
>>>> [  146.134637]  ret_from_fork+0x22/0x30
>>>> [  146.134698] irq event stamp: 74111
>>>> [  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
>>>> console_unlock+0x539/0x670
>>>> [  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
>>>> console_unlock+0x52f/0x670
>>>> [  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
>>>> wb_workfn+0x3f5/0x430
>>>> [  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
>>>> wb_wakeup_delayed+0x40/0xa0
>>>> [  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---
>>>>
>>>>
>>>> Happens with ast when doing
>>>>
>>>>     weston-launch
>>>>
>>>>
>>>>
>>>>> +        return 0;
>>>>> +
>>>>> +    return gbo->bo.mem.start;
>>>>> +}
>>>>> +
>>>>>    /**
>>>>>     * drm_gem_vram_offset() - \
>>>>>        Returns a GEM VRAM object's offset in video memory
>>>>> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct
>>>>> drm_gem_vram_object *gbo)
>>>>>    {
>>>>>        if (WARN_ON_ONCE(!gbo->pin_count))
>>>>>            return (s64)-ENODEV;
>>>>> -    return gbo->bo.offset;
>>>>> +    return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_gem_vram_offset);
>>>>>   
>>> _______________________________________________
>>> 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

-- 
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: 516 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-09-17 12:32         ` Christian König
@ 2020-09-17 14:57           ` Thomas Zimmermann
  2020-09-21 14:26           ` Thomas Zimmermann
  1 sibling, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2020-09-17 14:57 UTC (permalink / raw)
  To: Christian König, Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, kraxel


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

Hi

Am 17.09.20 um 14:32 schrieb Christian König:
> Am 17.09.20 um 14:29 schrieb Thomas Zimmermann:
>> Hi Christian
>>
>> Am 17.09.20 um 13:12 schrieb Christian König:
>>> Hi Thomas,
>>>
>>> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
>>>> Hi
>>>>
>>>> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
>>>>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>>>>> bo->offset.
>>>>>
>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> ---
>>>>>    drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>>>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> index 0023ce1d2cf7..ad096600d89f 100644
>>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct
>>>>> drm_gem_vram_object *gbo)
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>>>>    +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>>>>> +{
>>>>> +    /* Keep TTM behavior for now, remove when drivers are audited */
>>>>> +    if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>>>> At this line I got
>>> Sounds like ast forgot to pin the cursor to VRAM.
>>>
>>> If you look at ast_cursor_page_flip(), you see:
>>>>          off = drm_gem_vram_offset(gbo);
>>>>          if (drm_WARN_ON_ONCE(dev, off < 0))
>>>>                  return; /* Bug: we didn't pin the cursor HW BO to
>>>> VRAM. */
>>> The drm_WARN_ON_ONCE() just never triggered before because it checks for
>>> the wrong condition (off < 0).
>> GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV
>> if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere).
>> That's what we're testing here. Two cursor BOs are permanently pinned to
>> the top of VRAM memory by ast_cursor_init(). If perma-pinning in
>> ast_cursor_init() fails, driver initialization should fail entirely.
>>
>> These cursor BOs do some sort of double buffering, On successful
>> initialization, the actual cursor image is later blit-ed into one of the
>> BOs.
>>
>> All this used to work from what I can tell. Is there any chance that the
>> recent TTM refactoring broke this?
> 
> Yeah, certainly possible. If you have time please bisect.

FYI the bug is not in c44264f9f729 ("Merge tag 'v5.8' into drm-next")
from Aug 11. I'll try to bisect.

Best regards
Thomas

> 
> Thanks,
> Christian.
> 
>>
>> Best regards
>> Thomas
>>
>>> Regards,
>>> Christian.
>>>
>>>> [  146.133821] ------------[ cut here ]------------
>>>> [  146.133872] WARNING: CPU: 6 PID: 7 at
>>>> drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
>>>> [drm_vram_helper]
>>>> [  146.133880] Modules linked in: fuse(E) af_packet(E)
>>>> ebtable_filter(E)
>>>> ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
>>>> ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
>>>> intel_powerclamp(E) coretemp(E) kv)
>>>> [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
>>>> [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted:
>>>> G            E
>>>>       5.9.0-rc4-1-default+ #492
>>>> [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
>>>> FIRE X2270 M2, BIOS 2.05    07/01/2010
>>>> [  146.134099] Workqueue: events_unbound commit_work
>>>> [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60
>>>> [drm_vram_helper]
>>>> [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
>>>> d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
>>>> c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
>>>> [  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
>>>> [  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
>>>> ffffffffc032323b
>>>> [  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
>>>> ffff888111155278
>>>> [  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
>>>> 0000000000000002
>>>> [  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
>>>> ffff888109091050
>>>> [  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
>>>> 0000000000000000
>>>> [  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
>>>> knlGS:0000000000000000
>>>> [  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
>>>> 00000000000206e0
>>>> [  146.134223] Call Trace:
>>>> [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
>>>> [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
>>>> [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
>>>> [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
>>>> [  146.134357]  commit_tail+0x103/0x1c0
>>>> [  146.134390]  process_one_work+0x56a/0xa60
>>>> [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
>>>> [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
>>>> [  146.134460]  ? __lock_contended+0x4a0/0x4a0
>>>> [  146.134491]  ? worker_thread+0x150/0x620
>>>> [  146.134521]  worker_thread+0x8b/0x620
>>>> [  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
>>>> [  146.134583]  ? process_one_work+0xa60/0xa60
>>>> [  146.134597]  kthread+0x1e4/0x210
>>>> [  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
>>>> [  146.134637]  ret_from_fork+0x22/0x30
>>>> [  146.134698] irq event stamp: 74111
>>>> [  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
>>>> console_unlock+0x539/0x670
>>>> [  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
>>>> console_unlock+0x52f/0x670
>>>> [  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
>>>> wb_workfn+0x3f5/0x430
>>>> [  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
>>>> wb_wakeup_delayed+0x40/0xa0
>>>> [  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---
>>>>
>>>>
>>>> Happens with ast when doing
>>>>
>>>>     weston-launch
>>>>
>>>>
>>>>
>>>>> +        return 0;
>>>>> +
>>>>> +    return gbo->bo.mem.start;
>>>>> +}
>>>>> +
>>>>>    /**
>>>>>     * drm_gem_vram_offset() - \
>>>>>        Returns a GEM VRAM object's offset in video memory
>>>>> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct
>>>>> drm_gem_vram_object *gbo)
>>>>>    {
>>>>>        if (WARN_ON_ONCE(!gbo->pin_count))
>>>>>            return (s64)-ENODEV;
>>>>> -    return gbo->bo.offset;
>>>>> +    return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_gem_vram_offset);
>>>>>   
>>> _______________________________________________
>>> 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

-- 
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: 516 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-09-17 12:58         ` Nirmoy
@ 2020-09-17 13:18           ` Thomas Zimmermann
  0 siblings, 0 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2020-09-17 13:18 UTC (permalink / raw)
  To: Nirmoy, Christian König, Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, kraxel


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

Hi

Am 17.09.20 um 14:58 schrieb Nirmoy:
> 
> On 9/17/20 2:29 PM, Thomas Zimmermann wrote:
>> Hi Christian
>>
>> Am 17.09.20 um 13:12 schrieb Christian König:
>>> Hi Thomas,
>>>
>>> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
>>>> Hi
>>>>
>>>> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
>>>>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>>>>> bo->offset.
>>>>>
>>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>>> ---
>>>>>    drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>>>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> index 0023ce1d2cf7..ad096600d89f 100644
>>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>>> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct
>>>>> drm_gem_vram_object *gbo)
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>>>>    +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>>>>> +{
>>>>> +    /* Keep TTM behavior for now, remove when drivers are audited */
>>>>> +    if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>>>> At this line I got
>>> Sounds like ast forgot to pin the cursor to VRAM.
>>>
>>> If you look at ast_cursor_page_flip(), you see:
>>>>          off = drm_gem_vram_offset(gbo);
>>>>          if (drm_WARN_ON_ONCE(dev, off < 0))
>>>>                  return; /* Bug: we didn't pin the cursor HW BO to
>>>> VRAM. */
>>> The drm_WARN_ON_ONCE() just never triggered before because it checks for
>>> the wrong condition (off < 0).
>> GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV
>> if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere).
>> That's what we're testing here. Two cursor BOs are permanently pinned to
>> the top of VRAM memory by ast_cursor_init(). If perma-pinning in
>> ast_cursor_init() fails, driver initialization should fail entirely.
>>
>> These cursor BOs do some sort of double buffering, On successful
>> initialization, the actual cursor image is later blit-ed into one of the
>> BOs.
>>
>> All this used to work from what I can tell. Is there any chance that the
>> recent TTM refactoring broke this?
> 
> 
> In ast_cursor_blit(), cursor bo is pinned without any flags -->
> "drm_gem_vram_pin(gbo, 0);"
> 
> But in ast_cursor_init() both cursors are pinned with TOPDOWN and VRAM
> flag.
> 
> I wonder if that could cause any issue.

It shouldn't. The BO has been pinned already when blit is running. Only
the pin count is being incremented.

Best regards
Thomas

> 
> 
> Regards,
> 
> Nirmoy
> 
> 
> 
>>
>> Best regards
>> Thomas
>>
>>> Regards,
>>> Christian.
>>>
>>>> [  146.133821] ------------[ cut here ]------------
>>>> [  146.133872] WARNING: CPU: 6 PID: 7 at
>>>> drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
>>>> [drm_vram_helper]
>>>> [  146.133880] Modules linked in: fuse(E) af_packet(E)
>>>> ebtable_filter(E)
>>>> ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
>>>> ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
>>>> intel_powerclamp(E) coretemp(E) kv)
>>>> [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
>>>> [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted:
>>>> G            E
>>>>       5.9.0-rc4-1-default+ #492
>>>> [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
>>>> FIRE X2270 M2, BIOS 2.05    07/01/2010
>>>> [  146.134099] Workqueue: events_unbound commit_work
>>>> [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60
>>>> [drm_vram_helper]
>>>> [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
>>>> d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
>>>> c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
>>>> [  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
>>>> [  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
>>>> ffffffffc032323b
>>>> [  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
>>>> ffff888111155278
>>>> [  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
>>>> 0000000000000002
>>>> [  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
>>>> ffff888109091050
>>>> [  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
>>>> 0000000000000000
>>>> [  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
>>>> knlGS:0000000000000000
>>>> [  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
>>>> 00000000000206e0
>>>> [  146.134223] Call Trace:
>>>> [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
>>>> [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
>>>> [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
>>>> [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
>>>> [  146.134357]  commit_tail+0x103/0x1c0
>>>> [  146.134390]  process_one_work+0x56a/0xa60
>>>> [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
>>>> [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
>>>> [  146.134460]  ? __lock_contended+0x4a0/0x4a0
>>>> [  146.134491]  ? worker_thread+0x150/0x620
>>>> [  146.134521]  worker_thread+0x8b/0x620
>>>> [  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
>>>> [  146.134583]  ? process_one_work+0xa60/0xa60
>>>> [  146.134597]  kthread+0x1e4/0x210
>>>> [  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
>>>> [  146.134637]  ret_from_fork+0x22/0x30
>>>> [  146.134698] irq event stamp: 74111
>>>> [  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
>>>> console_unlock+0x539/0x670
>>>> [  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
>>>> console_unlock+0x52f/0x670
>>>> [  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
>>>> wb_workfn+0x3f5/0x430
>>>> [  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
>>>> wb_wakeup_delayed+0x40/0xa0
>>>> [  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---
>>>>
>>>>
>>>> Happens with ast when doing
>>>>
>>>>     weston-launch
>>>>
>>>>
>>>>
>>>>> +        return 0;
>>>>> +
>>>>> +    return gbo->bo.mem.start;
>>>>> +}
>>>>> +
>>>>>    /**
>>>>>     * drm_gem_vram_offset() - \
>>>>>        Returns a GEM VRAM object's offset in video memory
>>>>> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct
>>>>> drm_gem_vram_object *gbo)
>>>>>    {
>>>>>        if (WARN_ON_ONCE(!gbo->pin_count))
>>>>>            return (s64)-ENODEV;
>>>>> -    return gbo->bo.offset;
>>>>> +    return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>>>>>    }
>>>>>    EXPORT_SYMBOL(drm_gem_vram_offset);
>>>>>   
>>> _______________________________________________
>>> 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

-- 
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: 516 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-09-17 12:29       ` Thomas Zimmermann
  2020-09-17 12:32         ` Christian König
@ 2020-09-17 12:58         ` Nirmoy
  2020-09-17 13:18           ` Thomas Zimmermann
  1 sibling, 1 reply; 19+ messages in thread
From: Nirmoy @ 2020-09-17 12:58 UTC (permalink / raw)
  To: Thomas Zimmermann, Christian König, Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, kraxel


On 9/17/20 2:29 PM, Thomas Zimmermann wrote:
> Hi Christian
>
> Am 17.09.20 um 13:12 schrieb Christian König:
>> Hi Thomas,
>>
>> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
>>> Hi
>>>
>>> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
>>>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>>>> bo->offset.
>>>>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> ---
>>>>    drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> index 0023ce1d2cf7..ad096600d89f 100644
>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct
>>>> drm_gem_vram_object *gbo)
>>>>    }
>>>>    EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>>>    +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>>>> +{
>>>> +    /* Keep TTM behavior for now, remove when drivers are audited */
>>>> +    if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>>> At this line I got
>> Sounds like ast forgot to pin the cursor to VRAM.
>>
>> If you look at ast_cursor_page_flip(), you see:
>>>          off = drm_gem_vram_offset(gbo);
>>>          if (drm_WARN_ON_ONCE(dev, off < 0))
>>>                  return; /* Bug: we didn't pin the cursor HW BO to
>>> VRAM. */
>> The drm_WARN_ON_ONCE() just never triggered before because it checks for
>> the wrong condition (off < 0).
> GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV
> if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere).
> That's what we're testing here. Two cursor BOs are permanently pinned to
> the top of VRAM memory by ast_cursor_init(). If perma-pinning in
> ast_cursor_init() fails, driver initialization should fail entirely.
>
> These cursor BOs do some sort of double buffering, On successful
> initialization, the actual cursor image is later blit-ed into one of the
> BOs.
>
> All this used to work from what I can tell. Is there any chance that the
> recent TTM refactoring broke this?


In ast_cursor_blit(), cursor bo is pinned without any flags --> 
"drm_gem_vram_pin(gbo, 0);"

But in ast_cursor_init() both cursors are pinned with TOPDOWN and VRAM flag.

I wonder if that could cause any issue.


Regards,

Nirmoy



>
> Best regards
> Thomas
>
>> Regards,
>> Christian.
>>
>>> [  146.133821] ------------[ cut here ]------------
>>> [  146.133872] WARNING: CPU: 6 PID: 7 at
>>> drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
>>> [drm_vram_helper]
>>> [  146.133880] Modules linked in: fuse(E) af_packet(E) ebtable_filter(E)
>>> ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
>>> ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
>>> intel_powerclamp(E) coretemp(E) kv)
>>> [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
>>> [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted: G            E
>>>       5.9.0-rc4-1-default+ #492
>>> [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
>>> FIRE X2270 M2, BIOS 2.05    07/01/2010
>>> [  146.134099] Workqueue: events_unbound commit_work
>>> [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60 [drm_vram_helper]
>>> [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
>>> d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
>>> c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
>>> [  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
>>> [  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
>>> ffffffffc032323b
>>> [  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
>>> ffff888111155278
>>> [  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
>>> 0000000000000002
>>> [  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
>>> ffff888109091050
>>> [  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
>>> 0000000000000000
>>> [  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
>>> knlGS:0000000000000000
>>> [  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
>>> 00000000000206e0
>>> [  146.134223] Call Trace:
>>> [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
>>> [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
>>> [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
>>> [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
>>> [  146.134357]  commit_tail+0x103/0x1c0
>>> [  146.134390]  process_one_work+0x56a/0xa60
>>> [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
>>> [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
>>> [  146.134460]  ? __lock_contended+0x4a0/0x4a0
>>> [  146.134491]  ? worker_thread+0x150/0x620
>>> [  146.134521]  worker_thread+0x8b/0x620
>>> [  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
>>> [  146.134583]  ? process_one_work+0xa60/0xa60
>>> [  146.134597]  kthread+0x1e4/0x210
>>> [  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
>>> [  146.134637]  ret_from_fork+0x22/0x30
>>> [  146.134698] irq event stamp: 74111
>>> [  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
>>> console_unlock+0x539/0x670
>>> [  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
>>> console_unlock+0x52f/0x670
>>> [  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
>>> wb_workfn+0x3f5/0x430
>>> [  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
>>> wb_wakeup_delayed+0x40/0xa0
>>> [  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---
>>>
>>>
>>> Happens with ast when doing
>>>
>>>     weston-launch
>>>
>>>
>>>
>>>> +        return 0;
>>>> +
>>>> +    return gbo->bo.mem.start;
>>>> +}
>>>> +
>>>>    /**
>>>>     * drm_gem_vram_offset() - \
>>>>        Returns a GEM VRAM object's offset in video memory
>>>> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct
>>>> drm_gem_vram_object *gbo)
>>>>    {
>>>>        if (WARN_ON_ONCE(!gbo->pin_count))
>>>>            return (s64)-ENODEV;
>>>> -    return gbo->bo.offset;
>>>> +    return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>>>>    }
>>>>    EXPORT_SYMBOL(drm_gem_vram_offset);
>>>>   
>> _______________________________________________
>> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-09-17 12:29       ` Thomas Zimmermann
@ 2020-09-17 12:32         ` Christian König
  2020-09-17 14:57           ` Thomas Zimmermann
  2020-09-21 14:26           ` Thomas Zimmermann
  2020-09-17 12:58         ` Nirmoy
  1 sibling, 2 replies; 19+ messages in thread
From: Christian König @ 2020-09-17 12:32 UTC (permalink / raw)
  To: Thomas Zimmermann, Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, kraxel

Am 17.09.20 um 14:29 schrieb Thomas Zimmermann:
> Hi Christian
>
> Am 17.09.20 um 13:12 schrieb Christian König:
>> Hi Thomas,
>>
>> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
>>> Hi
>>>
>>> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
>>>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>>>> bo->offset.
>>>>
>>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> ---
>>>>    drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>>>>    1 file changed, 10 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> index 0023ce1d2cf7..ad096600d89f 100644
>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct
>>>> drm_gem_vram_object *gbo)
>>>>    }
>>>>    EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>>>    +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>>>> +{
>>>> +    /* Keep TTM behavior for now, remove when drivers are audited */
>>>> +    if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>>> At this line I got
>> Sounds like ast forgot to pin the cursor to VRAM.
>>
>> If you look at ast_cursor_page_flip(), you see:
>>>          off = drm_gem_vram_offset(gbo);
>>>          if (drm_WARN_ON_ONCE(dev, off < 0))
>>>                  return; /* Bug: we didn't pin the cursor HW BO to
>>> VRAM. */
>> The drm_WARN_ON_ONCE() just never triggered before because it checks for
>> the wrong condition (off < 0).
> GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV
> if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere).
> That's what we're testing here. Two cursor BOs are permanently pinned to
> the top of VRAM memory by ast_cursor_init(). If perma-pinning in
> ast_cursor_init() fails, driver initialization should fail entirely.
>
> These cursor BOs do some sort of double buffering, On successful
> initialization, the actual cursor image is later blit-ed into one of the
> BOs.
>
> All this used to work from what I can tell. Is there any chance that the
> recent TTM refactoring broke this?

Yeah, certainly possible. If you have time please bisect.

Thanks,
Christian.

>
> Best regards
> Thomas
>
>> Regards,
>> Christian.
>>
>>> [  146.133821] ------------[ cut here ]------------
>>> [  146.133872] WARNING: CPU: 6 PID: 7 at
>>> drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
>>> [drm_vram_helper]
>>> [  146.133880] Modules linked in: fuse(E) af_packet(E) ebtable_filter(E)
>>> ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
>>> ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
>>> intel_powerclamp(E) coretemp(E) kv)
>>> [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
>>> [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted: G            E
>>>       5.9.0-rc4-1-default+ #492
>>> [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
>>> FIRE X2270 M2, BIOS 2.05    07/01/2010
>>> [  146.134099] Workqueue: events_unbound commit_work
>>> [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60 [drm_vram_helper]
>>> [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
>>> d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
>>> c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
>>> [  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
>>> [  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
>>> ffffffffc032323b
>>> [  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
>>> ffff888111155278
>>> [  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
>>> 0000000000000002
>>> [  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
>>> ffff888109091050
>>> [  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
>>> 0000000000000000
>>> [  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
>>> knlGS:0000000000000000
>>> [  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
>>> 00000000000206e0
>>> [  146.134223] Call Trace:
>>> [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
>>> [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
>>> [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
>>> [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
>>> [  146.134357]  commit_tail+0x103/0x1c0
>>> [  146.134390]  process_one_work+0x56a/0xa60
>>> [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
>>> [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
>>> [  146.134460]  ? __lock_contended+0x4a0/0x4a0
>>> [  146.134491]  ? worker_thread+0x150/0x620
>>> [  146.134521]  worker_thread+0x8b/0x620
>>> [  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
>>> [  146.134583]  ? process_one_work+0xa60/0xa60
>>> [  146.134597]  kthread+0x1e4/0x210
>>> [  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
>>> [  146.134637]  ret_from_fork+0x22/0x30
>>> [  146.134698] irq event stamp: 74111
>>> [  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
>>> console_unlock+0x539/0x670
>>> [  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
>>> console_unlock+0x52f/0x670
>>> [  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
>>> wb_workfn+0x3f5/0x430
>>> [  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
>>> wb_wakeup_delayed+0x40/0xa0
>>> [  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---
>>>
>>>
>>> Happens with ast when doing
>>>
>>>     weston-launch
>>>
>>>
>>>
>>>> +        return 0;
>>>> +
>>>> +    return gbo->bo.mem.start;
>>>> +}
>>>> +
>>>>    /**
>>>>     * drm_gem_vram_offset() - \
>>>>        Returns a GEM VRAM object's offset in video memory
>>>> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct
>>>> drm_gem_vram_object *gbo)
>>>>    {
>>>>        if (WARN_ON_ONCE(!gbo->pin_count))
>>>>            return (s64)-ENODEV;
>>>> -    return gbo->bo.offset;
>>>> +    return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>>>>    }
>>>>    EXPORT_SYMBOL(drm_gem_vram_offset);
>>>>   
>> _______________________________________________
>> 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-09-17 11:12     ` Christian König
@ 2020-09-17 12:29       ` Thomas Zimmermann
  2020-09-17 12:32         ` Christian König
  2020-09-17 12:58         ` Nirmoy
  0 siblings, 2 replies; 19+ messages in thread
From: Thomas Zimmermann @ 2020-09-17 12:29 UTC (permalink / raw)
  To: Christian König, Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, kraxel


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

Hi Christian

Am 17.09.20 um 13:12 schrieb Christian König:
> Hi Thomas,
> 
> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
>>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>>> bo->offset.
>>>
>>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> ---
>>>   drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> index 0023ce1d2cf7..ad096600d89f 100644
>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct
>>> drm_gem_vram_object *gbo)
>>>   }
>>>   EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>>   +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>>> +{
>>> +    /* Keep TTM behavior for now, remove when drivers are audited */
>>> +    if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
>> At this line I got
> 
> Sounds like ast forgot to pin the cursor to VRAM.
> 
> If you look at ast_cursor_page_flip(), you see:
>>         off = drm_gem_vram_offset(gbo);
>>         if (drm_WARN_ON_ONCE(dev, off < 0))
>>                 return; /* Bug: we didn't pin the cursor HW BO to
>> VRAM. */
> 
> The drm_WARN_ON_ONCE() just never triggered before because it checks for
> the wrong condition (off < 0).

GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV
if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere).
That's what we're testing here. Two cursor BOs are permanently pinned to
the top of VRAM memory by ast_cursor_init(). If perma-pinning in
ast_cursor_init() fails, driver initialization should fail entirely.

These cursor BOs do some sort of double buffering, On successful
initialization, the actual cursor image is later blit-ed into one of the
BOs.

All this used to work from what I can tell. Is there any chance that the
recent TTM refactoring broke this?

Best regards
Thomas

> 
> Regards,
> Christian.
> 
>>
>> [  146.133821] ------------[ cut here ]------------
>> [  146.133872] WARNING: CPU: 6 PID: 7 at
>> drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
>> [drm_vram_helper]
>> [  146.133880] Modules linked in: fuse(E) af_packet(E) ebtable_filter(E)
>> ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
>> ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
>> intel_powerclamp(E) coretemp(E) kv)
>> [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
>> [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted: G            E
>>      5.9.0-rc4-1-default+ #492
>> [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
>> FIRE X2270 M2, BIOS 2.05    07/01/2010
>> [  146.134099] Workqueue: events_unbound commit_work
>> [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60 [drm_vram_helper]
>> [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
>> d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
>> c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
>> [  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
>> [  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
>> ffffffffc032323b
>> [  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
>> ffff888111155278
>> [  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
>> 0000000000000002
>> [  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
>> ffff888109091050
>> [  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
>> 0000000000000000
>> [  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
>> knlGS:0000000000000000
>> [  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
>> 00000000000206e0
>> [  146.134223] Call Trace:
>> [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
>> [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
>> [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
>> [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
>> [  146.134357]  commit_tail+0x103/0x1c0
>> [  146.134390]  process_one_work+0x56a/0xa60
>> [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
>> [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
>> [  146.134460]  ? __lock_contended+0x4a0/0x4a0
>> [  146.134491]  ? worker_thread+0x150/0x620
>> [  146.134521]  worker_thread+0x8b/0x620
>> [  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
>> [  146.134583]  ? process_one_work+0xa60/0xa60
>> [  146.134597]  kthread+0x1e4/0x210
>> [  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
>> [  146.134637]  ret_from_fork+0x22/0x30
>> [  146.134698] irq event stamp: 74111
>> [  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
>> console_unlock+0x539/0x670
>> [  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
>> console_unlock+0x52f/0x670
>> [  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
>> wb_workfn+0x3f5/0x430
>> [  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
>> wb_wakeup_delayed+0x40/0xa0
>> [  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---
>>
>>
>> Happens with ast when doing
>>
>>    weston-launch
>>
>>
>>
>>> +        return 0;
>>> +
>>> +    return gbo->bo.mem.start;
>>> +}
>>> +
>>>   /**
>>>    * drm_gem_vram_offset() - \
>>>       Returns a GEM VRAM object's offset in video memory
>>> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct
>>> drm_gem_vram_object *gbo)
>>>   {
>>>       if (WARN_ON_ONCE(!gbo->pin_count))
>>>           return (s64)-ENODEV;
>>> -    return gbo->bo.offset;
>>> +    return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>>>   }
>>>   EXPORT_SYMBOL(drm_gem_vram_offset);
>>>  
> 
> _______________________________________________
> 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: 516 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-09-17 10:51   ` Thomas Zimmermann
@ 2020-09-17 11:12     ` Christian König
  2020-09-17 12:29       ` Thomas Zimmermann
  0 siblings, 1 reply; 19+ messages in thread
From: Christian König @ 2020-09-17 11:12 UTC (permalink / raw)
  To: Thomas Zimmermann, Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, kraxel

Hi Thomas,

Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
> Hi
>
> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
>> Calculate GEM VRAM bo's offset within vram-helper without depending on
>> bo->offset.
>>
>> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>   drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 0023ce1d2cf7..ad096600d89f 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
>>   }
>>   EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>>   
>> +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
>> +{
>> +	/* Keep TTM behavior for now, remove when drivers are audited */
>> +	if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
> At this line I got

Sounds like ast forgot to pin the cursor to VRAM.

If you look at ast_cursor_page_flip(), you see:
>         off = drm_gem_vram_offset(gbo);
>         if (drm_WARN_ON_ONCE(dev, off < 0))
>                 return; /* Bug: we didn't pin the cursor HW BO to VRAM. */

The drm_WARN_ON_ONCE() just never triggered before because it checks for 
the wrong condition (off < 0).

Regards,
Christian.

>
> [  146.133821] ------------[ cut here ]------------
> [  146.133872] WARNING: CPU: 6 PID: 7 at
> drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
> [drm_vram_helper]
> [  146.133880] Modules linked in: fuse(E) af_packet(E) ebtable_filter(E)
> ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
> ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
> intel_powerclamp(E) coretemp(E) kv)
> [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
> [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted: G            E
>      5.9.0-rc4-1-default+ #492
> [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
> FIRE X2270 M2, BIOS 2.05    07/01/2010
> [  146.134099] Workqueue: events_unbound commit_work
> [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60 [drm_vram_helper]
> [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
> d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
> c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
> [  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
> [  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
> ffffffffc032323b
> [  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
> ffff888111155278
> [  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
> 0000000000000002
> [  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
> ffff888109091050
> [  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
> 0000000000000000
> [  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
> knlGS:0000000000000000
> [  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
> 00000000000206e0
> [  146.134223] Call Trace:
> [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
> [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
> [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
> [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
> [  146.134357]  commit_tail+0x103/0x1c0
> [  146.134390]  process_one_work+0x56a/0xa60
> [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
> [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
> [  146.134460]  ? __lock_contended+0x4a0/0x4a0
> [  146.134491]  ? worker_thread+0x150/0x620
> [  146.134521]  worker_thread+0x8b/0x620
> [  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
> [  146.134583]  ? process_one_work+0xa60/0xa60
> [  146.134597]  kthread+0x1e4/0x210
> [  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
> [  146.134637]  ret_from_fork+0x22/0x30
> [  146.134698] irq event stamp: 74111
> [  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
> console_unlock+0x539/0x670
> [  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
> console_unlock+0x52f/0x670
> [  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
> wb_workfn+0x3f5/0x430
> [  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
> wb_wakeup_delayed+0x40/0xa0
> [  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---
>
>
> Happens with ast when doing
>
>    weston-launch
>
>
>
>> +		return 0;
>> +
>> +	return gbo->bo.mem.start;
>> +}
>> +
>>   /**
>>    * drm_gem_vram_offset() - \
>>   	Returns a GEM VRAM object's offset in video memory
>> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
>>   {
>>   	if (WARN_ON_ONCE(!gbo->pin_count))
>>   		return (s64)-ENODEV;
>> -	return gbo->bo.offset;
>> +	return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>>   }
>>   EXPORT_SYMBOL(drm_gem_vram_offset);
>>   
>>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-06-24 18:26 ` [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4 Nirmoy Das
@ 2020-09-17 10:51   ` Thomas Zimmermann
  2020-09-17 11:12     ` Christian König
  0 siblings, 1 reply; 19+ messages in thread
From: Thomas Zimmermann @ 2020-09-17 10:51 UTC (permalink / raw)
  To: Nirmoy Das, dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, christian.koenig, kraxel


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

Hi

Am 24.06.20 um 20:26 schrieb Nirmoy Das:
> Calculate GEM VRAM bo's offset within vram-helper without depending on
> bo->offset.
> 
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 0023ce1d2cf7..ad096600d89f 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
>  }
>  EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>  
> +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
> +{
> +	/* Keep TTM behavior for now, remove when drivers are audited */
> +	if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))

At this line I got

[  146.133821] ------------[ cut here ]------------
[  146.133872] WARNING: CPU: 6 PID: 7 at
drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
[drm_vram_helper]
[  146.133880] Modules linked in: fuse(E) af_packet(E) ebtable_filter(E)
ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
intel_powerclamp(E) coretemp(E) kv)
[  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
[  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted: G            E
    5.9.0-rc4-1-default+ #492
[  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
FIRE X2270 M2, BIOS 2.05    07/01/2010
[  146.134099] Workqueue: events_unbound commit_work
[  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60 [drm_vram_helper]
[  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
[  146.134137] RSP: 0018:ffffc90000107c38 EFLAGS: 00010246
[  146.134149] RAX: 0000000000000000 RBX: ffff888111155000 RCX:
ffffffffc032323b
[  146.134158] RDX: dffffc0000000000 RSI: ffff88810e236300 RDI:
ffff888111155278
[  146.134168] RBP: ffff888109090000 R08: ffffffffc0323225 R09:
0000000000000002
[  146.134177] R10: ffffed1020675020 R11: 0000000000000001 R12:
ffff888109091050
[  146.134187] R13: ffff88810e236300 R14: ffff888109090000 R15:
0000000000000000
[  146.134197] FS:  0000000000000000(0000) GS:ffff888116000000(0000)
knlGS:0000000000000000
[  146.134206] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  146.134215] CR2: 00007f60547d9100 CR3: 0000000029216002 CR4:
00000000000206e0
[  146.134223] Call Trace:
[  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
[  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
[  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
[  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
[  146.134357]  commit_tail+0x103/0x1c0
[  146.134390]  process_one_work+0x56a/0xa60
[  146.134431]  ? __lock_acquired+0x1ca/0x3d0
[  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
[  146.134460]  ? __lock_contended+0x4a0/0x4a0
[  146.134491]  ? worker_thread+0x150/0x620
[  146.134521]  worker_thread+0x8b/0x620
[  146.134539]  ? _raw_spin_unlock_irqrestore+0x41/0x50
[  146.134583]  ? process_one_work+0xa60/0xa60
[  146.134597]  kthread+0x1e4/0x210
[  146.134612]  ? kthread_create_worker_on_cpu+0xb0/0xb0
[  146.134637]  ret_from_fork+0x22/0x30
[  146.134698] irq event stamp: 74111
[  146.134711] hardirqs last  enabled at (74117): [<ffffffff971c68f9>]
console_unlock+0x539/0x670
[  146.134723] hardirqs last disabled at (74122): [<ffffffff971c68ef>]
console_unlock+0x52f/0x670
[  146.134737] softirqs last  enabled at (73354): [<ffffffff975469d5>]
wb_workfn+0x3f5/0x430
[  146.134749] softirqs last disabled at (73350): [<ffffffff973f81d0>]
wb_wakeup_delayed+0x40/0xa0
[  146.134758] ---[ end trace 74dd5fac6a3a2c0c ]---


Happens with ast when doing

  weston-launch



> +		return 0;
> +
> +	return gbo->bo.mem.start;
> +}
> +
>  /**
>   * drm_gem_vram_offset() - \
>  	Returns a GEM VRAM object's offset in video memory
> @@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
>  {
>  	if (WARN_ON_ONCE(!gbo->pin_count))
>  		return (s64)-ENODEV;
> -	return gbo->bo.offset;
> +	return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
>  }
>  EXPORT_SYMBOL(drm_gem_vram_offset);
>  
> 

-- 
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: 516 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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4
  2020-06-24 18:26 [RESEND] [PATCH v6 0/8] do not store GPU address in TTM Nirmoy Das
@ 2020-06-24 18:26 ` Nirmoy Das
  2020-09-17 10:51   ` Thomas Zimmermann
  0 siblings, 1 reply; 19+ messages in thread
From: Nirmoy Das @ 2020-06-24 18:26 UTC (permalink / raw)
  To: dri-devel
  Cc: sashal, thellstrom, airlied, kenny.ho, brian.welty, nirmoy.das,
	linux-graphics-maintainer, bskeggs, Daniel Vetter,
	alexander.deucher, sean, christian.koenig, kraxel

Calculate GEM VRAM bo's offset within vram-helper without depending on
bo->offset.

Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 0023ce1d2cf7..ad096600d89f 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
 
+static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
+{
+	/* Keep TTM behavior for now, remove when drivers are audited */
+	if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
+		return 0;
+
+	return gbo->bo.mem.start;
+}
+
 /**
  * drm_gem_vram_offset() - \
 	Returns a GEM VRAM object's offset in video memory
@@ -297,7 +306,7 @@ s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
 {
 	if (WARN_ON_ONCE(!gbo->pin_count))
 		return (s64)-ENODEV;
-	return gbo->bo.offset;
+	return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
 }
 EXPORT_SYMBOL(drm_gem_vram_offset);
 
-- 
2.27.0

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-09-21 14:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 18:42 [PATCH v5 0/8] do not store GPU address in TTM Nirmoy Das
2020-04-01 18:42 ` [PATCH 1/1] drm/amdgpu: move ttm bo->offset to amdgpu_bo Nirmoy Das
2020-04-01 18:42 ` [PATCH 2/8] drm/radeon: don't use ttm bo->offset Nirmoy Das
2020-04-01 18:42 ` [PATCH 3/8] drm/vmwgfx: " Nirmoy Das
2020-04-01 18:42 ` [PATCH 4/8] drm/nouveau: don't use ttm bo->offset v3 Nirmoy Das
2020-04-02  7:04   ` Christian König
2020-04-01 18:42 ` [PATCH 5/8] drm/qxl: don't use ttm bo->offset Nirmoy Das
2020-04-01 18:42 ` [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4 Nirmoy Das
2020-04-01 18:42 ` [PATCH 7/8] drm/bochs: use drm_gem_vram_offset to get bo offset v2 Nirmoy Das
2020-04-01 18:42 ` [PATCH 8/8] drm/ttm: do not keep GPU dependent addresses Nirmoy Das
2020-06-24 18:26 [RESEND] [PATCH v6 0/8] do not store GPU address in TTM Nirmoy Das
2020-06-24 18:26 ` [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4 Nirmoy Das
2020-09-17 10:51   ` Thomas Zimmermann
2020-09-17 11:12     ` Christian König
2020-09-17 12:29       ` Thomas Zimmermann
2020-09-17 12:32         ` Christian König
2020-09-17 14:57           ` Thomas Zimmermann
2020-09-21 14:26           ` Thomas Zimmermann
2020-09-17 12:58         ` Nirmoy
2020-09-17 13:18           ` Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).