On Sat, Feb 22, 2025 at 10:50:56AM +0800, Yongbang Shi wrote:
> From: Baihan Li <libai...@huawei.com>
> 
> Add dp serdes cfg in link training process, and related adapting
> and modificating. Change some init values about training,
> because we want completely to negotiation process, so we start with
> the maximum rate and the electrical characteristic level is 0.
> 
> Signed-off-by: Baihan Li <libai...@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongb...@huawei.com>
> ---
> ChangeLog:
> v2 -> v3:
>   - change commit to an imperative sentence, suggested by Dmitry Baryshkov.
>   - put HIBMC_DP_HOST_SERDES_CTRL in dp_serdes.h, suggested by Dmitry 
> Baryshkov.
> v1 -> v2:
>   - splittting the patch and add more detailed the changes in the commit 
> message, suggested by Dmitry Baryshkov.
> ---
>  .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  1 +
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  5 ++-
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  | 33 ++++++++++++++++---
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |  1 +
>  .../gpu/drm/hisilicon/hibmc/dp/dp_serdes.h    |  6 ++++
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  8 ++---
>  6 files changed, 43 insertions(+), 11 deletions(-)
> 

Mostly LGTM.

> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h 
> b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h
> index 812d0794543c..e0537cc9af41 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h
> @@ -4,12 +4,15 @@
>  #ifndef DP_SERDES_H
>  #define DP_SERDES_H
>  
> +#include "dp_comm.h"

No, please include it directly, where required. This simplifies possible
inter-header dependencies.

> +
>  #define HIBMC_DP_HOST_OFFSET         0x10000
>  #define HIBMC_DP_LANE0_RATE_OFFSET   0x4
>  #define HIBMC_DP_LANE1_RATE_OFFSET   0xc
>  #define HIBMC_DP_LANE_STATUS_OFFSET  0x10
>  #define HIBMC_DP_PMA_LANE0_OFFSET    0x18
>  #define HIBMC_DP_PMA_LANE1_OFFSET    0x1c
> +#define HIBMC_DP_HOST_SERDES_CTRL    0x1f001c
>  #define HIBMC_DP_PMA_TXDEEMPH                GENMASK(18, 1)
>  
>  /* dp serdes TX-Deempth Configuration */
> @@ -24,6 +27,9 @@
>  #define DP_SERDES_VOL2_PRE1          0x4500
>  #define DP_SERDES_VOL3_PRE0          0x600
>  #define DP_SERDES_BW_8_1             0x3
> +#define DP_SERDES_BW_5_4             0x2
> +#define DP_SERDES_BW_2_7             0x1
> +#define DP_SERDES_BW_1_62            0x0
>  
>  #define DP_SERDES_DONE                       0x3
>  
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c 
> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index e6de6d5edf6b..67d39e258cac 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -28,9 +28,7 @@
>  #include "hibmc_drm_drv.h"
>  #include "hibmc_drm_regs.h"
>  
> -#define HIBMC_DP_HOST_SERDES_CTRL            0x1f001c
> -#define HIBMC_DP_HOST_SERDES_CTRL_VAL                0x8a00
> -#define HIBMC_DP_HOST_SERDES_CTRL_MASK               0x7ffff
> +#include "dp/dp_serdes.h"
>  
>  DEFINE_DRM_GEM_FOPS(hibmc_fops);
>  
> @@ -122,8 +120,8 @@ static int hibmc_kms_init(struct hibmc_drm_private *priv)
>       }
>  
>       /* if DP existed, init DP */
> -     if ((readl(priv->mmio + HIBMC_DP_HOST_SERDES_CTRL) &
> -          HIBMC_DP_HOST_SERDES_CTRL_MASK) == HIBMC_DP_HOST_SERDES_CTRL_VAL) {
> +     ret = readl(priv->mmio + HIBMC_DP_HOST_SERDES_CTRL);
> +     if (ret) {

Why?

>               ret = hibmc_dp_init(priv);
>               if (ret)
>                       drm_err(dev, "failed to init dp: %d\n", ret);
> -- 
> 2.33.0
> 

-- 
With best wishes
Dmitry

Reply via email to