Re: [PATCH 4/8] drm/ast: Initialize ASTDP in ast_post_gpu()

[Jocelyn Falempe <jfalempe@redhat.com>]

On 17/01/2025 11:29, Thomas Zimmermann wrote:
> Remove the call to ast_dp_launch() from ast_detect_tx_chip() and
> perform it unconditionally in ast_post_gpu().
> 
> Also add error handling: the detection code apparently used
> ast_dp_launch() to test for a working ASTDP, falling back to VGA on
> errors. As the VBIOS reports ASTDP, silently ignoring errors is
> questionable behavior. With the refactoring, failing to initialize
> the ASTDP will also fail probing the driver.

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.c  |  6 +++++-
>   drivers/gpu/drm/ast/ast_drv.h  |  2 +-
>   drivers/gpu/drm/ast/ast_main.c | 19 +++++++++++++------
>   drivers/gpu/drm/ast/ast_post.c | 13 ++++++++++---
>   4 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index ff3bcdd1cff2a..cddd69972e89d 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -393,11 +393,15 @@ static int ast_drm_freeze(struct drm_device *dev)
>   static int ast_drm_thaw(struct drm_device *dev)
>   {
>   	struct ast_device *ast = to_ast_device(dev);
> +	int ret;
>   
>   	ast_enable_vga(ast->ioregs);
>   	ast_open_key(ast->ioregs);
>   	ast_enable_mmio(dev->dev, ast->ioregs);
> -	ast_post_gpu(ast);
> +
> +	ret = ast_post_gpu(ast);
> +	if (ret)
> +		return ret;
>   
>   	return drm_mode_config_helper_resume(dev);
>   }
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 6b4305ac07d4f..cf9edef8fca66 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -445,7 +445,7 @@ int ast_mode_config_init(struct ast_device *ast);
>   int ast_mm_init(struct ast_device *ast);
>   
>   /* ast post */
> -void ast_post_gpu(struct ast_device *ast);
> +int ast_post_gpu(struct ast_device *ast);
>   u32 ast_mindwm(struct ast_device *ast, u32 r);
>   void ast_moutdwm(struct ast_device *ast, u32 r, u32 v);
>   void ast_patch_ahb_2500(void __iomem *regs);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 456230bef2736..474eb255b325b 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -138,10 +138,7 @@ static void ast_detect_tx_chip(struct ast_device *ast, bool need_post)
>   	} else if (IS_AST_GEN7(ast)) {
>   		if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xd1, AST_IO_VGACRD1_TX_TYPE_MASK) ==
>   		    AST_IO_VGACRD1_TX_ASTDP) {
> -			int ret = ast_dp_launch(ast);
> -
> -			if (!ret)
> -				ast->tx_chip = AST_TX_ASTDP;
> +			ast->tx_chip = AST_TX_ASTDP;
>   		}
>   	}
>   
> @@ -297,8 +294,18 @@ struct drm_device *ast_device_create(struct pci_dev *pdev,
>   		 ast->mclk, ast->dram_type, ast->dram_bus_width);
>   
>   	ast_detect_tx_chip(ast, need_post);
> -	if (need_post)
> -		ast_post_gpu(ast);
> +	switch (ast->tx_chip) {
> +	case AST_TX_ASTDP:
> +		ret = ast_post_gpu(ast);
> +		break;
> +	default:
> +		ret = 0;
> +		if (need_post)
> +			ret = ast_post_gpu(ast);
> +		break;
> +	}
> +	if (ret)
> +		return ERR_PTR(ret);
>   
>   	ret = ast_mm_init(ast);
>   	if (ret)
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 49f661760f9e5..0daa8e52a092a 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -340,13 +340,18 @@ static void ast_init_dram_reg(struct ast_device *ast)
>   	} while ((j & 0x40) == 0);
>   }
>   
> -void ast_post_gpu(struct ast_device *ast)
> +int ast_post_gpu(struct ast_device *ast)
>   {
> +	int ret;
> +
>   	ast_set_def_ext_reg(ast);
>   
>   	if (AST_GEN(ast) >= 7) {
> -		if (ast->tx_chip == AST_TX_ASTDP)
> -			ast_dp_launch(ast);
> +		if (ast->tx_chip == AST_TX_ASTDP) {
> +			ret = ast_dp_launch(ast);
> +			if (ret)
> +				return ret;
> +		}
>   	} else if (AST_GEN(ast) >= 6) {
>   		if (ast->config_mode == ast_use_p2a) {
>   			ast_post_chip_2500(ast);
> @@ -376,6 +381,8 @@ void ast_post_gpu(struct ast_device *ast)
>   			}
>   		}
>   	}
> +
> +	return 0;
>   }
>   
>   /* AST 2300 DRAM settings */