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

[Jocelyn Falempe <jfalempe@redhat.com>]

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