Re: [PATCH v3 drm-dp 2/8] drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage and pre-emphasis

Yongbang Shi <shiyongbang@xxxxxxxxxx> · Sat, 22 Feb 2025 18:31:32 +0800

On Sat, Feb 22, 2025 at 10:50:55AM +0800, Yongbang Shi wrote:
From: Baihan Li <libaihan@xxxxxxxxxx>

This dp controller need features of digital-to-analog conversion and
high-speed transmission in chip by its extern serdes controller. Our
serdes cfg is relatively simple, just need two register configurations.
Don't need too much functions, like: power on/off, initialize, and some
complex configurations, so I'm not going to use the phy framework.
This serdes is inited and configured in dp initialization, and also
integrating them into link training process.

For rate changing, we can change from 1.62-8.2Gpbs by cfg reg.
For voltage and pre-emphasis levels changing, we can cfg different
serdes ffe value.

Signed-off-by: Baihan Li <libaihan@xxxxxxxxxx>
Signed-off-by: Yongbang Shi <shiyongbang@xxxxxxxxxx>
---
ChangeLog:
v2 - > v3:
   - add commit log about dp serdes, suggested by Dmitry Baryshkov.
   - return value in hibmc_dp_serdes_init(), suggested by Dmitry Baryshkov.
   - add static const in the array of serdes_tx_cfg[], suggested by Dmitry Baryshkov.
   - change drm_warn to drm_dbg_dp, suggested by Dmitry Baryshkov.
   - add explanations about dp serdes macros, suggested by Dmitry Baryshkov.
v1 -> v2:
   - splittting the patch and add more detailed the changes in the commit message, suggested by Dmitry Baryshkov.
   - changing all names of dp phy to dp serdes.
---
  drivers/gpu/drm/hisilicon/hibmc/Makefile      |  2 +-
  drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  1 +
  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  6 ++
  .../gpu/drm/hisilicon/hibmc/dp/dp_serdes.c    | 72 +++++++++++++++++++
  .../gpu/drm/hisilicon/hibmc/dp/dp_serdes.h    | 34 +++++++++
  5 files changed, 114 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 95a4ed599d98..43de077d6769 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
+++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
@@ -1,5 +1,5 @@
  # SPDX-License-Identifier: GPL-2.0-only
  hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \
-	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o hibmc_drm_dp.o
+	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o dp/dp_serdes.o hibmc_drm_dp.o
  
  obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
index 2c52a4476c4d..e7746bc4b592 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
@@ -38,6 +38,7 @@ struct hibmc_dp_dev {
  	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
  	struct hibmc_dp_link link;
  	u8 dpcd[DP_RECEIVER_CAP_SIZE];
+	void __iomem *serdes_base;
  };
  
  #define dp_field_modify(reg_value, mask, val)				\
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index a8d543881c09..59078483ec55 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -7,6 +7,7 @@
  #include "dp_comm.h"
  #include "dp_reg.h"
  #include "dp_hw.h"
+#include "dp_serdes.h"
  
  static void hibmc_dp_set_tu(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
  {
@@ -151,6 +152,7 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
  {
  	struct drm_device *drm_dev = dp->drm_dev;
  	struct hibmc_dp_dev *dp_dev;
+	int ret;
  
  	dp_dev = devm_kzalloc(drm_dev->dev, sizeof(struct hibmc_dp_dev), GFP_KERNEL);
  	if (!dp_dev)
@@ -165,6 +167,10 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
  
  	hibmc_dp_aux_init(dp_dev);
  
+	ret = hibmc_dp_serdes_init(dp_dev);
+	if (ret)
+		return ret;
+
  	dp_dev->link.cap.lanes = 0x2;
  	dp_dev->link.cap.link_rate = DP_LINK_BW_2_7;
  
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
new file mode 100644
index 000000000000..241b9ef782b0
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2025 Hisilicon Limited.
+
+#include <linux/delay.h>
+#include <drm/drm_device.h>
+#include <drm/drm_print.h>
+#include "dp_comm.h"
+#include "dp_config.h"
+#include "dp_reg.h"
+#include "dp_serdes.h"
+
+int hibmc_dp_serdes_set_tx_cfg(struct hibmc_dp_dev *dp, u8 train_set[HIBMC_DP_LANE_NUM_MAX])
+{
+	static const u32 serdes_tx_cfg[4][4] = { {DP_SERDES_VOL0_PRE0, DP_SERDES_VOL0_PRE1,
+						  DP_SERDES_VOL0_PRE2, DP_SERDES_VOL0_PRE3},
+						 {DP_SERDES_VOL1_PRE0, DP_SERDES_VOL1_PRE1,
+						  DP_SERDES_VOL1_PRE2}, {DP_SERDES_VOL2_PRE0,
+						  DP_SERDES_VOL2_PRE1}, {DP_SERDES_VOL3_PRE0}};
+	int cfg[2];
+	int i;
+
+	for (i = 0; i < HIBMC_DP_LANE_NUM_MAX; i++) {
+		cfg[i] = serdes_tx_cfg[(train_set[i] & 0x3)]
+			 [(train_set[i] << DP_TRAIN_PRE_EMPHASIS_SHIFT & 0x3)];
I think this will not work as expected. There should be no need to shift
train_set[i], otherwise first '&' is executed, reducing array index to
(train_set[i]) << 3), which then is out-of-boundaries for the array.

Most likely you meant:

cfg[i] = serdes_tx_cfg[FIELD_GET(DP_TRAIN_VOLTAGE_SWING_MASK, train_set[i]]
		      [FIELD_GET(DP_TRAIN_PRE_EMPHASIS_MASK, train_set[i]];

Yeah, that's right, I'll change it. Thanks for your correcting！



+		if (!cfg[i])
+			return -EINVAL;
+
+		/* lane1 offset is 4 */
+		writel(FIELD_PREP(HIBMC_DP_PMA_TXDEEMPH, cfg[i]),
+		       dp->serdes_base + HIBMC_DP_PMA_LANE0_OFFSET + i * 4);
+	}
+
+	usleep_range(300, 500);
+
+	if (readl(dp->serdes_base + HIBMC_DP_LANE_STATUS_OFFSET) != DP_SERDES_DONE) {
+		drm_dbg_dp(dp->dev, "dp serdes cfg failed\n");
+		return -EAGAIN;
+	}
+
+	return 0;
+}
+
+int hibmc_dp_serdes_rate_switch(u8 rate, struct hibmc_dp_dev *dp)
+{
+	writel(rate, dp->serdes_base + HIBMC_DP_LANE0_RATE_OFFSET);
+	writel(rate, dp->serdes_base + HIBMC_DP_LANE1_RATE_OFFSET);
+
+	usleep_range(300, 500);
+
+	if (readl(dp->serdes_base + HIBMC_DP_LANE_STATUS_OFFSET) != DP_SERDES_DONE) {
+		drm_dbg_dp(dp->dev, "dp serdes rate switching failed\n");
+		return -EAGAIN;
+	}
+
+	if (rate < DP_SERDES_BW_8_1)
+		drm_dbg_dp(dp->dev, "reducing serdes rate to :%d\n",
+			   rate ? rate * HIBMC_DP_LINK_RATE_CAL * 10 : 162);
+
+	return 0;
+}
+
+int hibmc_dp_serdes_init(struct hibmc_dp_dev *dp)
+{
+	dp->serdes_base = dp->base + HIBMC_DP_HOST_OFFSET;
+
+	writel(FIELD_PREP(HIBMC_DP_PMA_TXDEEMPH, DP_SERDES_VOL0_PRE0),
+	       dp->serdes_base + HIBMC_DP_PMA_LANE0_OFFSET);
+	writel(FIELD_PREP(HIBMC_DP_PMA_TXDEEMPH, DP_SERDES_VOL0_PRE0),
+	       dp->serdes_base + HIBMC_DP_PMA_LANE1_OFFSET);
+
+	return hibmc_dp_serdes_rate_switch(DP_SERDES_BW_8_1, dp);
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h
new file mode 100644
index 000000000000..812d0794543c
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (c) 2025 Hisilicon Limited. */
+
+#ifndef DP_SERDES_H
+#define DP_SERDES_H
+
Why all these values are not a part of the dp_reg.h?

Okay, I'll merged together.


+#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_PMA_TXDEEMPH		GENMASK(18, 1)
+
+/* dp serdes TX-Deempth Configuration */
+#define DP_SERDES_VOL0_PRE0		0x280
+#define DP_SERDES_VOL0_PRE1		0x2300
+#define DP_SERDES_VOL0_PRE2		0x53c0
+#define DP_SERDES_VOL0_PRE3		0x8400
+#define DP_SERDES_VOL1_PRE0		0x380
+#define DP_SERDES_VOL1_PRE1		0x3440
+#define DP_SERDES_VOL1_PRE2		0x6480
+#define DP_SERDES_VOL2_PRE0		0x500
+#define DP_SERDES_VOL2_PRE1		0x4500
+#define DP_SERDES_VOL3_PRE0		0x600
+#define DP_SERDES_BW_8_1		0x3
+
+#define DP_SERDES_DONE			0x3
+
+int hibmc_dp_serdes_init(struct hibmc_dp_dev *dp);
+int hibmc_dp_serdes_rate_switch(u8 rate, struct hibmc_dp_dev *dp);
+int hibmc_dp_serdes_set_tx_cfg(struct hibmc_dp_dev *dp, u8 train_set[HIBMC_DP_LANE_NUM_MAX]);
+
+#endif
--
2.33.0







