All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over
@ 2018-04-12  4:08 Andrey Grodzovsky
       [not found] ` <1523506110-14002-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2018-04-12  4:08 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Alex Deucher, Alex Deucher, ramin.ranjbar-5C7GfCeVMHo,
	ray.huang-5C7GfCeVMHo, christian.koenig-5C7GfCeVMHo

From: Alex Deucher <alexdeucher@gmail.com>

Steal 9 MB for vga emulation and fb if vga is enabled, otherwise,
steal enough to cover the current display size as set by the vbios.

If no memory is used (e.g., secondary or headless card), skip
stolen memory reserve.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 17 +++++++------
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   | 17 ++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 17 ++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 17 ++++++++++++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 42 ++++++++++++++++++++++++++++++++-
 5 files changed, 99 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index ab73300e..0555821 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1441,12 +1441,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 		return r;
 	}
 
-	r = amdgpu_bo_create_kernel(adev, adev->gmc.stolen_size, PAGE_SIZE,
-				    AMDGPU_GEM_DOMAIN_VRAM,
-				    &adev->stolen_vga_memory,
-				    NULL, NULL);
-	if (r)
-		return r;
+	if (adev->gmc.stolen_size) {
+		r = amdgpu_bo_create_kernel(adev, adev->gmc.stolen_size, PAGE_SIZE,
+					    AMDGPU_GEM_DOMAIN_VRAM,
+					    &adev->stolen_vga_memory,
+					    NULL, NULL);
+		if (r)
+			return r;
+	}
 	DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
 		 (unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
 
@@ -1521,7 +1523,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
 		return;
 
 	amdgpu_ttm_debugfs_fini(adev);
-	amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
+	if (adev->gmc.stolen_size)
+		amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
 	amdgpu_ttm_fw_reserve_vram_fini(adev);
 	if (adev->mman.aper_base_kaddr)
 		iounmap(adev->mman.aper_base_kaddr);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 5617cf6..63f0b65 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -825,6 +825,21 @@ static int gmc_v6_0_late_init(void *handle)
 		return 0;
 }
 
+static unsigned gmc_v6_0_get_vbios_fb_size(struct amdgpu_device *adev)
+{
+	u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
+
+	if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
+		return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
+	} else {
+		u32 viewport = RREG32(mmVIEWPORT_SIZE);
+		unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
+				 REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
+				 4);
+		return size;
+	}
+}
+
 static int gmc_v6_0_sw_init(void *handle)
 {
 	int r;
@@ -851,7 +866,7 @@ static int gmc_v6_0_sw_init(void *handle)
 
 	adev->gmc.mc_mask = 0xffffffffffULL;
 
-	adev->gmc.stolen_size = 256 * 1024;
+	adev->gmc.stolen_size = gmc_v6_0_get_vbios_fb_size(adev);
 
 	adev->need_dma32 = false;
 	dma_bits = adev->need_dma32 ? 32 : 40;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 80054f3..2deb5c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -964,6 +964,21 @@ static int gmc_v7_0_late_init(void *handle)
 		return 0;
 }
 
+static unsigned gmc_v7_0_get_vbios_fb_size(struct amdgpu_device *adev)
+{
+	u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
+
+	if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
+		return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
+	} else {
+		u32 viewport = RREG32(mmVIEWPORT_SIZE);
+		unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
+				 REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
+				 4);
+		return size;
+	}
+}
+
 static int gmc_v7_0_sw_init(void *handle)
 {
 	int r;
@@ -998,7 +1013,7 @@ static int gmc_v7_0_sw_init(void *handle)
 	 */
 	adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
 
-	adev->gmc.stolen_size = 256 * 1024;
+	adev->gmc.stolen_size = gmc_v7_0_get_vbios_fb_size(adev);
 
 	/* set DMA mask + need_dma32 flags.
 	 * PCIE - can handle 40-bits.
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index d71d4cb..04b00df 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1055,6 +1055,21 @@ static int gmc_v8_0_late_init(void *handle)
 		return 0;
 }
 
+static unsigned gmc_v8_0_get_vbios_fb_size(struct amdgpu_device *adev)
+{
+	u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
+
+	if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
+		return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
+	} else {
+		u32 viewport = RREG32(mmVIEWPORT_SIZE);
+		unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
+				 REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
+				 4);
+		return size;
+	}
+}
+
 #define mmMC_SEQ_MISC0_FIJI 0xA71
 
 static int gmc_v8_0_sw_init(void *handle)
@@ -1096,7 +1111,7 @@ static int gmc_v8_0_sw_init(void *handle)
 	 */
 	adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
 
-	adev->gmc.stolen_size = 256 * 1024;
+	adev->gmc.stolen_size = gmc_v8_0_get_vbios_fb_size(adev);
 
 	/* set DMA mask + need_dma32 flags.
 	 * PCIE - can handle 40-bits.
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 070946e..252a6c69 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -793,6 +793,46 @@ static int gmc_v9_0_gart_init(struct amdgpu_device *adev)
 	return amdgpu_gart_table_vram_alloc(adev);
 }
 
+#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION                                                          0x055d
+#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION_BASE_IDX                                                 2
+#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH__SHIFT                                        0x0
+#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT__SHIFT                                       0x10
+#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH_MASK                                          0x00003FFFL
+#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT_MASK                                         0x3FFF0000L
+
+static unsigned gmc_v9_0_get_vbios_fb_size(struct amdgpu_device *adev)
+{
+	u32 d1vga_control = RREG32_SOC15(DCE, 0, mmD1VGA_CONTROL);
+
+	if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
+		return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
+	} else {
+		u32 viewport;
+		unsigned size;
+
+		switch (adev->asic_type) {
+		case CHIP_RAVEN:
+			viewport = RREG32_SOC15(DCE, 0, mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION);
+			size = (REG_GET_FIELD(viewport,
+					      HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_HEIGHT) *
+				REG_GET_FIELD(viewport,
+					      HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_WIDTH) *
+				4);
+			break;
+		case CHIP_VEGA10:
+		case CHIP_VEGA12:
+		default:
+			viewport = RREG32_SOC15(DCE, 0, mmSCL0_VIEWPORT_SIZE);
+			size = (REG_GET_FIELD(viewport, SCL0_VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
+				REG_GET_FIELD(viewport, SCL0_VIEWPORT_SIZE, VIEWPORT_WIDTH) *
+				4);
+			break;
+		}
+
+		return size;
+	}
+}
+
 static int gmc_v9_0_sw_init(void *handle)
 {
 	int r;
@@ -848,7 +888,7 @@ static int gmc_v9_0_sw_init(void *handle)
 	 * It needs to reserve 8M stolen memory for vega10
 	 * TODO: Figure out how to avoid that...
 	 */
-	adev->gmc.stolen_size = 8 * 1024 * 1024;
+	adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
 
 	/* set DMA mask + need_dma32 flags.
 	 * PCIE - can handle 44-bits.
-- 
2.7.4

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

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

* [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found] ` <1523506110-14002-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12  4:08   ` Andrey Grodzovsky
       [not found]     ` <1523506110-14002-2-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2018-04-12  4:16   ` [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over Alex Deucher
  1 sibling, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2018-04-12  4:08 UTC (permalink / raw)
  To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
  Cc: Alex Deucher, Andrey Grodzovsky, ramin.ranjbar-5C7GfCeVMHo,
	ray.huang-5C7GfCeVMHo, christian.koenig-5C7GfCeVMHo

Reserved VRAM is used to avoid overriding pre OS FB.
Once our display stack takes over we don't need the reserved
VRAM anymore.

v2:
Remove comment, we know actually why we need to reserve the stolen VRAM.
Fix return type for amdgpu_ttm_late_init.
v3:
Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
v4:
Don't release stolen memory for GMC9 ASICs untill GART corruption
on S3 resume is resolved.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  7 +++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 ++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  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      | 14 ++++++++++----
 8 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 9e23d6f..a160ef0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -852,6 +852,13 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
 	return amdgpu_ttm_init(adev);
 }
 
+int amdgpu_bo_late_init(struct amdgpu_device *adev)
+{
+	amdgpu_ttm_late_init(adev);
+
+	return 0;
+}
+
 void amdgpu_bo_fini(struct amdgpu_device *adev)
 {
 	amdgpu_ttm_fini(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 3bee133..1e9fe85 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -251,6 +251,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
 int amdgpu_bo_unpin(struct amdgpu_bo *bo);
 int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
 int amdgpu_bo_init(struct amdgpu_device *adev);
+int amdgpu_bo_late_init(struct amdgpu_device *adev);
 void amdgpu_bo_fini(struct amdgpu_device *adev);
 int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
 				struct vm_area_struct *vma);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 0555821..7a608cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1517,14 +1517,18 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 	return 0;
 }
 
+void amdgpu_ttm_late_init(struct amdgpu_device *adev)
+{
+	if (adev->gmc.stolen_size)
+		amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
+}
+
 void amdgpu_ttm_fini(struct amdgpu_device *adev)
 {
 	if (!adev->mman.initialized)
 		return;
 
 	amdgpu_ttm_debugfs_fini(adev);
-	if (adev->gmc.stolen_size)
-		amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
 	amdgpu_ttm_fw_reserve_vram_fini(adev);
 	if (adev->mman.aper_base_kaddr)
 		iounmap(adev->mman.aper_base_kaddr);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index 6ea7de8..e969c87 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -77,6 +77,7 @@ uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man);
 uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
 
 int amdgpu_ttm_init(struct amdgpu_device *adev);
+void amdgpu_ttm_late_init(struct amdgpu_device *adev);
 void amdgpu_ttm_fini(struct amdgpu_device *adev);
 void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev,
 					bool enable);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
index 63f0b65..4a8f9bd 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
@@ -819,6 +819,8 @@ static int gmc_v6_0_late_init(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	amdgpu_bo_late_init(adev);
+
 	if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
 		return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
 	else
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
index 2deb5c9..189fdf9 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
@@ -958,6 +958,8 @@ static int gmc_v7_0_late_init(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	amdgpu_bo_late_init(adev);
+
 	if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
 		return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
 	else
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
index 04b00df..19e153f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
@@ -1049,6 +1049,8 @@ static int gmc_v8_0_late_init(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 
+	amdgpu_bo_late_init(adev);
+
 	if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
 		return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
 	else
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 252a6c69..099e3ce5 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
 	unsigned i;
 	int r;
 
+	/*
+	 * TODO:
+	 * Currently there is a bug where some memory client outside
+	 * of the driver writes to first 8M of VRAM on S3 resume,
+	 * this overrides GART which by default gets placed in first 8M and
+	 * causes VM_FAULTS once GTT is accessed.
+	 * Keep the stolen memory reservation until this solved.
+	 */
+	/* amdgpu_bo_late_init(adev); /
+
 	for(i = 0; i < adev->num_rings; ++i) {
 		struct amdgpu_ring *ring = adev->rings[i];
 		unsigned vmhub = ring->funcs->vmhub;
@@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
 	 */
 	adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
 
-	/*
-	 * It needs to reserve 8M stolen memory for vega10
-	 * TODO: Figure out how to avoid that...
-	 */
 	adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
 
 	/* set DMA mask + need_dma32 flags.
-- 
2.7.4

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

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

* Re: [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over
       [not found] ` <1523506110-14002-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
  2018-04-12  4:08   ` [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible Andrey Grodzovsky
@ 2018-04-12  4:16   ` Alex Deucher
       [not found]     ` <CADnq5_Nw0Cwh-pg9==CWLRC2E709b2S8LEWMd93Eo+ZRfZ7GCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  1 sibling, 1 reply; 13+ messages in thread
From: Alex Deucher @ 2018-04-12  4:16 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: Deucher, Alexander, Ranjbar, Ramin, Huang Rui, Christian Koenig,
	amd-gfx list

On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
<andrey.grodzovsky@amd.com> wrote:
> From: Alex Deucher <alexdeucher@gmail.com>
>
> Steal 9 MB for vga emulation and fb if vga is enabled, otherwise,
> steal enough to cover the current display size as set by the vbios.
>
> If no memory is used (e.g., secondary or headless card), skip
> stolen memory reserve.
>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Looks like you used v1 rather than v2 of this patch:
https://patchwork.freedesktop.org/patch/215566/

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 17 +++++++------
>  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   | 17 ++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 17 ++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 17 ++++++++++++-
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 42 ++++++++++++++++++++++++++++++++-
>  5 files changed, 99 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index ab73300e..0555821 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1441,12 +1441,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>                 return r;
>         }
>
> -       r = amdgpu_bo_create_kernel(adev, adev->gmc.stolen_size, PAGE_SIZE,
> -                                   AMDGPU_GEM_DOMAIN_VRAM,
> -                                   &adev->stolen_vga_memory,
> -                                   NULL, NULL);
> -       if (r)
> -               return r;
> +       if (adev->gmc.stolen_size) {
> +               r = amdgpu_bo_create_kernel(adev, adev->gmc.stolen_size, PAGE_SIZE,
> +                                           AMDGPU_GEM_DOMAIN_VRAM,
> +                                           &adev->stolen_vga_memory,
> +                                           NULL, NULL);
> +               if (r)
> +                       return r;
> +       }
>         DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>                  (unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
>
> @@ -1521,7 +1523,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>                 return;
>
>         amdgpu_ttm_debugfs_fini(adev);
> -       amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
> +       if (adev->gmc.stolen_size)
> +               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
>         amdgpu_ttm_fw_reserve_vram_fini(adev);
>         if (adev->mman.aper_base_kaddr)
>                 iounmap(adev->mman.aper_base_kaddr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 5617cf6..63f0b65 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -825,6 +825,21 @@ static int gmc_v6_0_late_init(void *handle)
>                 return 0;
>  }
>
> +static unsigned gmc_v6_0_get_vbios_fb_size(struct amdgpu_device *adev)
> +{
> +       u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
> +
> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
> +       } else {
> +               u32 viewport = RREG32(mmVIEWPORT_SIZE);
> +               unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
> +                                REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
> +                                4);
> +               return size;
> +       }
> +}
> +
>  static int gmc_v6_0_sw_init(void *handle)
>  {
>         int r;
> @@ -851,7 +866,7 @@ static int gmc_v6_0_sw_init(void *handle)
>
>         adev->gmc.mc_mask = 0xffffffffffULL;
>
> -       adev->gmc.stolen_size = 256 * 1024;
> +       adev->gmc.stolen_size = gmc_v6_0_get_vbios_fb_size(adev);
>
>         adev->need_dma32 = false;
>         dma_bits = adev->need_dma32 ? 32 : 40;
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 80054f3..2deb5c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -964,6 +964,21 @@ static int gmc_v7_0_late_init(void *handle)
>                 return 0;
>  }
>
> +static unsigned gmc_v7_0_get_vbios_fb_size(struct amdgpu_device *adev)
> +{
> +       u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
> +
> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
> +       } else {
> +               u32 viewport = RREG32(mmVIEWPORT_SIZE);
> +               unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
> +                                REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
> +                                4);
> +               return size;
> +       }
> +}
> +
>  static int gmc_v7_0_sw_init(void *handle)
>  {
>         int r;
> @@ -998,7 +1013,7 @@ static int gmc_v7_0_sw_init(void *handle)
>          */
>         adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
>
> -       adev->gmc.stolen_size = 256 * 1024;
> +       adev->gmc.stolen_size = gmc_v7_0_get_vbios_fb_size(adev);
>
>         /* set DMA mask + need_dma32 flags.
>          * PCIE - can handle 40-bits.
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index d71d4cb..04b00df 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -1055,6 +1055,21 @@ static int gmc_v8_0_late_init(void *handle)
>                 return 0;
>  }
>
> +static unsigned gmc_v8_0_get_vbios_fb_size(struct amdgpu_device *adev)
> +{
> +       u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
> +
> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
> +       } else {
> +               u32 viewport = RREG32(mmVIEWPORT_SIZE);
> +               unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
> +                                REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
> +                                4);
> +               return size;
> +       }
> +}
> +
>  #define mmMC_SEQ_MISC0_FIJI 0xA71
>
>  static int gmc_v8_0_sw_init(void *handle)
> @@ -1096,7 +1111,7 @@ static int gmc_v8_0_sw_init(void *handle)
>          */
>         adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
>
> -       adev->gmc.stolen_size = 256 * 1024;
> +       adev->gmc.stolen_size = gmc_v8_0_get_vbios_fb_size(adev);
>
>         /* set DMA mask + need_dma32 flags.
>          * PCIE - can handle 40-bits.
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 070946e..252a6c69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -793,6 +793,46 @@ static int gmc_v9_0_gart_init(struct amdgpu_device *adev)
>         return amdgpu_gart_table_vram_alloc(adev);
>  }
>
> +#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION                                                          0x055d
> +#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION_BASE_IDX                                                 2
> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH__SHIFT                                        0x0
> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT__SHIFT                                       0x10
> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH_MASK                                          0x00003FFFL
> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT_MASK                                         0x3FFF0000L
> +
> +static unsigned gmc_v9_0_get_vbios_fb_size(struct amdgpu_device *adev)
> +{
> +       u32 d1vga_control = RREG32_SOC15(DCE, 0, mmD1VGA_CONTROL);
> +
> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
> +       } else {
> +               u32 viewport;
> +               unsigned size;
> +
> +               switch (adev->asic_type) {
> +               case CHIP_RAVEN:
> +                       viewport = RREG32_SOC15(DCE, 0, mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION);
> +                       size = (REG_GET_FIELD(viewport,
> +                                             HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_HEIGHT) *
> +                               REG_GET_FIELD(viewport,
> +                                             HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_WIDTH) *
> +                               4);
> +                       break;
> +               case CHIP_VEGA10:
> +               case CHIP_VEGA12:
> +               default:
> +                       viewport = RREG32_SOC15(DCE, 0, mmSCL0_VIEWPORT_SIZE);
> +                       size = (REG_GET_FIELD(viewport, SCL0_VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
> +                               REG_GET_FIELD(viewport, SCL0_VIEWPORT_SIZE, VIEWPORT_WIDTH) *
> +                               4);
> +                       break;
> +               }
> +
> +               return size;
> +       }
> +}
> +
>  static int gmc_v9_0_sw_init(void *handle)
>  {
>         int r;
> @@ -848,7 +888,7 @@ static int gmc_v9_0_sw_init(void *handle)
>          * It needs to reserve 8M stolen memory for vega10
>          * TODO: Figure out how to avoid that...
>          */
> -       adev->gmc.stolen_size = 8 * 1024 * 1024;
> +       adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
>
>         /* set DMA mask + need_dma32 flags.
>          * PCIE - can handle 44-bits.
> --
> 2.7.4
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over
       [not found]     ` <CADnq5_Nw0Cwh-pg9==CWLRC2E709b2S8LEWMd93Eo+ZRfZ7GCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-04-12  4:25       ` Andrey Grodzovsky
       [not found]         ` <5e42d9cf-2581-14e1-80fd-0378323617c5-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2018-04-12  4:25 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Deucher, Alexander, Ranjbar, Ramin, Huang Rui, Christian Koenig,
	amd-gfx list



On 04/12/2018 12:16 AM, Alex Deucher wrote:
> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
> <andrey.grodzovsky@amd.com> wrote:
>> From: Alex Deucher <alexdeucher@gmail.com>
>>
>> Steal 9 MB for vga emulation and fb if vga is enabled, otherwise,
>> steal enough to cover the current display size as set by the vbios.
>>
>> If no memory is used (e.g., secondary or headless card), skip
>> stolen memory reserve.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Looks like you used v1 rather than v2 of this patch:
> https://patchwork.freedesktop.org/patch/215566/
>
> Alex

To much patches circling around, NP, will respin tomorrow.

Thanks,
Andrey

>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 17 +++++++------
>>   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   | 17 ++++++++++++-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   | 17 ++++++++++++-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   | 17 ++++++++++++-
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 42 ++++++++++++++++++++++++++++++++-
>>   5 files changed, 99 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index ab73300e..0555821 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1441,12 +1441,14 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>                  return r;
>>          }
>>
>> -       r = amdgpu_bo_create_kernel(adev, adev->gmc.stolen_size, PAGE_SIZE,
>> -                                   AMDGPU_GEM_DOMAIN_VRAM,
>> -                                   &adev->stolen_vga_memory,
>> -                                   NULL, NULL);
>> -       if (r)
>> -               return r;
>> +       if (adev->gmc.stolen_size) {
>> +               r = amdgpu_bo_create_kernel(adev, adev->gmc.stolen_size, PAGE_SIZE,
>> +                                           AMDGPU_GEM_DOMAIN_VRAM,
>> +                                           &adev->stolen_vga_memory,
>> +                                           NULL, NULL);
>> +               if (r)
>> +                       return r;
>> +       }
>>          DRM_INFO("amdgpu: %uM of VRAM memory ready\n",
>>                   (unsigned) (adev->gmc.real_vram_size / (1024 * 1024)));
>>
>> @@ -1521,7 +1523,8 @@ void amdgpu_ttm_fini(struct amdgpu_device *adev)
>>                  return;
>>
>>          amdgpu_ttm_debugfs_fini(adev);
>> -       amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
>> +       if (adev->gmc.stolen_size)
>> +               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
>>          amdgpu_ttm_fw_reserve_vram_fini(adev);
>>          if (adev->mman.aper_base_kaddr)
>>                  iounmap(adev->mman.aper_base_kaddr);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 5617cf6..63f0b65 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -825,6 +825,21 @@ static int gmc_v6_0_late_init(void *handle)
>>                  return 0;
>>   }
>>
>> +static unsigned gmc_v6_0_get_vbios_fb_size(struct amdgpu_device *adev)
>> +{
>> +       u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
>> +
>> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
>> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
>> +       } else {
>> +               u32 viewport = RREG32(mmVIEWPORT_SIZE);
>> +               unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
>> +                                REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
>> +                                4);
>> +               return size;
>> +       }
>> +}
>> +
>>   static int gmc_v6_0_sw_init(void *handle)
>>   {
>>          int r;
>> @@ -851,7 +866,7 @@ static int gmc_v6_0_sw_init(void *handle)
>>
>>          adev->gmc.mc_mask = 0xffffffffffULL;
>>
>> -       adev->gmc.stolen_size = 256 * 1024;
>> +       adev->gmc.stolen_size = gmc_v6_0_get_vbios_fb_size(adev);
>>
>>          adev->need_dma32 = false;
>>          dma_bits = adev->need_dma32 ? 32 : 40;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 80054f3..2deb5c9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -964,6 +964,21 @@ static int gmc_v7_0_late_init(void *handle)
>>                  return 0;
>>   }
>>
>> +static unsigned gmc_v7_0_get_vbios_fb_size(struct amdgpu_device *adev)
>> +{
>> +       u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
>> +
>> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
>> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
>> +       } else {
>> +               u32 viewport = RREG32(mmVIEWPORT_SIZE);
>> +               unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
>> +                                REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
>> +                                4);
>> +               return size;
>> +       }
>> +}
>> +
>>   static int gmc_v7_0_sw_init(void *handle)
>>   {
>>          int r;
>> @@ -998,7 +1013,7 @@ static int gmc_v7_0_sw_init(void *handle)
>>           */
>>          adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
>>
>> -       adev->gmc.stolen_size = 256 * 1024;
>> +       adev->gmc.stolen_size = gmc_v7_0_get_vbios_fb_size(adev);
>>
>>          /* set DMA mask + need_dma32 flags.
>>           * PCIE - can handle 40-bits.
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index d71d4cb..04b00df 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -1055,6 +1055,21 @@ static int gmc_v8_0_late_init(void *handle)
>>                  return 0;
>>   }
>>
>> +static unsigned gmc_v8_0_get_vbios_fb_size(struct amdgpu_device *adev)
>> +{
>> +       u32 d1vga_control = RREG32(mmD1VGA_CONTROL);
>> +
>> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
>> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
>> +       } else {
>> +               u32 viewport = RREG32(mmVIEWPORT_SIZE);
>> +               unsigned size = (REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
>> +                                REG_GET_FIELD(viewport, VIEWPORT_SIZE, VIEWPORT_WIDTH) *
>> +                                4);
>> +               return size;
>> +       }
>> +}
>> +
>>   #define mmMC_SEQ_MISC0_FIJI 0xA71
>>
>>   static int gmc_v8_0_sw_init(void *handle)
>> @@ -1096,7 +1111,7 @@ static int gmc_v8_0_sw_init(void *handle)
>>           */
>>          adev->gmc.mc_mask = 0xffffffffffULL; /* 40 bit MC */
>>
>> -       adev->gmc.stolen_size = 256 * 1024;
>> +       adev->gmc.stolen_size = gmc_v8_0_get_vbios_fb_size(adev);
>>
>>          /* set DMA mask + need_dma32 flags.
>>           * PCIE - can handle 40-bits.
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 070946e..252a6c69 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -793,6 +793,46 @@ static int gmc_v9_0_gart_init(struct amdgpu_device *adev)
>>          return amdgpu_gart_table_vram_alloc(adev);
>>   }
>>
>> +#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION                                                          0x055d
>> +#define mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION_BASE_IDX                                                 2
>> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH__SHIFT                                        0x0
>> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT__SHIFT                                       0x10
>> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_WIDTH_MASK                                          0x00003FFFL
>> +#define HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION__PRI_VIEWPORT_HEIGHT_MASK                                         0x3FFF0000L
>> +
>> +static unsigned gmc_v9_0_get_vbios_fb_size(struct amdgpu_device *adev)
>> +{
>> +       u32 d1vga_control = RREG32_SOC15(DCE, 0, mmD1VGA_CONTROL);
>> +
>> +       if (REG_GET_FIELD(d1vga_control, D1VGA_CONTROL, D1VGA_MODE_ENABLE)) {
>> +               return 9 * 1024 * 1024; /* reserve 8MB for vga emulator and 1 MB for FB */
>> +       } else {
>> +               u32 viewport;
>> +               unsigned size;
>> +
>> +               switch (adev->asic_type) {
>> +               case CHIP_RAVEN:
>> +                       viewport = RREG32_SOC15(DCE, 0, mmHUBP0_DCSURF_PRI_VIEWPORT_DIMENSION);
>> +                       size = (REG_GET_FIELD(viewport,
>> +                                             HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_HEIGHT) *
>> +                               REG_GET_FIELD(viewport,
>> +                                             HUBP0_DCSURF_PRI_VIEWPORT_DIMENSION, PRI_VIEWPORT_WIDTH) *
>> +                               4);
>> +                       break;
>> +               case CHIP_VEGA10:
>> +               case CHIP_VEGA12:
>> +               default:
>> +                       viewport = RREG32_SOC15(DCE, 0, mmSCL0_VIEWPORT_SIZE);
>> +                       size = (REG_GET_FIELD(viewport, SCL0_VIEWPORT_SIZE, VIEWPORT_HEIGHT) *
>> +                               REG_GET_FIELD(viewport, SCL0_VIEWPORT_SIZE, VIEWPORT_WIDTH) *
>> +                               4);
>> +                       break;
>> +               }
>> +
>> +               return size;
>> +       }
>> +}
>> +
>>   static int gmc_v9_0_sw_init(void *handle)
>>   {
>>          int r;
>> @@ -848,7 +888,7 @@ static int gmc_v9_0_sw_init(void *handle)
>>           * It needs to reserve 8M stolen memory for vega10
>>           * TODO: Figure out how to avoid that...
>>           */
>> -       adev->gmc.stolen_size = 8 * 1024 * 1024;
>> +       adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
>>
>>          /* set DMA mask + need_dma32 flags.
>>           * PCIE - can handle 44-bits.
>> --
>> 2.7.4
>>
> _______________________________________________
> 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

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

* Re: [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found]     ` <1523506110-14002-2-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12  4:32       ` Alex Deucher
       [not found]         ` <CADnq5_PssA-Cz5q1Z-na88REbLn6pzz5T2eby12LUoyTj5F9dQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Alex Deucher @ 2018-04-12  4:32 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: Alex Deucher, Ranjbar, Ramin, Huang Rui, Christian Koenig, amd-gfx list

On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
<andrey.grodzovsky@amd.com> wrote:
> Reserved VRAM is used to avoid overriding pre OS FB.
> Once our display stack takes over we don't need the reserved
> VRAM anymore.
>
> v2:
> Remove comment, we know actually why we need to reserve the stolen VRAM.
> Fix return type for amdgpu_ttm_late_init.
> v3:
> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
> v4:
> Don't release stolen memory for GMC9 ASICs untill GART corruption
> on S3 resume is resolved.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Looks like you used the original version of this patch as well.
Updated version here:
https://patchwork.freedesktop.org/patch/215567/
more comments below.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  7 +++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 ++++++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  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      | 14 ++++++++++----
>  8 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 9e23d6f..a160ef0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -852,6 +852,13 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
>         return amdgpu_ttm_init(adev);
>  }
>
> +int amdgpu_bo_late_init(struct amdgpu_device *adev)
> +{
> +       amdgpu_ttm_late_init(adev);
> +
> +       return 0;
> +}
> +
>  void amdgpu_bo_fini(struct amdgpu_device *adev)
>  {
>         amdgpu_ttm_fini(adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 3bee133..1e9fe85 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -251,6 +251,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>  int amdgpu_bo_unpin(struct amdgpu_bo *bo);
>  int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
>  int amdgpu_bo_init(struct amdgpu_device *adev);
> +int amdgpu_bo_late_init(struct amdgpu_device *adev);
>  void amdgpu_bo_fini(struct amdgpu_device *adev);
>  int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
>                                 struct vm_area_struct *vma);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 0555821..7a608cf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1517,14 +1517,18 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>         return 0;
>  }
>
> +void amdgpu_ttm_late_init(struct amdgpu_device *adev)
> +{
> +       if (adev->gmc.stolen_size)

no need to check for NULL here.  amdgpu_bo_free_kernel() will do the
right thing even if stolen_vga_memory is NULL.

> +               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
> +}
> +
>  void amdgpu_ttm_fini(struct amdgpu_device *adev)
>  {
>         if (!adev->mman.initialized)
>                 return;
>
>         amdgpu_ttm_debugfs_fini(adev);
> -       if (adev->gmc.stolen_size)
> -               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
>         amdgpu_ttm_fw_reserve_vram_fini(adev);
>         if (adev->mman.aper_base_kaddr)
>                 iounmap(adev->mman.aper_base_kaddr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 6ea7de8..e969c87 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -77,6 +77,7 @@ uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man);
>  uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
>
>  int amdgpu_ttm_init(struct amdgpu_device *adev);
> +void amdgpu_ttm_late_init(struct amdgpu_device *adev);
>  void amdgpu_ttm_fini(struct amdgpu_device *adev);
>  void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev,
>                                         bool enable);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> index 63f0b65..4a8f9bd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> @@ -819,6 +819,8 @@ static int gmc_v6_0_late_init(void *handle)
>  {
>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> +       amdgpu_bo_late_init(adev);
> +
>         if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>                 return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>         else
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> index 2deb5c9..189fdf9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> @@ -958,6 +958,8 @@ static int gmc_v7_0_late_init(void *handle)
>  {
>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> +       amdgpu_bo_late_init(adev);
> +
>         if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>                 return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>         else
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> index 04b00df..19e153f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> @@ -1049,6 +1049,8 @@ static int gmc_v8_0_late_init(void *handle)
>  {
>         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> +       amdgpu_bo_late_init(adev);
> +
>         if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>                 return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>         else
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 252a6c69..099e3ce5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
>         unsigned i;
>         int r;
>
> +       /*
> +        * TODO:
> +        * Currently there is a bug where some memory client outside
> +        * of the driver writes to first 8M of VRAM on S3 resume,
> +        * this overrides GART which by default gets placed in first 8M and
> +        * causes VM_FAULTS once GTT is accessed.
> +        * Keep the stolen memory reservation until this solved.
> +        */
> +       /* amdgpu_bo_late_init(adev); /
> +

We still need to free this somewhere.  I'd suggest calling it in
gmc_v9_0_sw_fini() and add a comment there about moving it when we fix
the issue.

>         for(i = 0; i < adev->num_rings; ++i) {
>                 struct amdgpu_ring *ring = adev->rings[i];
>                 unsigned vmhub = ring->funcs->vmhub;
> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
>          */
>         adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>
> -       /*
> -        * It needs to reserve 8M stolen memory for vega10
> -        * TODO: Figure out how to avoid that...
> -        */
>         adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);

We may also just want to return 8MB or 9MB temporarily in
gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3
issue otherwise we're potentially wasting a lot more memory.

Alex


>
>         /* set DMA mask + need_dma32 flags.
> --
> 2.7.4
>
> _______________________________________________
> 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

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

* Re: [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over
       [not found]         ` <5e42d9cf-2581-14e1-80fd-0378323617c5-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12  7:49           ` Michel Dänzer
  2018-04-12  9:09           ` Huang Rui
  1 sibling, 0 replies; 13+ messages in thread
From: Michel Dänzer @ 2018-04-12  7:49 UTC (permalink / raw)
  To: Andrey Grodzovsky, Alex Deucher
  Cc: Deucher, Alexander, Ranjbar, Ramin, Huang Rui, Christian Koenig,
	amd-gfx list

On 2018-04-12 06:25 AM, Andrey Grodzovsky wrote:
> On 04/12/2018 12:16 AM, Alex Deucher wrote:
>> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
>> <andrey.grodzovsky@amd.com> wrote:
>>> From: Alex Deucher <alexdeucher@gmail.com>
>>>
>>> Steal 9 MB for vga emulation and fb if vga is enabled, otherwise,
>>> steal enough to cover the current display size as set by the vbios.
>>>
>>> If no memory is used (e.g., secondary or headless card), skip
>>> stolen memory reserve.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> Looks like you used v1 rather than v2 of this patch:
>> https://patchwork.freedesktop.org/patch/215566/
>>
>> Alex
> 
> To much patches circling around, NP, will respin tomorrow.

FWIW, it might help if everybody kept the status of their patches up to
date on https://patchwork.freedesktop.org/project/amd-xorg-ddx/patches/ :)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over
       [not found]         ` <5e42d9cf-2581-14e1-80fd-0378323617c5-5C7GfCeVMHo@public.gmane.org>
  2018-04-12  7:49           ` Michel Dänzer
@ 2018-04-12  9:09           ` Huang Rui
  1 sibling, 0 replies; 13+ messages in thread
From: Huang Rui @ 2018-04-12  9:09 UTC (permalink / raw)
  To: Grodzovsky, Andrey
  Cc: Alex Deucher, Deucher, Alexander, Ranjbar, Ramin, Koenig,
	Christian, amd-gfx list

On Thu, Apr 12, 2018 at 12:25:08PM +0800, Grodzovsky, Andrey wrote:
> 
> 
> On 04/12/2018 12:16 AM, Alex Deucher wrote:
> > On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
> > <andrey.grodzovsky@amd.com> wrote:
> >> From: Alex Deucher <alexdeucher@gmail.com>
> >>
> >> Steal 9 MB for vga emulation and fb if vga is enabled, otherwise,
> >> steal enough to cover the current display size as set by the vbios.
> >>
> >> If no memory is used (e.g., secondary or headless card), skip
> >> stolen memory reserve.
> >>
> >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > Looks like you used v1 rather than v2 of this patch:
> > https://patchwork.freedesktop.org/patch/215566/
> >
> > Alex
> 
> To much patches circling around, NP, will respin tomorrow.
> 

Nevermind, guy. I will verify them once you send the patches tomorrow. :-)

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

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

* Re: [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found]         ` <CADnq5_PssA-Cz5q1Z-na88REbLn6pzz5T2eby12LUoyTj5F9dQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-04-12 11:17           ` Andrey Grodzovsky
       [not found]             ` <9abad4d7-8ca7-30fa-6bb7-cadb4ae01799-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2018-04-12 11:17 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Alex Deucher, Ranjbar, Ramin, Huang Rui, Christian Koenig, amd-gfx list



On 04/12/2018 12:32 AM, Alex Deucher wrote:
> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
> <andrey.grodzovsky@amd.com> wrote:
>> Reserved VRAM is used to avoid overriding pre OS FB.
>> Once our display stack takes over we don't need the reserved
>> VRAM anymore.
>>
>> v2:
>> Remove comment, we know actually why we need to reserve the stolen VRAM.
>> Fix return type for amdgpu_ttm_late_init.
>> v3:
>> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
>> v4:
>> Don't release stolen memory for GMC9 ASICs untill GART corruption
>> on S3 resume is resolved.
>>
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> Looks like you used the original version of this patch as well.
> Updated version here:
> https://patchwork.freedesktop.org/patch/215567/
> more comments below.
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  7 +++++++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 ++++++--
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  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      | 14 ++++++++++----
>>   8 files changed, 31 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 9e23d6f..a160ef0 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -852,6 +852,13 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
>>          return amdgpu_ttm_init(adev);
>>   }
>>
>> +int amdgpu_bo_late_init(struct amdgpu_device *adev)
>> +{
>> +       amdgpu_ttm_late_init(adev);
>> +
>> +       return 0;
>> +}
>> +
>>   void amdgpu_bo_fini(struct amdgpu_device *adev)
>>   {
>>          amdgpu_ttm_fini(adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> index 3bee133..1e9fe85 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>> @@ -251,6 +251,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 domain,
>>   int amdgpu_bo_unpin(struct amdgpu_bo *bo);
>>   int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
>>   int amdgpu_bo_init(struct amdgpu_device *adev);
>> +int amdgpu_bo_late_init(struct amdgpu_device *adev);
>>   void amdgpu_bo_fini(struct amdgpu_device *adev);
>>   int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
>>                                  struct vm_area_struct *vma);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 0555821..7a608cf 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -1517,14 +1517,18 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>          return 0;
>>   }
>>
>> +void amdgpu_ttm_late_init(struct amdgpu_device *adev)
>> +{
>> +       if (adev->gmc.stolen_size)
> no need to check for NULL here.  amdgpu_bo_free_kernel() will do the
> right thing even if stolen_vga_memory is NULL.
>
>> +               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
>> +}
>> +
>>   void amdgpu_ttm_fini(struct amdgpu_device *adev)
>>   {
>>          if (!adev->mman.initialized)
>>                  return;
>>
>>          amdgpu_ttm_debugfs_fini(adev);
>> -       if (adev->gmc.stolen_size)
>> -               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL, NULL);
>>          amdgpu_ttm_fw_reserve_vram_fini(adev);
>>          if (adev->mman.aper_base_kaddr)
>>                  iounmap(adev->mman.aper_base_kaddr);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index 6ea7de8..e969c87 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -77,6 +77,7 @@ uint64_t amdgpu_vram_mgr_usage(struct ttm_mem_type_manager *man);
>>   uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
>>
>>   int amdgpu_ttm_init(struct amdgpu_device *adev);
>> +void amdgpu_ttm_late_init(struct amdgpu_device *adev);
>>   void amdgpu_ttm_fini(struct amdgpu_device *adev);
>>   void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev,
>>                                          bool enable);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> index 63f0b65..4a8f9bd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>> @@ -819,6 +819,8 @@ static int gmc_v6_0_late_init(void *handle)
>>   {
>>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>
>> +       amdgpu_bo_late_init(adev);
>> +
>>          if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>>                  return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>          else
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> index 2deb5c9..189fdf9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>> @@ -958,6 +958,8 @@ static int gmc_v7_0_late_init(void *handle)
>>   {
>>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>
>> +       amdgpu_bo_late_init(adev);
>> +
>>          if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>>                  return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>          else
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> index 04b00df..19e153f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>> @@ -1049,6 +1049,8 @@ static int gmc_v8_0_late_init(void *handle)
>>   {
>>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>
>> +       amdgpu_bo_late_init(adev);
>> +
>>          if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>>                  return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>          else
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 252a6c69..099e3ce5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
>>          unsigned i;
>>          int r;
>>
>> +       /*
>> +        * TODO:
>> +        * Currently there is a bug where some memory client outside
>> +        * of the driver writes to first 8M of VRAM on S3 resume,
>> +        * this overrides GART which by default gets placed in first 8M and
>> +        * causes VM_FAULTS once GTT is accessed.
>> +        * Keep the stolen memory reservation until this solved.
>> +        */
>> +       /* amdgpu_bo_late_init(adev); /
>> +
> We still need to free this somewhere.  I'd suggest calling it in
> gmc_v9_0_sw_fini() and add a comment there about moving it when we fix
> the issue.
>
>>          for(i = 0; i < adev->num_rings; ++i) {
>>                  struct amdgpu_ring *ring = adev->rings[i];
>>                  unsigned vmhub = ring->funcs->vmhub;
>> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
>>           */
>>          adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>
>> -       /*
>> -        * It needs to reserve 8M stolen memory for vega10
>> -        * TODO: Figure out how to avoid that...
>> -        */
>>          adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
> We may also just want to return 8MB or 9MB temporarily in
> gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3
> issue otherwise we're potentially wasting a lot more memory.
>
> Alex

But what if we have 4k display ? In this case returning 9M probably will 
not hide the corruption  we were originally dealing with. I remember in 
that case pre OS FB size would be 32M.

Andrey

>
>
>>          /* set DMA mask + need_dma32 flags.
>> --
>> 2.7.4
>>
>> _______________________________________________
>> 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

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

* Re: [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found]             ` <9abad4d7-8ca7-30fa-6bb7-cadb4ae01799-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 13:33               ` Alex Deucher
       [not found]                 ` <CADnq5_OrReLdTq6-K9ohDXRG7GDUd5MofoMpyOCUTwH2oufosw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Alex Deucher @ 2018-04-12 13:33 UTC (permalink / raw)
  To: Andrey Grodzovsky
  Cc: Alex Deucher, Ranjbar, Ramin, Huang Rui, Christian Koenig, amd-gfx list

On Thu, Apr 12, 2018 at 7:17 AM, Andrey Grodzovsky
<Andrey.Grodzovsky@amd.com> wrote:
>
>
> On 04/12/2018 12:32 AM, Alex Deucher wrote:
>>
>> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
>> <andrey.grodzovsky@amd.com> wrote:
>>>
>>> Reserved VRAM is used to avoid overriding pre OS FB.
>>> Once our display stack takes over we don't need the reserved
>>> VRAM anymore.
>>>
>>> v2:
>>> Remove comment, we know actually why we need to reserve the stolen VRAM.
>>> Fix return type for amdgpu_ttm_late_init.
>>> v3:
>>> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
>>> v4:
>>> Don't release stolen memory for GMC9 ASICs untill GART corruption
>>> on S3 resume is resolved.
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>
>> Looks like you used the original version of this patch as well.
>> Updated version here:
>> https://patchwork.freedesktop.org/patch/215567/
>> more comments below.
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  7 +++++++
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 ++++++--
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h    |  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      | 14 ++++++++++----
>>>   8 files changed, 31 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> index 9e23d6f..a160ef0 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>>> @@ -852,6 +852,13 @@ int amdgpu_bo_init(struct amdgpu_device *adev)
>>>          return amdgpu_ttm_init(adev);
>>>   }
>>>
>>> +int amdgpu_bo_late_init(struct amdgpu_device *adev)
>>> +{
>>> +       amdgpu_ttm_late_init(adev);
>>> +
>>> +       return 0;
>>> +}
>>> +
>>>   void amdgpu_bo_fini(struct amdgpu_device *adev)
>>>   {
>>>          amdgpu_ttm_fini(adev);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> index 3bee133..1e9fe85 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
>>> @@ -251,6 +251,7 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo,
>>> u32 domain,
>>>   int amdgpu_bo_unpin(struct amdgpu_bo *bo);
>>>   int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
>>>   int amdgpu_bo_init(struct amdgpu_device *adev);
>>> +int amdgpu_bo_late_init(struct amdgpu_device *adev);
>>>   void amdgpu_bo_fini(struct amdgpu_device *adev);
>>>   int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
>>>                                  struct vm_area_struct *vma);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 0555821..7a608cf 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1517,14 +1517,18 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>>>          return 0;
>>>   }
>>>
>>> +void amdgpu_ttm_late_init(struct amdgpu_device *adev)
>>> +{
>>> +       if (adev->gmc.stolen_size)
>>
>> no need to check for NULL here.  amdgpu_bo_free_kernel() will do the
>> right thing even if stolen_vga_memory is NULL.
>>
>>> +               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL,
>>> NULL);
>>> +}
>>> +
>>>   void amdgpu_ttm_fini(struct amdgpu_device *adev)
>>>   {
>>>          if (!adev->mman.initialized)
>>>                  return;
>>>
>>>          amdgpu_ttm_debugfs_fini(adev);
>>> -       if (adev->gmc.stolen_size)
>>> -               amdgpu_bo_free_kernel(&adev->stolen_vga_memory, NULL,
>>> NULL);
>>>          amdgpu_ttm_fw_reserve_vram_fini(adev);
>>>          if (adev->mman.aper_base_kaddr)
>>>                  iounmap(adev->mman.aper_base_kaddr);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index 6ea7de8..e969c87 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -77,6 +77,7 @@ uint64_t amdgpu_vram_mgr_usage(struct
>>> ttm_mem_type_manager *man);
>>>   uint64_t amdgpu_vram_mgr_vis_usage(struct ttm_mem_type_manager *man);
>>>
>>>   int amdgpu_ttm_init(struct amdgpu_device *adev);
>>> +void amdgpu_ttm_late_init(struct amdgpu_device *adev);
>>>   void amdgpu_ttm_fini(struct amdgpu_device *adev);
>>>   void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev,
>>>                                          bool enable);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> index 63f0b65..4a8f9bd 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
>>> @@ -819,6 +819,8 @@ static int gmc_v6_0_late_init(void *handle)
>>>   {
>>>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>>
>>> +       amdgpu_bo_late_init(adev);
>>> +
>>>          if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>>>                  return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>          else
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> index 2deb5c9..189fdf9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
>>> @@ -958,6 +958,8 @@ static int gmc_v7_0_late_init(void *handle)
>>>   {
>>>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>>
>>> +       amdgpu_bo_late_init(adev);
>>> +
>>>          if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>>>                  return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>          else
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> index 04b00df..19e153f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
>>> @@ -1049,6 +1049,8 @@ static int gmc_v8_0_late_init(void *handle)
>>>   {
>>>          struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>>>
>>> +       amdgpu_bo_late_init(adev);
>>> +
>>>          if (amdgpu_vm_fault_stop != AMDGPU_VM_FAULT_STOP_ALWAYS)
>>>                  return amdgpu_irq_get(adev, &adev->gmc.vm_fault, 0);
>>>          else
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> index 252a6c69..099e3ce5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
>>>          unsigned i;
>>>          int r;
>>>
>>> +       /*
>>> +        * TODO:
>>> +        * Currently there is a bug where some memory client outside
>>> +        * of the driver writes to first 8M of VRAM on S3 resume,
>>> +        * this overrides GART which by default gets placed in first 8M
>>> and
>>> +        * causes VM_FAULTS once GTT is accessed.
>>> +        * Keep the stolen memory reservation until this solved.
>>> +        */
>>> +       /* amdgpu_bo_late_init(adev); /
>>> +
>>
>> We still need to free this somewhere.  I'd suggest calling it in
>> gmc_v9_0_sw_fini() and add a comment there about moving it when we fix
>> the issue.
>>
>>>          for(i = 0; i < adev->num_rings; ++i) {
>>>                  struct amdgpu_ring *ring = adev->rings[i];
>>>                  unsigned vmhub = ring->funcs->vmhub;
>>> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
>>>           */
>>>          adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>
>>> -       /*
>>> -        * It needs to reserve 8M stolen memory for vega10
>>> -        * TODO: Figure out how to avoid that...
>>> -        */
>>>          adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
>>
>> We may also just want to return 8MB or 9MB temporarily in
>> gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3
>> issue otherwise we're potentially wasting a lot more memory.
>>
>> Alex
>
>
> But what if we have 4k display ? In this case returning 9M probably will not
> hide the corruption  we were originally dealing with. I remember in that
> case pre OS FB size would be 32M.

I guess it's a trade off, possible garbage monentary during bios to
driver transition vs. wasting an additional 24 MB of CPU accessible
vram for the life of the driver.

Alex


>
> Andrey
>
>
>>
>>
>>>          /* set DMA mask + need_dma32 flags.
>>> --
>>> 2.7.4
>>>
>>> _______________________________________________
>>> 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

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

* Re: [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found]                 ` <CADnq5_OrReLdTq6-K9ohDXRG7GDUd5MofoMpyOCUTwH2oufosw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2018-04-12 14:33                   ` Michel Dänzer
       [not found]                     ` <5968a37a-a89c-2c08-dfc4-dd6bc49f29be-otUistvHUpPR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Michel Dänzer @ 2018-04-12 14:33 UTC (permalink / raw)
  To: Alex Deucher, Andrey Grodzovsky
  Cc: Alex Deucher, Ranjbar, Ramin, Huang Rui, Christian Koenig, amd-gfx list

On 2018-04-12 03:33 PM, Alex Deucher wrote:
> On Thu, Apr 12, 2018 at 7:17 AM, Andrey Grodzovsky
> <Andrey.Grodzovsky@amd.com> wrote:
>> On 04/12/2018 12:32 AM, Alex Deucher wrote:
>>>
>>> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
>>> <andrey.grodzovsky@amd.com> wrote:
>>>>
>>>> Reserved VRAM is used to avoid overriding pre OS FB.
>>>> Once our display stack takes over we don't need the reserved
>>>> VRAM anymore.
>>>>
>>>> v2:
>>>> Remove comment, we know actually why we need to reserve the stolen VRAM.
>>>> Fix return type for amdgpu_ttm_late_init.
>>>> v3:
>>>> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
>>>> v4:
>>>> Don't release stolen memory for GMC9 ASICs untill GART corruption
>>>> on S3 resume is resolved.
>>>>
>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

[...]

>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> index 252a6c69..099e3ce5 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
>>>>          unsigned i;
>>>>          int r;
>>>>
>>>> +       /*
>>>> +        * TODO:
>>>> +        * Currently there is a bug where some memory client outside
>>>> +        * of the driver writes to first 8M of VRAM on S3 resume,
>>>> +        * this overrides GART which by default gets placed in first 8M
>>>> and
>>>> +        * causes VM_FAULTS once GTT is accessed.
>>>> +        * Keep the stolen memory reservation until this solved.
>>>> +        */
>>>> +       /* amdgpu_bo_late_init(adev); /
>>>> +
>>>
>>> We still need to free this somewhere.  I'd suggest calling it in
>>> gmc_v9_0_sw_fini() and add a comment there about moving it when we fix
>>> the issue.
>>>
>>>>          for(i = 0; i < adev->num_rings; ++i) {
>>>>                  struct amdgpu_ring *ring = adev->rings[i];
>>>>                  unsigned vmhub = ring->funcs->vmhub;
>>>> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>           */
>>>>          adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>
>>>> -       /*
>>>> -        * It needs to reserve 8M stolen memory for vega10
>>>> -        * TODO: Figure out how to avoid that...
>>>> -        */
>>>>          adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
>>>
>>> We may also just want to return 8MB or 9MB temporarily in
>>> gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3
>>> issue otherwise we're potentially wasting a lot more memory.
>>
>> But what if we have 4k display ? In this case returning 9M probably will not
>> hide the corruption  we were originally dealing with. I remember in that
>> case pre OS FB size would be 32M.
> 
> I guess it's a trade off, possible garbage monentary during bios to
> driver transition vs. wasting an additional 24 MB of CPU accessible
> vram for the life of the driver.

Can we free the reserved memory after initialization, then reserve it
again on resume?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found]                     ` <5968a37a-a89c-2c08-dfc4-dd6bc49f29be-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-04-12 14:58                       ` Andrey Grodzovsky
       [not found]                         ` <21a9adee-bec8-f86f-92a3-fcfca185458f-5C7GfCeVMHo@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Andrey Grodzovsky @ 2018-04-12 14:58 UTC (permalink / raw)
  To: Michel Dänzer, Alex Deucher
  Cc: Alex Deucher, Ranjbar, Ramin, Huang Rui, Christian Koenig, amd-gfx list



On 04/12/2018 10:33 AM, Michel Dänzer wrote:
> On 2018-04-12 03:33 PM, Alex Deucher wrote:
>> On Thu, Apr 12, 2018 at 7:17 AM, Andrey Grodzovsky
>> <Andrey.Grodzovsky@amd.com> wrote:
>>> On 04/12/2018 12:32 AM, Alex Deucher wrote:
>>>> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
>>>> <andrey.grodzovsky@amd.com> wrote:
>>>>> Reserved VRAM is used to avoid overriding pre OS FB.
>>>>> Once our display stack takes over we don't need the reserved
>>>>> VRAM anymore.
>>>>>
>>>>> v2:
>>>>> Remove comment, we know actually why we need to reserve the stolen VRAM.
>>>>> Fix return type for amdgpu_ttm_late_init.
>>>>> v3:
>>>>> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
>>>>> v4:
>>>>> Don't release stolen memory for GMC9 ASICs untill GART corruption
>>>>> on S3 resume is resolved.
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> [...]

Not sure what it means ?
>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> index 252a6c69..099e3ce5 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
>>>>>           unsigned i;
>>>>>           int r;
>>>>>
>>>>> +       /*
>>>>> +        * TODO:
>>>>> +        * Currently there is a bug where some memory client outside
>>>>> +        * of the driver writes to first 8M of VRAM on S3 resume,
>>>>> +        * this overrides GART which by default gets placed in first 8M
>>>>> and
>>>>> +        * causes VM_FAULTS once GTT is accessed.
>>>>> +        * Keep the stolen memory reservation until this solved.
>>>>> +        */
>>>>> +       /* amdgpu_bo_late_init(adev); /
>>>>> +
>>>> We still need to free this somewhere.  I'd suggest calling it in
>>>> gmc_v9_0_sw_fini() and add a comment there about moving it when we fix
>>>> the issue.
>>>>
>>>>>           for(i = 0; i < adev->num_rings; ++i) {
>>>>>                   struct amdgpu_ring *ring = adev->rings[i];
>>>>>                   unsigned vmhub = ring->funcs->vmhub;
>>>>> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>>            */
>>>>>           adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>>
>>>>> -       /*
>>>>> -        * It needs to reserve 8M stolen memory for vega10
>>>>> -        * TODO: Figure out how to avoid that...
>>>>> -        */
>>>>>           adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
>>>> We may also just want to return 8MB or 9MB temporarily in
>>>> gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3
>>>> issue otherwise we're potentially wasting a lot more memory.
>>> But what if we have 4k display ? In this case returning 9M probably will not
>>> hide the corruption  we were originally dealing with. I remember in that
>>> case pre OS FB size would be 32M.
>> I guess it's a trade off, possible garbage monentary during bios to
>> driver transition vs. wasting an additional 24 MB of CPU accessible
>> vram for the life of the driver.
> Can we free the reserved memory after initialization, then reserve it
> again on resume?

The issue here was that someone overrides the first 8M of VRAM and 
corrupts the GART table, which causes VM_FAULTS. Until we find who is 
writing into this area of VRAM and when exactly I think we cannot allow 
any data to be placed there since it's might get corrupted (even if we 
avoid placing the GART table there).

Andrey
>
>

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

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

* Re: [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found]                         ` <21a9adee-bec8-f86f-92a3-fcfca185458f-5C7GfCeVMHo@public.gmane.org>
@ 2018-04-12 15:10                           ` Michel Dänzer
       [not found]                             ` <cf801752-d1e7-f8de-3312-c7b4677dcf49-otUistvHUpPR7s880joybQ@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Michel Dänzer @ 2018-04-12 15:10 UTC (permalink / raw)
  To: Andrey Grodzovsky, Alex Deucher
  Cc: Alex Deucher, Ranjbar, Ramin, Huang Rui, Christian Koenig, amd-gfx list

On 2018-04-12 04:58 PM, Andrey Grodzovsky wrote:
> On 04/12/2018 10:33 AM, Michel Dänzer wrote:
>> On 2018-04-12 03:33 PM, Alex Deucher wrote:
>>> On Thu, Apr 12, 2018 at 7:17 AM, Andrey Grodzovsky
>>> <Andrey.Grodzovsky@amd.com> wrote:
>>>> On 04/12/2018 12:32 AM, Alex Deucher wrote:
>>>>> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
>>>>> <andrey.grodzovsky@amd.com> wrote:
>>>>>> Reserved VRAM is used to avoid overriding pre OS FB.
>>>>>> Once our display stack takes over we don't need the reserved
>>>>>> VRAM anymore.
>>>>>>
>>>>>> v2:
>>>>>> Remove comment, we know actually why we need to reserve the stolen
>>>>>> VRAM.
>>>>>> Fix return type for amdgpu_ttm_late_init.
>>>>>> v3:
>>>>>> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
>>>>>> v4:
>>>>>> Don't release stolen memory for GMC9 ASICs untill GART corruption
>>>>>> on S3 resume is resolved.
>>>>>>
>>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> [...]
> 
> Not sure what it means ?

It means I trimmed some quoted text. Would it be clearer if I put
quotation markers before it?


>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> index 252a6c69..099e3ce5 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
>>>>>>           unsigned i;
>>>>>>           int r;
>>>>>>
>>>>>> +       /*
>>>>>> +        * TODO:
>>>>>> +        * Currently there is a bug where some memory client outside
>>>>>> +        * of the driver writes to first 8M of VRAM on S3 resume,
>>>>>> +        * this overrides GART which by default gets placed in
>>>>>> first 8M
>>>>>> and
>>>>>> +        * causes VM_FAULTS once GTT is accessed.
>>>>>> +        * Keep the stolen memory reservation until this solved.
>>>>>> +        */
>>>>>> +       /* amdgpu_bo_late_init(adev); /
>>>>>> +
>>>>> We still need to free this somewhere.  I'd suggest calling it in
>>>>> gmc_v9_0_sw_fini() and add a comment there about moving it when we fix
>>>>> the issue.
>>>>>
>>>>>>           for(i = 0; i < adev->num_rings; ++i) {
>>>>>>                   struct amdgpu_ring *ring = adev->rings[i];
>>>>>>                   unsigned vmhub = ring->funcs->vmhub;
>>>>>> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>>>            */
>>>>>>           adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>>>
>>>>>> -       /*
>>>>>> -        * It needs to reserve 8M stolen memory for vega10
>>>>>> -        * TODO: Figure out how to avoid that...
>>>>>> -        */
>>>>>>           adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
>>>>> We may also just want to return 8MB or 9MB temporarily in
>>>>> gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3
>>>>> issue otherwise we're potentially wasting a lot more memory.
>>>> But what if we have 4k display ? In this case returning 9M probably
>>>> will not
>>>> hide the corruption  we were originally dealing with. I remember in
>>>> that
>>>> case pre OS FB size would be 32M.
>>> I guess it's a trade off, possible garbage monentary during bios to
>>> driver transition vs. wasting an additional 24 MB of CPU accessible
>>> vram for the life of the driver.
>> Can we free the reserved memory after initialization, then reserve it
>> again on resume?
> 
> The issue here was that someone overrides the first 8M of VRAM and
> corrupts the GART table, which causes VM_FAULTS. Until we find who is
> writing into this area of VRAM and when exactly I think we cannot allow
> any data to be placed there since it's might get corrupted (even if we
> avoid placing the GART table there).

I think it shouldn't be too hard in general to make sure the GART table,
and any other BOs which stay in VRAM across suspend/resume, don't fall
in the affected area.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer

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

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

* Re: [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible.
       [not found]                             ` <cf801752-d1e7-f8de-3312-c7b4677dcf49-otUistvHUpPR7s880joybQ@public.gmane.org>
@ 2018-04-12 19:59                               ` Andrey Grodzovsky
  0 siblings, 0 replies; 13+ messages in thread
From: Andrey Grodzovsky @ 2018-04-12 19:59 UTC (permalink / raw)
  To: Michel Dänzer, Alex Deucher
  Cc: Alex Deucher, Ranjbar, Ramin, Huang Rui, Christian Koenig, amd-gfx list



On 04/12/2018 11:10 AM, Michel Dänzer wrote:
> On 2018-04-12 04:58 PM, Andrey Grodzovsky wrote:
>> On 04/12/2018 10:33 AM, Michel Dänzer wrote:
>>> On 2018-04-12 03:33 PM, Alex Deucher wrote:
>>>> On Thu, Apr 12, 2018 at 7:17 AM, Andrey Grodzovsky
>>>> <Andrey.Grodzovsky@amd.com> wrote:
>>>>> On 04/12/2018 12:32 AM, Alex Deucher wrote:
>>>>>> On Thu, Apr 12, 2018 at 12:08 AM, Andrey Grodzovsky
>>>>>> <andrey.grodzovsky@amd.com> wrote:
>>>>>>> Reserved VRAM is used to avoid overriding pre OS FB.
>>>>>>> Once our display stack takes over we don't need the reserved
>>>>>>> VRAM anymore.
>>>>>>>
>>>>>>> v2:
>>>>>>> Remove comment, we know actually why we need to reserve the stolen
>>>>>>> VRAM.
>>>>>>> Fix return type for amdgpu_ttm_late_init.
>>>>>>> v3:
>>>>>>> Return 0 in amdgpu_bo_late_init, rebase on changes to previous patch
>>>>>>> v4:
>>>>>>> Don't release stolen memory for GMC9 ASICs untill GART corruption
>>>>>>> on S3 resume is resolved.
>>>>>>>
>>>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>>>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>>> [...]
>> Not sure what it means ?
> It means I trimmed some quoted text. Would it be clearer if I put
> quotation markers before it?

Got it now.

>
>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>>> index 252a6c69..099e3ce5 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>>>>>>> @@ -659,6 +659,16 @@ static int gmc_v9_0_late_init(void *handle)
>>>>>>>            unsigned i;
>>>>>>>            int r;
>>>>>>>
>>>>>>> +       /*
>>>>>>> +        * TODO:
>>>>>>> +        * Currently there is a bug where some memory client outside
>>>>>>> +        * of the driver writes to first 8M of VRAM on S3 resume,
>>>>>>> +        * this overrides GART which by default gets placed in
>>>>>>> first 8M
>>>>>>> and
>>>>>>> +        * causes VM_FAULTS once GTT is accessed.
>>>>>>> +        * Keep the stolen memory reservation until this solved.
>>>>>>> +        */
>>>>>>> +       /* amdgpu_bo_late_init(adev); /
>>>>>>> +
>>>>>> We still need to free this somewhere.  I'd suggest calling it in
>>>>>> gmc_v9_0_sw_fini() and add a comment there about moving it when we fix
>>>>>> the issue.
>>>>>>
>>>>>>>            for(i = 0; i < adev->num_rings; ++i) {
>>>>>>>                    struct amdgpu_ring *ring = adev->rings[i];
>>>>>>>                    unsigned vmhub = ring->funcs->vmhub;
>>>>>>> @@ -884,10 +894,6 @@ static int gmc_v9_0_sw_init(void *handle)
>>>>>>>             */
>>>>>>>            adev->gmc.mc_mask = 0xffffffffffffULL; /* 48 bit MC */
>>>>>>>
>>>>>>> -       /*
>>>>>>> -        * It needs to reserve 8M stolen memory for vega10
>>>>>>> -        * TODO: Figure out how to avoid that...
>>>>>>> -        */
>>>>>>>            adev->gmc.stolen_size = gmc_v9_0_get_vbios_fb_size(adev);
>>>>>> We may also just want to return 8MB or 9MB temporarily in
>>>>>> gmc_v9_0_get_vbios_fb_size until we sort out the root cause of the S3
>>>>>> issue otherwise we're potentially wasting a lot more memory.
>>>>> But what if we have 4k display ? In this case returning 9M probably
>>>>> will not
>>>>> hide the corruption  we were originally dealing with. I remember in
>>>>> that
>>>>> case pre OS FB size would be 32M.
>>>> I guess it's a trade off, possible garbage monentary during bios to
>>>> driver transition vs. wasting an additional 24 MB of CPU accessible
>>>> vram for the life of the driver.
>>> Can we free the reserved memory after initialization, then reserve it
>>> again on resume?
>> The issue here was that someone overrides the first 8M of VRAM and
>> corrupts the GART table, which causes VM_FAULTS. Until we find who is
>> writing into this area of VRAM and when exactly I think we cannot allow
>> any data to be placed there since it's might get corrupted (even if we
>> avoid placing the GART table there).
> I think it shouldn't be too hard in general to make sure the GART table,
> and any other BOs which stay in VRAM across suspend/resume, don't fall
> in the affected area.

Not sure I understand how easily to search for all this kind of objects 
across all IPs. Also how can we be sure the
effected region is ran over only during resume, we know that GART table 
is ran over during resume but we can't be
sure other areas in that region are not ran over during other times and 
if so it's dangerous to allow allocations there.

Andrey

>
>

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

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

end of thread, other threads:[~2018-04-12 19:59 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-12  4:08 [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over Andrey Grodzovsky
     [not found] ` <1523506110-14002-1-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
2018-04-12  4:08   ` [PATCH v4 2/2] drm/amdgpu: Free VGA stolen memory as soon as possible Andrey Grodzovsky
     [not found]     ` <1523506110-14002-2-git-send-email-andrey.grodzovsky-5C7GfCeVMHo@public.gmane.org>
2018-04-12  4:32       ` Alex Deucher
     [not found]         ` <CADnq5_PssA-Cz5q1Z-na88REbLn6pzz5T2eby12LUoyTj5F9dQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-04-12 11:17           ` Andrey Grodzovsky
     [not found]             ` <9abad4d7-8ca7-30fa-6bb7-cadb4ae01799-5C7GfCeVMHo@public.gmane.org>
2018-04-12 13:33               ` Alex Deucher
     [not found]                 ` <CADnq5_OrReLdTq6-K9ohDXRG7GDUd5MofoMpyOCUTwH2oufosw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-04-12 14:33                   ` Michel Dänzer
     [not found]                     ` <5968a37a-a89c-2c08-dfc4-dd6bc49f29be-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-04-12 14:58                       ` Andrey Grodzovsky
     [not found]                         ` <21a9adee-bec8-f86f-92a3-fcfca185458f-5C7GfCeVMHo@public.gmane.org>
2018-04-12 15:10                           ` Michel Dänzer
     [not found]                             ` <cf801752-d1e7-f8de-3312-c7b4677dcf49-otUistvHUpPR7s880joybQ@public.gmane.org>
2018-04-12 19:59                               ` Andrey Grodzovsky
2018-04-12  4:16   ` [PATCH v4 1/2] drm/amdgpu/gmc: steal the appropriate amount of vram for fw hand-over Alex Deucher
     [not found]     ` <CADnq5_Nw0Cwh-pg9==CWLRC2E709b2S8LEWMd93Eo+ZRfZ7GCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-04-12  4:25       ` Andrey Grodzovsky
     [not found]         ` <5e42d9cf-2581-14e1-80fd-0378323617c5-5C7GfCeVMHo@public.gmane.org>
2018-04-12  7:49           ` Michel Dänzer
2018-04-12  9:09           ` Huang Rui

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