| Message ID | 20240717143319.104012-6-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ast: Fix DP hotplugging and clean up | expand |
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@redhat.com> > > Signed-off-by: Thomas Zimmermann <tzimmermann@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)
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)
Simplify ast_astdp_read_edid(). Rename register constants. Drop unnecessary error handling. On success, the helper returns 0; an error code otherwise. Signed-off-by: Thomas Zimmermann <tzimmermann@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(-)