[PATCH 2/5] drm/ast: astdp: Test firmware status once during probing
Jocelyn Falempe
jfalempe at redhat.com
Tue Jul 30 07:40:47 UTC 2024
On 17/07/2024 16:24, Thomas Zimmermann wrote:
> Test for running ASTDP firmware during probe. Do not bother testing
> this later. We cannot do much anyway if the firmware fails. Do not
> initialize the ASTDP conenctor if the test fails during device probing.
>
Thanks, it looks good to me.
Reviewed-by: Jocelyn Falempe <jfalempe at redhat.com>
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> ---
> drivers/gpu/drm/ast/ast_dp.c | 41 +++++++++++++---------------------
> drivers/gpu/drm/ast/ast_drv.h | 2 +-
> drivers/gpu/drm/ast/ast_main.c | 6 +++--
> drivers/gpu/drm/ast/ast_post.c | 2 +-
> drivers/gpu/drm/ast/ast_reg.h | 4 ++--
> 5 files changed, 23 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index e6c7f0d64e99..59885d10d308 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -9,8 +9,6 @@
>
> bool ast_astdp_is_connected(struct ast_device *ast)
> {
> - if (!ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1, ASTDP_MCU_FW_EXECUTING))
> - return false;
> if (!ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF, ASTDP_HPD))
> return false;
> if (!ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC, ASTDP_LINK_SUCCESS))
> @@ -24,13 +22,11 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
> u8 i = 0, j = 0;
>
> /*
> - * CRD1[b5]: DP MCU FW is executing
> * CRDC[b0]: DP link success
> * CRDF[b0]: DP HPD
> * CRE5[b0]: Host reading EDID process is done
> */
> - if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1, ASTDP_MCU_FW_EXECUTING) &&
> - ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC, ASTDP_LINK_SUCCESS) &&
> + if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC, ASTDP_LINK_SUCCESS) &&
> ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF, ASTDP_HPD) &&
> ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xE5,
> ASTDP_HOST_EDID_READ_DONE_MASK))) {
> @@ -64,9 +60,7 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
> */
> mdelay(j+1);
>
> - if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1,
> - ASTDP_MCU_FW_EXECUTING) &&
> - ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC,
> + if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC,
> ASTDP_LINK_SUCCESS) &&
> ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF, ASTDP_HPD))) {
> goto err_astdp_jump_out_loop_of_edid;
> @@ -115,8 +109,6 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
> return (~(j+256) + 1);
>
> err_astdp_edid_not_ready:
> - if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1, ASTDP_MCU_FW_EXECUTING)))
> - return (~0xD1 + 1);
> if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC, ASTDP_LINK_SUCCESS)))
> return (~0xDC + 1);
> if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF, ASTDP_HPD)))
> @@ -130,32 +122,29 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
> /*
> * Launch Aspeed DP
> */
> -void ast_dp_launch(struct drm_device *dev)
> +int ast_dp_launch(struct ast_device *ast)
> {
> - u32 i = 0;
> - u8 bDPExecute = 1;
> - struct ast_device *ast = to_ast_device(dev);
> + struct drm_device *dev = &ast->base;
> + unsigned int i = 10;
>
> - // Wait one second then timeout.
> - while (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD1, ASTDP_MCU_FW_EXECUTING) !=
> - ASTDP_MCU_FW_EXECUTING) {
> - i++;
> - // wait 100 ms
> - msleep(100);
> + while (i) {
> + u8 vgacrd1 = ast_get_index_reg(ast, AST_IO_VGACRI, 0xd1);
>
> - if (i >= 10) {
> - // DP would not be ready.
> - bDPExecute = 0;
> + if (vgacrd1 & AST_IO_VGACRD1_MCU_FW_EXECUTING)
> break;
> - }
> + --i;
> + msleep(100);
> }
> -
> - if (!bDPExecute)
> + if (!i) {
> drm_err(dev, "Wait DPMCU executing timeout\n");
> + return -ENODEV;
> + }
>
> ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE5,
> (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
> ASTDP_HOST_EDID_READ_DONE);
> +
> + return 0;
> }
>
> bool ast_dp_power_is_on(struct ast_device *ast)
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 47bab5596c16..02476eb78119 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -471,7 +471,7 @@ void ast_init_3rdtx(struct drm_device *dev);
> /* aspeed DP */
> bool ast_astdp_is_connected(struct ast_device *ast);
> int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
> -void ast_dp_launch(struct drm_device *dev);
> +int ast_dp_launch(struct ast_device *ast);
> bool ast_dp_power_is_on(struct ast_device *ast);
> void ast_dp_power_on_off(struct drm_device *dev, bool no);
> void ast_dp_set_on_off(struct drm_device *dev, bool no);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 0637abb70361..d836f2a4f9f3 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -115,8 +115,10 @@ 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, TX_TYPE_MASK) ==
> ASTDP_DPMCU_TX) {
> - ast->tx_chip_types = AST_TX_ASTDP_BIT;
> - ast_dp_launch(&ast->base);
> + int ret = ast_dp_launch(ast);
> +
> + if (!ret)
> + ast->tx_chip_types = AST_TX_ASTDP_BIT;
> }
> }
>
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 22f548805dfb..65755798ab94 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -351,7 +351,7 @@ void ast_post_gpu(struct drm_device *dev)
>
> if (IS_AST_GEN7(ast)) {
> if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
> - ast_dp_launch(dev);
> + ast_dp_launch(ast);
> } else if (ast->config_mode == ast_use_p2a) {
> if (IS_AST_GEN6(ast))
> ast_post_chip_2500(dev);
> diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
> index 75671d345057..569de3188191 100644
> --- a/drivers/gpu/drm/ast/ast_reg.h
> +++ b/drivers/gpu/drm/ast/ast_reg.h
> @@ -37,6 +37,8 @@
> #define AST_IO_VGACRCB_HWC_16BPP BIT(0) /* set: ARGB4444, cleared: 2bpp palette */
> #define AST_IO_VGACRCB_HWC_ENABLED BIT(1)
>
> +#define AST_IO_VGACRD1_MCU_FW_EXECUTING BIT(5)
> +
> #define AST_IO_VGAIR1_R (0x5A)
> #define AST_IO_VGAIR1_VREFRESH BIT(3)
>
> @@ -67,12 +69,10 @@
> #define AST_DP_VIDEO_ENABLE BIT(0)
>
> /*
> - * CRD1[b5]: DP MCU FW is executing
> * CRDC[b0]: DP link success
> * CRDF[b0]: DP HPD
> * CRE5[b0]: Host reading EDID process is done
> */
> -#define ASTDP_MCU_FW_EXECUTING BIT(5)
> #define ASTDP_LINK_SUCCESS BIT(0)
> #define ASTDP_HPD BIT(0)
> #define ASTDP_HOST_EDID_READ_DONE BIT(0)
More information about the dri-devel
mailing list