[PATCH 00/10] *** bug fixing and cleanups ***

[PATCH 00/10] *** bug fixing and cleanups ***

[Monk Liu]

patch 3 is to fix kernel driver cannot removed bug (new regression)

patch 1,2,6,7,8,10 are the cleanups to make code more readable and simple

patch 4,5,9 are the fixes for  memory leak bug after driver unloaded

Monk Liu (10):
  drm/amdgpu:cleanup stolen vga memory finish
  drm/amdgpu:cleanup GMC & gart garbage function
  drm/amdgpu:fix NULL pointer access during drv remove
  drm/amdgpu:put reserve_fw_vram_fini to correct place
  drm/amdgpu:fix memleak in takedown
  drm/amdgpu:cleanup unused stack var
  drm/amdgpu:free CSA in unified place
  drm/amdgpu:cleanup firmware.fw_buf alloc/free
  drm/amdgpu:fix memleak
  drm/amdgpu:show error message if fail on event4

 drivers/gpu/drm/amd/amdgpu/amdgpu.h          |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c   |   8 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c     | 151 ++-------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h     |   4 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  |   6 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c      |  15 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c    |  43 ++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c     |   6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h     |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c |   5 -
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c        |   8 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c        |  10 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c        |   9 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c        |   9 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c        |   9 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c        |   9 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c        |   8 +-
 17 files changed, 72 insertions(+), 230 deletions(-)

-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 01/10] drm/amdgpu:cleanup stolen vga memory finish

[Monk Liu]

Change-Id: Id641e125f3dafc54223e49ee444ab64249e7e3a1
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 99e812d..cf21b38 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1640,14 +1640,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 		return;
 	amdgpu_ttm_debugfs_fini(adev);
 
-	if (adev->stolen_vga_memory) {
-		r = amdgpu_bo_reserve(adev->stolen_vga_memory, true);
-		if (r == 0) {
-			amdgpu_bo_unpin(adev->stolen_vga_memory);
-			amdgpu_bo_unreserve(adev->stolen_vga_memory);
-		}
-		amdgpu_bo_unref(&adev->stolen_vga_memory);
-	}
+	if (adev->stolen_vga_memory)
+		amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
+
 	amdgpu_ssg_fini(adev);
 	amdgpu_direct_gma_fini(adev);
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 02/10] drm/amdgpu:cleanup GMC & gart garbage function

[Monk Liu]

for gart_ram_alloc/free, they are never used in driver thus
ripe them out totally.

for gart_vram_pin/unpin, they are not needed becuase we can
use bo_creat_kernel/free to replace the original manual way
in the gart_vram_alloc/free, thus gart_vram_pin/unpin can
also be riped out.

Change-Id: Ic60949b59d8b363e5199f6c11d9ea4a15ec57519
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 136 ++-----------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h |   4 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |   2 -
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c    |   7 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c    |   7 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c    |   7 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |   6 +-
 7 files changed, 13 insertions(+), 156 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index e195e3c..4b5ec15 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -59,63 +59,6 @@
  */
 
 /**
- * amdgpu_gart_table_ram_alloc - allocate system ram for gart page table
- *
- * @adev: amdgpu_device pointer
- *
- * Allocate system memory for GART page table
- * (r1xx-r3xx, non-pcie r4xx, rs400).  These asics require the
- * gart table to be in system memory.
- * Returns 0 for success, -ENOMEM for failure.
- */
-int amdgpu_gart_table_ram_alloc(struct amdgpu_device *adev)
-{
-	void *ptr;
-
-	ptr = pci_alloc_consistent(adev->pdev, adev->gart.table_size,
-				   &adev->gart.table_addr);
-	if (ptr == NULL) {
-		return -ENOMEM;
-	}
-#ifdef CONFIG_X86
-	if (0) {
-		set_memory_uc((unsigned long)ptr,
-			      adev->gart.table_size >> PAGE_SHIFT);
-	}
-#endif
-	adev->gart.ptr = ptr;
-	memset((void *)adev->gart.ptr, 0, adev->gart.table_size);
-	return 0;
-}
-
-/**
- * amdgpu_gart_table_ram_free - free system ram for gart page table
- *
- * @adev: amdgpu_device pointer
- *
- * Free system memory for GART page table
- * (r1xx-r3xx, non-pcie r4xx, rs400).  These asics require the
- * gart table to be in system memory.
- */
-void amdgpu_gart_table_ram_free(struct amdgpu_device *adev)
-{
-	if (adev->gart.ptr == NULL) {
-		return;
-	}
-#ifdef CONFIG_X86
-	if (0) {
-		set_memory_wb((unsigned long)adev->gart.ptr,
-			      adev->gart.table_size >> PAGE_SHIFT);
-	}
-#endif
-	pci_free_consistent(adev->pdev, adev->gart.table_size,
-			    (void *)adev->gart.ptr,
-			    adev->gart.table_addr);
-	adev->gart.ptr = NULL;
-	adev->gart.table_addr = 0;
-}
-
-/**
  * amdgpu_gart_table_vram_alloc - allocate vram for gart page table
  *
  * @adev: amdgpu_device pointer
@@ -127,75 +70,9 @@ void amdgpu_gart_table_ram_free(struct amdgpu_device *adev)
  */
 int amdgpu_gart_table_vram_alloc(struct amdgpu_device *adev)
 {
-	int r;
-
-	if (adev->gart.robj == NULL) {
-		r = amdgpu_bo_create(adev, adev->gart.table_size,
-				     PAGE_SIZE, true, AMDGPU_GEM_DOMAIN_VRAM,
-				     AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
-				     AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
-				     NULL, NULL, 0, &adev->gart.robj);
-		if (r) {
-			return r;
-		}
-	}
-	return 0;
-}
-
-/**
- * amdgpu_gart_table_vram_pin - pin gart page table in vram
- *
- * @adev: amdgpu_device pointer
- *
- * Pin the GART page table in vram so it will not be moved
- * by the memory manager (pcie r4xx, r5xx+).  These asics require the
- * gart table to be in video memory.
- * Returns 0 for success, error for failure.
- */
-int amdgpu_gart_table_vram_pin(struct amdgpu_device *adev)
-{
-	uint64_t gpu_addr;
-	int r;
-
-	r = amdgpu_bo_reserve(adev->gart.robj, false);
-	if (unlikely(r != 0))
-		return r;
-	r = amdgpu_bo_pin(adev->gart.robj,
-				AMDGPU_GEM_DOMAIN_VRAM, &gpu_addr);
-	if (r) {
-		amdgpu_bo_unreserve(adev->gart.robj);
-		return r;
-	}
-	r = amdgpu_bo_kmap(adev->gart.robj, &adev->gart.ptr);
-	if (r)
-		amdgpu_bo_unpin(adev->gart.robj);
-	amdgpu_bo_unreserve(adev->gart.robj);
-	adev->gart.table_addr = gpu_addr;
-	return r;
-}
-
-/**
- * amdgpu_gart_table_vram_unpin - unpin gart page table in vram
- *
- * @adev: amdgpu_device pointer
- *
- * Unpin the GART page table in vram (pcie r4xx, r5xx+).
- * These asics require the gart table to be in video memory.
- */
-void amdgpu_gart_table_vram_unpin(struct amdgpu_device *adev)
-{
-	int r;
-
-	if (adev->gart.robj == NULL) {
-		return;
-	}
-	r = amdgpu_bo_reserve(adev->gart.robj, true);
-	if (likely(r == 0)) {
-		amdgpu_bo_kunmap(adev->gart.robj);
-		amdgpu_bo_unpin(adev->gart.robj);
-		amdgpu_bo_unreserve(adev->gart.robj);
-		adev->gart.ptr = NULL;
-	}
+	return amdgpu_bo_create_kernel(adev, adev->gart.table_size, PAGE_SIZE,
+					AMDGPU_GEM_DOMAIN_VRAM, &adev->gart.robj,
+					&adev->gart.table_addr, &adev->gart.ptr);
 }
 
 /**
@@ -209,10 +86,9 @@ void amdgpu_gart_table_vram_unpin(struct amdgpu_device *adev)
  */
 void amdgpu_gart_table_vram_free(struct amdgpu_device *adev)
 {
-	if (adev->gart.robj == NULL) {
-		return;
-	}
-	amdgpu_bo_unref(&adev->gart.robj);
+	amdgpu_bo_free_kernel(&adev->gart.robj,
+				&adev->gart.table_addr,
+				&adev->gart.ptr);
 }
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
index afbe803..f15e319 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
@@ -56,12 +56,8 @@ struct amdgpu_gart {
 	const struct amdgpu_gart_funcs *gart_funcs;
 };
 
-int amdgpu_gart_table_ram_alloc(struct amdgpu_device *adev);
-void amdgpu_gart_table_ram_free(struct amdgpu_device *adev);
 int amdgpu_gart_table_vram_alloc(struct amdgpu_device *adev);
 void amdgpu_gart_table_vram_free(struct amdgpu_device *adev);
-int amdgpu_gart_table_vram_pin(struct amdgpu_device *adev);
-void amdgpu_gart_table_vram_unpin(struct amdgpu_device *adev);
 int amdgpu_gart_init(struct amdgpu_device *adev);
 void amdgpu_gart_fini(struct amdgpu_device *adev);
 int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index cf21b38..e8401a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1634,8 +1634,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 
 void amdgpu_ttm_fini(struct amdgpu_device *adev)
 {
-	int r;
-
 	if (!adev->mman.initialized)
 		return;
 	amdgpu_ttm_debugfs_fini(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index f4603a7..aac493b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -477,16 +477,14 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
 
 static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
 {
-	int r, i;
+	int i;
 	u32 field;
 
 	if (adev->gart.robj == NULL) {
 		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
 		return -EINVAL;
 	}
-	r = amdgpu_gart_table_vram_pin(adev);
-	if (r)
-		return r;
+
 	/* Setup TLB control */
 	WREG32(mmMC_VM_MX_L1_TLB_CNTL,
 	       (0xA << 7) |
@@ -613,7 +611,6 @@ static void gmc_v6_0_gart_disable(struct amdgpu_device *adev)
 	WREG32(mmVM_L2_CNTL3,
 	       VM_L2_CNTL3__L2_CACHE_BIGK_ASSOCIATIVITY_MASK |
 	       (0UL << VM_L2_CNTL3__L2_CACHE_BIGK_FRAGMENT_SIZE__SHIFT));
-	amdgpu_gart_table_vram_unpin(adev);
 }
 
 static void gmc_v6_0_gart_fini(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 322edfe..dede91a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -582,16 +582,14 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
  */
 static int gmc_v7_0_gart_enable(struct amdgpu_device *adev)
 {
-	int r, i;
+	int i;
 	u32 tmp, field;
 
 	if (adev->gart.robj == NULL) {
 		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
 		return -EINVAL;
 	}
-	r = amdgpu_gart_table_vram_pin(adev);
-	if (r)
-		return r;
+
 	/* Setup TLB control */
 	tmp = RREG32(mmMC_VM_MX_L1_TLB_CNTL);
 	tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 1);
@@ -724,7 +722,6 @@ static void gmc_v7_0_gart_disable(struct amdgpu_device *adev)
 	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
 	WREG32(mmVM_L2_CNTL, tmp);
 	WREG32(mmVM_L2_CNTL2, 0);
-	amdgpu_gart_table_vram_unpin(adev);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index ed6b88f..847c046 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -785,16 +785,14 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
  */
 static int gmc_v8_0_gart_enable(struct amdgpu_device *adev)
 {
-	int r, i;
+	int i;
 	u32 tmp, field;
 
 	if (adev->gart.robj == NULL) {
 		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
 		return -EINVAL;
 	}
-	r = amdgpu_gart_table_vram_pin(adev);
-	if (r)
-		return r;
+
 	/* Setup TLB control */
 	tmp = RREG32(mmMC_VM_MX_L1_TLB_CNTL);
 	tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 1);
@@ -944,7 +942,6 @@ static void gmc_v8_0_gart_disable(struct amdgpu_device *adev)
 	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
 	WREG32(mmVM_L2_CNTL, tmp);
 	WREG32(mmVM_L2_CNTL2, 0);
-	amdgpu_gart_table_vram_unpin(adev);
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 3a855e0..d0acf26 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -651,7 +651,7 @@ static int gmc_v9_0_sw_init(void *handle)
 }
 
 /**
- * gmc_v8_0_gart_fini - vm fini callback
+ * gmc_v9_0_gart_fini - vm fini callback
  *
  * @adev: amdgpu_device pointer
  *
@@ -715,9 +715,6 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
 		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
 		return -EINVAL;
 	}
-	r = amdgpu_gart_table_vram_pin(adev);
-	if (r)
-		return r;
 
 	switch (adev->asic_type) {
 	case CHIP_RAVEN:
@@ -795,7 +792,6 @@ static void gmc_v9_0_gart_disable(struct amdgpu_device *adev)
 {
 	gfxhub_v1_0_gart_disable(adev);
 	mmhub_v1_0_gart_disable(adev);
-	amdgpu_gart_table_vram_unpin(adev);
 }
 
 static int gmc_v9_0_hw_fini(void *handle)
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 03/10] drm/amdgpu:fix NULL pointer access during drv remove

[Monk Liu]

NULL pointer is because original logic will step into
set_pde_pte() even after the gart.ptr is freed due to
there are twice gart_unbind() on all gart area.

also, there are other minor fixes:
1,since gart_init only create dummy page, the corresponding
gart_fini shouldn't do more like unbinding all GART, this is
unnecessary because in driver fini stage all GART unbinding
had already been done during each IP's SW_FINI (GMC's
SW_FINI is the last one called), so remove the step
for the GART unbinding in gart_fini().

2,gart_fini() is already invoked during each GMC IP's gart_fini
routine,e.g. gmc_vx_0_gart_fini(), so no need to manually
call it during ttm_fini().

3,amdgpu_gem_force_release() should be put ahead of
amdgpu_vm_manager_fini()

Change-Id: Ib1f31a2cf6bfbb9b54c7cc2a8cf9e4e3160bcfa0
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 15 +++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |  1 -
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  2 +-
 6 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
index 4b5ec15..c4eef4a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
@@ -255,10 +255,8 @@ int amdgpu_gart_init(struct amdgpu_device *adev)
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
 	/* Allocate pages table */
 	adev->gart.pages = vzalloc(sizeof(void *) * adev->gart.num_cpu_pages);
-	if (adev->gart.pages == NULL) {
-		amdgpu_gart_fini(adev);
+	if (adev->gart.pages == NULL)
 		return -ENOMEM;
-	}
 #endif
 
 	return 0;
@@ -273,14 +271,11 @@ int amdgpu_gart_init(struct amdgpu_device *adev)
  */
 void amdgpu_gart_fini(struct amdgpu_device *adev)
 {
-	if (adev->gart.ready) {
-		/* unbind pages */
-		amdgpu_gart_unbind(adev, 0, adev->gart.num_cpu_pages);
-	}
-	adev->gart.ready = false;
 #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
-	vfree(adev->gart.pages);
-	adev->gart.pages = NULL;
+	if (adev->gart.pages) {
+		vfree(adev->gart.pages);
+		adev->gart.pages = NULL;
+	}
 #endif
 	amdgpu_dummy_page_fini(adev);
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e8401a9..615b849 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1653,7 +1653,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 	if (adev->gds.oa.total_size)
 		ttm_bo_clean_mm(&adev->mman.bdev, AMDGPU_PL_OA);
 	ttm_bo_device_release(&adev->mman.bdev);
-	amdgpu_gart_fini(adev);
 	amdgpu_ttm_global_fini(adev);
 	adev->mman.initialized = false;
 	DRM_INFO("amdgpu: ttm finalized\n");
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index aac493b..6112dfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -894,9 +894,9 @@ static int gmc_v6_0_sw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	amdgpu_gem_force_release(adev);
 	amdgpu_vm_manager_fini(adev);
 	gmc_v6_0_gart_fini(adev);
-	amdgpu_gem_force_release(adev);
 	amdgpu_bo_fini(adev);
 	release_firmware(adev->mc.fw);
 	adev->mc.fw = NULL;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index dede91a..b57dc06 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -1067,9 +1067,9 @@ static int gmc_v7_0_sw_fini(void *handle)
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
 	kfree(adev->mc.vm_fault_info);
+	amdgpu_gem_force_release(adev);
 	amdgpu_vm_manager_fini(adev);
 	gmc_v7_0_gart_fini(adev);
-	amdgpu_gem_force_release(adev);
 	amdgpu_bo_fini(adev);
 	release_firmware(adev->mc.fw);
 	adev->mc.fw = NULL;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 847c046..5be4854 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1168,9 +1168,9 @@ static int gmc_v8_0_sw_fini(void *handle)
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
 	kfree(adev->mc.vm_fault_info);
+	amdgpu_gem_force_release(adev);
 	amdgpu_vm_manager_fini(adev);
 	gmc_v8_0_gart_fini(adev);
-	amdgpu_gem_force_release(adev);
 	amdgpu_bo_fini(adev);
 	release_firmware(adev->mc.fw);
 	adev->mc.fw = NULL;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index d0acf26..bea0774 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -667,9 +667,9 @@ static int gmc_v9_0_sw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	amdgpu_gem_force_release(adev);
 	amdgpu_vm_manager_fini(adev);
 	gmc_v9_0_gart_fini(adev);
-	amdgpu_gem_force_release(adev);
 	amdgpu_bo_fini(adev);
 
 	return 0;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 04/10] drm/amdgpu:put reserve_fw_vram_fini to correct place

[Monk Liu]

Move "reserve_fw_vram_fini" to the correct place
which should in amdgpu_ttm_fini() since reserve_fw_vram_init is
in amdgpu_ttm_init().

this can fix error report and memory leak in "vram_mgr_fini()".

Change-Id: I4b1dc7eecda62407f30a5a16f59555eee49f04a3
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 04a0a75..10735e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1459,6 +1459,7 @@ struct amdgpu_fw_vram_usage {
 };
 
 int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev);
+void amdgpu_fw_reserve_vram_fini(struct amdgpu_device *adev);
 
 /*
  * CGS
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index aa678ff..9471f47 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2479,7 +2479,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 	/* evict vram memory */
 	amdgpu_bo_evict_vram(adev);
 	amdgpu_ib_pool_fini(adev);
-	amdgpu_fw_reserve_vram_fini(adev);
 	amdgpu_fence_driver_fini(adev);
 	amdgpu_fbdev_fini(adev);
 	r = amdgpu_fini(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 615b849..03a5e44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1643,6 +1643,7 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 
 	amdgpu_ssg_fini(adev);
 	amdgpu_direct_gma_fini(adev);
+	amdgpu_fw_reserve_vram_fini(adev);
 
 	ttm_bo_clean_mm(&adev->mman.bdev, TTM_PL_VRAM);
 	ttm_bo_clean_mm(&adev->mman.bdev, TTM_PL_TT);
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 05/10] drm/amdgpu:fix memleak in takedown

[Monk Liu]

this can fix the memory leak under the case that not all
BO are freed during "takedown" stage, because originally
it blocks following kfree on mgr.

Change-Id: I56e832ac317215a4d9f58edb8e75ea722158715c
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 6 ------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 -----
 2 files changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 88709f9..72819cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -76,12 +76,6 @@ static int amdgpu_gtt_mgr_fini(struct ttm_mem_type_manager *man)
 {
 	struct amdgpu_gtt_mgr *mgr = man->priv;
 
-	spin_lock(&mgr->lock);
-	if (!drm_mm_clean(&mgr->mm)) {
-		spin_unlock(&mgr->lock);
-		return -EBUSY;
-	}
-
 	drm_mm_takedown(&mgr->mm);
 	spin_unlock(&mgr->lock);
 	kfree(mgr);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 4ca622b..3e883e5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -68,11 +68,6 @@ static int amdgpu_vram_mgr_fini(struct ttm_mem_type_manager *man)
 	struct amdgpu_vram_mgr *mgr = man->priv;
 
 	spin_lock(&mgr->lock);
-	if (!drm_mm_clean(&mgr->mm)) {
-		spin_unlock(&mgr->lock);
-		return -EBUSY;
-	}
-
 	drm_mm_takedown(&mgr->mm);
 	spin_unlock(&mgr->lock);
 	kfree(mgr);
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 06/10] drm/amdgpu:cleanup unused stack var

[Monk Liu]

Change-Id: I5d3ea5facc23bcc94806a098ae9455ce1564fa09
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 9471f47..c13b493 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -681,7 +681,6 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev)
 {
 	int r = 0;
 	int i;
-	u64 gpu_addr;
 	u64 vram_size = adev->mc.visible_vram_size;
 	u64 offset = adev->fw_vram_usage.start_offset;
 	u64 size = adev->fw_vram_usage.size;
@@ -725,7 +724,7 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev)
 			AMDGPU_GEM_DOMAIN_VRAM,
 			adev->fw_vram_usage.start_offset,
 			(adev->fw_vram_usage.start_offset +
-			adev->fw_vram_usage.size), &gpu_addr);
+			adev->fw_vram_usage.size), NULL);
 		if (r)
 			goto error_pin;
 		r = amdgpu_bo_kmap(adev->fw_vram_usage.reserved_bo,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 07/10] drm/amdgpu:free CSA in unified place

[Monk Liu]

instead of doing it in each GFX ip's sw_fini

Change-Id: Idf0fd500d4fc385cf7a930cc56305070c937bf20
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 6 ++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   | 1 +
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      | 1 -
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 1 -
 5 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index c13b493..ccb33ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1865,6 +1865,7 @@ static int amdgpu_fini(struct amdgpu_device *adev)
 		if (!adev->ip_blocks[i].status.hw)
 			continue;
 		if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) {
+			amdgpu_free_static_csa(adev);
 			amdgpu_wb_fini(adev);
 			amdgpu_vram_scratch_fini(adev);
 		}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 67fd110..118c84c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -47,6 +47,12 @@ int amdgpu_allocate_static_csa(struct amdgpu_device *adev)
 	return 0;
 }
 
+void amdgpu_free_static_csa(struct amdgpu_device *adev) {
+	amdgpu_bo_free_kernel(&adev->virt.csa_obj,
+						&adev->virt.csa_vmid0_addr,
+						NULL);
+}
+
 /*
  * amdgpu_map_static_csa should be called during amdgpu_vm_init
  * it maps virtual address "AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index f77d116..6a83425 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -283,6 +283,7 @@ bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
 int amdgpu_allocate_static_csa(struct amdgpu_device *adev);
 int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			  struct amdgpu_bo_va **bo_va);
+void amdgpu_free_static_csa(struct amdgpu_device *adev);
 void amdgpu_virt_init_setting(struct amdgpu_device *adev);
 uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
 void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 9f5d123..82d157e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -2114,7 +2114,6 @@ static int gfx_v8_0_sw_fini(void *handle)
 	amdgpu_gfx_compute_mqd_sw_fini(adev);
 	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
 	amdgpu_gfx_kiq_fini(adev);
-	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
 
 	gfx_v8_0_mec_fini(adev);
 	gfx_v8_0_rlc_fini(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 5a4c074..034bcbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1455,7 +1455,6 @@ static int gfx_v9_0_sw_fini(void *handle)
 	amdgpu_gfx_compute_mqd_sw_fini(adev);
 	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
 	amdgpu_gfx_kiq_fini(adev);
-	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
 
 	gfx_v9_0_mec_fini(adev);
 	gfx_v9_0_ngg_fini(adev);
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 08/10] drm/amdgpu:cleanup firmware.fw_buf alloc/free

[Monk Liu]

use bo_create/free_kernel instead of manually doing it

Change-Id: I128651d2b2207f6e16b484b050f3232c08b53de3
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 43 +++++++------------------------
 1 file changed, 9 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
index ab9b2d4..474f88f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
@@ -359,7 +359,6 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
 
 int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
 {
-	struct amdgpu_bo **bo = &adev->firmware.fw_buf;
 	uint64_t fw_offset = 0;
 	int i, err;
 	struct amdgpu_firmware_info *ucode = NULL;
@@ -371,35 +370,15 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
 	}
 
 	if (!adev->in_gpu_reset) {
-		err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true,
+		err = amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
 					amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
-					AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS|AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED,
-					NULL, NULL, 0, bo);
+					&adev->firmware.fw_buf,
+					&adev->firmware.fw_buf_mc,
+					&adev->firmware.fw_buf_ptr);
 		if (err) {
-			dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err);
+			dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n");
 			goto failed;
 		}
-
-		err = amdgpu_bo_reserve(*bo, false);
-		if (err) {
-			dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err);
-			goto failed_reserve;
-		}
-
-		err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
-					&adev->firmware.fw_buf_mc);
-		if (err) {
-			dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err);
-			goto failed_pin;
-		}
-
-		err = amdgpu_bo_kmap(*bo, &adev->firmware.fw_buf_ptr);
-		if (err) {
-			dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err);
-			goto failed_kmap;
-		}
-
-		amdgpu_bo_unreserve(*bo);
 	}
 
 	memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
@@ -436,12 +415,6 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
 	}
 	return 0;
 
-failed_kmap:
-	amdgpu_bo_unpin(*bo);
-failed_pin:
-	amdgpu_bo_unreserve(*bo);
-failed_reserve:
-	amdgpu_bo_unref(bo);
 failed:
 	if (err)
 		adev->firmware.load_type = AMDGPU_FW_LOAD_DIRECT;
@@ -464,8 +437,10 @@ int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
 			ucode->kaddr = NULL;
 		}
 	}
-	amdgpu_bo_unref(&adev->firmware.fw_buf);
-	adev->firmware.fw_buf = NULL;
+
+	amdgpu_bo_free_kernel(&adev->firmware.fw_buf,
+				&adev->firmware.fw_buf_mc,
+				&adev->firmware.fw_buf_ptr);
 
 	return 0;
 }
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 09/10] drm/amdgpu:fix memleak

[Monk Liu]

those RLC used buffers are not cleared in GFX's sw_fini

Change-Id: I0c9204753caa327cfbabdfb1465d2c03012ee977
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 8 ++++++++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 9 +++++++++
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 8 ++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 887f196..b6f94c6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -4672,6 +4672,14 @@ static int gfx_v7_0_sw_fini(void *handle)
 	gfx_v7_0_cp_compute_fini(adev);
 	gfx_v7_0_rlc_fini(adev);
 	gfx_v7_0_mec_fini(adev);
+	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
+				&adev->gfx.rlc.clear_state_gpu_addr,
+				(void **)&adev->gfx.rlc.cs_ptr);
+	if (adev->gfx.rlc.cp_table_size) {
+		amdgpu_bo_free_kernel(&adev->gfx.rlc.cp_table_obj,
+				&adev->gfx.rlc.cp_table_gpu_addr,
+				(void **)&adev->gfx.rlc.cp_table_ptr);
+	}
 	gfx_v7_0_free_microcode(adev);
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 82d157e..457af4a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -2117,6 +2117,15 @@ static int gfx_v8_0_sw_fini(void *handle)
 
 	gfx_v8_0_mec_fini(adev);
 	gfx_v8_0_rlc_fini(adev);
+	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
+				&adev->gfx.rlc.clear_state_gpu_addr,
+				(void **)&adev->gfx.rlc.cs_ptr);
+	if ((adev->asic_type == CHIP_CARRIZO) ||
+	    (adev->asic_type == CHIP_STONEY)) {
+		amdgpu_bo_free_kernel(&adev->gfx.rlc.cp_table_obj,
+				&adev->gfx.rlc.cp_table_gpu_addr,
+				(void **)&adev->gfx.rlc.cp_table_ptr);
+	}
 	gfx_v8_0_free_microcode(adev);
 
 	return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 034bcbe..4214988 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1458,6 +1458,14 @@ static int gfx_v9_0_sw_fini(void *handle)
 
 	gfx_v9_0_mec_fini(adev);
 	gfx_v9_0_ngg_fini(adev);
+	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
+				&adev->gfx.rlc.clear_state_gpu_addr,
+				(void **)&adev->gfx.rlc.cs_ptr);
+	if (adev->asic_type == CHIP_RAVEN) {
+		amdgpu_bo_free_kernel(&adev->gfx.rlc.cp_table_obj,
+				&adev->gfx.rlc.cp_table_gpu_addr,
+				(void **)&adev->gfx.rlc.cp_table_ptr);
+	}
 	gfx_v9_0_free_microcode(adev);
 
 	return 0;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 10/10] drm/amdgpu:show error message if fail on event4

[Monk Liu]

Change-Id: I984697748672421a05f179a7bf4eeb0255b28bdc
Signed-off-by: Monk Liu <Monk.Liu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index ccb33ef..35a1593 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1914,7 +1914,8 @@ static int amdgpu_fini(struct amdgpu_device *adev)
 	}
 
 	if (amdgpu_sriov_vf(adev))
-		amdgpu_virt_release_full_gpu(adev, false);
+		if (amdgpu_virt_release_full_gpu(adev, false))
+			DRM_ERROR("failed to release exclusive mode on fini\n");
 
 	return 0;
 }
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 02/10] drm/amdgpu:cleanup GMC & gart garbage function

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> for gart_ram_alloc/free, they are never used in driver thus
> ripe them out totally.
>
> for gart_vram_pin/unpin, they are not needed becuase we can
> use bo_creat_kernel/free to replace the original manual way
> in the gart_vram_alloc/free, thus gart_vram_pin/unpin can
> also be riped out.
>
> Change-Id: Ic60949b59d8b363e5199f6c11d9ea4a15ec57519
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

Nice, wanted to do this for a long time as well.

Patch is Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 136 ++-----------------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h |   4 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |   2 -
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c    |   7 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c    |   7 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c    |   7 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |   6 +-
>   7 files changed, 13 insertions(+), 156 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index e195e3c..4b5ec15 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -59,63 +59,6 @@
>    */
>   
>   /**
> - * amdgpu_gart_table_ram_alloc - allocate system ram for gart page table
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Allocate system memory for GART page table
> - * (r1xx-r3xx, non-pcie r4xx, rs400).  These asics require the
> - * gart table to be in system memory.
> - * Returns 0 for success, -ENOMEM for failure.
> - */
> -int amdgpu_gart_table_ram_alloc(struct amdgpu_device *adev)
> -{
> -	void *ptr;
> -
> -	ptr = pci_alloc_consistent(adev->pdev, adev->gart.table_size,
> -				   &adev->gart.table_addr);
> -	if (ptr == NULL) {
> -		return -ENOMEM;
> -	}
> -#ifdef CONFIG_X86
> -	if (0) {
> -		set_memory_uc((unsigned long)ptr,
> -			      adev->gart.table_size >> PAGE_SHIFT);
> -	}
> -#endif
> -	adev->gart.ptr = ptr;
> -	memset((void *)adev->gart.ptr, 0, adev->gart.table_size);
> -	return 0;
> -}
> -
> -/**
> - * amdgpu_gart_table_ram_free - free system ram for gart page table
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Free system memory for GART page table
> - * (r1xx-r3xx, non-pcie r4xx, rs400).  These asics require the
> - * gart table to be in system memory.
> - */
> -void amdgpu_gart_table_ram_free(struct amdgpu_device *adev)
> -{
> -	if (adev->gart.ptr == NULL) {
> -		return;
> -	}
> -#ifdef CONFIG_X86
> -	if (0) {
> -		set_memory_wb((unsigned long)adev->gart.ptr,
> -			      adev->gart.table_size >> PAGE_SHIFT);
> -	}
> -#endif
> -	pci_free_consistent(adev->pdev, adev->gart.table_size,
> -			    (void *)adev->gart.ptr,
> -			    adev->gart.table_addr);
> -	adev->gart.ptr = NULL;
> -	adev->gart.table_addr = 0;
> -}
> -
> -/**
>    * amdgpu_gart_table_vram_alloc - allocate vram for gart page table
>    *
>    * @adev: amdgpu_device pointer
> @@ -127,75 +70,9 @@ void amdgpu_gart_table_ram_free(struct amdgpu_device *adev)
>    */
>   int amdgpu_gart_table_vram_alloc(struct amdgpu_device *adev)
>   {
> -	int r;
> -
> -	if (adev->gart.robj == NULL) {
> -		r = amdgpu_bo_create(adev, adev->gart.table_size,
> -				     PAGE_SIZE, true, AMDGPU_GEM_DOMAIN_VRAM,
> -				     AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
> -				     AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS,
> -				     NULL, NULL, 0, &adev->gart.robj);
> -		if (r) {
> -			return r;
> -		}
> -	}
> -	return 0;
> -}
> -
> -/**
> - * amdgpu_gart_table_vram_pin - pin gart page table in vram
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Pin the GART page table in vram so it will not be moved
> - * by the memory manager (pcie r4xx, r5xx+).  These asics require the
> - * gart table to be in video memory.
> - * Returns 0 for success, error for failure.
> - */
> -int amdgpu_gart_table_vram_pin(struct amdgpu_device *adev)
> -{
> -	uint64_t gpu_addr;
> -	int r;
> -
> -	r = amdgpu_bo_reserve(adev->gart.robj, false);
> -	if (unlikely(r != 0))
> -		return r;
> -	r = amdgpu_bo_pin(adev->gart.robj,
> -				AMDGPU_GEM_DOMAIN_VRAM, &gpu_addr);
> -	if (r) {
> -		amdgpu_bo_unreserve(adev->gart.robj);
> -		return r;
> -	}
> -	r = amdgpu_bo_kmap(adev->gart.robj, &adev->gart.ptr);
> -	if (r)
> -		amdgpu_bo_unpin(adev->gart.robj);
> -	amdgpu_bo_unreserve(adev->gart.robj);
> -	adev->gart.table_addr = gpu_addr;
> -	return r;
> -}
> -
> -/**
> - * amdgpu_gart_table_vram_unpin - unpin gart page table in vram
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Unpin the GART page table in vram (pcie r4xx, r5xx+).
> - * These asics require the gart table to be in video memory.
> - */
> -void amdgpu_gart_table_vram_unpin(struct amdgpu_device *adev)
> -{
> -	int r;
> -
> -	if (adev->gart.robj == NULL) {
> -		return;
> -	}
> -	r = amdgpu_bo_reserve(adev->gart.robj, true);
> -	if (likely(r == 0)) {
> -		amdgpu_bo_kunmap(adev->gart.robj);
> -		amdgpu_bo_unpin(adev->gart.robj);
> -		amdgpu_bo_unreserve(adev->gart.robj);
> -		adev->gart.ptr = NULL;
> -	}
> +	return amdgpu_bo_create_kernel(adev, adev->gart.table_size, PAGE_SIZE,
> +					AMDGPU_GEM_DOMAIN_VRAM, &adev->gart.robj,
> +					&adev->gart.table_addr, &adev->gart.ptr);
>   }
>   
>   /**
> @@ -209,10 +86,9 @@ void amdgpu_gart_table_vram_unpin(struct amdgpu_device *adev)
>    */
>   void amdgpu_gart_table_vram_free(struct amdgpu_device *adev)
>   {
> -	if (adev->gart.robj == NULL) {
> -		return;
> -	}
> -	amdgpu_bo_unref(&adev->gart.robj);
> +	amdgpu_bo_free_kernel(&adev->gart.robj,
> +				&adev->gart.table_addr,
> +				&adev->gart.ptr);
>   }
>   
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
> index afbe803..f15e319 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.h
> @@ -56,12 +56,8 @@ struct amdgpu_gart {
>   	const struct amdgpu_gart_funcs *gart_funcs;
>   };
>   
> -int amdgpu_gart_table_ram_alloc(struct amdgpu_device *adev);
> -void amdgpu_gart_table_ram_free(struct amdgpu_device *adev);
>   int amdgpu_gart_table_vram_alloc(struct amdgpu_device *adev);
>   void amdgpu_gart_table_vram_free(struct amdgpu_device *adev);
> -int amdgpu_gart_table_vram_pin(struct amdgpu_device *adev);
> -void amdgpu_gart_table_vram_unpin(struct amdgpu_device *adev);
>   int amdgpu_gart_init(struct amdgpu_device *adev);
>   void amdgpu_gart_fini(struct amdgpu_device *adev);
>   int amdgpu_gart_unbind(struct amdgpu_device *adev, uint64_t offset,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index cf21b38..e8401a9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1634,8 +1634,6 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   
>   void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   {
> -	int r;
> -
>   	if (!adev->mman.initialized)
>   		return;
>   	amdgpu_ttm_debugfs_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index f4603a7..aac493b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -477,16 +477,14 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable)
>   
>   static int gmc_v6_0_gart_enable(struct amdgpu_device *adev)
>   {
> -	int r, i;
> +	int i;
>   	u32 field;
>   
>   	if (adev->gart.robj == NULL) {
>   		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
>   		return -EINVAL;
>   	}
> -	r = amdgpu_gart_table_vram_pin(adev);
> -	if (r)
> -		return r;
> +
>   	/* Setup TLB control */
>   	WREG32(mmMC_VM_MX_L1_TLB_CNTL,
>   	       (0xA << 7) |
> @@ -613,7 +611,6 @@ static void gmc_v6_0_gart_disable(struct amdgpu_device *adev)
>   	WREG32(mmVM_L2_CNTL3,
>   	       VM_L2_CNTL3__L2_CACHE_BIGK_ASSOCIATIVITY_MASK |
>   	       (0UL << VM_L2_CNTL3__L2_CACHE_BIGK_FRAGMENT_SIZE__SHIFT));
> -	amdgpu_gart_table_vram_unpin(adev);
>   }
>   
>   static void gmc_v6_0_gart_fini(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 322edfe..dede91a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -582,16 +582,14 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable)
>    */
>   static int gmc_v7_0_gart_enable(struct amdgpu_device *adev)
>   {
> -	int r, i;
> +	int i;
>   	u32 tmp, field;
>   
>   	if (adev->gart.robj == NULL) {
>   		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
>   		return -EINVAL;
>   	}
> -	r = amdgpu_gart_table_vram_pin(adev);
> -	if (r)
> -		return r;
> +
>   	/* Setup TLB control */
>   	tmp = RREG32(mmMC_VM_MX_L1_TLB_CNTL);
>   	tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 1);
> @@ -724,7 +722,6 @@ static void gmc_v7_0_gart_disable(struct amdgpu_device *adev)
>   	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
>   	WREG32(mmVM_L2_CNTL, tmp);
>   	WREG32(mmVM_L2_CNTL2, 0);
> -	amdgpu_gart_table_vram_unpin(adev);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index ed6b88f..847c046 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -785,16 +785,14 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable)
>    */
>   static int gmc_v8_0_gart_enable(struct amdgpu_device *adev)
>   {
> -	int r, i;
> +	int i;
>   	u32 tmp, field;
>   
>   	if (adev->gart.robj == NULL) {
>   		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
>   		return -EINVAL;
>   	}
> -	r = amdgpu_gart_table_vram_pin(adev);
> -	if (r)
> -		return r;
> +
>   	/* Setup TLB control */
>   	tmp = RREG32(mmMC_VM_MX_L1_TLB_CNTL);
>   	tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 1);
> @@ -944,7 +942,6 @@ static void gmc_v8_0_gart_disable(struct amdgpu_device *adev)
>   	tmp = REG_SET_FIELD(tmp, VM_L2_CNTL, ENABLE_L2_CACHE, 0);
>   	WREG32(mmVM_L2_CNTL, tmp);
>   	WREG32(mmVM_L2_CNTL2, 0);
> -	amdgpu_gart_table_vram_unpin(adev);
>   }
>   
>   /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 3a855e0..d0acf26 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -651,7 +651,7 @@ static int gmc_v9_0_sw_init(void *handle)
>   }
>   
>   /**
> - * gmc_v8_0_gart_fini - vm fini callback
> + * gmc_v9_0_gart_fini - vm fini callback
>    *
>    * @adev: amdgpu_device pointer
>    *
> @@ -715,9 +715,6 @@ static int gmc_v9_0_gart_enable(struct amdgpu_device *adev)
>   		dev_err(adev->dev, "No VRAM object for PCIE GART.\n");
>   		return -EINVAL;
>   	}
> -	r = amdgpu_gart_table_vram_pin(adev);
> -	if (r)
> -		return r;
>   
>   	switch (adev->asic_type) {
>   	case CHIP_RAVEN:
> @@ -795,7 +792,6 @@ static void gmc_v9_0_gart_disable(struct amdgpu_device *adev)
>   {
>   	gfxhub_v1_0_gart_disable(adev);
>   	mmhub_v1_0_gart_disable(adev);
> -	amdgpu_gart_table_vram_unpin(adev);
>   }
>   
>   static int gmc_v9_0_hw_fini(void *handle)


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 01/10] drm/amdgpu:cleanup stolen vga memory finish

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> Change-Id: Id641e125f3dafc54223e49ee444ab64249e7e3a1
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 11 +++--------
>   1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 99e812d..cf21b38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1640,14 +1640,9 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   		return;
>   	amdgpu_ttm_debugfs_fini(adev);
>   
> -	if (adev->stolen_vga_memory) {
> -		r = amdgpu_bo_reserve(adev->stolen_vga_memory, true);
> -		if (r == 0) {
> -			amdgpu_bo_unpin(adev->stolen_vga_memory);
> -			amdgpu_bo_unreserve(adev->stolen_vga_memory);
> -		}
> -		amdgpu_bo_unref(&adev->stolen_vga_memory);
> -	}
> +	if (adev->stolen_vga_memory)
> +		amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
> +

I think you can even drop the "if" here, since amdgpu_bo_free_kernel() 
checks that anyway.

Either way just a minor difference, patch is Reviewed-by: Christian 
König <christian.koenig@amd.com>.

Regards,
Christian.

>   	amdgpu_ssg_fini(adev);
>   	amdgpu_direct_gma_fini(adev);
>   


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 03/10] drm/amdgpu:fix NULL pointer access during drv remove

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> NULL pointer is because original logic will step into
> set_pde_pte() even after the gart.ptr is freed due to
> there are twice gart_unbind() on all gart area.
>
> also, there are other minor fixes:
> 1,since gart_init only create dummy page, the corresponding
> gart_fini shouldn't do more like unbinding all GART, this is
> unnecessary because in driver fini stage all GART unbinding
> had already been done during each IP's SW_FINI (GMC's
> SW_FINI is the last one called), so remove the step
> for the GART unbinding in gart_fini().
>
> 2,gart_fini() is already invoked during each GMC IP's gart_fini
> routine,e.g. gmc_vx_0_gart_fini(), so no need to manually
> call it during ttm_fini().
>
> 3,amdgpu_gem_force_release() should be put ahead of
> amdgpu_vm_manager_fini()
>
> Change-Id: Ib1f31a2cf6bfbb9b54c7cc2a8cf9e4e3160bcfa0
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 15 +++++----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |  1 -
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  2 +-
>   6 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index 4b5ec15..c4eef4a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -255,10 +255,8 @@ int amdgpu_gart_init(struct amdgpu_device *adev)
>   #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
>   	/* Allocate pages table */
>   	adev->gart.pages = vzalloc(sizeof(void *) * adev->gart.num_cpu_pages);
> -	if (adev->gart.pages == NULL) {
> -		amdgpu_gart_fini(adev);
> +	if (adev->gart.pages == NULL)
>   		return -ENOMEM;
> -	}
>   #endif
>   
>   	return 0;
> @@ -273,14 +271,11 @@ int amdgpu_gart_init(struct amdgpu_device *adev)
>    */
>   void amdgpu_gart_fini(struct amdgpu_device *adev)
>   {
> -	if (adev->gart.ready) {
> -		/* unbind pages */
> -		amdgpu_gart_unbind(adev, 0, adev->gart.num_cpu_pages);
> -	}
> -	adev->gart.ready = false;
>   #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
> -	vfree(adev->gart.pages);
> -	adev->gart.pages = NULL;
> +	if (adev->gart.pages) {
> +		vfree(adev->gart.pages);
> +		adev->gart.pages = NULL;
> +	}

vfree() is NULL save, so extra checks are usually complained about by 
automated testers.

I would just drop this hunk, the rest of the patch looks good to me.

Christian.

>   #endif
>   	amdgpu_dummy_page_fini(adev);
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e8401a9..615b849 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1653,7 +1653,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   	if (adev->gds.oa.total_size)
>   		ttm_bo_clean_mm(&adev->mman.bdev, AMDGPU_PL_OA);
>   	ttm_bo_device_release(&adev->mman.bdev);
> -	amdgpu_gart_fini(adev);
>   	amdgpu_ttm_global_fini(adev);
>   	adev->mman.initialized = false;
>   	DRM_INFO("amdgpu: ttm finalized\n");
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index aac493b..6112dfc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -894,9 +894,9 @@ static int gmc_v6_0_sw_fini(void *handle)
>   {
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v6_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   	release_firmware(adev->mc.fw);
>   	adev->mc.fw = NULL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index dede91a..b57dc06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -1067,9 +1067,9 @@ static int gmc_v7_0_sw_fini(void *handle)
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
>   	kfree(adev->mc.vm_fault_info);
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v7_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   	release_firmware(adev->mc.fw);
>   	adev->mc.fw = NULL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 847c046..5be4854 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -1168,9 +1168,9 @@ static int gmc_v8_0_sw_fini(void *handle)
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
>   	kfree(adev->mc.vm_fault_info);
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v8_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   	release_firmware(adev->mc.fw);
>   	adev->mc.fw = NULL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index d0acf26..bea0774 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -667,9 +667,9 @@ static int gmc_v9_0_sw_fini(void *handle)
>   {
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v9_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   
>   	return 0;


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 04/10] drm/amdgpu:put reserve_fw_vram_fini to correct place

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> Move "reserve_fw_vram_fini" to the correct place
> which should in amdgpu_ttm_fini() since reserve_fw_vram_init is
> in amdgpu_ttm_init().
>
> this can fix error report and memory leak in "vram_mgr_fini()".
>
> Change-Id: I4b1dc7eecda62407f30a5a16f59555eee49f04a3
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    | 1 +
>   3 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 04a0a75..10735e8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1459,6 +1459,7 @@ struct amdgpu_fw_vram_usage {
>   };
>   
>   int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev);
> +void amdgpu_fw_reserve_vram_fini(struct amdgpu_device *adev);
>   
>   /*
>    * CGS
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index aa678ff..9471f47 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2479,7 +2479,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>   	/* evict vram memory */
>   	amdgpu_bo_evict_vram(adev);
>   	amdgpu_ib_pool_fini(adev);
> -	amdgpu_fw_reserve_vram_fini(adev);
>   	amdgpu_fence_driver_fini(adev);
>   	amdgpu_fbdev_fini(adev);
>   	r = amdgpu_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 615b849..03a5e44 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1643,6 +1643,7 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   
>   	amdgpu_ssg_fini(adev);
>   	amdgpu_direct_gma_fini(adev);
> +	amdgpu_fw_reserve_vram_fini(adev);
>   
>   	ttm_bo_clean_mm(&adev->mman.bdev, TTM_PL_VRAM);
>   	ttm_bo_clean_mm(&adev->mman.bdev, TTM_PL_TT);


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 05/10] drm/amdgpu:fix memleak in takedown

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> this can fix the memory leak under the case that not all
> BO are freed during "takedown" stage, because originally
> it blocks following kfree on mgr.

NAK, all BOs must be freed before we take down at least the GTT manager.

That we run into a memory leak is intended, cause that is still better 
than crashing the system because we access already freed up memory.

Regards,
Christian.

>
> Change-Id: I56e832ac317215a4d9f58edb8e75ea722158715c
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 6 ------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 -----
>   2 files changed, 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> index 88709f9..72819cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
> @@ -76,12 +76,6 @@ static int amdgpu_gtt_mgr_fini(struct ttm_mem_type_manager *man)
>   {
>   	struct amdgpu_gtt_mgr *mgr = man->priv;
>   
> -	spin_lock(&mgr->lock);
> -	if (!drm_mm_clean(&mgr->mm)) {
> -		spin_unlock(&mgr->lock);
> -		return -EBUSY;
> -	}
> -
>   	drm_mm_takedown(&mgr->mm);
>   	spin_unlock(&mgr->lock);
>   	kfree(mgr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 4ca622b..3e883e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -68,11 +68,6 @@ static int amdgpu_vram_mgr_fini(struct ttm_mem_type_manager *man)
>   	struct amdgpu_vram_mgr *mgr = man->priv;
>   
>   	spin_lock(&mgr->lock);
> -	if (!drm_mm_clean(&mgr->mm)) {
> -		spin_unlock(&mgr->lock);
> -		return -EBUSY;
> -	}
> -
>   	drm_mm_takedown(&mgr->mm);
>   	spin_unlock(&mgr->lock);
>   	kfree(mgr);


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 06/10] drm/amdgpu:cleanup unused stack var

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> Change-Id: I5d3ea5facc23bcc94806a098ae9455ce1564fa09
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 9471f47..c13b493 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -681,7 +681,6 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev)
>   {
>   	int r = 0;
>   	int i;
> -	u64 gpu_addr;
>   	u64 vram_size = adev->mc.visible_vram_size;
>   	u64 offset = adev->fw_vram_usage.start_offset;
>   	u64 size = adev->fw_vram_usage.size;
> @@ -725,7 +724,7 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev)
>   			AMDGPU_GEM_DOMAIN_VRAM,
>   			adev->fw_vram_usage.start_offset,
>   			(adev->fw_vram_usage.start_offset +
> -			adev->fw_vram_usage.size), &gpu_addr);
> +			adev->fw_vram_usage.size), NULL);
>   		if (r)
>   			goto error_pin;
>   		r = amdgpu_bo_kmap(adev->fw_vram_usage.reserved_bo,


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 07/10] drm/amdgpu:free CSA in unified place

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> instead of doing it in each GFX ip's sw_fini

Mhm, is that allocated in the same way as well?

>
> Change-Id: Idf0fd500d4fc385cf7a930cc56305070c937bf20
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   | 1 +
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      | 1 -
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 1 -
>   5 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c13b493..ccb33ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1865,6 +1865,7 @@ static int amdgpu_fini(struct amdgpu_device *adev)
>   		if (!adev->ip_blocks[i].status.hw)
>   			continue;
>   		if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) {
> +			amdgpu_free_static_csa(adev);
>   			amdgpu_wb_fini(adev);
>   			amdgpu_vram_scratch_fini(adev);
>   		}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 67fd110..118c84c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -47,6 +47,12 @@ int amdgpu_allocate_static_csa(struct amdgpu_device *adev)
>   	return 0;
>   }
>   
> +void amdgpu_free_static_csa(struct amdgpu_device *adev) {
> +	amdgpu_bo_free_kernel(&adev->virt.csa_obj,
> +						&adev->virt.csa_vmid0_addr,
> +						NULL);

The coding style here looks odd.

Christian.

> +}
> +
>   /*
>    * amdgpu_map_static_csa should be called during amdgpu_vm_init
>    * it maps virtual address "AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index f77d116..6a83425 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -283,6 +283,7 @@ bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
>   int amdgpu_allocate_static_csa(struct amdgpu_device *adev);
>   int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			  struct amdgpu_bo_va **bo_va);
> +void amdgpu_free_static_csa(struct amdgpu_device *adev);
>   void amdgpu_virt_init_setting(struct amdgpu_device *adev);
>   uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
>   void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 9f5d123..82d157e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -2114,7 +2114,6 @@ static int gfx_v8_0_sw_fini(void *handle)
>   	amdgpu_gfx_compute_mqd_sw_fini(adev);
>   	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
>   	amdgpu_gfx_kiq_fini(adev);
> -	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
>   
>   	gfx_v8_0_mec_fini(adev);
>   	gfx_v8_0_rlc_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 5a4c074..034bcbe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1455,7 +1455,6 @@ static int gfx_v9_0_sw_fini(void *handle)
>   	amdgpu_gfx_compute_mqd_sw_fini(adev);
>   	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
>   	amdgpu_gfx_kiq_fini(adev);
> -	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
>   
>   	gfx_v9_0_mec_fini(adev);
>   	gfx_v9_0_ngg_fini(adev);


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 08/10] drm/amdgpu:cleanup firmware.fw_buf alloc/free

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> use bo_create/free_kernel instead of manually doing it
>
> Change-Id: I128651d2b2207f6e16b484b050f3232c08b53de3
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 43 +++++++------------------------
>   1 file changed, 9 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index ab9b2d4..474f88f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -359,7 +359,6 @@ static int amdgpu_ucode_patch_jt(struct amdgpu_firmware_info *ucode,
>   
>   int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
>   {
> -	struct amdgpu_bo **bo = &adev->firmware.fw_buf;
>   	uint64_t fw_offset = 0;
>   	int i, err;
>   	struct amdgpu_firmware_info *ucode = NULL;
> @@ -371,35 +370,15 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
>   	}
>   
>   	if (!adev->in_gpu_reset) {
> -		err = amdgpu_bo_create(adev, adev->firmware.fw_size, PAGE_SIZE, true,
> +		err = amdgpu_bo_create_kernel(adev, adev->firmware.fw_size, PAGE_SIZE,
>   					amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
> -					AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS|AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED,
> -					NULL, NULL, 0, bo);
> +					&adev->firmware.fw_buf,
> +					&adev->firmware.fw_buf_mc,
> +					&adev->firmware.fw_buf_ptr);
>   		if (err) {
> -			dev_err(adev->dev, "(%d) Firmware buffer allocate failed\n", err);
> +			dev_err(adev->dev, "failed to create kernel buffer for firmware.fw_buf\n");
>   			goto failed;
>   		}
> -
> -		err = amdgpu_bo_reserve(*bo, false);
> -		if (err) {
> -			dev_err(adev->dev, "(%d) Firmware buffer reserve failed\n", err);
> -			goto failed_reserve;
> -		}
> -
> -		err = amdgpu_bo_pin(*bo, amdgpu_sriov_vf(adev) ? AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT,
> -					&adev->firmware.fw_buf_mc);
> -		if (err) {
> -			dev_err(adev->dev, "(%d) Firmware buffer pin failed\n", err);
> -			goto failed_pin;
> -		}
> -
> -		err = amdgpu_bo_kmap(*bo, &adev->firmware.fw_buf_ptr);
> -		if (err) {
> -			dev_err(adev->dev, "(%d) Firmware buffer kmap failed\n", err);
> -			goto failed_kmap;
> -		}
> -
> -		amdgpu_bo_unreserve(*bo);
>   	}
>   
>   	memset(adev->firmware.fw_buf_ptr, 0, adev->firmware.fw_size);
> @@ -436,12 +415,6 @@ int amdgpu_ucode_init_bo(struct amdgpu_device *adev)
>   	}
>   	return 0;
>   
> -failed_kmap:
> -	amdgpu_bo_unpin(*bo);
> -failed_pin:
> -	amdgpu_bo_unreserve(*bo);
> -failed_reserve:
> -	amdgpu_bo_unref(bo);
>   failed:
>   	if (err)
>   		adev->firmware.load_type = AMDGPU_FW_LOAD_DIRECT;
> @@ -464,8 +437,10 @@ int amdgpu_ucode_fini_bo(struct amdgpu_device *adev)
>   			ucode->kaddr = NULL;
>   		}
>   	}
> -	amdgpu_bo_unref(&adev->firmware.fw_buf);
> -	adev->firmware.fw_buf = NULL;
> +
> +	amdgpu_bo_free_kernel(&adev->firmware.fw_buf,
> +				&adev->firmware.fw_buf_mc,
> +				&adev->firmware.fw_buf_ptr);
>   
>   	return 0;
>   }


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 09/10] drm/amdgpu:fix memleak

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> those RLC used buffers are not cleared in GFX's sw_fini
>
> Change-Id: I0c9204753caa327cfbabdfb1465d2c03012ee977
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 8 ++++++++
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 9 +++++++++
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 8 ++++++++
>   3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index 887f196..b6f94c6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -4672,6 +4672,14 @@ static int gfx_v7_0_sw_fini(void *handle)
>   	gfx_v7_0_cp_compute_fini(adev);
>   	gfx_v7_0_rlc_fini(adev);
>   	gfx_v7_0_mec_fini(adev);
> +	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
> +				&adev->gfx.rlc.clear_state_gpu_addr,
> +				(void **)&adev->gfx.rlc.cs_ptr);
> +	if (adev->gfx.rlc.cp_table_size) {
> +		amdgpu_bo_free_kernel(&adev->gfx.rlc.cp_table_obj,
> +				&adev->gfx.rlc.cp_table_gpu_addr,
> +				(void **)&adev->gfx.rlc.cp_table_ptr);
> +	}
>   	gfx_v7_0_free_microcode(adev);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 82d157e..457af4a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -2117,6 +2117,15 @@ static int gfx_v8_0_sw_fini(void *handle)
>   
>   	gfx_v8_0_mec_fini(adev);
>   	gfx_v8_0_rlc_fini(adev);
> +	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
> +				&adev->gfx.rlc.clear_state_gpu_addr,
> +				(void **)&adev->gfx.rlc.cs_ptr);
> +	if ((adev->asic_type == CHIP_CARRIZO) ||
> +	    (adev->asic_type == CHIP_STONEY)) {
> +		amdgpu_bo_free_kernel(&adev->gfx.rlc.cp_table_obj,
> +				&adev->gfx.rlc.cp_table_gpu_addr,
> +				(void **)&adev->gfx.rlc.cp_table_ptr);
> +	}
>   	gfx_v8_0_free_microcode(adev);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 034bcbe..4214988 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1458,6 +1458,14 @@ static int gfx_v9_0_sw_fini(void *handle)
>   
>   	gfx_v9_0_mec_fini(adev);
>   	gfx_v9_0_ngg_fini(adev);
> +	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
> +				&adev->gfx.rlc.clear_state_gpu_addr,
> +				(void **)&adev->gfx.rlc.cs_ptr);
> +	if (adev->asic_type == CHIP_RAVEN) {
> +		amdgpu_bo_free_kernel(&adev->gfx.rlc.cp_table_obj,
> +				&adev->gfx.rlc.cp_table_gpu_addr,
> +				(void **)&adev->gfx.rlc.cp_table_ptr);
> +	}
>   	gfx_v9_0_free_microcode(adev);
>   
>   	return 0;


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 10/10] drm/amdgpu:show error message if fail on event4

[Christian König]

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> Change-Id: I984697748672421a05f179a7bf4eeb0255b28bdc
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index ccb33ef..35a1593 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1914,7 +1914,8 @@ static int amdgpu_fini(struct amdgpu_device *adev)
>   	}
>   
>   	if (amdgpu_sriov_vf(adev))
> -		amdgpu_virt_release_full_gpu(adev, false);
> +		if (amdgpu_virt_release_full_gpu(adev, false))
> +			DRM_ERROR("failed to release exclusive mode on fini\n");
>   
>   	return 0;
>   }


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 05/10] drm/amdgpu:fix memleak in takedown

[Christian König]

Am 14.11.2017 um 12:52 schrieb Christian König:
> Am 14.11.2017 um 10:07 schrieb Monk Liu:
>> this can fix the memory leak under the case that not all
>> BO are freed during "takedown" stage, because originally
>> it blocks following kfree on mgr.
>
> NAK, all BOs must be freed before we take down at least the GTT manager.

Ah, wait a second. That is the address space manager here!

Mhm, returning -EBUSY is probably not a good idea, but we should at 
least print some warning.

Regards,
Christian.

> That we run into a memory leak is intended, cause that is still better 
> than crashing the system because we access already freed up memory.
>
> Regards,
> Christian.
>
>>
>> Change-Id: I56e832ac317215a4d9f58edb8e75ea722158715c
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 6 ------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 -----
>>   2 files changed, 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> index 88709f9..72819cc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> @@ -76,12 +76,6 @@ static int amdgpu_gtt_mgr_fini(struct 
>> ttm_mem_type_manager *man)
>>   {
>>       struct amdgpu_gtt_mgr *mgr = man->priv;
>>   -    spin_lock(&mgr->lock);
>> -    if (!drm_mm_clean(&mgr->mm)) {
>> -        spin_unlock(&mgr->lock);
>> -        return -EBUSY;
>> -    }
>> -
>>       drm_mm_takedown(&mgr->mm);
>>       spin_unlock(&mgr->lock);
>>       kfree(mgr);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> index 4ca622b..3e883e5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> @@ -68,11 +68,6 @@ static int amdgpu_vram_mgr_fini(struct 
>> ttm_mem_type_manager *man)
>>       struct amdgpu_vram_mgr *mgr = man->priv;
>>         spin_lock(&mgr->lock);
>> -    if (!drm_mm_clean(&mgr->mm)) {
>> -        spin_unlock(&mgr->lock);
>> -        return -EBUSY;
>> -    }
>> -
>>       drm_mm_takedown(&mgr->mm);
>>       spin_unlock(&mgr->lock);
>>       kfree(mgr);
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 03/10] drm/amdgpu:fix NULL pointer access during drv remove

[Liu, Monk]

ok

-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] 
Sent: 2017年11月14日 19:49
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 03/10] drm/amdgpu:fix NULL pointer access during drv remove

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> NULL pointer is because original logic will step into
> set_pde_pte() even after the gart.ptr is freed due to there are twice 
> gart_unbind() on all gart area.
>
> also, there are other minor fixes:
> 1,since gart_init only create dummy page, the corresponding gart_fini 
> shouldn't do more like unbinding all GART, this is unnecessary because 
> in driver fini stage all GART unbinding had already been done during 
> each IP's SW_FINI (GMC's SW_FINI is the last one called), so remove 
> the step for the GART unbinding in gart_fini().
>
> 2,gart_fini() is already invoked during each GMC IP's gart_fini 
> routine,e.g. gmc_vx_0_gart_fini(), so no need to manually call it 
> during ttm_fini().
>
> 3,amdgpu_gem_force_release() should be put ahead of
> amdgpu_vm_manager_fini()
>
> Change-Id: Ib1f31a2cf6bfbb9b54c7cc2a8cf9e4e3160bcfa0
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c | 15 +++++----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c  |  1 -
>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c    |  2 +-
>   6 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index 4b5ec15..c4eef4a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> @@ -255,10 +255,8 @@ int amdgpu_gart_init(struct amdgpu_device *adev)
>   #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
>   	/* Allocate pages table */
>   	adev->gart.pages = vzalloc(sizeof(void *) * adev->gart.num_cpu_pages);
> -	if (adev->gart.pages == NULL) {
> -		amdgpu_gart_fini(adev);
> +	if (adev->gart.pages == NULL)
>   		return -ENOMEM;
> -	}
>   #endif
>   
>   	return 0;
> @@ -273,14 +271,11 @@ int amdgpu_gart_init(struct amdgpu_device *adev)
>    */
>   void amdgpu_gart_fini(struct amdgpu_device *adev)
>   {
> -	if (adev->gart.ready) {
> -		/* unbind pages */
> -		amdgpu_gart_unbind(adev, 0, adev->gart.num_cpu_pages);
> -	}
> -	adev->gart.ready = false;
>   #ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
> -	vfree(adev->gart.pages);
> -	adev->gart.pages = NULL;
> +	if (adev->gart.pages) {
> +		vfree(adev->gart.pages);
> +		adev->gart.pages = NULL;
> +	}

vfree() is NULL save, so extra checks are usually complained about by automated testers.

I would just drop this hunk, the rest of the patch looks good to me.

Christian.

>   #endif
>   	amdgpu_dummy_page_fini(adev);
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index e8401a9..615b849 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1653,7 +1653,6 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>   	if (adev->gds.oa.total_size)
>   		ttm_bo_clean_mm(&adev->mman.bdev, AMDGPU_PL_OA);
>   	ttm_bo_device_release(&adev->mman.bdev);
> -	amdgpu_gart_fini(adev);
>   	amdgpu_ttm_global_fini(adev);
>   	adev->mman.initialized = false;
>   	DRM_INFO("amdgpu: ttm finalized\n"); diff --git 
> a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index aac493b..6112dfc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -894,9 +894,9 @@ static int gmc_v6_0_sw_fini(void *handle)
>   {
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v6_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   	release_firmware(adev->mc.fw);
>   	adev->mc.fw = NULL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index dede91a..b57dc06 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -1067,9 +1067,9 @@ static int gmc_v7_0_sw_fini(void *handle)
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
>   	kfree(adev->mc.vm_fault_info);
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v7_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   	release_firmware(adev->mc.fw);
>   	adev->mc.fw = NULL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 847c046..5be4854 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -1168,9 +1168,9 @@ static int gmc_v8_0_sw_fini(void *handle)
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
>   	kfree(adev->mc.vm_fault_info);
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v8_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   	release_firmware(adev->mc.fw);
>   	adev->mc.fw = NULL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index d0acf26..bea0774 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -667,9 +667,9 @@ static int gmc_v9_0_sw_fini(void *handle)
>   {
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   
> +	amdgpu_gem_force_release(adev);
>   	amdgpu_vm_manager_fini(adev);
>   	gmc_v9_0_gart_fini(adev);
> -	amdgpu_gem_force_release(adev);
>   	amdgpu_bo_fini(adev);
>   
>   	return 0;


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 05/10] drm/amdgpu:fix memleak in takedown

[Liu, Monk]

Actually " drm_mm_takedown" below will give you the warning if some BO is still in MM
That means below part is totally useless:

>>   -    spin_lock(&mgr->lock);
>> -    if (!drm_mm_clean(&mgr->mm)) {
>> -        spin_unlock(&mgr->lock);
>> -        return -EBUSY;
>> -    }
>> -

And the worse thing is this "return -EBUSY" would lead to memory leak because it blocks following kfree() on mgr...
BR Monk

-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] 
Sent: 2017年11月14日 20:01
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 05/10] drm/amdgpu:fix memleak in takedown

Am 14.11.2017 um 12:52 schrieb Christian König:
> Am 14.11.2017 um 10:07 schrieb Monk Liu:
>> this can fix the memory leak under the case that not all BO are freed 
>> during "takedown" stage, because originally it blocks following kfree 
>> on mgr.
>
> NAK, all BOs must be freed before we take down at least the GTT manager.

Ah, wait a second. That is the address space manager here!

Mhm, returning -EBUSY is probably not a good idea, but we should at least print some warning.

Regards,
Christian.

> That we run into a memory leak is intended, cause that is still better 
> than crashing the system because we access already freed up memory.
>
> Regards,
> Christian.
>
>>
>> Change-Id: I56e832ac317215a4d9f58edb8e75ea722158715c
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 6 ------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 -----
>>   2 files changed, 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> index 88709f9..72819cc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>> @@ -76,12 +76,6 @@ static int amdgpu_gtt_mgr_fini(struct 
>> ttm_mem_type_manager *man)
>>   {
>>       struct amdgpu_gtt_mgr *mgr = man->priv;
>>   -    spin_lock(&mgr->lock);
>> -    if (!drm_mm_clean(&mgr->mm)) {
>> -        spin_unlock(&mgr->lock);
>> -        return -EBUSY;
>> -    }
>> -
>>       drm_mm_takedown(&mgr->mm);
>>       spin_unlock(&mgr->lock);
>>       kfree(mgr);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> index 4ca622b..3e883e5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>> @@ -68,11 +68,6 @@ static int amdgpu_vram_mgr_fini(struct 
>> ttm_mem_type_manager *man)
>>       struct amdgpu_vram_mgr *mgr = man->priv;
>>         spin_lock(&mgr->lock);
>> -    if (!drm_mm_clean(&mgr->mm)) {
>> -        spin_unlock(&mgr->lock);
>> -        return -EBUSY;
>> -    }
>> -
>>       drm_mm_takedown(&mgr->mm);
>>       spin_unlock(&mgr->lock);
>>       kfree(mgr);
>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 07/10] drm/amdgpu:free CSA in unified place

[Liu, Monk]

Yeah CSA is allocated right after in GMC hw init (and after WB_INIT)
So we can put the free of it right before WB_INIT, thus avoiding doing it in each GFX IP's sw_init



-----Original Message-----
From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com] 
Sent: 2017年11月14日 19:55
To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 07/10] drm/amdgpu:free CSA in unified place

Am 14.11.2017 um 10:07 schrieb Monk Liu:
> instead of doing it in each GFX ip's sw_fini

Mhm, is that allocated in the same way as well?

>
> Change-Id: Idf0fd500d4fc385cf7a930cc56305070c937bf20
> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   | 1 +
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      | 1 -
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 1 -
>   5 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c13b493..ccb33ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1865,6 +1865,7 @@ static int amdgpu_fini(struct amdgpu_device *adev)
>   		if (!adev->ip_blocks[i].status.hw)
>   			continue;
>   		if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) {
> +			amdgpu_free_static_csa(adev);
>   			amdgpu_wb_fini(adev);
>   			amdgpu_vram_scratch_fini(adev);
>   		}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 67fd110..118c84c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -47,6 +47,12 @@ int amdgpu_allocate_static_csa(struct amdgpu_device *adev)
>   	return 0;
>   }
>   
> +void amdgpu_free_static_csa(struct amdgpu_device *adev) {
> +	amdgpu_bo_free_kernel(&adev->virt.csa_obj,
> +						&adev->virt.csa_vmid0_addr,
> +						NULL);

The coding style here looks odd.

Christian.

> +}
> +
>   /*
>    * amdgpu_map_static_csa should be called during amdgpu_vm_init
>    * it maps virtual address "AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index f77d116..6a83425 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -283,6 +283,7 @@ bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
>   int amdgpu_allocate_static_csa(struct amdgpu_device *adev);
>   int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			  struct amdgpu_bo_va **bo_va);
> +void amdgpu_free_static_csa(struct amdgpu_device *adev);
>   void amdgpu_virt_init_setting(struct amdgpu_device *adev);
>   uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
>   void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, 
> uint32_t v); diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 9f5d123..82d157e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -2114,7 +2114,6 @@ static int gfx_v8_0_sw_fini(void *handle)
>   	amdgpu_gfx_compute_mqd_sw_fini(adev);
>   	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
>   	amdgpu_gfx_kiq_fini(adev);
> -	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
>   
>   	gfx_v8_0_mec_fini(adev);
>   	gfx_v8_0_rlc_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 5a4c074..034bcbe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1455,7 +1455,6 @@ static int gfx_v9_0_sw_fini(void *handle)
>   	amdgpu_gfx_compute_mqd_sw_fini(adev);
>   	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
>   	amdgpu_gfx_kiq_fini(adev);
> -	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
>   
>   	gfx_v9_0_mec_fini(adev);
>   	gfx_v9_0_ngg_fini(adev);


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 05/10] drm/amdgpu:fix memleak in takedown

[Christian König]

Makes sense, Reviewed-by: Christian König <christian.koenig@amd.com> fro 
this one as well.

Regards,
Christian.

Am 14.11.2017 um 16:00 schrieb Liu, Monk:
> Actually " drm_mm_takedown" below will give you the warning if some BO is still in MM
> That means below part is totally useless:
>
>>>    -    spin_lock(&mgr->lock);
>>> -    if (!drm_mm_clean(&mgr->mm)) {
>>> -        spin_unlock(&mgr->lock);
>>> -        return -EBUSY;
>>> -    }
>>> -
> And the worse thing is this "return -EBUSY" would lead to memory leak because it blocks following kfree() on mgr...
> BR Monk
>
> -----Original Message-----
> From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
> Sent: 2017年11月14日 20:01
> To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 05/10] drm/amdgpu:fix memleak in takedown
>
> Am 14.11.2017 um 12:52 schrieb Christian König:
>> Am 14.11.2017 um 10:07 schrieb Monk Liu:
>>> this can fix the memory leak under the case that not all BO are freed
>>> during "takedown" stage, because originally it blocks following kfree
>>> on mgr.
>> NAK, all BOs must be freed before we take down at least the GTT manager.
> Ah, wait a second. That is the address space manager here!
>
> Mhm, returning -EBUSY is probably not a good idea, but we should at least print some warning.
>
> Regards,
> Christian.
>
>> That we run into a memory leak is intended, cause that is still better
>> than crashing the system because we access already freed up memory.
>>
>> Regards,
>> Christian.
>>
>>> Change-Id: I56e832ac317215a4d9f58edb8e75ea722158715c
>>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 6 ------
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 -----
>>>    2 files changed, 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> index 88709f9..72819cc 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> @@ -76,12 +76,6 @@ static int amdgpu_gtt_mgr_fini(struct
>>> ttm_mem_type_manager *man)
>>>    {
>>>        struct amdgpu_gtt_mgr *mgr = man->priv;
>>>    -    spin_lock(&mgr->lock);
>>> -    if (!drm_mm_clean(&mgr->mm)) {
>>> -        spin_unlock(&mgr->lock);
>>> -        return -EBUSY;
>>> -    }
>>> -
>>>        drm_mm_takedown(&mgr->mm);
>>>        spin_unlock(&mgr->lock);
>>>        kfree(mgr);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> index 4ca622b..3e883e5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> @@ -68,11 +68,6 @@ static int amdgpu_vram_mgr_fini(struct
>>> ttm_mem_type_manager *man)
>>>        struct amdgpu_vram_mgr *mgr = man->priv;
>>>          spin_lock(&mgr->lock);
>>> -    if (!drm_mm_clean(&mgr->mm)) {
>>> -        spin_unlock(&mgr->lock);
>>> -        return -EBUSY;
>>> -    }
>>> -
>>>        drm_mm_takedown(&mgr->mm);
>>>        spin_unlock(&mgr->lock);
>>>        kfree(mgr);
>>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 07/10] drm/amdgpu:free CSA in unified place

[Christian König]

Thought so, in this case the patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

Am 14.11.2017 um 16:02 schrieb Liu, Monk:
> Yeah CSA is allocated right after in GMC hw init (and after WB_INIT)
> So we can put the free of it right before WB_INIT, thus avoiding doing it in each GFX IP's sw_init
>
>
>
> -----Original Message-----
> From: Christian König [mailto:ckoenig.leichtzumerken@gmail.com]
> Sent: 2017年11月14日 19:55
> To: Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 07/10] drm/amdgpu:free CSA in unified place
>
> Am 14.11.2017 um 10:07 schrieb Monk Liu:
>> instead of doing it in each GFX ip's sw_fini
> Mhm, is that allocated in the same way as well?
>
>> Change-Id: Idf0fd500d4fc385cf7a930cc56305070c937bf20
>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>> ---
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 6 ++++++
>>    drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   | 1 +
>>    drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c      | 1 -
>>    drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c      | 1 -
>>    5 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index c13b493..ccb33ef 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -1865,6 +1865,7 @@ static int amdgpu_fini(struct amdgpu_device *adev)
>>    		if (!adev->ip_blocks[i].status.hw)
>>    			continue;
>>    		if (adev->ip_blocks[i].version->type == AMD_IP_BLOCK_TYPE_GMC) {
>> +			amdgpu_free_static_csa(adev);
>>    			amdgpu_wb_fini(adev);
>>    			amdgpu_vram_scratch_fini(adev);
>>    		}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> index 67fd110..118c84c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
>> @@ -47,6 +47,12 @@ int amdgpu_allocate_static_csa(struct amdgpu_device *adev)
>>    	return 0;
>>    }
>>    
>> +void amdgpu_free_static_csa(struct amdgpu_device *adev) {
>> +	amdgpu_bo_free_kernel(&adev->virt.csa_obj,
>> +						&adev->virt.csa_vmid0_addr,
>> +						NULL);
> The coding style here looks odd.
>
> Christian.
>
>> +}
>> +
>>    /*
>>     * amdgpu_map_static_csa should be called during amdgpu_vm_init
>>     * it maps virtual address "AMDGPU_VA_RESERVED_SIZE - AMDGPU_CSA_SIZE"
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> index f77d116..6a83425 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
>> @@ -283,6 +283,7 @@ bool amdgpu_virt_mmio_blocked(struct amdgpu_device *adev);
>>    int amdgpu_allocate_static_csa(struct amdgpu_device *adev);
>>    int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>    			  struct amdgpu_bo_va **bo_va);
>> +void amdgpu_free_static_csa(struct amdgpu_device *adev);
>>    void amdgpu_virt_init_setting(struct amdgpu_device *adev);
>>    uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg);
>>    void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg,
>> uint32_t v); diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index 9f5d123..82d157e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -2114,7 +2114,6 @@ static int gfx_v8_0_sw_fini(void *handle)
>>    	amdgpu_gfx_compute_mqd_sw_fini(adev);
>>    	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
>>    	amdgpu_gfx_kiq_fini(adev);
>> -	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
>>    
>>    	gfx_v8_0_mec_fini(adev);
>>    	gfx_v8_0_rlc_fini(adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index 5a4c074..034bcbe 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -1455,7 +1455,6 @@ static int gfx_v9_0_sw_fini(void *handle)
>>    	amdgpu_gfx_compute_mqd_sw_fini(adev);
>>    	amdgpu_gfx_kiq_free_ring(&adev->gfx.kiq.ring, &adev->gfx.kiq.irq);
>>    	amdgpu_gfx_kiq_fini(adev);
>> -	amdgpu_bo_free_kernel(&adev->virt.csa_obj, &adev->virt.csa_vmid0_addr, NULL);
>>    
>>    	gfx_v9_0_mec_fini(adev);
>>    	gfx_v9_0_ngg_fini(adev);
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 05/10] drm/amdgpu:fix memleak in takedown

[Chunming Zhou]

On 2017年11月14日 20:00, Christian König wrote:
> Am 14.11.2017 um 12:52 schrieb Christian König:
>> Am 14.11.2017 um 10:07 schrieb Monk Liu:
>>> this can fix the memory leak under the case that not all
>>> BO are freed during "takedown" stage, because originally
>>> it blocks following kfree on mgr.
>>
>> NAK, all BOs must be freed before we take down at least the GTT manager.
>
> Ah, wait a second. That is the address space manager here!
>
> Mhm, returning -EBUSY is probably not a good idea, but we should at 
> least print some warning.
Remove it and drm_mm_takedown will print a warning.

Regards,
David Zhou
>
> Regards,
> Christian.
>
>> That we run into a memory leak is intended, cause that is still 
>> better than crashing the system because we access already freed up 
>> memory.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Change-Id: I56e832ac317215a4d9f58edb8e75ea722158715c
>>> Signed-off-by: Monk Liu <Monk.Liu@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c  | 6 ------
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 -----
>>>   2 files changed, 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> index 88709f9..72819cc 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
>>> @@ -76,12 +76,6 @@ static int amdgpu_gtt_mgr_fini(struct 
>>> ttm_mem_type_manager *man)
>>>   {
>>>       struct amdgpu_gtt_mgr *mgr = man->priv;
>>>   -    spin_lock(&mgr->lock);
>>> -    if (!drm_mm_clean(&mgr->mm)) {
>>> -        spin_unlock(&mgr->lock);
>>> -        return -EBUSY;
>>> -    }
>>> -
>>>       drm_mm_takedown(&mgr->mm);
>>>       spin_unlock(&mgr->lock);
>>>       kfree(mgr);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> index 4ca622b..3e883e5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
>>> @@ -68,11 +68,6 @@ static int amdgpu_vram_mgr_fini(struct 
>>> ttm_mem_type_manager *man)
>>>       struct amdgpu_vram_mgr *mgr = man->priv;
>>>         spin_lock(&mgr->lock);
>>> -    if (!drm_mm_clean(&mgr->mm)) {
>>> -        spin_unlock(&mgr->lock);
>>> -        return -EBUSY;
>>> -    }
>>> -
>>>       drm_mm_takedown(&mgr->mm);
>>>       spin_unlock(&mgr->lock);
>>>       kfree(mgr);
>>
>>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx