ast: astdp: Perform link training during atomic_enable

Jocelyn Falempe Tue, 30 Jul 2024 00:41:37 -0700



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 <jfale...@redhat.com>

Signed-off-by: Thomas Zimmermann <tzimmerm...@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)

Re: [PATCH 4/5] drm/ast: astdp: Perform link training during atomic_enable

Reply via email to