All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/11] drm: remove optional dummy function from drivers using TTM
@ 2020-07-21  7:32 Christian König
  2020-07-21  7:32 ` [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau Christian König
                   ` (12 more replies)
  0 siblings, 13 replies; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

Implementing those is completely unecessary.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 -----
 drivers/gpu/drm/drm_gem_vram_helper.c      |  5 -----
 drivers/gpu/drm/qxl/qxl_ttm.c              |  6 ------
 drivers/gpu/drm/radeon/radeon_ttm.c        |  5 -----
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 11 -----------
 5 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3df685287cc1..9c0f12f74af9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -836,10 +836,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
 	return 0;
 }
 
-static void amdgpu_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
-{
-}
-
 static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
 					   unsigned long page_offset)
 {
@@ -1754,7 +1750,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
 	.release_notify = &amdgpu_bo_release_notify,
 	.fault_reserve_notify = &amdgpu_bo_fault_reserve_notify,
 	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
-	.io_mem_free = &amdgpu_ttm_io_mem_free,
 	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
 	.access_memory = &amdgpu_ttm_access_memory,
 	.del_from_lru_notify = &amdgpu_vm_del_from_lru_notify
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index d107a2679e23..3296ed3df358 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1081,10 +1081,6 @@ static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
 	return 0;
 }
 
-static void bo_driver_io_mem_free(struct ttm_bo_device *bdev,
-				  struct ttm_mem_reg *mem)
-{ }
-
 static struct ttm_bo_driver bo_driver = {
 	.ttm_tt_create = bo_driver_ttm_tt_create,
 	.ttm_tt_populate = ttm_pool_populate,
@@ -1094,7 +1090,6 @@ static struct ttm_bo_driver bo_driver = {
 	.evict_flags = bo_driver_evict_flags,
 	.move_notify = bo_driver_move_notify,
 	.io_mem_reserve = bo_driver_io_mem_reserve,
-	.io_mem_free = bo_driver_io_mem_free,
 };
 
 /*
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 52eaa2d22745..a6e67149ef4a 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -129,11 +129,6 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
 	return 0;
 }
 
-static void qxl_ttm_io_mem_free(struct ttm_bo_device *bdev,
-				struct ttm_mem_reg *mem)
-{
-}
-
 /*
  * TTM backend functions.
  */
@@ -247,7 +242,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
 	.evict_flags = &qxl_evict_flags,
 	.move = &qxl_bo_move,
 	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
-	.io_mem_free = &qxl_ttm_io_mem_free,
 	.move_notify = &qxl_bo_move_notify,
 };
 
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index f4f1e63731a5..73085523fad7 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -457,10 +457,6 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
 	return 0;
 }
 
-static void radeon_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
-{
-}
-
 /*
  * TTM backend functions.
  */
@@ -774,7 +770,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
 	.move_notify = &radeon_bo_move_notify,
 	.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
 	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
-	.io_mem_free = &radeon_ttm_io_mem_free,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index fbcd11a7b215..bfd0c54ec30a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -815,15 +815,6 @@ static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg
 	return 0;
 }
 
-static void vmw_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
-{
-}
-
-static int vmw_ttm_fault_reserve_notify(struct ttm_buffer_object *bo)
-{
-	return 0;
-}
-
 /**
  * vmw_move_notify - TTM move_notify_callback
  *
@@ -866,7 +857,5 @@ struct ttm_bo_driver vmw_bo_driver = {
 	.verify_access = vmw_verify_access,
 	.move_notify = vmw_move_notify,
 	.swap_notify = vmw_swap_notify,
-	.fault_reserve_notify = &vmw_ttm_fault_reserve_notify,
 	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
-	.io_mem_free = &vmw_ttm_io_mem_free,
 };
-- 
2.17.1

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

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

* [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  8:50   ` daniel
  2020-07-22  7:05   ` Chauhan, Madhav
  2020-07-21  7:32 ` [PATCH 03/11] drm/ttm: remove io_reserve_fastpath flag Christian König
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

Nouveau is the only user of this functionality and evicting io space
on -EAGAIN is really a misuse of the return code.

Instead switch to using -ENOSPC here which makes much more sense and
simplifies the code.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 2 --
 drivers/gpu/drm/ttm/ttm_bo_util.c    | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 61355cfb7335..a48652826f67 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1505,8 +1505,6 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
 			if (ret != 1) {
 				if (WARN_ON(ret == 0))
 					return -EINVAL;
-				if (ret == -ENOSPC)
-					return -EAGAIN;
 				return ret;
 			}
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 5e0f3a9caedc..7d2c50fef456 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -116,7 +116,7 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
 	struct ttm_buffer_object *bo;
 
 	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
-		return -EAGAIN;
+		return -ENOSPC;
 
 	bo = list_first_entry(&man->io_reserve_lru,
 			      struct ttm_buffer_object,
@@ -143,7 +143,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 	    mem->bus.io_reserved_count++ == 0) {
 retry:
 		ret = bdev->driver->io_mem_reserve(bdev, mem);
-		if (ret == -EAGAIN) {
+		if (ret == -ENOSPC) {
 			ret = ttm_mem_io_evict(man);
 			if (ret == 0)
 				goto retry;
-- 
2.17.1

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

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

* [PATCH 03/11] drm/ttm: remove io_reserve_fastpath flag
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
  2020-07-21  7:32 ` [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  8:52   ` daniel
  2020-07-21  7:32 ` [PATCH 04/11] drm/ttm: cleanup coding style and implementation Christian König
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

Just use the use_io_reserve_lru flag. It doesn't make much
sense to have two flags.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
 drivers/gpu/drm/ttm/ttm_bo.c         | 1 -
 drivers/gpu/drm/ttm/ttm_bo_util.c    | 8 ++++----
 include/drm/ttm/ttm_bo_driver.h      | 2 --
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index a48652826f67..a1037478fa3f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -675,7 +675,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 			}
 
 			man->func = &nouveau_vram_manager;
-			man->io_reserve_fastpath = false;
 			man->use_io_reserve_lru = true;
 		} else {
 			man->func = &ttm_bo_manager_func;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7be36b9996ed..8b9e7f62bea7 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1521,7 +1521,6 @@ int ttm_bo_init_mm(struct ttm_bo_device *bdev, unsigned type,
 	BUG_ON(type >= TTM_NUM_MEM_TYPES);
 	man = &bdev->man[type];
 	BUG_ON(man->has_type);
-	man->io_reserve_fastpath = true;
 	man->use_io_reserve_lru = false;
 	mutex_init(&man->io_reserve_mutex);
 	spin_lock_init(&man->move_lock);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 7d2c50fef456..6c05f4fd15ae 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -93,7 +93,7 @@ EXPORT_SYMBOL(ttm_bo_move_ttm);
 
 int ttm_mem_io_lock(struct ttm_mem_type_manager *man, bool interruptible)
 {
-	if (likely(man->io_reserve_fastpath))
+	if (likely(!man->use_io_reserve_lru))
 		return 0;
 
 	if (interruptible)
@@ -105,7 +105,7 @@ int ttm_mem_io_lock(struct ttm_mem_type_manager *man, bool interruptible)
 
 void ttm_mem_io_unlock(struct ttm_mem_type_manager *man)
 {
-	if (likely(man->io_reserve_fastpath))
+	if (likely(!man->use_io_reserve_lru))
 		return;
 
 	mutex_unlock(&man->io_reserve_mutex);
@@ -136,7 +136,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 
 	if (!bdev->driver->io_mem_reserve)
 		return 0;
-	if (likely(man->io_reserve_fastpath))
+	if (likely(!man->use_io_reserve_lru))
 		return bdev->driver->io_mem_reserve(bdev, mem);
 
 	if (bdev->driver->io_mem_reserve &&
@@ -157,7 +157,7 @@ void ttm_mem_io_free(struct ttm_bo_device *bdev,
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
 
-	if (likely(man->io_reserve_fastpath))
+	if (likely(!man->use_io_reserve_lru))
 		return;
 
 	if (bdev->driver->io_mem_reserve &&
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 732167cad130..45522e4fbd6b 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -155,7 +155,6 @@ struct ttm_mem_type_manager_func {
  * @use_io_reserve_lru: Use an lru list to try to unreserve io_mem_regions
  * reserved by the TTM vm system.
  * @io_reserve_lru: Optional lru list for unreserving io mem regions.
- * @io_reserve_fastpath: Only use bdev::driver::io_mem_reserve to obtain
  * @move_lock: lock for move fence
  * static information. bdev::driver::io_mem_free is never used.
  * @lru: The lru list for this memory type.
@@ -184,7 +183,6 @@ struct ttm_mem_type_manager {
 	void *priv;
 	struct mutex io_reserve_mutex;
 	bool use_io_reserve_lru;
-	bool io_reserve_fastpath;
 	spinlock_t move_lock;
 
 	/*
-- 
2.17.1

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

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

* [PATCH 04/11] drm/ttm: cleanup coding style and implementation.
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
  2020-07-21  7:32 ` [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau Christian König
  2020-07-21  7:32 ` [PATCH 03/11] drm/ttm: remove io_reserve_fastpath flag Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  8:56   ` daniel
  2020-07-21  7:32 ` [PATCH 05/11] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

Only functional change is to always keep io_reserved_count up to date
for debugging even when it is not used otherwise.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 97 +++++++++++++++----------------
 1 file changed, 48 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 6c05f4fd15ae..7fb3e0bcbab4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -115,39 +115,35 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
 {
 	struct ttm_buffer_object *bo;
 
-	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
+	bo = list_first_entry_or_null(&man->io_reserve_lru,
+				      struct ttm_buffer_object,
+				      io_reserve_lru);
+	if (!bo)
 		return -ENOSPC;
 
-	bo = list_first_entry(&man->io_reserve_lru,
-			      struct ttm_buffer_object,
-			      io_reserve_lru);
 	list_del_init(&bo->io_reserve_lru);
 	ttm_bo_unmap_virtual_locked(bo);
-
 	return 0;
 }
 
-
 int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 		       struct ttm_mem_reg *mem)
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-	int ret = 0;
+	int ret;
+
+	if (mem->bus.io_reserved_count++)
+		return 0;
 
 	if (!bdev->driver->io_mem_reserve)
 		return 0;
-	if (likely(!man->use_io_reserve_lru))
-		return bdev->driver->io_mem_reserve(bdev, mem);
 
-	if (bdev->driver->io_mem_reserve &&
-	    mem->bus.io_reserved_count++ == 0) {
 retry:
-		ret = bdev->driver->io_mem_reserve(bdev, mem);
-		if (ret == -ENOSPC) {
-			ret = ttm_mem_io_evict(man);
-			if (ret == 0)
-				goto retry;
-		}
+	ret = bdev->driver->io_mem_reserve(bdev, mem);
+	if (ret == -ENOSPC) {
+		ret = ttm_mem_io_evict(man);
+		if (ret == 0)
+			goto retry;
 	}
 	return ret;
 }
@@ -155,35 +151,31 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 void ttm_mem_io_free(struct ttm_bo_device *bdev,
 		     struct ttm_mem_reg *mem)
 {
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-
-	if (likely(!man->use_io_reserve_lru))
+	if (--mem->bus.io_reserved_count)
 		return;
 
-	if (bdev->driver->io_mem_reserve &&
-	    --mem->bus.io_reserved_count == 0 &&
-	    bdev->driver->io_mem_free)
-		bdev->driver->io_mem_free(bdev, mem);
+	if (!bdev->driver->io_mem_free)
+		return;
 
+	bdev->driver->io_mem_free(bdev, mem);
 }
 
 int ttm_mem_io_reserve_vm(struct ttm_buffer_object *bo)
 {
+	struct ttm_mem_type_manager *man = &bo->bdev->man[bo->mem.mem_type];
 	struct ttm_mem_reg *mem = &bo->mem;
 	int ret;
 
-	if (!mem->bus.io_reserved_vm) {
-		struct ttm_mem_type_manager *man =
-			&bo->bdev->man[mem->mem_type];
+	if (mem->bus.io_reserved_vm)
+		return 0;
 
-		ret = ttm_mem_io_reserve(bo->bdev, mem);
-		if (unlikely(ret != 0))
-			return ret;
-		mem->bus.io_reserved_vm = true;
-		if (man->use_io_reserve_lru)
-			list_add_tail(&bo->io_reserve_lru,
-				      &man->io_reserve_lru);
-	}
+	ret = ttm_mem_io_reserve(bo->bdev, mem);
+	if (unlikely(ret != 0))
+		return ret;
+	mem->bus.io_reserved_vm = true;
+	if (man->use_io_reserve_lru)
+		list_add_tail(&bo->io_reserve_lru,
+			      &man->io_reserve_lru);
 	return 0;
 }
 
@@ -191,15 +183,17 @@ void ttm_mem_io_free_vm(struct ttm_buffer_object *bo)
 {
 	struct ttm_mem_reg *mem = &bo->mem;
 
-	if (mem->bus.io_reserved_vm) {
-		mem->bus.io_reserved_vm = false;
-		list_del_init(&bo->io_reserve_lru);
-		ttm_mem_io_free(bo->bdev, mem);
-	}
+	if (!mem->bus.io_reserved_vm)
+		return;
+
+	mem->bus.io_reserved_vm = false;
+	list_del_init(&bo->io_reserve_lru);
+	ttm_mem_io_free(bo->bdev, mem);
 }
 
-static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
-			void **virtual)
+static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev,
+			       struct ttm_mem_reg *mem,
+			       void **virtual)
 {
 	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
 	int ret;
@@ -216,9 +210,11 @@ static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *m
 		addr = mem->bus.addr;
 	} else {
 		if (mem->placement & TTM_PL_FLAG_WC)
-			addr = ioremap_wc(mem->bus.base + mem->bus.offset, mem->bus.size);
+			addr = ioremap_wc(mem->bus.base + mem->bus.offset,
+					  mem->bus.size);
 		else
-			addr = ioremap(mem->bus.base + mem->bus.offset, mem->bus.size);
+			addr = ioremap(mem->bus.base + mem->bus.offset,
+				       mem->bus.size);
 		if (!addr) {
 			(void) ttm_mem_io_lock(man, false);
 			ttm_mem_io_free(bdev, mem);
@@ -230,8 +226,9 @@ static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *m
 	return 0;
 }
 
-static void ttm_mem_reg_iounmap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
-			 void *virtual)
+static void ttm_mem_reg_iounmap(struct ttm_bo_device *bdev,
+				struct ttm_mem_reg *mem,
+				void *virtual)
 {
 	struct ttm_mem_type_manager *man;
 
@@ -513,11 +510,13 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
 	} else {
 		map->bo_kmap_type = ttm_bo_map_iomap;
 		if (mem->placement & TTM_PL_FLAG_WC)
-			map->virtual = ioremap_wc(bo->mem.bus.base + bo->mem.bus.offset + offset,
+			map->virtual = ioremap_wc(bo->mem.bus.base +
+						  bo->mem.bus.offset + offset,
 						  size);
 		else
-			map->virtual = ioremap(bo->mem.bus.base + bo->mem.bus.offset + offset,
-						       size);
+			map->virtual = ioremap(bo->mem.bus.base +
+					       bo->mem.bus.offset + offset,
+					       size);
 	}
 	return (!map->virtual) ? -ENOMEM : 0;
 }
-- 
2.17.1

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

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

* [PATCH 05/11] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (2 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 04/11] drm/ttm: cleanup coding style and implementation Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  9:01   ` daniel
  2020-07-21  7:32 ` [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

The original intention was to avoid CPU page table unmaps
when BOs move between the GTT and SYSTEM domain.

The problem is that this never correctly handled changes
in the caching attributes or backing pages.

Just drop this for now and simply unmap the CPU page
tables in all cases.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +--
 drivers/gpu/drm/nouveau/nouveau_bo.c       |  3 +-
 drivers/gpu/drm/radeon/radeon_ttm.c        |  2 +-
 drivers/gpu/drm/ttm/ttm_bo.c               | 34 ++++------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  2 +-
 include/drm/ttm/ttm_bo_driver.h            |  1 -
 6 files changed, 11 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 9c0f12f74af9..44fa8bc49d18 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -93,7 +93,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->func = &amdgpu_gtt_mgr_func;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
+		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 		break;
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
@@ -108,7 +108,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED;
 		man->default_caching = TTM_PL_FLAG_UNCACHED;
 		break;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index a1037478fa3f..7883341f8c83 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -695,8 +695,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 				TTM_PL_FLAG_WC;
 			man->default_caching = TTM_PL_FLAG_WC;
 		} else {
-			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE |
-				     TTM_MEMTYPE_FLAG_CMA;
+			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 			man->available_caching = TTM_PL_MASK_CACHING;
 			man->default_caching = TTM_PL_FLAG_CACHED;
 		}
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 73085523fad7..54af06df865b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->func = &ttm_bo_manager_func;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
+		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 #if IS_ENABLED(CONFIG_AGP)
 		if (rdev->flags & RADEON_IS_AGP) {
 			if (!rdev->ddev->agp) {
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 8b9e7f62bea7..0768a054a916 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -272,20 +272,15 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 				  struct ttm_operation_ctx *ctx)
 {
 	struct ttm_bo_device *bdev = bo->bdev;
-	bool old_is_pci = ttm_mem_reg_is_pci(bdev, &bo->mem);
-	bool new_is_pci = ttm_mem_reg_is_pci(bdev, mem);
 	struct ttm_mem_type_manager *old_man = &bdev->man[bo->mem.mem_type];
 	struct ttm_mem_type_manager *new_man = &bdev->man[mem->mem_type];
-	int ret = 0;
+	int ret;
 
-	if (old_is_pci || new_is_pci ||
-	    ((mem->placement & bo->mem.placement & TTM_PL_MASK_CACHING) == 0)) {
-		ret = ttm_mem_io_lock(old_man, true);
-		if (unlikely(ret != 0))
-			goto out_err;
-		ttm_bo_unmap_virtual_locked(bo);
-		ttm_mem_io_unlock(old_man);
-	}
+	ret = ttm_mem_io_lock(old_man, true);
+	if (unlikely(ret != 0))
+		goto out_err;
+	ttm_bo_unmap_virtual_locked(bo);
+	ttm_mem_io_unlock(old_man);
 
 	/*
 	 * Create and bind a ttm if required.
@@ -1698,23 +1693,6 @@ EXPORT_SYMBOL(ttm_bo_device_init);
  * buffer object vm functions.
  */
 
-bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
-{
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
-
-	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
-		if (mem->mem_type == TTM_PL_SYSTEM)
-			return false;
-
-		if (man->flags & TTM_MEMTYPE_FLAG_CMA)
-			return false;
-
-		if (mem->placement & TTM_PL_FLAG_CACHED)
-			return false;
-	}
-	return true;
-}
-
 void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
 {
 	struct ttm_bo_device *bdev = bo->bdev;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index bfd0c54ec30a..6bea7548aee0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -762,7 +762,7 @@ 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->flags = TTM_MEMTYPE_FLAG_CMA | TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 45522e4fbd6b..71b195e78c7c 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -47,7 +47,6 @@
 
 #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
 #define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)	/* Memory mappable */
-#define TTM_MEMTYPE_FLAG_CMA           (1 << 3)	/* Can't map aperture */
 
 struct ttm_mem_type_manager;
 
-- 
2.17.1

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

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

* [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (3 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 05/11] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  9:24   ` daniel
  2020-07-21  7:32 ` [PATCH 07/11] drm/vmwgfx: " Christian König
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

The driver doesn't expose any not-mapable memory resources.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 54af06df865b..b474781a0920 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->func = &ttm_bo_manager_func;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 #if IS_ENABLED(CONFIG_AGP)
 		if (rdev->flags & RADEON_IS_AGP) {
 			if (!rdev->ddev->agp) {
@@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 					  (unsigned)type);
 				return -EINVAL;
 			}
-			if (!rdev->ddev->agp->cant_use_aperture)
-				man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 			man->available_caching = TTM_PL_FLAG_UNCACHED |
 						 TTM_PL_FLAG_WC;
 			man->default_caching = TTM_PL_FLAG_WC;
@@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
-			     TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
 		break;
@@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
 {
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
 	struct radeon_device *rdev = radeon_get_rdev(bdev);
 
 	mem->bus.addr = NULL;
@@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
 	mem->bus.size = mem->num_pages << PAGE_SHIFT;
 	mem->bus.base = 0;
 	mem->bus.is_iomem = false;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
+
 	switch (mem->mem_type) {
 	case TTM_PL_SYSTEM:
 		/* system memory */
-- 
2.17.1

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

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

* [PATCH 07/11] drm/vmwgfx: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (4 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  9:26   ` daniel
  2020-07-21  7:32 ` [PATCH 08/11] drm/amdgpu: " Christian König
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

The driver doesn't expose any not-mapable memory resources.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 6bea7548aee0..1d78187eaba6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -742,15 +742,13 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &vmw_thp_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -762,7 +760,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->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -789,7 +786,6 @@ static int vmw_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 
 static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
 {
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
 	struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
 
 	mem->bus.addr = NULL;
@@ -797,8 +793,7 @@ static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg
 	mem->bus.offset = 0;
 	mem->bus.size = mem->num_pages << PAGE_SHIFT;
 	mem->bus.base = 0;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
+
 	switch (mem->mem_type) {
 	case TTM_PL_SYSTEM:
 	case VMW_PL_GMR:
-- 
2.17.1

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

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

* [PATCH 08/11] drm/amdgpu: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (5 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 07/11] drm/vmwgfx: " Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  9:28   ` daniel
  2020-07-21  7:32 ` [PATCH 09/11] drm/nouveau: " Christian König
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

The driver does support some not-mapable resources, but
those are already handled correctly in the switch/case
statement in the code.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 44fa8bc49d18..0dd5e802091d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -84,7 +84,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -93,13 +93,12 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->func = &amdgpu_gtt_mgr_func;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 		break;
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &amdgpu_vram_mgr_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED |
-			     TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
 		break;
@@ -796,7 +795,6 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
  */
 static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
 {
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
 	struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
 	struct drm_mm_node *mm_node = mem->mm_node;
 
@@ -805,8 +803,7 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
 	mem->bus.size = mem->num_pages << PAGE_SHIFT;
 	mem->bus.base = 0;
 	mem->bus.is_iomem = false;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
+
 	switch (mem->mem_type) {
 	case TTM_PL_SYSTEM:
 		/* system memory */
-- 
2.17.1

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

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

* [PATCH 09/11] drm/nouveau: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (6 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 08/11] drm/amdgpu: " Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  9:28   ` daniel
  2020-07-21  7:32 ` [PATCH 10/11] drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE v2 Christian König
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

The driver doesn't expose any not-mapable memory resources.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 7883341f8c83..4ccf937df0d0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -655,13 +655,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 
 	switch (type) {
 	case TTM_PL_SYSTEM:
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_VRAM:
-		man->flags = TTM_MEMTYPE_FLAG_FIXED |
-			     TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED |
 					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
@@ -690,12 +689,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 			man->func = &ttm_bo_manager_func;
 
 		if (drm->agp.bridge) {
-			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+			man->flags = 0;
 			man->available_caching = TTM_PL_FLAG_UNCACHED |
 				TTM_PL_FLAG_WC;
 			man->default_caching = TTM_PL_FLAG_WC;
 		} else {
-			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+			man->flags = 0;
 			man->available_caching = TTM_PL_MASK_CACHING;
 			man->default_caching = TTM_PL_FLAG_CACHED;
 		}
@@ -1437,7 +1436,6 @@ nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 static int
 nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
 {
-	struct ttm_mem_type_manager *man = &bdev->man[reg->mem_type];
 	struct nouveau_drm *drm = nouveau_bdev(bdev);
 	struct nvkm_device *device = nvxx_device(&drm->client.device);
 	struct nouveau_mem *mem = nouveau_mem(reg);
@@ -1447,8 +1445,7 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
 	reg->bus.size = reg->num_pages << PAGE_SHIFT;
 	reg->bus.base = 0;
 	reg->bus.is_iomem = false;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
+
 	switch (reg->mem_type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-- 
2.17.1

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

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

* [PATCH 10/11] drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE v2
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (7 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 09/11] drm/nouveau: " Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  9:29   ` daniel
  2020-07-21  7:32 ` [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

The driver doesn't expose any not-mapable memory resources.

v2: remove unused man variable as well

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/qxl/qxl_ttm.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index a6e67149ef4a..1d8e07b8b19e 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -54,7 +54,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -62,8 +62,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_PRIV:
 		/* "On-card" video ram */
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED |
-			     TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -99,7 +98,6 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
 int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
 			   struct ttm_mem_reg *mem)
 {
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
 	struct qxl_device *qdev = qxl_get_qdev(bdev);
 
 	mem->bus.addr = NULL;
@@ -107,8 +105,7 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
 	mem->bus.size = mem->num_pages << PAGE_SHIFT;
 	mem->bus.base = 0;
 	mem->bus.is_iomem = false;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
+
 	switch (mem->mem_type) {
 	case TTM_PL_SYSTEM:
 		/* system memory */
-- 
2.17.1

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

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

* [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (8 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 10/11] drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE v2 Christian König
@ 2020-07-21  7:32 ` Christian König
  2020-07-21  9:30   ` daniel
  2020-07-21  9:54     ` kernel test robot
  2020-07-21  7:55 ` [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Daniel Vetter
                   ` (2 subsequent siblings)
  12 siblings, 2 replies; 35+ messages in thread
From: Christian König @ 2020-07-21  7:32 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

Not used any more. And it is bad design to use a TTM flag
to do a check inside a driver.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 include/drm/ttm/ttm_bo_driver.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 71b195e78c7c..9b251853afe2 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -46,7 +46,6 @@
 #define TTM_MAX_BO_PRIORITY	4U
 
 #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
-#define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)	/* Memory mappable */
 
 struct ttm_mem_type_manager;
 
-- 
2.17.1

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

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

* Re: [PATCH 01/11] drm: remove optional dummy function from drivers using TTM
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (9 preceding siblings ...)
  2020-07-21  7:32 ` [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-21  7:55 ` Daniel Vetter
  2020-07-21  8:02 ` Christian König
  2020-07-21  9:15 ` Chauhan, Madhav
  12 siblings, 0 replies; 35+ messages in thread
From: Daniel Vetter @ 2020-07-21  7:55 UTC (permalink / raw)
  To: Christian König
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:35AM +0200, Christian König wrote:
> Implementing those is completely unecessary.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 -----
>  drivers/gpu/drm/drm_gem_vram_helper.c      |  5 -----
>  drivers/gpu/drm/qxl/qxl_ttm.c              |  6 ------
>  drivers/gpu/drm/radeon/radeon_ttm.c        |  5 -----
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 11 -----------
>  5 files changed, 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 3df685287cc1..9c0f12f74af9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -836,10 +836,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>  	return 0;
>  }
>  
> -static void amdgpu_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> -{
> -}
> -
>  static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
>  					   unsigned long page_offset)
>  {
> @@ -1754,7 +1750,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>  	.release_notify = &amdgpu_bo_release_notify,
>  	.fault_reserve_notify = &amdgpu_bo_fault_reserve_notify,
>  	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
> -	.io_mem_free = &amdgpu_ttm_io_mem_free,
>  	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
>  	.access_memory = &amdgpu_ttm_access_memory,
>  	.del_from_lru_notify = &amdgpu_vm_del_from_lru_notify
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index d107a2679e23..3296ed3df358 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1081,10 +1081,6 @@ static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
>  	return 0;
>  }
>  
> -static void bo_driver_io_mem_free(struct ttm_bo_device *bdev,
> -				  struct ttm_mem_reg *mem)
> -{ }
> -
>  static struct ttm_bo_driver bo_driver = {
>  	.ttm_tt_create = bo_driver_ttm_tt_create,
>  	.ttm_tt_populate = ttm_pool_populate,
> @@ -1094,7 +1090,6 @@ static struct ttm_bo_driver bo_driver = {
>  	.evict_flags = bo_driver_evict_flags,
>  	.move_notify = bo_driver_move_notify,
>  	.io_mem_reserve = bo_driver_io_mem_reserve,
> -	.io_mem_free = bo_driver_io_mem_free,
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 52eaa2d22745..a6e67149ef4a 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -129,11 +129,6 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
>  	return 0;
>  }
>  
> -static void qxl_ttm_io_mem_free(struct ttm_bo_device *bdev,
> -				struct ttm_mem_reg *mem)
> -{
> -}
> -
>  /*
>   * TTM backend functions.
>   */
> @@ -247,7 +242,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
>  	.evict_flags = &qxl_evict_flags,
>  	.move = &qxl_bo_move,
>  	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
> -	.io_mem_free = &qxl_ttm_io_mem_free,
>  	.move_notify = &qxl_bo_move_notify,
>  };
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index f4f1e63731a5..73085523fad7 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -457,10 +457,6 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>  	return 0;
>  }
>  
> -static void radeon_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> -{
> -}
> -
>  /*
>   * TTM backend functions.
>   */
> @@ -774,7 +770,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
>  	.move_notify = &radeon_bo_move_notify,
>  	.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
>  	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
> -	.io_mem_free = &radeon_ttm_io_mem_free,
>  };
>  
>  int radeon_ttm_init(struct radeon_device *rdev)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index fbcd11a7b215..bfd0c54ec30a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -815,15 +815,6 @@ static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg
>  	return 0;
>  }
>  
> -static void vmw_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> -{
> -}
> -
> -static int vmw_ttm_fault_reserve_notify(struct ttm_buffer_object *bo)
> -{
> -	return 0;
> -}
> -
>  /**
>   * vmw_move_notify - TTM move_notify_callback
>   *
> @@ -866,7 +857,5 @@ struct ttm_bo_driver vmw_bo_driver = {
>  	.verify_access = vmw_verify_access,
>  	.move_notify = vmw_move_notify,
>  	.swap_notify = vmw_swap_notify,
> -	.fault_reserve_notify = &vmw_ttm_fault_reserve_notify,
>  	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
> -	.io_mem_free = &vmw_ttm_io_mem_free,
>  };
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 01/11] drm: remove optional dummy function from drivers using TTM
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (10 preceding siblings ...)
  2020-07-21  7:55 ` [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Daniel Vetter
@ 2020-07-21  8:02 ` Christian König
  2020-07-21  9:15 ` Chauhan, Madhav
  12 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2020-07-21  8:02 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann

Sorry, I once more forgot the cover letter.

This is just some cleanup all over drivers using TTM.

The only important functional change is the removal of the CMA (Can't 
Map Aperture) flag since this might be a bug fix.

Please review and/or comment,
Christian.

Am 21.07.20 um 09:32 schrieb Christian König:
> Implementing those is completely unecessary.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 -----
>   drivers/gpu/drm/drm_gem_vram_helper.c      |  5 -----
>   drivers/gpu/drm/qxl/qxl_ttm.c              |  6 ------
>   drivers/gpu/drm/radeon/radeon_ttm.c        |  5 -----
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 11 -----------
>   5 files changed, 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 3df685287cc1..9c0f12f74af9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -836,10 +836,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>   	return 0;
>   }
>   
> -static void amdgpu_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> -{
> -}
> -
>   static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
>   					   unsigned long page_offset)
>   {
> @@ -1754,7 +1750,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>   	.release_notify = &amdgpu_bo_release_notify,
>   	.fault_reserve_notify = &amdgpu_bo_fault_reserve_notify,
>   	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
> -	.io_mem_free = &amdgpu_ttm_io_mem_free,
>   	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
>   	.access_memory = &amdgpu_ttm_access_memory,
>   	.del_from_lru_notify = &amdgpu_vm_del_from_lru_notify
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index d107a2679e23..3296ed3df358 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1081,10 +1081,6 @@ static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
>   	return 0;
>   }
>   
> -static void bo_driver_io_mem_free(struct ttm_bo_device *bdev,
> -				  struct ttm_mem_reg *mem)
> -{ }
> -
>   static struct ttm_bo_driver bo_driver = {
>   	.ttm_tt_create = bo_driver_ttm_tt_create,
>   	.ttm_tt_populate = ttm_pool_populate,
> @@ -1094,7 +1090,6 @@ static struct ttm_bo_driver bo_driver = {
>   	.evict_flags = bo_driver_evict_flags,
>   	.move_notify = bo_driver_move_notify,
>   	.io_mem_reserve = bo_driver_io_mem_reserve,
> -	.io_mem_free = bo_driver_io_mem_free,
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 52eaa2d22745..a6e67149ef4a 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -129,11 +129,6 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
>   	return 0;
>   }
>   
> -static void qxl_ttm_io_mem_free(struct ttm_bo_device *bdev,
> -				struct ttm_mem_reg *mem)
> -{
> -}
> -
>   /*
>    * TTM backend functions.
>    */
> @@ -247,7 +242,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
>   	.evict_flags = &qxl_evict_flags,
>   	.move = &qxl_bo_move,
>   	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
> -	.io_mem_free = &qxl_ttm_io_mem_free,
>   	.move_notify = &qxl_bo_move_notify,
>   };
>   
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index f4f1e63731a5..73085523fad7 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -457,10 +457,6 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>   	return 0;
>   }
>   
> -static void radeon_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> -{
> -}
> -
>   /*
>    * TTM backend functions.
>    */
> @@ -774,7 +770,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
>   	.move_notify = &radeon_bo_move_notify,
>   	.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
>   	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
> -	.io_mem_free = &radeon_ttm_io_mem_free,
>   };
>   
>   int radeon_ttm_init(struct radeon_device *rdev)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index fbcd11a7b215..bfd0c54ec30a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -815,15 +815,6 @@ static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg
>   	return 0;
>   }
>   
> -static void vmw_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> -{
> -}
> -
> -static int vmw_ttm_fault_reserve_notify(struct ttm_buffer_object *bo)
> -{
> -	return 0;
> -}
> -
>   /**
>    * vmw_move_notify - TTM move_notify_callback
>    *
> @@ -866,7 +857,5 @@ struct ttm_bo_driver vmw_bo_driver = {
>   	.verify_access = vmw_verify_access,
>   	.move_notify = vmw_move_notify,
>   	.swap_notify = vmw_swap_notify,
> -	.fault_reserve_notify = &vmw_ttm_fault_reserve_notify,
>   	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
> -	.io_mem_free = &vmw_ttm_io_mem_free,
>   };

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

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

* Re: [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau
  2020-07-21  7:32 ` [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau Christian König
@ 2020-07-21  8:50   ` daniel
  2020-07-22  7:05   ` Chauhan, Madhav
  1 sibling, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  8:50 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:36AM +0200, Christian König wrote:
> Nouveau is the only user of this functionality and evicting io space
> on -EAGAIN is really a misuse of the return code.
> 
> Instead switch to using -ENOSPC here which makes much more sense and
> simplifies the code.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Going from EAGAIN to something else could unbreak something that's held
together as a almost livelock restarting ioctls or whatever. So I looked
for that a bit:

- mmap path seems fine, all errors from the io_reserve stuff here get
  remapped to sigbus

- but everywhere else we just pass down the errno it seems, and nouveau
  has a bunch of kmaps all around (gpu relocs on pre-nv50 is probably the
  big one, if we ignore the memcpy bo move fallback). I haven't found
  anything that indicates those chips don't have the ioremapping hw, so
  there's some risk I think. Otoh I also don't see anything that would
  unbreak the lifelook, so feels minimally.

With that impact to pushbuf ioctl documented in the commit message somehow
this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c | 2 --
>  drivers/gpu/drm/ttm/ttm_bo_util.c    | 4 ++--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 61355cfb7335..a48652826f67 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1505,8 +1505,6 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
>  			if (ret != 1) {
>  				if (WARN_ON(ret == 0))
>  					return -EINVAL;
> -				if (ret == -ENOSPC)
> -					return -EAGAIN;
>  				return ret;
>  			}
>  
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 5e0f3a9caedc..7d2c50fef456 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -116,7 +116,7 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
>  	struct ttm_buffer_object *bo;
>  
>  	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
> -		return -EAGAIN;
> +		return -ENOSPC;
>  
>  	bo = list_first_entry(&man->io_reserve_lru,
>  			      struct ttm_buffer_object,
> @@ -143,7 +143,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
>  	    mem->bus.io_reserved_count++ == 0) {
>  retry:
>  		ret = bdev->driver->io_mem_reserve(bdev, mem);
> -		if (ret == -EAGAIN) {
> +		if (ret == -ENOSPC) {
>  			ret = ttm_mem_io_evict(man);
>  			if (ret == 0)
>  				goto retry;
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 03/11] drm/ttm: remove io_reserve_fastpath flag
  2020-07-21  7:32 ` [PATCH 03/11] drm/ttm: remove io_reserve_fastpath flag Christian König
@ 2020-07-21  8:52   ` daniel
  0 siblings, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  8:52 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:37AM +0200, Christian König wrote:
> Just use the use_io_reserve_lru flag. It doesn't make much
> sense to have two flags.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Yeah looks entirely redundant.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c | 1 -
>  drivers/gpu/drm/ttm/ttm_bo.c         | 1 -
>  drivers/gpu/drm/ttm/ttm_bo_util.c    | 8 ++++----
>  include/drm/ttm/ttm_bo_driver.h      | 2 --
>  4 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index a48652826f67..a1037478fa3f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -675,7 +675,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  			}
>  
>  			man->func = &nouveau_vram_manager;
> -			man->io_reserve_fastpath = false;
>  			man->use_io_reserve_lru = true;
>  		} else {
>  			man->func = &ttm_bo_manager_func;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 7be36b9996ed..8b9e7f62bea7 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1521,7 +1521,6 @@ int ttm_bo_init_mm(struct ttm_bo_device *bdev, unsigned type,
>  	BUG_ON(type >= TTM_NUM_MEM_TYPES);
>  	man = &bdev->man[type];
>  	BUG_ON(man->has_type);
> -	man->io_reserve_fastpath = true;
>  	man->use_io_reserve_lru = false;
>  	mutex_init(&man->io_reserve_mutex);
>  	spin_lock_init(&man->move_lock);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 7d2c50fef456..6c05f4fd15ae 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -93,7 +93,7 @@ EXPORT_SYMBOL(ttm_bo_move_ttm);
>  
>  int ttm_mem_io_lock(struct ttm_mem_type_manager *man, bool interruptible)
>  {
> -	if (likely(man->io_reserve_fastpath))
> +	if (likely(!man->use_io_reserve_lru))
>  		return 0;
>  
>  	if (interruptible)
> @@ -105,7 +105,7 @@ int ttm_mem_io_lock(struct ttm_mem_type_manager *man, bool interruptible)
>  
>  void ttm_mem_io_unlock(struct ttm_mem_type_manager *man)
>  {
> -	if (likely(man->io_reserve_fastpath))
> +	if (likely(!man->use_io_reserve_lru))
>  		return;
>  
>  	mutex_unlock(&man->io_reserve_mutex);
> @@ -136,7 +136,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
>  
>  	if (!bdev->driver->io_mem_reserve)
>  		return 0;
> -	if (likely(man->io_reserve_fastpath))
> +	if (likely(!man->use_io_reserve_lru))
>  		return bdev->driver->io_mem_reserve(bdev, mem);
>  
>  	if (bdev->driver->io_mem_reserve &&
> @@ -157,7 +157,7 @@ void ttm_mem_io_free(struct ttm_bo_device *bdev,
>  {
>  	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>  
> -	if (likely(man->io_reserve_fastpath))
> +	if (likely(!man->use_io_reserve_lru))
>  		return;
>  
>  	if (bdev->driver->io_mem_reserve &&
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 732167cad130..45522e4fbd6b 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -155,7 +155,6 @@ struct ttm_mem_type_manager_func {
>   * @use_io_reserve_lru: Use an lru list to try to unreserve io_mem_regions
>   * reserved by the TTM vm system.
>   * @io_reserve_lru: Optional lru list for unreserving io mem regions.
> - * @io_reserve_fastpath: Only use bdev::driver::io_mem_reserve to obtain
>   * @move_lock: lock for move fence
>   * static information. bdev::driver::io_mem_free is never used.
>   * @lru: The lru list for this memory type.
> @@ -184,7 +183,6 @@ struct ttm_mem_type_manager {
>  	void *priv;
>  	struct mutex io_reserve_mutex;
>  	bool use_io_reserve_lru;
> -	bool io_reserve_fastpath;
>  	spinlock_t move_lock;
>  
>  	/*
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 04/11] drm/ttm: cleanup coding style and implementation.
  2020-07-21  7:32 ` [PATCH 04/11] drm/ttm: cleanup coding style and implementation Christian König
@ 2020-07-21  8:56   ` daniel
  0 siblings, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  8:56 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:38AM +0200, Christian König wrote:
> Only functional change is to always keep io_reserved_count up to date
> for debugging even when it is not used otherwise.

Functional change in a cleanup patch. Tsk. It looks correct though ...

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/ttm/ttm_bo_util.c | 97 +++++++++++++++----------------
>  1 file changed, 48 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 6c05f4fd15ae..7fb3e0bcbab4 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -115,39 +115,35 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
>  {
>  	struct ttm_buffer_object *bo;
>  
> -	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
> +	bo = list_first_entry_or_null(&man->io_reserve_lru,
> +				      struct ttm_buffer_object,
> +				      io_reserve_lru);
> +	if (!bo)
>  		return -ENOSPC;
>  
> -	bo = list_first_entry(&man->io_reserve_lru,
> -			      struct ttm_buffer_object,
> -			      io_reserve_lru);
>  	list_del_init(&bo->io_reserve_lru);
>  	ttm_bo_unmap_virtual_locked(bo);
> -
>  	return 0;
>  }
>  
> -
>  int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
>  		       struct ttm_mem_reg *mem)
>  {
>  	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> -	int ret = 0;
> +	int ret;
> +
> +	if (mem->bus.io_reserved_count++)
> +		return 0;
>  
>  	if (!bdev->driver->io_mem_reserve)
>  		return 0;
> -	if (likely(!man->use_io_reserve_lru))
> -		return bdev->driver->io_mem_reserve(bdev, mem);
>  
> -	if (bdev->driver->io_mem_reserve &&
> -	    mem->bus.io_reserved_count++ == 0) {
>  retry:
> -		ret = bdev->driver->io_mem_reserve(bdev, mem);
> -		if (ret == -ENOSPC) {
> -			ret = ttm_mem_io_evict(man);
> -			if (ret == 0)
> -				goto retry;
> -		}
> +	ret = bdev->driver->io_mem_reserve(bdev, mem);
> +	if (ret == -ENOSPC) {
> +		ret = ttm_mem_io_evict(man);
> +		if (ret == 0)
> +			goto retry;
>  	}
>  	return ret;
>  }
> @@ -155,35 +151,31 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
>  void ttm_mem_io_free(struct ttm_bo_device *bdev,
>  		     struct ttm_mem_reg *mem)
>  {
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> -
> -	if (likely(!man->use_io_reserve_lru))
> +	if (--mem->bus.io_reserved_count)
>  		return;
>  
> -	if (bdev->driver->io_mem_reserve &&
> -	    --mem->bus.io_reserved_count == 0 &&
> -	    bdev->driver->io_mem_free)
> -		bdev->driver->io_mem_free(bdev, mem);
> +	if (!bdev->driver->io_mem_free)
> +		return;
>  
> +	bdev->driver->io_mem_free(bdev, mem);
>  }
>  
>  int ttm_mem_io_reserve_vm(struct ttm_buffer_object *bo)
>  {
> +	struct ttm_mem_type_manager *man = &bo->bdev->man[bo->mem.mem_type];
>  	struct ttm_mem_reg *mem = &bo->mem;
>  	int ret;
>  
> -	if (!mem->bus.io_reserved_vm) {
> -		struct ttm_mem_type_manager *man =
> -			&bo->bdev->man[mem->mem_type];
> +	if (mem->bus.io_reserved_vm)
> +		return 0;
>  
> -		ret = ttm_mem_io_reserve(bo->bdev, mem);
> -		if (unlikely(ret != 0))
> -			return ret;
> -		mem->bus.io_reserved_vm = true;
> -		if (man->use_io_reserve_lru)
> -			list_add_tail(&bo->io_reserve_lru,
> -				      &man->io_reserve_lru);
> -	}
> +	ret = ttm_mem_io_reserve(bo->bdev, mem);
> +	if (unlikely(ret != 0))
> +		return ret;
> +	mem->bus.io_reserved_vm = true;
> +	if (man->use_io_reserve_lru)
> +		list_add_tail(&bo->io_reserve_lru,
> +			      &man->io_reserve_lru);
>  	return 0;
>  }
>  
> @@ -191,15 +183,17 @@ void ttm_mem_io_free_vm(struct ttm_buffer_object *bo)
>  {
>  	struct ttm_mem_reg *mem = &bo->mem;
>  
> -	if (mem->bus.io_reserved_vm) {
> -		mem->bus.io_reserved_vm = false;
> -		list_del_init(&bo->io_reserve_lru);
> -		ttm_mem_io_free(bo->bdev, mem);
> -	}
> +	if (!mem->bus.io_reserved_vm)
> +		return;
> +
> +	mem->bus.io_reserved_vm = false;
> +	list_del_init(&bo->io_reserve_lru);
> +	ttm_mem_io_free(bo->bdev, mem);
>  }
>  
> -static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
> -			void **virtual)
> +static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev,
> +			       struct ttm_mem_reg *mem,
> +			       void **virtual)
>  {
>  	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>  	int ret;
> @@ -216,9 +210,11 @@ static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *m
>  		addr = mem->bus.addr;
>  	} else {
>  		if (mem->placement & TTM_PL_FLAG_WC)
> -			addr = ioremap_wc(mem->bus.base + mem->bus.offset, mem->bus.size);
> +			addr = ioremap_wc(mem->bus.base + mem->bus.offset,
> +					  mem->bus.size);
>  		else
> -			addr = ioremap(mem->bus.base + mem->bus.offset, mem->bus.size);
> +			addr = ioremap(mem->bus.base + mem->bus.offset,
> +				       mem->bus.size);
>  		if (!addr) {
>  			(void) ttm_mem_io_lock(man, false);
>  			ttm_mem_io_free(bdev, mem);
> @@ -230,8 +226,9 @@ static int ttm_mem_reg_ioremap(struct ttm_bo_device *bdev, struct ttm_mem_reg *m
>  	return 0;
>  }
>  
> -static void ttm_mem_reg_iounmap(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem,
> -			 void *virtual)
> +static void ttm_mem_reg_iounmap(struct ttm_bo_device *bdev,
> +				struct ttm_mem_reg *mem,
> +				void *virtual)
>  {
>  	struct ttm_mem_type_manager *man;
>  
> @@ -513,11 +510,13 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
>  	} else {
>  		map->bo_kmap_type = ttm_bo_map_iomap;
>  		if (mem->placement & TTM_PL_FLAG_WC)
> -			map->virtual = ioremap_wc(bo->mem.bus.base + bo->mem.bus.offset + offset,
> +			map->virtual = ioremap_wc(bo->mem.bus.base +
> +						  bo->mem.bus.offset + offset,
>  						  size);
>  		else
> -			map->virtual = ioremap(bo->mem.bus.base + bo->mem.bus.offset + offset,
> -						       size);
> +			map->virtual = ioremap(bo->mem.bus.base +
> +					       bo->mem.bus.offset + offset,
> +					       size);
>  	}
>  	return (!map->virtual) ? -ENOMEM : 0;
>  }
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 05/11] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA
  2020-07-21  7:32 ` [PATCH 05/11] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
@ 2020-07-21  9:01   ` daniel
  0 siblings, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  9:01 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:39AM +0200, Christian König wrote:
> The original intention was to avoid CPU page table unmaps
> when BOs move between the GTT and SYSTEM domain.
> 
> The problem is that this never correctly handled changes
> in the caching attributes or backing pages.
> 
> Just drop this for now and simply unmap the CPU page
> tables in all cases.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +--
>  drivers/gpu/drm/nouveau/nouveau_bo.c       |  3 +-
>  drivers/gpu/drm/radeon/radeon_ttm.c        |  2 +-
>  drivers/gpu/drm/ttm/ttm_bo.c               | 34 ++++------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  2 +-
>  include/drm/ttm/ttm_bo_driver.h            |  1 -
>  6 files changed, 11 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 9c0f12f74af9..44fa8bc49d18 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -93,7 +93,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		man->func = &amdgpu_gtt_mgr_func;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
> +		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  		break;
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
> @@ -108,7 +108,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_CMA;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED;
>  		man->default_caching = TTM_PL_FLAG_UNCACHED;
>  		break;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index a1037478fa3f..7883341f8c83 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -695,8 +695,7 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  				TTM_PL_FLAG_WC;
>  			man->default_caching = TTM_PL_FLAG_WC;
>  		} else {
> -			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE |
> -				     TTM_MEMTYPE_FLAG_CMA;
> +			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  			man->available_caching = TTM_PL_MASK_CACHING;
>  			man->default_caching = TTM_PL_FLAG_CACHED;
>  		}
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 73085523fad7..54af06df865b 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		man->func = &ttm_bo_manager_func;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE | TTM_MEMTYPE_FLAG_CMA;
> +		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  #if IS_ENABLED(CONFIG_AGP)
>  		if (rdev->flags & RADEON_IS_AGP) {
>  			if (!rdev->ddev->agp) {
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 8b9e7f62bea7..0768a054a916 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -272,20 +272,15 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  				  struct ttm_operation_ctx *ctx)
>  {
>  	struct ttm_bo_device *bdev = bo->bdev;
> -	bool old_is_pci = ttm_mem_reg_is_pci(bdev, &bo->mem);
> -	bool new_is_pci = ttm_mem_reg_is_pci(bdev, mem);
>  	struct ttm_mem_type_manager *old_man = &bdev->man[bo->mem.mem_type];
>  	struct ttm_mem_type_manager *new_man = &bdev->man[mem->mem_type];
> -	int ret = 0;
> +	int ret;
>  
> -	if (old_is_pci || new_is_pci ||
> -	    ((mem->placement & bo->mem.placement & TTM_PL_MASK_CACHING) == 0)) {
> -		ret = ttm_mem_io_lock(old_man, true);
> -		if (unlikely(ret != 0))
> -			goto out_err;
> -		ttm_bo_unmap_virtual_locked(bo);
> -		ttm_mem_io_unlock(old_man);
> -	}

Fastpath for when there's no mapping at all might be a more reasonable
idea than trying to keep it around. Lots of one-shot upload or indirect
upload buffers generally, and maybe avoiding the ttm_mem_type_manager io
mutex is worth it. Anyway totally different thing.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +	ret = ttm_mem_io_lock(old_man, true);
> +	if (unlikely(ret != 0))
> +		goto out_err;
> +	ttm_bo_unmap_virtual_locked(bo);
> +	ttm_mem_io_unlock(old_man);
>  
>  	/*
>  	 * Create and bind a ttm if required.
> @@ -1698,23 +1693,6 @@ EXPORT_SYMBOL(ttm_bo_device_init);
>   * buffer object vm functions.
>   */
>  
> -bool ttm_mem_reg_is_pci(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> -{
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> -
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
> -		if (mem->mem_type == TTM_PL_SYSTEM)
> -			return false;
> -
> -		if (man->flags & TTM_MEMTYPE_FLAG_CMA)
> -			return false;
> -
> -		if (mem->placement & TTM_PL_FLAG_CACHED)
> -			return false;
> -	}
> -	return true;
> -}
> -
>  void ttm_bo_unmap_virtual_locked(struct ttm_buffer_object *bo)
>  {
>  	struct ttm_bo_device *bdev = bo->bdev;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index bfd0c54ec30a..6bea7548aee0 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -762,7 +762,7 @@ 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->flags = TTM_MEMTYPE_FLAG_CMA | TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 45522e4fbd6b..71b195e78c7c 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -47,7 +47,6 @@
>  
>  #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
>  #define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)	/* Memory mappable */
> -#define TTM_MEMTYPE_FLAG_CMA           (1 << 3)	/* Can't map aperture */
>  
>  struct ttm_mem_type_manager;
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* RE: [PATCH 01/11] drm: remove optional dummy function from drivers using TTM
  2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
                   ` (11 preceding siblings ...)
  2020-07-21  8:02 ` Christian König
@ 2020-07-21  9:15 ` Chauhan, Madhav
  12 siblings, 0 replies; 35+ messages in thread
From: Chauhan, Madhav @ 2020-07-21  9:15 UTC (permalink / raw)
  To: Christian König, dri-devel; +Cc: michael.j.ruhl, tzimmermann

[AMD Public Use]

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Tuesday, July 21, 2020 1:03 PM
To: dri-devel@lists.freedesktop.org
Cc: Chauhan, Madhav <Madhav.Chauhan@amd.com>; tzimmermann@suse.de; michael.j.ruhl@intel.com
Subject: [PATCH 01/11] drm: remove optional dummy function from drivers using TTM

Implementing those is completely unecessary.

LGTM. Nitpick: Please check spell of "unecessary"

Reviewed-by: Madhav Chauhan <madhav.chauhan@amd.com>

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  5 -----
 drivers/gpu/drm/drm_gem_vram_helper.c      |  5 -----
 drivers/gpu/drm/qxl/qxl_ttm.c              |  6 ------
 drivers/gpu/drm/radeon/radeon_ttm.c        |  5 -----
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 11 -----------
 5 files changed, 32 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 3df685287cc1..9c0f12f74af9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -836,10 +836,6 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
 	return 0;
 }
 
-static void amdgpu_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem) -{ -}
-
 static unsigned long amdgpu_ttm_io_mem_pfn(struct ttm_buffer_object *bo,
 					   unsigned long page_offset)
 {
@@ -1754,7 +1750,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
 	.release_notify = &amdgpu_bo_release_notify,
 	.fault_reserve_notify = &amdgpu_bo_fault_reserve_notify,
 	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
-	.io_mem_free = &amdgpu_ttm_io_mem_free,
 	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
 	.access_memory = &amdgpu_ttm_access_memory,
 	.del_from_lru_notify = &amdgpu_vm_del_from_lru_notify diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index d107a2679e23..3296ed3df358 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1081,10 +1081,6 @@ static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
 	return 0;
 }
 
-static void bo_driver_io_mem_free(struct ttm_bo_device *bdev,
-				  struct ttm_mem_reg *mem)
-{ }
-
 static struct ttm_bo_driver bo_driver = {
 	.ttm_tt_create = bo_driver_ttm_tt_create,
 	.ttm_tt_populate = ttm_pool_populate,
@@ -1094,7 +1090,6 @@ static struct ttm_bo_driver bo_driver = {
 	.evict_flags = bo_driver_evict_flags,
 	.move_notify = bo_driver_move_notify,
 	.io_mem_reserve = bo_driver_io_mem_reserve,
-	.io_mem_free = bo_driver_io_mem_free,
 };
 
 /*
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 52eaa2d22745..a6e67149ef4a 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -129,11 +129,6 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
 	return 0;
 }
 
-static void qxl_ttm_io_mem_free(struct ttm_bo_device *bdev,
-				struct ttm_mem_reg *mem)
-{
-}
-
 /*
  * TTM backend functions.
  */
@@ -247,7 +242,6 @@ static struct ttm_bo_driver qxl_bo_driver = {
 	.evict_flags = &qxl_evict_flags,
 	.move = &qxl_bo_move,
 	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
-	.io_mem_free = &qxl_ttm_io_mem_free,
 	.move_notify = &qxl_bo_move_notify,
 };
 
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index f4f1e63731a5..73085523fad7 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -457,10 +457,6 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
 	return 0;
 }
 
-static void radeon_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem) -{ -}
-
 /*
  * TTM backend functions.
  */
@@ -774,7 +770,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
 	.move_notify = &radeon_bo_move_notify,
 	.fault_reserve_notify = &radeon_bo_fault_reserve_notify,
 	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
-	.io_mem_free = &radeon_ttm_io_mem_free,
 };
 
 int radeon_ttm_init(struct radeon_device *rdev) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index fbcd11a7b215..bfd0c54ec30a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -815,15 +815,6 @@ static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg
 	return 0;
 }
 
-static void vmw_ttm_io_mem_free(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem) -{ -}
-
-static int vmw_ttm_fault_reserve_notify(struct ttm_buffer_object *bo) -{
-	return 0;
-}
-
 /**
  * vmw_move_notify - TTM move_notify_callback
  *
@@ -866,7 +857,5 @@ struct ttm_bo_driver vmw_bo_driver = {
 	.verify_access = vmw_verify_access,
 	.move_notify = vmw_move_notify,
 	.swap_notify = vmw_swap_notify,
-	.fault_reserve_notify = &vmw_ttm_fault_reserve_notify,
 	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
-	.io_mem_free = &vmw_ttm_io_mem_free,
 };
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 ` [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-21  9:24   ` daniel
  2020-07-21 14:46     ` Christian König
  0 siblings, 1 reply; 35+ messages in thread
From: daniel @ 2020-07-21  9:24 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
> The driver doesn't expose any not-mapable memory resources.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 54af06df865b..b474781a0920 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		man->func = &ttm_bo_manager_func;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  #if IS_ENABLED(CONFIG_AGP)
>  		if (rdev->flags & RADEON_IS_AGP) {
>  			if (!rdev->ddev->agp) {
> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  					  (unsigned)type);
>  				return -EINVAL;
>  			}
> -			if (!rdev->ddev->agp->cant_use_aperture)
> -				man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;

There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
flag set. And radeon.ko did at least once work on these. And your patch to
disable agp only changes the default, it doesn't rip out the code.

So not sure your assumption here is correct.
-Daniel

>  			man->available_caching = TTM_PL_FLAG_UNCACHED |
>  						 TTM_PL_FLAG_WC;
>  			man->default_caching = TTM_PL_FLAG_WC;
> @@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
>  		break;
> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>  
>  static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>  {
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>  	struct radeon_device *rdev = radeon_get_rdev(bdev);
>  
>  	mem->bus.addr = NULL;
> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>  	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>  	mem->bus.base = 0;
>  	mem->bus.is_iomem = false;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;
> +
>  	switch (mem->mem_type) {
>  	case TTM_PL_SYSTEM:
>  		/* system memory */
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 07/11] drm/vmwgfx: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 ` [PATCH 07/11] drm/vmwgfx: " Christian König
@ 2020-07-21  9:26   ` daniel
  0 siblings, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  9:26 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:41AM +0200, Christian König wrote:
> The driver doesn't expose any not-mapable memory resources.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 6bea7548aee0..1d78187eaba6 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -742,15 +742,13 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &vmw_thp_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED | TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -762,7 +760,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->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -789,7 +786,6 @@ static int vmw_verify_access(struct ttm_buffer_object *bo, struct file *filp)
>  
>  static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>  {
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>  	struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
>  
>  	mem->bus.addr = NULL;
> @@ -797,8 +793,7 @@ static int vmw_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg
>  	mem->bus.offset = 0;
>  	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>  	mem->bus.base = 0;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;
> +
>  	switch (mem->mem_type) {
>  	case TTM_PL_SYSTEM:
>  	case VMW_PL_GMR:
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 08/11] drm/amdgpu: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 ` [PATCH 08/11] drm/amdgpu: " Christian König
@ 2020-07-21  9:28   ` daniel
  2020-07-21  9:45     ` Chauhan, Madhav
  2020-07-21 14:30     ` Christian König
  0 siblings, 2 replies; 35+ messages in thread
From: daniel @ 2020-07-21  9:28 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:42AM +0200, Christian König wrote:
> The driver does support some not-mapable resources, but
> those are already handled correctly in the switch/case
> statement in the code.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 44fa8bc49d18..0dd5e802091d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -84,7 +84,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -93,13 +93,12 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		man->func = &amdgpu_gtt_mgr_func;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		break;
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &amdgpu_vram_mgr_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED |
> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
>  		break;
> @@ -796,7 +795,6 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>   */
>  static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>  {
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>  	struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
>  	struct drm_mm_node *mm_node = mem->mm_node;
>  
> @@ -805,8 +803,7 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>  	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>  	mem->bus.base = 0;
>  	mem->bus.is_iomem = false;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;

This check catches the various special on-board memories, or at least I
couldnt' find where mmap for these is disallowed.
-Daniel

> +
>  	switch (mem->mem_type) {
>  	case TTM_PL_SYSTEM:
>  		/* system memory */
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 09/11] drm/nouveau: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 ` [PATCH 09/11] drm/nouveau: " Christian König
@ 2020-07-21  9:28   ` daniel
  0 siblings, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  9:28 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:43AM +0200, Christian König wrote:
> The driver doesn't expose any not-mapable memory resources.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 7883341f8c83..4ccf937df0d0 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -655,13 +655,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_VRAM:
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED |
> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED |
>  					 TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
> @@ -690,12 +689,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  			man->func = &ttm_bo_manager_func;
>  
>  		if (drm->agp.bridge) {
> -			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +			man->flags = 0;
>  			man->available_caching = TTM_PL_FLAG_UNCACHED |
>  				TTM_PL_FLAG_WC;
>  			man->default_caching = TTM_PL_FLAG_WC;
>  		} else {
> -			man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +			man->flags = 0;
>  			man->available_caching = TTM_PL_MASK_CACHING;
>  			man->default_caching = TTM_PL_FLAG_CACHED;
>  		}
> @@ -1437,7 +1436,6 @@ nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
>  static int
>  nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
>  {
> -	struct ttm_mem_type_manager *man = &bdev->man[reg->mem_type];
>  	struct nouveau_drm *drm = nouveau_bdev(bdev);
>  	struct nvkm_device *device = nvxx_device(&drm->client.device);
>  	struct nouveau_mem *mem = nouveau_mem(reg);
> @@ -1447,8 +1445,7 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
>  	reg->bus.size = reg->num_pages << PAGE_SHIFT;
>  	reg->bus.base = 0;
>  	reg->bus.is_iomem = false;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;
> +
>  	switch (reg->mem_type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 10/11] drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE v2
  2020-07-21  7:32 ` [PATCH 10/11] drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE v2 Christian König
@ 2020-07-21  9:29   ` daniel
  0 siblings, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  9:29 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:44AM +0200, Christian König wrote:
> The driver doesn't expose any not-mapable memory resources.
> 
> v2: remove unused man variable as well
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/qxl/qxl_ttm.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index a6e67149ef4a..1d8e07b8b19e 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -54,7 +54,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -62,8 +62,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case TTM_PL_PRIV:
>  		/* "On-card" video ram */
>  		man->func = &ttm_bo_manager_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED |
> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -99,7 +98,6 @@ static void qxl_evict_flags(struct ttm_buffer_object *bo,
>  int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
>  			   struct ttm_mem_reg *mem)
>  {
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>  	struct qxl_device *qdev = qxl_get_qdev(bdev);
>  
>  	mem->bus.addr = NULL;
> @@ -107,8 +105,7 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
>  	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>  	mem->bus.base = 0;
>  	mem->bus.is_iomem = false;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;
> +
>  	switch (mem->mem_type) {
>  	case TTM_PL_SYSTEM:
>  		/* system memory */
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 ` [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-21  9:30   ` daniel
  2020-07-21  9:54     ` kernel test robot
  1 sibling, 0 replies; 35+ messages in thread
From: daniel @ 2020-07-21  9:30 UTC (permalink / raw)
  Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

On Tue, Jul 21, 2020 at 09:32:45AM +0200, Christian König wrote:
> Not used any more. And it is bad design to use a TTM flag
> to do a check inside a driver.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  include/drm/ttm/ttm_bo_driver.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 71b195e78c7c..9b251853afe2 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -46,7 +46,6 @@
>  #define TTM_MAX_BO_PRIORITY	4U
>  
>  #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
> -#define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)	/* Memory mappable */

I think you can still do this, and it makes sense to delete: Just code a
driver-specific check in the io callback which checks whether a buffer can
be mappable directly, instead of going through the indirection of using
this flag.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* RE: [PATCH 08/11] drm/amdgpu: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  9:28   ` daniel
@ 2020-07-21  9:45     ` Chauhan, Madhav
  2020-07-21 14:30     ` Christian König
  1 sibling, 0 replies; 35+ messages in thread
From: Chauhan, Madhav @ 2020-07-21  9:45 UTC (permalink / raw)
  To: daniel; +Cc: michael.j.ruhl, tzimmermann, dri-devel

[AMD Public Use]

-----Original Message-----
From: daniel@ffwll.ch <daniel@ffwll.ch> 
Sent: Tuesday, July 21, 2020 2:58 PM
Cc: dri-devel@lists.freedesktop.org; Chauhan, Madhav <Madhav.Chauhan@amd.com>; michael.j.ruhl@intel.com; tzimmermann@suse.de
Subject: Re: [PATCH 08/11] drm/amdgpu: stop using TTM_MEMTYPE_FLAG_MAPPABLE

On Tue, Jul 21, 2020 at 09:32:42AM +0200, Christian König wrote:
> The driver does support some not-mapable resources, but those are 
> already handled correctly in the switch/case statement in the code.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 44fa8bc49d18..0dd5e802091d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -84,7 +84,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -93,13 +93,12 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		man->func = &amdgpu_gtt_mgr_func;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		break;
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &amdgpu_vram_mgr_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED |
> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
>  		break;
> @@ -796,7 +795,6 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>   */
>  static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, 
> struct ttm_mem_reg *mem)  {
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>  	struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
>  	struct drm_mm_node *mm_node = mem->mm_node;
>  
> @@ -805,8 +803,7 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>  	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>  	mem->bus.base = 0;
>  	mem->bus.is_iomem = false;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;

This check catches the various special on-board memories, or at least I couldnt' find where mmap for these is disallowed.

For various on board memories (GDS, GWS, OA). TTM_MEMTYPE_FLAG_MAPPABLE  is not added while initializing the BO manger.
Can you please elaborate??

Regards,
Madhav

-Daniel

> +
>  	switch (mem->mem_type) {
>  	case TTM_PL_SYSTEM:
>  		/* system memory */
> --
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7C
> Madhav.Chauhan%40amd.com%7C38d9dcf0a17344b8fa9008d82d586405%7C3dd8961f
> e4884e608e11a82d994e183d%7C0%7C0%7C637309204946382107&amp;sdata=4gkueE
> gTCU%2FQBqZS%2BepCLjEp%2F%2FEn%2FmhJl6EjB5LCfsQ%3D&amp;reserved=0

--
Daniel Vetter
Software Engineer, Intel Corporation
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&amp;data=02%7C01%7CMadhav.Chauhan%40amd.com%7C38d9dcf0a17344b8fa9008d82d586405%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637309204946382107&amp;sdata=g%2F4b4hJZZ3XehZmCVRi61GfCiGIwnJZT8nG%2Bb025o6k%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  7:32 ` [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-21  9:54     ` kernel test robot
  2020-07-21  9:54     ` kernel test robot
  1 sibling, 0 replies; 35+ messages in thread
From: kernel test robot @ 2020-07-21  9:54 UTC (permalink / raw)
  To: Christian König, dri-devel
  Cc: Madhav.Chauhan, michael.j.ruhl, kbuild-all, tzimmermann

[-- Attachment #1: Type: text/plain, Size: 4153 bytes --]

Hi "Christian,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-tip/drm-tip]
[also build test ERROR on next-20200720]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.8-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christian-K-nig/drm-remove-optional-dummy-function-from-drivers-using-TTM/20200721-153530
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/drm_gem_vram_helper.c: In function 'bo_driver_init_mem_type':
>> drivers/gpu/drm/drm_gem_vram_helper.c:1012:16: error: 'TTM_MEMTYPE_FLAG_MAPPABLE' undeclared (first use in this function); did you mean 'TTM_MEMTYPE_FLAG_FIXED'?
    1012 |   man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                TTM_MEMTYPE_FLAG_FIXED
   drivers/gpu/drm/drm_gem_vram_helper.c:1012:16: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/drm_gem_vram_helper.c: In function 'bo_driver_io_mem_reserve':
   drivers/gpu/drm/drm_gem_vram_helper.c:1065:21: error: 'TTM_MEMTYPE_FLAG_MAPPABLE' undeclared (first use in this function); did you mean 'TTM_MEMTYPE_FLAG_FIXED'?
    1065 |  if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                     TTM_MEMTYPE_FLAG_FIXED

vim +1012 drivers/gpu/drm/drm_gem_vram_helper.c

6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1006  
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1007  static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1008  				   struct ttm_mem_type_manager *man)
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1009  {
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1010  	switch (type) {
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1011  	case TTM_PL_SYSTEM:
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11 @1012  		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1013  		man->available_caching = TTM_PL_MASK_CACHING;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1014  		man->default_caching = TTM_PL_FLAG_CACHED;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1015  		break;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1016  	case TTM_PL_VRAM:
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1017  		man->func = &ttm_bo_manager_func;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1018  		man->flags = TTM_MEMTYPE_FLAG_FIXED |
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1019  			     TTM_MEMTYPE_FLAG_MAPPABLE;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1020  		man->available_caching = TTM_PL_FLAG_UNCACHED |
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1021  					 TTM_PL_FLAG_WC;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1022  		man->default_caching = TTM_PL_FLAG_WC;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1023  		break;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1024  	default:
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1025  		return -EINVAL;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1026  	}
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1027  	return 0;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1028  }
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1029  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61112 bytes --]

[-- Attachment #3: 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] 35+ messages in thread

* Re: [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE
@ 2020-07-21  9:54     ` kernel test robot
  0 siblings, 0 replies; 35+ messages in thread
From: kernel test robot @ 2020-07-21  9:54 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4223 bytes --]

Hi "Christian,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-tip/drm-tip]
[also build test ERROR on next-20200720]
[cannot apply to drm-intel/for-linux-next drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.8-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christian-K-nig/drm-remove-optional-dummy-function-from-drivers-using-TTM/20200721-153530
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/gpu/drm/drm_gem_vram_helper.c: In function 'bo_driver_init_mem_type':
>> drivers/gpu/drm/drm_gem_vram_helper.c:1012:16: error: 'TTM_MEMTYPE_FLAG_MAPPABLE' undeclared (first use in this function); did you mean 'TTM_MEMTYPE_FLAG_FIXED'?
    1012 |   man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                TTM_MEMTYPE_FLAG_FIXED
   drivers/gpu/drm/drm_gem_vram_helper.c:1012:16: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/drm_gem_vram_helper.c: In function 'bo_driver_io_mem_reserve':
   drivers/gpu/drm/drm_gem_vram_helper.c:1065:21: error: 'TTM_MEMTYPE_FLAG_MAPPABLE' undeclared (first use in this function); did you mean 'TTM_MEMTYPE_FLAG_FIXED'?
    1065 |  if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~
         |                     TTM_MEMTYPE_FLAG_FIXED

vim +1012 drivers/gpu/drm/drm_gem_vram_helper.c

6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1006  
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1007  static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1008  				   struct ttm_mem_type_manager *man)
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1009  {
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1010  	switch (type) {
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1011  	case TTM_PL_SYSTEM:
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11 @1012  		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1013  		man->available_caching = TTM_PL_MASK_CACHING;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1014  		man->default_caching = TTM_PL_FLAG_CACHED;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1015  		break;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1016  	case TTM_PL_VRAM:
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1017  		man->func = &ttm_bo_manager_func;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1018  		man->flags = TTM_MEMTYPE_FLAG_FIXED |
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1019  			     TTM_MEMTYPE_FLAG_MAPPABLE;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1020  		man->available_caching = TTM_PL_FLAG_UNCACHED |
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1021  					 TTM_PL_FLAG_WC;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1022  		man->default_caching = TTM_PL_FLAG_WC;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1023  		break;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1024  	default:
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1025  		return -EINVAL;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1026  	}
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1027  	return 0;
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1028  }
6b5ce4a1fb8489 Thomas Zimmermann 2019-09-11  1029  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 61112 bytes --]

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

* Re: [PATCH 08/11] drm/amdgpu: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  9:28   ` daniel
  2020-07-21  9:45     ` Chauhan, Madhav
@ 2020-07-21 14:30     ` Christian König
  1 sibling, 0 replies; 35+ messages in thread
From: Christian König @ 2020-07-21 14:30 UTC (permalink / raw)
  To: daniel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

Am 21.07.20 um 11:28 schrieb daniel@ffwll.ch:
> On Tue, Jul 21, 2020 at 09:32:42AM +0200, Christian König wrote:
>> The driver does support some not-mapable resources, but
>> those are already handled correctly in the switch/case
>> statement in the code.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 44fa8bc49d18..0dd5e802091d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -84,7 +84,7 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* System memory */
>> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> +		man->flags = 0;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>> @@ -93,13 +93,12 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		man->func = &amdgpu_gtt_mgr_func;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> +		man->flags = 0;
>>   		break;
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>>   		man->func = &amdgpu_vram_mgr_func;
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED |
>> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
>> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>>   		break;
>> @@ -796,7 +795,6 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
>>    */
>>   static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>>   {
>> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>>   	struct amdgpu_device *adev = amdgpu_ttm_adev(bdev);
>>   	struct drm_mm_node *mm_node = mem->mm_node;
>>   
>> @@ -805,8 +803,7 @@ static int amdgpu_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>>   	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>>   	mem->bus.base = 0;
>>   	mem->bus.is_iomem = false;
>> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
>> -		return -EINVAL;
> This check catches the various special on-board memories, or at least I
> couldnt' find where mmap for these is disallowed.

See the switch (mem->mem_type) just below, that return -EINVAL as well 
for those.

There is exactly zero functionality change here :)

Christian.

> -Daniel
>
>> +
>>   	switch (mem->mem_type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* system memory */
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> 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] 35+ messages in thread

* Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21  9:24   ` daniel
@ 2020-07-21 14:46     ` Christian König
  2020-07-22  5:34       ` Daniel Vetter
  0 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-21 14:46 UTC (permalink / raw)
  To: daniel; +Cc: Madhav.Chauhan, michael.j.ruhl, tzimmermann, dri-devel

Am 21.07.20 um 11:24 schrieb daniel@ffwll.ch:
> On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
>> The driver doesn't expose any not-mapable memory resources.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>> index 54af06df865b..b474781a0920 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* System memory */
>> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> +		man->flags = 0;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		man->func = &ttm_bo_manager_func;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>> +		man->flags = 0;
>>   #if IS_ENABLED(CONFIG_AGP)
>>   		if (rdev->flags & RADEON_IS_AGP) {
>>   			if (!rdev->ddev->agp) {
>> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   					  (unsigned)type);
>>   				return -EINVAL;
>>   			}
>> -			if (!rdev->ddev->agp->cant_use_aperture)
>> -				man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
> flag set. And radeon.ko did at least once work on these. And your patch to
> disable agp only changes the default, it doesn't rip out the code.

The key pint is that the flags for AGP are the same as the one for the 
PCIe path. So no functional change at all :)

The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().

Christian.

>
> So not sure your assumption here is correct.
> -Daniel
>
>>   			man->available_caching = TTM_PL_FLAG_UNCACHED |
>>   						 TTM_PL_FLAG_WC;
>>   			man->default_caching = TTM_PL_FLAG_WC;
>> @@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
>> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
>> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>>   		break;
>> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>>   
>>   static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>>   {
>> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>>   	struct radeon_device *rdev = radeon_get_rdev(bdev);
>>   
>>   	mem->bus.addr = NULL;
>> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>>   	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>>   	mem->bus.base = 0;
>>   	mem->bus.is_iomem = false;
>> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
>> -		return -EINVAL;
>> +
>>   	switch (mem->mem_type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* system memory */
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> 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] 35+ messages in thread

* Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-21 14:46     ` Christian König
@ 2020-07-22  5:34       ` Daniel Vetter
  2020-07-22 11:13         ` Christian König
  0 siblings, 1 reply; 35+ messages in thread
From: Daniel Vetter @ 2020-07-22  5:34 UTC (permalink / raw)
  To: Christian König
  Cc: Madhav.Chauhan, Ruhl, Michael J, Thomas Zimmermann, dri-devel

On Tue, Jul 21, 2020 at 4:46 PM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 21.07.20 um 11:24 schrieb daniel@ffwll.ch:
> > On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
> >> The driver doesn't expose any not-mapable memory resources.
> >>
> >> Signed-off-by: Christian König <christian.koenig@amd.com>
> >> ---
> >>   drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
> >>   1 file changed, 4 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> >> index 54af06df865b..b474781a0920 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> >> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> >> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>      switch (type) {
> >>      case TTM_PL_SYSTEM:
> >>              /* System memory */
> >> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >> +            man->flags = 0;
> >>              man->available_caching = TTM_PL_MASK_CACHING;
> >>              man->default_caching = TTM_PL_FLAG_CACHED;
> >>              break;
> >> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>              man->func = &ttm_bo_manager_func;
> >>              man->available_caching = TTM_PL_MASK_CACHING;
> >>              man->default_caching = TTM_PL_FLAG_CACHED;
> >> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >> +            man->flags = 0;
> >>   #if IS_ENABLED(CONFIG_AGP)
> >>              if (rdev->flags & RADEON_IS_AGP) {
> >>                      if (!rdev->ddev->agp) {
> >> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>                                        (unsigned)type);
> >>                              return -EINVAL;
> >>                      }
> >> -                    if (!rdev->ddev->agp->cant_use_aperture)
> >> -                            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> > There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
> > flag set. And radeon.ko did at least once work on these. And your patch to
> > disable agp only changes the default, it doesn't rip out the code.
>
> The key pint is that the flags for AGP are the same as the one for the
> PCIe path. So no functional change at all :)

I misread the code somehow, I didn't spot the unconditional setting of
FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
thought that's another case.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

>
> The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().
>
> Christian.
>
> >
> > So not sure your assumption here is correct.
> > -Daniel
> >
> >>                      man->available_caching = TTM_PL_FLAG_UNCACHED |
> >>                                               TTM_PL_FLAG_WC;
> >>                      man->default_caching = TTM_PL_FLAG_WC;
> >> @@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
> >> -                         TTM_MEMTYPE_FLAG_MAPPABLE;
> >> +            man->flags = TTM_MEMTYPE_FLAG_FIXED;
> >>              man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> >>              man->default_caching = TTM_PL_FLAG_WC;
> >>              break;
> >> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
> >>
> >>   static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> >>   {
> >> -    struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> >>      struct radeon_device *rdev = radeon_get_rdev(bdev);
> >>
> >>      mem->bus.addr = NULL;
> >> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
> >>      mem->bus.size = mem->num_pages << PAGE_SHIFT;
> >>      mem->bus.base = 0;
> >>      mem->bus.is_iomem = false;
> >> -    if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> >> -            return -EINVAL;
> >> +
> >>      switch (mem->mem_type) {
> >>      case TTM_PL_SYSTEM:
> >>              /* system memory */
> >> --
> >> 2.17.1
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* RE: [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau
  2020-07-21  7:32 ` [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau Christian König
  2020-07-21  8:50   ` daniel
@ 2020-07-22  7:05   ` Chauhan, Madhav
  1 sibling, 0 replies; 35+ messages in thread
From: Chauhan, Madhav @ 2020-07-22  7:05 UTC (permalink / raw)
  To: Christian König, dri-devel; +Cc: michael.j.ruhl, tzimmermann

[AMD Public Use]

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Tuesday, July 21, 2020 1:03 PM
To: dri-devel@lists.freedesktop.org
Cc: Chauhan, Madhav <Madhav.Chauhan@amd.com>; tzimmermann@suse.de; michael.j.ruhl@intel.com
Subject: [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau

Nouveau is the only user of this functionality and evicting io space on -EAGAIN is really a misuse of the return code.

Instead switch to using -ENOSPC here which makes much more sense and simplifies the code.

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

Complete remaining cleanup patches (Patch 2- 11) looks fine,
Patch 2-11: Reviewed-by: Madhav Chauhan <madhav.chauhan@amd.com>

Regards,
Madhav
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 2 --
 drivers/gpu/drm/ttm/ttm_bo_util.c    | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 61355cfb7335..a48652826f67 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1505,8 +1505,6 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *reg)
 			if (ret != 1) {
 				if (WARN_ON(ret == 0))
 					return -EINVAL;
-				if (ret == -ENOSPC)
-					return -EAGAIN;
 				return ret;
 			}
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 5e0f3a9caedc..7d2c50fef456 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -116,7 +116,7 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager *man)
 	struct ttm_buffer_object *bo;
 
 	if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
-		return -EAGAIN;
+		return -ENOSPC;
 
 	bo = list_first_entry(&man->io_reserve_lru,
 			      struct ttm_buffer_object,
@@ -143,7 +143,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
 	    mem->bus.io_reserved_count++ == 0) {
 retry:
 		ret = bdev->driver->io_mem_reserve(bdev, mem);
-		if (ret == -EAGAIN) {
+		if (ret == -ENOSPC) {
 			ret = ttm_mem_io_evict(man);
 			if (ret == 0)
 				goto retry;
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-22  5:34       ` Daniel Vetter
@ 2020-07-22 11:13         ` Christian König
  2020-07-22 11:42           ` Daniel Vetter
  0 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-22 11:13 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Madhav.Chauhan, Ruhl, Michael J, Thomas Zimmermann, dri-devel

Am 22.07.20 um 07:34 schrieb Daniel Vetter:
> On Tue, Jul 21, 2020 at 4:46 PM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
>> Am 21.07.20 um 11:24 schrieb daniel@ffwll.ch:
>>> On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
>>>> The driver doesn't expose any not-mapable memory resources.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
>>>>    1 file changed, 4 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>>>> index 54af06df865b..b474781a0920 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>>>> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>>>       switch (type) {
>>>>       case TTM_PL_SYSTEM:
>>>>               /* System memory */
>>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>>>> +            man->flags = 0;
>>>>               man->available_caching = TTM_PL_MASK_CACHING;
>>>>               man->default_caching = TTM_PL_FLAG_CACHED;
>>>>               break;
>>>> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>>>               man->func = &ttm_bo_manager_func;
>>>>               man->available_caching = TTM_PL_MASK_CACHING;
>>>>               man->default_caching = TTM_PL_FLAG_CACHED;
>>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>>>> +            man->flags = 0;
>>>>    #if IS_ENABLED(CONFIG_AGP)
>>>>               if (rdev->flags & RADEON_IS_AGP) {
>>>>                       if (!rdev->ddev->agp) {
>>>> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>>>                                         (unsigned)type);
>>>>                               return -EINVAL;
>>>>                       }
>>>> -                    if (!rdev->ddev->agp->cant_use_aperture)
>>>> -                            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>>> There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
>>> flag set. And radeon.ko did at least once work on these. And your patch to
>>> disable agp only changes the default, it doesn't rip out the code.
>> The key pint is that the flags for AGP are the same as the one for the
>> PCIe path. So no functional change at all :)
> I misread the code somehow, I didn't spot the unconditional setting of
> FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
> thought that's another case.
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

And for the amdgpu patch? Otherwise I just ping Alex for an rb.

Thanks,
Christian.

>
>> The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().
>>
>> Christian.
>>
>>> So not sure your assumption here is correct.
>>> -Daniel
>>>
>>>>                       man->available_caching = TTM_PL_FLAG_UNCACHED |
>>>>                                                TTM_PL_FLAG_WC;
>>>>                       man->default_caching = TTM_PL_FLAG_WC;
>>>> @@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
>>>> -                         TTM_MEMTYPE_FLAG_MAPPABLE;
>>>> +            man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>>>               man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>>>>               man->default_caching = TTM_PL_FLAG_WC;
>>>>               break;
>>>> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>>>>
>>>>    static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>>>>    {
>>>> -    struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>>>>       struct radeon_device *rdev = radeon_get_rdev(bdev);
>>>>
>>>>       mem->bus.addr = NULL;
>>>> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>>>>       mem->bus.size = mem->num_pages << PAGE_SHIFT;
>>>>       mem->bus.base = 0;
>>>>       mem->bus.is_iomem = false;
>>>> -    if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
>>>> -            return -EINVAL;
>>>> +
>>>>       switch (mem->mem_type) {
>>>>       case TTM_PL_SYSTEM:
>>>>               /* system memory */
>>>> --
>>>> 2.17.1
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C2fb0200ef32f4afcc3e208d82e00ef7d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637309928847998899&amp;sdata=9VclRJ7e3xfohsaLsiVF6Y83c%2Bkncmbo5uqoeV6tT9M%3D&amp;reserved=0
>

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

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

* Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-22 11:13         ` Christian König
@ 2020-07-22 11:42           ` Daniel Vetter
  2020-07-22 11:49             ` Christian König
  0 siblings, 1 reply; 35+ messages in thread
From: Daniel Vetter @ 2020-07-22 11:42 UTC (permalink / raw)
  To: Christian König
  Cc: Madhav.Chauhan, Ruhl, Michael J, Thomas Zimmermann, dri-devel

On Wed, Jul 22, 2020 at 1:13 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 22.07.20 um 07:34 schrieb Daniel Vetter:
> > On Tue, Jul 21, 2020 at 4:46 PM Christian König
> > <ckoenig.leichtzumerken@gmail.com> wrote:
> >> Am 21.07.20 um 11:24 schrieb daniel@ffwll.ch:
> >>> On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
> >>>> The driver doesn't expose any not-mapable memory resources.
> >>>>
> >>>> Signed-off-by: Christian König <christian.koenig@amd.com>
> >>>> ---
> >>>>    drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
> >>>>    1 file changed, 4 insertions(+), 9 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> >>>> index 54af06df865b..b474781a0920 100644
> >>>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> >>>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> >>>> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>>>       switch (type) {
> >>>>       case TTM_PL_SYSTEM:
> >>>>               /* System memory */
> >>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >>>> +            man->flags = 0;
> >>>>               man->available_caching = TTM_PL_MASK_CACHING;
> >>>>               man->default_caching = TTM_PL_FLAG_CACHED;
> >>>>               break;
> >>>> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>>>               man->func = &ttm_bo_manager_func;
> >>>>               man->available_caching = TTM_PL_MASK_CACHING;
> >>>>               man->default_caching = TTM_PL_FLAG_CACHED;
> >>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >>>> +            man->flags = 0;
> >>>>    #if IS_ENABLED(CONFIG_AGP)
> >>>>               if (rdev->flags & RADEON_IS_AGP) {
> >>>>                       if (!rdev->ddev->agp) {
> >>>> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>>>                                         (unsigned)type);
> >>>>                               return -EINVAL;
> >>>>                       }
> >>>> -                    if (!rdev->ddev->agp->cant_use_aperture)
> >>>> -                            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >>> There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
> >>> flag set. And radeon.ko did at least once work on these. And your patch to
> >>> disable agp only changes the default, it doesn't rip out the code.
> >> The key pint is that the flags for AGP are the same as the one for the
> >> PCIe path. So no functional change at all :)
> > I misread the code somehow, I didn't spot the unconditional setting of
> > FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
> > thought that's another case.
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> And for the amdgpu patch? Otherwise I just ping Alex for an rb.

See my question over there, I'm not seeing how the code prevents mmap
for AMDGPU_PL_* domains after your patch. Once that's cleared up happy
to r-b that one and the final one too.
-Daniel

>
> Thanks,
> Christian.
>
> >
> >> The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().
> >>
> >> Christian.
> >>
> >>> So not sure your assumption here is correct.
> >>> -Daniel
> >>>
> >>>>                       man->available_caching = TTM_PL_FLAG_UNCACHED |
> >>>>                                                TTM_PL_FLAG_WC;
> >>>>                       man->default_caching = TTM_PL_FLAG_WC;
> >>>> @@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
> >>>> -                         TTM_MEMTYPE_FLAG_MAPPABLE;
> >>>> +            man->flags = TTM_MEMTYPE_FLAG_FIXED;
> >>>>               man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> >>>>               man->default_caching = TTM_PL_FLAG_WC;
> >>>>               break;
> >>>> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
> >>>>
> >>>>    static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> >>>>    {
> >>>> -    struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> >>>>       struct radeon_device *rdev = radeon_get_rdev(bdev);
> >>>>
> >>>>       mem->bus.addr = NULL;
> >>>> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
> >>>>       mem->bus.size = mem->num_pages << PAGE_SHIFT;
> >>>>       mem->bus.base = 0;
> >>>>       mem->bus.is_iomem = false;
> >>>> -    if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> >>>> -            return -EINVAL;
> >>>> +
> >>>>       switch (mem->mem_type) {
> >>>>       case TTM_PL_SYSTEM:
> >>>>               /* system memory */
> >>>> --
> >>>> 2.17.1
> >>>>
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel@lists.freedesktop.org
> >>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C2fb0200ef32f4afcc3e208d82e00ef7d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637309928847998899&amp;sdata=9VclRJ7e3xfohsaLsiVF6Y83c%2Bkncmbo5uqoeV6tT9M%3D&amp;reserved=0
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-22 11:42           ` Daniel Vetter
@ 2020-07-22 11:49             ` Christian König
  2020-07-22 11:58               ` Daniel Vetter
  0 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2020-07-22 11:49 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Madhav.Chauhan, Ruhl, Michael J, Thomas Zimmermann, dri-devel

Am 22.07.20 um 13:42 schrieb Daniel Vetter:
> On Wed, Jul 22, 2020 at 1:13 PM Christian König
> <christian.koenig@amd.com> wrote:
>> Am 22.07.20 um 07:34 schrieb Daniel Vetter:
>>> On Tue, Jul 21, 2020 at 4:46 PM Christian König
>>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>>> Am 21.07.20 um 11:24 schrieb daniel@ffwll.ch:
>>>>> On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
>>>>>> The driver doesn't expose any not-mapable memory resources.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>>> ---
>>>>>>     drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
>>>>>>     1 file changed, 4 insertions(+), 9 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>>>>>> index 54af06df865b..b474781a0920 100644
>>>>>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>>>>>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>>>>>> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>>>>>        switch (type) {
>>>>>>        case TTM_PL_SYSTEM:
>>>>>>                /* System memory */
>>>>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>>>>>> +            man->flags = 0;
>>>>>>                man->available_caching = TTM_PL_MASK_CACHING;
>>>>>>                man->default_caching = TTM_PL_FLAG_CACHED;
>>>>>>                break;
>>>>>> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>>>>>                man->func = &ttm_bo_manager_func;
>>>>>>                man->available_caching = TTM_PL_MASK_CACHING;
>>>>>>                man->default_caching = TTM_PL_FLAG_CACHED;
>>>>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>>>>>> +            man->flags = 0;
>>>>>>     #if IS_ENABLED(CONFIG_AGP)
>>>>>>                if (rdev->flags & RADEON_IS_AGP) {
>>>>>>                        if (!rdev->ddev->agp) {
>>>>>> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>>>>>                                          (unsigned)type);
>>>>>>                                return -EINVAL;
>>>>>>                        }
>>>>>> -                    if (!rdev->ddev->agp->cant_use_aperture)
>>>>>> -                            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>>>>> There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
>>>>> flag set. And radeon.ko did at least once work on these. And your patch to
>>>>> disable agp only changes the default, it doesn't rip out the code.
>>>> The key pint is that the flags for AGP are the same as the one for the
>>>> PCIe path. So no functional change at all :)
>>> I misread the code somehow, I didn't spot the unconditional setting of
>>> FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
>>> thought that's another case.
>>>
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> And for the amdgpu patch? Otherwise I just ping Alex for an rb.
> See my question over there, I'm not seeing how the code prevents mmap
> for AMDGPU_PL_* domains after your patch. Once that's cleared up happy
> to r-b that one and the final one too.

I already replied, sounds like you never got that.

Anyway see the switch just below the two lines I removed:
>         switch (mem->mem_type) {
>         case TTM_PL_SYSTEM:
....
>         case TTM_PL_TT:
...
>         case TTM_PL_VRAM:
...
>         default:
>                 return -EINVAL;
>         }

So again, no functional change at all.

Cheers,
Christian.

> -Daniel
>
>> Thanks,
>> Christian.
>>
>>>> The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().
>>>>
>>>> Christian.
>>>>
>>>>> So not sure your assumption here is correct.
>>>>> -Daniel
>>>>>
>>>>>>                        man->available_caching = TTM_PL_FLAG_UNCACHED |
>>>>>>                                                 TTM_PL_FLAG_WC;
>>>>>>                        man->default_caching = TTM_PL_FLAG_WC;
>>>>>> @@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
>>>>>> -                         TTM_MEMTYPE_FLAG_MAPPABLE;
>>>>>> +            man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>>>>>                man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>>>>>>                man->default_caching = TTM_PL_FLAG_WC;
>>>>>>                break;
>>>>>> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>>>>>>
>>>>>>     static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
>>>>>>     {
>>>>>> -    struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>>>>>>        struct radeon_device *rdev = radeon_get_rdev(bdev);
>>>>>>
>>>>>>        mem->bus.addr = NULL;
>>>>>> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>>>>>>        mem->bus.size = mem->num_pages << PAGE_SHIFT;
>>>>>>        mem->bus.base = 0;
>>>>>>        mem->bus.is_iomem = false;
>>>>>> -    if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
>>>>>> -            return -EINVAL;
>>>>>> +
>>>>>>        switch (mem->mem_type) {
>>>>>>        case TTM_PL_SYSTEM:
>>>>>>                /* system memory */
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Cb10b1b6716484915990d08d82e3465f4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637310149874876539&amp;sdata=Ub8%2F0d13KkJdu7YAtFWpJtoVXuT8RrTewKSuBU6QnYo%3D&amp;reserved=0
>

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

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

* Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-22 11:49             ` Christian König
@ 2020-07-22 11:58               ` Daniel Vetter
  0 siblings, 0 replies; 35+ messages in thread
From: Daniel Vetter @ 2020-07-22 11:58 UTC (permalink / raw)
  To: Christian König
  Cc: Madhav.Chauhan, Ruhl, Michael J, Thomas Zimmermann, dri-devel

On Wed, Jul 22, 2020 at 1:50 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 22.07.20 um 13:42 schrieb Daniel Vetter:
> > On Wed, Jul 22, 2020 at 1:13 PM Christian König
> > <christian.koenig@amd.com> wrote:
> >> Am 22.07.20 um 07:34 schrieb Daniel Vetter:
> >>> On Tue, Jul 21, 2020 at 4:46 PM Christian König
> >>> <ckoenig.leichtzumerken@gmail.com> wrote:
> >>>> Am 21.07.20 um 11:24 schrieb daniel@ffwll.ch:
> >>>>> On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
> >>>>>> The driver doesn't expose any not-mapable memory resources.
> >>>>>>
> >>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
> >>>>>> ---
> >>>>>>     drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
> >>>>>>     1 file changed, 4 insertions(+), 9 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> >>>>>> index 54af06df865b..b474781a0920 100644
> >>>>>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> >>>>>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> >>>>>> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>>>>>        switch (type) {
> >>>>>>        case TTM_PL_SYSTEM:
> >>>>>>                /* System memory */
> >>>>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >>>>>> +            man->flags = 0;
> >>>>>>                man->available_caching = TTM_PL_MASK_CACHING;
> >>>>>>                man->default_caching = TTM_PL_FLAG_CACHED;
> >>>>>>                break;
> >>>>>> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>>>>>                man->func = &ttm_bo_manager_func;
> >>>>>>                man->available_caching = TTM_PL_MASK_CACHING;
> >>>>>>                man->default_caching = TTM_PL_FLAG_CACHED;
> >>>>>> -            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >>>>>> +            man->flags = 0;
> >>>>>>     #if IS_ENABLED(CONFIG_AGP)
> >>>>>>                if (rdev->flags & RADEON_IS_AGP) {
> >>>>>>                        if (!rdev->ddev->agp) {
> >>>>>> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >>>>>>                                          (unsigned)type);
> >>>>>>                                return -EINVAL;
> >>>>>>                        }
> >>>>>> -                    if (!rdev->ddev->agp->cant_use_aperture)
> >>>>>> -                            man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >>>>> There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
> >>>>> flag set. And radeon.ko did at least once work on these. And your patch to
> >>>>> disable agp only changes the default, it doesn't rip out the code.
> >>>> The key pint is that the flags for AGP are the same as the one for the
> >>>> PCIe path. So no functional change at all :)
> >>> I misread the code somehow, I didn't spot the unconditional setting of
> >>> FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
> >>> thought that's another case.
> >>>
> >>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> And for the amdgpu patch? Otherwise I just ping Alex for an rb.
> > See my question over there, I'm not seeing how the code prevents mmap
> > for AMDGPU_PL_* domains after your patch. Once that's cleared up happy
> > to r-b that one and the final one too.
>
> I already replied, sounds like you never got that.

I got it, but I suck at reading mailing lists.

> Anyway see the switch just below the two lines I removed:
> >         switch (mem->mem_type) {
> >         case TTM_PL_SYSTEM:
> ....
> >         case TTM_PL_TT:
> ...
> >         case TTM_PL_VRAM:
> ...
> >         default:
> >                 return -EINVAL;
> >         }
>
> So again, no functional change at all.

Indeed, I score another point for being blind. r-b: also on the amdgpu
and final cleanup patch.
-Daniel

>
> Cheers,
> Christian.
>
> > -Daniel
> >
> >> Thanks,
> >> Christian.
> >>
> >>>> The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().
> >>>>
> >>>> Christian.
> >>>>
> >>>>> So not sure your assumption here is correct.
> >>>>> -Daniel
> >>>>>
> >>>>>>                        man->available_caching = TTM_PL_FLAG_UNCACHED |
> >>>>>>                                                 TTM_PL_FLAG_WC;
> >>>>>>                        man->default_caching = TTM_PL_FLAG_WC;
> >>>>>> @@ -103,8 +101,7 @@ 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->flags = TTM_MEMTYPE_FLAG_FIXED |
> >>>>>> -                         TTM_MEMTYPE_FLAG_MAPPABLE;
> >>>>>> +            man->flags = TTM_MEMTYPE_FLAG_FIXED;
> >>>>>>                man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> >>>>>>                man->default_caching = TTM_PL_FLAG_WC;
> >>>>>>                break;
> >>>>>> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
> >>>>>>
> >>>>>>     static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)
> >>>>>>     {
> >>>>>> -    struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
> >>>>>>        struct radeon_device *rdev = radeon_get_rdev(bdev);
> >>>>>>
> >>>>>>        mem->bus.addr = NULL;
> >>>>>> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
> >>>>>>        mem->bus.size = mem->num_pages << PAGE_SHIFT;
> >>>>>>        mem->bus.base = 0;
> >>>>>>        mem->bus.is_iomem = false;
> >>>>>> -    if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> >>>>>> -            return -EINVAL;
> >>>>>> +
> >>>>>>        switch (mem->mem_type) {
> >>>>>>        case TTM_PL_SYSTEM:
> >>>>>>                /* system memory */
> >>>>>> --
> >>>>>> 2.17.1
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> dri-devel mailing list
> >>>>>> dri-devel@lists.freedesktop.org
> >>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Cb10b1b6716484915990d08d82e3465f4%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637310149874876539&amp;sdata=Ub8%2F0d13KkJdu7YAtFWpJtoVXuT8RrTewKSuBU6QnYo%3D&amp;reserved=0
> >
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2020-07-22 11:58 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-21  7:32 [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Christian König
2020-07-21  7:32 ` [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau Christian König
2020-07-21  8:50   ` daniel
2020-07-22  7:05   ` Chauhan, Madhav
2020-07-21  7:32 ` [PATCH 03/11] drm/ttm: remove io_reserve_fastpath flag Christian König
2020-07-21  8:52   ` daniel
2020-07-21  7:32 ` [PATCH 04/11] drm/ttm: cleanup coding style and implementation Christian König
2020-07-21  8:56   ` daniel
2020-07-21  7:32 ` [PATCH 05/11] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
2020-07-21  9:01   ` daniel
2020-07-21  7:32 ` [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
2020-07-21  9:24   ` daniel
2020-07-21 14:46     ` Christian König
2020-07-22  5:34       ` Daniel Vetter
2020-07-22 11:13         ` Christian König
2020-07-22 11:42           ` Daniel Vetter
2020-07-22 11:49             ` Christian König
2020-07-22 11:58               ` Daniel Vetter
2020-07-21  7:32 ` [PATCH 07/11] drm/vmwgfx: " Christian König
2020-07-21  9:26   ` daniel
2020-07-21  7:32 ` [PATCH 08/11] drm/amdgpu: " Christian König
2020-07-21  9:28   ` daniel
2020-07-21  9:45     ` Chauhan, Madhav
2020-07-21 14:30     ` Christian König
2020-07-21  7:32 ` [PATCH 09/11] drm/nouveau: " Christian König
2020-07-21  9:28   ` daniel
2020-07-21  7:32 ` [PATCH 10/11] drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE v2 Christian König
2020-07-21  9:29   ` daniel
2020-07-21  7:32 ` [PATCH 11/11] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
2020-07-21  9:30   ` daniel
2020-07-21  9:54   ` kernel test robot
2020-07-21  9:54     ` kernel test robot
2020-07-21  7:55 ` [PATCH 01/11] drm: remove optional dummy function from drivers using TTM Daniel Vetter
2020-07-21  8:02 ` Christian König
2020-07-21  9:15 ` Chauhan, Madhav

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.