[PATCH 5/5] drm/ast: astdp: Clean up EDID reading
Jocelyn Falempe
jfalempe at redhat.com
Tue Jul 30 07:43:56 UTC 2024
On 17/07/2024 16:24, Thomas Zimmermann wrote:
> Simplify ast_astdp_read_edid(). Rename register constants. Drop
> unnecessary error handling. On success, the helper returns 0; an
> error code otherwise.
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 | 93 ++++++++++++++++-------------------
> drivers/gpu/drm/ast/ast_reg.h | 12 +----
> 2 files changed, 44 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index 6cbde46f24dc..5d07678b502c 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -17,54 +17,55 @@ bool ast_astdp_is_connected(struct ast_device *ast)
> int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
> {
> struct ast_device *ast = to_ast_device(dev);
> - u8 i = 0, j = 0;
> + int ret = 0;
> + u8 i;
>
> - /*
> - * CRE5[b0]: Host reading EDID process is done
> - */
> - if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xE5, ASTDP_HOST_EDID_READ_DONE_MASK)))
> - goto err_astdp_edid_not_ready;
> -
> - ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE5, (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
> - 0x00);
> + /* Start reading EDID data */
> + ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xe5, (u8)~AST_IO_VGACRE5_EDID_READ_DONE, 0x00);
>
> for (i = 0; i < 32; i++) {
> + unsigned int j;
> +
> /*
> * CRE4[7:0]: Read-Pointer for EDID (Unit: 4bytes); valid range: 0~64
> */
> - ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE4,
> - ASTDP_AND_CLEAR_MASK, (u8)i);
> - j = 0;
> + ast_set_index_reg(ast, AST_IO_VGACRI, 0xe4, i);
>
> /*
> * CRD7[b0]: valid flag for EDID
> * CRD6[b0]: mirror read pointer for EDID
> */
> - while ((ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD7,
> - ASTDP_EDID_VALID_FLAG_MASK) != 0x01) ||
> - (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD6,
> - ASTDP_EDID_READ_POINTER_MASK) != i)) {
> + for (j = 0; j < 200; ++j) {
> + u8 vgacrd7, vgacrd6;
> +
> /*
> * Delay are getting longer with each retry.
> - * 1. The Delays are often 2 loops when users request "Display Settings"
> + *
> + * 1. No delay on first try
> + * 2. The Delays are often 2 loops when users request "Display Settings"
> * of right-click of mouse.
> - * 2. The Delays are often longer a lot when system resume from S3/S4.
> + * 3. The Delays are often longer a lot when system resume from S3/S4.
> */
> - mdelay(j+1);
> -
> - j++;
> - if (j > 200)
> - goto err_astdp_jump_out_loop_of_edid;
> + if (j)
> + mdelay(j + 1);
> +
> + /* Wait for EDID offset to show up in mirror register */
> + vgacrd7 = ast_get_index_reg(ast, AST_IO_VGACRI, 0xd7);
> + if (vgacrd7 & AST_IO_VGACRD7_EDID_VALID_FLAG) {
> + vgacrd6 = ast_get_index_reg(ast, AST_IO_VGACRI, 0xd6);
> + if (vgacrd6 == i)
> + break;
> + }
> + }
> + if (j == 200) {
> + ret = -EBUSY;
> + goto out;
> }
>
> - *(ediddata) = ast_get_index_reg_mask(ast, AST_IO_VGACRI,
> - 0xD8, ASTDP_EDID_READ_DATA_MASK);
> - *(ediddata + 1) = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xD9,
> - ASTDP_EDID_READ_DATA_MASK);
> - *(ediddata + 2) = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDA,
> - ASTDP_EDID_READ_DATA_MASK);
> - *(ediddata + 3) = ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDB,
> - ASTDP_EDID_READ_DATA_MASK);
> + ediddata[0] = ast_get_index_reg(ast, AST_IO_VGACRI, 0xd8);
> + ediddata[1] = ast_get_index_reg(ast, AST_IO_VGACRI, 0xd9);
> + ediddata[2] = ast_get_index_reg(ast, AST_IO_VGACRI, 0xda);
> + ediddata[3] = ast_get_index_reg(ast, AST_IO_VGACRI, 0xdb);
>
> if (i == 31) {
> /*
> @@ -76,29 +77,19 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
> * The Bytes-126 indicates the Number of extensions to
> * follow. 0 represents noextensions.
> */
> - *(ediddata + 3) = *(ediddata + 3) + *(ediddata + 2);
> - *(ediddata + 2) = 0;
> + ediddata[3] = ediddata[3] + ediddata[2];
> + ediddata[2] = 0;
> }
>
> ediddata += 4;
> }
>
> - ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE5, (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
> - ASTDP_HOST_EDID_READ_DONE);
> -
> - return 0;
> -
> -err_astdp_jump_out_loop_of_edid:
> - ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE5,
> - (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
> - ASTDP_HOST_EDID_READ_DONE);
> - return (~(j+256) + 1);
> -
> -err_astdp_edid_not_ready:
> - if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xE5, ASTDP_HOST_EDID_READ_DONE_MASK)))
> - return (~0xE5 + 1);
> +out:
> + /* Signal end of reading */
> + ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xe5, (u8)~AST_IO_VGACRE5_EDID_READ_DONE,
> + AST_IO_VGACRE5_EDID_READ_DONE);
>
> - return 0;
> + return ret;
> }
>
> /*
> @@ -122,9 +113,9 @@ int ast_dp_launch(struct ast_device *ast)
> return -ENODEV;
> }
>
> - ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE5,
> - (u8) ~ASTDP_HOST_EDID_READ_DONE_MASK,
> - ASTDP_HOST_EDID_READ_DONE);
> + ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xe5,
> + (u8) ~AST_IO_VGACRE5_EDID_READ_DONE,
> + AST_IO_VGACRE5_EDID_READ_DONE);
>
> return 0;
> }
> diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
> index 28bb43f6795b..040961cc1a19 100644
> --- a/drivers/gpu/drm/ast/ast_reg.h
> +++ b/drivers/gpu/drm/ast/ast_reg.h
> @@ -38,8 +38,10 @@
> #define AST_IO_VGACRCB_HWC_ENABLED BIT(1)
>
> #define AST_IO_VGACRD1_MCU_FW_EXECUTING BIT(5)
> +#define AST_IO_VGACRD7_EDID_VALID_FLAG BIT(0)
> #define AST_IO_VGACRDC_LINK_SUCCESS BIT(0)
> #define AST_IO_VGACRDF_HPD BIT(0)
> +#define AST_IO_VGACRE5_EDID_READ_DONE BIT(0)
>
> #define AST_IO_VGAIR1_R (0x5A)
> #define AST_IO_VGAIR1_VREFRESH BIT(3)
> @@ -70,12 +72,6 @@
> #define AST_DP_PHY_SLEEP BIT(4)
> #define AST_DP_VIDEO_ENABLE BIT(0)
>
> -/*
> - * CRE5[b0]: Host reading EDID process is done
> - */
> -#define ASTDP_HOST_EDID_READ_DONE BIT(0)
> -#define ASTDP_HOST_EDID_READ_DONE_MASK GENMASK(0, 0)
> -
> /*
> * CRDF[b4]: Mirror of AST_DP_VIDEO_ENABLE
> * Precondition: A. ~AST_DP_PHY_SLEEP &&
> @@ -84,10 +80,6 @@
> */
> #define ASTDP_MIRROR_VIDEO_ENABLE BIT(4)
>
> -#define ASTDP_EDID_READ_POINTER_MASK GENMASK(7, 0)
> -#define ASTDP_EDID_VALID_FLAG_MASK GENMASK(0, 0)
> -#define ASTDP_EDID_READ_DATA_MASK GENMASK(7, 0)
> -
> /*
> * ASTDP setmode registers:
> * CRE0[7:0]: MISC0 ((0x00: 18-bpp) or (0x20: 24-bpp)
More information about the dri-devel
mailing list