[PATCH 4/5] drm/ast: astdp: Perform link training during atomic_enable
Jocelyn Falempe
jfalempe at redhat.com
Tue Jul 30 07:41:23 UTC 2024
On 17/07/2024 16:24, Thomas Zimmermann wrote:
> The place for link training is in the encoder's atomic_enable
> helper. Remove all related tests from other helper ASTDP functions;
> especially ast_astdp_is_connected(), which tests HPD status.
>
> DP link training is controlled by the firmware. A status flag reports
> success or failure. The process can be fragile on Aspeed hardware. Moving
> the test from connector detection to the atomic_enable allows for several
> retries and a longer timeout.
>
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 | 45 +++++++++++++++++-----------------
> drivers/gpu/drm/ast/ast_drv.h | 1 +
> drivers/gpu/drm/ast/ast_mode.c | 2 ++
> drivers/gpu/drm/ast/ast_reg.h | 3 +--
> 4 files changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index c45b336baf45..6cbde46f24dc 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -11,8 +11,6 @@ bool ast_astdp_is_connected(struct ast_device *ast)
> {
> 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;
> return true;
> }
>
> @@ -22,14 +20,10 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata)
> u8 i = 0, j = 0;
>
> /*
> - * CRDC[b0]: DP link success
> * 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, 0xE5,
> - ASTDP_HOST_EDID_READ_DONE_MASK))) {
> + 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);
> @@ -58,11 +52,6 @@ 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))) {
> - goto err_astdp_jump_out_loop_of_edid;
> - }
> -
> j++;
> if (j > 200)
> goto err_astdp_jump_out_loop_of_edid;
> @@ -106,8 +95,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, 0xDC, ASTDP_LINK_SUCCESS)))
> - return (~0xDC + 1);
> if (!(ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xE5, ASTDP_HOST_EDID_READ_DONE_MASK)))
> return (~0xE5 + 1);
>
> @@ -165,7 +152,22 @@ void ast_dp_power_on_off(struct drm_device *dev, bool on)
> ast_set_index_reg_mask(ast, AST_IO_VGACRI, 0xE3, (u8) ~AST_DP_PHY_SLEEP, bE3);
> }
>
> +void ast_dp_link_training(struct ast_device *ast)
> +{
> + struct drm_device *dev = &ast->base;
> + unsigned int i = 10;
> +
> + while (i--) {
> + u8 vgacrdc = ast_get_index_reg(ast, AST_IO_VGACRI, 0xdc);
>
> + if (vgacrdc & AST_IO_VGACRDC_LINK_SUCCESS)
> + break;
> + if (i)
> + msleep(100);
> + }
> + if (!i)
> + drm_err(dev, "Link training failed\n");
> +}
>
> void ast_dp_set_on_off(struct drm_device *dev, bool on)
> {
> @@ -176,16 +178,13 @@ void ast_dp_set_on_off(struct drm_device *dev, bool on)
> // Video On/Off
> 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)) {
> - video_on_off <<= 4;
> - while (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF,
> + video_on_off <<= 4;
> + while (ast_get_index_reg_mask(ast, AST_IO_VGACRI, 0xDF,
> ASTDP_MIRROR_VIDEO_ENABLE) != video_on_off) {
> - // wait 1 ms
> - mdelay(1);
> - if (++i > 200)
> - break;
> - }
> + // wait 1 ms
> + mdelay(1);
> + if (++i > 200)
> + break;
> }
> }
>
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 02476eb78119..d23b98ce4359 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -474,6 +474,7 @@ int ast_astdp_read_edid(struct drm_device *dev, u8 *ediddata);
> 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_link_training(struct ast_device *ast);
> 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_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 049ee1477c33..ddb7696acc04 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1622,6 +1622,8 @@ static void ast_astdp_encoder_helper_atomic_enable(struct drm_encoder *encoder,
> struct ast_device *ast = to_ast_device(dev);
>
> ast_dp_power_on_off(dev, AST_DP_POWER_ON);
> + ast_dp_link_training(ast);
> +
> ast_wait_for_vretrace(ast);
> ast_dp_set_on_off(dev, 1);
> }
> diff --git a/drivers/gpu/drm/ast/ast_reg.h b/drivers/gpu/drm/ast/ast_reg.h
> index e61954dabf1a..28bb43f6795b 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_VGACRDC_LINK_SUCCESS BIT(0)
> #define AST_IO_VGACRDF_HPD BIT(0)
>
> #define AST_IO_VGAIR1_R (0x5A)
> @@ -70,10 +71,8 @@
> #define AST_DP_VIDEO_ENABLE BIT(0)
>
> /*
> - * CRDC[b0]: DP link success
> * CRE5[b0]: Host reading EDID process is done
> */
> -#define ASTDP_LINK_SUCCESS BIT(0)
> #define ASTDP_HOST_EDID_READ_DONE BIT(0)
> #define ASTDP_HOST_EDID_READ_DONE_MASK GENMASK(0, 0)
>
More information about the dri-devel
mailing list