All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA
@ 2020-07-16 12:50 Christian König
  2020-07-16 12:50 ` [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

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

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

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
@ 2020-07-16 12:50 ` Christian König
  2020-07-19 12:57   ` Chauhan, Madhav
  2020-07-16 12:50 ` [PATCH 3/8] drm/vmwgfx: " Christian König
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 3/8] drm/vmwgfx: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
  2020-07-16 12:50 ` [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-16 12:50 ` Christian König
  2020-07-16 12:50 ` [PATCH 4/8] drm/amdgpu: " Christian König
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 4/8] drm/amdgpu: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
  2020-07-16 12:50 ` [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
  2020-07-16 12:50 ` [PATCH 3/8] drm/vmwgfx: " Christian König
@ 2020-07-16 12:50 ` Christian König
  2020-07-16 12:50 ` [PATCH 5/8] drm/nouveau: " Christian König
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 5/8] drm/nouveau: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
                   ` (2 preceding siblings ...)
  2020-07-16 12:50 ` [PATCH 4/8] drm/amdgpu: " Christian König
@ 2020-07-16 12:50 ` Christian König
  2020-07-16 12:50 ` [PATCH 6/8] drm/qxl: " Christian König
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 6/8] drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
                   ` (3 preceding siblings ...)
  2020-07-16 12:50 ` [PATCH 5/8] drm/nouveau: " Christian König
@ 2020-07-16 12:50 ` Christian König
  2020-07-16 12:50 ` [PATCH 7/8] drm/vram-helper: " Christian König
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index a6e67149ef4a..820eb190d292 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -54,7 +54,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -62,8 +62,7 @@ static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_PRIV:
 		/* "On-card" video ram */
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED |
-			     TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -107,8 +106,7 @@ int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
 	mem->bus.size = mem->num_pages << PAGE_SHIFT;
 	mem->bus.base = 0;
 	mem->bus.is_iomem = false;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
+
 	switch (mem->mem_type) {
 	case TTM_PL_SYSTEM:
 		/* system memory */
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH 7/8] drm/vram-helper: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
                   ` (4 preceding siblings ...)
  2020-07-16 12:50 ` [PATCH 6/8] drm/qxl: " Christian König
@ 2020-07-16 12:50 ` Christian König
  2020-07-16 13:01   ` Thomas Zimmermann
  2020-07-16 12:50 ` [PATCH 8/8] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
  2020-07-16 13:05 ` [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Thomas Zimmermann
  7 siblings, 1 reply; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index e62a2b68fe3a..b6e4e49027aa 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1017,14 +1017,13 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 {
 	switch (type) {
 	case TTM_PL_SYSTEM:
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
 	case TTM_PL_VRAM:
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED |
-			     TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED |
 					 TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
@@ -1067,12 +1066,8 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
 static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
 				    struct ttm_mem_reg *mem)
 {
-	struct ttm_mem_type_manager *man = bdev->man + mem->mem_type;
 	struct drm_vram_mm *vmm = drm_vram_mm_of_bdev(bdev);
 
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
-
 	mem->bus.addr = NULL;
 	mem->bus.size = mem->num_pages << PAGE_SHIFT;
 
-- 
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] 13+ messages in thread
* [PATCH 8/8] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
                   ` (5 preceding siblings ...)
  2020-07-16 12:50 ` [PATCH 7/8] drm/vram-helper: " Christian König
@ 2020-07-16 12:50 ` Christian König
  2020-07-16 13:05 ` [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Thomas Zimmermann
  7 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2020-07-16 12:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Madhav.Chauhan

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

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

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

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH 7/8] drm/vram-helper: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 ` [PATCH 7/8] drm/vram-helper: " Christian König
@ 2020-07-16 13:01   ` Thomas Zimmermann
  0 siblings, 0 replies; 13+ messages in thread
From: Thomas Zimmermann @ 2020-07-16 13:01 UTC (permalink / raw)
  To: Christian König, dri-devel; +Cc: Madhav.Chauhan


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



Am 16.07.20 um 14:50 schrieb Christian König:
> The helper doesn't expose any not-mapable memory resources.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

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

> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index e62a2b68fe3a..b6e4e49027aa 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1017,14 +1017,13 @@ static int bo_driver_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>  {
>  	switch (type) {
>  	case TTM_PL_SYSTEM:
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>  		man->available_caching = TTM_PL_MASK_CACHING;
>  		man->default_caching = TTM_PL_FLAG_CACHED;
>  		break;
>  	case TTM_PL_VRAM:
>  		man->func = &ttm_bo_manager_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED |
> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>  		man->available_caching = TTM_PL_FLAG_UNCACHED |
>  					 TTM_PL_FLAG_WC;
>  		man->default_caching = TTM_PL_FLAG_WC;
> @@ -1067,12 +1066,8 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
>  static int bo_driver_io_mem_reserve(struct ttm_bo_device *bdev,
>  				    struct ttm_mem_reg *mem)
>  {
> -	struct ttm_mem_type_manager *man = bdev->man + mem->mem_type;
>  	struct drm_vram_mm *vmm = drm_vram_mm_of_bdev(bdev);
>  
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;
> -
>  	mem->bus.addr = NULL;
>  	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>  
> 

-- 
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] 13+ messages in thread
* Re: [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA
  2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
                   ` (6 preceding siblings ...)
  2020-07-16 12:50 ` [PATCH 8/8] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-16 13:05 ` Thomas Zimmermann
  2020-07-20 11:10   ` Christian König
  7 siblings, 1 reply; 13+ messages in thread
From: Thomas Zimmermann @ 2020-07-16 13:05 UTC (permalink / raw)
  To: Christian König, dri-devel; +Cc: Madhav.Chauhan


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

Hi,

this patchset could have benefited from a cover letter.

Am 16.07.20 um 14:50 schrieb Christian König:
> The original intention was to avoid CPU page table unmaps
> when BOs move between the GTT and SYSTEM domain.
> 
> The problem is that this never correctly handled changes
> in the caching attributes or backing pages.
> 
> Just drop this for now and simply unmap the CPU page
> tables in all cases.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +--
>  drivers/gpu/drm/nouveau/nouveau_bo.c       |  3 +-
>  drivers/gpu/drm/radeon/radeon_ttm.c        |  2 +-
>  drivers/gpu/drm/ttm/ttm_bo.c               | 34 ++++------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  2 +-
>  include/drm/ttm/ttm_bo_driver.h            |  1 -
>  6 files changed, 11 insertions(+), 35 deletions(-)

Why's CMA cleaned up in one patch and MAPPABLE in the other 7?

Wouldn't it have been better to kill both the flags from ttm in 2
patches, then have one patch per driver and finally remove both flags
from the ttm header?

Best regards
Thomas

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

-- 
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] 13+ messages in thread
* RE: [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-16 12:50 ` [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
@ 2020-07-19 12:57   ` Chauhan, Madhav
  2020-07-20 11:02     ` Christian König
  0 siblings, 1 reply; 13+ messages in thread
From: Chauhan, Madhav @ 2020-07-19 12:57 UTC (permalink / raw)
  To: Christian König, dri-devel

[AMD Public Use]

-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@gmail.com> 
Sent: Thursday, July 16, 2020 6:21 PM
To: dri-devel@lists.freedesktop.org
Cc: Chauhan, Madhav <Madhav.Chauhan@amd.com>
Subject: [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE

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

Looks like spell mistake in "mapable". Please check.

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

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 54af06df865b..b474781a0920 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	switch (type) {
 	case TTM_PL_SYSTEM:
 		/* System memory */
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;

adev memory was set to zero while allocated and adev->mman.bdev used to fetch different mman.
Do we need explicit initialization to '0'?? 

Regards,
Madhav

 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
 		break;
@@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 		man->func = &ttm_bo_manager_func;
 		man->available_caching = TTM_PL_MASK_CACHING;
 		man->default_caching = TTM_PL_FLAG_CACHED;
-		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = 0;
 #if IS_ENABLED(CONFIG_AGP)
 		if (rdev->flags & RADEON_IS_AGP) {
 			if (!rdev->ddev->agp) {
@@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 					  (unsigned)type);
 				return -EINVAL;
 			}
-			if (!rdev->ddev->agp->cant_use_aperture)
-				man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
 			man->available_caching = TTM_PL_FLAG_UNCACHED |
 						 TTM_PL_FLAG_WC;
 			man->default_caching = TTM_PL_FLAG_WC; @@ -103,8 +101,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
 	case TTM_PL_VRAM:
 		/* "On-card" video ram */
 		man->func = &ttm_bo_manager_func;
-		man->flags = TTM_MEMTYPE_FLAG_FIXED |
-			     TTM_MEMTYPE_FLAG_MAPPABLE;
+		man->flags = TTM_MEMTYPE_FLAG_FIXED;
 		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
 		man->default_caching = TTM_PL_FLAG_WC;
 		break;
@@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
 
 static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)  {
-	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
 	struct radeon_device *rdev = radeon_get_rdev(bdev);
 
 	mem->bus.addr = NULL;
@@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
 	mem->bus.size = mem->num_pages << PAGE_SHIFT;
 	mem->bus.base = 0;
 	mem->bus.is_iomem = false;
-	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-		return -EINVAL;
+
 	switch (mem->mem_type) {
 	case TTM_PL_SYSTEM:
 		/* system memory */
--
2.17.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  2020-07-19 12:57   ` Chauhan, Madhav
@ 2020-07-20 11:02     ` Christian König
  0 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2020-07-20 11:02 UTC (permalink / raw)
  To: Chauhan, Madhav, dri-devel

Am 19.07.20 um 14:57 schrieb Chauhan, Madhav:
> [AMD Public Use]
>
> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken@gmail.com>
> Sent: Thursday, July 16, 2020 6:21 PM
> To: dri-devel@lists.freedesktop.org
> Cc: Chauhan, Madhav <Madhav.Chauhan@amd.com>
> Subject: [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE
>
> The driver doesn't expose any not-mapable memory resources.
>
> Looks like spell mistake in "mapable". Please check.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/radeon/radeon_ttm.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 54af06df865b..b474781a0920 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   	switch (type) {
>   	case TTM_PL_SYSTEM:
>   		/* System memory */
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>
> adev memory was set to zero while allocated and adev->mman.bdev used to fetch different mman.
> Do we need explicit initialization to '0'??

No, not really. But I wasn't sure if other drivers initialize their 
structures as well.

So I just kept it to be uniform across drivers to completely rework 
mem_type init at some point.

Regards,
Christian.

>
> Regards,
> Madhav
>
>   		man->available_caching = TTM_PL_MASK_CACHING;
>   		man->default_caching = TTM_PL_FLAG_CACHED;
>   		break;
> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   		man->func = &ttm_bo_manager_func;
>   		man->available_caching = TTM_PL_MASK_CACHING;
>   		man->default_caching = TTM_PL_FLAG_CACHED;
> -		man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = 0;
>   #if IS_ENABLED(CONFIG_AGP)
>   		if (rdev->flags & RADEON_IS_AGP) {
>   			if (!rdev->ddev->agp) {
> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   					  (unsigned)type);
>   				return -EINVAL;
>   			}
> -			if (!rdev->ddev->agp->cant_use_aperture)
> -				man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>   			man->available_caching = TTM_PL_FLAG_UNCACHED |
>   						 TTM_PL_FLAG_WC;
>   			man->default_caching = TTM_PL_FLAG_WC; @@ -103,8 +101,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
>   	case TTM_PL_VRAM:
>   		/* "On-card" video ram */
>   		man->func = &ttm_bo_manager_func;
> -		man->flags = TTM_MEMTYPE_FLAG_FIXED |
> -			     TTM_MEMTYPE_FLAG_MAPPABLE;
> +		man->flags = TTM_MEMTYPE_FLAG_FIXED;
>   		man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
>   		man->default_caching = TTM_PL_FLAG_WC;
>   		break;
> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
>   
>   static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_reg *mem)  {
> -	struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>   	struct radeon_device *rdev = radeon_get_rdev(bdev);
>   
>   	mem->bus.addr = NULL;
> @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct ttm_mem_
>   	mem->bus.size = mem->num_pages << PAGE_SHIFT;
>   	mem->bus.base = 0;
>   	mem->bus.is_iomem = false;
> -	if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
> -		return -EINVAL;
> +
>   	switch (mem->mem_type) {
>   	case TTM_PL_SYSTEM:
>   		/* system memory */
> --
> 2.17.1

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

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA
  2020-07-16 13:05 ` [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Thomas Zimmermann
@ 2020-07-20 11:10   ` Christian König
  0 siblings, 0 replies; 13+ messages in thread
From: Christian König @ 2020-07-20 11:10 UTC (permalink / raw)
  To: Thomas Zimmermann, dri-devel; +Cc: Madhav.Chauhan

Am 16.07.20 um 15:05 schrieb Thomas Zimmermann:
> Hi,
>
> this patchset could have benefited from a cover letter.

Yeah, I didn't thought it would result in such an extensive patch set.

> Am 16.07.20 um 14:50 schrieb Christian König:
>> The original intention was to avoid CPU page table unmaps
>> when BOs move between the GTT and SYSTEM domain.
>>
>> The problem is that this never correctly handled changes
>> in the caching attributes or backing pages.
>>
>> Just drop this for now and simply unmap the CPU page
>> tables in all cases.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  4 +--
>>   drivers/gpu/drm/nouveau/nouveau_bo.c       |  3 +-
>>   drivers/gpu/drm/radeon/radeon_ttm.c        |  2 +-
>>   drivers/gpu/drm/ttm/ttm_bo.c               | 34 ++++------------------
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  2 +-
>>   include/drm/ttm/ttm_bo_driver.h            |  1 -
>>   6 files changed, 11 insertions(+), 35 deletions(-)
> Why's CMA cleaned up in one patch and MAPPABLE in the other 7?

There is a chance that the CMA removal fixes some bugs and needs to be 
back-ported. But the other 7 are pure cleanup.

> Wouldn't it have been better to kill both the flags from ttm in 2
> patches, then have one patch per driver and finally remove both flags
> from the ttm header?

That's what I did initially. Basically I found that the 
TTM_MEMTYPE_FLAG_CMA flag might be the root of some problems because of 
the problematic implementation.

While removing it I've found that the TTM_MEMTYPE_FLAG_MAPPABLE is 
completely unused in TTM as well, so I removed both at the same time.

But that turned out to break drivers because they use the 
TTM_MEMTYPE_FLAG_MAPPABLE flag for no good reason at all :)

Anyway, I want to keep the order now since it might be a good idea to 
back port the CMA flag removal (but I'm not 100% sure of that).

Thanks,
Christian.

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

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

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-07-20 11:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 12:50 [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Christian König
2020-07-16 12:50 ` [PATCH 2/8] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE Christian König
2020-07-19 12:57   ` Chauhan, Madhav
2020-07-20 11:02     ` Christian König
2020-07-16 12:50 ` [PATCH 3/8] drm/vmwgfx: " Christian König
2020-07-16 12:50 ` [PATCH 4/8] drm/amdgpu: " Christian König
2020-07-16 12:50 ` [PATCH 5/8] drm/nouveau: " Christian König
2020-07-16 12:50 ` [PATCH 6/8] drm/qxl: " Christian König
2020-07-16 12:50 ` [PATCH 7/8] drm/vram-helper: " Christian König
2020-07-16 13:01   ` Thomas Zimmermann
2020-07-16 12:50 ` [PATCH 8/8] drm/ttm: remove TTM_MEMTYPE_FLAG_MAPPABLE Christian König
2020-07-16 13:05 ` [PATCH 1/8] drm/ttm: remove TTM_MEMTYPE_FLAG_CMA Thomas Zimmermann
2020-07-20 11:10   ` 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.