All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] drm/ast: Support AST2500
@ 2017-02-16  9:09 Benjamin Herrenschmidt
  2017-02-16 11:40 ` Benjamin Herrenschmidt
  2017-02-16 17:33 ` Emil Velikov
  0 siblings, 2 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-16  9:09 UTC (permalink / raw)
  To: dri-devel; +Cc: airlied, eich

From: "Y.C. Chen" <yc_chen@aspeedtech.com>

Add detection and POST code for AST2500 generation chip,
code originally from Aspeed and slightly reworked for
coding style mostly by Ben.

Signed-off-by: Y.C. Chen <yc_chen@aspeedtech.com>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
v2. [BenH]
  - Coding style fixes
  - Add timeout to main DRAM init loop
  - Improve boot time display of RAM info

Y.C. Please check that I didn't break POST as I haven't manage to test
that (we can't boot our machines if the BMC isn't already POSTed).

---
 drivers/gpu/drm/ast/ast_dram_tables.h |  62 +++++
 drivers/gpu/drm/ast/ast_drv.h         |   3 +
 drivers/gpu/drm/ast/ast_main.c        |  32 ++-
 drivers/gpu/drm/ast/ast_mode.c        |  25 +-
 drivers/gpu/drm/ast/ast_post.c        | 491 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/ast/ast_tables.h      |  57 +++-
 6 files changed, 586 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dram_tables.h b/drivers/gpu/drm/ast/ast_dram_tables.h
index cc04539..02265b5 100644
--- a/drivers/gpu/drm/ast/ast_dram_tables.h
+++ b/drivers/gpu/drm/ast/ast_dram_tables.h
@@ -141,4 +141,66 @@ static const struct ast_dramstruct ast2100_dram_table_data[] = {
 	{ 0xffff, 0xffffffff },
 };
 
+/*
+ * AST2500 DRAM settings modules
+ */
+#define REGTBL_NUM           17
+#define REGIDX_010           0
+#define REGIDX_014           1
+#define REGIDX_018           2
+#define REGIDX_020           3
+#define REGIDX_024           4
+#define REGIDX_02C           5
+#define REGIDX_030           6
+#define REGIDX_214           7
+#define REGIDX_2E0           8
+#define REGIDX_2E4           9
+#define REGIDX_2E8           10
+#define REGIDX_2EC           11
+#define REGIDX_2F0           12
+#define REGIDX_2F4           13
+#define REGIDX_2F8           14
+#define REGIDX_RFC           15
+#define REGIDX_PLL           16
+
+static u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
+	0x64604D38,                  /* 0x010 */
+	0x29690599,                  /* 0x014 */
+	0x00000300,                  /* 0x018 */
+	0x00000000,                  /* 0x020 */
+	0x00000000,                  /* 0x024 */
+	0x02181E70,                  /* 0x02C */
+	0x00000040,                  /* 0x030 */
+	0x00000024,                  /* 0x214 */
+	0x02001300,                  /* 0x2E0 */
+	0x0E0000A0,                  /* 0x2E4 */
+	0x000E001B,                  /* 0x2E8 */
+	0x35B8C105,                  /* 0x2EC */
+	0x08090408,                  /* 0x2F0 */
+	0x9B000800,                  /* 0x2F4 */
+	0x0E400A00,                  /* 0x2F8 */
+	0x9971452F,                  /* tRFC  */
+	0x000071C1		     /* PLL   */
+};
+
+static u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
+	0x63604E37,                  /* 0x010 */
+	0xE97AFA99,                  /* 0x014 */
+	0x00019000,                  /* 0x018 */
+	0x08000000,                  /* 0x020 */
+	0x00000400,                  /* 0x024 */
+	0x00000410,                  /* 0x02C */
+	0x00000101,                  /* 0x030 */
+	0x00000024,                  /* 0x214 */
+	0x03002900,                  /* 0x2E0 */
+	0x0E0000A0,                  /* 0x2E4 */
+	0x000E001C,                  /* 0x2E8 */
+	0x35B8C106,                  /* 0x2EC */
+	0x08080607,                  /* 0x2F0 */
+	0x9B000900,                  /* 0x2F4 */
+	0x0E400A00,                  /* 0x2F8 */
+	0x99714545,                  /* tRFC  */
+	0x000071C1                   /* PLL   */
+};
+
 #endif
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 3bedcf7..2db97e2 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -64,6 +64,7 @@ enum ast_chip {
 	AST2150,
 	AST2300,
 	AST2400,
+	AST2500,
 	AST1180,
 };
 
@@ -80,6 +81,7 @@ enum ast_tx_chip {
 #define AST_DRAM_1Gx32   3
 #define AST_DRAM_2Gx16   6
 #define AST_DRAM_4Gx16   7
+#define AST_DRAM_8Gx16   8
 
 struct ast_fbdev;
 
@@ -397,6 +399,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
 void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);
 void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
+
 /* ast dp501 */
 int ast_load_dp501_microcode(struct drm_device *dev);
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index d42a4f5..60f23cc 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -32,8 +32,6 @@
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 
-#include "ast_dram_tables.h"
-
 void ast_set_index_reg_mask(struct ast_private *ast,
 			    uint32_t base, uint8_t index,
 			    uint8_t mask, uint8_t val)
@@ -106,7 +104,10 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
 		ast->chip = AST1100;
 		DRM_INFO("AST 1180 detected\n");
 	} else {
-		if (dev->pdev->revision >= 0x30) {
+		if (dev->pdev->revision >= 0x40) {
+			ast->chip = AST2500;
+			DRM_INFO("AST 2500 detected\n");
+		} else if (dev->pdev->revision >= 0x30) {
 			ast->chip = AST2400;
 			DRM_INFO("AST 2400 detected\n");
 		} else if (dev->pdev->revision >= 0x20) {
@@ -174,6 +175,9 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
 			if (ast->chip == AST2400 &&
 			    (scu_rev & 0x300) == 0x100) /* ast1400 */
 				ast->support_wide_screen = true;
+			if (ast->chip == AST2500 &&
+			    scu_rev == 0x100)           /* ast2510 */
+				ast->support_wide_screen = true;
 		}
 		break;
 	}
@@ -276,7 +280,23 @@ static int ast_get_dram_info(struct drm_device *dev)
 	else
 		ast->dram_bus_width = 32;
 
-	if (ast->chip == AST2300 || ast->chip == AST2400) {
+	if (ast->chip == AST2500) {
+		switch (mcr_cfg & 0x03) {
+		case 0:
+			ast->dram_type = AST_DRAM_1Gx16;
+			break;
+		default:
+		case 1:
+			ast->dram_type = AST_DRAM_2Gx16;
+			break;
+		case 2:
+			ast->dram_type = AST_DRAM_4Gx16;
+			break;
+		case 3:
+			ast->dram_type = AST_DRAM_8Gx16;
+			break;
+		}
+	} else if (ast->chip == AST2300 || ast->chip == AST2400) {
 		switch (mcr_cfg & 0x03) {
 		case 0:
 			ast->dram_type = AST_DRAM_512Mx16;
@@ -474,7 +494,8 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
 		if (ret)
 			goto out_free;
 		ast->vram_size = ast_get_vram_info(dev);
-		DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast->dram_type, ast->dram_bus_width, ast->vram_size);
+		DRM_INFO("dram MCLK=%u type=%d bus_width=%d size=%08x\n",
+			 ast->mclk, ast->dram_type, ast->dram_bus_width, ast->vram_size);
 	}
 
 	if (need_post)
@@ -497,6 +518,7 @@ int ast_driver_load(struct drm_device *dev, unsigned long flags)
 	    ast->chip == AST2200 ||
 	    ast->chip == AST2300 ||
 	    ast->chip == AST2400 ||
+	    ast->chip == AST2500 ||
 	    ast->chip == AST1180) {
 		dev->mode_config.max_width = 1920;
 		dev->mode_config.max_height = 2048;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index e26c98f..02bebf6 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -271,7 +271,10 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
 {
 	struct ast_private *ast = crtc->dev->dev_private;
 	u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 0;
-	u16 temp;
+	u16 temp, precache = 0;
+
+	if ((ast->chip == AST2500) && (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
+		precache = 40;
 
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x11, 0x7f, 0x00);
 
@@ -297,12 +300,12 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
 		jregAD |= 0x01;  /* HBE D[5] */
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x03, 0xE0, (temp & 0x1f));
 
-	temp = (mode->crtc_hsync_start >> 3) - 1;
+	temp = ((mode->crtc_hsync_start-precache) >> 3) - 1;
 	if (temp & 0x100)
 		jregAC |= 0x40; /* HRS D[5] */
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x04, 0x00, temp);
 
-	temp = ((mode->crtc_hsync_end >> 3) - 1) & 0x3f;
+	temp = (((mode->crtc_hsync_end-precache) >> 3) - 1) & 0x3f;
 	if (temp & 0x20)
 		jregAD |= 0x04; /* HRE D[5] */
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x05, 0x60, (u8)((temp & 0x1f) | jreg05));
@@ -363,6 +366,11 @@ static void ast_set_crtc_reg(struct drm_crtc *crtc, struct drm_display_mode *mod
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x09, 0xdf, jreg09);
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xAE, 0x00, (jregAE | 0x80));
 
+    if (precache)
+		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0x3f, 0x80);
+    else
+		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0x3f, 0x00);
+
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x11, 0x7f, 0x80);
 }
 
@@ -383,12 +391,15 @@ static void ast_set_dclk_reg(struct drm_device *dev, struct drm_display_mode *mo
 	struct ast_private *ast = dev->dev_private;
 	struct ast_vbios_dclk_info *clk_info;
 
-	clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
+	if (ast->chip == AST2500)
+		clk_info = &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];
+	else
+		clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
 
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xc0, 0x00, clk_info->param1);
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xc1, 0x00, clk_info->param2);
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xbb, 0x0f,
-			       (clk_info->param3 & 0x80) | ((clk_info->param3 & 0x3) << 4));
+			       (clk_info->param3 & 0xc0) | ((clk_info->param3 & 0x3) << 4));
 }
 
 static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
@@ -421,7 +432,7 @@ static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa8, 0xfd, jregA8);
 
 	/* Set Threshold */
-	if (ast->chip == AST2300 || ast->chip == AST2400) {
+	if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500) {
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);
 	} else if (ast->chip == AST2100 ||
@@ -794,7 +805,7 @@ static int ast_mode_valid(struct drm_connector *connector,
 		if ((mode->hdisplay == 1600) && (mode->vdisplay == 900))
 			return MODE_OK;
 
-		if ((ast->chip == AST2100) || (ast->chip == AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip == AST1180)) {
+		if ((ast->chip == AST2100) || (ast->chip == AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip == AST2500) || (ast->chip == AST1180)) {
 			if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080))
 				return MODE_OK;
 
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 7197635..0b41fb6 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -31,7 +31,8 @@
 
 #include "ast_dram_tables.h"
 
-static void ast_init_dram_2300(struct drm_device *dev);
+static void ast_post_chip_2300(struct drm_device *dev);
+static void ast_post_chip_2500(struct drm_device *dev);
 
 void ast_enable_vga(struct drm_device *dev)
 {
@@ -82,7 +83,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
 	for (i = 0x81; i <= 0x8f; i++)
 		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
 
-	if (ast->chip == AST2300 || ast->chip == AST2400) {
+	if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500) {
 		if (dev->pdev->revision >= 0x20)
 			ext_reg_info = extreginfo_ast2300;
 		else
@@ -106,7 +107,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
 
 	/* Enable RAMDAC for A1 */
 	reg = 0x04;
-	if (ast->chip == AST2300 || ast->chip == AST2400)
+	if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500)
 		reg |= 0x20;
 	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
 }
@@ -380,8 +381,10 @@ void ast_post_gpu(struct drm_device *dev)
 	ast_set_def_ext_reg(dev);
 
 	if (ast->config_mode == ast_use_p2a) {
+		if (ast->chip == AST2500)
+			ast_post_chip_2500(dev);
 		if (ast->chip == AST2300 || ast->chip == AST2400)
-			ast_init_dram_2300(dev);
+			ast_post_chip_2300(dev);
 		else
 			ast_init_dram_reg(dev);
 
@@ -445,85 +448,53 @@ static const u32 pattern[8] = {
 	0x7C61D253
 };
 
-static int mmc_test_burst(struct ast_private *ast, u32 datagen)
+static int mmc_test(struct ast_private *ast, u32 datagen, u8 test_ctl)
 {
 	u32 data, timeout;
 
 	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, 0x000000c1 | (datagen << 3));
+	ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
 	timeout = 0;
 	do {
 		data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
 		if (data & 0x2000) {
-			return 0;
+			return -1;
 		}
 		if (++timeout > TIMEOUT) {
 			ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-			return 0;
+			return -1;
 		}
 	} while (!data);
+	data = ast_mindwm(ast, 0x1e6e0078);
+	data = (data | (data >> 16)) & 0xffff;
 	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	return 1;
+	return data;
 }
 
-static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
+
+static int mmc_test_burst(struct ast_private *ast, u32 datagen)
 {
-	u32 data, timeout;
+	return mmc_test(ast, datagen, 0xc1);
+}
 
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000041 | (datagen << 3));
-	timeout = 0;
-	do {
-		data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
-		if (++timeout > TIMEOUT) {
-			ast_moutdwm(ast, 0x1e6e0070, 0x0);
-			return -1;
-		}
-	} while (!data);
-	data = ast_mindwm(ast, 0x1e6e0078);
-	data = (data | (data >> 16)) & 0xffff;
-	ast_moutdwm(ast, 0x1e6e0070, 0x0);
-	return data;
+static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
+{
+	return mmc_test(ast, datagen, 0x41);
 }
 
 static int mmc_test_single(struct ast_private *ast, u32 datagen)
 {
-	u32 data, timeout;
-
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, 0x000000c5 | (datagen << 3));
-	timeout = 0;
-	do {
-		data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
-		if (data & 0x2000)
-			return 0;
-		if (++timeout > TIMEOUT) {
-			ast_moutdwm(ast, 0x1e6e0070, 0x0);
-			return 0;
-		}
-	} while (!data);
-	ast_moutdwm(ast, 0x1e6e0070, 0x0);
-	return 1;
+	return mmc_test(ast, datagen, 0xc5);
 }
 
 static int mmc_test_single2(struct ast_private *ast, u32 datagen)
 {
-	u32 data, timeout;
+	return mmc_test(ast, datagen, 0x05);
+}
 
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
-	ast_moutdwm(ast, 0x1e6e0070, 0x00000005 | (datagen << 3));
-	timeout = 0;
-	do {
-		data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
-		if (++timeout > TIMEOUT) {
-			ast_moutdwm(ast, 0x1e6e0070, 0x0);
-			return -1;
-		}
-	} while (!data);
-	data = ast_mindwm(ast, 0x1e6e0078);
-	data = (data | (data >> 16)) & 0xffff;
-	ast_moutdwm(ast, 0x1e6e0070, 0x0);
-	return data;
+static int mmc_test_single_2500(struct ast_private *ast, u32 datagen)
+{
+	return mmc_test(ast, datagen, 0x85);
 }
 
 static int cbr_test(struct ast_private *ast)
@@ -603,9 +574,9 @@ static u32 cbr_scan2(struct ast_private *ast)
 
 static u32 cbr_test3(struct ast_private *ast)
 {
-	if (!mmc_test_burst(ast, 0))
+	if (mmc_test_burst(ast, 0) < 0)
 		return 0;
-	if (!mmc_test_single(ast, 0))
+	if (mmc_test_single(ast, 0) < 0)
 		return 0;
 	return 1;
 }
@@ -1609,7 +1580,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
 
 }
 
-static void ast_init_dram_2300(struct drm_device *dev)
+static void ast_post_chip_2300(struct drm_device *dev)
 {
 	struct ast_private *ast = dev->dev_private;
 	struct ast2300_dram_param param;
@@ -1660,3 +1631,405 @@ static void ast_init_dram_2300(struct drm_device *dev)
 	} while ((reg & 0x40) == 0);
 }
 
+static bool cbr_test_2500(struct ast_private *ast)
+{
+	ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
+	ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
+	if (mmc_test_burst(ast, 0) < 0)
+		return false;
+	if (mmc_test_single_2500(ast, 0) < 0)
+		return false;
+	return true;
+}
+
+static bool ddr_test_2500(struct ast_private *ast)
+{
+	ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
+	ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
+	if (mmc_test_burst(ast, 0) < 0)
+		return false;
+	if (mmc_test_burst(ast, 1) < 0)
+		return false;
+	if (mmc_test_burst(ast, 2) < 0)
+		return false;
+	if (mmc_test_burst(ast, 3) < 0)
+		return false;
+	if (mmc_test_single_2500(ast, 0) < 0)
+		return false;
+	return false;
+}
+
+static void ddr_init_common_2500(struct ast_private *ast)
+{
+	ast_moutdwm(ast, 0x1E6E0034,0x00020080);
+	ast_moutdwm(ast, 0x1E6E0008,0x2003000F);
+	ast_moutdwm(ast, 0x1E6E0038,0x00000FFF);
+	ast_moutdwm(ast, 0x1E6E0040,0x88448844);
+	ast_moutdwm(ast, 0x1E6E0044,0x24422288);
+	ast_moutdwm(ast, 0x1E6E0048,0x22222222);
+	ast_moutdwm(ast, 0x1E6E004C,0x22222222);
+	ast_moutdwm(ast, 0x1E6E0050,0x80000000);
+	ast_moutdwm(ast, 0x1E6E0208,0x00000000);
+	ast_moutdwm(ast, 0x1E6E0218,0x00000000);
+	ast_moutdwm(ast, 0x1E6E0220,0x00000000);
+	ast_moutdwm(ast, 0x1E6E0228,0x00000000);
+	ast_moutdwm(ast, 0x1E6E0230,0x00000000);
+	ast_moutdwm(ast, 0x1E6E02A8,0x00000000);
+	ast_moutdwm(ast, 0x1E6E02B0,0x00000000);
+	ast_moutdwm(ast, 0x1E6E0240,0x86000000);
+	ast_moutdwm(ast, 0x1E6E0244,0x00008600);
+	ast_moutdwm(ast, 0x1E6E0248,0x80000000);
+	ast_moutdwm(ast, 0x1E6E024C,0x80808080);
+}
+
+static void ddr_phy_init_2500(struct ast_private *ast)
+{
+	u32 data, pass, timecnt;
+
+	pass = 0;
+	ast_moutdwm(ast, 0x1E6E0060,0x00000005);
+	while (!pass) {
+		for(timecnt = 0;timecnt < TIMEOUT;timecnt++) {
+			data = ast_mindwm(ast, 0x1E6E0060) & 0x1;
+			if (!data)
+				break;
+		}
+		if (timecnt != TIMEOUT) {
+			data = ast_mindwm(ast, 0x1E6E0300) & 0x000A0000;
+			if (!data)
+				pass = 1;
+		}
+		if (!pass) {
+			ast_moutdwm(ast, 0x1E6E0060,0x00000000);
+			udelay(10); /* delay 10 us */
+			ast_moutdwm(ast, 0x1E6E0060,0x00000005);
+		}
+	}
+
+	ast_moutdwm(ast, 0x1E6E0060,0x00000006);
+}
+
+/******************************************************************************
+ Check DRAM Size
+ 1Gb : 0x80000000 ~ 0x87FFFFFF
+ 2Gb : 0x80000000 ~ 0x8FFFFFFF
+ 4Gb : 0x80000000 ~ 0x9FFFFFFF
+ 8Gb : 0x80000000 ~ 0xBFFFFFFF
+*****************************************************************************/
+static void check_dram_size_2500(struct ast_private *ast, u32 tRFC)
+{
+	u32 reg_04, reg_14;
+
+	reg_04 = ast_mindwm(ast, 0x1E6E0004) & 0xfffffffc;
+	reg_14 = ast_mindwm(ast, 0x1E6E0014) & 0xffffff00;
+
+	ast_moutdwm(ast, 0xA0100000, 0x41424344);
+	ast_moutdwm(ast, 0x90100000, 0x35363738);
+	ast_moutdwm(ast, 0x88100000, 0x292A2B2C);
+	ast_moutdwm(ast, 0x80100000, 0x1D1E1F10);
+
+	/* Check 8Gbit */
+	if (ast_mindwm(ast, 0xA0100000) == 0x41424344) {
+		reg_04 |= 0x03;
+		reg_14 |= (tRFC >> 24) & 0xFF;
+		/* Check 4Gbit */
+	} else if (ast_mindwm(ast, 0x90100000) == 0x35363738) {
+		reg_04 |= 0x02;
+		reg_14 |= (tRFC >> 16) & 0xFF;
+		/* Check 2Gbit */
+	} else if (ast_mindwm(ast, 0x88100000) == 0x292A2B2C) {
+		reg_04 |= 0x01;
+		reg_14 |= (tRFC >> 8) & 0xFF;
+	} else {
+		reg_14 |= tRFC & 0xFF;
+	}
+	ast_moutdwm(ast, 0x1E6E0004, reg_04);
+	ast_moutdwm(ast, 0x1E6E0014, reg_14);
+}
+
+static void enable_cache_2500(struct ast_private *ast)
+{
+	u32 reg_04, data;
+
+	reg_04 = ast_mindwm(ast, 0x1E6E0004);
+	ast_moutdwm(ast, 0x1E6E0004, reg_04 | 0x1000);
+
+	do
+		data = ast_mindwm(ast, 0x1E6E0004);
+	while (!(data & 0x80000));
+	ast_moutdwm(ast, 0x1E6E0004, reg_04 | 0x400);
+}
+
+static void set_mpll_2500(struct ast_private *ast)
+{
+	u32 addr, data, param;
+
+	/* Reset MMC */
+	ast_moutdwm(ast, 0x1E6E0000,0xFC600309);
+	ast_moutdwm(ast, 0x1E6E0034,0x00020080);
+	for (addr = 0x1e6e0004; addr < 0x1e6e0090;) {
+		ast_moutdwm(ast, addr, 0x0);
+		addr += 4;
+	}
+	ast_moutdwm(ast, 0x1E6E0034,0x00020000);
+
+	ast_moutdwm(ast, 0x1E6E2000, 0x1688A8A8);
+	data = ast_mindwm(ast, 0x1E6E2070) & 0x00800000;
+	if (data) {
+		/* CLKIN = 25MHz */
+		param = 0x930023E0;
+		ast_moutdwm(ast, 0x1E6E2160, 0x00011320);
+	} else {
+		/* CLKIN = 24MHz */
+		param = 0x93002400;
+	}
+	ast_moutdwm(ast, 0x1E6E2020, param);
+	udelay(100);
+}
+
+static void reset_mmc_2500(struct ast_private *ast)
+{
+	ast_moutdwm(ast, 0x1E78505C,0x00000004);
+	ast_moutdwm(ast, 0x1E785044,0x00000001);
+	ast_moutdwm(ast, 0x1E785048,0x00004755);
+	ast_moutdwm(ast, 0x1E78504C,0x00000013);
+	mdelay(100);					/* delay 100ms */
+	ast_moutdwm(ast, 0x1E785054,0x00000077);
+	ast_moutdwm(ast, 0x1E6E0000,0xFC600309);
+}
+
+static void ddr3_init_2500(struct ast_private *ast, u32 *ddr_table)
+{
+
+	ast_moutdwm(ast, 0x1E6E0004,0x00000303);
+	ast_moutdwm(ast, 0x1E6E0010,ddr_table[REGIDX_010]);
+	ast_moutdwm(ast, 0x1E6E0014,ddr_table[REGIDX_014]);
+	ast_moutdwm(ast, 0x1E6E0018,ddr_table[REGIDX_018]);
+	ast_moutdwm(ast, 0x1E6E0020,ddr_table[REGIDX_020]);          /* MODEREG4/6 */
+	ast_moutdwm(ast, 0x1E6E0024,ddr_table[REGIDX_024]);          /* MODEREG5 */
+	ast_moutdwm(ast, 0x1E6E002C,ddr_table[REGIDX_02C] | 0x100);  /* MODEREG0/2 */
+	ast_moutdwm(ast, 0x1E6E0030,ddr_table[REGIDX_030]);          /* MODEREG1/3 */
+
+	/* DDR PHY Setting */
+	ast_moutdwm(ast, 0x1E6E0200,0x02492AAE);
+	ast_moutdwm(ast, 0x1E6E0204,0x00001001);
+	ast_moutdwm(ast, 0x1E6E020C,0x55E00B0B);
+	ast_moutdwm(ast, 0x1E6E0210,0x20000000);
+	ast_moutdwm(ast, 0x1E6E0214,ddr_table[REGIDX_214]);
+	ast_moutdwm(ast, 0x1E6E02E0,ddr_table[REGIDX_2E0]);
+	ast_moutdwm(ast, 0x1E6E02E4,ddr_table[REGIDX_2E4]);
+	ast_moutdwm(ast, 0x1E6E02E8,ddr_table[REGIDX_2E8]);
+	ast_moutdwm(ast, 0x1E6E02EC,ddr_table[REGIDX_2EC]);
+	ast_moutdwm(ast, 0x1E6E02F0,ddr_table[REGIDX_2F0]);
+	ast_moutdwm(ast, 0x1E6E02F4,ddr_table[REGIDX_2F4]);
+	ast_moutdwm(ast, 0x1E6E02F8,ddr_table[REGIDX_2F8]);
+	ast_moutdwm(ast, 0x1E6E0290,0x00100008);
+	ast_moutdwm(ast, 0x1E6E02C0,0x00000006);
+
+	/* Controller Setting */
+	ast_moutdwm(ast, 0x1E6E0034,0x00020091);
+
+	/* Wait DDR PHY init done */
+	ddr_phy_init_2500(ast);
+
+	ast_moutdwm(ast, 0x1E6E0120,ddr_table[REGIDX_PLL]);
+	ast_moutdwm(ast, 0x1E6E000C,0x42AA5C81);
+	ast_moutdwm(ast, 0x1E6E0034,0x0001AF93);
+
+	check_dram_size_2500(ast, ddr_table[REGIDX_RFC]);
+	enable_cache_2500(ast);
+	ast_moutdwm(ast, 0x1E6E001C,0x00000008);
+	ast_moutdwm(ast, 0x1E6E0038,0xFFFFFF00);
+}
+
+static void ddr4_init_2500(struct ast_private *ast, u32 *ddr_table)
+{
+	u32 data, data2, pass, retrycnt;
+	u32 ddr_vref, phy_vref;
+	u32 min_ddr_vref=0, min_phy_vref=0;
+	u32 max_ddr_vref=0, max_phy_vref=0;
+
+	ast_moutdwm(ast, 0x1E6E0004,0x00000313);
+	ast_moutdwm(ast, 0x1E6E0010,ddr_table[REGIDX_010]);
+	ast_moutdwm(ast, 0x1E6E0014,ddr_table[REGIDX_014]);
+	ast_moutdwm(ast, 0x1E6E0018,ddr_table[REGIDX_018]);
+	ast_moutdwm(ast, 0x1E6E0020,ddr_table[REGIDX_020]);          /* MODEREG4/6 */
+	ast_moutdwm(ast, 0x1E6E0024,ddr_table[REGIDX_024]);          /* MODEREG5 */
+	ast_moutdwm(ast, 0x1E6E002C,ddr_table[REGIDX_02C] | 0x100);  /* MODEREG0/2 */
+	ast_moutdwm(ast, 0x1E6E0030,ddr_table[REGIDX_030]);          /* MODEREG1/3 */
+
+	/* DDR PHY Setting */
+	ast_moutdwm(ast, 0x1E6E0200,0x42492AAE);
+	ast_moutdwm(ast, 0x1E6E0204,0x09002000);
+	ast_moutdwm(ast, 0x1E6E020C,0x55E00B0B);
+	ast_moutdwm(ast, 0x1E6E0210,0x20000000);
+	ast_moutdwm(ast, 0x1E6E0214,ddr_table[REGIDX_214]);
+	ast_moutdwm(ast, 0x1E6E02E0,ddr_table[REGIDX_2E0]);
+	ast_moutdwm(ast, 0x1E6E02E4,ddr_table[REGIDX_2E4]);
+	ast_moutdwm(ast, 0x1E6E02E8,ddr_table[REGIDX_2E8]);
+	ast_moutdwm(ast, 0x1E6E02EC,ddr_table[REGIDX_2EC]);
+	ast_moutdwm(ast, 0x1E6E02F0,ddr_table[REGIDX_2F0]);
+	ast_moutdwm(ast, 0x1E6E02F4,ddr_table[REGIDX_2F4]);
+	ast_moutdwm(ast, 0x1E6E02F8,ddr_table[REGIDX_2F8]);
+	ast_moutdwm(ast, 0x1E6E0290,0x00100008);
+	ast_moutdwm(ast, 0x1E6E02C4,0x3C183C3C);
+	ast_moutdwm(ast, 0x1E6E02C8,0x00631E0E);
+
+	/* Controller Setting */
+	ast_moutdwm(ast, 0x1E6E0034,0x0001A991);
+
+	/* Train PHY Vref first */
+	pass = 0;
+
+	for (retrycnt = 0;retrycnt < 4 && pass == 0;retrycnt++) {
+		max_phy_vref = 0x0;
+		pass = 0;
+		ast_moutdwm(ast, 0x1E6E02C0,0x00001C06);
+		for (phy_vref = 0x40;phy_vref < 0x80;phy_vref++) {
+			ast_moutdwm(ast, 0x1E6E000C,0x00000000);
+			ast_moutdwm(ast, 0x1E6E0060,0x00000000);
+			ast_moutdwm(ast, 0x1E6E02CC,phy_vref | (phy_vref << 8));
+			/* Fire DFI Init */
+			ddr_phy_init_2500(ast);
+			ast_moutdwm(ast, 0x1E6E000C,0x00005C01);
+			if (cbr_test_2500(ast)) {
+				pass++;
+				data = ast_mindwm(ast, 0x1E6E03D0);
+				data2 = data >> 8;
+				data  = data & 0xff;
+				if (data > data2)
+					data = data2;
+				if (max_phy_vref < data) {
+					max_phy_vref = data;
+					min_phy_vref = phy_vref;
+				}
+			} else if (pass > 0)
+				break;
+		}
+	}
+	ast_moutdwm(ast, 0x1E6E02CC,min_phy_vref | (min_phy_vref << 8));
+
+	/* Train DDR Vref next */
+	pass = 0;
+
+	for (retrycnt = 0;retrycnt < 4 && pass == 0;retrycnt++) {
+		min_ddr_vref = 0xFF;
+		max_ddr_vref = 0x0;
+		pass = 0;
+		for (ddr_vref = 0x00;ddr_vref < 0x40;ddr_vref++) {
+			ast_moutdwm(ast, 0x1E6E000C,0x00000000);
+			ast_moutdwm(ast, 0x1E6E0060,0x00000000);
+			ast_moutdwm(ast, 0x1E6E02C0,0x00000006 | (ddr_vref << 8));
+			/* Fire DFI Init */
+			ddr_phy_init_2500(ast);
+			ast_moutdwm(ast, 0x1E6E000C,0x00005C01);
+			if (cbr_test_2500(ast)) {
+				pass++;
+				if (min_ddr_vref > ddr_vref)
+					min_ddr_vref = ddr_vref;
+				if (max_ddr_vref < ddr_vref)
+					max_ddr_vref = ddr_vref;
+			} else if (pass != 0)
+				break;
+		}
+	}
+
+	ast_moutdwm(ast, 0x1E6E000C,0x00000000);
+	ast_moutdwm(ast, 0x1E6E0060,0x00000000);
+	ddr_vref = (min_ddr_vref + max_ddr_vref + 1) >> 1;
+	ast_moutdwm(ast, 0x1E6E02C0,0x00000006 | (ddr_vref << 8));
+
+	/* Wait DDR PHY init done */
+	ddr_phy_init_2500(ast);
+
+	ast_moutdwm(ast, 0x1E6E0120,ddr_table[REGIDX_PLL]);
+	ast_moutdwm(ast, 0x1E6E000C,0x42AA5C81);
+	ast_moutdwm(ast, 0x1E6E0034,0x0001AF93);
+
+	check_dram_size_2500(ast, ddr_table[REGIDX_RFC]);
+	enable_cache_2500(ast);
+	ast_moutdwm(ast, 0x1E6E001C,0x00000008);
+	ast_moutdwm(ast, 0x1E6E0038,0xFFFFFF00);
+}
+
+static bool ast_dram_init_2500(struct ast_private *ast)
+{
+	u32 data;
+	u32 max_tries = 5;
+
+	do {
+		if (max_tries-- == 0)
+			return false;
+		set_mpll_2500(ast);
+		reset_mmc_2500(ast);
+		ddr_init_common_2500(ast);
+
+		data = ast_mindwm(ast, 0x1E6E2070);
+		if (data & 0x01000000)
+			ddr4_init_2500(ast, ast2500_ddr4_1600_timing_table);
+		else
+			ddr3_init_2500(ast, ast2500_ddr3_1600_timing_table);
+	} while (!ddr_test_2500(ast));
+
+	ast_moutdwm(ast, 0x1E6E2040, ast_mindwm(ast, 0x1E6E2040) | 0x41);
+
+	/* Patch code */
+	data = ast_mindwm(ast, 0x1E6E200C) & 0xF9FFFFFF;
+	ast_moutdwm(ast, 0x1E6E200C, data | 0x10000000);
+
+	return true;
+}
+
+void ast_post_chip_2500(struct drm_device *dev)
+{
+	struct ast_private *ast = dev->dev_private;
+	u32 temp;
+	u8 reg;
+
+	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
+	if ((reg & 0x80) == 0) {/* vga only */
+  		/* Clear bus lock condition */
+  		ast_moutdwm(ast, 0x1e600000,0xAEED1A03);
+		ast_moutdwm(ast, 0x1e600084,0x00010000);
+		ast_moutdwm(ast, 0x1e600088,0x00000000);
+		ast_moutdwm(ast, 0x1e6e2000,0x1688A8A8);
+		ast_write32(ast, 0xf004, 0x1e6e0000);
+		ast_write32(ast, 0xf000, 0x1);
+		ast_write32(ast, 0x12000, 0x1688a8a8);
+		while (ast_read32(ast, 0x12000) != 0x1)
+			;
+
+		ast_write32(ast, 0x10000, 0xfc600309);
+		while (ast_read32(ast, 0x10000) != 0x1)
+			;
+
+		/* Slow down CPU/AHB CLK in VGA only mode */
+		temp = ast_read32(ast, 0x12008);
+		temp |= 0x73;
+		ast_write32(ast, 0x12008, temp);
+
+		/* Reset USB port to patch USB unknown device issue */
+ 	 	ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
+		temp  = ast_mindwm(ast, 0x1e6e2094);
+		temp |= 0x00004000;
+		ast_moutdwm(ast, 0x1e6e2094, temp);
+		temp  = ast_mindwm(ast, 0x1e6e2070);
+		if (temp & 0x00800000) {
+			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
+			mdelay(100);
+			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
+		}
+
+		if (!ast_dram_init_2500(ast)) {
+			DRM_ERROR("DRAM init failed !\n");
+		}
+
+		temp = ast_mindwm(ast, 0x1e6e2040);
+		ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
+	}
+
+	/* wait ready */
+	do {
+		reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
+	} while ((reg & 0x40) == 0);
+}
diff --git a/drivers/gpu/drm/ast/ast_tables.h b/drivers/gpu/drm/ast/ast_tables.h
index 3608d5a..24da4c2 100644
--- a/drivers/gpu/drm/ast/ast_tables.h
+++ b/drivers/gpu/drm/ast/ast_tables.h
@@ -47,6 +47,7 @@
 #define SyncPN			(PVSync | NHSync)
 #define SyncNP			(NVSync | PHSync)
 #define SyncNN			(NVSync | NHSync)
+#define AST2500PreCatchCRT		0x00004000
 
 /* DCLK Index */
 #define VCLK25_175     		0x00
@@ -108,6 +109,36 @@ static struct ast_vbios_dclk_info dclk_table[] = {
 	{0x3b, 0x2c, 0x81},					/* 1A: VCLK118_25	*/
 };
 
+static struct ast_vbios_dclk_info dclk_table_ast2500[] = {
+	{0x2C, 0xE7, 0x03},					/* 00: VCLK25_175	*/
+	{0x95, 0x62, 0x03},				        /* 01: VCLK28_322	*/
+	{0x67, 0x63, 0x01},				        /* 02: VCLK31_5         */
+	{0x76, 0x63, 0x01},				        /* 03: VCLK36         	*/
+	{0xEE, 0x67, 0x01},				        /* 04: VCLK40          	*/
+	{0x82, 0x62, 0x01}, 			        /* 05: VCLK49_5        	*/
+	{0xC6, 0x64, 0x01},                        	        /* 06: VCLK50          	*/
+	{0x94, 0x62, 0x01},                        	        /* 07: VCLK56_25       	*/
+	{0x80, 0x64, 0x00},                        	        /* 08: VCLK65		*/
+	{0x7B, 0x63, 0x00},                        	        /* 09: VCLK75	        */
+	{0x67, 0x62, 0x00},				        /* 0A: VCLK78_75       	*/
+	{0x7C, 0x62, 0x00},                        	        /* 0B: VCLK94_5        	*/
+	{0x8E, 0x62, 0x00},                        	        /* 0C: VCLK108         	*/
+	{0x85, 0x24, 0x00},                        	        /* 0D: VCLK135         	*/
+	{0x67, 0x22, 0x00},                        	        /* 0E: VCLK157_5       	*/
+	{0x6A, 0x22, 0x00},				        /* 0F: VCLK162         	*/
+	{0x4d, 0x4c, 0x80},				        /* 10: VCLK154      	*/
+	{0xa7, 0x78, 0x80},					/* 11: VCLK83.5         */
+	{0x28, 0x49, 0x80},					/* 12: VCLK106.5        */
+	{0x37, 0x49, 0x80},					/* 13: VCLK146.25       */
+	{0x1f, 0x45, 0x80},					/* 14: VCLK148.5        */
+	{0x47, 0x6c, 0x80},					/* 15: VCLK71       */
+	{0x25, 0x65, 0x80},					/* 16: VCLK88.75    */
+	{0x58, 0x01, 0x42},					/* 17: VCLK119      */
+	{0x32, 0x67, 0x80},				    /* 18: VCLK85_5     */
+	{0x6a, 0x6d, 0x80},					/* 19: VCLK97_75	*/
+	{0x44, 0x20, 0x43},					/* 1A: VCLK118_25	*/
+};
+
 static struct ast_vbios_stdtable vbios_stdtable[] = {
 	/* MD_2_3_400 */
 	{
@@ -244,32 +275,32 @@ static struct ast_vbios_enhtable res_1600x1200[] = {
 /* 16:9 */
 static struct ast_vbios_enhtable res_1360x768[] = {
 	{1792, 1360, 64,112, 795,  768, 3, 6, VCLK85_5,	         /* 60Hz */
-	 (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x39 },
+	 (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x39 },
 	{1792, 1360, 64,112, 795,  768, 3, 6, VCLK85_5,	         /* end */
 	 (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x39 },
 };
 
 static struct ast_vbios_enhtable res_1600x900[] = {
 	{1760, 1600, 48, 32, 926,  900, 3, 5, VCLK97_75,	/* 60Hz CVT RB */
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x3A },
-	{2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/* 60Hz CVT */
-	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 0x3A },
-	{2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/* 60Hz CVT */
-	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 2, 0x3A },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x3A },
+    {2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/* 60Hz CVT */
+     (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 0x3A },
+    {2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/* 60Hz CVT */
+     (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 2, 0x3A },
 };
 
 static struct ast_vbios_enhtable res_1920x1080[] = {
 	{2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,	/* 60Hz */
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x38 },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x38 },
 	{2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,	/* 60Hz */
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x38 },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 0xFF, 1, 0x38 },
 };
 
 
 /* 16:10 */
 static struct ast_vbios_enhtable res_1280x800[] = {
 	{1440, 1280, 48, 32,  823,  800, 3, 6, VCLK71,	/* 60Hz RB */
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x35 },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x35 },
 	{1680, 1280, 72,128,  831,  800, 3, 6, VCLK83_5,	/* 60Hz */
 	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 0x35 },
 	{1680, 1280, 72,128,  831,  800, 3, 6, VCLK83_5,	/* 60Hz */
@@ -279,7 +310,7 @@ static struct ast_vbios_enhtable res_1280x800[] = {
 
 static struct ast_vbios_enhtable res_1440x900[] = {
 	{1600, 1440, 48, 32,  926,  900, 3, 6, VCLK88_75,	/* 60Hz RB */
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x36 },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x36 },
 	{1904, 1440, 80,152,  934,  900, 3, 6, VCLK106_5,	/* 60Hz */
 	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 0x36 },
 	{1904, 1440, 80,152,  934,  900, 3, 6, VCLK106_5,	/* 60Hz */
@@ -288,7 +319,7 @@ static struct ast_vbios_enhtable res_1440x900[] = {
 
 static struct ast_vbios_enhtable res_1680x1050[] = {
 	{1840, 1680, 48, 32, 1080, 1050, 3, 6, VCLK119,	/* 60Hz RB */
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x37 },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x37 },
 	{2240, 1680,104,176, 1089, 1050, 3, 6, VCLK146_25,	/* 60Hz */
 	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 2, 0x37 },
 	{2240, 1680,104,176, 1089, 1050, 3, 6, VCLK146_25,	/* 60Hz */
@@ -297,9 +328,9 @@ static struct ast_vbios_enhtable res_1680x1050[] = {
 
 static struct ast_vbios_enhtable res_1920x1200[] = {
 	{2080, 1920, 48, 32, 1235, 1200, 3, 6, VCLK154,	/* 60Hz RB*/
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 60, 1, 0x34 },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x34 },
 	{2080, 1920, 48, 32, 1235, 1200, 3, 6, VCLK154,	/* 60Hz RB */
-	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo), 0xFF, 1, 0x34 },
+	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode | NewModeInfo | AST2500PreCatchCRT), 0xFF, 1, 0x34 },
 };
 
 #endif

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16  9:09 [PATCH 2/2] drm/ast: Support AST2500 Benjamin Herrenschmidt
@ 2017-02-16 11:40 ` Benjamin Herrenschmidt
  2017-02-16 17:33 ` Emil Velikov
  1 sibling, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-16 11:40 UTC (permalink / raw)
  To: dri-devel; +Cc: airlied, eich

On Thu, 2017-02-16 at 20:09 +1100, Benjamin Herrenschmidt wrote:
> From: "Y.C. Chen" <yc_chen@aspeedtech.com>
> 
> Add detection and POST code for AST2500 generation chip,
> code originally from Aspeed and slightly reworked for
> coding style mostly by Ben.

I just noticed there's still a bunch of minor checkpatch warnings,
I'll do a new spin tomorrow but without code changes.

Y.C. Can you still test this one please to make sure I didn't break
POST ? :-)

Cheers,
Ben.

> Signed-off-by: Y.C. Chen <yc_chen@aspeedtech.com>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> v2. [BenH]
>   - Coding style fixes
>   - Add timeout to main DRAM init loop
>   - Improve boot time display of RAM info
> 
> Y.C. Please check that I didn't break POST as I haven't manage to
> test
> that (we can't boot our machines if the BMC isn't already POSTed).
> 
> ---
>  drivers/gpu/drm/ast/ast_dram_tables.h |  62 +++++
>  drivers/gpu/drm/ast/ast_drv.h         |   3 +
>  drivers/gpu/drm/ast/ast_main.c        |  32 ++-
>  drivers/gpu/drm/ast/ast_mode.c        |  25 +-
>  drivers/gpu/drm/ast/ast_post.c        | 491
> ++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/ast/ast_tables.h      |  57 +++-
>  6 files changed, 586 insertions(+), 84 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dram_tables.h
> b/drivers/gpu/drm/ast/ast_dram_tables.h
> index cc04539..02265b5 100644
> --- a/drivers/gpu/drm/ast/ast_dram_tables.h
> +++ b/drivers/gpu/drm/ast/ast_dram_tables.h
> @@ -141,4 +141,66 @@ static const struct ast_dramstruct
> ast2100_dram_table_data[] = {
>  	{ 0xffff, 0xffffffff },
>  };
>  
> +/*
> + * AST2500 DRAM settings modules
> + */
> +#define REGTBL_NUM           17
> +#define REGIDX_010           0
> +#define REGIDX_014           1
> +#define REGIDX_018           2
> +#define REGIDX_020           3
> +#define REGIDX_024           4
> +#define REGIDX_02C           5
> +#define REGIDX_030           6
> +#define REGIDX_214           7
> +#define REGIDX_2E0           8
> +#define REGIDX_2E4           9
> +#define REGIDX_2E8           10
> +#define REGIDX_2EC           11
> +#define REGIDX_2F0           12
> +#define REGIDX_2F4           13
> +#define REGIDX_2F8           14
> +#define REGIDX_RFC           15
> +#define REGIDX_PLL           16
> +
> +static u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
> +	0x64604D38,                  /* 0x010 */
> +	0x29690599,                  /* 0x014 */
> +	0x00000300,                  /* 0x018 */
> +	0x00000000,                  /* 0x020 */
> +	0x00000000,                  /* 0x024 */
> +	0x02181E70,                  /* 0x02C */
> +	0x00000040,                  /* 0x030 */
> +	0x00000024,                  /* 0x214 */
> +	0x02001300,                  /* 0x2E0 */
> +	0x0E0000A0,                  /* 0x2E4 */
> +	0x000E001B,                  /* 0x2E8 */
> +	0x35B8C105,                  /* 0x2EC */
> +	0x08090408,                  /* 0x2F0 */
> +	0x9B000800,                  /* 0x2F4 */
> +	0x0E400A00,                  /* 0x2F8 */
> +	0x9971452F,                  /* tRFC  */
> +	0x000071C1		     /* PLL   */
> +};
> +
> +static u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
> +	0x63604E37,                  /* 0x010 */
> +	0xE97AFA99,                  /* 0x014 */
> +	0x00019000,                  /* 0x018 */
> +	0x08000000,                  /* 0x020 */
> +	0x00000400,                  /* 0x024 */
> +	0x00000410,                  /* 0x02C */
> +	0x00000101,                  /* 0x030 */
> +	0x00000024,                  /* 0x214 */
> +	0x03002900,                  /* 0x2E0 */
> +	0x0E0000A0,                  /* 0x2E4 */
> +	0x000E001C,                  /* 0x2E8 */
> +	0x35B8C106,                  /* 0x2EC */
> +	0x08080607,                  /* 0x2F0 */
> +	0x9B000900,                  /* 0x2F4 */
> +	0x0E400A00,                  /* 0x2F8 */
> +	0x99714545,                  /* tRFC  */
> +	0x000071C1                   /* PLL   */
> +};
> +
>  #endif
> diff --git a/drivers/gpu/drm/ast/ast_drv.h
> b/drivers/gpu/drm/ast/ast_drv.h
> index 3bedcf7..2db97e2 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -64,6 +64,7 @@ enum ast_chip {
>  	AST2150,
>  	AST2300,
>  	AST2400,
> +	AST2500,
>  	AST1180,
>  };
>  
> @@ -80,6 +81,7 @@ enum ast_tx_chip {
>  #define AST_DRAM_1Gx32   3
>  #define AST_DRAM_2Gx16   6
>  #define AST_DRAM_4Gx16   7
> +#define AST_DRAM_8Gx16   8
>  
>  struct ast_fbdev;
>  
> @@ -397,6 +399,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>  void ast_post_gpu(struct drm_device *dev);
>  u32 ast_mindwm(struct ast_private *ast, u32 r);
>  void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +
>  /* ast dp501 */
>  int ast_load_dp501_microcode(struct drm_device *dev);
>  void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
> diff --git a/drivers/gpu/drm/ast/ast_main.c
> b/drivers/gpu/drm/ast/ast_main.c
> index d42a4f5..60f23cc 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -32,8 +32,6 @@
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  
> -#include "ast_dram_tables.h"
> -
>  void ast_set_index_reg_mask(struct ast_private *ast,
>  			    uint32_t base, uint8_t index,
>  			    uint8_t mask, uint8_t val)
> @@ -106,7 +104,10 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>  		ast->chip = AST1100;
>  		DRM_INFO("AST 1180 detected\n");
>  	} else {
> -		if (dev->pdev->revision >= 0x30) {
> +		if (dev->pdev->revision >= 0x40) {
> +			ast->chip = AST2500;
> +			DRM_INFO("AST 2500 detected\n");
> +		} else if (dev->pdev->revision >= 0x30) {
>  			ast->chip = AST2400;
>  			DRM_INFO("AST 2400 detected\n");
>  		} else if (dev->pdev->revision >= 0x20) {
> @@ -174,6 +175,9 @@ static int ast_detect_chip(struct drm_device
> *dev, bool *need_post)
>  			if (ast->chip == AST2400 &&
>  			    (scu_rev & 0x300) == 0x100) /* ast1400
> */
>  				ast->support_wide_screen = true;
> +			if (ast->chip == AST2500 &&
> +			    scu_rev == 0x100)           /* ast2510
> */
> +				ast->support_wide_screen = true;
>  		}
>  		break;
>  	}
> @@ -276,7 +280,23 @@ static int ast_get_dram_info(struct drm_device
> *dev)
>  	else
>  		ast->dram_bus_width = 32;
>  
> -	if (ast->chip == AST2300 || ast->chip == AST2400) {
> +	if (ast->chip == AST2500) {
> +		switch (mcr_cfg & 0x03) {
> +		case 0:
> +			ast->dram_type = AST_DRAM_1Gx16;
> +			break;
> +		default:
> +		case 1:
> +			ast->dram_type = AST_DRAM_2Gx16;
> +			break;
> +		case 2:
> +			ast->dram_type = AST_DRAM_4Gx16;
> +			break;
> +		case 3:
> +			ast->dram_type = AST_DRAM_8Gx16;
> +			break;
> +		}
> +	} else if (ast->chip == AST2300 || ast->chip == AST2400) {
>  		switch (mcr_cfg & 0x03) {
>  		case 0:
>  			ast->dram_type = AST_DRAM_512Mx16;
> @@ -474,7 +494,8 @@ int ast_driver_load(struct drm_device *dev,
> unsigned long flags)
>  		if (ret)
>  			goto out_free;
>  		ast->vram_size = ast_get_vram_info(dev);
> -		DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast-
> >dram_type, ast->dram_bus_width, ast->vram_size);
> +		DRM_INFO("dram MCLK=%u type=%d bus_width=%d
> size=%08x\n",
> +			 ast->mclk, ast->dram_type, ast-
> >dram_bus_width, ast->vram_size);
>  	}
>  
>  	if (need_post)
> @@ -497,6 +518,7 @@ int ast_driver_load(struct drm_device *dev,
> unsigned long flags)
>  	    ast->chip == AST2200 ||
>  	    ast->chip == AST2300 ||
>  	    ast->chip == AST2400 ||
> +	    ast->chip == AST2500 ||
>  	    ast->chip == AST1180) {
>  		dev->mode_config.max_width = 1920;
>  		dev->mode_config.max_height = 2048;
> diff --git a/drivers/gpu/drm/ast/ast_mode.c
> b/drivers/gpu/drm/ast/ast_mode.c
> index e26c98f..02bebf6 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -271,7 +271,10 @@ static void ast_set_crtc_reg(struct drm_crtc
> *crtc, struct drm_display_mode *mod
>  {
>  	struct ast_private *ast = crtc->dev->dev_private;
>  	u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD =
> 0, jregAE = 0;
> -	u16 temp;
> +	u16 temp, precache = 0;
> +
> +	if ((ast->chip == AST2500) && (vbios_mode->enh_table->flags
> & AST2500PreCatchCRT))
> +		precache = 40;
>  
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x11, 0x7f,
> 0x00);
>  
> @@ -297,12 +300,12 @@ static void ast_set_crtc_reg(struct drm_crtc
> *crtc, struct drm_display_mode *mod
>  		jregAD |= 0x01;  /* HBE D[5] */
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x03, 0xE0,
> (temp & 0x1f));
>  
> -	temp = (mode->crtc_hsync_start >> 3) - 1;
> +	temp = ((mode->crtc_hsync_start-precache) >> 3) - 1;
>  	if (temp & 0x100)
>  		jregAC |= 0x40; /* HRS D[5] */
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x04, 0x00,
> temp);
>  
> -	temp = ((mode->crtc_hsync_end >> 3) - 1) & 0x3f;
> +	temp = (((mode->crtc_hsync_end-precache) >> 3) - 1) & 0x3f;
>  	if (temp & 0x20)
>  		jregAD |= 0x04; /* HRE D[5] */
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x05, 0x60,
> (u8)((temp & 0x1f) | jreg05));
> @@ -363,6 +366,11 @@ static void ast_set_crtc_reg(struct drm_crtc
> *crtc, struct drm_display_mode *mod
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x09, 0xdf,
> jreg09);
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xAE, 0x00,
> (jregAE | 0x80));
>  
> +    if (precache)
> +		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6,
> 0x3f, 0x80);
> +    else
> +		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6,
> 0x3f, 0x00);
> +
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0x11, 0x7f,
> 0x80);
>  }
>  
> @@ -383,12 +391,15 @@ static void ast_set_dclk_reg(struct drm_device
> *dev, struct drm_display_mode *mo
>  	struct ast_private *ast = dev->dev_private;
>  	struct ast_vbios_dclk_info *clk_info;
>  
> -	clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
> +	if (ast->chip == AST2500)
> +		clk_info = &dclk_table_ast2500[vbios_mode-
> >enh_table->dclk_index];
> +	else
> +		clk_info = &dclk_table[vbios_mode->enh_table-
> >dclk_index];
>  
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xc0, 0x00,
> clk_info->param1);
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xc1, 0x00,
> clk_info->param2);
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xbb, 0x0f,
> -			       (clk_info->param3 & 0x80) |
> ((clk_info->param3 & 0x3) << 4));
> +			       (clk_info->param3 & 0xc0) |
> ((clk_info->param3 & 0x3) << 4));
>  }
>  
>  static void ast_set_ext_reg(struct drm_crtc *crtc, struct
> drm_display_mode *mode,
> @@ -421,7 +432,7 @@ static void ast_set_ext_reg(struct drm_crtc
> *crtc, struct drm_display_mode *mode
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa8, 0xfd,
> jregA8);
>  
>  	/* Set Threshold */
> -	if (ast->chip == AST2300 || ast->chip == AST2400) {
> +	if (ast->chip == AST2300 || ast->chip == AST2400 || ast-
> >chip == AST2500) {
>  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7,
> 0x78);
>  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6,
> 0x60);
>  	} else if (ast->chip == AST2100 ||
> @@ -794,7 +805,7 @@ static int ast_mode_valid(struct drm_connector
> *connector,
>  		if ((mode->hdisplay == 1600) && (mode->vdisplay ==
> 900))
>  			return MODE_OK;
>  
> -		if ((ast->chip == AST2100) || (ast->chip == AST2200)
> || (ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip ==
> AST1180)) {
> +		if ((ast->chip == AST2100) || (ast->chip == AST2200)
> || (ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip ==
> AST2500) || (ast->chip == AST1180)) {
>  			if ((mode->hdisplay == 1920) && (mode-
> >vdisplay == 1080))
>  				return MODE_OK;
>  
> diff --git a/drivers/gpu/drm/ast/ast_post.c
> b/drivers/gpu/drm/ast/ast_post.c
> index 7197635..0b41fb6 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -31,7 +31,8 @@
>  
>  #include "ast_dram_tables.h"
>  
> -static void ast_init_dram_2300(struct drm_device *dev);
> +static void ast_post_chip_2300(struct drm_device *dev);
> +static void ast_post_chip_2500(struct drm_device *dev);
>  
>  void ast_enable_vga(struct drm_device *dev)
>  {
> @@ -82,7 +83,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>  	for (i = 0x81; i <= 0x8f; i++)
>  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
>  
> -	if (ast->chip == AST2300 || ast->chip == AST2400) {
> +	if (ast->chip == AST2300 || ast->chip == AST2400 || ast-
> >chip == AST2500) {
>  		if (dev->pdev->revision >= 0x20)
>  			ext_reg_info = extreginfo_ast2300;
>  		else
> @@ -106,7 +107,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
>  
>  	/* Enable RAMDAC for A1 */
>  	reg = 0x04;
> -	if (ast->chip == AST2300 || ast->chip == AST2400)
> +	if (ast->chip == AST2300 || ast->chip == AST2400 || ast-
> >chip == AST2500)
>  		reg |= 0x20;
>  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff,
> reg);
>  }
> @@ -380,8 +381,10 @@ void ast_post_gpu(struct drm_device *dev)
>  	ast_set_def_ext_reg(dev);
>  
>  	if (ast->config_mode == ast_use_p2a) {
> +		if (ast->chip == AST2500)
> +			ast_post_chip_2500(dev);
>  		if (ast->chip == AST2300 || ast->chip == AST2400)
> -			ast_init_dram_2300(dev);
> +			ast_post_chip_2300(dev);
>  		else
>  			ast_init_dram_reg(dev);
>  
> @@ -445,85 +448,53 @@ static const u32 pattern[8] = {
>  	0x7C61D253
>  };
>  
> -static int mmc_test_burst(struct ast_private *ast, u32 datagen)
> +static int mmc_test(struct ast_private *ast, u32 datagen, u8
> test_ctl)
>  {
>  	u32 data, timeout;
>  
>  	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -	ast_moutdwm(ast, 0x1e6e0070, 0x000000c1 | (datagen << 3));
> +	ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
>  	timeout = 0;
>  	do {
>  		data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
>  		if (data & 0x2000) {
> -			return 0;
> +			return -1;
>  		}
>  		if (++timeout > TIMEOUT) {
>  			ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -			return 0;
> +			return -1;
>  		}
>  	} while (!data);
> +	data = ast_mindwm(ast, 0x1e6e0078);
> +	data = (data | (data >> 16)) & 0xffff;
>  	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -	return 1;
> +	return data;
>  }
>  
> -static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
> +
> +static int mmc_test_burst(struct ast_private *ast, u32 datagen)
>  {
> -	u32 data, timeout;
> +	return mmc_test(ast, datagen, 0xc1);
> +}
>  
> -	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -	ast_moutdwm(ast, 0x1e6e0070, 0x00000041 | (datagen << 3));
> -	timeout = 0;
> -	do {
> -		data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
> -		if (++timeout > TIMEOUT) {
> -			ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -			return -1;
> -		}
> -	} while (!data);
> -	data = ast_mindwm(ast, 0x1e6e0078);
> -	data = (data | (data >> 16)) & 0xffff;
> -	ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -	return data;
> +static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
> +{
> +	return mmc_test(ast, datagen, 0x41);
>  }
>  
>  static int mmc_test_single(struct ast_private *ast, u32 datagen)
>  {
> -	u32 data, timeout;
> -
> -	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -	ast_moutdwm(ast, 0x1e6e0070, 0x000000c5 | (datagen << 3));
> -	timeout = 0;
> -	do {
> -		data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
> -		if (data & 0x2000)
> -			return 0;
> -		if (++timeout > TIMEOUT) {
> -			ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -			return 0;
> -		}
> -	} while (!data);
> -	ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -	return 1;
> +	return mmc_test(ast, datagen, 0xc5);
>  }
>  
>  static int mmc_test_single2(struct ast_private *ast, u32 datagen)
>  {
> -	u32 data, timeout;
> +	return mmc_test(ast, datagen, 0x05);
> +}
>  
> -	ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -	ast_moutdwm(ast, 0x1e6e0070, 0x00000005 | (datagen << 3));
> -	timeout = 0;
> -	do {
> -		data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
> -		if (++timeout > TIMEOUT) {
> -			ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -			return -1;
> -		}
> -	} while (!data);
> -	data = ast_mindwm(ast, 0x1e6e0078);
> -	data = (data | (data >> 16)) & 0xffff;
> -	ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -	return data;
> +static int mmc_test_single_2500(struct ast_private *ast, u32
> datagen)
> +{
> +	return mmc_test(ast, datagen, 0x85);
>  }
>  
>  static int cbr_test(struct ast_private *ast)
> @@ -603,9 +574,9 @@ static u32 cbr_scan2(struct ast_private *ast)
>  
>  static u32 cbr_test3(struct ast_private *ast)
>  {
> -	if (!mmc_test_burst(ast, 0))
> +	if (mmc_test_burst(ast, 0) < 0)
>  		return 0;
> -	if (!mmc_test_single(ast, 0))
> +	if (mmc_test_single(ast, 0) < 0)
>  		return 0;
>  	return 1;
>  }
> @@ -1609,7 +1580,7 @@ static void ddr2_init(struct ast_private *ast,
> struct ast2300_dram_param *param)
>  
>  }
>  
> -static void ast_init_dram_2300(struct drm_device *dev)
> +static void ast_post_chip_2300(struct drm_device *dev)
>  {
>  	struct ast_private *ast = dev->dev_private;
>  	struct ast2300_dram_param param;
> @@ -1660,3 +1631,405 @@ static void ast_init_dram_2300(struct
> drm_device *dev)
>  	} while ((reg & 0x40) == 0);
>  }
>  
> +static bool cbr_test_2500(struct ast_private *ast)
> +{
> +	ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
> +	ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
> +	if (mmc_test_burst(ast, 0) < 0)
> +		return false;
> +	if (mmc_test_single_2500(ast, 0) < 0)
> +		return false;
> +	return true;
> +}
> +
> +static bool ddr_test_2500(struct ast_private *ast)
> +{
> +	ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
> +	ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
> +	if (mmc_test_burst(ast, 0) < 0)
> +		return false;
> +	if (mmc_test_burst(ast, 1) < 0)
> +		return false;
> +	if (mmc_test_burst(ast, 2) < 0)
> +		return false;
> +	if (mmc_test_burst(ast, 3) < 0)
> +		return false;
> +	if (mmc_test_single_2500(ast, 0) < 0)
> +		return false;
> +	return false;
> +}
> +
> +static void ddr_init_common_2500(struct ast_private *ast)
> +{
> +	ast_moutdwm(ast, 0x1E6E0034,0x00020080);
> +	ast_moutdwm(ast, 0x1E6E0008,0x2003000F);
> +	ast_moutdwm(ast, 0x1E6E0038,0x00000FFF);
> +	ast_moutdwm(ast, 0x1E6E0040,0x88448844);
> +	ast_moutdwm(ast, 0x1E6E0044,0x24422288);
> +	ast_moutdwm(ast, 0x1E6E0048,0x22222222);
> +	ast_moutdwm(ast, 0x1E6E004C,0x22222222);
> +	ast_moutdwm(ast, 0x1E6E0050,0x80000000);
> +	ast_moutdwm(ast, 0x1E6E0208,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E0218,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E0220,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E0228,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E0230,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E02A8,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E02B0,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E0240,0x86000000);
> +	ast_moutdwm(ast, 0x1E6E0244,0x00008600);
> +	ast_moutdwm(ast, 0x1E6E0248,0x80000000);
> +	ast_moutdwm(ast, 0x1E6E024C,0x80808080);
> +}
> +
> +static void ddr_phy_init_2500(struct ast_private *ast)
> +{
> +	u32 data, pass, timecnt;
> +
> +	pass = 0;
> +	ast_moutdwm(ast, 0x1E6E0060,0x00000005);
> +	while (!pass) {
> +		for(timecnt = 0;timecnt < TIMEOUT;timecnt++) {
> +			data = ast_mindwm(ast, 0x1E6E0060) & 0x1;
> +			if (!data)
> +				break;
> +		}
> +		if (timecnt != TIMEOUT) {
> +			data = ast_mindwm(ast, 0x1E6E0300) &
> 0x000A0000;
> +			if (!data)
> +				pass = 1;
> +		}
> +		if (!pass) {
> +			ast_moutdwm(ast, 0x1E6E0060,0x00000000);
> +			udelay(10); /* delay 10 us */
> +			ast_moutdwm(ast, 0x1E6E0060,0x00000005);
> +		}
> +	}
> +
> +	ast_moutdwm(ast, 0x1E6E0060,0x00000006);
> +}
> +
> +/*******************************************************************
> ***********
> + Check DRAM Size
> + 1Gb : 0x80000000 ~ 0x87FFFFFF
> + 2Gb : 0x80000000 ~ 0x8FFFFFFF
> + 4Gb : 0x80000000 ~ 0x9FFFFFFF
> + 8Gb : 0x80000000 ~ 0xBFFFFFFF
> +********************************************************************
> *********/
> +static void check_dram_size_2500(struct ast_private *ast, u32 tRFC)
> +{
> +	u32 reg_04, reg_14;
> +
> +	reg_04 = ast_mindwm(ast, 0x1E6E0004) & 0xfffffffc;
> +	reg_14 = ast_mindwm(ast, 0x1E6E0014) & 0xffffff00;
> +
> +	ast_moutdwm(ast, 0xA0100000, 0x41424344);
> +	ast_moutdwm(ast, 0x90100000, 0x35363738);
> +	ast_moutdwm(ast, 0x88100000, 0x292A2B2C);
> +	ast_moutdwm(ast, 0x80100000, 0x1D1E1F10);
> +
> +	/* Check 8Gbit */
> +	if (ast_mindwm(ast, 0xA0100000) == 0x41424344) {
> +		reg_04 |= 0x03;
> +		reg_14 |= (tRFC >> 24) & 0xFF;
> +		/* Check 4Gbit */
> +	} else if (ast_mindwm(ast, 0x90100000) == 0x35363738) {
> +		reg_04 |= 0x02;
> +		reg_14 |= (tRFC >> 16) & 0xFF;
> +		/* Check 2Gbit */
> +	} else if (ast_mindwm(ast, 0x88100000) == 0x292A2B2C) {
> +		reg_04 |= 0x01;
> +		reg_14 |= (tRFC >> 8) & 0xFF;
> +	} else {
> +		reg_14 |= tRFC & 0xFF;
> +	}
> +	ast_moutdwm(ast, 0x1E6E0004, reg_04);
> +	ast_moutdwm(ast, 0x1E6E0014, reg_14);
> +}
> +
> +static void enable_cache_2500(struct ast_private *ast)
> +{
> +	u32 reg_04, data;
> +
> +	reg_04 = ast_mindwm(ast, 0x1E6E0004);
> +	ast_moutdwm(ast, 0x1E6E0004, reg_04 | 0x1000);
> +
> +	do
> +		data = ast_mindwm(ast, 0x1E6E0004);
> +	while (!(data & 0x80000));
> +	ast_moutdwm(ast, 0x1E6E0004, reg_04 | 0x400);
> +}
> +
> +static void set_mpll_2500(struct ast_private *ast)
> +{
> +	u32 addr, data, param;
> +
> +	/* Reset MMC */
> +	ast_moutdwm(ast, 0x1E6E0000,0xFC600309);
> +	ast_moutdwm(ast, 0x1E6E0034,0x00020080);
> +	for (addr = 0x1e6e0004; addr < 0x1e6e0090;) {
> +		ast_moutdwm(ast, addr, 0x0);
> +		addr += 4;
> +	}
> +	ast_moutdwm(ast, 0x1E6E0034,0x00020000);
> +
> +	ast_moutdwm(ast, 0x1E6E2000, 0x1688A8A8);
> +	data = ast_mindwm(ast, 0x1E6E2070) & 0x00800000;
> +	if (data) {
> +		/* CLKIN = 25MHz */
> +		param = 0x930023E0;
> +		ast_moutdwm(ast, 0x1E6E2160, 0x00011320);
> +	} else {
> +		/* CLKIN = 24MHz */
> +		param = 0x93002400;
> +	}
> +	ast_moutdwm(ast, 0x1E6E2020, param);
> +	udelay(100);
> +}
> +
> +static void reset_mmc_2500(struct ast_private *ast)
> +{
> +	ast_moutdwm(ast, 0x1E78505C,0x00000004);
> +	ast_moutdwm(ast, 0x1E785044,0x00000001);
> +	ast_moutdwm(ast, 0x1E785048,0x00004755);
> +	ast_moutdwm(ast, 0x1E78504C,0x00000013);
> +	mdelay(100);					/* delay
> 100ms */
> +	ast_moutdwm(ast, 0x1E785054,0x00000077);
> +	ast_moutdwm(ast, 0x1E6E0000,0xFC600309);
> +}
> +
> +static void ddr3_init_2500(struct ast_private *ast, u32 *ddr_table)
> +{
> +
> +	ast_moutdwm(ast, 0x1E6E0004,0x00000303);
> +	ast_moutdwm(ast, 0x1E6E0010,ddr_table[REGIDX_010]);
> +	ast_moutdwm(ast, 0x1E6E0014,ddr_table[REGIDX_014]);
> +	ast_moutdwm(ast, 0x1E6E0018,ddr_table[REGIDX_018]);
> +	ast_moutdwm(ast,
> 0x1E6E0020,ddr_table[REGIDX_020]);          /* MODEREG4/6 */
> +	ast_moutdwm(ast,
> 0x1E6E0024,ddr_table[REGIDX_024]);          /* MODEREG5 */
> +	ast_moutdwm(ast, 0x1E6E002C,ddr_table[REGIDX_02C] |
> 0x100);  /* MODEREG0/2 */
> +	ast_moutdwm(ast,
> 0x1E6E0030,ddr_table[REGIDX_030]);          /* MODEREG1/3 */
> +
> +	/* DDR PHY Setting */
> +	ast_moutdwm(ast, 0x1E6E0200,0x02492AAE);
> +	ast_moutdwm(ast, 0x1E6E0204,0x00001001);
> +	ast_moutdwm(ast, 0x1E6E020C,0x55E00B0B);
> +	ast_moutdwm(ast, 0x1E6E0210,0x20000000);
> +	ast_moutdwm(ast, 0x1E6E0214,ddr_table[REGIDX_214]);
> +	ast_moutdwm(ast, 0x1E6E02E0,ddr_table[REGIDX_2E0]);
> +	ast_moutdwm(ast, 0x1E6E02E4,ddr_table[REGIDX_2E4]);
> +	ast_moutdwm(ast, 0x1E6E02E8,ddr_table[REGIDX_2E8]);
> +	ast_moutdwm(ast, 0x1E6E02EC,ddr_table[REGIDX_2EC]);
> +	ast_moutdwm(ast, 0x1E6E02F0,ddr_table[REGIDX_2F0]);
> +	ast_moutdwm(ast, 0x1E6E02F4,ddr_table[REGIDX_2F4]);
> +	ast_moutdwm(ast, 0x1E6E02F8,ddr_table[REGIDX_2F8]);
> +	ast_moutdwm(ast, 0x1E6E0290,0x00100008);
> +	ast_moutdwm(ast, 0x1E6E02C0,0x00000006);
> +
> +	/* Controller Setting */
> +	ast_moutdwm(ast, 0x1E6E0034,0x00020091);
> +
> +	/* Wait DDR PHY init done */
> +	ddr_phy_init_2500(ast);
> +
> +	ast_moutdwm(ast, 0x1E6E0120,ddr_table[REGIDX_PLL]);
> +	ast_moutdwm(ast, 0x1E6E000C,0x42AA5C81);
> +	ast_moutdwm(ast, 0x1E6E0034,0x0001AF93);
> +
> +	check_dram_size_2500(ast, ddr_table[REGIDX_RFC]);
> +	enable_cache_2500(ast);
> +	ast_moutdwm(ast, 0x1E6E001C,0x00000008);
> +	ast_moutdwm(ast, 0x1E6E0038,0xFFFFFF00);
> +}
> +
> +static void ddr4_init_2500(struct ast_private *ast, u32 *ddr_table)
> +{
> +	u32 data, data2, pass, retrycnt;
> +	u32 ddr_vref, phy_vref;
> +	u32 min_ddr_vref=0, min_phy_vref=0;
> +	u32 max_ddr_vref=0, max_phy_vref=0;
> +
> +	ast_moutdwm(ast, 0x1E6E0004,0x00000313);
> +	ast_moutdwm(ast, 0x1E6E0010,ddr_table[REGIDX_010]);
> +	ast_moutdwm(ast, 0x1E6E0014,ddr_table[REGIDX_014]);
> +	ast_moutdwm(ast, 0x1E6E0018,ddr_table[REGIDX_018]);
> +	ast_moutdwm(ast,
> 0x1E6E0020,ddr_table[REGIDX_020]);          /* MODEREG4/6 */
> +	ast_moutdwm(ast,
> 0x1E6E0024,ddr_table[REGIDX_024]);          /* MODEREG5 */
> +	ast_moutdwm(ast, 0x1E6E002C,ddr_table[REGIDX_02C] |
> 0x100);  /* MODEREG0/2 */
> +	ast_moutdwm(ast,
> 0x1E6E0030,ddr_table[REGIDX_030]);          /* MODEREG1/3 */
> +
> +	/* DDR PHY Setting */
> +	ast_moutdwm(ast, 0x1E6E0200,0x42492AAE);
> +	ast_moutdwm(ast, 0x1E6E0204,0x09002000);
> +	ast_moutdwm(ast, 0x1E6E020C,0x55E00B0B);
> +	ast_moutdwm(ast, 0x1E6E0210,0x20000000);
> +	ast_moutdwm(ast, 0x1E6E0214,ddr_table[REGIDX_214]);
> +	ast_moutdwm(ast, 0x1E6E02E0,ddr_table[REGIDX_2E0]);
> +	ast_moutdwm(ast, 0x1E6E02E4,ddr_table[REGIDX_2E4]);
> +	ast_moutdwm(ast, 0x1E6E02E8,ddr_table[REGIDX_2E8]);
> +	ast_moutdwm(ast, 0x1E6E02EC,ddr_table[REGIDX_2EC]);
> +	ast_moutdwm(ast, 0x1E6E02F0,ddr_table[REGIDX_2F0]);
> +	ast_moutdwm(ast, 0x1E6E02F4,ddr_table[REGIDX_2F4]);
> +	ast_moutdwm(ast, 0x1E6E02F8,ddr_table[REGIDX_2F8]);
> +	ast_moutdwm(ast, 0x1E6E0290,0x00100008);
> +	ast_moutdwm(ast, 0x1E6E02C4,0x3C183C3C);
> +	ast_moutdwm(ast, 0x1E6E02C8,0x00631E0E);
> +
> +	/* Controller Setting */
> +	ast_moutdwm(ast, 0x1E6E0034,0x0001A991);
> +
> +	/* Train PHY Vref first */
> +	pass = 0;
> +
> +	for (retrycnt = 0;retrycnt < 4 && pass == 0;retrycnt++) {
> +		max_phy_vref = 0x0;
> +		pass = 0;
> +		ast_moutdwm(ast, 0x1E6E02C0,0x00001C06);
> +		for (phy_vref = 0x40;phy_vref < 0x80;phy_vref++) {
> +			ast_moutdwm(ast, 0x1E6E000C,0x00000000);
> +			ast_moutdwm(ast, 0x1E6E0060,0x00000000);
> +			ast_moutdwm(ast, 0x1E6E02CC,phy_vref |
> (phy_vref << 8));
> +			/* Fire DFI Init */
> +			ddr_phy_init_2500(ast);
> +			ast_moutdwm(ast, 0x1E6E000C,0x00005C01);
> +			if (cbr_test_2500(ast)) {
> +				pass++;
> +				data = ast_mindwm(ast, 0x1E6E03D0);
> +				data2 = data >> 8;
> +				data  = data & 0xff;
> +				if (data > data2)
> +					data = data2;
> +				if (max_phy_vref < data) {
> +					max_phy_vref = data;
> +					min_phy_vref = phy_vref;
> +				}
> +			} else if (pass > 0)
> +				break;
> +		}
> +	}
> +	ast_moutdwm(ast, 0x1E6E02CC,min_phy_vref | (min_phy_vref <<
> 8));
> +
> +	/* Train DDR Vref next */
> +	pass = 0;
> +
> +	for (retrycnt = 0;retrycnt < 4 && pass == 0;retrycnt++) {
> +		min_ddr_vref = 0xFF;
> +		max_ddr_vref = 0x0;
> +		pass = 0;
> +		for (ddr_vref = 0x00;ddr_vref < 0x40;ddr_vref++) {
> +			ast_moutdwm(ast, 0x1E6E000C,0x00000000);
> +			ast_moutdwm(ast, 0x1E6E0060,0x00000000);
> +			ast_moutdwm(ast, 0x1E6E02C0,0x00000006 |
> (ddr_vref << 8));
> +			/* Fire DFI Init */
> +			ddr_phy_init_2500(ast);
> +			ast_moutdwm(ast, 0x1E6E000C,0x00005C01);
> +			if (cbr_test_2500(ast)) {
> +				pass++;
> +				if (min_ddr_vref > ddr_vref)
> +					min_ddr_vref = ddr_vref;
> +				if (max_ddr_vref < ddr_vref)
> +					max_ddr_vref = ddr_vref;
> +			} else if (pass != 0)
> +				break;
> +		}
> +	}
> +
> +	ast_moutdwm(ast, 0x1E6E000C,0x00000000);
> +	ast_moutdwm(ast, 0x1E6E0060,0x00000000);
> +	ddr_vref = (min_ddr_vref + max_ddr_vref + 1) >> 1;
> +	ast_moutdwm(ast, 0x1E6E02C0,0x00000006 | (ddr_vref << 8));
> +
> +	/* Wait DDR PHY init done */
> +	ddr_phy_init_2500(ast);
> +
> +	ast_moutdwm(ast, 0x1E6E0120,ddr_table[REGIDX_PLL]);
> +	ast_moutdwm(ast, 0x1E6E000C,0x42AA5C81);
> +	ast_moutdwm(ast, 0x1E6E0034,0x0001AF93);
> +
> +	check_dram_size_2500(ast, ddr_table[REGIDX_RFC]);
> +	enable_cache_2500(ast);
> +	ast_moutdwm(ast, 0x1E6E001C,0x00000008);
> +	ast_moutdwm(ast, 0x1E6E0038,0xFFFFFF00);
> +}
> +
> +static bool ast_dram_init_2500(struct ast_private *ast)
> +{
> +	u32 data;
> +	u32 max_tries = 5;
> +
> +	do {
> +		if (max_tries-- == 0)
> +			return false;
> +		set_mpll_2500(ast);
> +		reset_mmc_2500(ast);
> +		ddr_init_common_2500(ast);
> +
> +		data = ast_mindwm(ast, 0x1E6E2070);
> +		if (data & 0x01000000)
> +			ddr4_init_2500(ast,
> ast2500_ddr4_1600_timing_table);
> +		else
> +			ddr3_init_2500(ast,
> ast2500_ddr3_1600_timing_table);
> +	} while (!ddr_test_2500(ast));
> +
> +	ast_moutdwm(ast, 0x1E6E2040, ast_mindwm(ast, 0x1E6E2040) |
> 0x41);
> +
> +	/* Patch code */
> +	data = ast_mindwm(ast, 0x1E6E200C) & 0xF9FFFFFF;
> +	ast_moutdwm(ast, 0x1E6E200C, data | 0x10000000);
> +
> +	return true;
> +}
> +
> +void ast_post_chip_2500(struct drm_device *dev)
> +{
> +	struct ast_private *ast = dev->dev_private;
> +	u32 temp;
> +	u8 reg;
> +
> +	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0,
> 0xff);
> +	if ((reg & 0x80) == 0) {/* vga only */
> +  		/* Clear bus lock condition */
> +  		ast_moutdwm(ast, 0x1e600000,0xAEED1A03);
> +		ast_moutdwm(ast, 0x1e600084,0x00010000);
> +		ast_moutdwm(ast, 0x1e600088,0x00000000);
> +		ast_moutdwm(ast, 0x1e6e2000,0x1688A8A8);
> +		ast_write32(ast, 0xf004, 0x1e6e0000);
> +		ast_write32(ast, 0xf000, 0x1);
> +		ast_write32(ast, 0x12000, 0x1688a8a8);
> +		while (ast_read32(ast, 0x12000) != 0x1)
> +			;
> +
> +		ast_write32(ast, 0x10000, 0xfc600309);
> +		while (ast_read32(ast, 0x10000) != 0x1)
> +			;
> +
> +		/* Slow down CPU/AHB CLK in VGA only mode */
> +		temp = ast_read32(ast, 0x12008);
> +		temp |= 0x73;
> +		ast_write32(ast, 0x12008, temp);
> +
> +		/* Reset USB port to patch USB unknown device issue
> */
> + 	 	ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
> +		temp  = ast_mindwm(ast, 0x1e6e2094);
> +		temp |= 0x00004000;
> +		ast_moutdwm(ast, 0x1e6e2094, temp);
> +		temp  = ast_mindwm(ast, 0x1e6e2070);
> +		if (temp & 0x00800000) {
> +			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
> +			mdelay(100);
> +			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
> +		}
> +
> +		if (!ast_dram_init_2500(ast)) {
> +			DRM_ERROR("DRAM init failed !\n");
> +		}
> +
> +		temp = ast_mindwm(ast, 0x1e6e2040);
> +		ast_moutdwm(ast, 0x1e6e2040, temp | 0x40);
> +	}
> +
> +	/* wait ready */
> +	do {
> +		reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT,
> 0xd0, 0xff);
> +	} while ((reg & 0x40) == 0);
> +}
> diff --git a/drivers/gpu/drm/ast/ast_tables.h
> b/drivers/gpu/drm/ast/ast_tables.h
> index 3608d5a..24da4c2 100644
> --- a/drivers/gpu/drm/ast/ast_tables.h
> +++ b/drivers/gpu/drm/ast/ast_tables.h
> @@ -47,6 +47,7 @@
>  #define SyncPN			(PVSync | NHSync)
>  #define SyncNP			(NVSync | PHSync)
>  #define SyncNN			(NVSync | NHSync)
> +#define AST2500PreCatchCRT		0x00004000
>  
>  /* DCLK Index */
>  #define VCLK25_175     		0x00
> @@ -108,6 +109,36 @@ static struct ast_vbios_dclk_info dclk_table[] =
> {
>  	{0x3b, 0x2c, 0x81},					/
> * 1A: VCLK118_25	*/
>  };
>  
> +static struct ast_vbios_dclk_info dclk_table_ast2500[] = {
> +	{0x2C, 0xE7, 0x03},					/
> * 00: VCLK25_175	*/
> +	{0x95, 0x62, 0x03},				        /
> * 01: VCLK28_322	*/
> +	{0x67, 0x63, 0x01},				        /
> * 02: VCLK31_5         */
> +	{0x76, 0x63, 0x01},				        /
> * 03: VCLK36         	*/
> +	{0xEE, 0x67, 0x01},				        /
> * 04: VCLK40          	*/
> +	{0x82, 0x62, 0x01}, 			        /* 05:
> VCLK49_5        	*/
> +	{0xC6, 0x64, 0x01},                        	        /
> * 06: VCLK50          	*/
> +	{0x94, 0x62, 0x01},                        	        /
> * 07: VCLK56_25       	*/
> +	{0x80, 0x64, 0x00},                        	        /
> * 08: VCLK65		*/
> +	{0x7B, 0x63, 0x00},                        	        /
> * 09: VCLK75	        */
> +	{0x67, 0x62, 0x00},				        /
> * 0A: VCLK78_75       	*/
> +	{0x7C, 0x62, 0x00},                        	        /
> * 0B: VCLK94_5        	*/
> +	{0x8E, 0x62, 0x00},                        	        /
> * 0C: VCLK108         	*/
> +	{0x85, 0x24, 0x00},                        	        /
> * 0D: VCLK135         	*/
> +	{0x67, 0x22, 0x00},                        	        /
> * 0E: VCLK157_5       	*/
> +	{0x6A, 0x22, 0x00},				        /
> * 0F: VCLK162         	*/
> +	{0x4d, 0x4c, 0x80},				        /
> * 10: VCLK154      	*/
> +	{0xa7, 0x78, 0x80},					/
> * 11: VCLK83.5         */
> +	{0x28, 0x49, 0x80},					/
> * 12: VCLK106.5        */
> +	{0x37, 0x49, 0x80},					/
> * 13: VCLK146.25       */
> +	{0x1f, 0x45, 0x80},					/
> * 14: VCLK148.5        */
> +	{0x47, 0x6c, 0x80},					/
> * 15: VCLK71       */
> +	{0x25, 0x65, 0x80},					/
> * 16: VCLK88.75    */
> +	{0x58, 0x01, 0x42},					/
> * 17: VCLK119      */
> +	{0x32, 0x67, 0x80},				    /*
> 18: VCLK85_5     */
> +	{0x6a, 0x6d, 0x80},					/
> * 19: VCLK97_75	*/
> +	{0x44, 0x20, 0x43},					/
> * 1A: VCLK118_25	*/
> +};
> +
>  static struct ast_vbios_stdtable vbios_stdtable[] = {
>  	/* MD_2_3_400 */
>  	{
> @@ -244,32 +275,32 @@ static struct ast_vbios_enhtable
> res_1600x1200[] = {
>  /* 16:9 */
>  static struct ast_vbios_enhtable res_1360x768[] = {
>  	{1792, 1360, 64,112, 795,  768, 3, 6, VCLK85_5,	     
>     /* 60Hz */
> -	 (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 1, 0x39 },
> +	 (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x39 },
>  	{1792, 1360, 64,112, 795,  768, 3, 6, VCLK85_5,	     
>     /* end */
>  	 (SyncPP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 0xFF, 1, 0x39 },
>  };
>  
>  static struct ast_vbios_enhtable res_1600x900[] = {
>  	{1760, 1600, 48, 32, 926,  900, 3, 5, VCLK97_75,	/*
> 60Hz CVT RB */
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 1, 0x3A },
> -	{2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/*
> 60Hz CVT */
> -	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 2, 0x3A },
> -	{2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/*
> 60Hz CVT */
> -	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 0xFF, 2, 0x3A },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x3A },
> +    {2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/* 60Hz
> CVT */
> +     (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 2, 0x3A },
> +    {2112, 1600, 88,168, 934,  900, 3, 5, VCLK118_25,	/* 60Hz
> CVT */
> +     (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 0xFF, 2, 0x3A },
>  };
>  
>  static struct ast_vbios_enhtable res_1920x1080[] = {
>  	{2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,	/*
> 60Hz */
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 1, 0x38 },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x38 },
>  	{2200, 1920, 88, 44, 1125, 1080, 4, 5, VCLK148_5,	/*
> 60Hz */
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 0xFF, 1, 0x38 },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 0xFF, 1, 0x38 },
>  };
>  
>  
>  /* 16:10 */
>  static struct ast_vbios_enhtable res_1280x800[] = {
>  	{1440, 1280, 48, 32,  823,  800, 3, 6, VCLK71,	/*
> 60Hz RB */
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 1, 0x35 },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x35 },
>  	{1680, 1280, 72,128,  831,  800, 3, 6, VCLK83_5,	/*
> 60Hz */
>  	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 2, 0x35 },
>  	{1680, 1280, 72,128,  831,  800, 3, 6, VCLK83_5,	/*
> 60Hz */
> @@ -279,7 +310,7 @@ static struct ast_vbios_enhtable res_1280x800[] =
> {
>  
>  static struct ast_vbios_enhtable res_1440x900[] = {
>  	{1600, 1440, 48, 32,  926,  900, 3, 6, VCLK88_75,	/*
> 60Hz RB */
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 1, 0x36 },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x36 },
>  	{1904, 1440, 80,152,  934,  900, 3, 6, VCLK106_5,	/*
> 60Hz */
>  	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 2, 0x36 },
>  	{1904, 1440, 80,152,  934,  900, 3, 6, VCLK106_5,	/*
> 60Hz */
> @@ -288,7 +319,7 @@ static struct ast_vbios_enhtable res_1440x900[] =
> {
>  
>  static struct ast_vbios_enhtable res_1680x1050[] = {
>  	{1840, 1680, 48, 32, 1080, 1050, 3, 6, VCLK119,	/*
> 60Hz RB */
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 1, 0x37 },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x37 },
>  	{2240, 1680,104,176, 1089, 1050, 3, 6, VCLK146_25,	/*
> 60Hz */
>  	 (SyncPN | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 2, 0x37 },
>  	{2240, 1680,104,176, 1089, 1050, 3, 6, VCLK146_25,	/*
> 60Hz */
> @@ -297,9 +328,9 @@ static struct ast_vbios_enhtable res_1680x1050[]
> = {
>  
>  static struct ast_vbios_enhtable res_1920x1200[] = {
>  	{2080, 1920, 48, 32, 1235, 1200, 3, 6, VCLK154,	/*
> 60Hz RB*/
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 60, 1, 0x34 },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 60, 1, 0x34 },
>  	{2080, 1920, 48, 32, 1235, 1200, 3, 6, VCLK154,	/*
> 60Hz RB */
> -	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo), 0xFF, 1, 0x34 },
> +	 (SyncNP | Charx8Dot | LineCompareOff | WideScreenMode |
> NewModeInfo | AST2500PreCatchCRT), 0xFF, 1, 0x34 },
>  };
>  
>  #endif
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16  9:09 [PATCH 2/2] drm/ast: Support AST2500 Benjamin Herrenschmidt
  2017-02-16 11:40 ` Benjamin Herrenschmidt
@ 2017-02-16 17:33 ` Emil Velikov
  2017-02-16 21:09   ` Benjamin Herrenschmidt
                     ` (4 more replies)
  1 sibling, 5 replies; 15+ messages in thread
From: Emil Velikov @ 2017-02-16 17:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On 16 February 2017 at 09:09, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> From: "Y.C. Chen" <yc_chen@aspeedtech.com>
>
> Add detection and POST code for AST2500 generation chip,
> code originally from Aspeed and slightly reworked for
> coding style mostly by Ben.
>
> Signed-off-by: Y.C. Chen <yc_chen@aspeedtech.com>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> v2. [BenH]
>   - Coding style fixes
>   - Add timeout to main DRAM init loop
>   - Improve boot time display of RAM info
>
> Y.C. Please check that I didn't break POST as I haven't manage to test
> that (we can't boot our machines if the BMC isn't already POSTed).
>
Hi Ben,

Barring the checkpatch.pl warnings/errors that you've spotted there's
a few other comments below.
I'm not familiar with the hardware, so it's just things that strike me
as odd ;-)


> +/*
> + * AST2500 DRAM settings modules
> + */
> +#define REGTBL_NUM           17
> +#define REGIDX_010           0
> +#define REGIDX_014           1
> +#define REGIDX_018           2
> +#define REGIDX_020           3
> +#define REGIDX_024           4
> +#define REGIDX_02C           5
> +#define REGIDX_030           6
> +#define REGIDX_214           7
> +#define REGIDX_2E0           8
> +#define REGIDX_2E4           9
> +#define REGIDX_2E8           10
> +#define REGIDX_2EC           11
> +#define REGIDX_2F0           12
> +#define REGIDX_2F4           13
> +#define REGIDX_2F8           14
> +#define REGIDX_RFC           15
> +#define REGIDX_PLL           16
These are used to address the correct value in the array, Worth using
C99 initailizer to ensure that things end in the right place ?
IIRC there was some security related GCC plugin which can fuzz the
order of array(?) and/or struct members ?

> +
> +static u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {

> +static u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
These two are constant data, although you'll need to annotate the users as well.


> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -32,8 +32,6 @@
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_crtc_helper.h>
>
> -#include "ast_dram_tables.h"
> -
Unrelated change ?


> -               DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast->dram_type, ast->dram_bus_width, ast->vram_size);
> +               DRM_INFO("dram MCLK=%u type=%d bus_width=%d size=%08x\n",
> +                        ast->mclk, ast->dram_type, ast->dram_bus_width, ast->vram_size);
Unrelated change ?


>  static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode,
> @@ -421,7 +432,7 @@ static void ast_set_ext_reg(struct drm_crtc *crtc, struct drm_display_mode *mode
>         ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa8, 0xfd, jregA8);
>
>         /* Set Threshold */
> -       if (ast->chip == AST2300 || ast->chip == AST2400) {
> +       if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500) {
>                 ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x78);
>                 ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x60);
>         } else if (ast->chip == AST2100 ||
> @@ -794,7 +805,7 @@ static int ast_mode_valid(struct drm_connector *connector,
>                 if ((mode->hdisplay == 1600) && (mode->vdisplay == 900))
>                         return MODE_OK;
>
> -               if ((ast->chip == AST2100) || (ast->chip == AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip == AST1180)) {
> +               if ((ast->chip == AST2100) || (ast->chip == AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip == AST2500) || (ast->chip == AST1180)) {
178 columns is getting a bit too much.


> -static int mmc_test_burst(struct ast_private *ast, u32 datagen)
> +static int mmc_test(struct ast_private *ast, u32 datagen, u8 test_ctl)
>  {
>         u32 data, timeout;
>
>         ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -       ast_moutdwm(ast, 0x1e6e0070, 0x000000c1 | (datagen << 3));
> +       ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
>         timeout = 0;
>         do {
>                 data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
>                 if (data & 0x2000) {
> -                       return 0;
> +                       return -1;
>                 }
>                 if (++timeout > TIMEOUT) {
>                         ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -                       return 0;
> +                       return -1;
>                 }
>         } while (!data);
> +       data = ast_mindwm(ast, 0x1e6e0078);
> +       data = (data | (data >> 16)) & 0xffff;
>         ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -       return 1;
> +       return data;
>  }
>
> -static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
> +
> +static int mmc_test_burst(struct ast_private *ast, u32 datagen)
>  {
> -       u32 data, timeout;
> +       return mmc_test(ast, datagen, 0xc1);
> +}
>
> -       ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -       ast_moutdwm(ast, 0x1e6e0070, 0x00000041 | (datagen << 3));
> -       timeout = 0;
> -       do {
> -               data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
> -               if (++timeout > TIMEOUT) {
> -                       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -                       return -1;
> -               }
> -       } while (!data);
> -       data = ast_mindwm(ast, 0x1e6e0078);
> -       data = (data | (data >> 16)) & 0xffff;
> -       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -       return data;
> +static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
> +{
> +       return mmc_test(ast, datagen, 0x41);
>  }
>
>  static int mmc_test_single(struct ast_private *ast, u32 datagen)
>  {
> -       u32 data, timeout;
> -
> -       ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -       ast_moutdwm(ast, 0x1e6e0070, 0x000000c5 | (datagen << 3));
> -       timeout = 0;
> -       do {
> -               data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
> -               if (data & 0x2000)
> -                       return 0;
> -               if (++timeout > TIMEOUT) {
> -                       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -                       return 0;
> -               }
> -       } while (!data);
> -       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -       return 1;
> +       return mmc_test(ast, datagen, 0xc5);
>  }
>
>  static int mmc_test_single2(struct ast_private *ast, u32 datagen)
>  {
> -       u32 data, timeout;
> +       return mmc_test(ast, datagen, 0x05);
> +}
>
> -       ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> -       ast_moutdwm(ast, 0x1e6e0070, 0x00000005 | (datagen << 3));
> -       timeout = 0;
> -       do {
> -               data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
> -               if (++timeout > TIMEOUT) {
> -                       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -                       return -1;
> -               }
> -       } while (!data);
> -       data = ast_mindwm(ast, 0x1e6e0078);
> -       data = (data | (data >> 16)) & 0xffff;
> -       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> -       return data;
> +static int mmc_test_single_2500(struct ast_private *ast, u32 datagen)
> +{
> +       return mmc_test(ast, datagen, 0x85);
>  }
>
>  static int cbr_test(struct ast_private *ast)
> @@ -603,9 +574,9 @@ static u32 cbr_scan2(struct ast_private *ast)
>
>  static u32 cbr_test3(struct ast_private *ast)
>  {
> -       if (!mmc_test_burst(ast, 0))
> +       if (mmc_test_burst(ast, 0) < 0)
>                 return 0;
> -       if (!mmc_test_single(ast, 0))
> +       if (mmc_test_single(ast, 0) < 0)

The above seems like a _lot_ of unrelated rework. Keep the refactoring
separate ?
New code seems to assume that only(?) -1 is returned on error, yet we do < 0.
This will come to bite you/others as the tests are extended/reworked.

>                 return 0;
>         return 1;
>  }
> @@ -1609,7 +1580,7 @@ static void ddr2_init(struct ast_private *ast, struct ast2300_dram_param *param)
>
>  }
>
> -static void ast_init_dram_2300(struct drm_device *dev)
> +static void ast_post_chip_2300(struct drm_device *dev)
Unrelated rename ?


> +static bool ddr_test_2500(struct ast_private *ast)
> +{
> +       ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
> +       ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
> +       if (mmc_test_burst(ast, 0) < 0)
> +               return false;
> +       if (mmc_test_burst(ast, 1) < 0)
> +               return false;
> +       if (mmc_test_burst(ast, 2) < 0)
> +               return false;
> +       if (mmc_test_burst(ast, 3) < 0)
> +               return false;
> +       if (mmc_test_single_2500(ast, 0) < 0)
> +               return false;
> +       return false;
Final return should be true... either things got funny with v2 or the
this patch was never tested ?


> +static void ddr_phy_init_2500(struct ast_private *ast)
> +{
> +       u32 data, pass, timecnt;
> +
> +       pass = 0;
> +       ast_moutdwm(ast, 0x1E6E0060,0x00000005);
> +       while (!pass) {
> +               for(timecnt = 0;timecnt < TIMEOUT;timecnt++) {
> +                       data = ast_mindwm(ast, 0x1E6E0060) & 0x1;
> +                       if (!data)
> +                               break;
> +               }
> +               if (timecnt != TIMEOUT) {
> +                       data = ast_mindwm(ast, 0x1E6E0300) & 0x000A0000;
> +                       if (!data)
> +                               pass = 1;
> +               }
> +               if (!pass) {
> +                       ast_moutdwm(ast, 0x1E6E0060,0x00000000);
> +                       udelay(10); /* delay 10 us */
> +                       ast_moutdwm(ast, 0x1E6E0060,0x00000005);
> +               }
> +       }
> +
> +       ast_moutdwm(ast, 0x1E6E0060,0x00000006);
I realise that there's likely no documentation, yet repeating the same
magic numbers multiple times is a bit meh.


> +}
> +
> +/******************************************************************************
> + Check DRAM Size
> + 1Gb : 0x80000000 ~ 0x87FFFFFF
> + 2Gb : 0x80000000 ~ 0x8FFFFFFF
> + 4Gb : 0x80000000 ~ 0x9FFFFFFF
> + 8Gb : 0x80000000 ~ 0xBFFFFFFF
> +*****************************************************************************/
> +static void check_dram_size_2500(struct ast_private *ast, u32 tRFC)
> +{
> +       u32 reg_04, reg_14;
> +
> +       reg_04 = ast_mindwm(ast, 0x1E6E0004) & 0xfffffffc;
> +       reg_14 = ast_mindwm(ast, 0x1E6E0014) & 0xffffff00;
> +
> +       ast_moutdwm(ast, 0xA0100000, 0x41424344);
> +       ast_moutdwm(ast, 0x90100000, 0x35363738);
> +       ast_moutdwm(ast, 0x88100000, 0x292A2B2C);
> +       ast_moutdwm(ast, 0x80100000, 0x1D1E1F10);
> +
> +       /* Check 8Gbit */
> +       if (ast_mindwm(ast, 0xA0100000) == 0x41424344) {
> +               reg_04 |= 0x03;
> +               reg_14 |= (tRFC >> 24) & 0xFF;
> +               /* Check 4Gbit */
> +       } else if (ast_mindwm(ast, 0x90100000) == 0x35363738) {
> +               reg_04 |= 0x02;
> +               reg_14 |= (tRFC >> 16) & 0xFF;
> +               /* Check 2Gbit */
> +       } else if (ast_mindwm(ast, 0x88100000) == 0x292A2B2C) {
> +               reg_04 |= 0x01;
> +               reg_14 |= (tRFC >> 8) & 0xFF;
> +       } else {
> +               reg_14 |= tRFC & 0xFF;
> +       }
Like in the case above...


> +static void reset_mmc_2500(struct ast_private *ast)
> +{
> +       ast_moutdwm(ast, 0x1E78505C,0x00000004);
> +       ast_moutdwm(ast, 0x1E785044,0x00000001);
> +       ast_moutdwm(ast, 0x1E785048,0x00004755);
> +       ast_moutdwm(ast, 0x1E78504C,0x00000013);
> +       mdelay(100);                                    /* delay 100ms */
"Water is wet" type of comment. Worth mentioning why it's so large -
mentioned in the documentation, experimental result, other ?
Same suggestion goes for the other mdelay(100) instances.


> +static bool ast_dram_init_2500(struct ast_private *ast)
> +{
> +       u32 data;
> +       u32 max_tries = 5;
> +
> +       do {
> +               if (max_tries-- == 0)
> +                       return false;
> +               set_mpll_2500(ast);
> +               reset_mmc_2500(ast);
> +               ddr_init_common_2500(ast);
> +
> +               data = ast_mindwm(ast, 0x1E6E2070);
> +               if (data & 0x01000000)
> +                       ddr4_init_2500(ast, ast2500_ddr4_1600_timing_table);
> +               else
> +                       ddr3_init_2500(ast, ast2500_ddr3_1600_timing_table);
> +       } while (!ddr_test_2500(ast));
> +
> +       ast_moutdwm(ast, 0x1E6E2040, ast_mindwm(ast, 0x1E6E2040) | 0x41);
> +
> +       /* Patch code */
> +       data = ast_mindwm(ast, 0x1E6E200C) & 0xF9FFFFFF;
> +       ast_moutdwm(ast, 0x1E6E200C, data | 0x10000000);
> +
> +       return true;
Drop the return type - function always returns true ?
I think there were a few other functions that could do the same.

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16 17:33 ` Emil Velikov
@ 2017-02-16 21:09   ` Benjamin Herrenschmidt
  2017-02-17 21:27     ` Emil Velikov
  2017-02-16 21:12   ` Benjamin Herrenschmidt
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-16 21:09 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Thu, 2017-02-16 at 17:33 +0000, Emil Velikov wrote:
> On 16 February 2017 at 09:09, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > From: "Y.C. Chen" <yc_chen@aspeedtech.com>
> > 
> > Add detection and POST code for AST2500 generation chip,
> > code originally from Aspeed and slightly reworked for
> > coding style mostly by Ben.
> > 
> > Signed-off-by: Y.C. Chen <yc_chen@aspeedtech.com>
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > ---
> > v2. [BenH]
> >   - Coding style fixes
> >   - Add timeout to main DRAM init loop
> >   - Improve boot time display of RAM info
> > 
> > Y.C. Please check that I didn't break POST as I haven't manage to
> > test
> > that (we can't boot our machines if the BMC isn't already POSTed).
> > 
> 
> Hi Ben,
> 
> Barring the checkpatch.pl warnings/errors that you've spotted there's
> a few other comments below.
> I'm not familiar with the hardware, so it's just things that strike
> me
> as odd ;-)

Heh ok. I don't want to change that POST code too much as I'm not
equipped to test it, but I'll have a look later today.

Thanks,
Ben.

> 
> > +/*
> > + * AST2500 DRAM settings modules
> > + */
> > +#define REGTBL_NUM           17
> > +#define REGIDX_010           0
> > +#define REGIDX_014           1
> > +#define REGIDX_018           2
> > +#define REGIDX_020           3
> > +#define REGIDX_024           4
> > +#define REGIDX_02C           5
> > +#define REGIDX_030           6
> > +#define REGIDX_214           7
> > +#define REGIDX_2E0           8
> > +#define REGIDX_2E4           9
> > +#define REGIDX_2E8           10
> > +#define REGIDX_2EC           11
> > +#define REGIDX_2F0           12
> > +#define REGIDX_2F4           13
> > +#define REGIDX_2F8           14
> > +#define REGIDX_RFC           15
> > +#define REGIDX_PLL           16
> 
> These are used to address the correct value in the array, Worth using
> C99 initailizer to ensure that things end in the right place ?
> IIRC there was some security related GCC plugin which can fuzz the
> order of array(?) and/or struct members ?
> 
> > +
> > +static u32 ast2500_ddr3_1600_timing_table[REGTBL_NUM] = {
> > +static u32 ast2500_ddr4_1600_timing_table[REGTBL_NUM] = {
> 
> These two are constant data, although you'll need to annotate the
> users as well.
> 
> 
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -32,8 +32,6 @@
> >  #include <drm/drm_fb_helper.h>
> >  #include <drm/drm_crtc_helper.h>
> > 
> > -#include "ast_dram_tables.h"
> > -
> 
> Unrelated change ?
> 
> 
> > -               DRM_INFO("dram %d %d %d %08x\n", ast->mclk, ast-
> > >dram_type, ast->dram_bus_width, ast->vram_size);
> > +               DRM_INFO("dram MCLK=%u type=%d bus_width=%d
> > size=%08x\n",
> > +                        ast->mclk, ast->dram_type, ast-
> > >dram_bus_width, ast->vram_size);
> 
> Unrelated change ?
> 
> 
> >  static void ast_set_ext_reg(struct drm_crtc *crtc, struct
> > drm_display_mode *mode,
> > @@ -421,7 +432,7 @@ static void ast_set_ext_reg(struct drm_crtc
> > *crtc, struct drm_display_mode *mode
> >         ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xa8, 0xfd,
> > jregA8);
> > 
> >         /* Set Threshold */
> > -       if (ast->chip == AST2300 || ast->chip == AST2400) {
> > +       if (ast->chip == AST2300 || ast->chip == AST2400 || ast-
> > >chip == AST2500) {
> >                 ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7,
> > 0x78);
> >                 ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6,
> > 0x60);
> >         } else if (ast->chip == AST2100 ||
> > @@ -794,7 +805,7 @@ static int ast_mode_valid(struct drm_connector
> > *connector,
> >                 if ((mode->hdisplay == 1600) && (mode->vdisplay ==
> > 900))
> >                         return MODE_OK;
> > 
> > -               if ((ast->chip == AST2100) || (ast->chip ==
> > AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) ||
> > (ast->chip == AST1180)) {
> > +               if ((ast->chip == AST2100) || (ast->chip ==
> > AST2200) || (ast->chip == AST2300) || (ast->chip == AST2400) ||
> > (ast->chip == AST2500) || (ast->chip == AST1180)) {
> 
> 178 columns is getting a bit too much.
> 
> 
> > -static int mmc_test_burst(struct ast_private *ast, u32 datagen)
> > +static int mmc_test(struct ast_private *ast, u32 datagen, u8
> > test_ctl)
> >  {
> >         u32 data, timeout;
> > 
> >         ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x000000c1 | (datagen << 3));
> > +       ast_moutdwm(ast, 0x1e6e0070, (datagen << 3) | test_ctl);
> >         timeout = 0;
> >         do {
> >                 data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
> >                 if (data & 0x2000) {
> > -                       return 0;
> > +                       return -1;
> >                 }
> >                 if (++timeout > TIMEOUT) {
> >                         ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> > -                       return 0;
> > +                       return -1;
> >                 }
> >         } while (!data);
> > +       data = ast_mindwm(ast, 0x1e6e0078);
> > +       data = (data | (data >> 16)) & 0xffff;
> >         ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> > -       return 1;
> > +       return data;
> >  }
> > 
> > -static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
> > +
> > +static int mmc_test_burst(struct ast_private *ast, u32 datagen)
> >  {
> > -       u32 data, timeout;
> > +       return mmc_test(ast, datagen, 0xc1);
> > +}
> > 
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x00000041 | (datagen << 3));
> > -       timeout = 0;
> > -       do {
> > -               data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
> > -               if (++timeout > TIMEOUT) {
> > -                       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> > -                       return -1;
> > -               }
> > -       } while (!data);
> > -       data = ast_mindwm(ast, 0x1e6e0078);
> > -       data = (data | (data >> 16)) & 0xffff;
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> > -       return data;
> > +static int mmc_test_burst2(struct ast_private *ast, u32 datagen)
> > +{
> > +       return mmc_test(ast, datagen, 0x41);
> >  }
> > 
> >  static int mmc_test_single(struct ast_private *ast, u32 datagen)
> >  {
> > -       u32 data, timeout;
> > -
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x000000c5 | (datagen << 3));
> > -       timeout = 0;
> > -       do {
> > -               data = ast_mindwm(ast, 0x1e6e0070) & 0x3000;
> > -               if (data & 0x2000)
> > -                       return 0;
> > -               if (++timeout > TIMEOUT) {
> > -                       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> > -                       return 0;
> > -               }
> > -       } while (!data);
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> > -       return 1;
> > +       return mmc_test(ast, datagen, 0xc5);
> >  }
> > 
> >  static int mmc_test_single2(struct ast_private *ast, u32 datagen)
> >  {
> > -       u32 data, timeout;
> > +       return mmc_test(ast, datagen, 0x05);
> > +}
> > 
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x00000000);
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x00000005 | (datagen << 3));
> > -       timeout = 0;
> > -       do {
> > -               data = ast_mindwm(ast, 0x1e6e0070) & 0x1000;
> > -               if (++timeout > TIMEOUT) {
> > -                       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> > -                       return -1;
> > -               }
> > -       } while (!data);
> > -       data = ast_mindwm(ast, 0x1e6e0078);
> > -       data = (data | (data >> 16)) & 0xffff;
> > -       ast_moutdwm(ast, 0x1e6e0070, 0x0);
> > -       return data;
> > +static int mmc_test_single_2500(struct ast_private *ast, u32
> > datagen)
> > +{
> > +       return mmc_test(ast, datagen, 0x85);
> >  }
> > 
> >  static int cbr_test(struct ast_private *ast)
> > @@ -603,9 +574,9 @@ static u32 cbr_scan2(struct ast_private *ast)
> > 
> >  static u32 cbr_test3(struct ast_private *ast)
> >  {
> > -       if (!mmc_test_burst(ast, 0))
> > +       if (mmc_test_burst(ast, 0) < 0)
> >                 return 0;
> > -       if (!mmc_test_single(ast, 0))
> > +       if (mmc_test_single(ast, 0) < 0)
> 
> The above seems like a _lot_ of unrelated rework. Keep the
> refactoring
> separate ?
> New code seems to assume that only(?) -1 is returned on error, yet we
> do < 0.
> This will come to bite you/others as the tests are extended/reworked.
> 
> >                 return 0;
> >         return 1;
> >  }
> > @@ -1609,7 +1580,7 @@ static void ddr2_init(struct ast_private
> > *ast, struct ast2300_dram_param *param)
> > 
> >  }
> > 
> > -static void ast_init_dram_2300(struct drm_device *dev)
> > +static void ast_post_chip_2300(struct drm_device *dev)
> 
> Unrelated rename ?
> 
> 
> > +static bool ddr_test_2500(struct ast_private *ast)
> > +{
> > +       ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
> > +       ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
> > +       if (mmc_test_burst(ast, 0) < 0)
> > +               return false;
> > +       if (mmc_test_burst(ast, 1) < 0)
> > +               return false;
> > +       if (mmc_test_burst(ast, 2) < 0)
> > +               return false;
> > +       if (mmc_test_burst(ast, 3) < 0)
> > +               return false;
> > +       if (mmc_test_single_2500(ast, 0) < 0)
> > +               return false;
> > +       return false;
> 
> Final return should be true... either things got funny with v2 or the
> this patch was never tested ?
> 
> 
> > +static void ddr_phy_init_2500(struct ast_private *ast)
> > +{
> > +       u32 data, pass, timecnt;
> > +
> > +       pass = 0;
> > +       ast_moutdwm(ast, 0x1E6E0060,0x00000005);
> > +       while (!pass) {
> > +               for(timecnt = 0;timecnt < TIMEOUT;timecnt++) {
> > +                       data = ast_mindwm(ast, 0x1E6E0060) & 0x1;
> > +                       if (!data)
> > +                               break;
> > +               }
> > +               if (timecnt != TIMEOUT) {
> > +                       data = ast_mindwm(ast, 0x1E6E0300) &
> > 0x000A0000;
> > +                       if (!data)
> > +                               pass = 1;
> > +               }
> > +               if (!pass) {
> > +                       ast_moutdwm(ast, 0x1E6E0060,0x00000000);
> > +                       udelay(10); /* delay 10 us */
> > +                       ast_moutdwm(ast, 0x1E6E0060,0x00000005);
> > +               }
> > +       }
> > +
> > +       ast_moutdwm(ast, 0x1E6E0060,0x00000006);
> 
> I realise that there's likely no documentation, yet repeating the
> same
> magic numbers multiple times is a bit meh.
> 
> 
> > +}
> > +
> > +/*****************************************************************
> > *************
> > + Check DRAM Size
> > + 1Gb : 0x80000000 ~ 0x87FFFFFF
> > + 2Gb : 0x80000000 ~ 0x8FFFFFFF
> > + 4Gb : 0x80000000 ~ 0x9FFFFFFF
> > + 8Gb : 0x80000000 ~ 0xBFFFFFFF
> > +******************************************************************
> > ***********/
> > +static void check_dram_size_2500(struct ast_private *ast, u32
> > tRFC)
> > +{
> > +       u32 reg_04, reg_14;
> > +
> > +       reg_04 = ast_mindwm(ast, 0x1E6E0004) & 0xfffffffc;
> > +       reg_14 = ast_mindwm(ast, 0x1E6E0014) & 0xffffff00;
> > +
> > +       ast_moutdwm(ast, 0xA0100000, 0x41424344);
> > +       ast_moutdwm(ast, 0x90100000, 0x35363738);
> > +       ast_moutdwm(ast, 0x88100000, 0x292A2B2C);
> > +       ast_moutdwm(ast, 0x80100000, 0x1D1E1F10);
> > +
> > +       /* Check 8Gbit */
> > +       if (ast_mindwm(ast, 0xA0100000) == 0x41424344) {
> > +               reg_04 |= 0x03;
> > +               reg_14 |= (tRFC >> 24) & 0xFF;
> > +               /* Check 4Gbit */
> > +       } else if (ast_mindwm(ast, 0x90100000) == 0x35363738) {
> > +               reg_04 |= 0x02;
> > +               reg_14 |= (tRFC >> 16) & 0xFF;
> > +               /* Check 2Gbit */
> > +       } else if (ast_mindwm(ast, 0x88100000) == 0x292A2B2C) {
> > +               reg_04 |= 0x01;
> > +               reg_14 |= (tRFC >> 8) & 0xFF;
> > +       } else {
> > +               reg_14 |= tRFC & 0xFF;
> > +       }
> 
> Like in the case above...
> 
> 
> > +static void reset_mmc_2500(struct ast_private *ast)
> > +{
> > +       ast_moutdwm(ast, 0x1E78505C,0x00000004);
> > +       ast_moutdwm(ast, 0x1E785044,0x00000001);
> > +       ast_moutdwm(ast, 0x1E785048,0x00004755);
> > +       ast_moutdwm(ast, 0x1E78504C,0x00000013);
> > +       mdelay(100);                                    /* delay
> > 100ms */
> 
> "Water is wet" type of comment. Worth mentioning why it's so large -
> mentioned in the documentation, experimental result, other ?
> Same suggestion goes for the other mdelay(100) instances.
> 
> 
> > +static bool ast_dram_init_2500(struct ast_private *ast)
> > +{
> > +       u32 data;
> > +       u32 max_tries = 5;
> > +
> > +       do {
> > +               if (max_tries-- == 0)
> > +                       return false;
> > +               set_mpll_2500(ast);
> > +               reset_mmc_2500(ast);
> > +               ddr_init_common_2500(ast);
> > +
> > +               data = ast_mindwm(ast, 0x1E6E2070);
> > +               if (data & 0x01000000)
> > +                       ddr4_init_2500(ast,
> > ast2500_ddr4_1600_timing_table);
> > +               else
> > +                       ddr3_init_2500(ast,
> > ast2500_ddr3_1600_timing_table);
> > +       } while (!ddr_test_2500(ast));
> > +
> > +       ast_moutdwm(ast, 0x1E6E2040, ast_mindwm(ast, 0x1E6E2040) |
> > 0x41);
> > +
> > +       /* Patch code */
> > +       data = ast_mindwm(ast, 0x1E6E200C) & 0xF9FFFFFF;
> > +       ast_moutdwm(ast, 0x1E6E200C, data | 0x10000000);
> > +
> > +       return true;
> 
> Drop the return type - function always returns true ?
> I think there were a few other functions that could do the same.
> 
> Regards,
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16 17:33 ` Emil Velikov
  2017-02-16 21:09   ` Benjamin Herrenschmidt
@ 2017-02-16 21:12   ` Benjamin Herrenschmidt
  2017-02-16 21:14   ` Benjamin Herrenschmidt
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-16 21:12 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Thu, 2017-02-16 at 17:33 +0000, Emil Velikov wrote:
> The above seems like a _lot_ of unrelated rework. Keep the
> refactoring
> separate ?
> New code seems to assume that only(?) -1 is returned on error, yet we
> do < 0.
> This will come to bite you/others as the tests are extended/reworked.

Not sure what you mean.

I made the error return consistent yes, using -1.

< 0 is a reasonable way to test this (and probably marginally more
efficient than comparing against -1) , the "good" values are always
in the range 0..ffff .

I did the refactoring in that patch because we have 4 functions doing
roughly the same thing and the 2500 patch was adding 2 more.

I could do a first patch just changing the existing code I suppose...

Ben.

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16 17:33 ` Emil Velikov
  2017-02-16 21:09   ` Benjamin Herrenschmidt
  2017-02-16 21:12   ` Benjamin Herrenschmidt
@ 2017-02-16 21:14   ` Benjamin Herrenschmidt
  2017-02-16 21:16   ` Benjamin Herrenschmidt
  2017-02-16 21:17   ` Benjamin Herrenschmidt
  4 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-16 21:14 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Thu, 2017-02-16 at 17:33 +0000, Emil Velikov wrote:
> I realise that there's likely no documentation, yet repeating the
> same
> magic numbers multiple times is a bit meh.

This is all Aspeed original code. I merely fixed the coding style ;-)

The other POST functions for the other chip gens are the same in that
regard.

That said I do have the doc, I could create constants for everything
but I really don't have that much time and we are in an emergency toget that into distros...

What I might do is do a separate patch later that replaces a bunch
of the hard coded values with properly defined constants.

Cheers,
Ben.


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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16 17:33 ` Emil Velikov
                     ` (2 preceding siblings ...)
  2017-02-16 21:14   ` Benjamin Herrenschmidt
@ 2017-02-16 21:16   ` Benjamin Herrenschmidt
  2017-02-16 21:17   ` Benjamin Herrenschmidt
  4 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-16 21:16 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Thu, 2017-02-16 at 17:33 +0000, Emil Velikov wrote:
> "Water is wet" type of comment. Worth mentioning why it's so large -
> mentioned in the documentation, experimental result, other ?
> Same suggestion goes for the other mdelay(100) instances.

Ah I removed most of those useless comments, I must have missed
that one. As for why 100ms, that's aspeed code. The init sequence
per-se isn't documented well, so I assume they know what they are doing
:-)

Keep in mind that this code is almost never used. It's only used
if the BMC is running no code at all, to "remotely" initialize its
memory controller.

Cheers,
Ben.

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16 17:33 ` Emil Velikov
                     ` (3 preceding siblings ...)
  2017-02-16 21:16   ` Benjamin Herrenschmidt
@ 2017-02-16 21:17   ` Benjamin Herrenschmidt
  4 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-16 21:17 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Thu, 2017-02-16 at 17:33 +0000, Emil Velikov wrote:
> > +static bool ast_dram_init_2500(struct ast_private *ast)
> > +{
> > +       u32 data;
> > +       u32 max_tries = 5;
> > +
> > +       do {
> > +               if (max_tries-- == 0)
> > +                       return false;
> > +               set_mpll_2500(ast);
> > +               reset_mmc_2500(ast);
> > +               ddr_init_common_2500(ast);
> > +
> > +               data = ast_mindwm(ast, 0x1E6E2070);
> > +               if (data & 0x01000000)
> > +                       ddr4_init_2500(ast, ast2500_ddr4_1600_timing_table);
> > +               else
> > +                       ddr3_init_2500(ast, ast2500_ddr3_1600_timing_table);
> > +       } while (!ddr_test_2500(ast));
> > +
> > +       ast_moutdwm(ast, 0x1E6E2040, ast_mindwm(ast, 0x1E6E2040) | 0x41);
> > +
> > +       /* Patch code */
> > +       data = ast_mindwm(ast, 0x1E6E200C) & 0xF9FFFFFF;
> > +       ast_moutdwm(ast, 0x1E6E200C, data | 0x10000000);
> > +
> > +       return true;
> 
> Drop the return type - function always returns true ?
> I think there were a few other functions that could do the same.

It's not. I added a timeout with a return false ;-)

Cheers,
Ben.

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-16 21:09   ` Benjamin Herrenschmidt
@ 2017-02-17 21:27     ` Emil Velikov
  2017-02-17 21:36       ` Benjamin Herrenschmidt
  2017-02-18  3:41       ` Benjamin Herrenschmidt
  0 siblings, 2 replies; 15+ messages in thread
From: Emil Velikov @ 2017-02-17 21:27 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

Hi Ben,

On 16 February 2017 at 21:09, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2017-02-16 at 17:33 +0000, Emil Velikov wrote:
>> On 16 February 2017 at 09:09, Benjamin Herrenschmidt
>> <benh@kernel.crashing.org> wrote:
>> > From: "Y.C. Chen" <yc_chen@aspeedtech.com>
>> >
>> > Add detection and POST code for AST2500 generation chip,
>> > code originally from Aspeed and slightly reworked for
>> > coding style mostly by Ben.
>> >
>> > Signed-off-by: Y.C. Chen <yc_chen@aspeedtech.com>
>> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> > ---
>> > v2. [BenH]
>> >   - Coding style fixes
>> >   - Add timeout to main DRAM init loop
>> >   - Improve boot time display of RAM info
>> >
>> > Y.C. Please check that I didn't break POST as I haven't manage to
>> > test
>> > that (we can't boot our machines if the BMC isn't already POSTed).
>> >
>>
>> Hi Ben,
>>
>> Barring the checkpatch.pl warnings/errors that you've spotted there's
>> a few other comments below.
>> I'm not familiar with the hardware, so it's just things that strike
>> me
>> as odd ;-)
>
> Heh ok. I don't want to change that POST code too much as I'm not
> equipped to test it, but I'll have a look later today.
>
Not sure why you opted for splitting each suggestion in separate
email, but it seems to have lead to a [serious] bugfix to go
unnoticed.
Namely:

>> > +static bool ddr_test_2500(struct ast_private *ast)
>> > +{
>> > +       ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
>> > +       ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
>> > +       if (mmc_test_burst(ast, 0) < 0)
>> > +               return false;
>> > +       if (mmc_test_burst(ast, 1) < 0)
>> > +               return false;
>> > +       if (mmc_test_burst(ast, 2) < 0)
>> > +               return false;
>> > +       if (mmc_test_burst(ast, 3) < 0)
>> > +               return false;
>> > +       if (mmc_test_single_2500(ast, 0) < 0)
>> > +               return false;
>> > +       return false;
>>
>> Final return should be true... either things got funny with v2 or the
>> this patch was never tested ?
>>
This here.

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-17 21:27     ` Emil Velikov
@ 2017-02-17 21:36       ` Benjamin Herrenschmidt
  2017-02-18 14:55         ` Emil Velikov
  2017-02-18  3:41       ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-17 21:36 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Fri, 2017-02-17 at 21:27 +0000, Emil Velikov wrote:
> > Heh ok. I don't want to change that POST code too much as I'm not
> > equipped to test it, but I'll have a look later today.
> > 
> 
> Not sure why you opted for splitting each suggestion in separate
> email, but it seems to have lead to a [serious] bugfix to go
> unnoticed.
> Namely:

Dunno why either. I think I was distracted doing too many things at
once.

> > > > +static bool ddr_test_2500(struct ast_private *ast)
> > > > +{
> > > > +       ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
> > > > +       ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
> > > > +       if (mmc_test_burst(ast, 0) < 0)
> > > > +               return false;
> > > > +       if (mmc_test_burst(ast, 1) < 0)
> > > > +               return false;
> > > > +       if (mmc_test_burst(ast, 2) < 0)
> > > > +               return false;
> > > > +       if (mmc_test_burst(ast, 3) < 0)
> > > > +               return false;
> > > > +       if (mmc_test_single_2500(ast, 0) < 0)
> > > > +               return false;
> > > > +       return false;
> > > 
> > > Final return should be true... either things got funny with v2 or
> > > the
> > > this patch was never tested ?

As I said, never tested, I don't have the means, I'm waiting for Aspeed
to test it, hopefully monday. I can test the basic function but not
POST. I'll send a respin anyway.

Note that the POST patch is purposefully at the end of the series, it
can wait. The reason is that that code is only useful if the BMC has
no code running on it, not even u-boot, and thus its memory controller
needs to be remotely initialized by the host.

Most servers out there have something running on the BMC and all my
POWER9 systems won't boot without something on the BMC making them do
so :-)

So the POST patch can be merged later once it has had more massaging
and testing.

Cheers,
Ben.
> 
> This here.
> 
> Regards,
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-17 21:27     ` Emil Velikov
  2017-02-17 21:36       ` Benjamin Herrenschmidt
@ 2017-02-18  3:41       ` Benjamin Herrenschmidt
  2017-02-18 15:43         ` Emil Velikov
  1 sibling, 1 reply; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-18  3:41 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Fri, 2017-02-17 at 21:27 +0000, Emil Velikov wrote:
> Hi Ben,

 .../...

> Not sure why you opted for splitting each suggestion in separate
> email, but it seems to have lead to a [serious] bugfix to go
> unnoticed.

Many thanks for your reviews. I've put a tentative new series there

https://github.com/ozbenh/linux-ast

I'm waiting for Aspeed to test on Monday on their HW (they can test
the POST code) and I'll be testing again on POWER8 and POWER9 this
week-end. If all goes well I'll send the new series to the list then.

I've changed my refactoring of mmc_test_* to be closer to the original
code as I had missed a difference in loop exit condition.

Cheers,
Ben.

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-17 21:36       ` Benjamin Herrenschmidt
@ 2017-02-18 14:55         ` Emil Velikov
  0 siblings, 0 replies; 15+ messages in thread
From: Emil Velikov @ 2017-02-18 14:55 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On 17 February 2017 at 21:36, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2017-02-17 at 21:27 +0000, Emil Velikov wrote:
>> > Heh ok. I don't want to change that POST code too much as I'm not
>> > equipped to test it, but I'll have a look later today.
>> >
>>
>> Not sure why you opted for splitting each suggestion in separate
>> email, but it seems to have lead to a [serious] bugfix to go
>> unnoticed.
>> Namely:
>
> Dunno why either. I think I was distracted doing too many things at
> once.
>
>> > > > +static bool ddr_test_2500(struct ast_private *ast)
>> > > > +{
>> > > > +       ast_moutdwm(ast, 0x1E6E0074, 0x0000FFFF);
>> > > > +       ast_moutdwm(ast, 0x1E6E007C, 0xFF00FF00);
>> > > > +       if (mmc_test_burst(ast, 0) < 0)
>> > > > +               return false;
>> > > > +       if (mmc_test_burst(ast, 1) < 0)
>> > > > +               return false;
>> > > > +       if (mmc_test_burst(ast, 2) < 0)
>> > > > +               return false;
>> > > > +       if (mmc_test_burst(ast, 3) < 0)
>> > > > +               return false;
>> > > > +       if (mmc_test_single_2500(ast, 0) < 0)
>> > > > +               return false;
>> > > > +       return false;
>> > >
>> > > Final return should be true... either things got funny with v2 or
>> > > the
>> > > this patch was never tested ?
>
> As I said, never tested, I don't have the means, I'm waiting for Aspeed
> to test it, hopefully monday. I can test the basic function but not
> POST. I'll send a respin anyway.
>
> Note that the POST patch is purposefully at the end of the series, it
> can wait. The reason is that that code is only useful if the BMC has
> no code running on it, not even u-boot, and thus its memory controller
> needs to be remotely initialized by the host.
>
> Most servers out there have something running on the BMC and all my
> POWER9 systems won't boot without something on the BMC making them do
> so :-)
>
> So the POST patch can be merged later once it has had more massaging
> and testing.
>
Heh, I tried to be subtle here and not point fingers, but seemingly
that came out confusing ;-)

Afaict the code is 'broken' since it was sent to the ML by Y.C. Chen,
thus they likely [seemingly] haven't tested the POST either.
It's not my call how (or which parts) it should go it. Merely pointing
out what seems like bugs/issues.

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-18  3:41       ` Benjamin Herrenschmidt
@ 2017-02-18 15:43         ` Emil Velikov
  2017-02-18 22:22           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Emil Velikov @ 2017-02-18 15:43 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On 18 February 2017 at 03:41, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2017-02-17 at 21:27 +0000, Emil Velikov wrote:
>> Hi Ben,
>
>  .../...
>
>> Not sure why you opted for splitting each suggestion in separate
>> email, but it seems to have lead to a [serious] bugfix to go
>> unnoticed.
>
> Many thanks for your reviews. I've put a tentative new series there
>
> https://github.com/ozbenh/linux-ast
>
> I'm waiting for Aspeed to test on Monday on their HW (they can test
> the POST code) and I'll be testing again on POWER8 and POWER9 this
> week-end. If all goes well I'll send the new series to the list then.
>
Out of curiosity - can you try testing with and w/o the ddr_test_2500
bug fix(?).
Would be interesting to see if any of the omitted code [in
ast_dram_init_2500] has noticeable effect.

> I've changed my refactoring of mmc_test_* to be closer to the original
> code as I had missed a difference in loop exit condition.
>
The series looks a lot, better. Thank you for addressing my suggestions !

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

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-18 15:43         ` Emil Velikov
@ 2017-02-18 22:22           ` Benjamin Herrenschmidt
  2017-02-20 15:21             ` Emil Velikov
  0 siblings, 1 reply; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-18 22:22 UTC (permalink / raw)
  To: Emil Velikov; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On Sat, 2017-02-18 at 15:43 +0000, Emil Velikov wrote:
> 
> Out of curiosity - can you try testing with and w/o the ddr_test_2500
> bug fix(?).
> Would be interesting to see if any of the omitted code [in
> ast_dram_init_2500] has noticeable effect.

Afaik, the original ddr_test_2500 from aspeed wasn't buggy, was it ?

I thought I introduced the bug you found when I turned the values
into bools.

Anyway, I can't test the POST code so... I'll have to wait for Y.C.Chen
to do that.

> > I've changed my refactoring of mmc_test_* to be closer to the
> > original
> > code as I had missed a difference in loop exit condition.
> > 
> 
> The series looks a lot, better. Thank you for addressing my
> suggestions !
> 
> Regards
> Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/2] drm/ast: Support AST2500
  2017-02-18 22:22           ` Benjamin Herrenschmidt
@ 2017-02-20 15:21             ` Emil Velikov
  0 siblings, 0 replies; 15+ messages in thread
From: Emil Velikov @ 2017-02-20 15:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Dave Airlie, ML dri-devel, Egbert Eich

On 18 February 2017 at 22:22, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Sat, 2017-02-18 at 15:43 +0000, Emil Velikov wrote:
>>
>> Out of curiosity - can you try testing with and w/o the ddr_test_2500
>> bug fix(?).
>> Would be interesting to see if any of the omitted code [in
>> ast_dram_init_2500] has noticeable effect.
>
> Afaik, the original ddr_test_2500 from aspeed wasn't buggy, was it ?
>
I stand corrected - yes the original code did not have the
[ddr_test_2500 always returns false] bug.
Pardon, for my earlier blunder.

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

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

end of thread, other threads:[~2017-02-20 15:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-16  9:09 [PATCH 2/2] drm/ast: Support AST2500 Benjamin Herrenschmidt
2017-02-16 11:40 ` Benjamin Herrenschmidt
2017-02-16 17:33 ` Emil Velikov
2017-02-16 21:09   ` Benjamin Herrenschmidt
2017-02-17 21:27     ` Emil Velikov
2017-02-17 21:36       ` Benjamin Herrenschmidt
2017-02-18 14:55         ` Emil Velikov
2017-02-18  3:41       ` Benjamin Herrenschmidt
2017-02-18 15:43         ` Emil Velikov
2017-02-18 22:22           ` Benjamin Herrenschmidt
2017-02-20 15:21             ` Emil Velikov
2017-02-16 21:12   ` Benjamin Herrenschmidt
2017-02-16 21:14   ` Benjamin Herrenschmidt
2017-02-16 21:16   ` Benjamin Herrenschmidt
2017-02-16 21:17   ` Benjamin Herrenschmidt

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.