Fix GPU triggering random system read after VRAM start change

Fix GPU triggering random system read after VRAM start change

[j.glisse]

So first patch is a fix in itself, smallest possible and should go to
stable. Second patch is an improvement as a first step to flicker free
boot.

I have yet extensively tested second patch, especialy not on AGP but
so far on few GPU/motherboard it looks good. It can probably wait 3.7.
Will test it more and report.

I have a third patch that is a step closer to flicker free boot on uefi,
waiting ack to release a reg.

Cheers,
Jerome

[PATCH 1/3] drm/radeon: do not reenable crtc after moving vram start address

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

It seems we can not update the crtc scanout address. After disabling
crtc, update to base address do not take effect after crtc being
reenable leading to at least frame being scanout from the old crtc
base address. Disabling crtc display request lead to same behavior.

So after changing the vram address if we don't keep crtc disabled
we will have the GPU trying to read some random system memory address
with some iommu this will broke the crtc engine and will lead to
broken display and iommu error message.

So to avoid this, disable crtc. For flicker less boot we will need
to avoid moving the vram start address.

This patch should also fix :

https://bugs.freedesktop.org/show_bug.cgi?id=42373

Cc: <stable@vger.kernel.org>
Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c   |   57 ----------------------------------
 drivers/gpu/drm/radeon/radeon_asic.h |    8 ++---
 drivers/gpu/drm/radeon/rv515.c       |   13 --------
 3 files changed, 2 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index e585a3b..db85262 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -1229,24 +1229,8 @@ void evergreen_agp_enable(struct radeon_device *rdev)
 
 void evergreen_mc_stop(struct radeon_device *rdev, struct evergreen_mc_save *save)
 {
-	save->vga_control[0] = RREG32(D1VGA_CONTROL);
-	save->vga_control[1] = RREG32(D2VGA_CONTROL);
 	save->vga_render_control = RREG32(VGA_RENDER_CONTROL);
 	save->vga_hdp_control = RREG32(VGA_HDP_CONTROL);
-	save->crtc_control[0] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET);
-	save->crtc_control[1] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET);
-	if (rdev->num_crtc >= 4) {
-		save->vga_control[2] = RREG32(EVERGREEN_D3VGA_CONTROL);
-		save->vga_control[3] = RREG32(EVERGREEN_D4VGA_CONTROL);
-		save->crtc_control[2] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET);
-		save->crtc_control[3] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET);
-	}
-	if (rdev->num_crtc >= 6) {
-		save->vga_control[4] = RREG32(EVERGREEN_D5VGA_CONTROL);
-		save->vga_control[5] = RREG32(EVERGREEN_D6VGA_CONTROL);
-		save->crtc_control[4] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET);
-		save->crtc_control[5] = RREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET);
-	}
 
 	/* Stop all video */
 	WREG32(VGA_RENDER_CONTROL, 0);
@@ -1357,47 +1341,6 @@ void evergreen_mc_resume(struct radeon_device *rdev, struct evergreen_mc_save *s
 	/* Unlock host access */
 	WREG32(VGA_HDP_CONTROL, save->vga_hdp_control);
 	mdelay(1);
-	/* Restore video state */
-	WREG32(D1VGA_CONTROL, save->vga_control[0]);
-	WREG32(D2VGA_CONTROL, save->vga_control[1]);
-	if (rdev->num_crtc >= 4) {
-		WREG32(EVERGREEN_D3VGA_CONTROL, save->vga_control[2]);
-		WREG32(EVERGREEN_D4VGA_CONTROL, save->vga_control[3]);
-	}
-	if (rdev->num_crtc >= 6) {
-		WREG32(EVERGREEN_D5VGA_CONTROL, save->vga_control[4]);
-		WREG32(EVERGREEN_D6VGA_CONTROL, save->vga_control[5]);
-	}
-	WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC0_REGISTER_OFFSET, 1);
-	WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC1_REGISTER_OFFSET, 1);
-	if (rdev->num_crtc >= 4) {
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC2_REGISTER_OFFSET, 1);
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC3_REGISTER_OFFSET, 1);
-	}
-	if (rdev->num_crtc >= 6) {
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC4_REGISTER_OFFSET, 1);
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC5_REGISTER_OFFSET, 1);
-	}
-	WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC0_REGISTER_OFFSET, save->crtc_control[0]);
-	WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC1_REGISTER_OFFSET, save->crtc_control[1]);
-	if (rdev->num_crtc >= 4) {
-		WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC2_REGISTER_OFFSET, save->crtc_control[2]);
-		WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC3_REGISTER_OFFSET, save->crtc_control[3]);
-	}
-	if (rdev->num_crtc >= 6) {
-		WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC4_REGISTER_OFFSET, save->crtc_control[4]);
-		WREG32(EVERGREEN_CRTC_CONTROL + EVERGREEN_CRTC5_REGISTER_OFFSET, save->crtc_control[5]);
-	}
-	WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC0_REGISTER_OFFSET, 0);
-	WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC1_REGISTER_OFFSET, 0);
-	if (rdev->num_crtc >= 4) {
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC2_REGISTER_OFFSET, 0);
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC3_REGISTER_OFFSET, 0);
-	}
-	if (rdev->num_crtc >= 6) {
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC4_REGISTER_OFFSET, 0);
-		WREG32(EVERGREEN_CRTC_UPDATE_LOCK + EVERGREEN_CRTC5_REGISTER_OFFSET, 0);
-	}
 	WREG32(VGA_RENDER_CONTROL, save->vga_render_control);
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
index f4af243..0d445e7 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.h
+++ b/drivers/gpu/drm/radeon/radeon_asic.h
@@ -255,13 +255,10 @@ extern int rs690_mc_wait_for_idle(struct radeon_device *rdev);
  * rv515
  */
 struct rv515_mc_save {
-	u32 d1vga_control;
-	u32 d2vga_control;
 	u32 vga_render_control;
 	u32 vga_hdp_control;
-	u32 d1crtc_control;
-	u32 d2crtc_control;
 };
+
 int rv515_init(struct radeon_device *rdev);
 void rv515_fini(struct radeon_device *rdev);
 uint32_t rv515_mc_rreg(struct radeon_device *rdev, uint32_t reg);
@@ -389,11 +386,10 @@ void r700_cp_fini(struct radeon_device *rdev);
  * evergreen
  */
 struct evergreen_mc_save {
-	u32 vga_control[6];
 	u32 vga_render_control;
 	u32 vga_hdp_control;
-	u32 crtc_control[6];
 };
+
 void evergreen_pcie_gart_tlb_flush(struct radeon_device *rdev);
 int evergreen_init(struct radeon_device *rdev);
 void evergreen_fini(struct radeon_device *rdev);
diff --git a/drivers/gpu/drm/radeon/rv515.c b/drivers/gpu/drm/radeon/rv515.c
index a12fbcc..aa8ef49 100644
--- a/drivers/gpu/drm/radeon/rv515.c
+++ b/drivers/gpu/drm/radeon/rv515.c
@@ -281,12 +281,8 @@ int rv515_debugfs_ga_info_init(struct radeon_device *rdev)
 
 void rv515_mc_stop(struct radeon_device *rdev, struct rv515_mc_save *save)
 {
-	save->d1vga_control = RREG32(R_000330_D1VGA_CONTROL);
-	save->d2vga_control = RREG32(R_000338_D2VGA_CONTROL);
 	save->vga_render_control = RREG32(R_000300_VGA_RENDER_CONTROL);
 	save->vga_hdp_control = RREG32(R_000328_VGA_HDP_CONTROL);
-	save->d1crtc_control = RREG32(R_006080_D1CRTC_CONTROL);
-	save->d2crtc_control = RREG32(R_006880_D2CRTC_CONTROL);
 
 	/* Stop all video */
 	WREG32(R_0068E8_D2CRTC_UPDATE_LOCK, 0);
@@ -311,15 +307,6 @@ void rv515_mc_resume(struct radeon_device *rdev, struct rv515_mc_save *save)
 	/* Unlock host access */
 	WREG32(R_000328_VGA_HDP_CONTROL, save->vga_hdp_control);
 	mdelay(1);
-	/* Restore video state */
-	WREG32(R_000330_D1VGA_CONTROL, save->d1vga_control);
-	WREG32(R_000338_D2VGA_CONTROL, save->d2vga_control);
-	WREG32(R_0060E8_D1CRTC_UPDATE_LOCK, 1);
-	WREG32(R_0068E8_D2CRTC_UPDATE_LOCK, 1);
-	WREG32(R_006080_D1CRTC_CONTROL, save->d1crtc_control);
-	WREG32(R_006880_D2CRTC_CONTROL, save->d2crtc_control);
-	WREG32(R_0060E8_D1CRTC_UPDATE_LOCK, 0);
-	WREG32(R_0068E8_D2CRTC_UPDATE_LOCK, 0);
 	WREG32(R_000300_VGA_RENDER_CONTROL, save->vga_render_control);
 }
 
-- 
1.7.10.4

[PATCH 2/3] drm/radeon: try to keep current vram GPU address

[j.glisse]

From: Jerome Glisse <jglisse@redhat.com>

It seems we can't move the VRAM GPU address without disabling CRTC.
Thus if we want to support flicker free boot from UEFI to X, we need
to keep the VRAM GPU address UEFI programmed. So far on all UEFI
checked this address was something sane.

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c   |   10 +++-
 drivers/gpu/drm/radeon/r600.c        |  105 ++++++++++++++++++++--------------
 drivers/gpu/drm/radeon/radeon_asic.h |    3 +-
 drivers/gpu/drm/radeon/rv770.c       |   44 ++------------
 drivers/gpu/drm/radeon/si.c          |   59 ++-----------------
 5 files changed, 80 insertions(+), 141 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index db85262..c56fa0a 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -2166,6 +2166,7 @@ static void evergreen_gpu_init(struct radeon_device *rdev)
 
 int evergreen_mc_init(struct radeon_device *rdev)
 {
+	u64 fb_base;
 	u32 tmp;
 	int chansize, numchan;
 
@@ -2217,7 +2218,14 @@ int evergreen_mc_init(struct radeon_device *rdev)
 		rdev->mc.real_vram_size = RREG32(CONFIG_MEMSIZE) * 1024 * 1024;
 	}
 	rdev->mc.visible_vram_size = rdev->mc.aper_size;
-	r700_vram_gtt_location(rdev, &rdev->mc);
+	fb_base = RREG32(MC_VM_FB_LOCATION) & 0xffff;
+	fb_base <<= 24;
+	if ((rdev->family == CHIP_PALM) ||
+	    (rdev->family == CHIP_SUMO) ||
+	    (rdev->family == CHIP_SUMO2)) {
+		fb_base |= ((RREG32(MC_FUS_VM_FB_OFFSET) >> 20) & 0xf) << 20;
+	}
+	r600_vram_gtt_location(rdev, &rdev->mc, fb_base, 1ULL << 36ULL);
 	radeon_update_bandwidth_info(rdev);
 
 	return 0;
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 637280f..87f47b6 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -1097,69 +1097,84 @@ static void r600_mc_program(struct radeon_device *rdev)
  * r600_vram_gtt_location - try to find VRAM & GTT location
  * @rdev: radeon device structure holding all necessary informations
  * @mc: memory controller structure holding memory informations
+ * @fb_base: current start address of VRAM
+ * @mc_max: maximum address supported by the MC controler (32bits on
+ *          r6xx/r7xx, 36bits on evergreen, 40bits cayman and newer)
  *
- * Function will place try to place VRAM at same place as in CPU (PCI)
- * address space as some GPU seems to have issue when we reprogram at
- * different address space.
+ * Function will place try to not move VRAM start address. But if VRAM
+ * conflict with AGP then we move VRAM as we have more issue with moving
+ * AGP than with moving VRAM start address.
  *
- * If there is not enough space to fit the unvisible VRAM after the
- * aperture then we limit the VRAM size to the aperture.
+ * We will also move VRAM if current address + VRAM size overflow mc_max.
  *
- * If we are using AGP then place VRAM adjacent to AGP aperture are we need
- * them to be in one from GPU point of view so that we can program GPU to
- * catch access outside them (weird GPU policy see ??).
- *
- * This function will never fails, worst case are limiting VRAM or GTT.
+ * GTT is place before VRAM if there is enough room there, or after VRAM.
+ * GTT is shrink to maximum size we can find, we shrink GTT instead of
+ * VRAM as VRAM is better than GTT :)
  *
  * Note: GTT start, end, size should be initialized before calling this
  * function on AGP platform.
  */
-static void r600_vram_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc)
+void r600_vram_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc,
+				uint64_t fb, uint64_t mc_max)
 {
-	u64 size_bf, size_af;
+	/* try to keep same vram address */
+	mc->vram_start = fb;
 
-	if (mc->mc_vram_size > 0xE0000000) {
-		/* leave room for at least 512M GTT */
-		dev_warn(rdev->dev, "limiting VRAM\n");
-		mc->real_vram_size = 0xE0000000;
-		mc->mc_vram_size = 0xE0000000;
+	/* make sure there is enough room for vram */
+	if ((mc->vram_start + mc->mc_vram_size) > mc_max) {
+		/* moving fb, show me the GPU on which this happen free beer reward */
+		mc->vram_start = mc_max - mc->mc_vram_size;
 	}
+	mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
+
 	if (rdev->flags & RADEON_IS_AGP) {
-		size_bf = mc->gtt_start;
-		size_af = 0xFFFFFFFF - mc->gtt_end;
-		if (size_bf > size_af) {
-			if (mc->mc_vram_size > size_bf) {
-				dev_warn(rdev->dev, "limiting VRAM\n");
-				mc->real_vram_size = size_bf;
-				mc->mc_vram_size = size_bf;
-			}
-			mc->vram_start = mc->gtt_start - mc->mc_vram_size;
-		} else {
-			if (mc->mc_vram_size > size_af) {
-				dev_warn(rdev->dev, "limiting VRAM\n");
-				mc->real_vram_size = size_af;
-				mc->mc_vram_size = size_af;
+		if ((fb + mc->mc_vram_size) >= mc->gtt_start && fb < mc->gtt_end) {
+			/* current configuration overlap */
+			if (mc->gtt_start > mc->mc_vram_size) {
+				mc->vram_start = 0;
+			} else {
+				mc->vram_start = mc->gtt_end + 1;
+				if ((mc->vram_start + mc->mc_vram_size) > mc_max) {
+					/* show me the GPU on which this happen free beer reward */
+					mc->real_vram_size = mc_max - mc->vram_start;
+					mc->mc_vram_size = mc->real_vram_size;
+					dev_warn(rdev->dev, "limiting VRAM 0x%016llx\n", mc->mc_vram_size);
+				}
 			}
-			mc->vram_start = mc->gtt_end + 1;
+			mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
 		}
-		mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
-		dev_info(rdev->dev, "VRAM: %lluM 0x%08llX - 0x%08llX (%lluM used)\n",
-				mc->mc_vram_size >> 20, mc->vram_start,
-				mc->vram_end, mc->real_vram_size >> 20);
 	} else {
-		u64 base = 0;
-		if (rdev->flags & RADEON_IS_IGP) {
-			base = RREG32(MC_VM_FB_LOCATION) & 0xFFFF;
-			base <<= 24;
+		mc->gtt_start = 0;
+		if (mc->gtt_size > mc->vram_start) {
+			mc->gtt_start = mc->vram_end + 1;
+			if ((mc->gtt_start + mc->gtt_size) > mc_max) {
+				/* limit gtt */
+				u64 size_bf, size_af;
+
+				size_bf = mc->vram_start;
+				size_af = mc_max - mc->gtt_start;
+				if (size_bf > size_af) {
+					mc->gtt_start = 0;
+					mc->gtt_size = size_bf;
+				} else {
+					mc->gtt_size = size_af;
+				}
+				dev_warn(rdev->dev, "limiting GTT to 0x%016llx\n", mc->gtt_size);
+			}
 		}
-		radeon_vram_location(rdev, &rdev->mc, base);
-		rdev->mc.gtt_base_align = 0;
-		radeon_gtt_location(rdev, mc);
+		mc->gtt_end = mc->gtt_start + mc->gtt_size - 1;
 	}
+
+	dev_info(rdev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
+			mc->mc_vram_size >> 20, mc->vram_start,
+			mc->vram_end, mc->real_vram_size >> 20);
+	dev_info(rdev->dev, "GTT: %lluM 0x%016llX - 0x%016llX\n",
+			mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
 }
 
 int r600_mc_init(struct radeon_device *rdev)
 {
+	u64 fb_base;
 	u32 tmp;
 	int chansize, numchan;
 
@@ -1197,7 +1212,9 @@ int r600_mc_init(struct radeon_device *rdev)
 	rdev->mc.mc_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.real_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.visible_vram_size = rdev->mc.aper_size;
-	r600_vram_gtt_location(rdev, &rdev->mc);
+	fb_base = RREG32(MC_VM_FB_LOCATION) & 0xFFFF;
+	fb_base <<= 24ULL;
+	r600_vram_gtt_location(rdev, &rdev->mc, fb_base, 1ULL << 32ULL);
 
 	if (rdev->flags & RADEON_IS_IGP) {
 		rs690_pm_info(rdev);
diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
index 0d445e7..accd2ca 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.h
+++ b/drivers/gpu/drm/radeon/radeon_asic.h
@@ -368,6 +368,8 @@ void r600_kms_blit_copy(struct radeon_device *rdev,
 			unsigned num_gpu_pages,
 			struct radeon_sa_bo *vb);
 int r600_mc_wait_for_idle(struct radeon_device *rdev);
+void r600_vram_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc,
+				uint64_t fb, uint64_t mc_max);
 
 /*
  * rv770,rv730,rv710,rv740
@@ -378,7 +380,6 @@ int rv770_suspend(struct radeon_device *rdev);
 int rv770_resume(struct radeon_device *rdev);
 void rv770_pm_misc(struct radeon_device *rdev);
 u32 rv770_page_flip(struct radeon_device *rdev, int crtc, u64 crtc_base);
-void r700_vram_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc);
 void r700_cp_stop(struct radeon_device *rdev);
 void r700_cp_fini(struct radeon_device *rdev);
 
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index ca8ffec..a5b915d 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -800,47 +800,9 @@ static void rv770_gpu_init(struct radeon_device *rdev)
 	WREG32(VC_ENHANCE, 0);
 }
 
-void r700_vram_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc)
-{
-	u64 size_bf, size_af;
-
-	if (mc->mc_vram_size > 0xE0000000) {
-		/* leave room for at least 512M GTT */
-		dev_warn(rdev->dev, "limiting VRAM\n");
-		mc->real_vram_size = 0xE0000000;
-		mc->mc_vram_size = 0xE0000000;
-	}
-	if (rdev->flags & RADEON_IS_AGP) {
-		size_bf = mc->gtt_start;
-		size_af = 0xFFFFFFFF - mc->gtt_end;
-		if (size_bf > size_af) {
-			if (mc->mc_vram_size > size_bf) {
-				dev_warn(rdev->dev, "limiting VRAM\n");
-				mc->real_vram_size = size_bf;
-				mc->mc_vram_size = size_bf;
-			}
-			mc->vram_start = mc->gtt_start - mc->mc_vram_size;
-		} else {
-			if (mc->mc_vram_size > size_af) {
-				dev_warn(rdev->dev, "limiting VRAM\n");
-				mc->real_vram_size = size_af;
-				mc->mc_vram_size = size_af;
-			}
-			mc->vram_start = mc->gtt_end + 1;
-		}
-		mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
-		dev_info(rdev->dev, "VRAM: %lluM 0x%08llX - 0x%08llX (%lluM used)\n",
-				mc->mc_vram_size >> 20, mc->vram_start,
-				mc->vram_end, mc->real_vram_size >> 20);
-	} else {
-		radeon_vram_location(rdev, &rdev->mc, 0);
-		rdev->mc.gtt_base_align = 0;
-		radeon_gtt_location(rdev, mc);
-	}
-}
-
 int rv770_mc_init(struct radeon_device *rdev)
 {
+	u64 fb_base;
 	u32 tmp;
 	int chansize, numchan;
 
@@ -878,7 +840,9 @@ int rv770_mc_init(struct radeon_device *rdev)
 	rdev->mc.mc_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.real_vram_size = RREG32(CONFIG_MEMSIZE);
 	rdev->mc.visible_vram_size = rdev->mc.aper_size;
-	r700_vram_gtt_location(rdev, &rdev->mc);
+	fb_base = RREG32(MC_VM_FB_LOCATION) & 0xFFFF;
+	fb_base <<= 24ULL;
+	r600_vram_gtt_location(rdev, &rdev->mc, fb_base, 1ULL << 32ULL);
 	radeon_update_bandwidth_info(rdev);
 
 	return 0;
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index c053f81..439dd81 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -2229,62 +2229,9 @@ static void si_mc_program(struct radeon_device *rdev)
 	rv515_vga_render_disable(rdev);
 }
 
-/* SI MC address space is 40 bits */
-static void si_vram_location(struct radeon_device *rdev,
-			     struct radeon_mc *mc, u64 base)
-{
-	mc->vram_start = base;
-	if (mc->mc_vram_size > (0xFFFFFFFFFFULL - base + 1)) {
-		dev_warn(rdev->dev, "limiting VRAM to PCI aperture size\n");
-		mc->real_vram_size = mc->aper_size;
-		mc->mc_vram_size = mc->aper_size;
-	}
-	mc->vram_end = mc->vram_start + mc->mc_vram_size - 1;
-	dev_info(rdev->dev, "VRAM: %lluM 0x%016llX - 0x%016llX (%lluM used)\n",
-			mc->mc_vram_size >> 20, mc->vram_start,
-			mc->vram_end, mc->real_vram_size >> 20);
-}
-
-static void si_gtt_location(struct radeon_device *rdev, struct radeon_mc *mc)
-{
-	u64 size_af, size_bf;
-
-	size_af = ((0xFFFFFFFFFFULL - mc->vram_end) + mc->gtt_base_align) & ~mc->gtt_base_align;
-	size_bf = mc->vram_start & ~mc->gtt_base_align;
-	if (size_bf > size_af) {
-		if (mc->gtt_size > size_bf) {
-			dev_warn(rdev->dev, "limiting GTT\n");
-			mc->gtt_size = size_bf;
-		}
-		mc->gtt_start = (mc->vram_start & ~mc->gtt_base_align) - mc->gtt_size;
-	} else {
-		if (mc->gtt_size > size_af) {
-			dev_warn(rdev->dev, "limiting GTT\n");
-			mc->gtt_size = size_af;
-		}
-		mc->gtt_start = (mc->vram_end + 1 + mc->gtt_base_align) & ~mc->gtt_base_align;
-	}
-	mc->gtt_end = mc->gtt_start + mc->gtt_size - 1;
-	dev_info(rdev->dev, "GTT: %lluM 0x%016llX - 0x%016llX\n",
-			mc->gtt_size >> 20, mc->gtt_start, mc->gtt_end);
-}
-
-static void si_vram_gtt_location(struct radeon_device *rdev,
-				 struct radeon_mc *mc)
-{
-	if (mc->mc_vram_size > 0xFFC0000000ULL) {
-		/* leave room for at least 1024M GTT */
-		dev_warn(rdev->dev, "limiting VRAM\n");
-		mc->real_vram_size = 0xFFC0000000ULL;
-		mc->mc_vram_size = 0xFFC0000000ULL;
-	}
-	si_vram_location(rdev, &rdev->mc, 0);
-	rdev->mc.gtt_base_align = 0;
-	si_gtt_location(rdev, mc);
-}
-
 static int si_mc_init(struct radeon_device *rdev)
 {
+	u64 fb_base;
 	u32 tmp;
 	int chansize, numchan;
 
@@ -2337,7 +2284,9 @@ static int si_mc_init(struct radeon_device *rdev)
 	rdev->mc.mc_vram_size = RREG32(CONFIG_MEMSIZE) * 1024 * 1024;
 	rdev->mc.real_vram_size = RREG32(CONFIG_MEMSIZE) * 1024 * 1024;
 	rdev->mc.visible_vram_size = rdev->mc.aper_size;
-	si_vram_gtt_location(rdev, &rdev->mc);
+	fb_base = RREG32(MC_VM_FB_LOCATION) & 0xFFFF;
+	fb_base <<= 24ULL;
+	r600_vram_gtt_location(rdev, &rdev->mc, fb_base, 1ULL << 40ULL);
 	radeon_update_bandwidth_info(rdev);
 
 	return 0;
-- 
1.7.10.4

Re: Fix GPU triggering random system read after VRAM start change

[Alex Deucher]

On Fri, Jul 27, 2012 at 4:32 PM,  <j.glisse@gmail.com> wrote:
> So first patch is a fix in itself, smallest possible and should go to
> stable. Second patch is an improvement as a first step to flicker free
> boot.

First patch looks ok.  In mc_stop we should disable the crtc and then
poll CRTC_CONTROL.CRTC_CURRENT_MASTER_EN_STATE until it goes to 0 to
make sure the crtc has actually stopped reading before we change the
mc config.  Other than that,

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> I have yet extensively tested second patch, especialy not on AGP but
> so far on few GPU/motherboard it looks good. It can probably wait 3.7.
> Will test it more and report.

Looks good as well.  A nice clean up to boot.

>
> I have a third patch that is a step closer to flicker free boot on uefi,
> waiting ack to release a reg.
>
> Cheers,
> Jerome
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Fix GPU triggering random system read after VRAM start change

[Jerome Glisse]

On Tue, Jul 31, 2012 at 10:56 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Fri, Jul 27, 2012 at 4:32 PM,  <j.glisse@gmail.com> wrote:
>> So first patch is a fix in itself, smallest possible and should go to
>> stable. Second patch is an improvement as a first step to flicker free
>> boot.
>
> First patch looks ok.  In mc_stop we should disable the crtc and then
> poll CRTC_CONTROL.CRTC_CURRENT_MASTER_EN_STATE until it goes to 0 to
> make sure the crtc has actually stopped reading before we change the
> mc config.  Other than that,

In all my test is was instantaneous, ie less than 1us to take effect,
so i did not wanted to make the patch bigger than it is. But if you
really think we should do that i can readd that code in patch 1.

Cheers,
Jerome

>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
>>
>> I have yet extensively tested second patch, especialy not on AGP but
>> so far on few GPU/motherboard it looks good. It can probably wait 3.7.
>> Will test it more and report.
>
> Looks good as well.  A nice clean up to boot.
>
>>
>> I have a third patch that is a step closer to flicker free boot on uefi,
>> waiting ack to release a reg.
>>
>> Cheers,
>> Jerome
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Fix GPU triggering random system read after VRAM start change

[Alex Deucher]

On Tue, Jul 31, 2012 at 11:15 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Tue, Jul 31, 2012 at 10:56 AM, Alex Deucher <alexdeucher@gmail.com> wrote:
>> On Fri, Jul 27, 2012 at 4:32 PM,  <j.glisse@gmail.com> wrote:
>>> So first patch is a fix in itself, smallest possible and should go to
>>> stable. Second patch is an improvement as a first step to flicker free
>>> boot.
>>
>> First patch looks ok.  In mc_stop we should disable the crtc and then
>> poll CRTC_CONTROL.CRTC_CURRENT_MASTER_EN_STATE until it goes to 0 to
>> make sure the crtc has actually stopped reading before we change the
>> mc config.  Other than that,
>
> In all my test is was instantaneous, ie less than 1us to take effect,
> so i did not wanted to make the patch bigger than it is. But if you
> really think we should do that i can readd that code in patch 1.

Thinking about it more, It should fine as is since we take the crtc lock.

Alex

Re: [PATCH 1/3] drm/radeon: do not reenable crtc after moving vram start address

[Brad Campbell]

On 28/07/12 04:32, j.glisse@gmail.com wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> It seems we can not update the crtc scanout address. After disabling
> crtc, update to base address do not take effect after crtc being
> reenable leading to at least frame being scanout from the old crtc
> base address. Disabling crtc display request lead to same behavior.
>
> So after changing the vram address if we don't keep crtc disabled
> we will have the GPU trying to read some random system memory address
> with some iommu this will broke the crtc engine and will lead to
> broken display and iommu error message.
>
> So to avoid this, disable crtc. For flicker less boot we will need
> to avoid moving the vram start address.
>
> This patch should also fix :
>
> https://bugs.freedesktop.org/show_bug.cgi?id=42373

G'day Jerome,

I'm running a Mid 2011, iMac with three heads. Card :
01:00.0 VGA compatible controller: ATI Technologies Inc Device 6720

To make this usable (ie to not cook the machine), I must force the card 
into low power mode which I do with this patch :

diff --git a/drivers/gpu/drm/radeon/radeon_pm.c 
b/drivers/gpu/drm/radeon/radeon_pm.c
index 6fabe89..de85eda 100644
--- a/drivers/gpu/drm/radeon/radeon_pm.c
+++ b/drivers/gpu/drm/radeon/radeon_pm.c
@@ -102,7 +102,7 @@ static void radeon_pm_update_profile(struct 
radeon_device *rdev)
  		break;
  	case PM_PROFILE_LOW:
  		if (rdev->pm.active_crtc_count > 1)
-			rdev->pm.profile_index = PM_PROFILE_LOW_MH_IDX;
+			rdev->pm.profile_index = PM_PROFILE_LOW_SH_IDX;
  		else
  			rdev->pm.profile_index = PM_PROFILE_LOW_SH_IDX;
  		break;

The patch this mail refers to causes moving corruption (like noise) on 
about the right hand 1/3rd of the primary monitor.

Reverting either of these patches makes the corruption go away, however 
reverting my patch makes the machine unusable as it simply cooks.

I also have to revert : [PATCH] drm/radeon: fix bo creation retry path 
or the machine simply panics at X login, however I see that has already 
been queued for reversion.

Any advice you could offer to assist me in sorting this would be much 
appreciated.

Regards,
Brad.

Re: [PATCH 1/3] drm/radeon: do not reenable crtc after moving vram start address

[Jerome Glisse]

On Wed, Sep 5, 2012 at 12:39 AM, Brad Campbell <brad@fnarfbargle.com> wrote:
> On 28/07/12 04:32, j.glisse@gmail.com wrote:
>>
>> From: Jerome Glisse <jglisse@redhat.com>
>>
>> It seems we can not update the crtc scanout address. After disabling
>> crtc, update to base address do not take effect after crtc being
>> reenable leading to at least frame being scanout from the old crtc
>> base address. Disabling crtc display request lead to same behavior.
>>
>> So after changing the vram address if we don't keep crtc disabled
>> we will have the GPU trying to read some random system memory address
>> with some iommu this will broke the crtc engine and will lead to
>> broken display and iommu error message.
>>
>> So to avoid this, disable crtc. For flicker less boot we will need
>> to avoid moving the vram start address.
>>
>> This patch should also fix :
>>
>> https://bugs.freedesktop.org/show_bug.cgi?id=42373
>
>
> G'day Jerome,
>
> I'm running a Mid 2011, iMac with three heads. Card :
> 01:00.0 VGA compatible controller: ATI Technologies Inc Device 6720
>
> To make this usable (ie to not cook the machine), I must force the card into
> low power mode which I do with this patch :
>
> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
> b/drivers/gpu/drm/radeon/radeon_pm.c
> index 6fabe89..de85eda 100644
> --- a/drivers/gpu/drm/radeon/radeon_pm.c
> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
> @@ -102,7 +102,7 @@ static void radeon_pm_update_profile(struct
> radeon_device *rdev)
>                 break;
>         case PM_PROFILE_LOW:
>                 if (rdev->pm.active_crtc_count > 1)
> -                       rdev->pm.profile_index = PM_PROFILE_LOW_MH_IDX;
> +                       rdev->pm.profile_index = PM_PROFILE_LOW_SH_IDX;
>                 else
>                         rdev->pm.profile_index = PM_PROFILE_LOW_SH_IDX;
>                 break;
>
> The patch this mail refers to causes moving corruption (like noise) on about
> the right hand 1/3rd of the primary monitor.
>
> Reverting either of these patches makes the corruption go away, however
> reverting my patch makes the machine unusable as it simply cooks.
>
> I also have to revert : [PATCH] drm/radeon: fix bo creation retry path or
> the machine simply panics at X login, however I see that has already been
> queued for reversion.
>
> Any advice you could offer to assist me in sorting this would be much
> appreciated.
>
> Regards,
> Brad.

Low profile is not suited to drive 2 monitors and hence is not supported.

Cheers,
Jerome

Re: [PATCH 1/3] drm/radeon: do not reenable crtc after moving vram start address

[Brad Campbell]

> On Wed, Sep 5, 2012 at 12:39 AM, Brad Campbell <brad@fnarfbargle.com> wrote:
>> On 28/07/12 04:32, j.glisse@gmail.com wrote:
>>> 
>>> From: Jerome Glisse <jglisse@redhat.com>
>>> 
>>> It seems we can not update the crtc scanout address. After disabling
>>> crtc, update to base address do not take effect after crtc being
>>> reenable leading to at least frame being scanout from the old crtc
>>> base address. Disabling crtc display request lead to same behavior.
>>> 
>>> So after changing the vram address if we don't keep crtc disabled
>>> we will have the GPU trying to read some random system memory address
>>> with some iommu this will broke the crtc engine and will lead to
>>> broken display and iommu error message.
>>> 
>>> So to avoid this, disable crtc. For flicker less boot we will need
>>> to avoid moving the vram start address.
>>> 
>>> This patch should also fix :
>>> 
>>> https://bugs.freedesktop.org/show_bug.cgi?id=42373
>> 
>> 
>> G'day Jerome,
>> 
>> I'm running a Mid 2011, iMac with three heads. Card :
>> 01:00.0 VGA compatible controller: ATI Technologies Inc Device 6720
>> 
>> To make this usable (ie to not cook the machine), I must force the card into
>> low power mode which I do with this patch :
>> 
>> diff --git a/drivers/gpu/drm/radeon/radeon_pm.c
>> b/drivers/gpu/drm/radeon/radeon_pm.c
>> index 6fabe89..de85eda 100644
>> --- a/drivers/gpu/drm/radeon/radeon_pm.c
>> +++ b/drivers/gpu/drm/radeon/radeon_pm.c
>> @@ -102,7 +102,7 @@ static void radeon_pm_update_profile(struct
>> radeon_device *rdev)
>>                break;
>>        case PM_PROFILE_LOW:
>>                if (rdev->pm.active_crtc_count > 1)
>> -                       rdev->pm.profile_index = PM_PROFILE_LOW_MH_IDX;
>> +                       rdev->pm.profile_index = PM_PROFILE_LOW_SH_IDX;
>>                else
>>                        rdev->pm.profile_index = PM_PROFILE_LOW_SH_IDX;
>>                break;
>> 
>> The patch this mail refers to causes moving corruption (like noise) on about
>> the right hand 1/3rd of the primary monitor.
>> 
>> Reverting either of these patches makes the corruption go away, however
>> reverting my patch makes the machine unusable as it simply cooks.
>> 
>> I also have to revert : [PATCH] drm/radeon: fix bo creation retry path or
>> the machine simply panics at X login, however I see that has already been
>> queued for reversion.
>> 
>> Any advice you could offer to assist me in sorting this would be much
>> appreciated.
>> 
>> Regards,
>> Brad.
> 
> Low profile is not suited to drive 2 monitors and hence is not supported.
> 
> Cheers,
> Jerome

Thanks for that. I was naive enough to expect that answer, yet ask anyway.

Is there anyone that can point me towards a configuration (three heads) with functioning power management?

Regards,
Brad