Hi On 2023/6/19 16:22, Thomas Zimmermann wrote:
Hi Am 17.06.23 um 10:35 schrieb Sui Jingfeng: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 wherepossible. The few places that require per-model handling are now clearlymarked. 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.cindex 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.hindex 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() ?Makes sense.+#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.cindex 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.cindex 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);elseast_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_modeif ((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.The history and feature set of the earlier chips appears convoluted. The chip generation is meant for the cases where all chips behave the same. I'm considering to create a patch that reorganizes these branches to be more readable. But for now, it's just about making the current code more understandable.
Yeah, the history it too heavy, there so much model need to be processed, it deserve another patch. I'm agreed with you for now.
Best regards Thomas+ 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.cindex 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
