[PATCH v5 drm-dp 0/5] Add dp module in hibmc driver

[PATCH v5 drm-dp 0/5] Add dp module in hibmc driver

[Yongbang Shi]

From: baihan li <libaihan@huawei.com>

Realizing the basic display function of DP cable for DP connector
displaying. Add DP module in hibmc drm driver, which is for Hisilicon
Hibmc SoC which used for Out-of-band management. Blow is the general
hardware connection, both the Hibmc and the host CPU are on the same
mother board.

+----------+       +----------+      +----- ----+      +----------------+
|          | PCIe  |  Hibmc   |      |          |      |                |
|host CPU( |<----->| display  |<---->| dp kapi  |<---->| dp aux moduel  |
|arm64,x86)|       |subsystem |      |  moduel  |<---->| dp link moduel |
+----------+       +----------+      +----------+      +----------------+

---
ChangeLog:
v4 -> v5:
  - fixing build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/
  v4:https://lore.kernel.org/all/20241112132348.2631150-1-shiyongbang@huawei.com/
v3 -> v4:
  - retun error codes in  result incorrect branch, suggested by Dmitry Baryshkov.
  - replacing all ret= with returns, suggested by Dmitry Baryshkov.
  - moving the comment below the judgment statement, suggested by Dmitry Baryshkov.
  - moving definations to the source file and clearing headers, suggested by Dmitry Baryshkov.
  - reanaming dp_prefix to hibmc_dp_prefix, suggested by Dmitry Baryshkov.
  - changing hibmc_dp_reg_write_field to static inline and lock, suggested by Dmitry Baryshkov.
  - moving some structs to later patch, suggested by Dmitry Baryshkov.
  - optimizing hibmc_dp_link_get_adjust_train() to delete for loop, suggested by Dmitry Baryshkov.
  - changing ELNRNG to EIO error code, suggested by Dmitry Baryshkov.
  - deleting meaningless macro, suggested by Dmitry Baryshkov.
  - fixing build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202411041559.WIfxRN6n-lkp@intel.com/
  - changed the type of train_set to array, suggested by Dmitry Baryshkov.
  - using actual link rate instead of magic num, suggested by Dmitry Baryshkov.
  - deleting hibmc_dp_hw_uninit(), suggested by Dmitry Baryshkov.
  - separating hibmc_vdac and hibmc_dp changes into separate patche, suggested by Dmitry Baryshkov.
  - static int hibmc_dp_prepare(), suggested by Dmitry Baryshkov.
  v3:https://lore.kernel.org/all/20241101105028.2177274-1-shiyongbang@huawei.com/
v2 -> v3:
  - put the macro definations in latter patch where they are actually used, suggested by Dmitry Baryshkov.
  - rename some macro definations to make them sensible, suggested by Dmitry Baryshkov.
  - using FIELD_PREP and FIELD_GET, suggested by Dmitry Baryshkov.
  - using DP_DPCD_REV_foo, suggested by Dmitry Baryshkov.
  - using switchcase in dp_link_reduce_lane, suggested by Dmitry Baryshkov.
  - deleting dp_link_pattern2dpcd function and using macros directly, suggested by Dmitry Baryshkov.
  - deleting EFAULT error codes, suggested by Dmitry Baryshkov.
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202410250305.UHKDhtxy-lkp@intel.com/
    Closes: https://lore.kernel.org/oe-kbuild-all/202410250931.UDQ9s66H-lkp@intel.com/
    Closes: https://lore.kernel.org/oe-kbuild-all/202410251136.1m7BlR68-lkp@intel.com/
  v2:https://lore.kernel.org/all/20241022124148.1952761-1-shiyongbang@huawei.com/
v1 -> v2:
  - using drm_dp_aux frame implement dp aux read and write functions, suggested by Jani Nikula.
  - using drm dp header files' dp macros instead, suggested by Andy Yan.
  - using drm_dp_* functions implement dp link training process, suggested by Jani Nikula.
  - changed some defines and functions to former patch, suggested by Dmitry Baryshkov.
  - sorting the headers including in dp_hw.h and hibmc_drm_drv.c files, suggested by Dmitry Baryshkov.
  - deleting struct dp_mode and dp_mode_cfg function, suggested by Dmitry Baryshkov.
  - modifying drm_simple_encoder_init function, suggested by Dmitry Baryshkov.
  - refactoring struct hibmc_connector, suggested by Dmitry Baryshkov.
  - withdrawing the modification in hibmc_kms_init, suggested by Dmitry Baryshkov.
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202410031735.8iRZZR6T-lkp@intel.com/
    Closes: https://lore.kernel.org/oe-kbuild-all/202410040328.VeVxM9yB-lkp@intel.com/
  v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
---

baihan li (5):
  drm/hisilicon/hibmc: add dp aux in hibmc drivers
  drm/hisilicon/hibmc: add dp link moduel in hibmc drivers
  drm/hisilicon/hibmc: add dp hw moduel in hibmc driver
  drm/hisilicon/hibmc: refactored struct hibmc_drm_private
  drm/hisilicon/hibmc: add dp module in hibmc

 drivers/gpu/drm/hisilicon/hibmc/Makefile      |   3 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   | 164 +++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  62 ++++
 .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  19 +
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    | 217 +++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  28 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  | 339 ++++++++++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |  76 ++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 118 ++++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  12 +
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  19 +-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c   |  41 ++-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  |  20 +-
 13 files changed, 1079 insertions(+), 39 deletions(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c

-- 
2.33.0

[PATCH v5 drm-dp 1/5] drm/hisilicon/hibmc: add dp aux in hibmc

[Yongbang Shi]

From: baihan li <libaihan@huawei.com>

Add dp aux read/write functions. They are basic functions
and will be used later.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v4 -> v5:
  - fixing build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/
v3 -> v4:
  - retun error codes in  result incorrect branch, suggested by Dmitry Baryshkov.
  - replacing all ret= with returns, suggested by Dmitry Baryshkov.
  - moving the comment below the judgment statement, suggested by Dmitry Baryshkov.
  - moving definations to the source file and clearing headers, suggested by Dmitry Baryshkov.
  - reanaming dp_prefix to hibmc_dp_prefix, suggested by Dmitry Baryshkov.
  - changing hibmc_dp_reg_write_field to static inline and lock, suggested by Dmitry Baryshkov.
  - moving some structs to later patch, suggested by Dmitry Baryshkov.
v2 -> v3:
  - put the macro definations in latter patch where they are actually used, suggested by Dmitry Baryshkov.
  - rename some macro definations to make them sensible, suggested by Dmitry Baryshkov.
  - using FIELD_PREP and FIELD_GET, suggested by Dmitry Baryshkov.
  - using DP_DPCD_REV_foo, suggested by Dmitry Baryshkov.
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202410250305.UHKDhtxy-lkp@intel.com/
v1 -> v2:
  - using drm_dp_aux frame implement dp aux read and write functions, suggested by Jani Nikula.
  - using drm dp header files' dp macros instead, suggested by Andy Yan.
  v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
---
 drivers/gpu/drm/hisilicon/hibmc/Makefile     |   3 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c  | 164 +++++++++++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h |  38 +++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h  |  27 +++
 4 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index d25c75e60d3d..8770ec6dfffd 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
+++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
@@ -1,4 +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
+hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \
+	       dp/dp_aux.o
 
 obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
new file mode 100644
index 000000000000..16bdfefbf255
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-or-laterHIBMC_BYTES_IN_U32
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/minmax.h>
+#include <drm/drm_device.h>
+#include <drm/drm_print.h>
+#include "dp_comm.h"
+#include "dp_reg.h"
+
+#define HIBMC_AUX_CMD_REQ_LEN		GENMASK(7, 4)
+#define HIBMC_AUX_CMD_ADDR		GENMASK(27, 8)
+#define HIBMC_AUX_CMD_I2C_ADDR_ONLY	BIT(28)
+#define HIBMC_BYTES_IN_U32		4
+#define HIBMC_AUX_I2C_WRITE_SUCCESS	0x1
+#define HIBMC_DP_MIN_PULSE_NUM		0x9
+#define BITS_IN_U8			8
+
+static inline void hibmc_dp_aux_reset(struct hibmc_dp_dev *dp)
+{
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x0);
+	usleep_range(10, 15);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x1);
+}
+
+static void hibmc_dp_aux_read_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
+{
+	u32 reg_num;
+	u32 value;
+	u32 num;
+	u8 i, j;
+
+	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
+	for (i = 0; i < reg_num; i++) {
+		/* number of bytes read from a single register */
+		num = min(size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
+		value = readl(dp->base + HIBMC_DP_AUX_RD_DATA0 + i * HIBMC_BYTES_IN_U32);
+		/* convert the 32-bit value of the register to the buffer. */
+		for (j = 0; j < num; j++)
+			buf[i * HIBMC_BYTES_IN_U32 + j] = value >> (j * BITS_IN_U8);
+	}
+}
+
+static void hibmc_dp_aux_write_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
+{
+	u32 reg_num;
+	u32 value;
+	u32 num;
+	u8 i, j;
+
+	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
+	for (i = 0; i < reg_num; i++) {
+		/* number of bytes written to a single register */
+		num = min_t(u8, size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
+		value = 0;
+		/* obtain the 32-bit value written to a single register. */
+		for (j = 0; j < num; j++)
+			value |= buf[i * HIBMC_BYTES_IN_U32 + j] << (j * BITS_IN_U8);
+		/* writing data to a single register */
+		writel(value, dp->base + HIBMC_DP_AUX_WR_DATA0 + i * HIBMC_BYTES_IN_U32);
+	}
+}
+
+static u32 hibmc_dp_aux_build_cmd(const struct drm_dp_aux_msg *msg)
+{
+	u32 aux_cmd = msg->request;
+
+	if (msg->size)
+		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_REQ_LEN, (msg->size - 1));
+	else
+		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_I2C_ADDR_ONLY, 1);
+
+	aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_ADDR, msg->address);
+
+	return aux_cmd;
+}
+
+/* ret >= 0, ret is size; ret < 0, ret is err code */
+static int hibmc_dp_aux_parse_xfer(struct hibmc_dp_dev *dp, struct drm_dp_aux_msg *msg)
+{
+	u32 buf_data_cnt;
+	u32 aux_status;
+
+	aux_status = readl(dp->base + HIBMC_DP_AUX_STATUS);
+	msg->reply = FIELD_GET(HIBMC_DP_CFG_AUX_STATUS, aux_status);
+
+	if (aux_status & HIBMC_DP_CFG_AUX_TIMEOUT)
+		return -ETIMEDOUT;
+
+	/* only address */
+	if (!msg->size)
+		return 0;
+
+	if (msg->reply != DP_AUX_NATIVE_REPLY_ACK)
+		return -EIO;
+
+	buf_data_cnt = FIELD_GET(HIBMC_DP_CFG_AUX_READY_DATA_BYTE, aux_status);
+
+	switch (msg->request) {
+	case DP_AUX_NATIVE_WRITE:
+		return msg->size;
+	case DP_AUX_I2C_WRITE | DP_AUX_I2C_MOT:
+		if (buf_data_cnt == HIBMC_AUX_I2C_WRITE_SUCCESS)
+			return msg->size;
+		else
+			return FIELD_GET(HIBMC_DP_CFG_AUX, aux_status);
+	case DP_AUX_NATIVE_READ:
+	case DP_AUX_I2C_READ | DP_AUX_I2C_MOT:
+		buf_data_cnt--;
+		if (buf_data_cnt != msg->size) {
+			/* only the successful part of data is read */
+			return -EBUSY;
+		}
+
+		/* all data is successfully read */
+		hibmc_dp_aux_read_data(dp, msg->buffer, msg->size);
+		return msg->size;
+	default:
+		return -EINVAL;
+	}
+}
+
+/* ret >= 0 ,ret is size; ret < 0, ret is err code */
+static ssize_t hibmc_dp_aux_xfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
+{
+	struct hibmc_dp_dev *dp = container_of(aux, struct hibmc_dp_dev, aux);
+	u32 aux_cmd;
+	int ret;
+	u32 val; /* val will be assigned at the beginning of readl_poll_timeout function */
+
+	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA0);
+	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA1);
+	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA2);
+	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA3);
+
+	hibmc_dp_aux_write_data(dp, msg->buffer, msg->size);
+
+	aux_cmd = hibmc_dp_aux_build_cmd(msg);
+	writel(aux_cmd, dp->base + HIBMC_DP_AUX_CMD_ADDR);
+
+	/* enable aux transfer */
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_REQ, 0x1);
+	ret = readl_poll_timeout(dp->base + HIBMC_DP_AUX_REQ, val,
+				 !(val & HIBMC_DP_CFG_AUX_REQ), 50, 5000);
+	if (ret) {
+		hibmc_dp_aux_reset(dp);
+		return ret;
+	}
+
+	return hibmc_dp_aux_parse_xfer(dp, msg);
+}
+
+void hibmc_dp_aux_init(struct hibmc_dp_dev *dp)
+{
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_SYNC_LEN_SEL, 0x0);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_TIMER_TIMEOUT, 0x1);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_MIN_PULSE_NUM,
+				 HIBMC_DP_MIN_PULSE_NUM);
+
+	dp->aux.transfer = hibmc_dp_aux_xfer;
+	dp->aux.is_remote = 0;
+	drm_dp_aux_init(&dp->aux);
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
new file mode 100644
index 000000000000..ce3b6fa4ea9e
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef DP_COMM_H
+#define DP_COMM_H
+
+#include <linux/types.h>
+#include <linux/bitops.h>
+#include <linux/errno.h>
+#include <linux/mutex.h>
+#include <linux/kernel.h>
+#include <linux/bitfield.h>
+#include <linux/io.h>
+#include <drm/display/drm_dp_helper.h>
+
+struct hibmc_dp_dev {
+	struct drm_dp_aux aux;
+	struct drm_device *dev;
+	void __iomem *base;
+	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
+};
+
+#define dp_field_modify(reg_value, mask, val) ({		\
+	(reg_value) &= ~(mask);					\
+	(reg_value) |= FIELD_PREP(mask, val); })
+
+#define hibmc_dp_reg_write_field(dp, offset, mask, val) ({	\
+	typeof(dp) _dp = dp;					\
+	typeof(_dp->base) addr = (_dp->base + (offset));	\
+	mutex_lock(&_dp->lock);					\
+	u32 reg_value = readl(addr);				\
+	dp_field_modify(reg_value, mask, val);			\
+	writel(reg_value, addr);				\
+	mutex_unlock(&_dp->lock); })
+
+void hibmc_dp_aux_init(struct hibmc_dp_dev *dp);
+
+#endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
new file mode 100644
index 000000000000..f3e6781e111a
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -0,0 +1,27 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef DP_REG_H
+#define DP_REG_H
+
+#define HIBMC_DP_AUX_CMD_ADDR			0x50
+#define HIBMC_DP_AUX_WR_DATA0			0x54
+#define HIBMC_DP_AUX_WR_DATA1			0x58
+#define HIBMC_DP_AUX_WR_DATA2			0x5c
+#define HIBMC_DP_AUX_WR_DATA3			0x60
+#define HIBMC_DP_AUX_RD_DATA0			0x64
+#define HIBMC_DP_AUX_REQ			0x74
+#define HIBMC_DP_AUX_STATUS			0x78
+#define HIBMC_DP_DPTX_RST_CTRL			0x700
+
+#define HIBMC_DP_CFG_AUX_SYNC_LEN_SEL		BIT(1)
+#define HIBMC_DP_CFG_AUX_TIMER_TIMEOUT		BIT(2)
+#define HIBMC_DP_CFG_AUX_MIN_PULSE_NUM		GENMASK(13, 9)
+#define HIBMC_DP_CFG_AUX_REQ			BIT(0)
+#define HIBMC_DP_CFG_AUX_RST_N			BIT(4)
+#define HIBMC_DP_CFG_AUX_TIMEOUT		BIT(0)
+#define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
+#define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
+#define HIBMC_DP_CFG_AUX_STATUS			GENMASK(11, 4)
+
+#endif
-- 
2.33.0

[PATCH v5 drm-dp 2/5] drm/hisilicon/hibmc: add dp link moduel in hibmc

[Yongbang Shi]

From: baihan li <libaihan@huawei.com>

Add link training process functions in this moduel.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
Changelog:
v3 -> v4:
  - optimizing hibmc_dp_link_get_adjust_train() to delete for loop, suggested by Dmitry Baryshkov.
  - changing ELNRNG to EIO error code, suggested by Dmitry Baryshkov.
  - deleting meaningless macro, suggested by Dmitry Baryshkov.
  - fixing build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202411041559.WIfxRN6n-lkp@intel.com/
v2 -> v3:
  - using switchcase in dp_link_reduce_lane, suggested by Dmitry Baryshkov.
  - deleting dp_link_pattern2dpcd function and using macros directly, suggested by Dmitry Baryshkov.
  - deleting EFAULT error codes, suggested by Dmitry Baryshkov.
v1 -> v2:
  - using drm_dp_* functions implement dp link training process, suggested by Jani Nikula.
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202410031735.8iRZZR6T-lkp@intel.com/
  v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
---
 drivers/gpu/drm/hisilicon/hibmc/Makefile     |   2 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h |  24 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c | 339 +++++++++++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h  |   8 +
 4 files changed, 372 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 8770ec6dfffd..94d77da88bbf 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_aux.o dp/dp_link.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 ce3b6fa4ea9e..8553e531ba67 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
@@ -13,11 +13,34 @@
 #include <linux/io.h>
 #include <drm/display/drm_dp_helper.h>
 
+#define HIBMC_DP_LANE_NUM_MAX 2
+
+struct hibmc_link_status {
+	bool clock_recovered;
+	bool channel_equalized;
+};
+
+struct hibmc_link_cap {
+	int rx_dpcd_revision;
+	u8 link_rate;
+	u8 lanes;
+	bool is_tps3;
+	bool is_tps4;
+};
+
+struct hibmc_dp_link {
+	struct hibmc_link_status status;
+	u8 train_set[HIBMC_DP_LANE_NUM_MAX];
+	struct hibmc_link_cap cap;
+};
+
 struct hibmc_dp_dev {
 	struct drm_dp_aux aux;
 	struct drm_device *dev;
 	void __iomem *base;
 	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
+	struct hibmc_dp_link link;
+	u8 dpcd[DP_RECEIVER_CAP_SIZE];
 };
 
 #define dp_field_modify(reg_value, mask, val) ({		\
@@ -34,5 +57,6 @@ struct hibmc_dp_dev {
 	mutex_unlock(&_dp->lock); })
 
 void hibmc_dp_aux_init(struct hibmc_dp_dev *dp);
+int hibmc_dp_link_training(struct hibmc_dp_dev *dp);
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
new file mode 100644
index 000000000000..b7ae9e54126c
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/delay.h>
+#include <drm/drm_device.h>
+#include <drm/drm_print.h>
+#include "dp_comm.h"
+#include "dp_reg.h"
+
+#define HIBMC_EQ_MAX_RETRY 5
+
+static int hibmc_dp_link_training_configure(struct hibmc_dp_dev *dp)
+{
+	u8 buf[2];
+	int ret;
+
+	/* DP 2 lane */
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_PHYIF_CTRL0, HIBMC_DP_CFG_LANE_DATA_EN,
+				 dp->link.cap.lanes == 0x2 ? 0x3 : 0x1);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_GCTL0, HIBMC_DP_CFG_PHY_LANE_NUM,
+				 dp->link.cap.lanes == 0x2 ? 0x1 : 0);
+
+	/* enhanced frame */
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CTRL, HIBMC_DP_CFG_STREAM_FRAME_MODE, 0x1);
+
+	/* set rate and lane count */
+	buf[0] = dp->link.cap.link_rate;
+	buf[1] = DP_LANE_COUNT_ENHANCED_FRAME_EN | dp->link.cap.lanes;
+	ret = drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, buf, sizeof(buf));
+	if (ret != sizeof(buf)) {
+		drm_dbg_dp(dp->dev, "dp aux write link rate and lanes failed, ret: %d\n", ret);
+		return ret >= 0 ? -EIO : ret;
+	}
+
+	/* set 8b/10b and downspread */
+	buf[0] = 0x10;
+	buf[1] = 0x1;
+	ret = drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, buf, sizeof(buf));
+	if (ret != sizeof(buf)) {
+		drm_dbg_dp(dp->dev, "dp aux write 8b/10b and downspread failed, ret: %d\n", ret);
+		return ret >= 0 ? -EIO : ret;
+	}
+
+	ret = drm_dp_read_dpcd_caps(&dp->aux, dp->dpcd);
+	if (ret)
+		drm_err(dp->dev, "dp aux read dpcd failed, ret: %d\n", ret);
+
+	return ret;
+}
+
+static int hibmc_dp_link_set_pattern(struct hibmc_dp_dev *dp, int pattern)
+{
+	int ret;
+	u8 val;
+	u8 buf;
+
+	buf = (u8)pattern;
+	if (pattern != DP_TRAINING_PATTERN_DISABLE && pattern != DP_TRAINING_PATTERN_4) {
+		buf |= DP_LINK_SCRAMBLING_DISABLE;
+		hibmc_dp_reg_write_field(dp, HIBMC_DP_PHYIF_CTRL0, HIBMC_DP_CFG_SCRAMBLE_EN, 0x1);
+	} else {
+		hibmc_dp_reg_write_field(dp, HIBMC_DP_PHYIF_CTRL0, HIBMC_DP_CFG_SCRAMBLE_EN, 0);
+	}
+
+	switch (pattern) {
+	case DP_TRAINING_PATTERN_1:
+		val = 1;
+		break;
+	case DP_TRAINING_PATTERN_2:
+		val = 2;
+		break;
+	case DP_TRAINING_PATTERN_3:
+		val = 3;
+		break;
+	case DP_TRAINING_PATTERN_4:
+		val = 4;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_PHYIF_CTRL0, HIBMC_DP_CFG_PAT_SEL, val);
+
+	ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_PATTERN_SET, &buf, sizeof(buf));
+	if (ret != sizeof(buf)) {
+		drm_dbg_dp(dp->dev, "dp aux write training pattern set failed\n");
+		return ret >= 0 ? -EIO : ret;
+	}
+
+	return 0;
+}
+
+static int hibmc_dp_link_training_cr_pre(struct hibmc_dp_dev *dp)
+{
+	u8 *train_set = dp->link.train_set;
+	int ret;
+	u8 i;
+
+	ret = hibmc_dp_link_training_configure(dp);
+	if (ret)
+		return ret;
+
+	ret = hibmc_dp_link_set_pattern(dp, DP_TRAINING_PATTERN_1);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < dp->link.cap.lanes; i++)
+		train_set[i] = DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
+
+	ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set, dp->link.cap.lanes);
+	if (ret != dp->link.cap.lanes) {
+		drm_dbg_dp(dp->dev, "dp aux write training lane set failed\n");
+		return ret >= 0 ? -EIO : ret;
+	}
+
+	return 0;
+}
+
+static bool hibmc_dp_link_get_adjust_train(struct hibmc_dp_dev *dp,
+					   u8 lane_status[DP_LINK_STATUS_SIZE])
+{
+	u8 train_set[HIBMC_DP_LANE_NUM_MAX] = {0};
+	u8 lane;
+
+	for (lane = 0; lane < dp->link.cap.lanes; lane++)
+		train_set[lane] = drm_dp_get_adjust_request_voltage(lane_status, lane) |
+				  drm_dp_get_adjust_request_pre_emphasis(lane_status, lane);
+
+	if (memcmp(dp->link.train_set, train_set, HIBMC_DP_LANE_NUM_MAX)) {
+		memcpy(dp->link.train_set, train_set, HIBMC_DP_LANE_NUM_MAX);
+		return true;
+	}
+
+	return false;
+}
+
+static inline int hibmc_dp_link_reduce_rate(struct hibmc_dp_dev *dp)
+{
+	switch (dp->link.cap.link_rate) {
+	case DP_LINK_BW_2_7:
+		dp->link.cap.link_rate = DP_LINK_BW_1_62;
+		return 0;
+	case DP_LINK_BW_5_4:
+		dp->link.cap.link_rate = DP_LINK_BW_2_7;
+		return 0;
+	case DP_LINK_BW_8_1:
+		dp->link.cap.link_rate = DP_LINK_BW_5_4;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static inline int hibmc_dp_link_reduce_lane(struct hibmc_dp_dev *dp)
+{
+	switch (dp->link.cap.lanes) {
+	case 0x2:
+		dp->link.cap.lanes--;
+		break;
+	case 0x1:
+		drm_err(dp->dev, "dp link training reduce lane failed, already reach minimum\n");
+		return -EIO;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int hibmc_dp_link_training_cr(struct hibmc_dp_dev *dp)
+{
+	u8 lane_status[DP_LINK_STATUS_SIZE] = {0};
+	bool level_changed;
+	u32 voltage_tries;
+	u32 cr_tries;
+	u32 max_cr;
+	int ret;
+
+	/*
+	 * DP 1.4 spec define 10 for maxtries value, for pre DP 1.4 version set a limit of 80
+	 * (4 voltage levels x 4 preemphasis levels x 5 identical voltage retries)
+	 */
+	max_cr = dp->link.cap.rx_dpcd_revision >= DP_DPCD_REV_14 ? 10 : 80;
+
+	voltage_tries = 1;
+	for (cr_tries = 0; cr_tries < max_cr; cr_tries++) {
+		drm_dp_link_train_clock_recovery_delay(&dp->aux, dp->dpcd);
+
+		ret = drm_dp_dpcd_read_link_status(&dp->aux, lane_status);
+		if (ret != DP_LINK_STATUS_SIZE) {
+			drm_err(dp->dev, "Get lane status failed\n");
+			return ret;
+		}
+
+		if (drm_dp_clock_recovery_ok(lane_status, dp->link.cap.lanes)) {
+			drm_info(dp->dev, "dp link training cr done\n");
+			dp->link.status.clock_recovered = true;
+			return 0;
+		}
+
+		if (voltage_tries == 5) {
+			drm_info(dp->dev, "same voltage tries 5 times\n");
+			dp->link.status.clock_recovered = false;
+			return 0;
+		}
+
+		level_changed = hibmc_dp_link_get_adjust_train(dp, lane_status);
+		ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, dp->link.train_set,
+					dp->link.cap.lanes);
+		if (ret != dp->link.cap.lanes) {
+			drm_dbg_dp(dp->dev, "Update link training failed\n");
+			return ret >= 0 ? -EIO : ret;
+		}
+
+		voltage_tries = level_changed ? 1 : voltage_tries + 1;
+	}
+
+	drm_err(dp->dev, "dp link training clock recovery %u timers failed\n", max_cr);
+	dp->link.status.clock_recovered = false;
+
+	return 0;
+}
+
+static int hibmc_dp_link_training_channel_eq(struct hibmc_dp_dev *dp)
+{
+	u8 lane_status[DP_LINK_STATUS_SIZE] = {0};
+	u8 eq_tries;
+	int tps;
+	int ret;
+
+	if (dp->link.cap.is_tps4)
+		tps = DP_TRAINING_PATTERN_4;
+	else if (dp->link.cap.is_tps3)
+		tps = DP_TRAINING_PATTERN_3;
+	else
+		tps = DP_TRAINING_PATTERN_2;
+
+	ret = hibmc_dp_link_set_pattern(dp, tps);
+	if (ret)
+		return ret;
+
+	for (eq_tries = 0; eq_tries < HIBMC_EQ_MAX_RETRY; eq_tries++) {
+		drm_dp_link_train_channel_eq_delay(&dp->aux, dp->dpcd);
+
+		ret = drm_dp_dpcd_read_link_status(&dp->aux, lane_status);
+		if (ret != DP_LINK_STATUS_SIZE) {
+			drm_err(dp->dev, "get lane status failed\n");
+			break;
+		}
+
+		if (!drm_dp_clock_recovery_ok(lane_status, dp->link.cap.lanes)) {
+			drm_info(dp->dev, "clock recovery check failed\n");
+			drm_info(dp->dev, "cannot continue channel equalization\n");
+			dp->link.status.clock_recovered = false;
+			break;
+		}
+
+		if (drm_dp_channel_eq_ok(lane_status, dp->link.cap.lanes)) {
+			dp->link.status.channel_equalized = true;
+			drm_info(dp->dev, "dp link training eq done\n");
+			break;
+		}
+
+		hibmc_dp_link_get_adjust_train(dp, lane_status);
+		ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
+					dp->link.train_set, dp->link.cap.lanes);
+		if (ret != dp->link.cap.lanes) {
+			drm_dbg_dp(dp->dev, "Update link training failed\n");
+			ret = (ret >= 0) ? -EIO : ret;
+			break;
+		}
+	}
+
+	if (eq_tries == HIBMC_EQ_MAX_RETRY)
+		drm_err(dp->dev, "channel equalization failed %u times\n", eq_tries);
+
+	hibmc_dp_link_set_pattern(dp, DP_TRAINING_PATTERN_DISABLE);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int hibmc_dp_link_downgrade_training_cr(struct hibmc_dp_dev *dp)
+{
+	if (hibmc_dp_link_reduce_rate(dp))
+		return hibmc_dp_link_reduce_lane(dp);
+
+	return 0;
+}
+
+static int hibmc_dp_link_downgrade_training_eq(struct hibmc_dp_dev *dp)
+{
+	if ((dp->link.status.clock_recovered && !dp->link.status.channel_equalized)) {
+		if (!hibmc_dp_link_reduce_lane(dp))
+			return 0;
+	}
+
+	return hibmc_dp_link_reduce_rate(dp);
+}
+
+int hibmc_dp_link_training(struct hibmc_dp_dev *dp)
+{
+	struct hibmc_dp_link *link = &dp->link;
+	int ret;
+
+	while (true) {
+		ret = hibmc_dp_link_training_cr_pre(dp);
+		if (ret)
+			goto err;
+
+		ret = hibmc_dp_link_training_cr(dp);
+		if (ret)
+			goto err;
+
+		if (!link->status.clock_recovered) {
+			ret = hibmc_dp_link_downgrade_training_cr(dp);
+			if (ret)
+				goto err;
+			continue;
+		}
+
+		ret = hibmc_dp_link_training_channel_eq(dp);
+		if (ret)
+			goto err;
+
+		if (!link->status.channel_equalized) {
+			ret = hibmc_dp_link_downgrade_training_eq(dp);
+			if (ret)
+				goto err;
+			continue;
+		}
+
+		return 0;
+	}
+
+err:
+	hibmc_dp_link_set_pattern(dp, DP_TRAINING_PATTERN_DISABLE);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index f3e6781e111a..0bd308eccdc5 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -12,16 +12,24 @@
 #define HIBMC_DP_AUX_RD_DATA0			0x64
 #define HIBMC_DP_AUX_REQ			0x74
 #define HIBMC_DP_AUX_STATUS			0x78
+#define HIBMC_DP_PHYIF_CTRL0			0xa0
+#define HIBMC_DP_VIDEO_CTRL			0x100
 #define HIBMC_DP_DPTX_RST_CTRL			0x700
+#define HIBMC_DP_DPTX_GCTL0			0x708
 
 #define HIBMC_DP_CFG_AUX_SYNC_LEN_SEL		BIT(1)
 #define HIBMC_DP_CFG_AUX_TIMER_TIMEOUT		BIT(2)
+#define HIBMC_DP_CFG_STREAM_FRAME_MODE		BIT(6)
 #define HIBMC_DP_CFG_AUX_MIN_PULSE_NUM		GENMASK(13, 9)
+#define HIBMC_DP_CFG_LANE_DATA_EN		GENMASK(11, 8)
+#define HIBMC_DP_CFG_PHY_LANE_NUM		GENMASK(2, 1)
 #define HIBMC_DP_CFG_AUX_REQ			BIT(0)
 #define HIBMC_DP_CFG_AUX_RST_N			BIT(4)
 #define HIBMC_DP_CFG_AUX_TIMEOUT		BIT(0)
 #define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
 #define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
 #define HIBMC_DP_CFG_AUX_STATUS			GENMASK(11, 4)
+#define HIBMC_DP_CFG_SCRAMBLE_EN		BIT(0)
+#define HIBMC_DP_CFG_PAT_SEL			GENMASK(7, 4)
 
 #endif
-- 
2.33.0

[PATCH v5 drm-dp 3/5] drm/hisilicon/hibmc: add dp hw moduel in hibmc

[Yongbang Shi]

From: baihan li <libaihan@huawei.com>

Build a dp level that hibmc driver can enable dp by
calling their functions.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v3 -> v4:
  - changed the type of train_set to array, suggested by Dmitry Baryshkov.
  - using actual link rate instead of magic num, suggested by Dmitry Baryshkov.
  - deleting hibmc_dp_hw_uninit(), suggested by Dmitry Baryshkov.
v2 -> v3:
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202410250931.UDQ9s66H-lkp@intel.com/
v1 -> v2:
  - changed some defines and functions to former patch, suggested by Dmitry Baryshkov.
  - sorting the headers including in dp_hw.h and hibmc_drm_drv.c files, suggested by Dmitry Baryshkov.
  - deleting struct dp_mode and dp_mode_cfg function, suggested by Dmitry Baryshkov.
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202410040328.VeVxM9yB-lkp@intel.com/
  v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
---
 drivers/gpu/drm/hisilicon/hibmc/Makefile      |   2 +-
 .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  19 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    | 217 ++++++++++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  28 +++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |  41 ++++
 5 files changed, 306 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 94d77da88bbf..214228052ccf 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_aux.o dp/dp_link.o dp/dp_hw.o
 
 obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
new file mode 100644
index 000000000000..74dd9956144e
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef DP_CONFIG_H
+#define DP_CONFIG_H
+
+#define HIBMC_DP_BPP			24
+#define HIBMC_DP_SYMBOL_PER_FCLK	4
+#define HIBMC_DP_MSA1			0x20
+#define HIBMC_DP_MSA2			0x845c00
+#define HIBMC_DP_OFFSET			0x1e0000
+#define HIBMC_DP_HDCP			0x2
+#define HIBMC_DP_INT_RST		0xffff
+#define HIBMC_DP_DPTX_RST		0x3ff
+#define HIBMC_DP_CLK_EN			0x7
+#define HIBMC_DP_SYNC_EN_MASK		0x3
+#define HIBMC_DP_LINK_RATE_CAL		27
+
+#endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
new file mode 100644
index 000000000000..9d7337cd9309
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -0,0 +1,217 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/io.h>
+#include <linux/delay.h>
+#include "dp_config.h"
+#include "dp_comm.h"
+#include "dp_reg.h"
+#include "dp_hw.h"
+
+static void hibmc_dp_set_tu(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
+{
+	u32 tu_symbol_frac_size;
+	u32 tu_symbol_size;
+	u32 rate_ks;
+	u8 lane_num;
+	u32 value;
+	u32 bpp;
+
+	lane_num = dp->link.cap.lanes;
+	if (lane_num == 0) {
+		drm_err(dp->dev, "set tu failed, lane num cannot be 0!\n");
+		return;
+	}
+
+	bpp = HIBMC_DP_BPP;
+	rate_ks = dp->link.cap.link_rate * HIBMC_DP_LINK_RATE_CAL;
+	value = (mode->clock * bpp * 5) / (61 * lane_num * rate_ks);
+
+	if (value % 10 == 9) { /* 9 carry */
+		tu_symbol_size = value / 10 + 1;
+		tu_symbol_frac_size = 0;
+	} else {
+		tu_symbol_size = value / 10;
+		tu_symbol_frac_size = value % 10 + 1;
+	}
+
+	drm_info(dp->dev, "tu value: %u.%u value: %u\n",
+		 tu_symbol_size, tu_symbol_frac_size, value);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_PACKET,
+				 HIBMC_DP_CFG_STREAM_TU_SYMBOL_SIZE, tu_symbol_size);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_PACKET,
+				 HIBMC_DP_CFG_STREAM_TU_SYMBOL_FRAC_SIZE, tu_symbol_frac_size);
+}
+
+static void hibmc_dp_set_sst(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
+{
+	u32 hblank_size;
+	u32 htotal_size;
+	u32 htotal_int;
+	u32 hblank_int;
+	u32 fclk; /* flink_clock */
+
+	fclk = dp->link.cap.link_rate * HIBMC_DP_LINK_RATE_CAL;
+
+	htotal_int = mode->htotal * 9947 / 10000;
+	htotal_size = htotal_int * fclk / (HIBMC_DP_SYMBOL_PER_FCLK * (mode->clock / 1000));
+
+	hblank_int = mode->htotal - mode->hdisplay - mode->hdisplay * 53 / 10000;
+	hblank_size = hblank_int * fclk * 9947 /
+		      (mode->clock * 10 * HIBMC_DP_SYMBOL_PER_FCLK);
+
+	drm_info(dp->dev, "h_active %u v_active %u htotal_size %u hblank_size %u",
+		 mode->hdisplay, mode->vdisplay, htotal_size, hblank_size);
+	drm_info(dp->dev, "flink_clock %u pixel_clock %d", fclk, mode->clock / 1000);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_HORIZONTAL_SIZE,
+				 HIBMC_DP_CFG_STREAM_HTOTAL_SIZE, htotal_size);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_HORIZONTAL_SIZE,
+				 HIBMC_DP_CFG_STREAM_HBLANK_SIZE, hblank_size);
+}
+
+static void hibmc_dp_link_cfg(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
+{
+	u32 timing_delay;
+	u32 vblank;
+	u32 hstart;
+	u32 vstart;
+
+	vblank = mode->vtotal - mode->vdisplay;
+	timing_delay = mode->htotal - mode->hsync_start;
+	hstart = mode->htotal - mode->hsync_start;
+	vstart = mode->vtotal - mode->vsync_start;
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_TIMING_GEN_CONFIG0,
+				 HIBMC_DP_CFG_TIMING_GEN0_HBLANK, mode->htotal - mode->hdisplay);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_TIMING_GEN_CONFIG0,
+				 HIBMC_DP_CFG_TIMING_GEN0_HACTIVE, mode->hdisplay);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_TIMING_GEN_CONFIG2,
+				 HIBMC_DP_CFG_TIMING_GEN0_VBLANK, vblank);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_TIMING_GEN_CONFIG2,
+				 HIBMC_DP_CFG_TIMING_GEN0_VACTIVE, mode->vdisplay);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_TIMING_GEN_CONFIG3,
+				 HIBMC_DP_CFG_TIMING_GEN0_VFRONT_PORCH,
+				 mode->vsync_start - mode->vdisplay);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CONFIG0,
+				 HIBMC_DP_CFG_STREAM_HACTIVE, mode->hdisplay);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CONFIG0,
+				 HIBMC_DP_CFG_STREAM_HBLANK, mode->htotal - mode->hdisplay);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CONFIG2,
+				 HIBMC_DP_CFG_STREAM_HSYNC_WIDTH,
+				 mode->hsync_end - mode->hsync_start);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CONFIG1,
+				 HIBMC_DP_CFG_STREAM_VACTIVE, mode->vdisplay);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CONFIG1,
+				 HIBMC_DP_CFG_STREAM_VBLANK, vblank);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CONFIG3,
+				 HIBMC_DP_CFG_STREAM_VFRONT_PORCH,
+				 mode->vsync_start - mode->vdisplay);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CONFIG3,
+				 HIBMC_DP_CFG_STREAM_VSYNC_WIDTH,
+				 mode->vsync_end - mode->vsync_start);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_MSA0,
+				 HIBMC_DP_CFG_STREAM_VSTART, vstart);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_MSA0,
+				 HIBMC_DP_CFG_STREAM_HSTART, hstart);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CTRL, HIBMC_DP_CFG_STREAM_VSYNC_POLARITY,
+				 mode->flags & DRM_MODE_FLAG_PVSYNC ? 1 : 0);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CTRL, HIBMC_DP_CFG_STREAM_HSYNC_POLARITY,
+				 mode->flags & DRM_MODE_FLAG_PHSYNC ? 1 : 0);
+
+	/* MSA mic 0 and 1 */
+	writel(HIBMC_DP_MSA1, dp->base + HIBMC_DP_VIDEO_MSA1);
+	writel(HIBMC_DP_MSA2, dp->base + HIBMC_DP_VIDEO_MSA2);
+
+	hibmc_dp_set_tu(dp, mode);
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CTRL, HIBMC_DP_CFG_STREAM_RGB_ENABLE, 0x1);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_CTRL, HIBMC_DP_CFG_STREAM_VIDEO_MAPPING, 0);
+
+	/* divide 2: up even */
+	if (timing_delay % 2)
+		timing_delay++;
+
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_TIMING_MODEL_CTRL,
+				 HIBMC_DP_CFG_PIXEL_NUM_TIMING_MODE_SEL1, timing_delay);
+
+	hibmc_dp_set_sst(dp, mode);
+}
+
+int hibmc_dp_hw_init(struct hibmc_dp *dp)
+{
+	struct drm_device *drm_dev = dp->drm_dev;
+	struct hibmc_dp_dev *dp_dev;
+
+	dp_dev = devm_kzalloc(drm_dev->dev, sizeof(struct hibmc_dp_dev), GFP_KERNEL);
+	if (!dp_dev)
+		return -ENOMEM;
+
+	mutex_init(&dp_dev->lock);
+
+	dp->dp_dev = dp_dev;
+
+	dp_dev->dev = drm_dev;
+	dp_dev->base = dp->mmio + HIBMC_DP_OFFSET;
+
+	hibmc_dp_aux_init(dp_dev);
+
+	dp_dev->link.cap.lanes = 0x2;
+	dp_dev->link.cap.link_rate = DP_LINK_BW_2_7;
+
+	/* hdcp data */
+	writel(HIBMC_DP_HDCP, dp_dev->base + HIBMC_DP_HDCP_CFG);
+	/* int init */
+	writel(0, dp_dev->base + HIBMC_DP_INTR_ENABLE);
+	writel(HIBMC_DP_INT_RST, dp_dev->base + HIBMC_DP_INTR_ORIGINAL_STATUS);
+	/* rst */
+	writel(HIBMC_DP_DPTX_RST, dp_dev->base + HIBMC_DP_DPTX_RST_CTRL);
+	/* clock enable */
+	writel(HIBMC_DP_CLK_EN, dp_dev->base + HIBMC_DP_DPTX_CLK_CTRL);
+
+	return 0;
+}
+
+void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable)
+{
+	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
+
+	if (enable) {
+		hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_VIDEO_CTRL, BIT(0), 0x1);
+		writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL);
+		hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_DPTX_GCTL0, BIT(10), 0x1);
+		writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL);
+	} else {
+		hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_DPTX_GCTL0, BIT(10), 0);
+		writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL);
+		hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_VIDEO_CTRL, BIT(0), 0);
+		writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL);
+	}
+
+	msleep(50);
+}
+
+int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode)
+{
+	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
+	int ret;
+
+	if (!dp_dev->link.status.channel_equalized) {
+		ret = hibmc_dp_link_training(dp_dev);
+		if (ret) {
+			drm_err(dp->drm_dev, "dp link training failed, ret: %d\n", ret);
+			return ret;
+		}
+	}
+
+	hibmc_dp_display_en(dp, false);
+	hibmc_dp_link_cfg(dp_dev, mode);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
new file mode 100644
index 000000000000..4dc13b3d9875
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (c) 2024 Hisilicon Limited. */
+
+#ifndef DP_KAPI_H
+#define DP_KAPI_H
+
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <drm/drm_device.h>
+#include <drm/drm_encoder.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_print.h>
+
+struct hibmc_dp_dev;
+
+struct hibmc_dp {
+	struct hibmc_dp_dev *dp_dev;
+	struct drm_device *drm_dev;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
+	void __iomem *mmio;
+};
+
+int hibmc_dp_hw_init(struct hibmc_dp *dp);
+int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode);
+void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable);
+
+#endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index 0bd308eccdc5..4a515c726d52 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -14,8 +14,26 @@
 #define HIBMC_DP_AUX_STATUS			0x78
 #define HIBMC_DP_PHYIF_CTRL0			0xa0
 #define HIBMC_DP_VIDEO_CTRL			0x100
+#define HIBMC_DP_VIDEO_CONFIG0			0x104
+#define HIBMC_DP_VIDEO_CONFIG1			0x108
+#define HIBMC_DP_VIDEO_CONFIG2			0x10c
+#define HIBMC_DP_VIDEO_CONFIG3			0x110
+#define HIBMC_DP_VIDEO_PACKET			0x114
+#define HIBMC_DP_VIDEO_MSA0			0x118
+#define HIBMC_DP_VIDEO_MSA1			0x11c
+#define HIBMC_DP_VIDEO_MSA2			0x120
+#define HIBMC_DP_VIDEO_HORIZONTAL_SIZE		0X124
+#define HIBMC_DP_TIMING_GEN_CONFIG0		0x26c
+#define HIBMC_DP_TIMING_GEN_CONFIG2		0x274
+#define HIBMC_DP_TIMING_GEN_CONFIG3		0x278
+#define HIBMC_DP_HDCP_CFG			0x600
 #define HIBMC_DP_DPTX_RST_CTRL			0x700
+#define HIBMC_DP_DPTX_CLK_CTRL			0x704
 #define HIBMC_DP_DPTX_GCTL0			0x708
+#define HIBMC_DP_INTR_ENABLE			0x720
+#define HIBMC_DP_INTR_ORIGINAL_STATUS		0x728
+#define HIBMC_DP_TIMING_MODEL_CTRL		0x884
+#define HIBMC_DP_TIMING_SYNC_CTRL		0xFF0
 
 #define HIBMC_DP_CFG_AUX_SYNC_LEN_SEL		BIT(1)
 #define HIBMC_DP_CFG_AUX_TIMER_TIMEOUT		BIT(2)
@@ -31,5 +49,28 @@
 #define HIBMC_DP_CFG_AUX_STATUS			GENMASK(11, 4)
 #define HIBMC_DP_CFG_SCRAMBLE_EN		BIT(0)
 #define HIBMC_DP_CFG_PAT_SEL			GENMASK(7, 4)
+#define HIBMC_DP_CFG_TIMING_GEN0_HACTIVE	GENMASK(31, 16)
+#define HIBMC_DP_CFG_TIMING_GEN0_HBLANK		GENMASK(15, 0)
+#define HIBMC_DP_CFG_TIMING_GEN0_VACTIVE	GENMASK(31, 16)
+#define HIBMC_DP_CFG_TIMING_GEN0_VBLANK		GENMASK(15, 0)
+#define HIBMC_DP_CFG_TIMING_GEN0_VFRONT_PORCH	GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_HACTIVE		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_HBLANK		GENMASK(15, 0)
+#define HIBMC_DP_CFG_STREAM_HSYNC_WIDTH		GENMASK(15, 0)
+#define HIBMC_DP_CFG_STREAM_VACTIVE		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_VBLANK		GENMASK(15, 0)
+#define HIBMC_DP_CFG_STREAM_VFRONT_PORCH	GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_VSYNC_WIDTH		GENMASK(15, 0)
+#define HIBMC_DP_CFG_STREAM_VSTART		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_HSTART		GENMASK(15, 0)
+#define HIBMC_DP_CFG_STREAM_VSYNC_POLARITY	BIT(8)
+#define HIBMC_DP_CFG_STREAM_HSYNC_POLARITY	BIT(7)
+#define HIBMC_DP_CFG_STREAM_RGB_ENABLE		BIT(1)
+#define HIBMC_DP_CFG_STREAM_VIDEO_MAPPING	GENMASK(5, 2)
+#define HIBMC_DP_CFG_PIXEL_NUM_TIMING_MODE_SEL1	GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_TU_SYMBOL_SIZE	GENMASK(5, 0)
+#define HIBMC_DP_CFG_STREAM_TU_SYMBOL_FRAC_SIZE	GENMASK(9, 6)
+#define HIBMC_DP_CFG_STREAM_HTOTAL_SIZE		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_HBLANK_SIZE		GENMASK(15, 0)
 
 #endif
-- 
2.33.0

[PATCH v5 drm-dp 4/5] drm/hisilicon/hibmc: separate struct of vdac

[Yongbang Shi]

From: baihan li <libaihan@huawei.com>

Refactored struct hibmc_drm_private to separate VGA module from
generic struct.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v3 -> v4:
  - separating hibmc_vdac and hibmc_dp changes into separate patche, suggested by Dmitry Baryshkov.
v2 -> v3:
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202410251136.1m7BlR68-lkp@intel.com/
v1 -> v2:
  - deleting struct dp_mode and dp_mode_cfg function, suggested by Dmitry Baryshkov.
  - modifying drm_simple_encoder_init function, suggested by Dmitry Baryshkov.
  - refactoring struct hibmc_connector, suggested by Dmitry Baryshkov.
  - withdrawing the modification in hibmc_kms_init, suggested by Dmitry Baryshkov.
  v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   | 16 ++++----
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c   | 41 +++++++++----------
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 20 ++++-----
 3 files changed, 38 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 6b566f3aeecb..42f0ab8f9b5a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -20,9 +20,10 @@
 
 #include <drm/drm_framebuffer.h>
 
-struct hibmc_connector {
-	struct drm_connector base;
-
+struct hibmc_vdac {
+	struct drm_device *dev;
+	struct drm_encoder encoder;
+	struct drm_connector connector;
 	struct i2c_adapter adapter;
 	struct i2c_algo_bit_data bit_data;
 };
@@ -35,13 +36,12 @@ struct hibmc_drm_private {
 	struct drm_device dev;
 	struct drm_plane primary_plane;
 	struct drm_crtc crtc;
-	struct drm_encoder encoder;
-	struct hibmc_connector connector;
+	struct hibmc_vdac vdac;
 };
 
-static inline struct hibmc_connector *to_hibmc_connector(struct drm_connector *connector)
+static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
 {
-	return container_of(connector, struct hibmc_connector, base);
+	return container_of(connector, struct hibmc_vdac, connector);
 }
 
 static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev)
@@ -57,6 +57,6 @@ void hibmc_set_current_gate(struct hibmc_drm_private *priv,
 int hibmc_de_init(struct hibmc_drm_private *priv);
 int hibmc_vdac_init(struct hibmc_drm_private *priv);
 
-int hibmc_ddc_create(struct drm_device *drm_dev, struct hibmc_connector *connector);
+int hibmc_ddc_create(struct drm_device *drm_dev, struct hibmc_vdac *connector);
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
index e6e48651c15c..99b3b77b5445 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c
@@ -25,8 +25,8 @@
 
 static void hibmc_set_i2c_signal(void *data, u32 mask, int value)
 {
-	struct hibmc_connector *hibmc_connector = data;
-	struct hibmc_drm_private *priv = to_hibmc_drm_private(hibmc_connector->base.dev);
+	struct hibmc_vdac *vdac = data;
+	struct hibmc_drm_private *priv = to_hibmc_drm_private(vdac->connector.dev);
 	u32 tmp_dir = readl(priv->mmio + GPIO_DATA_DIRECTION);
 
 	if (value) {
@@ -45,8 +45,8 @@ static void hibmc_set_i2c_signal(void *data, u32 mask, int value)
 
 static int hibmc_get_i2c_signal(void *data, u32 mask)
 {
-	struct hibmc_connector *hibmc_connector = data;
-	struct hibmc_drm_private *priv = to_hibmc_drm_private(hibmc_connector->base.dev);
+	struct hibmc_vdac *vdac = data;
+	struct hibmc_drm_private *priv = to_hibmc_drm_private(vdac->connector.dev);
 	u32 tmp_dir = readl(priv->mmio + GPIO_DATA_DIRECTION);
 
 	if ((tmp_dir & mask) != mask) {
@@ -77,22 +77,21 @@ static int hibmc_ddc_getscl(void *data)
 	return hibmc_get_i2c_signal(data, I2C_SCL_MASK);
 }
 
-int hibmc_ddc_create(struct drm_device *drm_dev,
-		     struct hibmc_connector *connector)
+int hibmc_ddc_create(struct drm_device *drm_dev, struct hibmc_vdac *vdac)
 {
-	connector->adapter.owner = THIS_MODULE;
-	snprintf(connector->adapter.name, I2C_NAME_SIZE, "HIS i2c bit bus");
-	connector->adapter.dev.parent = drm_dev->dev;
-	i2c_set_adapdata(&connector->adapter, connector);
-	connector->adapter.algo_data = &connector->bit_data;
-
-	connector->bit_data.udelay = 20;
-	connector->bit_data.timeout = usecs_to_jiffies(2000);
-	connector->bit_data.data = connector;
-	connector->bit_data.setsda = hibmc_ddc_setsda;
-	connector->bit_data.setscl = hibmc_ddc_setscl;
-	connector->bit_data.getsda = hibmc_ddc_getsda;
-	connector->bit_data.getscl = hibmc_ddc_getscl;
-
-	return i2c_bit_add_bus(&connector->adapter);
+	vdac->adapter.owner = THIS_MODULE;
+	snprintf(vdac->adapter.name, I2C_NAME_SIZE, "HIS i2c bit bus");
+	vdac->adapter.dev.parent = drm_dev->dev;
+	i2c_set_adapdata(&vdac->adapter, vdac);
+	vdac->adapter.algo_data = &vdac->bit_data;
+
+	vdac->bit_data.udelay = 20;
+	vdac->bit_data.timeout = usecs_to_jiffies(2000);
+	vdac->bit_data.data = vdac;
+	vdac->bit_data.setsda = hibmc_ddc_setsda;
+	vdac->bit_data.setscl = hibmc_ddc_setscl;
+	vdac->bit_data.getsda = hibmc_ddc_getsda;
+	vdac->bit_data.getscl = hibmc_ddc_getscl;
+
+	return i2c_bit_add_bus(&vdac->adapter);
 }
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 409c551c92af..05e19ea4c9f9 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -24,11 +24,11 @@
 
 static int hibmc_connector_get_modes(struct drm_connector *connector)
 {
-	struct hibmc_connector *hibmc_connector = to_hibmc_connector(connector);
+	struct hibmc_vdac *vdac = to_hibmc_vdac(connector);
 	const struct drm_edid *drm_edid;
 	int count;
 
-	drm_edid = drm_edid_read_ddc(connector, &hibmc_connector->adapter);
+	drm_edid = drm_edid_read_ddc(connector, &vdac->adapter);
 
 	drm_edid_connector_update(connector, drm_edid);
 
@@ -51,9 +51,9 @@ static int hibmc_connector_get_modes(struct drm_connector *connector)
 
 static void hibmc_connector_destroy(struct drm_connector *connector)
 {
-	struct hibmc_connector *hibmc_connector = to_hibmc_connector(connector);
+	struct hibmc_vdac *vdac = to_hibmc_vdac(connector);
 
-	i2c_del_adapter(&hibmc_connector->adapter);
+	i2c_del_adapter(&vdac->adapter);
 	drm_connector_cleanup(connector);
 }
 
@@ -93,20 +93,20 @@ static const struct drm_encoder_helper_funcs hibmc_encoder_helper_funcs = {
 int hibmc_vdac_init(struct hibmc_drm_private *priv)
 {
 	struct drm_device *dev = &priv->dev;
-	struct hibmc_connector *hibmc_connector = &priv->connector;
-	struct drm_encoder *encoder = &priv->encoder;
+	struct hibmc_vdac *vdac = &priv->vdac;
+	struct drm_encoder *encoder = &vdac->encoder;
 	struct drm_crtc *crtc = &priv->crtc;
-	struct drm_connector *connector = &hibmc_connector->base;
+	struct drm_connector *connector = &vdac->connector;
 	int ret;
 
-	ret = hibmc_ddc_create(dev, hibmc_connector);
+	ret = hibmc_ddc_create(dev, vdac);
 	if (ret) {
 		drm_err(dev, "failed to create ddc: %d\n", ret);
 		return ret;
 	}
 
 	encoder->possible_crtcs = drm_crtc_mask(crtc);
-	ret = drm_simple_encoder_init(dev, encoder, DRM_MODE_ENCODER_DAC);
+	ret = drmm_encoder_init(dev, encoder, NULL, DRM_MODE_ENCODER_DAC, NULL);
 	if (ret) {
 		drm_err(dev, "failed to init encoder: %d\n", ret);
 		return ret;
@@ -117,7 +117,7 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 	ret = drm_connector_init_with_ddc(dev, connector,
 					  &hibmc_connector_funcs,
 					  DRM_MODE_CONNECTOR_VGA,
-					  &hibmc_connector->adapter);
+					  &vdac->adapter);
 	if (ret) {
 		drm_err(dev, "failed to init connector: %d\n", ret);
 		return ret;
-- 
2.33.0

[PATCH v5 drm-dp 5/5] drm/hisilicon/hibmc: add dp module in hibmc

[Yongbang Shi]

From: baihan li <libaihan@huawei.com>

To support DP interface displaying in hibmc driver. Add
a encoder and connector for DP modual.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v3 -> v4:
  - static inline hibmc_dp_prepare(), suggested by Dmitry Baryshkov.
---
 drivers/gpu/drm/hisilicon/hibmc/Makefile      |   2 +-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 118 ++++++++++++++++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  12 ++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |   5 +
 4 files changed, 136 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 214228052ccf..95a4ed599d98 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
+	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o hibmc_drm_dp.o
 
 obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
new file mode 100644
index 000000000000..603d6b198a54
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/io.h>
+
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_modes.h>
+#include <drm/drm_drv.h>
+#include <drm/drm_edid.h>
+
+#include "hibmc_drm_drv.h"
+#include "dp/dp_hw.h"
+
+static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
+{
+	int count;
+
+	count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width,
+				     connector->dev->mode_config.max_height);
+	drm_set_preferred_mode(connector, 1024, 768); // temporary implementation
+
+	return count;
+}
+
+static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
+	.get_modes = hibmc_dp_connector_get_modes,
+};
+
+static const struct drm_connector_funcs hibmc_dp_conn_funcs = {
+	.reset = drm_atomic_helper_connector_reset,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = drm_connector_cleanup,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
+{
+	int ret;
+
+	hibmc_dp_display_en(dp, false);
+
+	ret = hibmc_dp_mode_set(dp, mode);
+	if (ret)
+		drm_err(dp->drm_dev, "hibmc dp mode set failed: %d\n", ret);
+
+	return ret;
+}
+
+static void hibmc_dp_encoder_enable(struct drm_encoder *drm_encoder,
+				    struct drm_atomic_state *state)
+{
+	struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder);
+	struct drm_display_mode *mode = &drm_encoder->crtc->state->mode;
+
+	if (hibmc_dp_prepare(dp, mode))
+		return;
+
+	hibmc_dp_display_en(dp, true);
+}
+
+static void hibmc_dp_encoder_disable(struct drm_encoder *drm_encoder,
+				     struct drm_atomic_state *state)
+{
+	struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder);
+
+	hibmc_dp_display_en(dp, false);
+}
+
+static const struct drm_encoder_helper_funcs hibmc_dp_encoder_helper_funcs = {
+	.atomic_enable = hibmc_dp_encoder_enable,
+	.atomic_disable = hibmc_dp_encoder_disable,
+};
+
+int hibmc_dp_init(struct hibmc_drm_private *priv)
+{
+	struct drm_device *dev = &priv->dev;
+	struct drm_crtc *crtc = &priv->crtc;
+	struct hibmc_dp *dp = &priv->dp;
+	struct drm_connector *connector = &dp->connector;
+	struct drm_encoder *encoder = &dp->encoder;
+	int ret;
+
+	dp->mmio = priv->mmio;
+	dp->drm_dev = dev;
+
+	ret = hibmc_dp_hw_init(&priv->dp);
+	if (ret) {
+		drm_err(dev, "hibmc dp hw init failed: %d\n", ret);
+		return ret;
+	}
+
+	hibmc_dp_display_en(&priv->dp, false);
+
+	encoder->possible_crtcs = drm_crtc_mask(crtc);
+	ret = drmm_encoder_init(dev, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
+	if (ret) {
+		drm_err(dev, "init dp encoder failed: %d\n", ret);
+		return ret;
+	}
+
+	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
+
+	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
+				 DRM_MODE_CONNECTOR_DisplayPort);
+	if (ret) {
+		drm_err(dev, "init dp connector failed: %d\n", ret);
+		return ret;
+	}
+
+	drm_connector_helper_add(connector, &hibmc_dp_conn_helper_funcs);
+
+	drm_connector_attach_encoder(connector, encoder);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 9f9b19ea0587..39fd8c5c8227 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -27,6 +27,10 @@
 #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
+
 DEFINE_DRM_GEM_FOPS(hibmc_fops);
 
 static irqreturn_t hibmc_interrupt(int irq, void *arg)
@@ -116,6 +120,14 @@ static int hibmc_kms_init(struct hibmc_drm_private *priv)
 		return ret;
 	}
 
+	/* 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 = hibmc_dp_init(priv);
+		if (ret)
+			drm_err(dev, "failed to init dp: %d\n", ret);
+	}
+
 	ret = hibmc_vdac_init(priv);
 	if (ret) {
 		drm_err(dev, "failed to init vdac: %d\n", ret);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 42f0ab8f9b5a..d982f1e4b958 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -20,6 +20,8 @@
 
 #include <drm/drm_framebuffer.h>
 
+#include "dp/dp_hw.h"
+
 struct hibmc_vdac {
 	struct drm_device *dev;
 	struct drm_encoder encoder;
@@ -37,6 +39,7 @@ struct hibmc_drm_private {
 	struct drm_plane primary_plane;
 	struct drm_crtc crtc;
 	struct hibmc_vdac vdac;
+	struct hibmc_dp dp;
 };
 
 static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
@@ -59,4 +62,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv);
 
 int hibmc_ddc_create(struct drm_device *drm_dev, struct hibmc_vdac *connector);
 
+int hibmc_dp_init(struct hibmc_drm_private *priv);
+
 #endif
-- 
2.33.0

Re: [PATCH v5 drm-dp 1/5] drm/hisilicon/hibmc: add dp aux in hibmc

[Dmitry Baryshkov]

On Mon, Nov 18, 2024 at 10:28:01PM +0800, Yongbang Shi wrote:
> From: baihan li <libaihan@huawei.com>
> 
> Add dp aux read/write functions. They are basic functions
> and will be used later.
> 
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> ---
> ChangeLog:
> v4 -> v5:
>   - fixing build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/
> v3 -> v4:
>   - retun error codes in  result incorrect branch, suggested by Dmitry Baryshkov.
>   - replacing all ret= with returns, suggested by Dmitry Baryshkov.
>   - moving the comment below the judgment statement, suggested by Dmitry Baryshkov.
>   - moving definations to the source file and clearing headers, suggested by Dmitry Baryshkov.
>   - reanaming dp_prefix to hibmc_dp_prefix, suggested by Dmitry Baryshkov.
>   - changing hibmc_dp_reg_write_field to static inline and lock, suggested by Dmitry Baryshkov.
>   - moving some structs to later patch, suggested by Dmitry Baryshkov.
> v2 -> v3:
>   - put the macro definations in latter patch where they are actually used, suggested by Dmitry Baryshkov.
>   - rename some macro definations to make them sensible, suggested by Dmitry Baryshkov.
>   - using FIELD_PREP and FIELD_GET, suggested by Dmitry Baryshkov.
>   - using DP_DPCD_REV_foo, suggested by Dmitry Baryshkov.
>   - fix build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202410250305.UHKDhtxy-lkp@intel.com/
> v1 -> v2:
>   - using drm_dp_aux frame implement dp aux read and write functions, suggested by Jani Nikula.
>   - using drm dp header files' dp macros instead, suggested by Andy Yan.
>   v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
> ---
>  drivers/gpu/drm/hisilicon/hibmc/Makefile     |   3 +-
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c  | 164 +++++++++++++++++++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h |  38 +++++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h  |  27 +++
>  4 files changed, 231 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index d25c75e60d3d..8770ec6dfffd 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> @@ -1,4 +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
> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \
> +	       dp/dp_aux.o
>  
>  obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> new file mode 100644
> index 000000000000..16bdfefbf255
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0-or-laterHIBMC_BYTES_IN_U32
> +// Copyright (c) 2024 Hisilicon Limited.
> +
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/minmax.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_print.h>
> +#include "dp_comm.h"
> +#include "dp_reg.h"
> +
> +#define HIBMC_AUX_CMD_REQ_LEN		GENMASK(7, 4)
> +#define HIBMC_AUX_CMD_ADDR		GENMASK(27, 8)
> +#define HIBMC_AUX_CMD_I2C_ADDR_ONLY	BIT(28)
> +#define HIBMC_BYTES_IN_U32		4
> +#define HIBMC_AUX_I2C_WRITE_SUCCESS	0x1
> +#define HIBMC_DP_MIN_PULSE_NUM		0x9
> +#define BITS_IN_U8			8
> +
> +static inline void hibmc_dp_aux_reset(struct hibmc_dp_dev *dp)
> +{
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x0);
> +	usleep_range(10, 15);
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x1);
> +}
> +
> +static void hibmc_dp_aux_read_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
> +{
> +	u32 reg_num;
> +	u32 value;
> +	u32 num;
> +	u8 i, j;
> +
> +	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
> +	for (i = 0; i < reg_num; i++) {
> +		/* number of bytes read from a single register */
> +		num = min(size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
> +		value = readl(dp->base + HIBMC_DP_AUX_RD_DATA0 + i * HIBMC_BYTES_IN_U32);
> +		/* convert the 32-bit value of the register to the buffer. */
> +		for (j = 0; j < num; j++)
> +			buf[i * HIBMC_BYTES_IN_U32 + j] = value >> (j * BITS_IN_U8);
> +	}
> +}
> +
> +static void hibmc_dp_aux_write_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
> +{
> +	u32 reg_num;
> +	u32 value;
> +	u32 num;
> +	u8 i, j;
> +
> +	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
> +	for (i = 0; i < reg_num; i++) {
> +		/* number of bytes written to a single register */
> +		num = min_t(u8, size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
> +		value = 0;
> +		/* obtain the 32-bit value written to a single register. */
> +		for (j = 0; j < num; j++)
> +			value |= buf[i * HIBMC_BYTES_IN_U32 + j] << (j * BITS_IN_U8);
> +		/* writing data to a single register */
> +		writel(value, dp->base + HIBMC_DP_AUX_WR_DATA0 + i * HIBMC_BYTES_IN_U32);
> +	}
> +}
> +
> +static u32 hibmc_dp_aux_build_cmd(const struct drm_dp_aux_msg *msg)
> +{
> +	u32 aux_cmd = msg->request;
> +
> +	if (msg->size)
> +		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_REQ_LEN, (msg->size - 1));
> +	else
> +		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_I2C_ADDR_ONLY, 1);
> +
> +	aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_ADDR, msg->address);
> +
> +	return aux_cmd;
> +}
> +
> +/* ret >= 0, ret is size; ret < 0, ret is err code */
> +static int hibmc_dp_aux_parse_xfer(struct hibmc_dp_dev *dp, struct drm_dp_aux_msg *msg)
> +{
> +	u32 buf_data_cnt;
> +	u32 aux_status;
> +
> +	aux_status = readl(dp->base + HIBMC_DP_AUX_STATUS);
> +	msg->reply = FIELD_GET(HIBMC_DP_CFG_AUX_STATUS, aux_status);
> +
> +	if (aux_status & HIBMC_DP_CFG_AUX_TIMEOUT)
> +		return -ETIMEDOUT;
> +
> +	/* only address */
> +	if (!msg->size)
> +		return 0;
> +
> +	if (msg->reply != DP_AUX_NATIVE_REPLY_ACK)
> +		return -EIO;
> +
> +	buf_data_cnt = FIELD_GET(HIBMC_DP_CFG_AUX_READY_DATA_BYTE, aux_status);
> +
> +	switch (msg->request) {
> +	case DP_AUX_NATIVE_WRITE:
> +		return msg->size;
> +	case DP_AUX_I2C_WRITE | DP_AUX_I2C_MOT:
> +		if (buf_data_cnt == HIBMC_AUX_I2C_WRITE_SUCCESS)
> +			return msg->size;
> +		else
> +			return FIELD_GET(HIBMC_DP_CFG_AUX, aux_status);
> +	case DP_AUX_NATIVE_READ:
> +	case DP_AUX_I2C_READ | DP_AUX_I2C_MOT:
> +		buf_data_cnt--;
> +		if (buf_data_cnt != msg->size) {
> +			/* only the successful part of data is read */
> +			return -EBUSY;
> +		}
> +
> +		/* all data is successfully read */
> +		hibmc_dp_aux_read_data(dp, msg->buffer, msg->size);
> +		return msg->size;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/* ret >= 0 ,ret is size; ret < 0, ret is err code */
> +static ssize_t hibmc_dp_aux_xfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> +{
> +	struct hibmc_dp_dev *dp = container_of(aux, struct hibmc_dp_dev, aux);
> +	u32 aux_cmd;
> +	int ret;
> +	u32 val; /* val will be assigned at the beginning of readl_poll_timeout function */
> +
> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA0);
> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA1);
> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA2);
> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA3);
> +
> +	hibmc_dp_aux_write_data(dp, msg->buffer, msg->size);
> +
> +	aux_cmd = hibmc_dp_aux_build_cmd(msg);
> +	writel(aux_cmd, dp->base + HIBMC_DP_AUX_CMD_ADDR);
> +
> +	/* enable aux transfer */
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_REQ, 0x1);
> +	ret = readl_poll_timeout(dp->base + HIBMC_DP_AUX_REQ, val,
> +				 !(val & HIBMC_DP_CFG_AUX_REQ), 50, 5000);
> +	if (ret) {
> +		hibmc_dp_aux_reset(dp);
> +		return ret;
> +	}
> +
> +	return hibmc_dp_aux_parse_xfer(dp, msg);
> +}
> +
> +void hibmc_dp_aux_init(struct hibmc_dp_dev *dp)
> +{
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_SYNC_LEN_SEL, 0x0);
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_TIMER_TIMEOUT, 0x1);
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_MIN_PULSE_NUM,
> +				 HIBMC_DP_MIN_PULSE_NUM);
> +
> +	dp->aux.transfer = hibmc_dp_aux_xfer;
> +	dp->aux.is_remote = 0;
> +	drm_dp_aux_init(&dp->aux);
> +}
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> new file mode 100644
> index 000000000000..ce3b6fa4ea9e
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (c) 2024 Hisilicon Limited. */
> +
> +#ifndef DP_COMM_H
> +#define DP_COMM_H
> +
> +#include <linux/types.h>
> +#include <linux/bitops.h>
> +#include <linux/errno.h>
> +#include <linux/mutex.h>
> +#include <linux/kernel.h>
> +#include <linux/bitfield.h>
> +#include <linux/io.h>
> +#include <drm/display/drm_dp_helper.h>
> +
> +struct hibmc_dp_dev {
> +	struct drm_dp_aux aux;
> +	struct drm_device *dev;
> +	void __iomem *base;
> +	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
> +};
> +
> +#define dp_field_modify(reg_value, mask, val) ({		\
> +	(reg_value) &= ~(mask);					\
> +	(reg_value) |= FIELD_PREP(mask, val); })

do { ... } while (0) or static inline. Or just inline into the calling
function, if there is just one place where it is used.

> +
> +#define hibmc_dp_reg_write_field(dp, offset, mask, val) ({	\
> +	typeof(dp) _dp = dp;					\
> +	typeof(_dp->base) addr = (_dp->base + (offset));	\
> +	mutex_lock(&_dp->lock);					\
> +	u32 reg_value = readl(addr);				\
> +	dp_field_modify(reg_value, mask, val);			\
> +	writel(reg_value, addr);				\
> +	mutex_unlock(&_dp->lock); })

I'd prefer a static inline function. Other than that:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> +
> +void hibmc_dp_aux_init(struct hibmc_dp_dev *dp);
> +
> +#endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> new file mode 100644
> index 000000000000..f3e6781e111a
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (c) 2024 Hisilicon Limited. */
> +
> +#ifndef DP_REG_H
> +#define DP_REG_H
> +
> +#define HIBMC_DP_AUX_CMD_ADDR			0x50
> +#define HIBMC_DP_AUX_WR_DATA0			0x54
> +#define HIBMC_DP_AUX_WR_DATA1			0x58
> +#define HIBMC_DP_AUX_WR_DATA2			0x5c
> +#define HIBMC_DP_AUX_WR_DATA3			0x60
> +#define HIBMC_DP_AUX_RD_DATA0			0x64
> +#define HIBMC_DP_AUX_REQ			0x74
> +#define HIBMC_DP_AUX_STATUS			0x78
> +#define HIBMC_DP_DPTX_RST_CTRL			0x700
> +
> +#define HIBMC_DP_CFG_AUX_SYNC_LEN_SEL		BIT(1)
> +#define HIBMC_DP_CFG_AUX_TIMER_TIMEOUT		BIT(2)
> +#define HIBMC_DP_CFG_AUX_MIN_PULSE_NUM		GENMASK(13, 9)
> +#define HIBMC_DP_CFG_AUX_REQ			BIT(0)
> +#define HIBMC_DP_CFG_AUX_RST_N			BIT(4)
> +#define HIBMC_DP_CFG_AUX_TIMEOUT		BIT(0)
> +#define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
> +#define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
> +#define HIBMC_DP_CFG_AUX_STATUS			GENMASK(11, 4)
> +
> +#endif
> -- 
> 2.33.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 drm-dp 2/5] drm/hisilicon/hibmc: add dp link moduel in hibmc

[Dmitry Baryshkov]

On Mon, Nov 18, 2024 at 10:28:02PM +0800, Yongbang Shi wrote:
> From: baihan li <libaihan@huawei.com>
> 
> Add link training process functions in this moduel.
> 
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> ---
> Changelog:
> v3 -> v4:
>   - optimizing hibmc_dp_link_get_adjust_train() to delete for loop, suggested by Dmitry Baryshkov.
>   - changing ELNRNG to EIO error code, suggested by Dmitry Baryshkov.
>   - deleting meaningless macro, suggested by Dmitry Baryshkov.
>   - fixing build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202411041559.WIfxRN6n-lkp@intel.com/
> v2 -> v3:
>   - using switchcase in dp_link_reduce_lane, suggested by Dmitry Baryshkov.
>   - deleting dp_link_pattern2dpcd function and using macros directly, suggested by Dmitry Baryshkov.
>   - deleting EFAULT error codes, suggested by Dmitry Baryshkov.
> v1 -> v2:
>   - using drm_dp_* functions implement dp link training process, suggested by Jani Nikula.
>   - fix build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202410031735.8iRZZR6T-lkp@intel.com/
>   v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
> ---
>  drivers/gpu/drm/hisilicon/hibmc/Makefile     |   2 +-
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h |  24 ++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c | 339 +++++++++++++++++++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h  |   8 +
>  4 files changed, 372 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v5 drm-dp 3/5] drm/hisilicon/hibmc: add dp hw moduel in hibmc

[Dmitry Baryshkov]

On Mon, Nov 18, 2024 at 10:28:03PM +0800, Yongbang Shi wrote:
> From: baihan li <libaihan@huawei.com>
> 
> Build a dp level that hibmc driver can enable dp by
> calling their functions.
> 
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> ---
> ChangeLog:
> v3 -> v4:
>   - changed the type of train_set to array, suggested by Dmitry Baryshkov.
>   - using actual link rate instead of magic num, suggested by Dmitry Baryshkov.
>   - deleting hibmc_dp_hw_uninit(), suggested by Dmitry Baryshkov.
> v2 -> v3:
>   - fix build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202410250931.UDQ9s66H-lkp@intel.com/
> v1 -> v2:
>   - changed some defines and functions to former patch, suggested by Dmitry Baryshkov.
>   - sorting the headers including in dp_hw.h and hibmc_drm_drv.c files, suggested by Dmitry Baryshkov.
>   - deleting struct dp_mode and dp_mode_cfg function, suggested by Dmitry Baryshkov.
>   - fix build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202410040328.VeVxM9yB-lkp@intel.com/
>   v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
> ---
>  drivers/gpu/drm/hisilicon/hibmc/Makefile      |   2 +-
>  .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  19 ++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    | 217 ++++++++++++++++++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  28 +++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |  41 ++++
>  5 files changed, 306 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index 94d77da88bbf..214228052ccf 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_aux.o dp/dp_link.o dp/dp_hw.o
>  
>  obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
> new file mode 100644
> index 000000000000..74dd9956144e
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/* Copyright (c) 2024 Hisilicon Limited. */
> +
> +#ifndef DP_CONFIG_H
> +#define DP_CONFIG_H
> +
> +#define HIBMC_DP_BPP			24
> +#define HIBMC_DP_SYMBOL_PER_FCLK	4
> +#define HIBMC_DP_MSA1			0x20
> +#define HIBMC_DP_MSA2			0x845c00
> +#define HIBMC_DP_OFFSET			0x1e0000
> +#define HIBMC_DP_HDCP			0x2
> +#define HIBMC_DP_INT_RST		0xffff
> +#define HIBMC_DP_DPTX_RST		0x3ff
> +#define HIBMC_DP_CLK_EN			0x7
> +#define HIBMC_DP_SYNC_EN_MASK		0x3
> +#define HIBMC_DP_LINK_RATE_CAL		27
> +
> +#endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> new file mode 100644
> index 000000000000..9d7337cd9309
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright (c) 2024 Hisilicon Limited.
> +
> +#include <linux/io.h>
> +#include <linux/delay.h>
> +#include "dp_config.h"
> +#include "dp_comm.h"
> +#include "dp_reg.h"
> +#include "dp_hw.h"
> +
> +static void hibmc_dp_set_tu(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
> +{
> +	u32 tu_symbol_frac_size;
> +	u32 tu_symbol_size;
> +	u32 rate_ks;
> +	u8 lane_num;
> +	u32 value;
> +	u32 bpp;
> +
> +	lane_num = dp->link.cap.lanes;
> +	if (lane_num == 0) {
> +		drm_err(dp->dev, "set tu failed, lane num cannot be 0!\n");
> +		return;
> +	}
> +
> +	bpp = HIBMC_DP_BPP;
> +	rate_ks = dp->link.cap.link_rate * HIBMC_DP_LINK_RATE_CAL;
> +	value = (mode->clock * bpp * 5) / (61 * lane_num * rate_ks);
> +
> +	if (value % 10 == 9) { /* 9 carry */
> +		tu_symbol_size = value / 10 + 1;
> +		tu_symbol_frac_size = 0;
> +	} else {
> +		tu_symbol_size = value / 10;
> +		tu_symbol_frac_size = value % 10 + 1;
> +	}
> +
> +	drm_info(dp->dev, "tu value: %u.%u value: %u\n",
> +		 tu_symbol_size, tu_symbol_frac_size, value);

drm_dbg_driver()

> +
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_PACKET,
> +				 HIBMC_DP_CFG_STREAM_TU_SYMBOL_SIZE, tu_symbol_size);
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_PACKET,
> +				 HIBMC_DP_CFG_STREAM_TU_SYMBOL_FRAC_SIZE, tu_symbol_frac_size);
> +}
> +
> +static void hibmc_dp_set_sst(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
> +{
> +	u32 hblank_size;
> +	u32 htotal_size;
> +	u32 htotal_int;
> +	u32 hblank_int;
> +	u32 fclk; /* flink_clock */
> +
> +	fclk = dp->link.cap.link_rate * HIBMC_DP_LINK_RATE_CAL;
> +
> +	htotal_int = mode->htotal * 9947 / 10000;
> +	htotal_size = htotal_int * fclk / (HIBMC_DP_SYMBOL_PER_FCLK * (mode->clock / 1000));
> +
> +	hblank_int = mode->htotal - mode->hdisplay - mode->hdisplay * 53 / 10000;
> +	hblank_size = hblank_int * fclk * 9947 /

Still no idea, what 0.9947 is.

> +		      (mode->clock * 10 * HIBMC_DP_SYMBOL_PER_FCLK);
> +
> +	drm_info(dp->dev, "h_active %u v_active %u htotal_size %u hblank_size %u",
> +		 mode->hdisplay, mode->vdisplay, htotal_size, hblank_size);
> +	drm_info(dp->dev, "flink_clock %u pixel_clock %d", fclk, mode->clock / 1000);

And here. Please review the driver not to output any debug information
by default.

> +
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_HORIZONTAL_SIZE,
> +				 HIBMC_DP_CFG_STREAM_HTOTAL_SIZE, htotal_size);
> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_HORIZONTAL_SIZE,
> +				 HIBMC_DP_CFG_STREAM_HBLANK_SIZE, hblank_size);
> +}
> +

Other than that LGTM

-- 
With best wishes
Dmitry

Re: [PATCH v5 drm-dp 4/5] drm/hisilicon/hibmc: separate struct of vdac

[Dmitry Baryshkov]

On Mon, Nov 18, 2024 at 10:28:04PM +0800, Yongbang Shi wrote:
> From: baihan li <libaihan@huawei.com>
> 
> Refactored struct hibmc_drm_private to separate VGA module from
> generic struct.
> 
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> ---
> ChangeLog:
> v3 -> v4:
>   - separating hibmc_vdac and hibmc_dp changes into separate patche, suggested by Dmitry Baryshkov.
> v2 -> v3:
>   - fix build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202410251136.1m7BlR68-lkp@intel.com/
> v1 -> v2:
>   - deleting struct dp_mode and dp_mode_cfg function, suggested by Dmitry Baryshkov.
>   - modifying drm_simple_encoder_init function, suggested by Dmitry Baryshkov.
>   - refactoring struct hibmc_connector, suggested by Dmitry Baryshkov.
>   - withdrawing the modification in hibmc_kms_init, suggested by Dmitry Baryshkov.
>   v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
> ---
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   | 16 ++++----
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_i2c.c   | 41 +++++++++----------
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  | 20 ++++-----
>  3 files changed, 38 insertions(+), 39 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

Re: [PATCH v5 drm-dp 5/5] drm/hisilicon/hibmc: add dp module in hibmc

[Dmitry Baryshkov]

On Mon, Nov 18, 2024 at 10:28:05PM +0800, Yongbang Shi wrote:
> From: baihan li <libaihan@huawei.com>
> 
> To support DP interface displaying in hibmc driver. Add
> a encoder and connector for DP modual.
> 
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> ---
> ChangeLog:
> v3 -> v4:
>   - static inline hibmc_dp_prepare(), suggested by Dmitry Baryshkov.
> ---
>  drivers/gpu/drm/hisilicon/hibmc/Makefile      |   2 +-
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 118 ++++++++++++++++++
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  12 ++
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |   5 +
>  4 files changed, 136 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> index 214228052ccf..95a4ed599d98 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
> +	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o hibmc_drm_dp.o
>  
>  obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> new file mode 100644
> index 000000000000..603d6b198a54
> --- /dev/null
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright (c) 2024 Hisilicon Limited.
> +
> +#include <linux/io.h>
> +
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_modes.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_edid.h>
> +
> +#include "hibmc_drm_drv.h"
> +#include "dp/dp_hw.h"
> +
> +static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
> +{
> +	int count;
> +
> +	count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width,
> +				     connector->dev->mode_config.max_height);
> +	drm_set_preferred_mode(connector, 1024, 768); // temporary implementation

Ideally this should be mentioned in the commit message.

> +
> +	return count;
> +}
> +
> +static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
> +	.get_modes = hibmc_dp_connector_get_modes,
> +};
> +
> +static const struct drm_connector_funcs hibmc_dp_conn_funcs = {
> +	.reset = drm_atomic_helper_connector_reset,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = drm_connector_cleanup,
> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,

No .detect / .detect_ctx ? No HPD support? Is it being targeted the DP
or eDP cases?

> +};
> +
> +static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
> +{
> +	int ret;
> +
> +	hibmc_dp_display_en(dp, false);
> +
> +	ret = hibmc_dp_mode_set(dp, mode);
> +	if (ret)
> +		drm_err(dp->drm_dev, "hibmc dp mode set failed: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static void hibmc_dp_encoder_enable(struct drm_encoder *drm_encoder,
> +				    struct drm_atomic_state *state)
> +{
> +	struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder);
> +	struct drm_display_mode *mode = &drm_encoder->crtc->state->mode;
> +
> +	if (hibmc_dp_prepare(dp, mode))
> +		return;
> +
> +	hibmc_dp_display_en(dp, true);
> +}
> +
> +static void hibmc_dp_encoder_disable(struct drm_encoder *drm_encoder,
> +				     struct drm_atomic_state *state)
> +{
> +	struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder);
> +
> +	hibmc_dp_display_en(dp, false);
> +}
> +
> +static const struct drm_encoder_helper_funcs hibmc_dp_encoder_helper_funcs = {
> +	.atomic_enable = hibmc_dp_encoder_enable,
> +	.atomic_disable = hibmc_dp_encoder_disable,
> +};
> +
> +int hibmc_dp_init(struct hibmc_drm_private *priv)
> +{
> +	struct drm_device *dev = &priv->dev;
> +	struct drm_crtc *crtc = &priv->crtc;
> +	struct hibmc_dp *dp = &priv->dp;
> +	struct drm_connector *connector = &dp->connector;
> +	struct drm_encoder *encoder = &dp->encoder;
> +	int ret;
> +
> +	dp->mmio = priv->mmio;
> +	dp->drm_dev = dev;
> +
> +	ret = hibmc_dp_hw_init(&priv->dp);
> +	if (ret) {
> +		drm_err(dev, "hibmc dp hw init failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	hibmc_dp_display_en(&priv->dp, false);
> +
> +	encoder->possible_crtcs = drm_crtc_mask(crtc);
> +	ret = drmm_encoder_init(dev, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
> +	if (ret) {
> +		drm_err(dev, "init dp encoder failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
> +
> +	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
> +				 DRM_MODE_CONNECTOR_DisplayPort);
> +	if (ret) {
> +		drm_err(dev, "init dp connector failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	drm_connector_helper_add(connector, &hibmc_dp_conn_helper_funcs);
> +
> +	drm_connector_attach_encoder(connector, encoder);
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> index 9f9b19ea0587..39fd8c5c8227 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
> @@ -27,6 +27,10 @@
>  #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
> +
>  DEFINE_DRM_GEM_FOPS(hibmc_fops);
>  
>  static irqreturn_t hibmc_interrupt(int irq, void *arg)
> @@ -116,6 +120,14 @@ static int hibmc_kms_init(struct hibmc_drm_private *priv)
>  		return ret;
>  	}
>  
> +	/* 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 = hibmc_dp_init(priv);
> +		if (ret)
> +			drm_err(dev, "failed to init dp: %d\n", ret);
> +	}
> +
>  	ret = hibmc_vdac_init(priv);
>  	if (ret) {
>  		drm_err(dev, "failed to init vdac: %d\n", ret);
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index 42f0ab8f9b5a..d982f1e4b958 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -20,6 +20,8 @@
>  
>  #include <drm/drm_framebuffer.h>
>  
> +#include "dp/dp_hw.h"
> +
>  struct hibmc_vdac {
>  	struct drm_device *dev;
>  	struct drm_encoder encoder;
> @@ -37,6 +39,7 @@ struct hibmc_drm_private {
>  	struct drm_plane primary_plane;
>  	struct drm_crtc crtc;
>  	struct hibmc_vdac vdac;
> +	struct hibmc_dp dp;
>  };
>  
>  static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
> @@ -59,4 +62,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv);
>  
>  int hibmc_ddc_create(struct drm_device *drm_dev, struct hibmc_vdac *connector);
>  
> +int hibmc_dp_init(struct hibmc_drm_private *priv);
> +
>  #endif
> -- 
> 2.33.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v5 drm-dp 1/5] drm/hisilicon/hibmc: add dp aux in hibmc

[Yongbang Shi]

在 2024/11/22 9:42, Dmitry Baryshkov 写道:
> On Mon, Nov 18, 2024 at 10:28:01PM +0800, Yongbang Shi wrote:
>> From: baihan li <libaihan@huawei.com>
>>
>> Add dp aux read/write functions. They are basic functions
>> and will be used later.
>>
>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>> ---
>> ChangeLog:
>> v4 -> v5:
>>    - fixing build errors reported by kernel test robot <lkp@intel.com>
>>      Closes: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/
>> v3 -> v4:
>>    - retun error codes in  result incorrect branch, suggested by Dmitry Baryshkov.
>>    - replacing all ret= with returns, suggested by Dmitry Baryshkov.
>>    - moving the comment below the judgment statement, suggested by Dmitry Baryshkov.
>>    - moving definations to the source file and clearing headers, suggested by Dmitry Baryshkov.
>>    - reanaming dp_prefix to hibmc_dp_prefix, suggested by Dmitry Baryshkov.
>>    - changing hibmc_dp_reg_write_field to static inline and lock, suggested by Dmitry Baryshkov.
>>    - moving some structs to later patch, suggested by Dmitry Baryshkov.
>> v2 -> v3:
>>    - put the macro definations in latter patch where they are actually used, suggested by Dmitry Baryshkov.
>>    - rename some macro definations to make them sensible, suggested by Dmitry Baryshkov.
>>    - using FIELD_PREP and FIELD_GET, suggested by Dmitry Baryshkov.
>>    - using DP_DPCD_REV_foo, suggested by Dmitry Baryshkov.
>>    - fix build errors reported by kernel test robot <lkp@intel.com>
>>      Closes: https://lore.kernel.org/oe-kbuild-all/202410250305.UHKDhtxy-lkp@intel.com/
>> v1 -> v2:
>>    - using drm_dp_aux frame implement dp aux read and write functions, suggested by Jani Nikula.
>>    - using drm dp header files' dp macros instead, suggested by Andy Yan.
>>    v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/Makefile     |   3 +-
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c  | 164 +++++++++++++++++++
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h |  38 +++++
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h  |  27 +++
>>   4 files changed, 231 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> index d25c75e60d3d..8770ec6dfffd 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> @@ -1,4 +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
>> +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \
>> +	       dp/dp_aux.o
>>   
>>   obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>> new file mode 100644
>> index 000000000000..16bdfefbf255
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>> @@ -0,0 +1,164 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-laterHIBMC_BYTES_IN_U32
>> +// Copyright (c) 2024 Hisilicon Limited.
>> +
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/minmax.h>
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_print.h>
>> +#include "dp_comm.h"
>> +#include "dp_reg.h"
>> +
>> +#define HIBMC_AUX_CMD_REQ_LEN		GENMASK(7, 4)
>> +#define HIBMC_AUX_CMD_ADDR		GENMASK(27, 8)
>> +#define HIBMC_AUX_CMD_I2C_ADDR_ONLY	BIT(28)
>> +#define HIBMC_BYTES_IN_U32		4
>> +#define HIBMC_AUX_I2C_WRITE_SUCCESS	0x1
>> +#define HIBMC_DP_MIN_PULSE_NUM		0x9
>> +#define BITS_IN_U8			8
>> +
>> +static inline void hibmc_dp_aux_reset(struct hibmc_dp_dev *dp)
>> +{
>> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x0);
>> +	usleep_range(10, 15);
>> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x1);
>> +}
>> +
>> +static void hibmc_dp_aux_read_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
>> +{
>> +	u32 reg_num;
>> +	u32 value;
>> +	u32 num;
>> +	u8 i, j;
>> +
>> +	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
>> +	for (i = 0; i < reg_num; i++) {
>> +		/* number of bytes read from a single register */
>> +		num = min(size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
>> +		value = readl(dp->base + HIBMC_DP_AUX_RD_DATA0 + i * HIBMC_BYTES_IN_U32);
>> +		/* convert the 32-bit value of the register to the buffer. */
>> +		for (j = 0; j < num; j++)
>> +			buf[i * HIBMC_BYTES_IN_U32 + j] = value >> (j * BITS_IN_U8);
>> +	}
>> +}
>> +
>> +static void hibmc_dp_aux_write_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
>> +{
>> +	u32 reg_num;
>> +	u32 value;
>> +	u32 num;
>> +	u8 i, j;
>> +
>> +	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
>> +	for (i = 0; i < reg_num; i++) {
>> +		/* number of bytes written to a single register */
>> +		num = min_t(u8, size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
>> +		value = 0;
>> +		/* obtain the 32-bit value written to a single register. */
>> +		for (j = 0; j < num; j++)
>> +			value |= buf[i * HIBMC_BYTES_IN_U32 + j] << (j * BITS_IN_U8);
>> +		/* writing data to a single register */
>> +		writel(value, dp->base + HIBMC_DP_AUX_WR_DATA0 + i * HIBMC_BYTES_IN_U32);
>> +	}
>> +}
>> +
>> +static u32 hibmc_dp_aux_build_cmd(const struct drm_dp_aux_msg *msg)
>> +{
>> +	u32 aux_cmd = msg->request;
>> +
>> +	if (msg->size)
>> +		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_REQ_LEN, (msg->size - 1));
>> +	else
>> +		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_I2C_ADDR_ONLY, 1);
>> +
>> +	aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_ADDR, msg->address);
>> +
>> +	return aux_cmd;
>> +}
>> +
>> +/* ret >= 0, ret is size; ret < 0, ret is err code */
>> +static int hibmc_dp_aux_parse_xfer(struct hibmc_dp_dev *dp, struct drm_dp_aux_msg *msg)
>> +{
>> +	u32 buf_data_cnt;
>> +	u32 aux_status;
>> +
>> +	aux_status = readl(dp->base + HIBMC_DP_AUX_STATUS);
>> +	msg->reply = FIELD_GET(HIBMC_DP_CFG_AUX_STATUS, aux_status);
>> +
>> +	if (aux_status & HIBMC_DP_CFG_AUX_TIMEOUT)
>> +		return -ETIMEDOUT;
>> +
>> +	/* only address */
>> +	if (!msg->size)
>> +		return 0;
>> +
>> +	if (msg->reply != DP_AUX_NATIVE_REPLY_ACK)
>> +		return -EIO;
>> +
>> +	buf_data_cnt = FIELD_GET(HIBMC_DP_CFG_AUX_READY_DATA_BYTE, aux_status);
>> +
>> +	switch (msg->request) {
>> +	case DP_AUX_NATIVE_WRITE:
>> +		return msg->size;
>> +	case DP_AUX_I2C_WRITE | DP_AUX_I2C_MOT:
>> +		if (buf_data_cnt == HIBMC_AUX_I2C_WRITE_SUCCESS)
>> +			return msg->size;
>> +		else
>> +			return FIELD_GET(HIBMC_DP_CFG_AUX, aux_status);
>> +	case DP_AUX_NATIVE_READ:
>> +	case DP_AUX_I2C_READ | DP_AUX_I2C_MOT:
>> +		buf_data_cnt--;
>> +		if (buf_data_cnt != msg->size) {
>> +			/* only the successful part of data is read */
>> +			return -EBUSY;
>> +		}
>> +
>> +		/* all data is successfully read */
>> +		hibmc_dp_aux_read_data(dp, msg->buffer, msg->size);
>> +		return msg->size;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +/* ret >= 0 ,ret is size; ret < 0, ret is err code */
>> +static ssize_t hibmc_dp_aux_xfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>> +{
>> +	struct hibmc_dp_dev *dp = container_of(aux, struct hibmc_dp_dev, aux);
>> +	u32 aux_cmd;
>> +	int ret;
>> +	u32 val; /* val will be assigned at the beginning of readl_poll_timeout function */
>> +
>> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA0);
>> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA1);
>> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA2);
>> +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA3);
>> +
>> +	hibmc_dp_aux_write_data(dp, msg->buffer, msg->size);
>> +
>> +	aux_cmd = hibmc_dp_aux_build_cmd(msg);
>> +	writel(aux_cmd, dp->base + HIBMC_DP_AUX_CMD_ADDR);
>> +
>> +	/* enable aux transfer */
>> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_REQ, 0x1);
>> +	ret = readl_poll_timeout(dp->base + HIBMC_DP_AUX_REQ, val,
>> +				 !(val & HIBMC_DP_CFG_AUX_REQ), 50, 5000);
>> +	if (ret) {
>> +		hibmc_dp_aux_reset(dp);
>> +		return ret;
>> +	}
>> +
>> +	return hibmc_dp_aux_parse_xfer(dp, msg);
>> +}
>> +
>> +void hibmc_dp_aux_init(struct hibmc_dp_dev *dp)
>> +{
>> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_SYNC_LEN_SEL, 0x0);
>> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_TIMER_TIMEOUT, 0x1);
>> +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_MIN_PULSE_NUM,
>> +				 HIBMC_DP_MIN_PULSE_NUM);
>> +
>> +	dp->aux.transfer = hibmc_dp_aux_xfer;
>> +	dp->aux.is_remote = 0;
>> +	drm_dp_aux_init(&dp->aux);
>> +}
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
>> new file mode 100644
>> index 000000000000..ce3b6fa4ea9e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
>> @@ -0,0 +1,38 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/* Copyright (c) 2024 Hisilicon Limited. */
>> +
>> +#ifndef DP_COMM_H
>> +#define DP_COMM_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/bitops.h>
>> +#include <linux/errno.h>
>> +#include <linux/mutex.h>
>> +#include <linux/kernel.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/io.h>
>> +#include <drm/display/drm_dp_helper.h>
>> +
>> +struct hibmc_dp_dev {
>> +	struct drm_dp_aux aux;
>> +	struct drm_device *dev;
>> +	void __iomem *base;
>> +	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
>> +};
>> +
>> +#define dp_field_modify(reg_value, mask, val) ({		\
>> +	(reg_value) &= ~(mask);					\
>> +	(reg_value) |= FIELD_PREP(mask, val); })
> do { ... } while (0) or static inline. Or just inline into the calling
> function, if there is just one place where it is used.
>
>> +
>> +#define hibmc_dp_reg_write_field(dp, offset, mask, val) ({	\
>> +	typeof(dp) _dp = dp;					\
>> +	typeof(_dp->base) addr = (_dp->base + (offset));	\
>> +	mutex_lock(&_dp->lock);					\
>> +	u32 reg_value = readl(addr);				\
>> +	dp_field_modify(reg_value, mask, val);			\
>> +	writel(reg_value, addr);				\
>> +	mutex_unlock(&_dp->lock); })
> I'd prefer a static inline function. Other than that:

Dear Dmitry,
Thanks for your all advice and your very carefully reviewing! I will take your all other suggestion.
For this function, it will be used in multiple files. If we static inline it in header, there
will be kernel test robot's build error becuase FIELD_PREP()'s parameter is a variable.
Here's the error's link: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/

Thanks,
Baihan


>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
>
>> +
>> +void hibmc_dp_aux_init(struct hibmc_dp_dev *dp);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
>> new file mode 100644
>> index 000000000000..f3e6781e111a
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
>> @@ -0,0 +1,27 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/* Copyright (c) 2024 Hisilicon Limited. */
>> +
>> +#ifndef DP_REG_H
>> +#define DP_REG_H
>> +
>> +#define HIBMC_DP_AUX_CMD_ADDR			0x50
>> +#define HIBMC_DP_AUX_WR_DATA0			0x54
>> +#define HIBMC_DP_AUX_WR_DATA1			0x58
>> +#define HIBMC_DP_AUX_WR_DATA2			0x5c
>> +#define HIBMC_DP_AUX_WR_DATA3			0x60
>> +#define HIBMC_DP_AUX_RD_DATA0			0x64
>> +#define HIBMC_DP_AUX_REQ			0x74
>> +#define HIBMC_DP_AUX_STATUS			0x78
>> +#define HIBMC_DP_DPTX_RST_CTRL			0x700
>> +
>> +#define HIBMC_DP_CFG_AUX_SYNC_LEN_SEL		BIT(1)
>> +#define HIBMC_DP_CFG_AUX_TIMER_TIMEOUT		BIT(2)
>> +#define HIBMC_DP_CFG_AUX_MIN_PULSE_NUM		GENMASK(13, 9)
>> +#define HIBMC_DP_CFG_AUX_REQ			BIT(0)
>> +#define HIBMC_DP_CFG_AUX_RST_N			BIT(4)
>> +#define HIBMC_DP_CFG_AUX_TIMEOUT		BIT(0)
>> +#define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
>> +#define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
>> +#define HIBMC_DP_CFG_AUX_STATUS			GENMASK(11, 4)
>> +
>> +#endif
>> -- 
>> 2.33.0
>>

Re: [PATCH v5 drm-dp 1/5] drm/hisilicon/hibmc: add dp aux in hibmc

[Dmitry Baryshkov]

On Tue, Nov 26, 2024 at 09:38:00AM +0800, Yongbang Shi wrote:
> 
> 在 2024/11/22 9:42, Dmitry Baryshkov 写道:
> > On Mon, Nov 18, 2024 at 10:28:01PM +0800, Yongbang Shi wrote:
> > > From: baihan li <libaihan@huawei.com>
> > > 
> > > Add dp aux read/write functions. They are basic functions
> > > and will be used later.
> > > 
> > > Signed-off-by: Baihan Li <libaihan@huawei.com>
> > > Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> > > ---
> > > ChangeLog:
> > > v4 -> v5:
> > >    - fixing build errors reported by kernel test robot <lkp@intel.com>
> > >      Closes: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/
> > > v3 -> v4:
> > >    - retun error codes in  result incorrect branch, suggested by Dmitry Baryshkov.
> > >    - replacing all ret= with returns, suggested by Dmitry Baryshkov.
> > >    - moving the comment below the judgment statement, suggested by Dmitry Baryshkov.
> > >    - moving definations to the source file and clearing headers, suggested by Dmitry Baryshkov.
> > >    - reanaming dp_prefix to hibmc_dp_prefix, suggested by Dmitry Baryshkov.
> > >    - changing hibmc_dp_reg_write_field to static inline and lock, suggested by Dmitry Baryshkov.
> > >    - moving some structs to later patch, suggested by Dmitry Baryshkov.
> > > v2 -> v3:
> > >    - put the macro definations in latter patch where they are actually used, suggested by Dmitry Baryshkov.
> > >    - rename some macro definations to make them sensible, suggested by Dmitry Baryshkov.
> > >    - using FIELD_PREP and FIELD_GET, suggested by Dmitry Baryshkov.
> > >    - using DP_DPCD_REV_foo, suggested by Dmitry Baryshkov.
> > >    - fix build errors reported by kernel test robot <lkp@intel.com>
> > >      Closes: https://lore.kernel.org/oe-kbuild-all/202410250305.UHKDhtxy-lkp@intel.com/
> > > v1 -> v2:
> > >    - using drm_dp_aux frame implement dp aux read and write functions, suggested by Jani Nikula.
> > >    - using drm dp header files' dp macros instead, suggested by Andy Yan.
> > >    v1:https://lore.kernel.org/all/20240930100610.782363-1-shiyongbang@huawei.com/
> > > ---
> > >   drivers/gpu/drm/hisilicon/hibmc/Makefile     |   3 +-
> > >   drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c  | 164 +++++++++++++++++++
> > >   drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h |  38 +++++
> > >   drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h  |  27 +++
> > >   4 files changed, 231 insertions(+), 1 deletion(-)
> > >   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> > >   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> > >   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> > > 
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> > > index d25c75e60d3d..8770ec6dfffd 100644
> > > --- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
> > > @@ -1,4 +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
> > > +hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \
> > > +	       dp/dp_aux.o
> > >   obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> > > new file mode 100644
> > > index 000000000000..16bdfefbf255
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> > > @@ -0,0 +1,164 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-laterHIBMC_BYTES_IN_U32
> > > +// Copyright (c) 2024 Hisilicon Limited.
> > > +
> > > +#include <linux/io.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/minmax.h>
> > > +#include <drm/drm_device.h>
> > > +#include <drm/drm_print.h>
> > > +#include "dp_comm.h"
> > > +#include "dp_reg.h"
> > > +
> > > +#define HIBMC_AUX_CMD_REQ_LEN		GENMASK(7, 4)
> > > +#define HIBMC_AUX_CMD_ADDR		GENMASK(27, 8)
> > > +#define HIBMC_AUX_CMD_I2C_ADDR_ONLY	BIT(28)
> > > +#define HIBMC_BYTES_IN_U32		4
> > > +#define HIBMC_AUX_I2C_WRITE_SUCCESS	0x1
> > > +#define HIBMC_DP_MIN_PULSE_NUM		0x9
> > > +#define BITS_IN_U8			8
> > > +
> > > +static inline void hibmc_dp_aux_reset(struct hibmc_dp_dev *dp)
> > > +{
> > > +	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x0);
> > > +	usleep_range(10, 15);
> > > +	hibmc_dp_reg_write_field(dp, HIBMC_DP_DPTX_RST_CTRL, HIBMC_DP_CFG_AUX_RST_N, 0x1);
> > > +}
> > > +
> > > +static void hibmc_dp_aux_read_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
> > > +{
> > > +	u32 reg_num;
> > > +	u32 value;
> > > +	u32 num;
> > > +	u8 i, j;
> > > +
> > > +	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
> > > +	for (i = 0; i < reg_num; i++) {
> > > +		/* number of bytes read from a single register */
> > > +		num = min(size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
> > > +		value = readl(dp->base + HIBMC_DP_AUX_RD_DATA0 + i * HIBMC_BYTES_IN_U32);
> > > +		/* convert the 32-bit value of the register to the buffer. */
> > > +		for (j = 0; j < num; j++)
> > > +			buf[i * HIBMC_BYTES_IN_U32 + j] = value >> (j * BITS_IN_U8);
> > > +	}
> > > +}
> > > +
> > > +static void hibmc_dp_aux_write_data(struct hibmc_dp_dev *dp, u8 *buf, u8 size)
> > > +{
> > > +	u32 reg_num;
> > > +	u32 value;
> > > +	u32 num;
> > > +	u8 i, j;
> > > +
> > > +	reg_num = DIV_ROUND_UP(size, HIBMC_BYTES_IN_U32);
> > > +	for (i = 0; i < reg_num; i++) {
> > > +		/* number of bytes written to a single register */
> > > +		num = min_t(u8, size - i * HIBMC_BYTES_IN_U32, HIBMC_BYTES_IN_U32);
> > > +		value = 0;
> > > +		/* obtain the 32-bit value written to a single register. */
> > > +		for (j = 0; j < num; j++)
> > > +			value |= buf[i * HIBMC_BYTES_IN_U32 + j] << (j * BITS_IN_U8);
> > > +		/* writing data to a single register */
> > > +		writel(value, dp->base + HIBMC_DP_AUX_WR_DATA0 + i * HIBMC_BYTES_IN_U32);
> > > +	}
> > > +}
> > > +
> > > +static u32 hibmc_dp_aux_build_cmd(const struct drm_dp_aux_msg *msg)
> > > +{
> > > +	u32 aux_cmd = msg->request;
> > > +
> > > +	if (msg->size)
> > > +		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_REQ_LEN, (msg->size - 1));
> > > +	else
> > > +		aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_I2C_ADDR_ONLY, 1);
> > > +
> > > +	aux_cmd |= FIELD_PREP(HIBMC_AUX_CMD_ADDR, msg->address);
> > > +
> > > +	return aux_cmd;
> > > +}
> > > +
> > > +/* ret >= 0, ret is size; ret < 0, ret is err code */
> > > +static int hibmc_dp_aux_parse_xfer(struct hibmc_dp_dev *dp, struct drm_dp_aux_msg *msg)
> > > +{
> > > +	u32 buf_data_cnt;
> > > +	u32 aux_status;
> > > +
> > > +	aux_status = readl(dp->base + HIBMC_DP_AUX_STATUS);
> > > +	msg->reply = FIELD_GET(HIBMC_DP_CFG_AUX_STATUS, aux_status);
> > > +
> > > +	if (aux_status & HIBMC_DP_CFG_AUX_TIMEOUT)
> > > +		return -ETIMEDOUT;
> > > +
> > > +	/* only address */
> > > +	if (!msg->size)
> > > +		return 0;
> > > +
> > > +	if (msg->reply != DP_AUX_NATIVE_REPLY_ACK)
> > > +		return -EIO;
> > > +
> > > +	buf_data_cnt = FIELD_GET(HIBMC_DP_CFG_AUX_READY_DATA_BYTE, aux_status);
> > > +
> > > +	switch (msg->request) {
> > > +	case DP_AUX_NATIVE_WRITE:
> > > +		return msg->size;
> > > +	case DP_AUX_I2C_WRITE | DP_AUX_I2C_MOT:
> > > +		if (buf_data_cnt == HIBMC_AUX_I2C_WRITE_SUCCESS)
> > > +			return msg->size;
> > > +		else
> > > +			return FIELD_GET(HIBMC_DP_CFG_AUX, aux_status);
> > > +	case DP_AUX_NATIVE_READ:
> > > +	case DP_AUX_I2C_READ | DP_AUX_I2C_MOT:
> > > +		buf_data_cnt--;
> > > +		if (buf_data_cnt != msg->size) {
> > > +			/* only the successful part of data is read */
> > > +			return -EBUSY;
> > > +		}
> > > +
> > > +		/* all data is successfully read */
> > > +		hibmc_dp_aux_read_data(dp, msg->buffer, msg->size);
> > > +		return msg->size;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +
> > > +/* ret >= 0 ,ret is size; ret < 0, ret is err code */
> > > +static ssize_t hibmc_dp_aux_xfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> > > +{
> > > +	struct hibmc_dp_dev *dp = container_of(aux, struct hibmc_dp_dev, aux);
> > > +	u32 aux_cmd;
> > > +	int ret;
> > > +	u32 val; /* val will be assigned at the beginning of readl_poll_timeout function */
> > > +
> > > +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA0);
> > > +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA1);
> > > +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA2);
> > > +	writel(0, dp->base + HIBMC_DP_AUX_WR_DATA3);
> > > +
> > > +	hibmc_dp_aux_write_data(dp, msg->buffer, msg->size);
> > > +
> > > +	aux_cmd = hibmc_dp_aux_build_cmd(msg);
> > > +	writel(aux_cmd, dp->base + HIBMC_DP_AUX_CMD_ADDR);
> > > +
> > > +	/* enable aux transfer */
> > > +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_REQ, 0x1);
> > > +	ret = readl_poll_timeout(dp->base + HIBMC_DP_AUX_REQ, val,
> > > +				 !(val & HIBMC_DP_CFG_AUX_REQ), 50, 5000);
> > > +	if (ret) {
> > > +		hibmc_dp_aux_reset(dp);
> > > +		return ret;
> > > +	}
> > > +
> > > +	return hibmc_dp_aux_parse_xfer(dp, msg);
> > > +}
> > > +
> > > +void hibmc_dp_aux_init(struct hibmc_dp_dev *dp)
> > > +{
> > > +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_SYNC_LEN_SEL, 0x0);
> > > +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_TIMER_TIMEOUT, 0x1);
> > > +	hibmc_dp_reg_write_field(dp, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_MIN_PULSE_NUM,
> > > +				 HIBMC_DP_MIN_PULSE_NUM);
> > > +
> > > +	dp->aux.transfer = hibmc_dp_aux_xfer;
> > > +	dp->aux.is_remote = 0;
> > > +	drm_dp_aux_init(&dp->aux);
> > > +}
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> > > new file mode 100644
> > > index 000000000000..ce3b6fa4ea9e
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
> > > @@ -0,0 +1,38 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/* Copyright (c) 2024 Hisilicon Limited. */
> > > +
> > > +#ifndef DP_COMM_H
> > > +#define DP_COMM_H
> > > +
> > > +#include <linux/types.h>
> > > +#include <linux/bitops.h>
> > > +#include <linux/errno.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/bitfield.h>
> > > +#include <linux/io.h>
> > > +#include <drm/display/drm_dp_helper.h>
> > > +
> > > +struct hibmc_dp_dev {
> > > +	struct drm_dp_aux aux;
> > > +	struct drm_device *dev;
> > > +	void __iomem *base;
> > > +	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
> > > +};
> > > +
> > > +#define dp_field_modify(reg_value, mask, val) ({		\
> > > +	(reg_value) &= ~(mask);					\
> > > +	(reg_value) |= FIELD_PREP(mask, val); })
> > do { ... } while (0) or static inline. Or just inline into the calling
> > function, if there is just one place where it is used.
> > 
> > > +
> > > +#define hibmc_dp_reg_write_field(dp, offset, mask, val) ({	\
> > > +	typeof(dp) _dp = dp;					\
> > > +	typeof(_dp->base) addr = (_dp->base + (offset));	\
> > > +	mutex_lock(&_dp->lock);					\
> > > +	u32 reg_value = readl(addr);				\
> > > +	dp_field_modify(reg_value, mask, val);			\
> > > +	writel(reg_value, addr);				\
> > > +	mutex_unlock(&_dp->lock); })
> > I'd prefer a static inline function. Other than that:
> 
> Dear Dmitry,
> Thanks for your all advice and your very carefully reviewing! I will take your all other suggestion.
> For this function, it will be used in multiple files. If we static inline it in header, there
> will be kernel test robot's build error becuase FIELD_PREP()'s parameter is a variable.
> Here's the error's link: https://lore.kernel.org/oe-kbuild-all/202411131438.RZWYrWTE-lkp@intel.com/

Ack. Then at least do { } while(0) please.

> 
> Thanks,
> Baihan
> 
> 
> > 
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > 
> > 
> > > +
> > > +void hibmc_dp_aux_init(struct hibmc_dp_dev *dp);
> > > +
> > > +#endif
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> > > new file mode 100644
> > > index 000000000000..f3e6781e111a
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> > > @@ -0,0 +1,27 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > > +/* Copyright (c) 2024 Hisilicon Limited. */
> > > +
> > > +#ifndef DP_REG_H
> > > +#define DP_REG_H
> > > +
> > > +#define HIBMC_DP_AUX_CMD_ADDR			0x50
> > > +#define HIBMC_DP_AUX_WR_DATA0			0x54
> > > +#define HIBMC_DP_AUX_WR_DATA1			0x58
> > > +#define HIBMC_DP_AUX_WR_DATA2			0x5c
> > > +#define HIBMC_DP_AUX_WR_DATA3			0x60
> > > +#define HIBMC_DP_AUX_RD_DATA0			0x64
> > > +#define HIBMC_DP_AUX_REQ			0x74
> > > +#define HIBMC_DP_AUX_STATUS			0x78
> > > +#define HIBMC_DP_DPTX_RST_CTRL			0x700
> > > +
> > > +#define HIBMC_DP_CFG_AUX_SYNC_LEN_SEL		BIT(1)
> > > +#define HIBMC_DP_CFG_AUX_TIMER_TIMEOUT		BIT(2)
> > > +#define HIBMC_DP_CFG_AUX_MIN_PULSE_NUM		GENMASK(13, 9)
> > > +#define HIBMC_DP_CFG_AUX_REQ			BIT(0)
> > > +#define HIBMC_DP_CFG_AUX_RST_N			BIT(4)
> > > +#define HIBMC_DP_CFG_AUX_TIMEOUT		BIT(0)
> > > +#define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
> > > +#define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
> > > +#define HIBMC_DP_CFG_AUX_STATUS			GENMASK(11, 4)
> > > +
> > > +#endif
> > > -- 
> > > 2.33.0
> > > 

-- 
With best wishes
Dmitry

Re: [PATCH v5 drm-dp 5/5] drm/hisilicon/hibmc: add dp module in hibmc

[Yongbang Shi]

> On Mon, Nov 18, 2024 at 10:28:05PM +0800, Yongbang Shi wrote:
>> From: baihan li <libaihan@huawei.com>
>>
>> To support DP interface displaying in hibmc driver. Add
>> a encoder and connector for DP modual.
>>
>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>> ---
>> ChangeLog:
>> v3 -> v4:
>>    - static inline hibmc_dp_prepare(), suggested by Dmitry Baryshkov.
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/Makefile      |   2 +-
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 118 ++++++++++++++++++
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  12 ++
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |   5 +
>>   4 files changed, 136 insertions(+), 1 deletion(-)
>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
>> index 214228052ccf..95a4ed599d98 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
>> +	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o hibmc_drm_dp.o
>>   
>>   obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> new file mode 100644
>> index 000000000000..603d6b198a54
>> --- /dev/null
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> @@ -0,0 +1,118 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +// Copyright (c) 2024 Hisilicon Limited.
>> +
>> +#include <linux/io.h>
>> +
>> +#include <drm/drm_probe_helper.h>
>> +#include <drm/drm_simple_kms_helper.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_modes.h>
>> +#include <drm/drm_drv.h>
>> +#include <drm/drm_edid.h>
>> +
>> +#include "hibmc_drm_drv.h"
>> +#include "dp/dp_hw.h"
>> +
>> +static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>> +{
>> +	int count;
>> +
>> +	count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width,
>> +				     connector->dev->mode_config.max_height);
>> +	drm_set_preferred_mode(connector, 1024, 768); // temporary implementation
> Ideally this should be mentioned in the commit message.
>
>> +
>> +	return count;
>> +}
>> +
>> +static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
>> +	.get_modes = hibmc_dp_connector_get_modes,
>> +};
>> +
>> +static const struct drm_connector_funcs hibmc_dp_conn_funcs = {
>> +	.reset = drm_atomic_helper_connector_reset,
>> +	.fill_modes = drm_helper_probe_single_connector_modes,
>> +	.destroy = drm_connector_cleanup,
>> +	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> +	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> No .detect / .detect_ctx ? No HPD support? Is it being targeted the DP
> or eDP cases?

Hi Dmitry,
Thanks for your asking. Yes, I will add hpd and get edid functions in next patch,
and I will mention these in the commit message in v6.

Sincerely,
Baihan Li


>> +};
>> +
>> +static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
>> +{
>> +	int ret;
>> +
>> +	hibmc_dp_display_en(dp, false);
>> +
>> +	ret = hibmc_dp_mode_set(dp, mode);
>> +	if (ret)
>> +		drm_err(dp->drm_dev, "hibmc dp mode set failed: %d\n", ret);
>> +
>> +	return ret;
>> +}
>> +
>> +static void hibmc_dp_encoder_enable(struct drm_encoder *drm_encoder,
>> +				    struct drm_atomic_state *state)
>> +{
>> +	struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder);
>> +	struct drm_display_mode *mode = &drm_encoder->crtc->state->mode;
>> +
>> +	if (hibmc_dp_prepare(dp, mode))
>> +		return;
>> +
>> +	hibmc_dp_display_en(dp, true);
>> +}
>> +
>> +static void hibmc_dp_encoder_disable(struct drm_encoder *drm_encoder,
>> +				     struct drm_atomic_state *state)
>> +{
>> +	struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder);
>> +
>> +	hibmc_dp_display_en(dp, false);
>> +}
>> +
>> +static const struct drm_encoder_helper_funcs hibmc_dp_encoder_helper_funcs = {
>> +	.atomic_enable = hibmc_dp_encoder_enable,
>> +	.atomic_disable = hibmc_dp_encoder_disable,
>> +};
>> +
>> +int hibmc_dp_init(struct hibmc_drm_private *priv)
>> +{
>> +	struct drm_device *dev = &priv->dev;
>> +	struct drm_crtc *crtc = &priv->crtc;
>> +	struct hibmc_dp *dp = &priv->dp;
>> +	struct drm_connector *connector = &dp->connector;
>> +	struct drm_encoder *encoder = &dp->encoder;
>> +	int ret;
>> +
>> +	dp->mmio = priv->mmio;
>> +	dp->drm_dev = dev;
>> +
>> +	ret = hibmc_dp_hw_init(&priv->dp);
>> +	if (ret) {
>> +		drm_err(dev, "hibmc dp hw init failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	hibmc_dp_display_en(&priv->dp, false);
>> +
>> +	encoder->possible_crtcs = drm_crtc_mask(crtc);
>> +	ret = drmm_encoder_init(dev, encoder, NULL, DRM_MODE_ENCODER_TMDS, NULL);
>> +	if (ret) {
>> +		drm_err(dev, "init dp encoder failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
>> +
>> +	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
>> +				 DRM_MODE_CONNECTOR_DisplayPort);
>> +	if (ret) {
>> +		drm_err(dev, "init dp connector failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	drm_connector_helper_add(connector, &hibmc_dp_conn_helper_funcs);
>> +
>> +	drm_connector_attach_encoder(connector, encoder);
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> index 9f9b19ea0587..39fd8c5c8227 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
>> @@ -27,6 +27,10 @@
>>   #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
>> +
>>   DEFINE_DRM_GEM_FOPS(hibmc_fops);
>>   
>>   static irqreturn_t hibmc_interrupt(int irq, void *arg)
>> @@ -116,6 +120,14 @@ static int hibmc_kms_init(struct hibmc_drm_private *priv)
>>   		return ret;
>>   	}
>>   
>> +	/* 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 = hibmc_dp_init(priv);
>> +		if (ret)
>> +			drm_err(dev, "failed to init dp: %d\n", ret);
>> +	}
>> +
>>   	ret = hibmc_vdac_init(priv);
>>   	if (ret) {
>>   		drm_err(dev, "failed to init vdac: %d\n", ret);
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> index 42f0ab8f9b5a..d982f1e4b958 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -20,6 +20,8 @@
>>   
>>   #include <drm/drm_framebuffer.h>
>>   
>> +#include "dp/dp_hw.h"
>> +
>>   struct hibmc_vdac {
>>   	struct drm_device *dev;
>>   	struct drm_encoder encoder;
>> @@ -37,6 +39,7 @@ struct hibmc_drm_private {
>>   	struct drm_plane primary_plane;
>>   	struct drm_crtc crtc;
>>   	struct hibmc_vdac vdac;
>> +	struct hibmc_dp dp;
>>   };
>>   
>>   static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
>> @@ -59,4 +62,6 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv);
>>   
>>   int hibmc_ddc_create(struct drm_device *drm_dev, struct hibmc_vdac *connector);
>>   
>> +int hibmc_dp_init(struct hibmc_drm_private *priv);
>> +
>>   #endif
>> -- 
>> 2.33.0
>>