All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/9] drm/ttm: initialize the system domain with defaults
@ 2020-07-23 15:16 Christian König
  2020-07-23 15:16 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Christian König @ 2020-07-23 15:16 UTC (permalink / raw)
  To: dri-devel

Instead of repeating that in each driver.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 3 ---
 drivers/gpu/drm/drm_gem_vram_helper.c      | 3 ---
 drivers/gpu/drm/nouveau/nouveau_bo.c       | 3 ---
 drivers/gpu/drm/qxl/qxl_ttm.c              | 3 ---
 drivers/gpu/drm/radeon/radeon_ttm.c        | 3 ---
 drivers/gpu/drm/ttm/ttm_bo.c               | 2 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
 7 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0dd5e802091d..e57c49a91b73 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -84,9 +84,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = 0;
-		man->available_caching = TTM_PL_MASK_CACHING;
-		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_TT:
 		/* GTT memory  */
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 3296ed3df358..be177afdeb9a 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1009,9 +1009,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 {
 	switch (type) {
 	case TTM_PL_SYSTEM:
-		man->flags = 0;
-		man->available_caching = TTM_PL_MASK_CACHING;
-		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_VRAM:
 		man->func = &ttm_bo_manager_func;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 4ccf937df0d0..53af25020bb2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -655,9 +655,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 
 	switch (type) {
 	case TTM_PL_SYSTEM:
-		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;
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 1d8e07b8b19e..e9b8c921c1f0 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -54,9 +54,6 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = 0;
-		man->available_caching = TTM_PL_MASK_CACHING;
-		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_VRAM:
 	case TTM_PL_PRIV:
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index b474781a0920..b4cb75361577 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -76,9 +76,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = 0;
-		man->available_caching = TTM_PL_MASK_CACHING;
-		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_TT:
 		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 7c02ce784805..1f1f9e463265 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1677,6 +1677,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	 * Initialize the system memory buffer type.
 	 * Other types need to be driver / IOCTL initialized.
 	 */
+	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
+	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
 	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
 	if (unlikely(ret != 0))
 		goto out_no_sys;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 1d78187eaba6..00cef1a3a178 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -742,8 +742,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->available_caching = TTM_PL_FLAG_CACHED;
-		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
-- 
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] 17+ messages in thread

* [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
  2020-07-23 15:16 [PATCH 1/9] drm/ttm: initialize the system domain with defaults Christian König
@ 2020-07-23 15:16 ` Christian König
  2020-07-27  9:48   ` daniel
  2020-07-23 15:16 ` [PATCH 3/9] drm/radeon: stop implementing init_mem_type Christian König
  2020-07-27  9:42 ` [PATCH 1/9] drm/ttm: initialize the system domain with defaults daniel
  2 siblings, 1 reply; 17+ messages in thread
From: Christian König @ 2020-07-23 15:16 UTC (permalink / raw)
  To: dri-devel

Instead use a boolean field in the memory manager structure.

Also invert the meaning of the field since the use of a TT
structure is the special case here.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +---
 drivers/gpu/drm/drm_gem_vram_helper.c      |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c       |  4 +---
 drivers/gpu/drm/qxl/qxl_ttm.c              |  1 -
 drivers/gpu/drm/radeon/radeon_ttm.c        |  3 +--
 drivers/gpu/drm/ttm/ttm_bo.c               | 14 +++++++-------
 drivers/gpu/drm/ttm/ttm_bo_util.c          | 12 ++++++------
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  3 ++-
 include/drm/ttm/ttm_bo_driver.h            |  4 +---
 9 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e57c49a91b73..406bcb03df48 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		break;
 	case TTM_PL_TT:
 		/* GTT memory  */
+		man->use_tt = true;
 		man->func = &amdgpu_gtt_mgr_func;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->flags = 0;
 		break;
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &amdgpu_vram_mgr_func;
-		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;
@@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case AMDGPU_PL_OA:
 		/* On-chip GDS memory*/
 		man->func = &ttm_bo_manager_func;
-		man->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/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index be177afdeb9a..801a14c6e9e0 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		break;
 	case TTM_PL_VRAM:
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED |
 					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 53af25020bb2..a3ad66ad3817 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_SYSTEM:
 		break;
 	case TTM_PL_VRAM:
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED |
 					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
@@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		else
 			man->func = &ttm_bo_manager_func;
 
+		man->use_tt = true;
 		if (drm->agp.bridge) {
-			man->flags = 0;
 			man->available_caching = TTM_PL_FLAG_UNCACHED |
 				TTM_PL_FLAG_WC;
 			man->default_caching = TTM_PL_FLAG_WC;
 		} else {
-			man->flags = 0;
 			man->available_caching = TTM_PL_MASK_CACHING;
 			man->default_caching = TTM_PL_FLAG_CACHED;
 		}
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index e9b8c921c1f0..abb9fa4d80cf 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -59,7 +59,6 @@ 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;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index b4cb75361577..9aba18a143e7 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -81,7 +81,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 = 0;
+		man->use_tt = true;
 #if IS_ENABLED(CONFIG_AGP)
 		if (rdev->flags & RADEON_IS_AGP) {
 			if (!rdev->ddev->agp) {
@@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
 		break;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 1f1f9e463265..6dea56dce350 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
 
 	drm_printf(p, "    has_type: %d\n", man->has_type);
 	drm_printf(p, "    use_type: %d\n", man->use_type);
-	drm_printf(p, "    flags: 0x%08X\n", man->flags);
+	drm_printf(p, "    use_tt: %d\n", man->use_tt);
 	drm_printf(p, "    size: %llu\n", man->size);
 	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
 	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
@@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
 	man = &bdev->man[mem->mem_type];
 	list_add_tail(&bo->lru, &man->lru[bo->priority]);
 
-	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
+	if (man->use_tt && bo->ttm &&
 	    !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
 				     TTM_PAGE_FLAG_SWAPPED))) {
 		list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
@@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 	 * Create and bind a ttm if required.
 	 */
 
-	if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
-		bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
+	if (new_man->use_tt) {
+		bool zero = old_man->use_tt;
 
 		ret = ttm_tt_create(bo, zero);
 		if (ret)
@@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 	if (bdev->driver->move_notify)
 		bdev->driver->move_notify(bo, evict, mem);
 
-	if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
-	    !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
+	if (old_man->use_tt && new_man->use_tt)
 		ret = ttm_bo_move_ttm(bo, ctx, mem);
 	else if (bdev->driver->move)
 		ret = bdev->driver->move(bo, evict, ctx, mem);
@@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 
 out_err:
 	new_man = &bdev->man[bo->mem.mem_type];
-	if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
+	if (!new_man->use_tt) {
 		ttm_tt_destroy(bo->ttm);
 		bo->ttm = NULL;
 	}
@@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	 * Initialize the system memory buffer type.
 	 * Other types need to be driver / IOCTL initialized.
 	 */
+	bdev->man[TTM_PL_SYSTEM].use_tt = true;
 	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
 	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
 	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 7fb3e0bcbab4..1f502be0b646 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
 	*old_mem = *new_mem;
 	new_mem->mm_node = NULL;
 
-	if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
+	if (!man->use_tt) {
 		ttm_tt_destroy(ttm);
 		bo->ttm = NULL;
 	}
@@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 		if (ret)
 			return ret;
 
-		if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
+		if (!man->use_tt) {
 			ttm_tt_destroy(bo->ttm);
 			bo->ttm = NULL;
 		}
@@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 		 * bo to be unbound and destroyed.
 		 */
 
-		if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
+		if (man->use_tt)
 			ghost_obj->ttm = NULL;
 		else
 			bo->ttm = NULL;
@@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		 * bo to be unbound and destroyed.
 		 */
 
-		if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
+		if (to->use_tt)
 			ghost_obj->ttm = NULL;
 		else
 			bo->ttm = NULL;
@@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		dma_resv_unlock(&ghost_obj->base._resv);
 		ttm_bo_put(ghost_obj);
 
-	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
+	} else if (!from->use_tt) {
 
 		/**
 		 * BO doesn't have a TTM we need to bind/unbind. Just remember
@@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		if (ret)
 			return ret;
 
-		if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
+		if (!to->use_tt) {
 			ttm_tt_destroy(bo->ttm);
 			bo->ttm = NULL;
 		}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 00cef1a3a178..5d8179d9f394 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &vmw_thp_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->func = &vmw_gmrid_manager_func;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
+		/* TODO: This is most likely not correct */
+		man->use_tt = true;
 		break;
 	default:
 		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 9b251853afe2..adac4cd0ba23 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -45,8 +45,6 @@
 
 #define TTM_MAX_BO_PRIORITY	4U
 
-#define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
-
 struct ttm_mem_type_manager;
 
 struct ttm_mem_type_manager_func {
@@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
 
 	bool has_type;
 	bool use_type;
-	uint32_t flags;
+	bool use_tt;
 	uint64_t size;
 	uint32_t available_caching;
 	uint32_t default_caching;
-- 
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] 17+ messages in thread

* [PATCH 3/9] drm/radeon: stop implementing init_mem_type
  2020-07-23 15:16 [PATCH 1/9] drm/ttm: initialize the system domain with defaults Christian König
  2020-07-23 15:16 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
@ 2020-07-23 15:16 ` Christian König
  2020-07-27 10:30   ` daniel
  2020-07-27  9:42 ` [PATCH 1/9] drm/ttm: initialize the system domain with defaults daniel
  2 siblings, 1 reply; 17+ messages in thread
From: Christian König @ 2020-07-23 15:16 UTC (permalink / raw)
  To: dri-devel

Instead just initialize the memory type parameters
before calling ttm_bo_init_mm.

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

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 9aba18a143e7..b0b59c553785 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -69,43 +69,43 @@ struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
 static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 				struct ttm_mem_type_manager *man)
 {
-	struct radeon_device *rdev;
+	return 0;
+}
 
-	rdev = radeon_get_rdev(bdev);
+static int radeon_ttm_init_vram(struct radeon_device *rdev)
+{
+	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_VRAM];
 
-	switch (type) {
-	case TTM_PL_SYSTEM:
-		/* System memory */
-		break;
-	case TTM_PL_TT:
-		man->func = &ttm_bo_manager_func;
-		man->available_caching = TTM_PL_MASK_CACHING;
-		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->use_tt = true;
+	man->func = &ttm_bo_manager_func;
+	man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
+	man->default_caching = TTM_PL_FLAG_WC;
+
+	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
+			      rdev->mc.real_vram_size >> PAGE_SHIFT);
+}
+
+static int radeon_ttm_init_gtt(struct radeon_device *rdev)
+{
+	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_TT];
+
+	man->func = &ttm_bo_manager_func;
+	man->available_caching = TTM_PL_MASK_CACHING;
+	man->default_caching = TTM_PL_FLAG_CACHED;
+	man->use_tt = true;
 #if IS_ENABLED(CONFIG_AGP)
-		if (rdev->flags & RADEON_IS_AGP) {
-			if (!rdev->ddev->agp) {
-				DRM_ERROR("AGP is not enabled for memory type %u\n",
-					  (unsigned)type);
-				return -EINVAL;
-			}
-			man->available_caching = TTM_PL_FLAG_UNCACHED |
-						 TTM_PL_FLAG_WC;
-			man->default_caching = TTM_PL_FLAG_WC;
+	if (rdev->flags & RADEON_IS_AGP) {
+		if (!rdev->ddev->agp) {
+			DRM_ERROR("AGP is not enabled\n");
+			return -EINVAL;
 		}
-#endif
-		break;
-	case TTM_PL_VRAM:
-		/* "On-card" video ram */
-		man->func = &ttm_bo_manager_func;
-		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
+		man->available_caching = TTM_PL_FLAG_UNCACHED |
+					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
-		break;
-	default:
-		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
-		return -EINVAL;
 	}
-	return 0;
+#endif
+
+	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
+			      rdev->mc.gtt_size >> PAGE_SHIFT);
 }
 
 static void radeon_evict_flags(struct ttm_buffer_object *bo,
@@ -778,8 +778,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
 		return r;
 	}
 	rdev->mman.initialized = true;
-	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
-				rdev->mc.real_vram_size >> PAGE_SHIFT);
+
+	r = radeon_ttm_init_vram(rdev);
 	if (r) {
 		DRM_ERROR("Failed initializing VRAM heap.\n");
 		return r;
@@ -804,8 +804,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
 	}
 	DRM_INFO("radeon: %uM of VRAM memory ready\n",
 		 (unsigned) (rdev->mc.real_vram_size / (1024 * 1024)));
-	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
-				rdev->mc.gtt_size >> PAGE_SHIFT);
+
+	r = radeon_ttm_init_gtt(rdev);
 	if (r) {
 		DRM_ERROR("Failed initializing GTT heap.\n");
 		return r;
-- 
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] 17+ messages in thread

* Re: [PATCH 1/9] drm/ttm: initialize the system domain with defaults
  2020-07-23 15:16 [PATCH 1/9] drm/ttm: initialize the system domain with defaults Christian König
  2020-07-23 15:16 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
  2020-07-23 15:16 ` [PATCH 3/9] drm/radeon: stop implementing init_mem_type Christian König
@ 2020-07-27  9:42 ` daniel
  2020-07-27 10:39   ` Christian König
  2 siblings, 1 reply; 17+ messages in thread
From: daniel @ 2020-07-27  9:42 UTC (permalink / raw)
  Cc: dri-devel

On Thu, Jul 23, 2020 at 05:16:13PM +0200, Christian König wrote:
> Instead of repeating that in each driver.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 3 ---
>  drivers/gpu/drm/drm_gem_vram_helper.c      | 3 ---
>  drivers/gpu/drm/nouveau/nouveau_bo.c       | 3 ---
>  drivers/gpu/drm/qxl/qxl_ttm.c              | 3 ---
>  drivers/gpu/drm/radeon/radeon_ttm.c        | 3 ---
>  drivers/gpu/drm/ttm/ttm_bo.c               | 2 ++
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
>  7 files changed, 2 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 0dd5e802091d..e57c49a91b73 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -84,9 +84,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->flags = 0;
> -		man->available_caching = TTM_PL_MASK_CACHING;
> -		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_TT:
>  		/* GTT memory  */
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 3296ed3df358..be177afdeb9a 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1009,9 +1009,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  {
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
> -		man->flags = 0;
> -		man->available_caching = TTM_PL_MASK_CACHING;
> -		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_VRAM:
>  		man->func = &ttm_bo_manager_func;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 4ccf937df0d0..53af25020bb2 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -655,9 +655,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
> -		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;
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 1d8e07b8b19e..e9b8c921c1f0 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -54,9 +54,6 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->flags = 0;
> -		man->available_caching = TTM_PL_MASK_CACHING;
> -		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_VRAM:
>  	case TTM_PL_PRIV:
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index b474781a0920..b4cb75361577 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -76,9 +76,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->flags = 0;
> -		man->available_caching = TTM_PL_MASK_CACHING;
> -		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_TT:
>  		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 7c02ce784805..1f1f9e463265 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1677,6 +1677,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>  	 * Initialize the system memory buffer type.
>  	 * Other types need to be driver / IOCTL initialized.
>  	 */
> +	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
> +	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
>  	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
>  	if (unlikely(ret != 0))
>  		goto out_no_sys;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 1d78187eaba6..00cef1a3a178 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -742,8 +742,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
>  		/* System memory */
> -		man->available_caching = TTM_PL_FLAG_CACHED;

Above is CACHED, not CACHING, so needs to stay to overwrite the default.
With that fixed:

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

> -		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
> -- 
> 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] 17+ messages in thread

* Re: [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
  2020-07-23 15:16 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
@ 2020-07-27  9:48   ` daniel
  2020-07-27  9:54     ` Christian König
  0 siblings, 1 reply; 17+ messages in thread
From: daniel @ 2020-07-27  9:48 UTC (permalink / raw)
  Cc: dri-devel

On Thu, Jul 23, 2020 at 05:16:14PM +0200, Christian König wrote:
> Instead use a boolean field in the memory manager structure.
> 
> Also invert the meaning of the field since the use of a TT
> structure is the special case here.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +---
>  drivers/gpu/drm/drm_gem_vram_helper.c      |  1 -
>  drivers/gpu/drm/nouveau/nouveau_bo.c       |  4 +---
>  drivers/gpu/drm/qxl/qxl_ttm.c              |  1 -
>  drivers/gpu/drm/radeon/radeon_ttm.c        |  3 +--
>  drivers/gpu/drm/ttm/ttm_bo.c               | 14 +++++++-------
>  drivers/gpu/drm/ttm/ttm_bo_util.c          | 12 ++++++------
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  3 ++-
>  include/drm/ttm/ttm_bo_driver.h            |  4 +---
>  9 files changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e57c49a91b73..406bcb03df48 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		break;
>  	case TTM_PL_TT:
>  		/* GTT memory  */
> +		man->use_tt = true;
>  		man->func = &amdgpu_gtt_mgr_func;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = 0;
>  		break;
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &amdgpu_vram_mgr_func;
> -		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;
> @@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case AMDGPU_PL_OA:
>  		/* On-chip GDS memory*/
>  		man->func = &ttm_bo_manager_func;
> -		man->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/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index be177afdeb9a..801a14c6e9e0 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		break;
>  	case TTM_PL_VRAM:
>  		man->func = &ttm_bo_manager_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED |
>  					 TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 53af25020bb2..a3ad66ad3817 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case TTM_PL_SYSTEM:
>  		break;
>  	case TTM_PL_VRAM:
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED |
>  					 TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
> @@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		else
>  			man->func = &ttm_bo_manager_func;
>  
> +		man->use_tt = true;
>  		if (drm->agp.bridge) {
> -			man->flags = 0;
>  			man->available_caching = TTM_PL_FLAG_UNCACHED |
>  				TTM_PL_FLAG_WC;
>  			man->default_caching = TTM_PL_FLAG_WC;
>  		} else {
> -			man->flags = 0;
>  			man->available_caching = TTM_PL_MASK_CACHING;
>  			man->default_caching = TTM_PL_FLAG_CACHED;
>  		}
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index e9b8c921c1f0..abb9fa4d80cf 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -59,7 +59,6 @@ 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;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index b4cb75361577..9aba18a143e7 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -81,7 +81,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 = 0;
> +		man->use_tt = true;
>  #if IS_ENABLED(CONFIG_AGP)
>  		if (rdev->flags & RADEON_IS_AGP) {
>  			if (!rdev->ddev->agp) {
> @@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &ttm_bo_manager_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
>  		break;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 1f1f9e463265..6dea56dce350 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
>  
>  	drm_printf(p, "    has_type: %d\n", man->has_type);
>  	drm_printf(p, "    use_type: %d\n", man->use_type);
> -	drm_printf(p, "    flags: 0x%08X\n", man->flags);
> +	drm_printf(p, "    use_tt: %d\n", man->use_tt);
>  	drm_printf(p, "    size: %llu\n", man->size);
>  	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
>  	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
> @@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
>  	man = &bdev->man[mem->mem_type];
>  	list_add_tail(&bo->lru, &man->lru[bo->priority]);
>  
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
> +	if (man->use_tt && bo->ttm &&
>  	    !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
>  				     TTM_PAGE_FLAG_SWAPPED))) {
>  		list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
> @@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  	 * Create and bind a ttm if required.
>  	 */
>  
> -	if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
> -		bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
> +	if (new_man->use_tt) {
> +		bool zero = old_man->use_tt;
>  
>  		ret = ttm_tt_create(bo, zero);
>  		if (ret)
> @@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  	if (bdev->driver->move_notify)
>  		bdev->driver->move_notify(bo, evict, mem);
>  
> -	if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
> -	    !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
> +	if (old_man->use_tt && new_man->use_tt)
>  		ret = ttm_bo_move_ttm(bo, ctx, mem);
>  	else if (bdev->driver->move)
>  		ret = bdev->driver->move(bo, evict, ctx, mem);
> @@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  
>  out_err:
>  	new_man = &bdev->man[bo->mem.mem_type];
> -	if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +	if (!new_man->use_tt) {
>  		ttm_tt_destroy(bo->ttm);
>  		bo->ttm = NULL;
>  	}
> @@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>  	 * Initialize the system memory buffer type.
>  	 * Other types need to be driver / IOCTL initialized.
>  	 */
> +	bdev->man[TTM_PL_SYSTEM].use_tt = true;
>  	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
>  	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
>  	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 7fb3e0bcbab4..1f502be0b646 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>  	*old_mem = *new_mem;
>  	new_mem->mm_node = NULL;
>  
> -	if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +	if (!man->use_tt) {
>  		ttm_tt_destroy(ttm);
>  		bo->ttm = NULL;
>  	}
> @@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>  		if (ret)
>  			return ret;
>  
> -		if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +		if (!man->use_tt) {
>  			ttm_tt_destroy(bo->ttm);
>  			bo->ttm = NULL;
>  		}
> @@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>  		 * bo to be unbound and destroyed.
>  		 */
>  
> -		if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
> +		if (man->use_tt)
>  			ghost_obj->ttm = NULL;
>  		else
>  			bo->ttm = NULL;
> @@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>  		 * bo to be unbound and destroyed.
>  		 */
>  
> -		if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
> +		if (to->use_tt)
>  			ghost_obj->ttm = NULL;
>  		else
>  			bo->ttm = NULL;
> @@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>  		dma_resv_unlock(&ghost_obj->base._resv);
>  		ttm_bo_put(ghost_obj);
>  
> -	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +	} else if (!from->use_tt) {
>  
>  		/**
>  		 * BO doesn't have a TTM we need to bind/unbind. Just remember
> @@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>  		if (ret)
>  			return ret;
>  
> -		if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +		if (!to->use_tt) {
>  			ttm_tt_destroy(bo->ttm);
>  			bo->ttm = NULL;
>  		}
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 00cef1a3a178..5d8179d9f394 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &vmw_thp_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		man->func = &vmw_gmrid_manager_func;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> +		/* TODO: This is most likely not correct */

Comment suggests it's a remapping thing, and I've seen some idr allocator
thing in vmwgfx before, i.e. it allocates remapping ids for bo, instead of
remapping space. So I think this is all ok, and no need for the TODO here.

With that:

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

> +		man->use_tt = true;
>  		break;
>  	default:
>  		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 9b251853afe2..adac4cd0ba23 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -45,8 +45,6 @@
>  
>  #define TTM_MAX_BO_PRIORITY	4U
>  
> -#define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
> -
>  struct ttm_mem_type_manager;
>  
>  struct ttm_mem_type_manager_func {
> @@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
>  
>  	bool has_type;
>  	bool use_type;
> -	uint32_t flags;
> +	bool use_tt;
>  	uint64_t size;
>  	uint32_t available_caching;
>  	uint32_t default_caching;
> -- 
> 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] 17+ messages in thread

* Re: [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
  2020-07-27  9:48   ` daniel
@ 2020-07-27  9:54     ` Christian König
  2020-07-27 10:27       ` daniel
  0 siblings, 1 reply; 17+ messages in thread
From: Christian König @ 2020-07-27  9:54 UTC (permalink / raw)
  To: daniel; +Cc: dri-devel

Am 27.07.20 um 11:48 schrieb daniel@ffwll.ch:
> On Thu, Jul 23, 2020 at 05:16:14PM +0200, Christian König wrote:
>> Instead use a boolean field in the memory manager structure.
>>
>> Also invert the meaning of the field since the use of a TT
>> structure is the special case here.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +---
>>   drivers/gpu/drm/drm_gem_vram_helper.c      |  1 -
>>   drivers/gpu/drm/nouveau/nouveau_bo.c       |  4 +---
>>   drivers/gpu/drm/qxl/qxl_ttm.c              |  1 -
>>   drivers/gpu/drm/radeon/radeon_ttm.c        |  3 +--
>>   drivers/gpu/drm/ttm/ttm_bo.c               | 14 +++++++-------
>>   drivers/gpu/drm/ttm/ttm_bo_util.c          | 12 ++++++------
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  3 ++-
>>   include/drm/ttm/ttm_bo_driver.h            |  4 +---
>>   9 files changed, 19 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index e57c49a91b73..406bcb03df48 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		break;
>>   	case TTM_PL_TT:
>>   		/* GTT memory  */
>> +		man->use_tt = true;
>>   		man->func = &amdgpu_gtt_mgr_func;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>> -		man->flags = 0;
>>   		break;
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>>   		man->func = &amdgpu_vram_mgr_func;
>> -		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;
>> @@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case AMDGPU_PL_OA:
>>   		/* On-chip GDS memory*/
>>   		man->func = &ttm_bo_manager_func;
>> -		man->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/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index be177afdeb9a..801a14c6e9e0 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		break;
>>   	case TTM_PL_VRAM:
>>   		man->func = &ttm_bo_manager_func;
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED |
>>   					 TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> index 53af25020bb2..a3ad66ad3817 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> @@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case TTM_PL_SYSTEM:
>>   		break;
>>   	case TTM_PL_VRAM:
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED |
>>   					 TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>> @@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		else
>>   			man->func = &ttm_bo_manager_func;
>>   
>> +		man->use_tt = true;
>>   		if (drm->agp.bridge) {
>> -			man->flags = 0;
>>   			man->available_caching = TTM_PL_FLAG_UNCACHED |
>>   				TTM_PL_FLAG_WC;
>>   			man->default_caching = TTM_PL_FLAG_WC;
>>   		} else {
>> -			man->flags = 0;
>>   			man->available_caching = TTM_PL_MASK_CACHING;
>>   			man->default_caching = TTM_PL_FLAG_CACHED;
>>   		}
>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
>> index e9b8c921c1f0..abb9fa4d80cf 100644
>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
>> @@ -59,7 +59,6 @@ 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;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>> index b4cb75361577..9aba18a143e7 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>> @@ -81,7 +81,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 = 0;
>> +		man->use_tt = true;
>>   #if IS_ENABLED(CONFIG_AGP)
>>   		if (rdev->flags & RADEON_IS_AGP) {
>>   			if (!rdev->ddev->agp) {
>> @@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>>   		man->func = &ttm_bo_manager_func;
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>>   		break;
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 1f1f9e463265..6dea56dce350 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
>>   
>>   	drm_printf(p, "    has_type: %d\n", man->has_type);
>>   	drm_printf(p, "    use_type: %d\n", man->use_type);
>> -	drm_printf(p, "    flags: 0x%08X\n", man->flags);
>> +	drm_printf(p, "    use_tt: %d\n", man->use_tt);
>>   	drm_printf(p, "    size: %llu\n", man->size);
>>   	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
>>   	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
>> @@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
>>   	man = &bdev->man[mem->mem_type];
>>   	list_add_tail(&bo->lru, &man->lru[bo->priority]);
>>   
>> -	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
>> +	if (man->use_tt && bo->ttm &&
>>   	    !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
>>   				     TTM_PAGE_FLAG_SWAPPED))) {
>>   		list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
>> @@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>   	 * Create and bind a ttm if required.
>>   	 */
>>   
>> -	if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
>> -		bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
>> +	if (new_man->use_tt) {
>> +		bool zero = old_man->use_tt;
>>   
>>   		ret = ttm_tt_create(bo, zero);
>>   		if (ret)
>> @@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>   	if (bdev->driver->move_notify)
>>   		bdev->driver->move_notify(bo, evict, mem);
>>   
>> -	if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
>> -	    !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
>> +	if (old_man->use_tt && new_man->use_tt)
>>   		ret = ttm_bo_move_ttm(bo, ctx, mem);
>>   	else if (bdev->driver->move)
>>   		ret = bdev->driver->move(bo, evict, ctx, mem);
>> @@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>   
>>   out_err:
>>   	new_man = &bdev->man[bo->mem.mem_type];
>> -	if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +	if (!new_man->use_tt) {
>>   		ttm_tt_destroy(bo->ttm);
>>   		bo->ttm = NULL;
>>   	}
>> @@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>>   	 * Initialize the system memory buffer type.
>>   	 * Other types need to be driver / IOCTL initialized.
>>   	 */
>> +	bdev->man[TTM_PL_SYSTEM].use_tt = true;
>>   	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
>>   	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
>>   	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index 7fb3e0bcbab4..1f502be0b646 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>>   	*old_mem = *new_mem;
>>   	new_mem->mm_node = NULL;
>>   
>> -	if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +	if (!man->use_tt) {
>>   		ttm_tt_destroy(ttm);
>>   		bo->ttm = NULL;
>>   	}
>> @@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>>   		if (ret)
>>   			return ret;
>>   
>> -		if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +		if (!man->use_tt) {
>>   			ttm_tt_destroy(bo->ttm);
>>   			bo->ttm = NULL;
>>   		}
>> @@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>>   		 * bo to be unbound and destroyed.
>>   		 */
>>   
>> -		if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
>> +		if (man->use_tt)
>>   			ghost_obj->ttm = NULL;
>>   		else
>>   			bo->ttm = NULL;
>> @@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>>   		 * bo to be unbound and destroyed.
>>   		 */
>>   
>> -		if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
>> +		if (to->use_tt)
>>   			ghost_obj->ttm = NULL;
>>   		else
>>   			bo->ttm = NULL;
>> @@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>>   		dma_resv_unlock(&ghost_obj->base._resv);
>>   		ttm_bo_put(ghost_obj);
>>   
>> -	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +	} else if (!from->use_tt) {
>>   
>>   		/**
>>   		 * BO doesn't have a TTM we need to bind/unbind. Just remember
>> @@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>>   		if (ret)
>>   			return ret;
>>   
>> -		if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +		if (!to->use_tt) {
>>   			ttm_tt_destroy(bo->ttm);
>>   			bo->ttm = NULL;
>>   		}
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> index 00cef1a3a178..5d8179d9f394 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> @@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>>   		man->func = &vmw_thp_func;
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_CACHED;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>> @@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		man->func = &vmw_gmrid_manager_func;
>>   		man->available_caching = TTM_PL_FLAG_CACHED;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>> +		/* TODO: This is most likely not correct */
> Comment suggests it's a remapping thing, and I've seen some idr allocator
> thing in vmwgfx before, i.e. it allocates remapping ids for bo, instead of
> remapping space. So I think this is all ok, and no need for the TODO here.

Yeah and exactly because of this I think that allocating a TT structure 
doesn't make much sense.

Why should I need an pages array and backing page if I just want to 
allocate a number from an idr?

My best guess is that we don't leak memory and because of this nobody 
has ever noticed this.

Christian.

>
> With that:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>> +		man->use_tt = true;
>>   		break;
>>   	default:
>>   		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
>> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
>> index 9b251853afe2..adac4cd0ba23 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -45,8 +45,6 @@
>>   
>>   #define TTM_MAX_BO_PRIORITY	4U
>>   
>> -#define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
>> -
>>   struct ttm_mem_type_manager;
>>   
>>   struct ttm_mem_type_manager_func {
>> @@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
>>   
>>   	bool has_type;
>>   	bool use_type;
>> -	uint32_t flags;
>> +	bool use_tt;
>>   	uint64_t size;
>>   	uint32_t available_caching;
>>   	uint32_t default_caching;
>> -- 
>> 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] 17+ messages in thread

* Re: [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
  2020-07-27  9:54     ` Christian König
@ 2020-07-27 10:27       ` daniel
  0 siblings, 0 replies; 17+ messages in thread
From: daniel @ 2020-07-27 10:27 UTC (permalink / raw)
  Cc: dri-devel

On Mon, Jul 27, 2020 at 11:54:41AM +0200, Christian König wrote:
> Am 27.07.20 um 11:48 schrieb daniel@ffwll.ch:
> > On Thu, Jul 23, 2020 at 05:16:14PM +0200, Christian König wrote:
> > > Instead use a boolean field in the memory manager structure.
> > > 
> > > Also invert the meaning of the field since the use of a TT
> > > structure is the special case here.
> > > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +---
> > >   drivers/gpu/drm/drm_gem_vram_helper.c      |  1 -
> > >   drivers/gpu/drm/nouveau/nouveau_bo.c       |  4 +---
> > >   drivers/gpu/drm/qxl/qxl_ttm.c              |  1 -
> > >   drivers/gpu/drm/radeon/radeon_ttm.c        |  3 +--
> > >   drivers/gpu/drm/ttm/ttm_bo.c               | 14 +++++++-------
> > >   drivers/gpu/drm/ttm/ttm_bo_util.c          | 12 ++++++------
> > >   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  3 ++-
> > >   include/drm/ttm/ttm_bo_driver.h            |  4 +---
> > >   9 files changed, 19 insertions(+), 27 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > index e57c49a91b73..406bcb03df48 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > @@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   		break;
> > >   	case TTM_PL_TT:
> > >   		/* GTT memory  */
> > > +		man->use_tt = true;
> > >   		man->func = &amdgpu_gtt_mgr_func;
> > >   		man->available_caching = TTM_PL_MASK_CACHING;
> > >   		man->default_caching = TTM_PL_FLAG_CACHED;
> > > -		man->flags = 0;
> > >   		break;
> > >   	case TTM_PL_VRAM:
> > >   		/* "On-card" video ram */
> > >   		man->func = &amdgpu_vram_mgr_func;
> > > -		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;
> > > @@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   	case AMDGPU_PL_OA:
> > >   		/* On-chip GDS memory*/
> > >   		man->func = &ttm_bo_manager_func;
> > > -		man->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/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > index be177afdeb9a..801a14c6e9e0 100644
> > > --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> > > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > @@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   		break;
> > >   	case TTM_PL_VRAM:
> > >   		man->func = &ttm_bo_manager_func;
> > > -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
> > >   		man->available_caching = TTM_PL_FLAG_UNCACHED |
> > >   					 TTM_PL_FLAG_WC;
> > >   		man->default_caching = TTM_PL_FLAG_WC;
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > index 53af25020bb2..a3ad66ad3817 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > @@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   	case TTM_PL_SYSTEM:
> > >   		break;
> > >   	case TTM_PL_VRAM:
> > > -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
> > >   		man->available_caching = TTM_PL_FLAG_UNCACHED |
> > >   					 TTM_PL_FLAG_WC;
> > >   		man->default_caching = TTM_PL_FLAG_WC;
> > > @@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   		else
> > >   			man->func = &ttm_bo_manager_func;
> > > +		man->use_tt = true;
> > >   		if (drm->agp.bridge) {
> > > -			man->flags = 0;
> > >   			man->available_caching = TTM_PL_FLAG_UNCACHED |
> > >   				TTM_PL_FLAG_WC;
> > >   			man->default_caching = TTM_PL_FLAG_WC;
> > >   		} else {
> > > -			man->flags = 0;
> > >   			man->available_caching = TTM_PL_MASK_CACHING;
> > >   			man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		}
> > > diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > index e9b8c921c1f0..abb9fa4d80cf 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > @@ -59,7 +59,6 @@ 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;
> > >   		man->available_caching = TTM_PL_MASK_CACHING;
> > >   		man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		break;
> > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > index b4cb75361577..9aba18a143e7 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > @@ -81,7 +81,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 = 0;
> > > +		man->use_tt = true;
> > >   #if IS_ENABLED(CONFIG_AGP)
> > >   		if (rdev->flags & RADEON_IS_AGP) {
> > >   			if (!rdev->ddev->agp) {
> > > @@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   	case TTM_PL_VRAM:
> > >   		/* "On-card" video ram */
> > >   		man->func = &ttm_bo_manager_func;
> > > -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
> > >   		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> > >   		man->default_caching = TTM_PL_FLAG_WC;
> > >   		break;
> > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> > > index 1f1f9e463265..6dea56dce350 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > > @@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
> > >   	drm_printf(p, "    has_type: %d\n", man->has_type);
> > >   	drm_printf(p, "    use_type: %d\n", man->use_type);
> > > -	drm_printf(p, "    flags: 0x%08X\n", man->flags);
> > > +	drm_printf(p, "    use_tt: %d\n", man->use_tt);
> > >   	drm_printf(p, "    size: %llu\n", man->size);
> > >   	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
> > >   	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
> > > @@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
> > >   	man = &bdev->man[mem->mem_type];
> > >   	list_add_tail(&bo->lru, &man->lru[bo->priority]);
> > > -	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
> > > +	if (man->use_tt && bo->ttm &&
> > >   	    !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
> > >   				     TTM_PAGE_FLAG_SWAPPED))) {
> > >   		list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
> > > @@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> > >   	 * Create and bind a ttm if required.
> > >   	 */
> > > -	if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
> > > -		bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
> > > +	if (new_man->use_tt) {
> > > +		bool zero = old_man->use_tt;
> > >   		ret = ttm_tt_create(bo, zero);
> > >   		if (ret)
> > > @@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> > >   	if (bdev->driver->move_notify)
> > >   		bdev->driver->move_notify(bo, evict, mem);
> > > -	if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
> > > -	    !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
> > > +	if (old_man->use_tt && new_man->use_tt)
> > >   		ret = ttm_bo_move_ttm(bo, ctx, mem);
> > >   	else if (bdev->driver->move)
> > >   		ret = bdev->driver->move(bo, evict, ctx, mem);
> > > @@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> > >   out_err:
> > >   	new_man = &bdev->man[bo->mem.mem_type];
> > > -	if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> > > +	if (!new_man->use_tt) {
> > >   		ttm_tt_destroy(bo->ttm);
> > >   		bo->ttm = NULL;
> > >   	}
> > > @@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
> > >   	 * Initialize the system memory buffer type.
> > >   	 * Other types need to be driver / IOCTL initialized.
> > >   	 */
> > > +	bdev->man[TTM_PL_SYSTEM].use_tt = true;
> > >   	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
> > >   	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
> > >   	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
> > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > index 7fb3e0bcbab4..1f502be0b646 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > > @@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
> > >   	*old_mem = *new_mem;
> > >   	new_mem->mm_node = NULL;
> > > -	if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> > > +	if (!man->use_tt) {
> > >   		ttm_tt_destroy(ttm);
> > >   		bo->ttm = NULL;
> > >   	}
> > > @@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
> > >   		if (ret)
> > >   			return ret;
> > > -		if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> > > +		if (!man->use_tt) {
> > >   			ttm_tt_destroy(bo->ttm);
> > >   			bo->ttm = NULL;
> > >   		}
> > > @@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
> > >   		 * bo to be unbound and destroyed.
> > >   		 */
> > > -		if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
> > > +		if (man->use_tt)
> > >   			ghost_obj->ttm = NULL;
> > >   		else
> > >   			bo->ttm = NULL;
> > > @@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> > >   		 * bo to be unbound and destroyed.
> > >   		 */
> > > -		if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
> > > +		if (to->use_tt)
> > >   			ghost_obj->ttm = NULL;
> > >   		else
> > >   			bo->ttm = NULL;
> > > @@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> > >   		dma_resv_unlock(&ghost_obj->base._resv);
> > >   		ttm_bo_put(ghost_obj);
> > > -	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
> > > +	} else if (!from->use_tt) {
> > >   		/**
> > >   		 * BO doesn't have a TTM we need to bind/unbind. Just remember
> > > @@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
> > >   		if (ret)
> > >   			return ret;
> > > -		if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
> > > +		if (!to->use_tt) {
> > >   			ttm_tt_destroy(bo->ttm);
> > >   			bo->ttm = NULL;
> > >   		}
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > index 00cef1a3a178..5d8179d9f394 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > @@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   	case TTM_PL_VRAM:
> > >   		/* "On-card" video ram */
> > >   		man->func = &vmw_thp_func;
> > > -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
> > >   		man->available_caching = TTM_PL_FLAG_CACHED;
> > >   		man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		break;
> > > @@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > >   		man->func = &vmw_gmrid_manager_func;
> > >   		man->available_caching = TTM_PL_FLAG_CACHED;
> > >   		man->default_caching = TTM_PL_FLAG_CACHED;
> > > +		/* TODO: This is most likely not correct */
> > Comment suggests it's a remapping thing, and I've seen some idr allocator
> > thing in vmwgfx before, i.e. it allocates remapping ids for bo, instead of
> > remapping space. So I think this is all ok, and no need for the TODO here.
> 
> Yeah and exactly because of this I think that allocating a TT structure
> doesn't make much sense.
> 
> Why should I need an pages array and backing page if I just want to allocate
> a number from an idr?
> 
> My best guess is that we don't leak memory and because of this nobody has
> ever noticed this.

Hm yeah, I guess that's a question for vmwgfx folks to answer then. Feel
free to leave the todo in there.
-Daniel

> 
> Christian.
> 
> > 
> > With that:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > > +		man->use_tt = true;
> > >   		break;
> > >   	default:
> > >   		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
> > > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> > > index 9b251853afe2..adac4cd0ba23 100644
> > > --- a/include/drm/ttm/ttm_bo_driver.h
> > > +++ b/include/drm/ttm/ttm_bo_driver.h
> > > @@ -45,8 +45,6 @@
> > >   #define TTM_MAX_BO_PRIORITY	4U
> > > -#define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
> > > -
> > >   struct ttm_mem_type_manager;
> > >   struct ttm_mem_type_manager_func {
> > > @@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
> > >   	bool has_type;
> > >   	bool use_type;
> > > -	uint32_t flags;
> > > +	bool use_tt;
> > >   	uint64_t size;
> > >   	uint32_t available_caching;
> > >   	uint32_t default_caching;
> > > -- 
> > > 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] 17+ messages in thread

* Re: [PATCH 3/9] drm/radeon: stop implementing init_mem_type
  2020-07-23 15:16 ` [PATCH 3/9] drm/radeon: stop implementing init_mem_type Christian König
@ 2020-07-27 10:30   ` daniel
  2020-07-27 10:30     ` daniel
  2020-07-27 10:54     ` Christian König
  0 siblings, 2 replies; 17+ messages in thread
From: daniel @ 2020-07-27 10:30 UTC (permalink / raw)
  Cc: dri-devel

On Thu, Jul 23, 2020 at 05:16:15PM +0200, Christian König wrote:
> Instead just initialize the memory type parameters
> before calling ttm_bo_init_mm.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Hm what's the motivation here? I do agree that the init_mem_type callback
is rather midlayer-y (having a per-type cast in a callback is a very clear
signal something with the layering is all busted). So removing this sounds
like a good idea, but not really following why just for radeon? Or simply
wip?
-Daniel



> ---
>  drivers/gpu/drm/radeon/radeon_ttm.c | 70 ++++++++++++++---------------
>  1 file changed, 35 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 9aba18a143e7..b0b59c553785 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -69,43 +69,43 @@ struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
>  static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  				struct ttm_mem_type_manager *man)
>  {
> -	struct radeon_device *rdev;
> +	return 0;
> +}
>  
> -	rdev = radeon_get_rdev(bdev);
> +static int radeon_ttm_init_vram(struct radeon_device *rdev)
> +{
> +	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_VRAM];
>  
> -	switch (type) {
> -	case TTM_PL_SYSTEM:
> -		/* System memory */
> -		break;
> -	case TTM_PL_TT:
> -		man->func = &ttm_bo_manager_func;
> -		man->available_caching = TTM_PL_MASK_CACHING;
> -		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->use_tt = true;
> +	man->func = &ttm_bo_manager_func;
> +	man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> +	man->default_caching = TTM_PL_FLAG_WC;
> +
> +	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
> +			      rdev->mc.real_vram_size >> PAGE_SHIFT);
> +}
> +
> +static int radeon_ttm_init_gtt(struct radeon_device *rdev)
> +{
> +	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_TT];
> +
> +	man->func = &ttm_bo_manager_func;
> +	man->available_caching = TTM_PL_MASK_CACHING;
> +	man->default_caching = TTM_PL_FLAG_CACHED;
> +	man->use_tt = true;
>  #if IS_ENABLED(CONFIG_AGP)
> -		if (rdev->flags & RADEON_IS_AGP) {
> -			if (!rdev->ddev->agp) {
> -				DRM_ERROR("AGP is not enabled for memory type %u\n",
> -					  (unsigned)type);
> -				return -EINVAL;
> -			}
> -			man->available_caching = TTM_PL_FLAG_UNCACHED |
> -						 TTM_PL_FLAG_WC;
> -			man->default_caching = TTM_PL_FLAG_WC;
> +	if (rdev->flags & RADEON_IS_AGP) {
> +		if (!rdev->ddev->agp) {
> +			DRM_ERROR("AGP is not enabled\n");
> +			return -EINVAL;
>  		}
> -#endif
> -		break;
> -	case TTM_PL_VRAM:
> -		/* "On-card" video ram */
> -		man->func = &ttm_bo_manager_func;
> -		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> +		man->available_caching = TTM_PL_FLAG_UNCACHED |
> +					 TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
> -		break;
> -	default:
> -		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
> -		return -EINVAL;
>  	}
> -	return 0;
> +#endif
> +
> +	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
> +			      rdev->mc.gtt_size >> PAGE_SHIFT);
>  }
>  
>  static void radeon_evict_flags(struct ttm_buffer_object *bo,
> @@ -778,8 +778,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
>  		return r;
>  	}
>  	rdev->mman.initialized = true;
> -	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
> -				rdev->mc.real_vram_size >> PAGE_SHIFT);
> +
> +	r = radeon_ttm_init_vram(rdev);
>  	if (r) {
>  		DRM_ERROR("Failed initializing VRAM heap.\n");
>  		return r;
> @@ -804,8 +804,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
>  	}
>  	DRM_INFO("radeon: %uM of VRAM memory ready\n",
>  		 (unsigned) (rdev->mc.real_vram_size / (1024 * 1024)));
> -	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
> -				rdev->mc.gtt_size >> PAGE_SHIFT);
> +
> +	r = radeon_ttm_init_gtt(rdev);
>  	if (r) {
>  		DRM_ERROR("Failed initializing GTT heap.\n");
>  		return r;
> -- 
> 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] 17+ messages in thread

* Re: [PATCH 3/9] drm/radeon: stop implementing init_mem_type
  2020-07-27 10:30   ` daniel
@ 2020-07-27 10:30     ` daniel
  2020-07-27 10:54     ` Christian König
  1 sibling, 0 replies; 17+ messages in thread
From: daniel @ 2020-07-27 10:30 UTC (permalink / raw)
  To: dri-devel

On Mon, Jul 27, 2020 at 12:30:00PM +0200, daniel@ffwll.ch wrote:
> On Thu, Jul 23, 2020 at 05:16:15PM +0200, Christian König wrote:
> > Instead just initialize the memory type parameters
> > before calling ttm_bo_init_mm.
> > 
> > Signed-off-by: Christian König <christian.koenig@amd.com>
> 
> Hm what's the motivation here? I do agree that the init_mem_type callback
> is rather midlayer-y (having a per-type cast in a callback is a very clear
> signal something with the layering is all busted). So removing this sounds
> like a good idea, but not really following why just for radeon? Or simply
> wip?

nvm, I've seen the next series :-) And Alex already r-b'ed it.
-Daniel
> 
> 
> 
> > ---
> >  drivers/gpu/drm/radeon/radeon_ttm.c | 70 ++++++++++++++---------------
> >  1 file changed, 35 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> > index 9aba18a143e7..b0b59c553785 100644
> > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > @@ -69,43 +69,43 @@ struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
> >  static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> >  				struct ttm_mem_type_manager *man)
> >  {
> > -	struct radeon_device *rdev;
> > +	return 0;
> > +}
> >  
> > -	rdev = radeon_get_rdev(bdev);
> > +static int radeon_ttm_init_vram(struct radeon_device *rdev)
> > +{
> > +	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_VRAM];
> >  
> > -	switch (type) {
> > -	case TTM_PL_SYSTEM:
> > -		/* System memory */
> > -		break;
> > -	case TTM_PL_TT:
> > -		man->func = &ttm_bo_manager_func;
> > -		man->available_caching = TTM_PL_MASK_CACHING;
> > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > -		man->use_tt = true;
> > +	man->func = &ttm_bo_manager_func;
> > +	man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> > +	man->default_caching = TTM_PL_FLAG_WC;
> > +
> > +	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
> > +			      rdev->mc.real_vram_size >> PAGE_SHIFT);
> > +}
> > +
> > +static int radeon_ttm_init_gtt(struct radeon_device *rdev)
> > +{
> > +	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_TT];
> > +
> > +	man->func = &ttm_bo_manager_func;
> > +	man->available_caching = TTM_PL_MASK_CACHING;
> > +	man->default_caching = TTM_PL_FLAG_CACHED;
> > +	man->use_tt = true;
> >  #if IS_ENABLED(CONFIG_AGP)
> > -		if (rdev->flags & RADEON_IS_AGP) {
> > -			if (!rdev->ddev->agp) {
> > -				DRM_ERROR("AGP is not enabled for memory type %u\n",
> > -					  (unsigned)type);
> > -				return -EINVAL;
> > -			}
> > -			man->available_caching = TTM_PL_FLAG_UNCACHED |
> > -						 TTM_PL_FLAG_WC;
> > -			man->default_caching = TTM_PL_FLAG_WC;
> > +	if (rdev->flags & RADEON_IS_AGP) {
> > +		if (!rdev->ddev->agp) {
> > +			DRM_ERROR("AGP is not enabled\n");
> > +			return -EINVAL;
> >  		}
> > -#endif
> > -		break;
> > -	case TTM_PL_VRAM:
> > -		/* "On-card" video ram */
> > -		man->func = &ttm_bo_manager_func;
> > -		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
> > +		man->available_caching = TTM_PL_FLAG_UNCACHED |
> > +					 TTM_PL_FLAG_WC;
> >  		man->default_caching = TTM_PL_FLAG_WC;
> > -		break;
> > -	default:
> > -		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
> > -		return -EINVAL;
> >  	}
> > -	return 0;
> > +#endif
> > +
> > +	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
> > +			      rdev->mc.gtt_size >> PAGE_SHIFT);
> >  }
> >  
> >  static void radeon_evict_flags(struct ttm_buffer_object *bo,
> > @@ -778,8 +778,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
> >  		return r;
> >  	}
> >  	rdev->mman.initialized = true;
> > -	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
> > -				rdev->mc.real_vram_size >> PAGE_SHIFT);
> > +
> > +	r = radeon_ttm_init_vram(rdev);
> >  	if (r) {
> >  		DRM_ERROR("Failed initializing VRAM heap.\n");
> >  		return r;
> > @@ -804,8 +804,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
> >  	}
> >  	DRM_INFO("radeon: %uM of VRAM memory ready\n",
> >  		 (unsigned) (rdev->mc.real_vram_size / (1024 * 1024)));
> > -	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
> > -				rdev->mc.gtt_size >> PAGE_SHIFT);
> > +
> > +	r = radeon_ttm_init_gtt(rdev);
> >  	if (r) {
> >  		DRM_ERROR("Failed initializing GTT heap.\n");
> >  		return r;
> > -- 
> > 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

-- 
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] 17+ messages in thread

* Re: [PATCH 1/9] drm/ttm: initialize the system domain with defaults
  2020-07-27  9:42 ` [PATCH 1/9] drm/ttm: initialize the system domain with defaults daniel
@ 2020-07-27 10:39   ` Christian König
  2020-07-27 10:50     ` Hellstrom, Thomas
  0 siblings, 1 reply; 17+ messages in thread
From: Christian König @ 2020-07-27 10:39 UTC (permalink / raw)
  To: daniel, Thomas Hellström; +Cc: dri-devel

Am 27.07.20 um 11:42 schrieb daniel@ffwll.ch:
> On Thu, Jul 23, 2020 at 05:16:13PM +0200, Christian König wrote:
>> Instead of repeating that in each driver.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 3 ---
>>   drivers/gpu/drm/drm_gem_vram_helper.c      | 3 ---
>>   drivers/gpu/drm/nouveau/nouveau_bo.c       | 3 ---
>>   drivers/gpu/drm/qxl/qxl_ttm.c              | 3 ---
>>   drivers/gpu/drm/radeon/radeon_ttm.c        | 3 ---
>>   drivers/gpu/drm/ttm/ttm_bo.c               | 2 ++
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
>>   7 files changed, 2 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 0dd5e802091d..e57c49a91b73 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -84,9 +84,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* System memory */
>> -		man->flags = 0;
>> -		man->available_caching = TTM_PL_MASK_CACHING;
>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>>   	case TTM_PL_TT:
>>   		/* GTT memory  */
>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index 3296ed3df358..be177afdeb9a 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -1009,9 +1009,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   {
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>> -		man->flags = 0;
>> -		man->available_caching = TTM_PL_MASK_CACHING;
>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>>   	case TTM_PL_VRAM:
>>   		man->func = &ttm_bo_manager_func;
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> index 4ccf937df0d0..53af25020bb2 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> @@ -655,9 +655,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>> -		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;
>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
>> index 1d8e07b8b19e..e9b8c921c1f0 100644
>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
>> @@ -54,9 +54,6 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* System memory */
>> -		man->flags = 0;
>> -		man->available_caching = TTM_PL_MASK_CACHING;
>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>>   	case TTM_PL_VRAM:
>>   	case TTM_PL_PRIV:
>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>> index b474781a0920..b4cb75361577 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>> @@ -76,9 +76,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* System memory */
>> -		man->flags = 0;
>> -		man->available_caching = TTM_PL_MASK_CACHING;
>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>>   	case TTM_PL_TT:
>>   		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 7c02ce784805..1f1f9e463265 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -1677,6 +1677,8 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>>   	 * Initialize the system memory buffer type.
>>   	 * Other types need to be driver / IOCTL initialized.
>>   	 */
>> +	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
>> +	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
>>   	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
>>   	if (unlikely(ret != 0))
>>   		goto out_no_sys;
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> index 1d78187eaba6..00cef1a3a178 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> @@ -742,8 +742,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	switch (type) {
>>   	case TTM_PL_SYSTEM:
>>   		/* System memory */
>> -		man->available_caching = TTM_PL_FLAG_CACHED;
> Above is CACHED, not CACHING, so needs to stay to overwrite the default.

Crap I missed that. Problem is that I wanted to remove the possibility 
to init the system domain with different caching attributes.

I don't see how vmwgfx is every going to use this? Thomas do you have 
any idea what this was good for?

Thanks,
Christian.

> With that fixed:
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>> -- 
>> 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] 17+ messages in thread

* Re: [PATCH 1/9] drm/ttm: initialize the system domain with defaults
  2020-07-27 10:39   ` Christian König
@ 2020-07-27 10:50     ` Hellstrom, Thomas
  2020-07-27 12:23       ` Christian König
  0 siblings, 1 reply; 17+ messages in thread
From: Hellstrom, Thomas @ 2020-07-27 10:50 UTC (permalink / raw)
  To: daniel, christian.koenig; +Cc: dri-devel

On Mon, 2020-07-27 at 12:39 +0200, Christian König wrote:
> Am 27.07.20 um 11:42 schrieb daniel@ffwll.ch:
> > On Thu, Jul 23, 2020 at 05:16:13PM +0200, Christian König wrote:
> > > Instead of repeating that in each driver.
> > > 
> > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 3 ---
> > >   drivers/gpu/drm/drm_gem_vram_helper.c      | 3 ---
> > >   drivers/gpu/drm/nouveau/nouveau_bo.c       | 3 ---
> > >   drivers/gpu/drm/qxl/qxl_ttm.c              | 3 ---
> > >   drivers/gpu/drm/radeon/radeon_ttm.c        | 3 ---
> > >   drivers/gpu/drm/ttm/ttm_bo.c               | 2 ++
> > >   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
> > >   7 files changed, 2 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > index 0dd5e802091d..e57c49a91b73 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > @@ -84,9 +84,6 @@ static int amdgpu_init_mem_type(struct
> > > ttm_bo_device *bdev, uint32_t type,
> > >   	switch (type) {
> > >   	case TTM_PL_SYSTEM:
> > >   		/* System memory */
> > > -		man->flags = 0;
> > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		break;
> > >   	case TTM_PL_TT:
> > >   		/* GTT memory  */
> > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> > > b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > index 3296ed3df358..be177afdeb9a 100644
> > > --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> > > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > @@ -1009,9 +1009,6 @@ static int bo_driver_init_mem_type(struct
> > > ttm_bo_device *bdev, uint32_t type,
> > >   {
> > >   	switch (type) {
> > >   	case TTM_PL_SYSTEM:
> > > -		man->flags = 0;
> > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		break;
> > >   	case TTM_PL_VRAM:
> > >   		man->func = &ttm_bo_manager_func;
> > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > index 4ccf937df0d0..53af25020bb2 100644
> > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > @@ -655,9 +655,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device
> > > *bdev, uint32_t type,
> > >   
> > >   	switch (type) {
> > >   	case TTM_PL_SYSTEM:
> > > -		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;
> > > diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c
> > > b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > index 1d8e07b8b19e..e9b8c921c1f0 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > @@ -54,9 +54,6 @@ static int qxl_init_mem_type(struct
> > > ttm_bo_device *bdev, uint32_t type,
> > >   	switch (type) {
> > >   	case TTM_PL_SYSTEM:
> > >   		/* System memory */
> > > -		man->flags = 0;
> > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		break;
> > >   	case TTM_PL_VRAM:
> > >   	case TTM_PL_PRIV:
> > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
> > > b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > index b474781a0920..b4cb75361577 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > @@ -76,9 +76,6 @@ static int radeon_init_mem_type(struct
> > > ttm_bo_device *bdev, uint32_t type,
> > >   	switch (type) {
> > >   	case TTM_PL_SYSTEM:
> > >   		/* System memory */
> > > -		man->flags = 0;
> > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		break;
> > >   	case TTM_PL_TT:
> > >   		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 7c02ce784805..1f1f9e463265 100644
> > > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > > @@ -1677,6 +1677,8 @@ int ttm_bo_device_init(struct ttm_bo_device
> > > *bdev,
> > >   	 * Initialize the system memory buffer type.
> > >   	 * Other types need to be driver / IOCTL initialized.
> > >   	 */
> > > +	bdev->man[TTM_PL_SYSTEM].available_caching =
> > > TTM_PL_MASK_CACHING;
> > > +	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
> > >   	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
> > >   	if (unlikely(ret != 0))
> > >   		goto out_no_sys;
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > index 1d78187eaba6..00cef1a3a178 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > @@ -742,8 +742,6 @@ static int vmw_init_mem_type(struct
> > > ttm_bo_device *bdev, uint32_t type,
> > >   	switch (type) {
> > >   	case TTM_PL_SYSTEM:
> > >   		/* System memory */
> > > -		man->available_caching = TTM_PL_FLAG_CACHED;
> > Above is CACHED, not CACHING, so needs to stay to overwrite the
> > default.
> 
> Crap I missed that. Problem is that I wanted to remove the
> possibility 
> to init the system domain with different caching attributes.
> 
> I don't see how vmwgfx is every going to use this? Thomas do you
> have 
> any idea what this was good for?

I interpret that like vmwgfx can't handle any other caching attribute
than 'cached'. But I can't see anyone feeding vmwgfx system memory
buffers with other caching attributes. In that case, those would be
ignored.

/Thomas


> 
> Thanks,
> Christian.
> 
> > With that fixed:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > >   		break;
> > >   	case TTM_PL_VRAM:
> > >   		/* "On-card" video ram */
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
----------------------------------------------------------------------
Intel Sweden AB
Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 3/9] drm/radeon: stop implementing init_mem_type
  2020-07-27 10:30   ` daniel
  2020-07-27 10:30     ` daniel
@ 2020-07-27 10:54     ` Christian König
  1 sibling, 0 replies; 17+ messages in thread
From: Christian König @ 2020-07-27 10:54 UTC (permalink / raw)
  To: daniel; +Cc: dri-devel

Am 27.07.20 um 12:30 schrieb daniel@ffwll.ch:
> On Thu, Jul 23, 2020 at 05:16:15PM +0200, Christian König wrote:
>> Instead just initialize the memory type parameters
>> before calling ttm_bo_init_mm.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> Hm what's the motivation here? I do agree that the init_mem_type callback
> is rather midlayer-y (having a per-type cast in a callback is a very clear
> signal something with the layering is all busted). So removing this sounds
> like a good idea, but not really following why just for radeon? Or simply
> wip?

On the first try only the first 3 mails made it to the list and then my 
internet connection crashed.

I've send out the full set a minute later once more.

Christian.

> -Daniel
>
>
>
>> ---
>>   drivers/gpu/drm/radeon/radeon_ttm.c | 70 ++++++++++++++---------------
>>   1 file changed, 35 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>> index 9aba18a143e7..b0b59c553785 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>> @@ -69,43 +69,43 @@ struct radeon_device *radeon_get_rdev(struct ttm_bo_device *bdev)
>>   static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   				struct ttm_mem_type_manager *man)
>>   {
>> -	struct radeon_device *rdev;
>> +	return 0;
>> +}
>>   
>> -	rdev = radeon_get_rdev(bdev);
>> +static int radeon_ttm_init_vram(struct radeon_device *rdev)
>> +{
>> +	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_VRAM];
>>   
>> -	switch (type) {
>> -	case TTM_PL_SYSTEM:
>> -		/* System memory */
>> -		break;
>> -	case TTM_PL_TT:
>> -		man->func = &ttm_bo_manager_func;
>> -		man->available_caching = TTM_PL_MASK_CACHING;
>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>> -		man->use_tt = true;
>> +	man->func = &ttm_bo_manager_func;
>> +	man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>> +	man->default_caching = TTM_PL_FLAG_WC;
>> +
>> +	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
>> +			      rdev->mc.real_vram_size >> PAGE_SHIFT);
>> +}
>> +
>> +static int radeon_ttm_init_gtt(struct radeon_device *rdev)
>> +{
>> +	struct ttm_mem_type_manager *man = &rdev->mman.bdev.man[TTM_PL_TT];
>> +
>> +	man->func = &ttm_bo_manager_func;
>> +	man->available_caching = TTM_PL_MASK_CACHING;
>> +	man->default_caching = TTM_PL_FLAG_CACHED;
>> +	man->use_tt = true;
>>   #if IS_ENABLED(CONFIG_AGP)
>> -		if (rdev->flags & RADEON_IS_AGP) {
>> -			if (!rdev->ddev->agp) {
>> -				DRM_ERROR("AGP is not enabled for memory type %u\n",
>> -					  (unsigned)type);
>> -				return -EINVAL;
>> -			}
>> -			man->available_caching = TTM_PL_FLAG_UNCACHED |
>> -						 TTM_PL_FLAG_WC;
>> -			man->default_caching = TTM_PL_FLAG_WC;
>> +	if (rdev->flags & RADEON_IS_AGP) {
>> +		if (!rdev->ddev->agp) {
>> +			DRM_ERROR("AGP is not enabled\n");
>> +			return -EINVAL;
>>   		}
>> -#endif
>> -		break;
>> -	case TTM_PL_VRAM:
>> -		/* "On-card" video ram */
>> -		man->func = &ttm_bo_manager_func;
>> -		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>> +		man->available_caching = TTM_PL_FLAG_UNCACHED |
>> +					 TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>> -		break;
>> -	default:
>> -		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
>> -		return -EINVAL;
>>   	}
>> -	return 0;
>> +#endif
>> +
>> +	return ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
>> +			      rdev->mc.gtt_size >> PAGE_SHIFT);
>>   }
>>   
>>   static void radeon_evict_flags(struct ttm_buffer_object *bo,
>> @@ -778,8 +778,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
>>   		return r;
>>   	}
>>   	rdev->mman.initialized = true;
>> -	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_VRAM,
>> -				rdev->mc.real_vram_size >> PAGE_SHIFT);
>> +
>> +	r = radeon_ttm_init_vram(rdev);
>>   	if (r) {
>>   		DRM_ERROR("Failed initializing VRAM heap.\n");
>>   		return r;
>> @@ -804,8 +804,8 @@ int radeon_ttm_init(struct radeon_device *rdev)
>>   	}
>>   	DRM_INFO("radeon: %uM of VRAM memory ready\n",
>>   		 (unsigned) (rdev->mc.real_vram_size / (1024 * 1024)));
>> -	r = ttm_bo_init_mm(&rdev->mman.bdev, TTM_PL_TT,
>> -				rdev->mc.gtt_size >> PAGE_SHIFT);
>> +
>> +	r = radeon_ttm_init_gtt(rdev);
>>   	if (r) {
>>   		DRM_ERROR("Failed initializing GTT heap.\n");
>>   		return r;
>> -- 
>> 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] 17+ messages in thread

* Re: [PATCH 1/9] drm/ttm: initialize the system domain with defaults
  2020-07-27 10:50     ` Hellstrom, Thomas
@ 2020-07-27 12:23       ` Christian König
  2020-07-27 13:01         ` Hellstrom, Thomas
  0 siblings, 1 reply; 17+ messages in thread
From: Christian König @ 2020-07-27 12:23 UTC (permalink / raw)
  To: Hellstrom, Thomas, daniel; +Cc: dri-devel

Am 27.07.20 um 12:50 schrieb Hellstrom, Thomas:
> On Mon, 2020-07-27 at 12:39 +0200, Christian König wrote:
>> Am 27.07.20 um 11:42 schrieb daniel@ffwll.ch:
>>> On Thu, Jul 23, 2020 at 05:16:13PM +0200, Christian König wrote:
>>>> Instead of repeating that in each driver.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 3 ---
>>>>    drivers/gpu/drm/drm_gem_vram_helper.c      | 3 ---
>>>>    drivers/gpu/drm/nouveau/nouveau_bo.c       | 3 ---
>>>>    drivers/gpu/drm/qxl/qxl_ttm.c              | 3 ---
>>>>    drivers/gpu/drm/radeon/radeon_ttm.c        | 3 ---
>>>>    drivers/gpu/drm/ttm/ttm_bo.c               | 2 ++
>>>>    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
>>>>    7 files changed, 2 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> index 0dd5e802091d..e57c49a91b73 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>>> @@ -84,9 +84,6 @@ static int amdgpu_init_mem_type(struct
>>>> ttm_bo_device *bdev, uint32_t type,
>>>>    	switch (type) {
>>>>    	case TTM_PL_SYSTEM:
>>>>    		/* System memory */
>>>> -		man->flags = 0;
>>>> -		man->available_caching = TTM_PL_MASK_CACHING;
>>>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>>>    		break;
>>>>    	case TTM_PL_TT:
>>>>    		/* GTT memory  */
>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> index 3296ed3df358..be177afdeb9a 100644
>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>>> @@ -1009,9 +1009,6 @@ static int bo_driver_init_mem_type(struct
>>>> ttm_bo_device *bdev, uint32_t type,
>>>>    {
>>>>    	switch (type) {
>>>>    	case TTM_PL_SYSTEM:
>>>> -		man->flags = 0;
>>>> -		man->available_caching = TTM_PL_MASK_CACHING;
>>>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>>>    		break;
>>>>    	case TTM_PL_VRAM:
>>>>    		man->func = &ttm_bo_manager_func;
>>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
>>>> b/drivers/gpu/drm/nouveau/nouveau_bo.c
>>>> index 4ccf937df0d0..53af25020bb2 100644
>>>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>>>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>>>> @@ -655,9 +655,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device
>>>> *bdev, uint32_t type,
>>>>    
>>>>    	switch (type) {
>>>>    	case TTM_PL_SYSTEM:
>>>> -		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;
>>>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c
>>>> b/drivers/gpu/drm/qxl/qxl_ttm.c
>>>> index 1d8e07b8b19e..e9b8c921c1f0 100644
>>>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
>>>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
>>>> @@ -54,9 +54,6 @@ static int qxl_init_mem_type(struct
>>>> ttm_bo_device *bdev, uint32_t type,
>>>>    	switch (type) {
>>>>    	case TTM_PL_SYSTEM:
>>>>    		/* System memory */
>>>> -		man->flags = 0;
>>>> -		man->available_caching = TTM_PL_MASK_CACHING;
>>>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>>>    		break;
>>>>    	case TTM_PL_VRAM:
>>>>    	case TTM_PL_PRIV:
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
>>>> b/drivers/gpu/drm/radeon/radeon_ttm.c
>>>> index b474781a0920..b4cb75361577 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>>>> @@ -76,9 +76,6 @@ static int radeon_init_mem_type(struct
>>>> ttm_bo_device *bdev, uint32_t type,
>>>>    	switch (type) {
>>>>    	case TTM_PL_SYSTEM:
>>>>    		/* System memory */
>>>> -		man->flags = 0;
>>>> -		man->available_caching = TTM_PL_MASK_CACHING;
>>>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>>>    		break;
>>>>    	case TTM_PL_TT:
>>>>    		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 7c02ce784805..1f1f9e463265 100644
>>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>>> @@ -1677,6 +1677,8 @@ int ttm_bo_device_init(struct ttm_bo_device
>>>> *bdev,
>>>>    	 * Initialize the system memory buffer type.
>>>>    	 * Other types need to be driver / IOCTL initialized.
>>>>    	 */
>>>> +	bdev->man[TTM_PL_SYSTEM].available_caching =
>>>> TTM_PL_MASK_CACHING;
>>>> +	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
>>>>    	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
>>>>    	if (unlikely(ret != 0))
>>>>    		goto out_no_sys;
>>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>>>> b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>>>> index 1d78187eaba6..00cef1a3a178 100644
>>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>>>> @@ -742,8 +742,6 @@ static int vmw_init_mem_type(struct
>>>> ttm_bo_device *bdev, uint32_t type,
>>>>    	switch (type) {
>>>>    	case TTM_PL_SYSTEM:
>>>>    		/* System memory */
>>>> -		man->available_caching = TTM_PL_FLAG_CACHED;
>>> Above is CACHED, not CACHING, so needs to stay to overwrite the
>>> default.
>> Crap I missed that. Problem is that I wanted to remove the
>> possibility
>> to init the system domain with different caching attributes.
>>
>> I don't see how vmwgfx is every going to use this? Thomas do you
>> have
>> any idea what this was good for?
> I interpret that like vmwgfx can't handle any other caching attribute
> than 'cached'. But I can't see anyone feeding vmwgfx system memory
> buffers with other caching attributes. In that case, those would be
> ignored.

Yeah, agree. So any objections to change that setting in vmwgfx?

This way we could have the same settings for all drivers in the kernel 
and I don't see why any driver should have something special here.

Christian.

>
> /Thomas
>
>
>> Thanks,
>> Christian.
>>
>>> With that fixed:
>>>
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>
>>>> -		man->default_caching = TTM_PL_FLAG_CACHED;
>>>>    		break;
>>>>    	case TTM_PL_VRAM:
>>>>    		/* "On-card" video ram */
>>>> -- 
>>>> 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%7C66991383884c46a79f4608d8321ae136%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314438323255193&amp;sdata=Y86QJdnrTmhaG5Sx1NmKaeUbbOkrVvQQP%2BBB29SuA9I%3D&amp;reserved=0
> ----------------------------------------------------------------------
> Intel Sweden AB
> Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
> Registration Number: 556189-6027
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

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

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

* Re: [PATCH 1/9] drm/ttm: initialize the system domain with defaults
  2020-07-27 12:23       ` Christian König
@ 2020-07-27 13:01         ` Hellstrom, Thomas
  0 siblings, 0 replies; 17+ messages in thread
From: Hellstrom, Thomas @ 2020-07-27 13:01 UTC (permalink / raw)
  To: daniel, christian.koenig; +Cc: dri-devel

On Mon, 2020-07-27 at 14:23 +0200, Christian König wrote:
> Am 27.07.20 um 12:50 schrieb Hellstrom, Thomas:
> > On Mon, 2020-07-27 at 12:39 +0200, Christian König wrote:
> > > Am 27.07.20 um 11:42 schrieb daniel@ffwll.ch:
> > > > On Thu, Jul 23, 2020 at 05:16:13PM +0200, Christian König
> > > > wrote:
> > > > > Instead of repeating that in each driver.
> > > > > 
> > > > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > > > ---
> > > > >    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 3 ---
> > > > >    drivers/gpu/drm/drm_gem_vram_helper.c      | 3 ---
> > > > >    drivers/gpu/drm/nouveau/nouveau_bo.c       | 3 ---
> > > > >    drivers/gpu/drm/qxl/qxl_ttm.c              | 3 ---
> > > > >    drivers/gpu/drm/radeon/radeon_ttm.c        | 3 ---
> > > > >    drivers/gpu/drm/ttm/ttm_bo.c               | 2 ++
> > > > >    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 2 --
> > > > >    7 files changed, 2 insertions(+), 17 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > index 0dd5e802091d..e57c49a91b73 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > > > @@ -84,9 +84,6 @@ static int amdgpu_init_mem_type(struct
> > > > > ttm_bo_device *bdev, uint32_t type,
> > > > >    	switch (type) {
> > > > >    	case TTM_PL_SYSTEM:
> > > > >    		/* System memory */
> > > > > -		man->flags = 0;
> > > > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > > > >    		break;
> > > > >    	case TTM_PL_TT:
> > > > >    		/* GTT memory  */
> > > > > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > index 3296ed3df358..be177afdeb9a 100644
> > > > > --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> > > > > @@ -1009,9 +1009,6 @@ static int
> > > > > bo_driver_init_mem_type(struct
> > > > > ttm_bo_device *bdev, uint32_t type,
> > > > >    {
> > > > >    	switch (type) {
> > > > >    	case TTM_PL_SYSTEM:
> > > > > -		man->flags = 0;
> > > > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > > > >    		break;
> > > > >    	case TTM_PL_VRAM:
> > > > >    		man->func = &ttm_bo_manager_func;
> > > > > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > > index 4ccf937df0d0..53af25020bb2 100644
> > > > > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > > > > @@ -655,9 +655,6 @@ nouveau_bo_init_mem_type(struct
> > > > > ttm_bo_device
> > > > > *bdev, uint32_t type,
> > > > >    
> > > > >    	switch (type) {
> > > > >    	case TTM_PL_SYSTEM:
> > > > > -		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;
> > > > > diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > index 1d8e07b8b19e..e9b8c921c1f0 100644
> > > > > --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> > > > > @@ -54,9 +54,6 @@ static int qxl_init_mem_type(struct
> > > > > ttm_bo_device *bdev, uint32_t type,
> > > > >    	switch (type) {
> > > > >    	case TTM_PL_SYSTEM:
> > > > >    		/* System memory */
> > > > > -		man->flags = 0;
> > > > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > > > >    		break;
> > > > >    	case TTM_PL_VRAM:
> > > > >    	case TTM_PL_PRIV:
> > > > > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > index b474781a0920..b4cb75361577 100644
> > > > > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > > > > @@ -76,9 +76,6 @@ static int radeon_init_mem_type(struct
> > > > > ttm_bo_device *bdev, uint32_t type,
> > > > >    	switch (type) {
> > > > >    	case TTM_PL_SYSTEM:
> > > > >    		/* System memory */
> > > > > -		man->flags = 0;
> > > > > -		man->available_caching = TTM_PL_MASK_CACHING;
> > > > > -		man->default_caching = TTM_PL_FLAG_CACHED;
> > > > >    		break;
> > > > >    	case TTM_PL_TT:
> > > > >    		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 7c02ce784805..1f1f9e463265 100644
> > > > > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > > > > @@ -1677,6 +1677,8 @@ int ttm_bo_device_init(struct
> > > > > ttm_bo_device
> > > > > *bdev,
> > > > >    	 * Initialize the system memory buffer type.
> > > > >    	 * Other types need to be driver / IOCTL initialized.
> > > > >    	 */
> > > > > +	bdev->man[TTM_PL_SYSTEM].available_caching =
> > > > > TTM_PL_MASK_CACHING;
> > > > > +	bdev->man[TTM_PL_SYSTEM].default_caching =
> > > > > TTM_PL_FLAG_CACHED;
> > > > >    	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
> > > > >    	if (unlikely(ret != 0))
> > > > >    		goto out_no_sys;
> > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > > > b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > > > index 1d78187eaba6..00cef1a3a178 100644
> > > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > > > > @@ -742,8 +742,6 @@ static int vmw_init_mem_type(struct
> > > > > ttm_bo_device *bdev, uint32_t type,
> > > > >    	switch (type) {
> > > > >    	case TTM_PL_SYSTEM:
> > > > >    		/* System memory */
> > > > > -		man->available_caching = TTM_PL_FLAG_CACHED;
> > > > Above is CACHED, not CACHING, so needs to stay to overwrite the
> > > > default.
> > > Crap I missed that. Problem is that I wanted to remove the
> > > possibility
> > > to init the system domain with different caching attributes.
> > > 
> > > I don't see how vmwgfx is every going to use this? Thomas do you
> > > have
> > > any idea what this was good for?
> > I interpret that like vmwgfx can't handle any other caching
> > attribute
> > than 'cached'. But I can't see anyone feeding vmwgfx system memory
> > buffers with other caching attributes. In that case, those would be
> > ignored.
> 
> Yeah, agree. So any objections to change that setting in vmwgfx?
> 
> This way we could have the same settings for all drivers in the
> kernel 
> and I don't see why any driver should have something special here.
> 
> Christian.
> 

No objections from my side, although I figure Roland has the last
saying.

/Thomas



----------------------------------------------------------------------
Intel Sweden AB
Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm, Sweden
Registration Number: 556189-6027

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
  2020-07-24  6:50   ` Thomas Zimmermann
@ 2020-07-24  7:27     ` Christian König
  0 siblings, 0 replies; 17+ messages in thread
From: Christian König @ 2020-07-24  7:27 UTC (permalink / raw)
  To: Thomas Zimmermann, dri-devel

Am 24.07.20 um 08:50 schrieb Thomas Zimmermann:
>
> Am 23.07.20 um 17:17 schrieb Christian König:
>> Instead use a boolean field in the memory manager structure.
>>
>> Also invert the meaning of the field since the use of a TT
>> structure is the special case here.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> There's a comment further below. In any case
>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +---
>>   drivers/gpu/drm/drm_gem_vram_helper.c      |  1 -
>>   drivers/gpu/drm/nouveau/nouveau_bo.c       |  4 +---
>>   drivers/gpu/drm/qxl/qxl_ttm.c              |  1 -
>>   drivers/gpu/drm/radeon/radeon_ttm.c        |  3 +--
>>   drivers/gpu/drm/ttm/ttm_bo.c               | 14 +++++++-------
>>   drivers/gpu/drm/ttm/ttm_bo_util.c          | 12 ++++++------
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  3 ++-
>>   include/drm/ttm/ttm_bo_driver.h            |  4 +---
>>   9 files changed, 19 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index e57c49a91b73..406bcb03df48 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		break;
>>   	case TTM_PL_TT:
>>   		/* GTT memory  */
>> +		man->use_tt = true;
>>   		man->func = &amdgpu_gtt_mgr_func;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>> -		man->flags = 0;
>>   		break;
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>>   		man->func = &amdgpu_vram_mgr_func;
>> -		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;
>> @@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case AMDGPU_PL_OA:
>>   		/* On-chip GDS memory*/
>>   		man->func = &ttm_bo_manager_func;
>> -		man->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/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>> index be177afdeb9a..801a14c6e9e0 100644
>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>> @@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		break;
>>   	case TTM_PL_VRAM:
>>   		man->func = &ttm_bo_manager_func;
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED |
>>   					 TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> index 53af25020bb2..a3ad66ad3817 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>> @@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case TTM_PL_SYSTEM:
>>   		break;
>>   	case TTM_PL_VRAM:
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED |
>>   					 TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>> @@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		else
>>   			man->func = &ttm_bo_manager_func;
>>   
>> +		man->use_tt = true;
>>   		if (drm->agp.bridge) {
>> -			man->flags = 0;
>>   			man->available_caching = TTM_PL_FLAG_UNCACHED |
>>   				TTM_PL_FLAG_WC;
>>   			man->default_caching = TTM_PL_FLAG_WC;
>>   		} else {
>> -			man->flags = 0;
>>   			man->available_caching = TTM_PL_MASK_CACHING;
>>   			man->default_caching = TTM_PL_FLAG_CACHED;
>>   		}
>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
>> index e9b8c921c1f0..abb9fa4d80cf 100644
>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
>> @@ -59,7 +59,6 @@ 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;
>>   		man->available_caching = TTM_PL_MASK_CACHING;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>> index b4cb75361577..9aba18a143e7 100644
>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>> @@ -81,7 +81,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 = 0;
>> +		man->use_tt = true;
>>   #if IS_ENABLED(CONFIG_AGP)
>>   		if (rdev->flags & RADEON_IS_AGP) {
>>   			if (!rdev->ddev->agp) {
>> @@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>>   		man->func = &ttm_bo_manager_func;
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>>   		man->default_caching = TTM_PL_FLAG_WC;
>>   		break;
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>> index 1f1f9e463265..6dea56dce350 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>> @@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
>>   
>>   	drm_printf(p, "    has_type: %d\n", man->has_type);
>>   	drm_printf(p, "    use_type: %d\n", man->use_type);
>> -	drm_printf(p, "    flags: 0x%08X\n", man->flags);
>> +	drm_printf(p, "    use_tt: %d\n", man->use_tt);
>>   	drm_printf(p, "    size: %llu\n", man->size);
>>   	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
>>   	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
>> @@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
>>   	man = &bdev->man[mem->mem_type];
>>   	list_add_tail(&bo->lru, &man->lru[bo->priority]);
>>   
>> -	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
>> +	if (man->use_tt && bo->ttm &&
>>   	    !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
>>   				     TTM_PAGE_FLAG_SWAPPED))) {
>>   		list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
>> @@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>   	 * Create and bind a ttm if required.
>>   	 */
>>   
>> -	if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
>> -		bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
>> +	if (new_man->use_tt) {
>> +		bool zero = old_man->use_tt;
> There's little use in copying to zero.
>
>>   
>>   		ret = ttm_tt_create(bo, zero);
> Maybe rather pass old_man->use_tt directly and leave a comment why that
> makes sense.

Good point, going to fix that.

Thanks,
Christian.

>
> Best regards
> Thomas
>
>>   		if (ret)
>> @@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>   	if (bdev->driver->move_notify)
>>   		bdev->driver->move_notify(bo, evict, mem);
>>   
>> -	if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
>> -	    !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
>> +	if (old_man->use_tt && new_man->use_tt)
>>   		ret = ttm_bo_move_ttm(bo, ctx, mem);
>>   	else if (bdev->driver->move)
>>   		ret = bdev->driver->move(bo, evict, ctx, mem);
>> @@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>   
>>   out_err:
>>   	new_man = &bdev->man[bo->mem.mem_type];
>> -	if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +	if (!new_man->use_tt) {
>>   		ttm_tt_destroy(bo->ttm);
>>   		bo->ttm = NULL;
>>   	}
>> @@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>>   	 * Initialize the system memory buffer type.
>>   	 * Other types need to be driver / IOCTL initialized.
>>   	 */
>> +	bdev->man[TTM_PL_SYSTEM].use_tt = true;
>>   	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
>>   	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
>>   	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
>> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> index 7fb3e0bcbab4..1f502be0b646 100644
>> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
>> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
>> @@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>>   	*old_mem = *new_mem;
>>   	new_mem->mm_node = NULL;
>>   
>> -	if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +	if (!man->use_tt) {
>>   		ttm_tt_destroy(ttm);
>>   		bo->ttm = NULL;
>>   	}
>> @@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>>   		if (ret)
>>   			return ret;
>>   
>> -		if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +		if (!man->use_tt) {
>>   			ttm_tt_destroy(bo->ttm);
>>   			bo->ttm = NULL;
>>   		}
>> @@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>>   		 * bo to be unbound and destroyed.
>>   		 */
>>   
>> -		if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
>> +		if (man->use_tt)
>>   			ghost_obj->ttm = NULL;
>>   		else
>>   			bo->ttm = NULL;
>> @@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>>   		 * bo to be unbound and destroyed.
>>   		 */
>>   
>> -		if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
>> +		if (to->use_tt)
>>   			ghost_obj->ttm = NULL;
>>   		else
>>   			bo->ttm = NULL;
>> @@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>>   		dma_resv_unlock(&ghost_obj->base._resv);
>>   		ttm_bo_put(ghost_obj);
>>   
>> -	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +	} else if (!from->use_tt) {
>>   
>>   		/**
>>   		 * BO doesn't have a TTM we need to bind/unbind. Just remember
>> @@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>>   		if (ret)
>>   			return ret;
>>   
>> -		if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
>> +		if (!to->use_tt) {
>>   			ttm_tt_destroy(bo->ttm);
>>   			bo->ttm = NULL;
>>   		}
>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> index 00cef1a3a178..5d8179d9f394 100644
>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>> @@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   	case TTM_PL_VRAM:
>>   		/* "On-card" video ram */
>>   		man->func = &vmw_thp_func;
>> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>>   		man->available_caching = TTM_PL_FLAG_CACHED;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>>   		break;
>> @@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>>   		man->func = &vmw_gmrid_manager_func;
>>   		man->available_caching = TTM_PL_FLAG_CACHED;
>>   		man->default_caching = TTM_PL_FLAG_CACHED;
>> +		/* TODO: This is most likely not correct */
>> +		man->use_tt = true;
>>   		break;
>>   	default:
>>   		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
>> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
>> index 9b251853afe2..adac4cd0ba23 100644
>> --- a/include/drm/ttm/ttm_bo_driver.h
>> +++ b/include/drm/ttm/ttm_bo_driver.h
>> @@ -45,8 +45,6 @@
>>   
>>   #define TTM_MAX_BO_PRIORITY	4U
>>   
>> -#define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
>> -
>>   struct ttm_mem_type_manager;
>>   
>>   struct ttm_mem_type_manager_func {
>> @@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
>>   
>>   	bool has_type;
>>   	bool use_type;
>> -	uint32_t flags;
>> +	bool use_tt;
>>   	uint64_t size;
>>   	uint32_t available_caching;
>>   	uint32_t default_caching;
>>

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

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

* Re: [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
  2020-07-23 15:17 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
@ 2020-07-24  6:50   ` Thomas Zimmermann
  2020-07-24  7:27     ` Christian König
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Zimmermann @ 2020-07-24  6:50 UTC (permalink / raw)
  To: Christian König, dri-devel


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



Am 23.07.20 um 17:17 schrieb Christian König:
> Instead use a boolean field in the memory manager structure.
> 
> Also invert the meaning of the field since the use of a TT
> structure is the special case here.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

There's a comment further below. In any case

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +---
>  drivers/gpu/drm/drm_gem_vram_helper.c      |  1 -
>  drivers/gpu/drm/nouveau/nouveau_bo.c       |  4 +---
>  drivers/gpu/drm/qxl/qxl_ttm.c              |  1 -
>  drivers/gpu/drm/radeon/radeon_ttm.c        |  3 +--
>  drivers/gpu/drm/ttm/ttm_bo.c               | 14 +++++++-------
>  drivers/gpu/drm/ttm/ttm_bo_util.c          | 12 ++++++------
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  3 ++-
>  include/drm/ttm/ttm_bo_driver.h            |  4 +---
>  9 files changed, 19 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e57c49a91b73..406bcb03df48 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		break;
>  	case TTM_PL_TT:
>  		/* GTT memory  */
> +		man->use_tt = true;
>  		man->func = &amdgpu_gtt_mgr_func;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = 0;
>  		break;
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &amdgpu_vram_mgr_func;
> -		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;
> @@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case AMDGPU_PL_OA:
>  		/* On-chip GDS memory*/
>  		man->func = &ttm_bo_manager_func;
> -		man->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/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index be177afdeb9a..801a14c6e9e0 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		break;
>  	case TTM_PL_VRAM:
>  		man->func = &ttm_bo_manager_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED |
>  					 TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 53af25020bb2..a3ad66ad3817 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case TTM_PL_SYSTEM:
>  		break;
>  	case TTM_PL_VRAM:
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED |
>  					 TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
> @@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		else
>  			man->func = &ttm_bo_manager_func;
>  
> +		man->use_tt = true;
>  		if (drm->agp.bridge) {
> -			man->flags = 0;
>  			man->available_caching = TTM_PL_FLAG_UNCACHED |
>  				TTM_PL_FLAG_WC;
>  			man->default_caching = TTM_PL_FLAG_WC;
>  		} else {
> -			man->flags = 0;
>  			man->available_caching = TTM_PL_MASK_CACHING;
>  			man->default_caching = TTM_PL_FLAG_CACHED;
>  		}
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index e9b8c921c1f0..abb9fa4d80cf 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -59,7 +59,6 @@ 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;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index b4cb75361577..9aba18a143e7 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -81,7 +81,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 = 0;
> +		man->use_tt = true;
>  #if IS_ENABLED(CONFIG_AGP)
>  		if (rdev->flags & RADEON_IS_AGP) {
>  			if (!rdev->ddev->agp) {
> @@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &ttm_bo_manager_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
>  		break;
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 1f1f9e463265..6dea56dce350 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
>  
>  	drm_printf(p, "    has_type: %d\n", man->has_type);
>  	drm_printf(p, "    use_type: %d\n", man->use_type);
> -	drm_printf(p, "    flags: 0x%08X\n", man->flags);
> +	drm_printf(p, "    use_tt: %d\n", man->use_tt);
>  	drm_printf(p, "    size: %llu\n", man->size);
>  	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
>  	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
> @@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
>  	man = &bdev->man[mem->mem_type];
>  	list_add_tail(&bo->lru, &man->lru[bo->priority]);
>  
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
> +	if (man->use_tt && bo->ttm &&
>  	    !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
>  				     TTM_PAGE_FLAG_SWAPPED))) {
>  		list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
> @@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  	 * Create and bind a ttm if required.
>  	 */
>  
> -	if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
> -		bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
> +	if (new_man->use_tt) {
> +		bool zero = old_man->use_tt;

There's little use in copying to zero.

>  
>  		ret = ttm_tt_create(bo, zero);

Maybe rather pass old_man->use_tt directly and leave a comment why that
makes sense.

Best regards
Thomas

>  		if (ret)
> @@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  	if (bdev->driver->move_notify)
>  		bdev->driver->move_notify(bo, evict, mem);
>  
> -	if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
> -	    !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
> +	if (old_man->use_tt && new_man->use_tt)
>  		ret = ttm_bo_move_ttm(bo, ctx, mem);
>  	else if (bdev->driver->move)
>  		ret = bdev->driver->move(bo, evict, ctx, mem);
> @@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>  
>  out_err:
>  	new_man = &bdev->man[bo->mem.mem_type];
> -	if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +	if (!new_man->use_tt) {
>  		ttm_tt_destroy(bo->ttm);
>  		bo->ttm = NULL;
>  	}
> @@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>  	 * Initialize the system memory buffer type.
>  	 * Other types need to be driver / IOCTL initialized.
>  	 */
> +	bdev->man[TTM_PL_SYSTEM].use_tt = true;
>  	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
>  	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
>  	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 7fb3e0bcbab4..1f502be0b646 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>  	*old_mem = *new_mem;
>  	new_mem->mm_node = NULL;
>  
> -	if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +	if (!man->use_tt) {
>  		ttm_tt_destroy(ttm);
>  		bo->ttm = NULL;
>  	}
> @@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>  		if (ret)
>  			return ret;
>  
> -		if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +		if (!man->use_tt) {
>  			ttm_tt_destroy(bo->ttm);
>  			bo->ttm = NULL;
>  		}
> @@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
>  		 * bo to be unbound and destroyed.
>  		 */
>  
> -		if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
> +		if (man->use_tt)
>  			ghost_obj->ttm = NULL;
>  		else
>  			bo->ttm = NULL;
> @@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>  		 * bo to be unbound and destroyed.
>  		 */
>  
> -		if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
> +		if (to->use_tt)
>  			ghost_obj->ttm = NULL;
>  		else
>  			bo->ttm = NULL;
> @@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>  		dma_resv_unlock(&ghost_obj->base._resv);
>  		ttm_bo_put(ghost_obj);
>  
> -	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +	} else if (!from->use_tt) {
>  
>  		/**
>  		 * BO doesn't have a TTM we need to bind/unbind. Just remember
> @@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
>  		if (ret)
>  			return ret;
>  
> -		if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
> +		if (!to->use_tt) {
>  			ttm_tt_destroy(bo->ttm);
>  			bo->ttm = NULL;
>  		}
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index 00cef1a3a178..5d8179d9f394 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  	case TTM_PL_VRAM:
>  		/* "On-card" video ram */
>  		man->func = &vmw_thp_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
> @@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  		man->func = &vmw_gmrid_manager_func;
>  		man->available_caching = TTM_PL_FLAG_CACHED;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
> +		/* TODO: This is most likely not correct */
> +		man->use_tt = true;
>  		break;
>  	default:
>  		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 9b251853afe2..adac4cd0ba23 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -45,8 +45,6 @@
>  
>  #define TTM_MAX_BO_PRIORITY	4U
>  
> -#define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
> -
>  struct ttm_mem_type_manager;
>  
>  struct ttm_mem_type_manager_func {
> @@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
>  
>  	bool has_type;
>  	bool use_type;
> -	uint32_t flags;
> +	bool use_tt;
>  	uint64_t size;
>  	uint32_t available_caching;
>  	uint32_t default_caching;
> 

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


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

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

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

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

* [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED
  2020-07-23 15:17 More TTM cleanups Christian König
@ 2020-07-23 15:17 ` Christian König
  2020-07-24  6:50   ` Thomas Zimmermann
  0 siblings, 1 reply; 17+ messages in thread
From: Christian König @ 2020-07-23 15:17 UTC (permalink / raw)
  To: dri-devel

Instead use a boolean field in the memory manager structure.

Also invert the meaning of the field since the use of a TT
structure is the special case here.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +---
 drivers/gpu/drm/drm_gem_vram_helper.c      |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c       |  4 +---
 drivers/gpu/drm/qxl/qxl_ttm.c              |  1 -
 drivers/gpu/drm/radeon/radeon_ttm.c        |  3 +--
 drivers/gpu/drm/ttm/ttm_bo.c               | 14 +++++++-------
 drivers/gpu/drm/ttm/ttm_bo_util.c          | 12 ++++++------
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  3 ++-
 include/drm/ttm/ttm_bo_driver.h            |  4 +---
 9 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e57c49a91b73..406bcb03df48 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -87,15 +87,14 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		break;
 	case TTM_PL_TT:
 		/* GTT memory  */
+		man->use_tt = true;
 		man->func = &amdgpu_gtt_mgr_func;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->flags = 0;
 		break;
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &amdgpu_vram_mgr_func;
-		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;
@@ -104,7 +103,6 @@ static int amdgpu_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case AMDGPU_PL_OA:
 		/* On-chip GDS memory*/
 		man->func = &ttm_bo_manager_func;
-		man->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/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index be177afdeb9a..801a14c6e9e0 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1012,7 +1012,6 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		break;
 	case TTM_PL_VRAM:
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED |
 					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 53af25020bb2..a3ad66ad3817 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -657,7 +657,6 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_SYSTEM:
 		break;
 	case TTM_PL_VRAM:
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED |
 					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
@@ -685,13 +684,12 @@ nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		else
 			man->func = &ttm_bo_manager_func;
 
+		man->use_tt = true;
 		if (drm->agp.bridge) {
-			man->flags = 0;
 			man->available_caching = TTM_PL_FLAG_UNCACHED |
 				TTM_PL_FLAG_WC;
 			man->default_caching = TTM_PL_FLAG_WC;
 		} else {
-			man->flags = 0;
 			man->available_caching = TTM_PL_MASK_CACHING;
 			man->default_caching = TTM_PL_FLAG_CACHED;
 		}
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index e9b8c921c1f0..abb9fa4d80cf 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -59,7 +59,6 @@ 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;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index b4cb75361577..9aba18a143e7 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -81,7 +81,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 = 0;
+		man->use_tt = true;
 #if IS_ENABLED(CONFIG_AGP)
 		if (rdev->flags & RADEON_IS_AGP) {
 			if (!rdev->ddev->agp) {
@@ -98,7 +98,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
 		break;
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 1f1f9e463265..6dea56dce350 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -84,7 +84,7 @@ static void ttm_mem_type_debug(struct ttm_bo_device *bdev, struct drm_printer *p
 
 	drm_printf(p, "    has_type: %d\n", man->has_type);
 	drm_printf(p, "    use_type: %d\n", man->use_type);
-	drm_printf(p, "    flags: 0x%08X\n", man->flags);
+	drm_printf(p, "    use_tt: %d\n", man->use_tt);
 	drm_printf(p, "    size: %llu\n", man->size);
 	drm_printf(p, "    available_caching: 0x%08X\n", man->available_caching);
 	drm_printf(p, "    default_caching: 0x%08X\n", man->default_caching);
@@ -159,7 +159,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo,
 	man = &bdev->man[mem->mem_type];
 	list_add_tail(&bo->lru, &man->lru[bo->priority]);
 
-	if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED) && bo->ttm &&
+	if (man->use_tt && bo->ttm &&
 	    !(bo->ttm->page_flags & (TTM_PAGE_FLAG_SG |
 				     TTM_PAGE_FLAG_SWAPPED))) {
 		list_add_tail(&bo->swap, &ttm_bo_glob.swap_lru[bo->priority]);
@@ -286,8 +286,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 	 * Create and bind a ttm if required.
 	 */
 
-	if (!(new_man->flags & TTM_MEMTYPE_FLAG_FIXED)) {
-		bool zero = !(old_man->flags & TTM_MEMTYPE_FLAG_FIXED);
+	if (new_man->use_tt) {
+		bool zero = old_man->use_tt;
 
 		ret = ttm_tt_create(bo, zero);
 		if (ret)
@@ -314,8 +314,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 	if (bdev->driver->move_notify)
 		bdev->driver->move_notify(bo, evict, mem);
 
-	if (!(old_man->flags & TTM_MEMTYPE_FLAG_FIXED) &&
-	    !(new_man->flags & TTM_MEMTYPE_FLAG_FIXED))
+	if (old_man->use_tt && new_man->use_tt)
 		ret = ttm_bo_move_ttm(bo, ctx, mem);
 	else if (bdev->driver->move)
 		ret = bdev->driver->move(bo, evict, ctx, mem);
@@ -340,7 +339,7 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 
 out_err:
 	new_man = &bdev->man[bo->mem.mem_type];
-	if (new_man->flags & TTM_MEMTYPE_FLAG_FIXED) {
+	if (!new_man->use_tt) {
 		ttm_tt_destroy(bo->ttm);
 		bo->ttm = NULL;
 	}
@@ -1677,6 +1676,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
 	 * Initialize the system memory buffer type.
 	 * Other types need to be driver / IOCTL initialized.
 	 */
+	bdev->man[TTM_PL_SYSTEM].use_tt = true;
 	bdev->man[TTM_PL_SYSTEM].available_caching = TTM_PL_MASK_CACHING;
 	bdev->man[TTM_PL_SYSTEM].default_caching = TTM_PL_FLAG_CACHED;
 	ret = ttm_bo_init_mm(bdev, TTM_PL_SYSTEM, 0);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 7fb3e0bcbab4..1f502be0b646 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -384,7 +384,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
 	*old_mem = *new_mem;
 	new_mem->mm_node = NULL;
 
-	if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
+	if (!man->use_tt) {
 		ttm_tt_destroy(ttm);
 		bo->ttm = NULL;
 	}
@@ -645,7 +645,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 		if (ret)
 			return ret;
 
-		if (man->flags & TTM_MEMTYPE_FLAG_FIXED) {
+		if (!man->use_tt) {
 			ttm_tt_destroy(bo->ttm);
 			bo->ttm = NULL;
 		}
@@ -674,7 +674,7 @@ int ttm_bo_move_accel_cleanup(struct ttm_buffer_object *bo,
 		 * bo to be unbound and destroyed.
 		 */
 
-		if (!(man->flags & TTM_MEMTYPE_FLAG_FIXED))
+		if (man->use_tt)
 			ghost_obj->ttm = NULL;
 		else
 			bo->ttm = NULL;
@@ -730,7 +730,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		 * bo to be unbound and destroyed.
 		 */
 
-		if (!(to->flags & TTM_MEMTYPE_FLAG_FIXED))
+		if (to->use_tt)
 			ghost_obj->ttm = NULL;
 		else
 			bo->ttm = NULL;
@@ -738,7 +738,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		dma_resv_unlock(&ghost_obj->base._resv);
 		ttm_bo_put(ghost_obj);
 
-	} else if (from->flags & TTM_MEMTYPE_FLAG_FIXED) {
+	} else if (!from->use_tt) {
 
 		/**
 		 * BO doesn't have a TTM we need to bind/unbind. Just remember
@@ -768,7 +768,7 @@ int ttm_bo_pipeline_move(struct ttm_buffer_object *bo,
 		if (ret)
 			return ret;
 
-		if (to->flags & TTM_MEMTYPE_FLAG_FIXED) {
+		if (!to->use_tt) {
 			ttm_tt_destroy(bo->ttm);
 			bo->ttm = NULL;
 		}
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 00cef1a3a178..5d8179d9f394 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -746,7 +746,6 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &vmw_thp_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -760,6 +759,8 @@ static int vmw_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->func = &vmw_gmrid_manager_func;
 		man->available_caching = TTM_PL_FLAG_CACHED;
 		man->default_caching = TTM_PL_FLAG_CACHED;
+		/* TODO: This is most likely not correct */
+		man->use_tt = true;
 		break;
 	default:
 		DRM_ERROR("Unsupported memory type %u\n", (unsigned)type);
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 9b251853afe2..adac4cd0ba23 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -45,8 +45,6 @@
 
 #define TTM_MAX_BO_PRIORITY	4U
 
-#define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
-
 struct ttm_mem_type_manager;
 
 struct ttm_mem_type_manager_func {
@@ -173,7 +171,7 @@ struct ttm_mem_type_manager {
 
 	bool has_type;
 	bool use_type;
-	uint32_t flags;
+	bool use_tt;
 	uint64_t size;
 	uint32_t available_caching;
 	uint32_t default_caching;
-- 
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] 17+ messages in thread

end of thread, other threads:[~2020-07-27 13:01 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 15:16 [PATCH 1/9] drm/ttm: initialize the system domain with defaults Christian König
2020-07-23 15:16 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
2020-07-27  9:48   ` daniel
2020-07-27  9:54     ` Christian König
2020-07-27 10:27       ` daniel
2020-07-23 15:16 ` [PATCH 3/9] drm/radeon: stop implementing init_mem_type Christian König
2020-07-27 10:30   ` daniel
2020-07-27 10:30     ` daniel
2020-07-27 10:54     ` Christian König
2020-07-27  9:42 ` [PATCH 1/9] drm/ttm: initialize the system domain with defaults daniel
2020-07-27 10:39   ` Christian König
2020-07-27 10:50     ` Hellstrom, Thomas
2020-07-27 12:23       ` Christian König
2020-07-27 13:01         ` Hellstrom, Thomas
2020-07-23 15:17 More TTM cleanups Christian König
2020-07-23 15:17 ` [PATCH 2/9] drm/ttm: remove TTM_MEMTYPE_FLAG_FIXED Christian König
2020-07-24  6:50   ` Thomas Zimmermann
2020-07-24  7:27     ` Christian König

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.