[PATCH v2] drm/ast: Fix long time waiting on s3/s4 resume

Jammy Huang jammy_huang at aspeedtech.com
Wed May 24 02:34:11 UTC 2023


Hi Thomas,

Could you help review this patch?

This is an issue leading to kernel panic found by Intel. Wendy has 
confirmed issue resolved by this patch.

On 2023/4/14 下午 03:42, Jammy Huang wrote:
> In resume, DP's launch function, ast_dp_launch, could wait at most 30
> seconds before timeout to check if DP is enabled.
>
> To avoid this problem, we only check if DP enable or not at driver probe.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217278
> Signed-off-by: Jammy Huang <jammy_huang at aspeedtech.com>
> ---
>   v2 changes:
>    - Fix build error.
> ---
>   drivers/gpu/drm/ast/ast_dp.c   | 55 +++++++++++-----------------------
>   drivers/gpu/drm/ast/ast_drv.h  |  2 +-
>   drivers/gpu/drm/ast/ast_main.c | 11 +++++--
>   drivers/gpu/drm/ast/ast_post.c |  3 +-
>   4 files changed, 29 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index 56483860306b..eee2f264c880 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -119,53 +119,32 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
>   /*
>    * Launch Aspeed DP
>    */
> -void ast_dp_launch(struct drm_device *dev, u8 bPower)
> +void ast_dp_launch(struct drm_device *dev)
>   {
> -	u32 i = 0, j = 0, WaitCount = 1;
> -	u8 bDPTX = 0;
> +	u32 i = 0;
>   	u8 bDPExecute = 1;
> -
>   	struct ast_private *ast = to_ast_private(dev);
> -	// S3 come back, need more time to wait BMC ready.
> -	if (bPower)
> -		WaitCount = 300;
> -
> -
> -	// Wait total count by different condition.
> -	for (j = 0; j < WaitCount; j++) {
> -		bDPTX = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, TX_TYPE_MASK);
> -
> -		if (bDPTX)
> -			break;
>   
> +	// Wait one second then timeout.
> +	while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, COPROCESSOR_LAUNCH) !=
> +		COPROCESSOR_LAUNCH) {
> +		i++;
> +		// wait 100 ms
>   		msleep(100);
> -	}
>   
> -	// 0xE : ASTDP with DPMCU FW handling
> -	if (bDPTX == ASTDP_DPMCU_TX) {
> -		// Wait one second then timeout.
> -		i = 0;
> -
> -		while (ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xD1, COPROCESSOR_LAUNCH) !=
> -			COPROCESSOR_LAUNCH) {
> -			i++;
> -			// wait 100 ms
> -			msleep(100);
> -
> -			if (i >= 10) {
> -				// DP would not be ready.
> -				bDPExecute = 0;
> -				break;
> -			}
> +		if (i >= 10) {
> +			// DP would not be ready.
> +			bDPExecute = 0;
> +			break;
>   		}
> +	}
>   
> -		if (bDPExecute)
> -			ast->tx_chip_types |= BIT(AST_TX_ASTDP);
> +	if (!bDPExecute)
> +		drm_err(dev, "Wait DPMCU executing timeout\n");
>   
> -		ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
> -							(u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
> -							ASTDP_HOST_EDID_READ_DONE);
> -	}
> +	ast_set_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xE5,
> +			       (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
> +			       ASTDP_HOST_EDID_READ_DONE);
>   }
>   
>   
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index d51b81fea9c8..15e86394be4f 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -498,7 +498,7 @@ struct ast_i2c_chan *ast_i2c_create(struct drm_device *dev);
>   
>   /* aspeed DP */
>   int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
> -void ast_dp_launch(struct drm_device *dev, u8 bPower);
> +void ast_dp_launch(struct drm_device *dev);
>   void ast_dp_power_on_off(struct drm_device *dev, bool no);
>   void ast_dp_set_on_off(struct drm_device *dev, bool no);
>   void ast_dp_set_mode(struct drm_crtc *crtc, struct ast_vbios_mode_info *vbios_mode);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index f83ce77127cb..8ecddf20113f 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -254,8 +254,13 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>   		case 0x0c:
>   			ast->tx_chip_types = AST_TX_DP501_BIT;
>   		}
> -	} else if (ast->chip == AST2600)
> -		ast_dp_launch(&ast->base, 0);
> +	} else if (ast->chip == AST2600) {
> +		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;
> +			ast_dp_launch(&ast->base);
> +		}
> +	}
>   
>   	/* Print stuff for diagnostic purposes */
>   	if (ast->tx_chip_types & AST_TX_NONE_BIT)
> @@ -264,6 +269,8 @@ static int ast_detect_chip(struct drm_device *dev, bool *need_post)
>   		drm_info(dev, "Using Sil164 TMDS transmitter\n");
>   	if (ast->tx_chip_types & AST_TX_DP501_BIT)
>   		drm_info(dev, "Using DP501 DisplayPort transmitter\n");
> +	if (ast->tx_chip_types & AST_TX_ASTDP_BIT)
> +		drm_info(dev, "Using ASPEED DisplayPort transmitter\n");
>   
>   	return 0;
>   }
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 82fd3c8adee1..90e40f59aff7 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -380,7 +380,8 @@ void ast_post_gpu(struct drm_device *dev)
>   	ast_set_def_ext_reg(dev);
>   
>   	if (ast->chip == AST2600) {
> -		ast_dp_launch(dev, 1);
> +		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)
>   			ast_post_chip_2500(dev);
>
> base-commit: e62252bc55b6d4eddc6c2bdbf95a448180d6a08d

-- 
Best Regards
Jammy



More information about the dri-devel mailing list