Re: [PATCH 3/5] drm/ast: astdp: Only test HDP state in ast_astdp_is_connected()

[Jocelyn Falempe <jfalempe@xxxxxxxxxx>]

On 17/07/2024 16:24, Thomas Zimmermann wrote:
> The overall control flow of the driver ensures that it never reads
> EDID or sets display state on unconnected outputs. Therefore remove
> all tests for Hot Plug Detection from these helpers. Also rename
> the register constants.

Thanks, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
>   drivers/gpu/drm/ast/ast_dp.c  | 12 +++---------
>   drivers/gpu/drm/ast/ast_reg.h |  3 +--
>   2 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index 59885d10d308..c45b336baf45 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -9,7 +9,7 @@
>   
>   bool ast_astdp_is_connected(struct ast_device *ast)
>   {
> -	if (!ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF, ASTDP_HPD))
> +	if (!ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF, AST_IO_VGACRDF_HPD))
>   		return false;
>   	if (!ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC, ASTDP_LINK_SUCCESS))
>   		return false;
> @@ -23,11 +23,9 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
>   
>   	/*
>   	 * 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, 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))) {
>   		goto err_astdp_edid_not_ready;
> @@ -61,8 +59,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, 0xDC,
> -							ASTDP_LINK_SUCCESS) &&
> -				ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF, ASTDP_HPD))) {
> +							ASTDP_LINK_SUCCESS))) {
>   				goto err_astdp_jump_out_loop_of_edid;
>   			}
>   
> @@ -111,8 +108,6 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
>   err_astdp_edid_not_ready:
>   	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)))
> -		return (~0xDF + 1);
>   	if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xE5, ASTDP_HOST_EDID_READ_DONE_MASK)))
>   		return (~0xE5 + 1);
>   
> @@ -182,8 +177,7 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
>   	ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) ~AST_DP_VIDEO_ENABLE, on);
>   
>   	// If DP plug in and link successful then check video on / off status
> -	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)) {
> +	if (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDC, ASTDP_LINK_SUCCESS)) {
>   		video_on_off <<= 4;
>   		while (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF,
>   						ASTDP_MIRROR_VIDEO_ENABLE) != video_on_off) {
> diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
> index 569de3188191..e61954dabf1a 100644
> --- a/drivers/gpu/drm/ast/ast_reg.h
> +++ b/drivers/gpu/drm/ast/ast_reg.h
> @@ -38,6 +38,7 @@
>   #define AST_IO_VGACRCB_HWC_ENABLED	BIT(1)
>   
>   #define AST_IO_VGACRD1_MCU_FW_EXECUTING	BIT(5)
> +#define AST_IO_VGACRDF_HPD		BIT(0)
>   
>   #define AST_IO_VGAIR1_R			(0x5A)
>   #define AST_IO_VGAIR1_VREFRESH		BIT(3)
> @@ -70,11 +71,9 @@
>   
>   /*
>    * CRDC[b0]: DP link success
> - * CRDF[b0]: DP HPD
>    * CRE5[b0]: Host reading EDID process is done
>    */
>   #define ASTDP_LINK_SUCCESS		BIT(0)
> -#define ASTDP_HPD			BIT(0)
>   #define ASTDP_HOST_EDID_READ_DONE	BIT(0)
>   #define ASTDP_HOST_EDID_READ_DONE_MASK	GENMASK(0, 0)
>