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

Sui Jingfeng suijingfeng at loongson.cn
Sat Jun 17 08:35:25 UTC 2023


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 at suse.de>
> 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 at 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



More information about the dri-devel mailing list