[PATCH v2 0/6] ast: Replace DRAM info code

[PATCH v2 0/6] ast: Replace DRAM info code

[Thomas Zimmermann]

The ast driver's ast_get_dram_info() helper is supposed to return
information about the hardware's memory setup. It depends on the
various types of configurations and the chipset model. The problem
is that most of this is not needed or outright wrong.

Really needed is only the bank layout of the memory for posting Gen2
and Gen3 systems with P2A configuration.

This series removes ast_gem_dram_info(). The code for retrieving the
bank layout goes into the Gen2 source code and runs when required.

Tested on AST2100 and AST2600 hardware.

Thomas Zimmermann (6):
  drm/ast: Do not print DRAM info
  drm/ast: Remove unused dram_bus_width field
  drm/ast: Remove unused mclk field
  drm/ast: Remove unused SCU-MPLL and SCU-STRAP values
  drm/ast: Move DRAM info next to its only user
  drm/ast: Put AST_DRAM_ constants into enum ast_dram_layout

 drivers/gpu/drm/ast/ast_2100.c |  46 +++++++++++-
 drivers/gpu/drm/ast/ast_drv.h  |  20 +++---
 drivers/gpu/drm/ast/ast_main.c | 126 ---------------------------------
 3 files changed, 52 insertions(+), 140 deletions(-)

-- 
2.50.1

[PATCH v2 1/6] drm/ast: Do not print DRAM info

[Thomas Zimmermann]

Most of the information in the DRAM status output is irrelevant; some
is even wrong. Only the DRAM type is used on some older models. Drop
the output entirely.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 44b9b5f659fc..830f1c5fe893 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -355,8 +355,6 @@ struct drm_device *ast_device_create(struct pci_dev *pdev,
 	ret = ast_get_dram_info(ast);
 	if (ret)
 		return ERR_PTR(ret);
-	drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d\n",
-		 ast->mclk, ast->dram_type, ast->dram_bus_width);
 
 	ast_detect_tx_chip(ast, need_post);
 	switch (ast->tx_chip) {
-- 
2.50.1

Re: [PATCH v2 1/6] drm/ast: Do not print DRAM info

[Jocelyn Falempe]

On 26/08/2025 08:49, Thomas Zimmermann wrote:
> Most of the information in the DRAM status output is irrelevant; some
> is even wrong. Only the DRAM type is used on some older models. Drop
> the output entirely.
> 

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_main.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 44b9b5f659fc..830f1c5fe893 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -355,8 +355,6 @@ struct drm_device *ast_device_create(struct pci_dev *pdev,
>   	ret = ast_get_dram_info(ast);
>   	if (ret)
>   		return ERR_PTR(ret);
> -	drm_info(dev, "dram MCLK=%u Mhz type=%d bus_width=%d\n",
> -		 ast->mclk, ast->dram_type, ast->dram_bus_width);
>   
>   	ast_detect_tx_chip(ast, need_post);
>   	switch (ast->tx_chip) {

[PATCH v2 2/6] drm/ast: Remove unused dram_bus_width field

[Thomas Zimmermann]

The DRAM bus width is not necessary for the driver. Remove it.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  | 1 -
 drivers/gpu/drm/ast/ast_main.c | 6 ------
 2 files changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e37a55295ed7..403b86f00a54 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -172,7 +172,6 @@ struct ast_device {
 	enum ast_config_mode config_mode;
 	enum ast_chip chip;
 
-	uint32_t dram_bus_width;
 	uint32_t dram_type;
 	uint32_t mclk;
 
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 830f1c5fe893..0b3fb6856db7 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -242,7 +242,6 @@ static int ast_get_dram_info(struct ast_device *ast)
 		break;
 	case ast_use_defaults:
 	default:
-		ast->dram_bus_width = 16;
 		ast->dram_type = AST_DRAM_1Gx16;
 		if (IS_AST_GEN6(ast))
 			ast->mclk = 800;
@@ -251,11 +250,6 @@ static int ast_get_dram_info(struct ast_device *ast)
 		return 0;
 	}
 
-	if (mcr_cfg & 0x40)
-		ast->dram_bus_width = 16;
-	else
-		ast->dram_bus_width = 32;
-
 	if (IS_AST_GEN6(ast)) {
 		switch (mcr_cfg & 0x03) {
 		case 0:
-- 
2.50.1

Re: [PATCH v2 2/6] drm/ast: Remove unused dram_bus_width field

[Jocelyn Falempe]

On 26/08/2025 08:49, Thomas Zimmermann wrote:
> The DRAM bus width is not necessary for the driver. Remove it.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  | 1 -
>   drivers/gpu/drm/ast/ast_main.c | 6 ------
>   2 files changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index e37a55295ed7..403b86f00a54 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -172,7 +172,6 @@ struct ast_device {
>   	enum ast_config_mode config_mode;
>   	enum ast_chip chip;
>   
> -	uint32_t dram_bus_width;
>   	uint32_t dram_type;
>   	uint32_t mclk;
>   
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 830f1c5fe893..0b3fb6856db7 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -242,7 +242,6 @@ static int ast_get_dram_info(struct ast_device *ast)
>   		break;
>   	case ast_use_defaults:
>   	default:
> -		ast->dram_bus_width = 16;
>   		ast->dram_type = AST_DRAM_1Gx16;
>   		if (IS_AST_GEN6(ast))
>   			ast->mclk = 800;
> @@ -251,11 +250,6 @@ static int ast_get_dram_info(struct ast_device *ast)
>   		return 0;
>   	}
>   
> -	if (mcr_cfg & 0x40)
> -		ast->dram_bus_width = 16;
> -	else
> -		ast->dram_bus_width = 32;
> -
>   	if (IS_AST_GEN6(ast)) {
>   		switch (mcr_cfg & 0x03) {
>   		case 0:

[PATCH v2 3/6] drm/ast: Remove unused mclk field

[Thomas Zimmermann]

The memory clock is not necessary for the driver. In default for
AST2600 is event incorrect; should be 800 MHz. Remove it.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 -
 drivers/gpu/drm/ast/ast_main.c | 26 --------------------------
 2 files changed, 27 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 403b86f00a54..c9c933b5a70d 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -173,7 +173,6 @@ struct ast_device {
 	enum ast_chip chip;
 
 	uint32_t dram_type;
-	uint32_t mclk;
 
 	void __iomem	*vram;
 	unsigned long	vram_base;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 0b3fb6856db7..492cc8461eb0 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -215,7 +215,6 @@ static int ast_get_dram_info(struct ast_device *ast)
 	struct drm_device *dev = &ast->base;
 	struct device_node *np = dev->dev->of_node;
 	uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
-	uint32_t denum, num, div, ref_pll, dsel;
 
 	switch (ast->config_mode) {
 	case ast_use_dt:
@@ -243,10 +242,6 @@ static int ast_get_dram_info(struct ast_device *ast)
 	case ast_use_defaults:
 	default:
 		ast->dram_type = AST_DRAM_1Gx16;
-		if (IS_AST_GEN6(ast))
-			ast->mclk = 800;
-		else
-			ast->mclk = 396;
 		return 0;
 	}
 
@@ -300,27 +295,6 @@ static int ast_get_dram_info(struct ast_device *ast)
 		}
 	}
 
-	if (mcr_scu_strap & 0x2000)
-		ref_pll = 14318;
-	else
-		ref_pll = 12000;
-
-	denum = mcr_scu_mpll & 0x1f;
-	num = (mcr_scu_mpll & 0x3fe0) >> 5;
-	dsel = (mcr_scu_mpll & 0xc000) >> 14;
-	switch (dsel) {
-	case 3:
-		div = 0x4;
-		break;
-	case 2:
-	case 1:
-		div = 0x2;
-		break;
-	default:
-		div = 0x1;
-		break;
-	}
-	ast->mclk = ref_pll * (num + 2) / ((denum + 2) * (div * 1000));
 	return 0;
 }
 
-- 
2.50.1

Re: [PATCH v2 3/6] drm/ast: Remove unused mclk field

[Jocelyn Falempe]

On 26/08/2025 08:49, Thomas Zimmermann wrote:
> The memory clock is not necessary for the driver. In default for
> AST2600 is event incorrect; should be 800 MHz. Remove it.
> 

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 -
>   drivers/gpu/drm/ast/ast_main.c | 26 --------------------------
>   2 files changed, 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 403b86f00a54..c9c933b5a70d 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -173,7 +173,6 @@ struct ast_device {
>   	enum ast_chip chip;
>   
>   	uint32_t dram_type;
> -	uint32_t mclk;
>   
>   	void __iomem	*vram;
>   	unsigned long	vram_base;
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 0b3fb6856db7..492cc8461eb0 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -215,7 +215,6 @@ static int ast_get_dram_info(struct ast_device *ast)
>   	struct drm_device *dev = &ast->base;
>   	struct device_node *np = dev->dev->of_node;
>   	uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
> -	uint32_t denum, num, div, ref_pll, dsel;
>   
>   	switch (ast->config_mode) {
>   	case ast_use_dt:
> @@ -243,10 +242,6 @@ static int ast_get_dram_info(struct ast_device *ast)
>   	case ast_use_defaults:
>   	default:
>   		ast->dram_type = AST_DRAM_1Gx16;
> -		if (IS_AST_GEN6(ast))
> -			ast->mclk = 800;
> -		else
> -			ast->mclk = 396;
>   		return 0;
>   	}
>   
> @@ -300,27 +295,6 @@ static int ast_get_dram_info(struct ast_device *ast)
>   		}
>   	}
>   
> -	if (mcr_scu_strap & 0x2000)
> -		ref_pll = 14318;
> -	else
> -		ref_pll = 12000;
> -
> -	denum = mcr_scu_mpll & 0x1f;
> -	num = (mcr_scu_mpll & 0x3fe0) >> 5;
> -	dsel = (mcr_scu_mpll & 0xc000) >> 14;
> -	switch (dsel) {
> -	case 3:
> -		div = 0x4;
> -		break;
> -	case 2:
> -	case 1:
> -		div = 0x2;
> -		break;
> -	default:
> -		div = 0x1;
> -		break;
> -	}
> -	ast->mclk = ref_pll * (num + 2) / ((denum + 2) * (div * 1000));
>   	return 0;
>   }
>   

[PATCH v2 4/6] drm/ast: Remove unused SCU-MPLL and SCU-STRAP values

[Thomas Zimmermann]

The ast driver used SCU-MPLL and SCU-STRAP to compute the memory
clock. Remove the now unused values.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_main.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 492cc8461eb0..41ff880cfdec 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -214,7 +214,7 @@ static int ast_get_dram_info(struct ast_device *ast)
 {
 	struct drm_device *dev = &ast->base;
 	struct device_node *np = dev->dev->of_node;
-	uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
+	uint32_t mcr_cfg;
 
 	switch (ast->config_mode) {
 	case ast_use_dt:
@@ -222,22 +222,13 @@ static int ast_get_dram_info(struct ast_device *ast)
 		 * If some properties are missing, use reasonable
 		 * defaults for GEN5
 		 */
-		if (of_property_read_u32(np, "aspeed,mcr-configuration",
-					 &mcr_cfg))
+		if (of_property_read_u32(np, "aspeed,mcr-configuration", &mcr_cfg))
 			mcr_cfg = 0x00000577;
-		if (of_property_read_u32(np, "aspeed,mcr-scu-mpll",
-					 &mcr_scu_mpll))
-			mcr_scu_mpll = 0x000050C0;
-		if (of_property_read_u32(np, "aspeed,mcr-scu-strap",
-					 &mcr_scu_strap))
-			mcr_scu_strap = 0;
 		break;
 	case ast_use_p2a:
 		ast_write32(ast, 0xf004, 0x1e6e0000);
 		ast_write32(ast, 0xf000, 0x1);
 		mcr_cfg = ast_read32(ast, 0x10004);
-		mcr_scu_mpll = ast_read32(ast, 0x10120);
-		mcr_scu_strap = ast_read32(ast, 0x10170);
 		break;
 	case ast_use_defaults:
 	default:
-- 
2.50.1

Re: [PATCH v2 4/6] drm/ast: Remove unused SCU-MPLL and SCU-STRAP values

[Jocelyn Falempe]

On 26/08/2025 08:49, Thomas Zimmermann wrote:
> The ast driver used SCU-MPLL and SCU-STRAP to compute the memory
> clock. Remove the now unused values.
> 

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>


> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_main.c | 13 ++-----------
>   1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 492cc8461eb0..41ff880cfdec 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -214,7 +214,7 @@ static int ast_get_dram_info(struct ast_device *ast)
>   {
>   	struct drm_device *dev = &ast->base;
>   	struct device_node *np = dev->dev->of_node;
> -	uint32_t mcr_cfg, mcr_scu_mpll, mcr_scu_strap;
> +	uint32_t mcr_cfg;
>   
>   	switch (ast->config_mode) {
>   	case ast_use_dt:
> @@ -222,22 +222,13 @@ static int ast_get_dram_info(struct ast_device *ast)
>   		 * If some properties are missing, use reasonable
>   		 * defaults for GEN5
>   		 */
> -		if (of_property_read_u32(np, "aspeed,mcr-configuration",
> -					 &mcr_cfg))
> +		if (of_property_read_u32(np, "aspeed,mcr-configuration", &mcr_cfg))
>   			mcr_cfg = 0x00000577;
> -		if (of_property_read_u32(np, "aspeed,mcr-scu-mpll",
> -					 &mcr_scu_mpll))
> -			mcr_scu_mpll = 0x000050C0;
> -		if (of_property_read_u32(np, "aspeed,mcr-scu-strap",
> -					 &mcr_scu_strap))
> -			mcr_scu_strap = 0;
>   		break;
>   	case ast_use_p2a:
>   		ast_write32(ast, 0xf004, 0x1e6e0000);
>   		ast_write32(ast, 0xf000, 0x1);
>   		mcr_cfg = ast_read32(ast, 0x10004);
> -		mcr_scu_mpll = ast_read32(ast, 0x10120);
> -		mcr_scu_strap = ast_read32(ast, 0x10170);
>   		break;
>   	case ast_use_defaults:
>   	default:

[PATCH v2 5/6] drm/ast: Move DRAM info next to its only user

[Thomas Zimmermann]

The only place in the ast driver that uses the DRAM type is the
P2A DRAM initialization for Gen2 and Gen3 of the chip. Condense
the code in ast_get_dram_info() to exactly this use case and move
it into the Gen's custom source file. Remove the field dram_type
from struct ast_device.

The AST_DRAM_ constants are also used in Gen4 POST helpers, but
independently from the dram_type field. No changes there.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2100.c | 47 +++++++++++++++++--
 drivers/gpu/drm/ast/ast_drv.h  |  2 -
 drivers/gpu/drm/ast/ast_main.c | 83 ----------------------------------
 3 files changed, 44 insertions(+), 88 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2100.c b/drivers/gpu/drm/ast/ast_2100.c
index 477ee15eff5d..44c33dd050eb 100644
--- a/drivers/gpu/drm/ast/ast_2100.c
+++ b/drivers/gpu/drm/ast/ast_2100.c
@@ -31,6 +31,38 @@
 #include "ast_drv.h"
 #include "ast_post.h"
 
+/*
+ * DRAM type
+ */
+
+static int ast_2100_get_dram_type_p2a(struct ast_device *ast)
+{
+	u32 mcr_cfg;
+	int dram_type;
+
+	ast_write32(ast, 0xf004, 0x1e6e0000);
+	ast_write32(ast, 0xf000, 0x1);
+	mcr_cfg = ast_read32(ast, 0x10004);
+
+	switch (mcr_cfg & 0x0c) {
+	case 0:
+	case 4:
+		dram_type = AST_DRAM_512Mx16;
+		break;
+	case 8:
+		if (mcr_cfg & 0x40)
+			dram_type = AST_DRAM_1Gx16;
+		else
+			dram_type = AST_DRAM_512Mx32;
+		break;
+	case 0xc:
+		dram_type = AST_DRAM_1Gx32;
+		break;
+	}
+
+	return dram_type;
+}
+
 /*
  * POST
  */
@@ -266,6 +298,9 @@ static void ast_post_chip_2100(struct ast_device *ast)
 	u8 j;
 	u32 data, temp, i;
 	const struct ast_dramstruct *dram_reg_info;
+	int dram_type;
+
+	dram_type = ast_2100_get_dram_type_p2a(ast);
 
 	j = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 
@@ -292,11 +327,17 @@ static void ast_post_chip_2100(struct ast_device *ast)
 				for (i = 0; i < 15; i++)
 					udelay(dram_reg_info->data);
 			} else if (AST_DRAMSTRUCT_IS(dram_reg_info, DRAM_TYPE)) {
-				data = dram_reg_info->data;
-				if (ast->dram_type == AST_DRAM_1Gx16)
+				switch (dram_type) {
+				case AST_DRAM_1Gx16:
 					data = 0x00000d89;
-				else if (ast->dram_type == AST_DRAM_1Gx32)
+					break;
+				case AST_DRAM_1Gx32:
 					data = 0x00000c8d;
+					break;
+				default:
+					data = dram_reg_info->data;
+					break;
+				};
 
 				temp = ast_read32(ast, 0x12070);
 				temp &= 0xc;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index c9c933b5a70d..4c29ae9fb511 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -172,8 +172,6 @@ struct ast_device {
 	enum ast_config_mode config_mode;
 	enum ast_chip chip;
 
-	uint32_t dram_type;
-
 	void __iomem	*vram;
 	unsigned long	vram_base;
 	unsigned long	vram_size;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 41ff880cfdec..3eea6a6cdacd 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -210,85 +210,6 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
 	drm_info(dev, "Using %s\n", info_str[ast->tx_chip]);
 }
 
-static int ast_get_dram_info(struct ast_device *ast)
-{
-	struct drm_device *dev = &ast->base;
-	struct device_node *np = dev->dev->of_node;
-	uint32_t mcr_cfg;
-
-	switch (ast->config_mode) {
-	case ast_use_dt:
-		/*
-		 * If some properties are missing, use reasonable
-		 * defaults for GEN5
-		 */
-		if (of_property_read_u32(np, "aspeed,mcr-configuration", &mcr_cfg))
-			mcr_cfg = 0x00000577;
-		break;
-	case ast_use_p2a:
-		ast_write32(ast, 0xf004, 0x1e6e0000);
-		ast_write32(ast, 0xf000, 0x1);
-		mcr_cfg = ast_read32(ast, 0x10004);
-		break;
-	case ast_use_defaults:
-	default:
-		ast->dram_type = AST_DRAM_1Gx16;
-		return 0;
-	}
-
-	if (IS_AST_GEN6(ast)) {
-		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 (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
-		switch (mcr_cfg & 0x03) {
-		case 0:
-			ast->dram_type = AST_DRAM_512Mx16;
-			break;
-		default:
-		case 1:
-			ast->dram_type = AST_DRAM_1Gx16;
-			break;
-		case 2:
-			ast->dram_type = AST_DRAM_2Gx16;
-			break;
-		case 3:
-			ast->dram_type = AST_DRAM_4Gx16;
-			break;
-		}
-	} else {
-		switch (mcr_cfg & 0x0c) {
-		case 0:
-		case 4:
-			ast->dram_type = AST_DRAM_512Mx16;
-			break;
-		case 8:
-			if (mcr_cfg & 0x40)
-				ast->dram_type = AST_DRAM_1Gx16;
-			else
-				ast->dram_type = AST_DRAM_512Mx32;
-			break;
-		case 0xc:
-			ast->dram_type = AST_DRAM_1Gx32;
-			break;
-		}
-	}
-
-	return 0;
-}
-
 struct drm_device *ast_device_create(struct pci_dev *pdev,
 				     const struct drm_driver *drv,
 				     enum ast_chip chip,
@@ -311,10 +232,6 @@ struct drm_device *ast_device_create(struct pci_dev *pdev,
 	ast->regs = regs;
 	ast->ioregs = ioregs;
 
-	ret = ast_get_dram_info(ast);
-	if (ret)
-		return ERR_PTR(ret);
-
 	ast_detect_tx_chip(ast, need_post);
 	switch (ast->tx_chip) {
 	case AST_TX_ASTDP:
-- 
2.50.1

Re: [PATCH v2 5/6] drm/ast: Move DRAM info next to its only user

[Jocelyn Falempe]

On 26/08/2025 08:49, Thomas Zimmermann wrote:
> The only place in the ast driver that uses the DRAM type is the
> P2A DRAM initialization for Gen2 and Gen3 of the chip. Condense
> the code in ast_get_dram_info() to exactly this use case and move
> it into the Gen's custom source file. Remove the field dram_type
> from struct ast_device.
> 
> The AST_DRAM_ constants are also used in Gen4 POST helpers, but
> independently from the dram_type field. No changes there.
> 

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_2100.c | 47 +++++++++++++++++--
>   drivers/gpu/drm/ast/ast_drv.h  |  2 -
>   drivers/gpu/drm/ast/ast_main.c | 83 ----------------------------------
>   3 files changed, 44 insertions(+), 88 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_2100.c b/drivers/gpu/drm/ast/ast_2100.c
> index 477ee15eff5d..44c33dd050eb 100644
> --- a/drivers/gpu/drm/ast/ast_2100.c
> +++ b/drivers/gpu/drm/ast/ast_2100.c
> @@ -31,6 +31,38 @@
>   #include "ast_drv.h"
>   #include "ast_post.h"
>   
> +/*
> + * DRAM type
> + */
> +
> +static int ast_2100_get_dram_type_p2a(struct ast_device *ast)
> +{
> +	u32 mcr_cfg;
> +	int dram_type;
> +
> +	ast_write32(ast, 0xf004, 0x1e6e0000);
> +	ast_write32(ast, 0xf000, 0x1);
> +	mcr_cfg = ast_read32(ast, 0x10004);
> +
> +	switch (mcr_cfg & 0x0c) {
> +	case 0:
> +	case 4:
> +		dram_type = AST_DRAM_512Mx16;
> +		break;
> +	case 8:
> +		if (mcr_cfg & 0x40)
> +			dram_type = AST_DRAM_1Gx16;
> +		else
> +			dram_type = AST_DRAM_512Mx32;
> +		break;
> +	case 0xc:
> +		dram_type = AST_DRAM_1Gx32;
> +		break;
> +	}
> +
> +	return dram_type;
> +}
> +
>   /*
>    * POST
>    */
> @@ -266,6 +298,9 @@ static void ast_post_chip_2100(struct ast_device *ast)
>   	u8 j;
>   	u32 data, temp, i;
>   	const struct ast_dramstruct *dram_reg_info;
> +	int dram_type;
> +
> +	dram_type = ast_2100_get_dram_type_p2a(ast);
>   
>   	j = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
>   
> @@ -292,11 +327,17 @@ static void ast_post_chip_2100(struct ast_device *ast)
>   				for (i = 0; i < 15; i++)
>   					udelay(dram_reg_info->data);
>   			} else if (AST_DRAMSTRUCT_IS(dram_reg_info, DRAM_TYPE)) {
> -				data = dram_reg_info->data;
> -				if (ast->dram_type == AST_DRAM_1Gx16)
> +				switch (dram_type) {
> +				case AST_DRAM_1Gx16:
>   					data = 0x00000d89;
> -				else if (ast->dram_type == AST_DRAM_1Gx32)
> +					break;
> +				case AST_DRAM_1Gx32:
>   					data = 0x00000c8d;
> +					break;
> +				default:
> +					data = dram_reg_info->data;
> +					break;
> +				};
>   
>   				temp = ast_read32(ast, 0x12070);
>   				temp &= 0xc;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index c9c933b5a70d..4c29ae9fb511 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -172,8 +172,6 @@ struct ast_device {
>   	enum ast_config_mode config_mode;
>   	enum ast_chip chip;
>   
> -	uint32_t dram_type;
> -
>   	void __iomem	*vram;
>   	unsigned long	vram_base;
>   	unsigned long	vram_size;
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 41ff880cfdec..3eea6a6cdacd 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -210,85 +210,6 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
>   	drm_info(dev, "Using %s\n", info_str[ast->tx_chip]);
>   }
>   
> -static int ast_get_dram_info(struct ast_device *ast)
> -{
> -	struct drm_device *dev = &ast->base;
> -	struct device_node *np = dev->dev->of_node;
> -	uint32_t mcr_cfg;
> -
> -	switch (ast->config_mode) {
> -	case ast_use_dt:
> -		/*
> -		 * If some properties are missing, use reasonable
> -		 * defaults for GEN5
> -		 */
> -		if (of_property_read_u32(np, "aspeed,mcr-configuration", &mcr_cfg))
> -			mcr_cfg = 0x00000577;
> -		break;
> -	case ast_use_p2a:
> -		ast_write32(ast, 0xf004, 0x1e6e0000);
> -		ast_write32(ast, 0xf000, 0x1);
> -		mcr_cfg = ast_read32(ast, 0x10004);
> -		break;
> -	case ast_use_defaults:
> -	default:
> -		ast->dram_type = AST_DRAM_1Gx16;
> -		return 0;
> -	}
> -
> -	if (IS_AST_GEN6(ast)) {
> -		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 (IS_AST_GEN4(ast) || IS_AST_GEN5(ast)) {
> -		switch (mcr_cfg & 0x03) {
> -		case 0:
> -			ast->dram_type = AST_DRAM_512Mx16;
> -			break;
> -		default:
> -		case 1:
> -			ast->dram_type = AST_DRAM_1Gx16;
> -			break;
> -		case 2:
> -			ast->dram_type = AST_DRAM_2Gx16;
> -			break;
> -		case 3:
> -			ast->dram_type = AST_DRAM_4Gx16;
> -			break;
> -		}
> -	} else {
> -		switch (mcr_cfg & 0x0c) {
> -		case 0:
> -		case 4:
> -			ast->dram_type = AST_DRAM_512Mx16;
> -			break;
> -		case 8:
> -			if (mcr_cfg & 0x40)
> -				ast->dram_type = AST_DRAM_1Gx16;
> -			else
> -				ast->dram_type = AST_DRAM_512Mx32;
> -			break;
> -		case 0xc:
> -			ast->dram_type = AST_DRAM_1Gx32;
> -			break;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>   struct drm_device *ast_device_create(struct pci_dev *pdev,
>   				     const struct drm_driver *drv,
>   				     enum ast_chip chip,
> @@ -311,10 +232,6 @@ struct drm_device *ast_device_create(struct pci_dev *pdev,
>   	ast->regs = regs;
>   	ast->ioregs = ioregs;
>   
> -	ret = ast_get_dram_info(ast);
> -	if (ret)
> -		return ERR_PTR(ret);
> -
>   	ast_detect_tx_chip(ast, need_post);
>   	switch (ast->tx_chip) {
>   	case AST_TX_ASTDP:

[PATCH v2 6/6] drm/ast: Put AST_DRAM_ constants into enum ast_dram_layout

[Thomas Zimmermann]

The AST_DRAM_ constants belong together, so put them in an enum
type. Rename type and variables to 'drm_layout', as there's already
another DRAM type in the ast driver (AST_DDR2, AST_DDR3).

v2:
- avoid compiler warning with switch default (Dan)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ast/ast_2100.c | 21 ++++++++++-----------
 drivers/gpu/drm/ast/ast_drv.h  | 16 +++++++++-------
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_2100.c b/drivers/gpu/drm/ast/ast_2100.c
index 44c33dd050eb..91541c8eaff7 100644
--- a/drivers/gpu/drm/ast/ast_2100.c
+++ b/drivers/gpu/drm/ast/ast_2100.c
@@ -35,10 +35,10 @@
  * DRAM type
  */
 
-static int ast_2100_get_dram_type_p2a(struct ast_device *ast)
+static enum ast_dram_layout ast_2100_get_dram_layout_p2a(struct ast_device *ast)
 {
 	u32 mcr_cfg;
-	int dram_type;
+	enum ast_dram_layout dram_layout;
 
 	ast_write32(ast, 0xf004, 0x1e6e0000);
 	ast_write32(ast, 0xf000, 0x1);
@@ -47,20 +47,21 @@ static int ast_2100_get_dram_type_p2a(struct ast_device *ast)
 	switch (mcr_cfg & 0x0c) {
 	case 0:
 	case 4:
-		dram_type = AST_DRAM_512Mx16;
+	default:
+		dram_layout = AST_DRAM_512Mx16;
 		break;
 	case 8:
 		if (mcr_cfg & 0x40)
-			dram_type = AST_DRAM_1Gx16;
+			dram_layout = AST_DRAM_1Gx16;
 		else
-			dram_type = AST_DRAM_512Mx32;
+			dram_layout = AST_DRAM_512Mx32;
 		break;
 	case 0xc:
-		dram_type = AST_DRAM_1Gx32;
+		dram_layout = AST_DRAM_1Gx32;
 		break;
 	}
 
-	return dram_type;
+	return dram_layout;
 }
 
 /*
@@ -298,9 +299,7 @@ static void ast_post_chip_2100(struct ast_device *ast)
 	u8 j;
 	u32 data, temp, i;
 	const struct ast_dramstruct *dram_reg_info;
-	int dram_type;
-
-	dram_type = ast_2100_get_dram_type_p2a(ast);
+	enum ast_dram_layout dram_layout  = ast_2100_get_dram_layout_p2a(ast);
 
 	j = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
 
@@ -327,7 +326,7 @@ static void ast_post_chip_2100(struct ast_device *ast)
 				for (i = 0; i < 15; i++)
 					udelay(dram_reg_info->data);
 			} else if (AST_DRAMSTRUCT_IS(dram_reg_info, DRAM_TYPE)) {
-				switch (dram_type) {
+				switch (dram_layout) {
 				case AST_DRAM_1Gx16:
 					data = 0x00000d89;
 					break;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 4c29ae9fb511..c15aef014f69 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -98,13 +98,15 @@ enum ast_config_mode {
 	ast_use_defaults
 };
 
-#define AST_DRAM_512Mx16 0
-#define AST_DRAM_1Gx16   1
-#define AST_DRAM_512Mx32 2
-#define AST_DRAM_1Gx32   3
-#define AST_DRAM_2Gx16   6
-#define AST_DRAM_4Gx16   7
-#define AST_DRAM_8Gx16   8
+enum ast_dram_layout {
+	AST_DRAM_512Mx16 = 0,
+	AST_DRAM_1Gx16 = 1,
+	AST_DRAM_512Mx32 = 2,
+	AST_DRAM_1Gx32 = 3,
+	AST_DRAM_2Gx16 = 6,
+	AST_DRAM_4Gx16 = 7,
+	AST_DRAM_8Gx16 = 8,
+};
 
 /*
  * Hardware cursor
-- 
2.50.1

Re: [PATCH v2 6/6] drm/ast: Put AST_DRAM_ constants into enum ast_dram_layout

[Jocelyn Falempe]

On 26/08/2025 08:49, Thomas Zimmermann wrote:
> The AST_DRAM_ constants belong together, so put them in an enum
> type. Rename type and variables to 'drm_layout', as there's already
> another DRAM type in the ast driver (AST_DDR2, AST_DDR3).
> 

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> v2:
> - avoid compiler warning with switch default (Dan)
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>   drivers/gpu/drm/ast/ast_2100.c | 21 ++++++++++-----------
>   drivers/gpu/drm/ast/ast_drv.h  | 16 +++++++++-------
>   2 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_2100.c b/drivers/gpu/drm/ast/ast_2100.c
> index 44c33dd050eb..91541c8eaff7 100644
> --- a/drivers/gpu/drm/ast/ast_2100.c
> +++ b/drivers/gpu/drm/ast/ast_2100.c
> @@ -35,10 +35,10 @@
>    * DRAM type
>    */
>   
> -static int ast_2100_get_dram_type_p2a(struct ast_device *ast)
> +static enum ast_dram_layout ast_2100_get_dram_layout_p2a(struct ast_device *ast)
>   {
>   	u32 mcr_cfg;
> -	int dram_type;
> +	enum ast_dram_layout dram_layout;
>   
>   	ast_write32(ast, 0xf004, 0x1e6e0000);
>   	ast_write32(ast, 0xf000, 0x1);
> @@ -47,20 +47,21 @@ static int ast_2100_get_dram_type_p2a(struct ast_device *ast)
>   	switch (mcr_cfg & 0x0c) {
>   	case 0:
>   	case 4:
> -		dram_type = AST_DRAM_512Mx16;
> +	default:
> +		dram_layout = AST_DRAM_512Mx16;
>   		break;
>   	case 8:
>   		if (mcr_cfg & 0x40)
> -			dram_type = AST_DRAM_1Gx16;
> +			dram_layout = AST_DRAM_1Gx16;
>   		else
> -			dram_type = AST_DRAM_512Mx32;
> +			dram_layout = AST_DRAM_512Mx32;
>   		break;
>   	case 0xc:
> -		dram_type = AST_DRAM_1Gx32;
> +		dram_layout = AST_DRAM_1Gx32;
>   		break;
>   	}
>   
> -	return dram_type;
> +	return dram_layout;
>   }
>   
>   /*
> @@ -298,9 +299,7 @@ static void ast_post_chip_2100(struct ast_device *ast)
>   	u8 j;
>   	u32 data, temp, i;
>   	const struct ast_dramstruct *dram_reg_info;
> -	int dram_type;
> -
> -	dram_type = ast_2100_get_dram_type_p2a(ast);
> +	enum ast_dram_layout dram_layout  = ast_2100_get_dram_layout_p2a(ast);
>   
>   	j = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd0, 0xff);
>   
> @@ -327,7 +326,7 @@ static void ast_post_chip_2100(struct ast_device *ast)
>   				for (i = 0; i < 15; i++)
>   					udelay(dram_reg_info->data);
>   			} else if (AST_DRAMSTRUCT_IS(dram_reg_info, DRAM_TYPE)) {
> -				switch (dram_type) {
> +				switch (dram_layout) {
>   				case AST_DRAM_1Gx16:
>   					data = 0x00000d89;
>   					break;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 4c29ae9fb511..c15aef014f69 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -98,13 +98,15 @@ enum ast_config_mode {
>   	ast_use_defaults
>   };
>   
> -#define AST_DRAM_512Mx16 0
> -#define AST_DRAM_1Gx16   1
> -#define AST_DRAM_512Mx32 2
> -#define AST_DRAM_1Gx32   3
> -#define AST_DRAM_2Gx16   6
> -#define AST_DRAM_4Gx16   7
> -#define AST_DRAM_8Gx16   8
> +enum ast_dram_layout {
> +	AST_DRAM_512Mx16 = 0,
> +	AST_DRAM_1Gx16 = 1,
> +	AST_DRAM_512Mx32 = 2,
> +	AST_DRAM_1Gx32 = 3,
> +	AST_DRAM_2Gx16 = 6,
> +	AST_DRAM_4Gx16 = 7,
> +	AST_DRAM_8Gx16 = 8,
> +};
>   
>   /*
>    * Hardware cursor