On Sat, Feb 22, 2025 at 10:50:56AM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@xxxxxxxxxx>
>
> 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 <libaihan@xxxxxxxxxx>
> Signed-off-by: Yongbang Shi <shiyongbang@xxxxxxxxxx>
> ---
> 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
