Re: [09/14] drm/ast: Distinguish among chip generations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 2023/6/16 21:52, Thomas Zimmermann wrote:
ASpeed distinguishes among various generations of the AST graphics
chipset with various models. [1] The most-recent model AST 2600 is
of the 7th generation, the AST 2500 is of the 6th generation, and so
on.

The ast driver simply picks one of the models as representative for
the whole generation. In several places, individual models of the
same generation need to be handled differently, which then requires
additional code for detecting the model.

Introduce different generations of the Aspeed chipset. In the source
code, refer to the generation instead of the representation model where
possible. The few places that require per-model handling are now clearly
marked.

In the enum ast_chip, we arrange each model's value such that it
encodes the generation. This allows for an easy test. The actual values
are ordered, but not of interest to the driver.

Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
Link: https://web.archive.org/web/20141007093258/http://www.aspeedtech.com/products.php?fPath=20 # 1
---
  drivers/gpu/drm/ast/ast_dp501.c |  6 ++--
  drivers/gpu/drm/ast/ast_drv.h   | 56 +++++++++++++++++++++++++++------
  drivers/gpu/drm/ast/ast_main.c  | 22 ++++++-------
  drivers/gpu/drm/ast/ast_mode.c  | 35 ++++++++++-----------
  drivers/gpu/drm/ast/ast_post.c  | 27 +++++++---------
  5 files changed, 89 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 1bc35a992369d..a5d285850ffb1 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -350,7 +350,7 @@ static bool ast_init_dvo(struct drm_device *dev)
  		data |= 0x00000500;
  		ast_write32(ast, 0x12008, data);
- if (ast->chip == AST2300) {
+		if (IS_AST_GEN4(ast)) {
  			data = ast_read32(ast, 0x12084);
  			/* multi-pins for DVO single-edge */
  			data |= 0xfffe0000;
@@ -366,7 +366,7 @@ static bool ast_init_dvo(struct drm_device *dev)
  			data &= 0xffffffcf;
  			data |= 0x00000020;
  			ast_write32(ast, 0x12090, data);
-		} else { /* AST2400 */
+		} else { /* AST GEN5+ */
  			data = ast_read32(ast, 0x12088);
  			/* multi-pins for DVO single-edge */
  			data |= 0x30000000;
@@ -437,7 +437,7 @@ void ast_init_3rdtx(struct drm_device *dev)
  	struct ast_device *ast = to_ast_device(dev);
  	u8 jreg;
- if (ast->chip == AST2300 || ast->chip == AST2400) {
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
  		jreg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
  		switch (jreg & 0x0e) {
  		case 0x04:
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index c42dfb86e040d..c209d7e4e4194 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -55,18 +55,38 @@
  #define AST_PCI_OPTION_MEM_ACCESS_ENABLE	BIT(1)
  #define AST_PCI_OPTION_IO_ACCESS_ENABLE		BIT(0)
+#define __AST_CHIP(__gen, __index) ((__gen) << 16 | (__index))
+
  enum ast_chip {
-	AST2000,
-	AST2100,
-	AST1100,
-	AST2200,
-	AST2150,
-	AST2300,
-	AST2400,
-	AST2500,
-	AST2600,
+	/* 1st gen */
+	AST1000 = __AST_CHIP(1, 0), // unused
+	AST2000 = __AST_CHIP(1, 1),
+	/* 2nd gen */
+	AST1100 = __AST_CHIP(2, 0),
+	AST2100 = __AST_CHIP(2, 1),
+	AST2050 = __AST_CHIP(2, 2), // unused
+	/* 3rd gen */
+	AST2200 = __AST_CHIP(3, 0),
+	AST2150 = __AST_CHIP(3, 1),
+	/* 4th gen */
+	AST2300 = __AST_CHIP(4, 0),
+	AST1300 = __AST_CHIP(4, 1), // unused
+	AST1050 = __AST_CHIP(4, 2), // unused
+	/* 5th gen */
+	AST2400 = __AST_CHIP(5, 0),
+	AST1400 = __AST_CHIP(5, 1), // unused
+	AST1250 = __AST_CHIP(5, 2), // unused
+	/* 6th gen */
+	AST2500 = __AST_CHIP(6, 0),
+	AST2510 = __AST_CHIP(6, 1), // unused
+	AST2520 = __AST_CHIP(6, 2), // unused
+	/* 7th gen */
+	AST2600 = __AST_CHIP(7, 0),
+	AST2620 = __AST_CHIP(7, 1), // unused
  };
+#define __AST_CHIP_GEN(__chip) (((unsigned long)(__chip)) >> 16)
+
  enum ast_tx_chip {
  	AST_TX_NONE,
  	AST_TX_SIL164,
@@ -220,6 +240,24 @@ struct ast_device *ast_device_create(const struct drm_driver *drv,
  				     struct pci_dev *pdev,
  				     unsigned long flags);
+static inline unsigned long __ast_gen(struct ast_device *ast)
+{
+	return __AST_CHIP_GEN(ast->chip);
+}
+#define AST_GEN(__ast)	__ast_gen(__ast)
+
+static inline bool __ast_is_gen(struct ast_device *ast, unsigned long gen)
+{
+	return __ast_gen(ast) == gen;
+}

Changed to __ast_gen_is_equal() ?


+#define IS_AST_GEN1(__ast)	__ast_is_gen(__ast, 1)
+#define IS_AST_GEN2(__ast)	__ast_is_gen(__ast, 2)
+#define IS_AST_GEN3(__ast)	__ast_is_gen(__ast, 3)
+#define IS_AST_GEN4(__ast)	__ast_is_gen(__ast, 4)
+#define IS_AST_GEN5(__ast)	__ast_is_gen(__ast, 5)
+#define IS_AST_GEN6(__ast)	__ast_is_gen(__ast, 6)
+#define IS_AST_GEN7(__ast)	__ast_is_gen(__ast, 7)
+
  #define AST_IO_AR_PORT_WRITE		(0x40)
  #define AST_IO_MISC_PORT_WRITE		(0x42)
  #define AST_IO_VGA_ENABLE_PORT		(0x43)
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 6ff4b837e64d7..3cd94a74150bf 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -128,7 +128,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
  	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
  	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
  	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
-		/* Patch AST2500 */
+		/* Patch GEN6 */
  		if (((pdev->revision & 0xF0) == 0x40)
  			&& ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0))
  			ast_patch_ahb_2500(ast);
@@ -197,8 +197,8 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
  	}
/* Check if we support wide screen */
-	switch (ast->chip) {
-	case AST2000:
+	switch (AST_GEN(ast)) {
+	case 1:
  		ast->support_wide_screen = false;
  		break;
  	default:
@@ -218,7 +218,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
  			if (ast->chip == AST2500 &&
  			    scu_rev == 0x100)           /* ast2510 */
  				ast->support_wide_screen = true;
-			if (ast->chip == AST2600)		/* ast2600 */
+			if (IS_AST_GEN7(ast))
  				ast->support_wide_screen = true;
  		}
  		break;
@@ -241,9 +241,9 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
  			ast->tx_chip_types = AST_TX_SIL164_BIT;
  	}
- if ((ast->chip == AST2300) || (ast->chip == AST2400) || (ast->chip == AST2500)) {
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast)) {
  		/*
-		 * On AST2300 and 2400, look the configuration set by the SoC in
+		 * On AST GEN4+, look the configuration set by the SoC in
  		 * the SOC scratch register #1 bits 11:8 (interestingly marked
  		 * as "reserved" in the spec)
  		 */
@@ -265,7 +265,7 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev)
  		case 0x0c:
  			ast->tx_chip_types = AST_TX_DP501_BIT;
  		}
-	} else if (ast->chip == AST2600) {
+	} else if (IS_AST_GEN7(ast)) {
  		if (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, TX_TYPE_MASK) ==
  		    ASTDP_DPMCU_TX) {
  			ast->tx_chip_types = AST_TX_ASTDP_BIT;
@@ -297,7 +297,7 @@ static int ast_get_dram_info(struct drm_device *dev)
  	case ast_use_dt:
  		/*
  		 * If some properties are missing, use reasonable
-		 * defaults for AST2400
+		 * defaults for GEN5
  		 */
  		if (of_property_read_u32(np, "aspeed,mcr-configuration",
  					 &mcr_cfg))
@@ -320,7 +320,7 @@ static int ast_get_dram_info(struct drm_device *dev)
  	default:
  		ast->dram_bus_width = 16;
  		ast->dram_type = AST_DRAM_1Gx16;
-		if (ast->chip == AST2500)
+		if (IS_AST_GEN6(ast))
  			ast->mclk = 800;
  		else
  			ast->mclk = 396;
@@ -332,7 +332,7 @@ static int ast_get_dram_info(struct drm_device *dev)
  	else
  		ast->dram_bus_width = 32;
- if (ast->chip == AST2500) {
+	if (IS_AST_GEN6(ast)) {
  		switch (mcr_cfg & 0x03) {
  		case 0:
  			ast->dram_type = AST_DRAM_1Gx16;
@@ -348,7 +348,7 @@ static int ast_get_dram_info(struct drm_device *dev)
  			ast->dram_type = AST_DRAM_8Gx16;
  			break;
  		}
-	} else if (ast->chip == AST2300 || ast->chip == AST2400) {
+	} else if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
  		switch (mcr_cfg & 0x03) {
  		case 0:
  			ast->dram_type = AST_DRAM_512Mx16;
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index b3c670af6ef2b..f711d592da52b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -342,7 +342,7 @@ static void ast_set_crtc_reg(struct ast_device *ast,
  	u8 jreg05 = 0, jreg07 = 0, jreg09 = 0, jregAC = 0, jregAD = 0, jregAE = 0;
  	u16 temp, precache = 0;
- if ((ast->chip == AST2500 || ast->chip == AST2600) &&
+	if ((IS_AST_GEN6(ast) || IS_AST_GEN7(ast)) &&
  	    (vbios_mode->enh_table->flags & AST2500PreCatchCRT))
  		precache = 40;
@@ -384,7 +384,7 @@ static void ast_set_crtc_reg(struct ast_device *ast,
  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xAD, 0x00, jregAD);
// Workaround for HSync Time non octave pixels (1920x1080@60Hz HSync 44 pixels);
-	if ((ast->chip == AST2600) && (mode->crtc_vdisplay == 1080))
+	if (IS_AST_GEN7(ast) && (mode->crtc_vdisplay == 1080))
  		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xFC, 0xFD, 0x02);
  	else
  		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xFC, 0xFD, 0x00);
@@ -466,7 +466,7 @@ static void ast_set_dclk_reg(struct ast_device *ast,
  {
  	const struct ast_vbios_dclk_info *clk_info;
- if ((ast->chip == AST2500) || (ast->chip == AST2600))
+	if (IS_AST_GEN6(ast) || IS_AST_GEN7(ast))
  		clk_info = &dclk_table_ast2500[vbios_mode->enh_table->dclk_index];
  	else
  		clk_info = &dclk_table[vbios_mode->enh_table->dclk_index];
@@ -510,17 +510,13 @@ static void ast_set_color_reg(struct ast_device *ast,
  static void ast_set_crtthd_reg(struct ast_device *ast)
  {
  	/* Set Threshold */
-	if (ast->chip == AST2600) {
+	if (IS_AST_GEN7(ast)) {
  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0xe0);
  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0xa0);
-	} else if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500) {
+	} else if (IS_AST_GEN6(ast) || IS_AST_GEN5(ast) || IS_AST_GEN4(ast)) {
  		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 ||
-		   ast->chip == AST1100 ||
-		   ast->chip == AST2200 ||
-		   ast->chip == AST2150) {
+	} else if (IS_AST_GEN3(ast) || IS_AST_GEN2(ast)) {
  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa7, 0x3f);
  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa6, 0x2f);
  	} else {
@@ -1082,9 +1078,10 @@ ast_crtc_helper_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode
  		if ((mode->hdisplay == 1152) && (mode->vdisplay == 864))
  			return MODE_OK;
- if ((ast->chip == AST2100) || (ast->chip == AST2200) ||
-		    (ast->chip == AST2300) || (ast->chip == AST2400) ||
-		    (ast->chip == AST2500) || (ast->chip == AST2600)) {
+		if ((ast->chip == AST2100) || // GEN2, but not AST1100 (?)
+		    (ast->chip == AST2200) || // GEN3, but not AST2150 (?)

If chips from the same generation is not compatible, then this actually introduce confusion.

It's not your patch is bad, it is the naming and/or the hardware is bad.

On such a situation, the patch is not good enough to suppress problems incurred by the hardware versions.

It is not clean and It still a tangled implement. But I'm fine if you want to see the limitation.

+		    IS_AST_GEN4(ast) || IS_AST_GEN5(ast) ||
+		    IS_AST_GEN6(ast) || IS_AST_GEN7(ast)) {
  			if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080))
  				return MODE_OK;
@@ -1800,12 +1797,12 @@ int ast_mode_config_init(struct ast_device *ast)
  	dev->mode_config.min_height = 0;
  	dev->mode_config.preferred_depth = 24;
- if (ast->chip == AST2100 ||
-	    ast->chip == AST2200 ||
-	    ast->chip == AST2300 ||
-	    ast->chip == AST2400 ||
-	    ast->chip == AST2500 ||
-	    ast->chip == AST2600) {
+	if (ast->chip == AST2100 || // GEN2, but not AST1100 (?)
+	    ast->chip == AST2200 || // GEN3, but not AST2150 (?)
+	    IS_AST_GEN7(ast) ||
+	    IS_AST_GEN6(ast) ||
+	    IS_AST_GEN5(ast) ||
+	    IS_AST_GEN4(ast)) {
  		dev->mode_config.max_width = 1920;
  		dev->mode_config.max_height = 2048;
  	} else {
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index b765eeb55e5f1..13e15173f2c5b 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -51,7 +51,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
  	for (i = 0x81; i <= 0x9f; i++)
  		ast_set_index_reg(ast, AST_IO_CRTC_PORT, i, 0x00);
- if (ast->chip == AST2300 || ast->chip == AST2400 || ast->chip == AST2500)
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast))
  		ext_reg_info = extreginfo_ast2300;
  	else
  		ext_reg_info = extreginfo;
@@ -72,8 +72,7 @@ ast_set_def_ext_reg(struct drm_device *dev)
/* Enable RAMDAC for A1 */
  	reg = 0x04;
-	if (ast->chip == AST2300 || ast->chip == AST2400 ||
-	    ast->chip == AST2500)
+	if (IS_AST_GEN4(ast) || IS_AST_GEN5(ast) || IS_AST_GEN6(ast))
  		reg |= 0x20;
  	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xb6, 0xff, reg);
  }
@@ -249,7 +248,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
  	j = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
if ((j & 0x80) == 0) { /* VGA only */
-		if (ast->chip == AST2000) {
+		if (IS_AST_GEN1(ast)) {
  			dram_reg_info = ast2000_dram_table_data;
  			ast_write32(ast, 0xf004, 0x1e6e0000);
  			ast_write32(ast, 0xf000, 0x1);
@@ -258,7 +257,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
  			do {
  				;
  			} while (ast_read32(ast, 0x10100) != 0xa8);
-		} else {/* AST2100/1100 */
+		} else { /* GEN2/GEN3 */
  			if (ast->chip == AST2100 || ast->chip == AST2200)
  				dram_reg_info = ast2100_dram_table_data;
  			else
@@ -281,7 +280,7 @@ static void ast_init_dram_reg(struct drm_device *dev)
  			if (dram_reg_info->index == 0xff00) {/* delay fn */
  				for (i = 0; i < 15; i++)
  					udelay(dram_reg_info->data);
-			} else if (dram_reg_info->index == 0x4 && ast->chip != AST2000) {
+			} else if (dram_reg_info->index == 0x4 && !IS_AST_GEN1(ast)) {
  				data = dram_reg_info->data;
  				if (ast->dram_type == AST_DRAM_1Gx16)
  					data = 0x00000d89;
@@ -307,15 +306,13 @@ static void ast_init_dram_reg(struct drm_device *dev)
  				cbrdlli_ast2150(ast, 32); /* 32 bits */
  		}
- switch (ast->chip) {
-		case AST2000:
+		switch (AST_GEN(ast)) {
+		case 1:
  			temp = ast_read32(ast, 0x10140);
  			ast_write32(ast, 0x10140, temp | 0x40);
  			break;
-		case AST1100:
-		case AST2100:
-		case AST2200:
-		case AST2150:
+		case 2:
+		case 3:
  			temp = ast_read32(ast, 0x1200c);
  			ast_write32(ast, 0x1200c, temp & 0xfffffffd);
  			temp = ast_read32(ast, 0x12040);
@@ -338,13 +335,13 @@ void ast_post_gpu(struct drm_device *dev)
ast_set_def_ext_reg(dev); - if (ast->chip == AST2600) {
+	if (IS_AST_GEN7(ast)) {
  		if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
  			ast_dp_launch(dev);
  	} else if (ast->config_mode == ast_use_p2a) {
-		if (ast->chip == AST2500)
+		if (IS_AST_GEN6(ast))
  			ast_post_chip_2500(dev);
-		else if (ast->chip == AST2300 || ast->chip == AST2400)
+		else if (IS_AST_GEN5(ast) || IS_AST_GEN4(ast))
  			ast_post_chip_2300(dev);
  		else
  			ast_init_dram_reg(dev);

--
Jingfeng




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux