[PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function

dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function
@ 2025-03-19  3:24 Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 1/9] drm/hisilicon/hibmc: Restructuring the header dp_reg.h Yongbang Shi
                   ` (9 more replies)
  0 siblings, 10 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

To support DP HPD, edid printing, and colorbar display features based on
the Hisislcon DP devices.
---
ChangeLog:
v6 -> v7:
  - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
  v6: https://lore.kernel.org/all/20250310040138.2025715-1-shiyongbang@huawei.com/
v5 -> v6:
  - fix the DP_SERDES_VOL2_PRE0 value after electrical test.
  - move the detect_ctx() to the patch 7/9.
  - add detect_ctx with 200ms delay, suggested by Dmitry Baryshkov.
  v5: https://lore.kernel.org/all/20250307101640.4003229-1-shiyongbang@huawei.com/
v4 -> v5:
  - add commit log about hibmc_kms_init(), suggested by Dmitry Baryshkov.
  - fix the format of block comments, suggested by Dmitry Baryshkov.
  - add hibmc_dp_get_serdes_rate_cfg() to correct transferring serdes cfg.
  - separate the vga part commit, suggested by Dmitry Baryshkov.
  - remove pci_disable_msi() in hibmc_unload()
  v4: https://lore.kernel.org/all/20250305112647.2344438-1-shiyongbang@huawei.com/
v3 -> v4:
  - fix the serdes cfg in hibmc_dp_serdes_set_tx_cfg(), suggested by Dmitry Baryshkov.
  - move the dp serdes registers to dp_reg.h, suggested by Dmitry Baryshkov.
  - add comments for if-statement of dp_init(), suggested by Dmitry Baryshkov.
  - fix the comment log to imperative sentence, suggested by Dmitry Baryshkov.
  - add comments in hibmc_control_write(), suggested by Dmitry Baryshkov.
  - add link reset of rates and lanes in pre link training process, suggested by Dmitry Baryshkov.
  - add vdac detect and connected/disconnected status to enable HPD process, suggested by Dmitry Baryshkov.
  - remove a drm_client, suggested by Dmitry Baryshkov.
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202502231304.BCzV4Y8D-lkp@intel.com/
  v3: https://lore.kernel.org/all/20250222025102.1519798-1-shiyongbang@huawei.com/
v2 -> v3:
  - restructuring the header p_reg.h, suggested by Dmitry Baryshkov.
  - add commit log about dp serdes, suggested by Dmitry Baryshkov.
  - return value in hibmc_dp_serdes_init(), suggested by Dmitry Baryshkov.
  - add static const in the array of serdes_tx_cfg[], suggested by Dmitry Baryshkov.
  - change drm_warn to drm_dbg_dp, suggested by Dmitry Baryshkov.
  - add explanations about dp serdes macros, suggested by Dmitry Baryshkov.
  - change commit to an imperative sentence, suggested by Dmitry Baryshkov.
  - put HIBMC_DP_HOST_SERDES_CTRL in dp_serdes.h, suggested by Dmitry Baryshkov.
  - split the patch into two parts, suggested by Dmitry Baryshkov.
  - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
  - rewrite the commit log, suggested by Dmitry Baryshkov.
  - move colorbar debugfs entry to this patch, suggested by Dmitry Baryshkov.
  - change binary format to integer format, suggested by Dmitry Baryshkov.
  - remove mdelay(100) hpd function in ISR, suggested by Dmitry Baryshkov.
  - remove enble_display in ISR, suggested by Dmitry Baryshkov.
  - change drm_kms_helper_connector_hotplug_event() to
    drm_connector_helper_hpd_irq_event(), suggested by Dmitry Baryshkov.
  - move macros to dp_reg.h, suggested by Dmitry Baryshkov.
  - remove struct irqs, suggested by Dmitry Baryshkov.
  - split this patch into two parts, suggested by Dmitry Baryshkov.
  v2: https://lore.kernel.org/all/20250210144959.100551-1-shiyongbang@huawei.com/
v1 -> v2:
  - splittting the patch and add more detailed the changes in the commit message, suggested by Dmitry Baryshkov.
  - changing all names of dp phy to dp serdes.
  - deleting type conversion, suggested by Dmitry Baryshkov.
  - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
  - add colorbar introduction in commit, suggested by Dmitry Baryshkov.
  - deleting edid decoder and its debugfs, suggested by Dmitry Baryshkov.
  - using debugfs_init() callback, suggested by Dmitry Baryshkov.
  - splittting colorbar and debugfs in different patches, suggested by Dmitry Baryshkov.
  - optimizing the description in commit message, suggested by Dmitry Baryshkov.
  - add mdelay(100) comments, suggested by Dmitry Baryshkov.
  - deleting display enable in hpd event, suggested by Dmitry Baryshkov.
  v1: https://lore.kernel.org/all/20250127032024.1542219-1-shiyongbang@huawei.com/
---

Baihan Li (9):
  drm/hisilicon/hibmc: Restructuring the header dp_reg.h
  drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage
    and pre-emphasis
  drm/hisilicon/hibmc: Add dp serdes cfg in dp process
  drm/hisilicon/hibmc: Refactor the member of drm_aux in struct hibmc_dp
  drm/hisilicon/hibmc: Getting connector info and EDID by using AUX
    channel
  drm/hisilicon/hibmc: Add colorbar-cfg feature and its debugfs file
  drm/hisilicon/hibmc: Enable this hot plug detect of irq feature
  drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD
  drm/hisilicon/hibmc: Add vga connector detect functions

 drivers/gpu/drm/hisilicon/hibmc/Makefile      |   3 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  16 ++-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  10 +-
 .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |   2 +
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  91 +++++++++++-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  36 +++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  |  97 +++++++++----
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   | 130 +++++++++++++-----
 .../gpu/drm/hisilicon/hibmc/dp/dp_serdes.c    |  71 ++++++++++
 .../drm/hisilicon/hibmc/hibmc_drm_debugfs.c   | 104 ++++++++++++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    |  78 ++++++++++-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  87 +++++++++---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  12 ++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  |   3 +
 14 files changed, 638 insertions(+), 102 deletions(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c

-- 
2.33.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 1/9] drm/hisilicon/hibmc: Restructuring the header dp_reg.h
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 2/9] drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage and pre-emphasis Yongbang Shi
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

Move the macros below their corresponding registers to make
them more obvious.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v2 -> v3:
  - restructuring the header dp_reg.h, suggested by Dmitry Baryshkov.
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h | 98 +++++++++++++--------
 1 file changed, 60 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index 4a515c726d52..dc2bd3f80b70 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -5,72 +5,94 @@
 #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_CFG_AUX_REQ			BIT(0)
+#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_AUX_STATUS			0x78
+#define HIBMC_DP_CFG_AUX_TIMEOUT		BIT(0)
+#define HIBMC_DP_CFG_AUX_STATUS			GENMASK(11, 4)
+#define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
+#define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
+
 #define HIBMC_DP_PHYIF_CTRL0			0xa0
+#define HIBMC_DP_CFG_SCRAMBLE_EN		BIT(0)
+#define HIBMC_DP_CFG_PAT_SEL			GENMASK(7, 4)
+#define HIBMC_DP_CFG_LANE_DATA_EN		GENMASK(11, 8)
+
 #define HIBMC_DP_VIDEO_CTRL			0x100
+#define HIBMC_DP_CFG_STREAM_RGB_ENABLE		BIT(1)
+#define HIBMC_DP_CFG_STREAM_VIDEO_MAPPING	GENMASK(5, 2)
+#define HIBMC_DP_CFG_STREAM_FRAME_MODE		BIT(6)
+#define HIBMC_DP_CFG_STREAM_HSYNC_POLARITY	BIT(7)
+#define HIBMC_DP_CFG_STREAM_VSYNC_POLARITY	BIT(8)
+
 #define HIBMC_DP_VIDEO_CONFIG0			0x104
+#define HIBMC_DP_CFG_STREAM_HACTIVE		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_HBLANK		GENMASK(15, 0)
+
 #define HIBMC_DP_VIDEO_CONFIG1			0x108
+#define HIBMC_DP_CFG_STREAM_VACTIVE		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_VBLANK		GENMASK(15, 0)
+
 #define HIBMC_DP_VIDEO_CONFIG2			0x10c
+#define HIBMC_DP_CFG_STREAM_HSYNC_WIDTH		GENMASK(15, 0)
+
 #define HIBMC_DP_VIDEO_CONFIG3			0x110
+#define HIBMC_DP_CFG_STREAM_VSYNC_WIDTH		GENMASK(15, 0)
+#define HIBMC_DP_CFG_STREAM_VFRONT_PORCH	GENMASK(31, 16)
+
 #define HIBMC_DP_VIDEO_PACKET			0x114
+#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_VIDEO_MSA0			0x118
+#define HIBMC_DP_CFG_STREAM_VSTART		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_HSTART		GENMASK(15, 0)
+
 #define HIBMC_DP_VIDEO_MSA1			0x11c
 #define HIBMC_DP_VIDEO_MSA2			0x120
+
 #define HIBMC_DP_VIDEO_HORIZONTAL_SIZE		0X124
+#define HIBMC_DP_CFG_STREAM_HTOTAL_SIZE		GENMASK(31, 16)
+#define HIBMC_DP_CFG_STREAM_HBLANK_SIZE		GENMASK(15, 0)
+
 #define HIBMC_DP_TIMING_GEN_CONFIG0		0x26c
+#define HIBMC_DP_CFG_TIMING_GEN0_HACTIVE	GENMASK(31, 16)
+#define HIBMC_DP_CFG_TIMING_GEN0_HBLANK		GENMASK(15, 0)
+
 #define HIBMC_DP_TIMING_GEN_CONFIG2		0x274
+#define HIBMC_DP_CFG_TIMING_GEN0_VACTIVE	GENMASK(31, 16)
+#define HIBMC_DP_CFG_TIMING_GEN0_VBLANK		GENMASK(15, 0)
+
 #define HIBMC_DP_TIMING_GEN_CONFIG3		0x278
+#define HIBMC_DP_CFG_TIMING_GEN0_VFRONT_PORCH	GENMASK(31, 16)
+
 #define HIBMC_DP_HDCP_CFG			0x600
+
 #define HIBMC_DP_DPTX_RST_CTRL			0x700
+#define HIBMC_DP_CFG_AUX_RST_N			BIT(4)
+
 #define HIBMC_DP_DPTX_CLK_CTRL			0x704
+
 #define HIBMC_DP_DPTX_GCTL0			0x708
+#define HIBMC_DP_CFG_PHY_LANE_NUM		GENMASK(2, 1)
+
 #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)
-#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)
-#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_TIMING_MODEL_CTRL		0x884
 #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)
+
+#define HIBMC_DP_TIMING_SYNC_CTRL		0xFF0
 
 #endif
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 2/9] drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage and pre-emphasis
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 1/9] drm/hisilicon/hibmc: Restructuring the header dp_reg.h Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 3/9] drm/hisilicon/hibmc: Add dp serdes cfg in dp process Yongbang Shi
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

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

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

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v5 -> v6:
  - fix the DP_SERDES_VOL2_PRE0 value after electrical test.
v3 -> v4:
  - fix the serdes cfg in hibmc_dp_serdes_set_tx_cfg(), suggested by Dmitry Baryshkov.
  - move the dp serdes registers to dp_reg.h, suggested by Dmitry Baryshkov.
  - 
v2 -> v3:
  - add commit log about dp serdes, suggested by Dmitry Baryshkov.
  - return value in hibmc_dp_serdes_init(), suggested by Dmitry Baryshkov.
  - add static const in the array of serdes_tx_cfg[], suggested by Dmitry Baryshkov.
  - change drm_warn to drm_dbg_dp, suggested by Dmitry Baryshkov.
  - add explanations about dp serdes macros, suggested by Dmitry Baryshkov.
v1 -> v2:
  - splittting the patch and add more detailed the changes in the commit message, suggested by Dmitry Baryshkov.
  - changing all names of dp phy to dp serdes.
---
 drivers/gpu/drm/hisilicon/hibmc/Makefile      |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  4 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  5 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   | 23 ++++++
 .../gpu/drm/hisilicon/hibmc/dp/dp_serdes.c    | 71 +++++++++++++++++++
 5 files changed, 104 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 95a4ed599d98..43de077d6769 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
+++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 hibmc-drm-y := hibmc_drm_drv.o hibmc_drm_de.o hibmc_drm_vdac.o hibmc_drm_i2c.o \
-	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o hibmc_drm_dp.o
+	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o dp/dp_serdes.o hibmc_drm_dp.o
 
 obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
index 2c52a4476c4d..e0c6a3b7463b 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
@@ -38,6 +38,7 @@ struct hibmc_dp_dev {
 	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
 	struct hibmc_dp_link link;
 	u8 dpcd[DP_RECEIVER_CAP_SIZE];
+	void __iomem *serdes_base;
 };
 
 #define dp_field_modify(reg_value, mask, val)				\
@@ -59,5 +60,8 @@ struct hibmc_dp_dev {
 
 void hibmc_dp_aux_init(struct hibmc_dp_dev *dp);
 int hibmc_dp_link_training(struct hibmc_dp_dev *dp);
+int hibmc_dp_serdes_init(struct hibmc_dp_dev *dp);
+int hibmc_dp_serdes_rate_switch(u8 rate, struct hibmc_dp_dev *dp);
+int hibmc_dp_serdes_set_tx_cfg(struct hibmc_dp_dev *dp, u8 train_set[HIBMC_DP_LANE_NUM_MAX]);
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index a8d543881c09..3612f3c5ab23 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -151,6 +151,7 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
 {
 	struct drm_device *drm_dev = dp->drm_dev;
 	struct hibmc_dp_dev *dp_dev;
+	int ret;
 
 	dp_dev = devm_kzalloc(drm_dev->dev, sizeof(struct hibmc_dp_dev), GFP_KERNEL);
 	if (!dp_dev)
@@ -165,6 +166,10 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
 
 	hibmc_dp_aux_init(dp_dev);
 
+	ret = hibmc_dp_serdes_init(dp_dev);
+	if (ret)
+		return ret;
+
 	dp_dev->link.cap.lanes = 0x2;
 	dp_dev->link.cap.link_rate = DP_LINK_BW_2_7;
 
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index dc2bd3f80b70..16ea58903598 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -95,4 +95,27 @@
 
 #define HIBMC_DP_TIMING_SYNC_CTRL		0xFF0
 
+/* dp serdes reg */
+#define HIBMC_DP_HOST_OFFSET		0x10000
+#define HIBMC_DP_LANE0_RATE_OFFSET	0x4
+#define HIBMC_DP_LANE1_RATE_OFFSET	0xc
+#define HIBMC_DP_LANE_STATUS_OFFSET	0x10
+#define HIBMC_DP_PMA_LANE0_OFFSET	0x18
+#define HIBMC_DP_PMA_LANE1_OFFSET	0x1c
+#define HIBMC_DP_PMA_TXDEEMPH		GENMASK(18, 1)
+#define DP_SERDES_DONE			0x3
+
+/* dp serdes TX-Deempth Configuration */
+#define DP_SERDES_VOL0_PRE0		0x280
+#define DP_SERDES_VOL0_PRE1		0x2300
+#define DP_SERDES_VOL0_PRE2		0x53c0
+#define DP_SERDES_VOL0_PRE3		0x8400
+#define DP_SERDES_VOL1_PRE0		0x380
+#define DP_SERDES_VOL1_PRE1		0x3440
+#define DP_SERDES_VOL1_PRE2		0x6480
+#define DP_SERDES_VOL2_PRE0		0x4c1
+#define DP_SERDES_VOL2_PRE1		0x4500
+#define DP_SERDES_VOL3_PRE0		0x600
+#define DP_SERDES_BW_8_1		0x3
+
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
new file mode 100644
index 000000000000..676059d4c1e6
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2025 Hisilicon Limited.
+
+#include <linux/delay.h>
+#include <drm/drm_device.h>
+#include <drm/drm_print.h>
+#include "dp_comm.h"
+#include "dp_config.h"
+#include "dp_reg.h"
+
+int hibmc_dp_serdes_set_tx_cfg(struct hibmc_dp_dev *dp, u8 train_set[HIBMC_DP_LANE_NUM_MAX])
+{
+	static const u32 serdes_tx_cfg[4][4] = { {DP_SERDES_VOL0_PRE0, DP_SERDES_VOL0_PRE1,
+						  DP_SERDES_VOL0_PRE2, DP_SERDES_VOL0_PRE3},
+						 {DP_SERDES_VOL1_PRE0, DP_SERDES_VOL1_PRE1,
+						  DP_SERDES_VOL1_PRE2}, {DP_SERDES_VOL2_PRE0,
+						  DP_SERDES_VOL2_PRE1}, {DP_SERDES_VOL3_PRE0}};
+	int cfg[2];
+	int i;
+
+	for (i = 0; i < HIBMC_DP_LANE_NUM_MAX; i++) {
+		cfg[i] = serdes_tx_cfg[FIELD_GET(DP_TRAIN_VOLTAGE_SWING_MASK, train_set[i])]
+				      [FIELD_GET(DP_TRAIN_PRE_EMPHASIS_MASK, train_set[i])];
+		if (!cfg[i])
+			return -EINVAL;
+
+		/* lane1 offset is 4 */
+		writel(FIELD_PREP(HIBMC_DP_PMA_TXDEEMPH, cfg[i]),
+		       dp->serdes_base + HIBMC_DP_PMA_LANE0_OFFSET + i * 4);
+	}
+
+	usleep_range(300, 500);
+
+	if (readl(dp->serdes_base + HIBMC_DP_LANE_STATUS_OFFSET) != DP_SERDES_DONE) {
+		drm_dbg_dp(dp->dev, "dp serdes cfg failed\n");
+		return -EAGAIN;
+	}
+
+	return 0;
+}
+
+int hibmc_dp_serdes_rate_switch(u8 rate, struct hibmc_dp_dev *dp)
+{
+	writel(rate, dp->serdes_base + HIBMC_DP_LANE0_RATE_OFFSET);
+	writel(rate, dp->serdes_base + HIBMC_DP_LANE1_RATE_OFFSET);
+
+	usleep_range(300, 500);
+
+	if (readl(dp->serdes_base + HIBMC_DP_LANE_STATUS_OFFSET) != DP_SERDES_DONE) {
+		drm_dbg_dp(dp->dev, "dp serdes rate switching failed\n");
+		return -EAGAIN;
+	}
+
+	if (rate < DP_SERDES_BW_8_1)
+		drm_dbg_dp(dp->dev, "reducing serdes rate to :%d\n",
+			   rate ? rate * HIBMC_DP_LINK_RATE_CAL * 10 : 162);
+
+	return 0;
+}
+
+int hibmc_dp_serdes_init(struct hibmc_dp_dev *dp)
+{
+	dp->serdes_base = dp->base + HIBMC_DP_HOST_OFFSET;
+
+	writel(FIELD_PREP(HIBMC_DP_PMA_TXDEEMPH, DP_SERDES_VOL0_PRE0),
+	       dp->serdes_base + HIBMC_DP_PMA_LANE0_OFFSET);
+	writel(FIELD_PREP(HIBMC_DP_PMA_TXDEEMPH, DP_SERDES_VOL0_PRE0),
+	       dp->serdes_base + HIBMC_DP_PMA_LANE1_OFFSET);
+
+	return hibmc_dp_serdes_rate_switch(DP_SERDES_BW_8_1, dp);
+}
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 3/9] drm/hisilicon/hibmc: Add dp serdes cfg in dp process
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 1/9] drm/hisilicon/hibmc: Restructuring the header dp_reg.h Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 2/9] drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage and pre-emphasis Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 4/9] drm/hisilicon/hibmc: Refactor the member of drm_aux in struct hibmc_dp Yongbang Shi
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

Add dp serdes cfg in link training process, and related adapting
and modificating. Change some init values about training, because we want
completely to negotiation process, so we start with the maximum rate and
the electrical characteristic level is 0. Because serdes default cfgs is
changed and used in hibmc_kms_init(), we changed the if-statement to check
whether the value is 0.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v4 -> v5:
  - add commit log about hibmc_kms_init(), suggested by Dmitry Baryshkov.
  - fix the format of block comments, suggested by Dmitry Baryshkov.
  - add hibmc_dp_get_serdes_rate_cfg() to correct transferring serdes cfg.
v3 -> v4:
  - add comments for if-statement of dp_init(), suggested by Dmitry Baryshkov.
v2 -> v3:
  - change commit to an imperative sentence, suggested by Dmitry Baryshkov.
  - put HIBMC_DP_HOST_SERDES_CTRL in dp_serdes.h, suggested by Dmitry Baryshkov.
v1 -> v2:
  - splittting the patch and add more detailed the changes in the commit message, suggested by Dmitry Baryshkov.
---
 .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  1 +
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  5 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  | 77 ++++++++++++++-----
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |  5 ++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 13 ++--
 5 files changed, 75 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
index 74dd9956144e..c5feef8dc27d 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
@@ -15,5 +15,6 @@
 #define HIBMC_DP_CLK_EN			0x7
 #define HIBMC_DP_SYNC_EN_MASK		0x3
 #define HIBMC_DP_LINK_RATE_CAL		27
+#define HIBMC_DP_SYNC_DELAY(lanes)	((lanes) == 0x2 ? 86 : 46)
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index 3612f3c5ab23..dcb2ab5ea6bb 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -72,6 +72,9 @@ static void hibmc_dp_set_sst(struct hibmc_dp_dev *dp, struct drm_display_mode *m
 				 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);
+	hibmc_dp_reg_write_field(dp, HIBMC_DP_VIDEO_PACKET,
+				 HIBMC_DP_CFG_STREAM_SYNC_CALIBRATION,
+				 HIBMC_DP_SYNC_DELAY(dp->link.cap.lanes));
 }
 
 static void hibmc_dp_link_cfg(struct hibmc_dp_dev *dp, struct drm_display_mode *mode)
@@ -171,7 +174,7 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
 		return ret;
 
 	dp_dev->link.cap.lanes = 0x2;
-	dp_dev->link.cap.link_rate = DP_LINK_BW_2_7;
+	dp_dev->link.cap.link_rate = DP_LINK_BW_8_1;
 
 	/* hdcp data */
 	writel(HIBMC_DP_HDCP, dp_dev->base + HIBMC_DP_HDCP_CFG);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
index f6355c16cc0a..2b4940ea809c 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
@@ -9,6 +9,22 @@
 
 #define HIBMC_EQ_MAX_RETRY 5
 
+static inline int hibmc_dp_get_serdes_rate_cfg(struct hibmc_dp_dev *dp)
+{
+	switch (dp->link.cap.link_rate) {
+	case DP_LINK_BW_1_62:
+		return DP_SERDES_BW_1_62;
+	case DP_LINK_BW_2_7:
+		return DP_SERDES_BW_2_7;
+	case DP_LINK_BW_5_4:
+		return DP_SERDES_BW_5_4;
+	case DP_LINK_BW_8_1:
+		return DP_SERDES_BW_8_1;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int hibmc_dp_link_training_configure(struct hibmc_dp_dev *dp)
 {
 	u8 buf[2];
@@ -41,11 +57,7 @@ static int hibmc_dp_link_training_configure(struct hibmc_dp_dev *dp)
 		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;
+	return 0;
 }
 
 static int hibmc_dp_link_set_pattern(struct hibmc_dp_dev *dp, int pattern)
@@ -108,7 +120,11 @@ static int hibmc_dp_link_training_cr_pre(struct hibmc_dp_dev *dp)
 		return ret;
 
 	for (i = 0; i < dp->link.cap.lanes; i++)
-		train_set[i] = DP_TRAIN_VOLTAGE_SWING_LEVEL_2;
+		train_set[i] = DP_TRAIN_VOLTAGE_SWING_LEVEL_0;
+
+	ret = hibmc_dp_serdes_set_tx_cfg(dp, dp->link.train_set);
+	if (ret)
+		return ret;
 
 	ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set, dp->link.cap.lanes);
 	if (ret != dp->link.cap.lanes) {
@@ -137,21 +153,18 @@ static bool hibmc_dp_link_get_adjust_train(struct hibmc_dp_dev *dp,
 	return false;
 }
 
-static inline int hibmc_dp_link_reduce_rate(struct hibmc_dp_dev *dp)
+static 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:
+	int ret;
+
+	if (dp->link.cap.link_rate == DP_LINK_BW_1_62)
 		return -EINVAL;
-	}
+
+	ret = hibmc_dp_get_serdes_rate_cfg(dp);
+	if (ret < 0)
+		return ret;
+
+	return hibmc_dp_serdes_rate_switch(--ret, dp);
 }
 
 static inline int hibmc_dp_link_reduce_lane(struct hibmc_dp_dev *dp)
@@ -159,6 +172,7 @@ static inline int hibmc_dp_link_reduce_lane(struct hibmc_dp_dev *dp)
 	switch (dp->link.cap.lanes) {
 	case 0x2:
 		dp->link.cap.lanes--;
+		drm_dbg_dp(dp->dev, "dp link training reduce to 1 lane\n");
 		break;
 	case 0x1:
 		drm_err(dp->dev, "dp link training reduce lane failed, already reach minimum\n");
@@ -206,6 +220,11 @@ static int hibmc_dp_link_training_cr(struct hibmc_dp_dev *dp)
 		}
 
 		level_changed = hibmc_dp_link_get_adjust_train(dp, lane_status);
+
+		ret = hibmc_dp_serdes_set_tx_cfg(dp, dp->link.train_set);
+		if (ret)
+			return ret;
+
 		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) {
@@ -255,6 +274,11 @@ static int hibmc_dp_link_training_channel_eq(struct hibmc_dp_dev *dp)
 		}
 
 		hibmc_dp_link_get_adjust_train(dp, lane_status);
+
+		ret = hibmc_dp_serdes_set_tx_cfg(dp, dp->link.train_set);
+		if (ret)
+			return ret;
+
 		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) {
@@ -295,6 +319,21 @@ int hibmc_dp_link_training(struct hibmc_dp_dev *dp)
 	struct hibmc_dp_link *link = &dp->link;
 	int 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);
+
+	dp->link.cap.link_rate = dp->dpcd[DP_MAX_LINK_RATE];
+	dp->link.cap.lanes = 0x2;
+
+	ret = hibmc_dp_get_serdes_rate_cfg(dp);
+	if (ret < 0)
+		return ret;
+
+	ret = hibmc_dp_serdes_rate_switch(ret, dp);
+	if (ret)
+		return ret;
+
 	while (true) {
 		ret = hibmc_dp_link_training_cr_pre(dp);
 		if (ret)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index 16ea58903598..6eb76decc636 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -54,6 +54,7 @@
 #define HIBMC_DP_VIDEO_PACKET			0x114
 #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_SYNC_CALIBRATION	GENMASK(31, 20)
 
 #define HIBMC_DP_VIDEO_MSA0			0x118
 #define HIBMC_DP_CFG_STREAM_VSTART		GENMASK(31, 16)
@@ -102,6 +103,7 @@
 #define HIBMC_DP_LANE_STATUS_OFFSET	0x10
 #define HIBMC_DP_PMA_LANE0_OFFSET	0x18
 #define HIBMC_DP_PMA_LANE1_OFFSET	0x1c
+#define HIBMC_DP_HOST_SERDES_CTRL	0x1f001c
 #define HIBMC_DP_PMA_TXDEEMPH		GENMASK(18, 1)
 #define DP_SERDES_DONE			0x3
 
@@ -117,5 +119,8 @@
 #define DP_SERDES_VOL2_PRE1		0x4500
 #define DP_SERDES_VOL3_PRE0		0x600
 #define DP_SERDES_BW_8_1		0x3
+#define DP_SERDES_BW_5_4		0x2
+#define DP_SERDES_BW_2_7		0x1
+#define DP_SERDES_BW_1_62		0x0
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index e6de6d5edf6b..98b01c8aee8e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -28,9 +28,7 @@
 #include "hibmc_drm_drv.h"
 #include "hibmc_drm_regs.h"
 
-#define HIBMC_DP_HOST_SERDES_CTRL		0x1f001c
-#define HIBMC_DP_HOST_SERDES_CTRL_VAL		0x8a00
-#define HIBMC_DP_HOST_SERDES_CTRL_MASK		0x7ffff
+#include "dp/dp_reg.h"
 
 DEFINE_DRM_GEM_FOPS(hibmc_fops);
 
@@ -121,9 +119,12 @@ 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) {
+	/*
+	 * If the serdes reg is readable and is not equal to 0,
+	 * DP block exists and initializes it.
+	 */
+	ret = readl(priv->mmio + HIBMC_DP_HOST_SERDES_CTRL);
+	if (ret) {
 		ret = hibmc_dp_init(priv);
 		if (ret)
 			drm_err(dev, "failed to init dp: %d\n", ret);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 4/9] drm/hisilicon/hibmc: Refactor the member of drm_aux in struct hibmc_dp
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
                   ` (2 preceding siblings ...)
  2025-03-19  3:24 ` [PATCH v7 drm-dp 3/9] drm/hisilicon/hibmc: Add dp serdes cfg in dp process Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel Yongbang Shi
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

Because the drm_aux of struct hibmc_dp_dev's member is not easy to get in
hibmc_drm_dp.c, move the drm_aux to struct hibmc_dp. Then there are some
adaptations and modifications to make this patch compile.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v3 -> v4:
  - fix the comment log to imperative sentence, suggested by Dmitry Baryshkov.
v2 -> v3:
  - split the patch into two parts, suggested by Dmitry Baryshkov.
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c  | 13 +++++++-----
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h |  6 ++++--
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c   |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h   |  2 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c | 22 ++++++++++----------
 5 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
index 0a903cce1fa9..ded9e7ce887a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
@@ -8,6 +8,7 @@
 #include <drm/drm_print.h>
 #include "dp_comm.h"
 #include "dp_reg.h"
+#include "dp_hw.h"
 
 #define HIBMC_AUX_CMD_REQ_LEN		GENMASK(7, 4)
 #define HIBMC_AUX_CMD_ADDR		GENMASK(27, 8)
@@ -124,7 +125,8 @@ static int hibmc_dp_aux_parse_xfer(struct hibmc_dp_dev *dp, struct drm_dp_aux_ms
 /* 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);
+	struct hibmc_dp *dp_priv = container_of(aux, struct hibmc_dp, aux);
+	struct hibmc_dp_dev *dp = dp_priv->dp_dev;
 	u32 aux_cmd;
 	int ret;
 	u32 val; /* val will be assigned at the beginning of readl_poll_timeout function */
@@ -151,14 +153,15 @@ static ssize_t hibmc_dp_aux_xfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *
 	return hibmc_dp_aux_parse_xfer(dp, msg);
 }
 
-void hibmc_dp_aux_init(struct hibmc_dp_dev *dp)
+void hibmc_dp_aux_init(struct hibmc_dp *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_reg_write_field(dp->dp_dev, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_SYNC_LEN_SEL, 0x0);
+	hibmc_dp_reg_write_field(dp->dp_dev, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_TIMER_TIMEOUT, 0x1);
+	hibmc_dp_reg_write_field(dp->dp_dev, 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);
+	dp->dp_dev->aux = &dp->aux;
 }
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
index e0c6a3b7463b..4add05c7f161 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h
@@ -13,6 +13,8 @@
 #include <linux/io.h>
 #include <drm/display/drm_dp_helper.h>
 
+#include "dp_hw.h"
+
 #define HIBMC_DP_LANE_NUM_MAX 2
 
 struct hibmc_link_status {
@@ -32,7 +34,7 @@ struct hibmc_dp_link {
 };
 
 struct hibmc_dp_dev {
-	struct drm_dp_aux aux;
+	struct drm_dp_aux *aux;
 	struct drm_device *dev;
 	void __iomem *base;
 	struct mutex lock; /* protects concurrent RW in hibmc_dp_reg_write_field() */
@@ -58,7 +60,7 @@ struct hibmc_dp_dev {
 		mutex_unlock(&_dp->lock);				\
 	} while (0)
 
-void hibmc_dp_aux_init(struct hibmc_dp_dev *dp);
+void hibmc_dp_aux_init(struct hibmc_dp *dp);
 int hibmc_dp_link_training(struct hibmc_dp_dev *dp);
 int hibmc_dp_serdes_init(struct hibmc_dp_dev *dp);
 int hibmc_dp_serdes_rate_switch(u8 rate, struct hibmc_dp_dev *dp);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index dcb2ab5ea6bb..aa9354a996c9 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -167,7 +167,7 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
 	dp_dev->dev = drm_dev;
 	dp_dev->base = dp->mmio + HIBMC_DP_OFFSET;
 
-	hibmc_dp_aux_init(dp_dev);
+	hibmc_dp_aux_init(dp);
 
 	ret = hibmc_dp_serdes_init(dp_dev);
 	if (ret)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
index 4dc13b3d9875..53b6d0beecea 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -10,6 +10,7 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_print.h>
+#include <drm/display/drm_dp_helper.h>
 
 struct hibmc_dp_dev;
 
@@ -19,6 +20,7 @@ struct hibmc_dp {
 	struct drm_encoder encoder;
 	struct drm_connector connector;
 	void __iomem *mmio;
+	struct drm_dp_aux aux;
 };
 
 int hibmc_dp_hw_init(struct hibmc_dp *dp);
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
index 2b4940ea809c..f80e1d82511b 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c
@@ -42,7 +42,7 @@ static int hibmc_dp_link_training_configure(struct hibmc_dp_dev *dp)
 	/* 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));
+	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;
@@ -51,7 +51,7 @@ static int hibmc_dp_link_training_configure(struct hibmc_dp_dev *dp)
 	/* set 8b/10b and downspread */
 	buf[0] = DP_SPREAD_AMP_0_5;
 	buf[1] = DP_SET_ANSI_8B10B;
-	ret = drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, buf, sizeof(buf));
+	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;
@@ -96,7 +96,7 @@ static int hibmc_dp_link_set_pattern(struct hibmc_dp_dev *dp, int pattern)
 
 	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));
+	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;
@@ -126,7 +126,7 @@ static int hibmc_dp_link_training_cr_pre(struct hibmc_dp_dev *dp)
 	if (ret)
 		return ret;
 
-	ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set, dp->link.cap.lanes);
+	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;
@@ -199,9 +199,9 @@ static int hibmc_dp_link_training_cr(struct hibmc_dp_dev *dp)
 
 	voltage_tries = 1;
 	for (cr_tries = 0; cr_tries < 80; cr_tries++) {
-		drm_dp_link_train_clock_recovery_delay(&dp->aux, dp->dpcd);
+		drm_dp_link_train_clock_recovery_delay(dp->aux, dp->dpcd);
 
-		ret = drm_dp_dpcd_read_link_status(&dp->aux, lane_status);
+		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;
@@ -225,7 +225,7 @@ static int hibmc_dp_link_training_cr(struct hibmc_dp_dev *dp)
 		if (ret)
 			return ret;
 
-		ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, dp->link.train_set,
+		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");
@@ -252,9 +252,9 @@ static int hibmc_dp_link_training_channel_eq(struct hibmc_dp_dev *dp)
 		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);
+		drm_dp_link_train_channel_eq_delay(dp->aux, dp->dpcd);
 
-		ret = drm_dp_dpcd_read_link_status(&dp->aux, lane_status);
+		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;
@@ -279,7 +279,7 @@ static int hibmc_dp_link_training_channel_eq(struct hibmc_dp_dev *dp)
 		if (ret)
 			return ret;
 
-		ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
+		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");
@@ -319,7 +319,7 @@ int hibmc_dp_link_training(struct hibmc_dp_dev *dp)
 	struct hibmc_dp_link *link = &dp->link;
 	int ret;
 
-	ret = drm_dp_read_dpcd_caps(&dp->aux, dp->dpcd);
+	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);
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
                   ` (3 preceding siblings ...)
  2025-03-19  3:24 ` [PATCH v7 drm-dp 4/9] drm/hisilicon/hibmc: Refactor the member of drm_aux in struct hibmc_dp Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-20  9:50   ` Jani Nikula
  2025-03-19  3:24 ` [PATCH v7 drm-dp 6/9] drm/hisilicon/hibmc: Add colorbar-cfg feature and its debugfs file Yongbang Shi
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

Add registering drm_aux and use it to get connector edid with drm
functions. Add ddc channel in connector initialization to put drm_aux
in drm_connector.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v6 -> v7:
  - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
v5 -> v6:
  - move the detect_ctx() to the patch 7/9.
v2 -> v3:
  - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
v1 -> v2:
  - deleting type conversion, suggested by Dmitry Baryshkov.
  - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  3 +-
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 ++++++++++++++++---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  5 +++
 3 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
index ded9e7ce887a..e0bb9b14d9d8 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
@@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp)
 				 HIBMC_DP_MIN_PULSE_NUM);
 
 	dp->aux.transfer = hibmc_dp_aux_xfer;
-	dp->aux.is_remote = 0;
+	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
+	dp->aux.drm_dev = dp->drm_dev;
 	drm_dp_aux_init(&dp->aux);
 	dp->dp_dev->aux = &dp->aux;
 }
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
index 603d6b198a54..0256724d8b9b 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -15,11 +15,20 @@
 
 static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
 {
+	struct hibmc_dp *dp = to_hibmc_dp(connector);
+	const struct drm_edid *drm_edid;
 	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
+	if (!dp->aux.name)
+		return 0;
+
+	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
+
+	drm_edid_connector_update(connector, drm_edid);
+
+	count = drm_edid_connector_add_modes(connector);
+
+	drm_edid_free(drm_edid);
 
 	return count;
 }
@@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
 	.get_modes = hibmc_dp_connector_get_modes,
 };
 
+static int hibmc_dp_late_register(struct drm_connector *connector)
+{
+	struct hibmc_dp *dp = to_hibmc_dp(connector);
+
+	return drm_dp_aux_register(&dp->aux);
+}
+
+static void hibmc_dp_early_unregister(struct drm_connector *connector)
+{
+	struct hibmc_dp *dp = to_hibmc_dp(connector);
+
+	drm_dp_aux_unregister(&dp->aux);
+}
+
 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,
+	.late_register = hibmc_dp_late_register,
+	.early_unregister = hibmc_dp_early_unregister,
 };
 
 static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
@@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
 
 	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
 
-	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
-				 DRM_MODE_CONNECTOR_DisplayPort);
+	ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs,
+					  DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc);
 	if (ret) {
 		drm_err(dev, "init dp connector failed: %d\n", ret);
 		return ret;
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index d982f1e4b958..3ddd71aada66 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
 	return container_of(connector, struct hibmc_vdac, connector);
 }
 
+static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector)
+{
+	return container_of(connector, struct hibmc_dp, connector);
+}
+
 static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev)
 {
 	return container_of(dev, struct hibmc_drm_private, dev);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 6/9] drm/hisilicon/hibmc: Add colorbar-cfg feature and its debugfs file
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
                   ` (4 preceding siblings ...)
  2025-03-19  3:24 ` [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 7/9] drm/hisilicon/hibmc: Enable this hot plug detect of irq feature Yongbang Shi
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

DP controller can support generating a color bar signal over the
DisplayPort interface. This can be useful to check for possible DDR
or GPU problems, as the signal generator resides completely in the DP
block. Add debugfs file that controls colorbar generator.

echo: config the color bar register to display
cat: print the color bar configuration

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v3 -> v4:
  - add comments in hibmc_control_write(), suggested by Dmitry Baryshkov.
v2 -> v3:
  - rewrite the commit log, suggested by Dmitry Baryshkov.
  - move colorbar debugfs entry to this patch, suggested by Dmitry Baryshkov.
  - change binary format to integer format, suggested by Dmitry Baryshkov.
v1 -> v2:
  - add colorbar introduction in commit, suggested by Dmitry Baryshkov.
  - splittting colorbar and debugfs in different patches, suggested by Dmitry Baryshkov.
  - deleting edid decoder and its debugfs, suggested by Dmitry Baryshkov.
  - using debugfs_init() callback, suggested by Dmitry Baryshkov.
---
 drivers/gpu/drm/hisilicon/hibmc/Makefile      |   3 +-
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  43 ++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  29 +++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |   3 +
 .../drm/hisilicon/hibmc/hibmc_drm_debugfs.c   | 104 ++++++++++++++++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    |   1 +
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |   2 +
 7 files changed, 184 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c

diff --git a/drivers/gpu/drm/hisilicon/hibmc/Makefile b/drivers/gpu/drm/hisilicon/hibmc/Makefile
index 43de077d6769..1f65c683282f 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/Makefile
+++ b/drivers/gpu/drm/hisilicon/hibmc/Makefile
@@ -1,5 +1,6 @@
 # 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_serdes.o hibmc_drm_dp.o
+	       dp/dp_aux.o dp/dp_link.o dp/dp_hw.o dp/dp_serdes.o hibmc_drm_dp.o \
+	       hibmc_drm_debugfs.o
 
 obj-$(CONFIG_DRM_HISI_HIBMC) += hibmc-drm.o
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index aa9354a996c9..ce7cb07815b2 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -226,3 +226,46 @@ int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode)
 
 	return 0;
 }
+
+static const struct hibmc_dp_color_raw g_rgb_raw[] = {
+	{CBAR_COLOR_BAR, 0x000, 0x000, 0x000},
+	{CBAR_WHITE,     0xfff, 0xfff, 0xfff},
+	{CBAR_RED,       0xfff, 0x000, 0x000},
+	{CBAR_ORANGE,    0xfff, 0x800, 0x000},
+	{CBAR_YELLOW,    0xfff, 0xfff, 0x000},
+	{CBAR_GREEN,     0x000, 0xfff, 0x000},
+	{CBAR_CYAN,      0x000, 0x800, 0x800},
+	{CBAR_BLUE,      0x000, 0x000, 0xfff},
+	{CBAR_PURPLE,    0x800, 0x000, 0x800},
+	{CBAR_BLACK,     0x000, 0x000, 0x000},
+};
+
+void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg)
+{
+	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
+	struct hibmc_dp_color_raw raw_data;
+
+	if (cfg->enable) {
+		hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(9),
+					 cfg->self_timing);
+		hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, GENMASK(8, 1),
+					 cfg->dynamic_rate);
+		if (cfg->pattern == CBAR_COLOR_BAR) {
+			hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(10), 0);
+		} else {
+			raw_data = g_rgb_raw[cfg->pattern];
+			drm_dbg_dp(dp->drm_dev, "r:%x g:%x b:%x\n", raw_data.r_value,
+				   raw_data.g_value, raw_data.b_value);
+			hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(10), 1);
+			hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, GENMASK(23, 12),
+						 raw_data.r_value);
+			hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL1, GENMASK(23, 12),
+						 raw_data.g_value);
+			hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL1, GENMASK(11, 0),
+						 raw_data.b_value);
+		}
+	}
+
+	hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_COLOR_BAR_CTRL, BIT(0), cfg->enable);
+	writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL);
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
index 53b6d0beecea..83a53dae8012 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -14,6 +14,33 @@
 
 struct hibmc_dp_dev;
 
+enum hibmc_dp_cbar_pattern {
+	CBAR_COLOR_BAR,
+	CBAR_WHITE,
+	CBAR_RED,
+	CBAR_ORANGE,
+	CBAR_YELLOW,
+	CBAR_GREEN,
+	CBAR_CYAN,
+	CBAR_BLUE,
+	CBAR_PURPLE,
+	CBAR_BLACK,
+};
+
+struct hibmc_dp_color_raw {
+	enum hibmc_dp_cbar_pattern pattern;
+	u32 r_value;
+	u32 g_value;
+	u32 b_value;
+};
+
+struct hibmc_dp_cbar_cfg {
+	u8 enable;
+	u8 self_timing;
+	u8 dynamic_rate; /* 0:static, 1-255(frame):dynamic */
+	enum hibmc_dp_cbar_pattern pattern;
+};
+
 struct hibmc_dp {
 	struct hibmc_dp_dev *dp_dev;
 	struct drm_device *drm_dev;
@@ -21,10 +48,12 @@ struct hibmc_dp {
 	struct drm_connector connector;
 	void __iomem *mmio;
 	struct drm_dp_aux aux;
+	struct hibmc_dp_cbar_cfg cfg;
 };
 
 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);
+void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg);
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index 6eb76decc636..5614b727a710 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -67,6 +67,9 @@
 #define HIBMC_DP_CFG_STREAM_HTOTAL_SIZE		GENMASK(31, 16)
 #define HIBMC_DP_CFG_STREAM_HBLANK_SIZE		GENMASK(15, 0)
 
+#define HIBMC_DP_COLOR_BAR_CTRL			0x260
+#define HIBMC_DP_COLOR_BAR_CTRL1		0x264
+
 #define HIBMC_DP_TIMING_GEN_CONFIG0		0x26c
 #define HIBMC_DP_CFG_TIMING_GEN0_HACTIVE	GENMASK(31, 16)
 #define HIBMC_DP_CFG_TIMING_GEN0_HBLANK		GENMASK(15, 0)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
new file mode 100644
index 000000000000..f585387c3a49
--- /dev/null
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// Copyright (c) 2024 Hisilicon Limited.
+
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/seq_file.h>
+#include <linux/pci.h>
+
+#include <drm/drm_drv.h>
+#include <drm/drm_file.h>
+#include <drm/drm_debugfs.h>
+#include <drm/drm_edid.h>
+
+#include "hibmc_drm_drv.h"
+
+#define MAX_BUF_SIZE 12
+
+static ssize_t hibmc_control_write(struct file *file, const char __user *user_buf,
+				   size_t count, loff_t *ppos)
+{
+	struct hibmc_drm_private *priv = file_inode(file)->i_private;
+	struct hibmc_dp_cbar_cfg *cfg = &priv->dp.cfg;
+	int ret, idx;
+	u8 buf[MAX_BUF_SIZE];
+
+	if (count >= MAX_BUF_SIZE)
+		return -EINVAL;
+
+	if (copy_from_user(buf, user_buf, count))
+		return -EFAULT;
+
+	buf[count] = '\0';
+
+	/* Only 4 parameters is allowed, the ranger are as follow:
+	 * [0] enable/disable colorbar feature
+	       0: enable colorbar, 1: disable colorbar
+	 * [1] the timing source of colorbar displaying
+	       0: timing follows XDP, 1: internal self timing
+	 * [2] the movment of colorbar displaying
+	       0: static colorbar image,
+	 *     1~255: right shifting a type of color per (1~255)frames
+	 * [3] the color type of colorbar displaying
+	       0~9: color bar, white, red, orange,
+	 *          yellow, green, cyan, bule, pupper, black
+	 */
+	if (sscanf(buf, "%hhu %hhu %hhu %u", &cfg->enable, &cfg->self_timing,
+		   &cfg->dynamic_rate, &cfg->pattern) != 4) {
+		return -EINVAL;
+	}
+
+	if (cfg->pattern > 9 || cfg->enable > 1 || cfg->self_timing > 1)
+		return -EINVAL;
+
+	ret = drm_dev_enter(&priv->dev, &idx);
+	if (!ret)
+		return -ENODEV;
+
+	hibmc_dp_set_cbar(&priv->dp, cfg);
+
+	drm_dev_exit(idx);
+
+	return count;
+}
+
+static int hibmc_dp_dbgfs_show(struct seq_file *m, void *arg)
+{
+	struct hibmc_drm_private *priv = m->private;
+	struct hibmc_dp_cbar_cfg *cfg = &priv->dp.cfg;
+	int idx;
+
+	if (!drm_dev_enter(&priv->dev, &idx))
+		return -ENODEV;
+
+	seq_printf(m, "hibmc dp colorbar cfg: %u %u %u %u\n", cfg->enable, cfg->self_timing,
+		   cfg->dynamic_rate, cfg->pattern);
+
+	drm_dev_exit(idx);
+
+	return 0;
+}
+
+static int hibmc_open(struct inode *inode, struct file *filp)
+{
+	return single_open(filp, hibmc_dp_dbgfs_show, inode->i_private);
+}
+
+static const struct file_operations hibmc_dbg_fops = {
+	.owner   = THIS_MODULE,
+	.write   = hibmc_control_write,
+	.read    = seq_read,
+	.open    = hibmc_open,
+	.llseek  = seq_lseek,
+	.release = single_release,
+};
+
+void hibmc_debugfs_init(struct drm_connector *connector, struct dentry *root)
+{
+	struct drm_device *dev = connector->dev;
+	struct hibmc_drm_private *priv = to_hibmc_drm_private(dev);
+
+	/* create the file in drm directory, so we don't need to remove manually */
+	debugfs_create_file("colorbar-cfg", 0200,
+			    root, priv, &hibmc_dbg_fops);
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
index 0256724d8b9b..8d3c67dcb5b3 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -59,6 +59,7 @@ static const struct drm_connector_funcs hibmc_dp_conn_funcs = {
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 	.late_register = hibmc_dp_late_register,
 	.early_unregister = hibmc_dp_early_unregister,
+	.debugfs_init = hibmc_debugfs_init,
 };
 
 static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index 3ddd71aada66..bc89e4b9f4e3 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -69,4 +69,6 @@ int hibmc_ddc_create(struct drm_device *drm_dev, struct hibmc_vdac *connector);
 
 int hibmc_dp_init(struct hibmc_drm_private *priv);
 
+void hibmc_debugfs_init(struct drm_connector *connector, struct dentry *root);
+
 #endif
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 7/9] drm/hisilicon/hibmc: Enable this hot plug detect of irq feature
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
                   ` (5 preceding siblings ...)
  2025-03-19  3:24 ` [PATCH v7 drm-dp 6/9] drm/hisilicon/hibmc: Add colorbar-cfg feature and its debugfs file Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 8/9] drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD Yongbang Shi
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

Add HPD interrupt enable functions in drm framework, and also add
detect_ctx functions. Because of the debouncing when HPD pulled out,
add 200 ms delay in detect. Add link reset process to reset link status
when a new connector pulgged in.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v5 -> v6:
  - add detect_ctx with 200ms delay, suggested by Dmitry Baryshkov.
v4 -> v5:
  - separate the vga part commit, suggested by Dmitry Baryshkov.
v3 -> v4:
  - add link reset of rates and lanes in pre link training process, suggested by Dmitry Baryshkov.
  - add vdac detect and connected/disconnected status to enable HPD process, suggested by Dmitry Baryshkov.
  - remove a drm_client, suggested by Dmitry Baryshkov.
  - fix build errors reported by kernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202502231304.BCzV4Y8D-lkp@intel.com/
v2 -> v3:
  - remove mdelay(100) hpd function in ISR, suggested by Dmitry Baryshkov.
  - remove enble_display in ISR, suggested by Dmitry Baryshkov.
  - change drm_kms_helper_connector_hotplug_event() to
    drm_connector_helper_hpd_irq_event(), suggested by Dmitry Baryshkov.
  - move macros to dp_reg.h, suggested by Dmitry Baryshkov.
  - remove struct irqs, suggested by Dmitry Baryshkov.
  - split this patch into two parts, suggested by Dmitry Baryshkov.
  - add a drm client dev to handle HPD event.
v1 -> v2:
  - optimizing the description in commit message, suggested by Dmitry Baryshkov.
  - add mdelay(100) comments, suggested by Dmitry Baryshkov.
  - deleting display enable in hpd event, suggested by Dmitry Baryshkov.
---
 .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  1 +
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    | 36 ++++++++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  5 +++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 42 +++++++++++++++++++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  2 +
 5 files changed, 86 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
index c5feef8dc27d..08f9e1caf7fc 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
@@ -16,5 +16,6 @@
 #define HIBMC_DP_SYNC_EN_MASK		0x3
 #define HIBMC_DP_LINK_RATE_CAL		27
 #define HIBMC_DP_SYNC_DELAY(lanes)	((lanes) == 0x2 ? 86 : 46)
+#define HIBMC_DP_INT_ENABLE		0xc
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index ce7cb07815b2..8f0daec7d174 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -189,6 +189,36 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
 	return 0;
 }
 
+void hibmc_dp_enable_int(struct hibmc_dp *dp)
+{
+	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
+
+	writel(HIBMC_DP_INT_ENABLE, dp_dev->base + HIBMC_DP_INTR_ENABLE);
+}
+
+void hibmc_dp_disable_int(struct hibmc_dp *dp)
+{
+	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
+
+	writel(0, dp_dev->base + HIBMC_DP_INTR_ENABLE);
+	writel(HIBMC_DP_INT_RST, dp_dev->base + HIBMC_DP_INTR_ORIGINAL_STATUS);
+}
+
+void hibmc_dp_hpd_cfg(struct hibmc_dp *dp)
+{
+	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
+
+	hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_SYNC_LEN_SEL, 0x0);
+	hibmc_dp_reg_write_field(dp_dev, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_TIMER_TIMEOUT, 0x1);
+	hibmc_dp_reg_write_field(dp->dp_dev, HIBMC_DP_AUX_REQ, HIBMC_DP_CFG_AUX_MIN_PULSE_NUM, 0x9);
+	writel(HIBMC_DP_HDCP, dp_dev->base + HIBMC_DP_HDCP_CFG);
+	writel(0, dp_dev->base + HIBMC_DP_INTR_ENABLE);
+	writel(HIBMC_DP_INT_RST, dp_dev->base + HIBMC_DP_INTR_ORIGINAL_STATUS);
+	writel(HIBMC_DP_INT_ENABLE, dp_dev->base + HIBMC_DP_INTR_ENABLE);
+	writel(HIBMC_DP_DPTX_RST, dp_dev->base + HIBMC_DP_DPTX_RST_CTRL);
+	writel(HIBMC_DP_CLK_EN, dp_dev->base + HIBMC_DP_DPTX_CLK_CTRL);
+}
+
 void hibmc_dp_display_en(struct hibmc_dp *dp, bool enable)
 {
 	struct hibmc_dp_dev *dp_dev = dp->dp_dev;
@@ -227,6 +257,12 @@ int hibmc_dp_mode_set(struct hibmc_dp *dp, struct drm_display_mode *mode)
 	return 0;
 }
 
+void hibmc_dp_reset_link(struct hibmc_dp *dp)
+{
+	dp->dp_dev->link.status.clock_recovered = false;
+	dp->dp_dev->link.status.channel_equalized = false;
+}
+
 static const struct hibmc_dp_color_raw g_rgb_raw[] = {
 	{CBAR_COLOR_BAR, 0x000, 0x000, 0x000},
 	{CBAR_WHITE,     0xfff, 0xfff, 0xfff},
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
index 83a53dae8012..665f5b166dfb 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -49,11 +49,16 @@ struct hibmc_dp {
 	void __iomem *mmio;
 	struct drm_dp_aux aux;
 	struct hibmc_dp_cbar_cfg cfg;
+	u32 irq_status;
 };
 
 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);
 void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg);
+void hibmc_dp_reset_link(struct hibmc_dp *dp);
+void hibmc_dp_hpd_cfg(struct hibmc_dp *dp);
+void hibmc_dp_enable_int(struct hibmc_dp *dp);
+void hibmc_dp_disable_int(struct hibmc_dp *dp);
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
index 8d3c67dcb5b3..881066c1221a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -13,6 +13,8 @@
 #include "hibmc_drm_drv.h"
 #include "dp/dp_hw.h"
 
+#define DP_MASKED_SINK_HPD_PLUG_INT	BIT(2)
+
 static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
 {
 	struct hibmc_dp *dp = to_hibmc_dp(connector);
@@ -33,14 +35,25 @@ static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
 	return count;
 }
 
+static int hibmc_dp_detect(struct drm_connector *connector,
+			   struct drm_modeset_acquire_ctx *ctx, bool force)
+{
+	mdelay(200);
+
+	return drm_connector_helper_detect_from_ddc(connector, ctx, force);
+}
+
 static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
 	.get_modes = hibmc_dp_connector_get_modes,
+	.detect_ctx = hibmc_dp_detect,
 };
 
 static int hibmc_dp_late_register(struct drm_connector *connector)
 {
 	struct hibmc_dp *dp = to_hibmc_dp(connector);
 
+	hibmc_dp_enable_int(dp);
+
 	return drm_dp_aux_register(&dp->aux);
 }
 
@@ -49,6 +62,8 @@ static void hibmc_dp_early_unregister(struct drm_connector *connector)
 	struct hibmc_dp *dp = to_hibmc_dp(connector);
 
 	drm_dp_aux_unregister(&dp->aux);
+
+	hibmc_dp_disable_int(dp);
 }
 
 static const struct drm_connector_funcs hibmc_dp_conn_funcs = {
@@ -100,6 +115,31 @@ static const struct drm_encoder_helper_funcs hibmc_dp_encoder_helper_funcs = {
 	.atomic_disable = hibmc_dp_encoder_disable,
 };
 
+irqreturn_t hibmc_dp_hpd_isr(int irq, void *arg)
+{
+	struct drm_device *dev = (struct drm_device *)arg;
+	struct hibmc_drm_private *priv = to_hibmc_drm_private(dev);
+	int idx;
+
+	if (!drm_dev_enter(dev, &idx))
+		return -ENODEV;
+
+	if (priv->dp.irq_status & DP_MASKED_SINK_HPD_PLUG_INT) {
+		drm_dbg_dp(&priv->dev, "HPD IN isr occur!\n");
+		hibmc_dp_hpd_cfg(&priv->dp);
+	} else {
+		drm_dbg_dp(&priv->dev, "HPD OUT isr occur!\n");
+		hibmc_dp_reset_link(&priv->dp);
+	}
+
+	if (dev->registered)
+		drm_connector_helper_hpd_irq_event(&priv->dp.connector);
+
+	drm_dev_exit(idx);
+
+	return IRQ_HANDLED;
+}
+
 int hibmc_dp_init(struct hibmc_drm_private *priv)
 {
 	struct drm_device *dev = &priv->dev;
@@ -140,5 +180,7 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
 
 	drm_connector_attach_encoder(connector, encoder);
 
+	connector->polled = DRM_CONNECTOR_POLL_HPD;
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index bc89e4b9f4e3..daed1330b961 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -71,4 +71,6 @@ int hibmc_dp_init(struct hibmc_drm_private *priv);
 
 void hibmc_debugfs_init(struct drm_connector *connector, struct dentry *root);
 
+irqreturn_t hibmc_dp_hpd_isr(int irq, void *arg);
+
 #endif
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 8/9] drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
                   ` (6 preceding siblings ...)
  2025-03-19  3:24 ` [PATCH v7 drm-dp 7/9] drm/hisilicon/hibmc: Enable this hot plug detect of irq feature Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19  3:24 ` [PATCH v7 drm-dp 9/9] drm/hisilicon/hibmc: Add vga connector detect functions Yongbang Shi
  2025-03-19 11:54 ` [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Dmitry Baryshkov
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

To realize HPD feature, request irq for HPD , add its handler function.
We use pci_alloc_irq_vectors() to get our msi irq, because we have two
interrupts now.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
ChangeLog:
v4 -> v5:
  - remove pci_disable_msi() in hibmc_unload()
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |  3 +
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   | 74 +++++++++++++++----
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  3 +
 3 files changed, 66 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index 5614b727a710..394b1e933c3a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -99,6 +99,9 @@
 
 #define HIBMC_DP_TIMING_SYNC_CTRL		0xFF0
 
+#define HIBMC_DP_INTSTAT			0x1e0724
+#define HIBMC_DP_INTCLR				0x1e0728
+
 /* dp serdes reg */
 #define HIBMC_DP_HOST_OFFSET		0x10000
 #define HIBMC_DP_LANE0_RATE_OFFSET	0x4
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index 98b01c8aee8e..768b97f9e74a 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -32,6 +32,8 @@
 
 DEFINE_DRM_GEM_FOPS(hibmc_fops);
 
+static const char *g_irqs_names_map[HIBMC_MAX_VECTORS] = { "vblank", "hpd" };
+
 static irqreturn_t hibmc_interrupt(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *)arg;
@@ -49,6 +51,22 @@ static irqreturn_t hibmc_interrupt(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t hibmc_dp_interrupt(int irq, void *arg)
+{
+	struct drm_device *dev = (struct drm_device *)arg;
+	struct hibmc_drm_private *priv = to_hibmc_drm_private(dev);
+	u32 status;
+
+	status = readl(priv->mmio + HIBMC_DP_INTSTAT);
+	if (status) {
+		priv->dp.irq_status = status;
+		writel(status, priv->mmio + HIBMC_DP_INTCLR);
+		return IRQ_WAKE_THREAD;
+	}
+
+	return IRQ_HANDLED;
+}
+
 static int hibmc_dumb_create(struct drm_file *file, struct drm_device *dev,
 			     struct drm_mode_create_dumb *args)
 {
@@ -251,15 +269,48 @@ static int hibmc_hw_init(struct hibmc_drm_private *priv)
 	return 0;
 }
 
-static int hibmc_unload(struct drm_device *dev)
+static void hibmc_unload(struct drm_device *dev)
 {
-	struct pci_dev *pdev = to_pci_dev(dev->dev);
-
 	drm_atomic_helper_shutdown(dev);
+}
 
-	free_irq(pdev->irq, dev);
+static int hibmc_msi_init(struct drm_device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	char name[32] = {0};
+	int valid_irq_num;
+	int irq;
+	int ret;
 
-	pci_disable_msi(to_pci_dev(dev->dev));
+	ret = pci_alloc_irq_vectors(pdev, HIBMC_MIN_VECTORS,
+				    HIBMC_MAX_VECTORS, PCI_IRQ_MSI);
+	if (ret < 0) {
+		drm_err(dev, "enabling MSI failed: %d\n", ret);
+		return ret;
+	}
+
+	valid_irq_num = ret;
+
+	for (int i = 0; i < valid_irq_num; i++) {
+		snprintf(name, ARRAY_SIZE(name) - 1, "%s-%s-%s",
+			 dev->driver->name, pci_name(pdev), g_irqs_names_map[i]);
+
+		irq = pci_irq_vector(pdev, i);
+
+		if (i)
+			/* PCI devices require shared interrupts. */
+			ret = devm_request_threaded_irq(&pdev->dev, irq,
+							hibmc_dp_interrupt,
+							hibmc_dp_hpd_isr,
+							IRQF_SHARED, name, dev);
+		else
+			ret = devm_request_irq(&pdev->dev, irq, hibmc_interrupt,
+					       IRQF_SHARED, name, dev);
+		if (ret) {
+			drm_err(dev, "install irq failed: %d\n", ret);
+			return ret;
+		}
+	}
 
 	return 0;
 }
@@ -291,15 +342,10 @@ static int hibmc_load(struct drm_device *dev)
 		goto err;
 	}
 
-	ret = pci_enable_msi(pdev);
+	ret = hibmc_msi_init(dev);
 	if (ret) {
-		drm_warn(dev, "enabling MSI failed: %d\n", ret);
-	} else {
-		/* PCI devices require shared interrupts. */
-		ret = request_irq(pdev->irq, hibmc_interrupt, IRQF_SHARED,
-				  dev->driver->name, dev);
-		if (ret)
-			drm_warn(dev, "install irq failed: %d\n", ret);
+		drm_err(dev, "hibmc msi init failed, ret:%d\n", ret);
+		goto err;
 	}
 
 	/* reset all the states of crtc/plane/encoder/connector */
@@ -375,7 +421,7 @@ static void hibmc_pci_remove(struct pci_dev *pdev)
 
 static void hibmc_pci_shutdown(struct pci_dev *pdev)
 {
-	drm_atomic_helper_shutdown(pci_get_drvdata(pdev));
+	hibmc_pci_remove(pdev);
 }
 
 static const struct pci_device_id hibmc_pci_table[] = {
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
index daed1330b961..274feabe7df0 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
@@ -22,6 +22,9 @@
 
 #include "dp/dp_hw.h"
 
+#define HIBMC_MIN_VECTORS	1
+#define HIBMC_MAX_VECTORS	2
+
 struct hibmc_vdac {
 	struct drm_device *dev;
 	struct drm_encoder encoder;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v7 drm-dp 9/9] drm/hisilicon/hibmc: Add vga connector detect functions
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
                   ` (7 preceding siblings ...)
  2025-03-19  3:24 ` [PATCH v7 drm-dp 8/9] drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD Yongbang Shi
@ 2025-03-19  3:24 ` Yongbang Shi
  2025-03-19 11:54 ` [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Dmitry Baryshkov
  9 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-19  3:24 UTC (permalink / raw)
  To: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

From: Baihan Li <libaihan@huawei.com>

Because the connected VGA connector would make driver can't get the
userspace call, adding detect_ctx in vga connector to make HPD active
userspace.

Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
index 05e19ea4c9f9..e8a527ede854 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c
@@ -60,6 +60,7 @@ static void hibmc_connector_destroy(struct drm_connector *connector)
 static const struct drm_connector_helper_funcs
 	hibmc_connector_helper_funcs = {
 	.get_modes = hibmc_connector_get_modes,
+	.detect_ctx = drm_connector_helper_detect_from_ddc,
 };
 
 static const struct drm_connector_funcs hibmc_connector_funcs = {
@@ -127,5 +128,7 @@ int hibmc_vdac_init(struct hibmc_drm_private *priv)
 
 	drm_connector_attach_encoder(connector, encoder);
 
+	connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
+
 	return 0;
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function
  2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
                   ` (8 preceding siblings ...)
  2025-03-19  3:24 ` [PATCH v7 drm-dp 9/9] drm/hisilicon/hibmc: Add vga connector detect functions Yongbang Shi
@ 2025-03-19 11:54 ` Dmitry Baryshkov
  2025-03-20  5:12   ` Yongbang Shi
  9 siblings, 1 reply; 24+ messages in thread
From: Dmitry Baryshkov @ 2025-03-19 11:54 UTC (permalink / raw)
  To: Yongbang Shi
  Cc: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei, liangjian010, chenjianmin,
	lidongming5, libaihan, shenjian15, shaojijie, dri-devel,
	linux-kernel

On Wed, Mar 19, 2025 at 11:24:26AM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@huawei.com>
> 
> To support DP HPD, edid printing, and colorbar display features based on
> the Hisislcon DP devices.
> ---
> ChangeLog:
> v6 -> v7:
>   - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula

Where was that suggestion? Also, would you please implement his feedback
regarding drm_edid_read()?

>   v6: https://lore.kernel.org/all/20250310040138.2025715-1-shiyongbang@huawei.com/
> v5 -> v6:
>   - fix the DP_SERDES_VOL2_PRE0 value after electrical test.
>   - move the detect_ctx() to the patch 7/9.
>   - add detect_ctx with 200ms delay, suggested by Dmitry Baryshkov.
>   v5: https://lore.kernel.org/all/20250307101640.4003229-1-shiyongbang@huawei.com/
> v4 -> v5:
>   - add commit log about hibmc_kms_init(), suggested by Dmitry Baryshkov.
>   - fix the format of block comments, suggested by Dmitry Baryshkov.
>   - add hibmc_dp_get_serdes_rate_cfg() to correct transferring serdes cfg.
>   - separate the vga part commit, suggested by Dmitry Baryshkov.
>   - remove pci_disable_msi() in hibmc_unload()
>   v4: https://lore.kernel.org/all/20250305112647.2344438-1-shiyongbang@huawei.com/
> v3 -> v4:
>   - fix the serdes cfg in hibmc_dp_serdes_set_tx_cfg(), suggested by Dmitry Baryshkov.
>   - move the dp serdes registers to dp_reg.h, suggested by Dmitry Baryshkov.
>   - add comments for if-statement of dp_init(), suggested by Dmitry Baryshkov.
>   - fix the comment log to imperative sentence, suggested by Dmitry Baryshkov.
>   - add comments in hibmc_control_write(), suggested by Dmitry Baryshkov.
>   - add link reset of rates and lanes in pre link training process, suggested by Dmitry Baryshkov.
>   - add vdac detect and connected/disconnected status to enable HPD process, suggested by Dmitry Baryshkov.
>   - remove a drm_client, suggested by Dmitry Baryshkov.
>   - fix build errors reported by kernel test robot <lkp@intel.com>
>     Closes: https://lore.kernel.org/oe-kbuild-all/202502231304.BCzV4Y8D-lkp@intel.com/
>   v3: https://lore.kernel.org/all/20250222025102.1519798-1-shiyongbang@huawei.com/
> v2 -> v3:
>   - restructuring the header p_reg.h, suggested by Dmitry Baryshkov.
>   - add commit log about dp serdes, suggested by Dmitry Baryshkov.
>   - return value in hibmc_dp_serdes_init(), suggested by Dmitry Baryshkov.
>   - add static const in the array of serdes_tx_cfg[], suggested by Dmitry Baryshkov.
>   - change drm_warn to drm_dbg_dp, suggested by Dmitry Baryshkov.
>   - add explanations about dp serdes macros, suggested by Dmitry Baryshkov.
>   - change commit to an imperative sentence, suggested by Dmitry Baryshkov.
>   - put HIBMC_DP_HOST_SERDES_CTRL in dp_serdes.h, suggested by Dmitry Baryshkov.
>   - split the patch into two parts, suggested by Dmitry Baryshkov.
>   - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
>   - rewrite the commit log, suggested by Dmitry Baryshkov.
>   - move colorbar debugfs entry to this patch, suggested by Dmitry Baryshkov.
>   - change binary format to integer format, suggested by Dmitry Baryshkov.
>   - remove mdelay(100) hpd function in ISR, suggested by Dmitry Baryshkov.
>   - remove enble_display in ISR, suggested by Dmitry Baryshkov.
>   - change drm_kms_helper_connector_hotplug_event() to
>     drm_connector_helper_hpd_irq_event(), suggested by Dmitry Baryshkov.
>   - move macros to dp_reg.h, suggested by Dmitry Baryshkov.
>   - remove struct irqs, suggested by Dmitry Baryshkov.
>   - split this patch into two parts, suggested by Dmitry Baryshkov.
>   v2: https://lore.kernel.org/all/20250210144959.100551-1-shiyongbang@huawei.com/
> v1 -> v2:
>   - splittting the patch and add more detailed the changes in the commit message, suggested by Dmitry Baryshkov.
>   - changing all names of dp phy to dp serdes.
>   - deleting type conversion, suggested by Dmitry Baryshkov.
>   - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
>   - add colorbar introduction in commit, suggested by Dmitry Baryshkov.
>   - deleting edid decoder and its debugfs, suggested by Dmitry Baryshkov.
>   - using debugfs_init() callback, suggested by Dmitry Baryshkov.
>   - splittting colorbar and debugfs in different patches, suggested by Dmitry Baryshkov.
>   - optimizing the description in commit message, suggested by Dmitry Baryshkov.
>   - add mdelay(100) comments, suggested by Dmitry Baryshkov.
>   - deleting display enable in hpd event, suggested by Dmitry Baryshkov.
>   v1: https://lore.kernel.org/all/20250127032024.1542219-1-shiyongbang@huawei.com/
> ---
> 
> Baihan Li (9):
>   drm/hisilicon/hibmc: Restructuring the header dp_reg.h
>   drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage
>     and pre-emphasis
>   drm/hisilicon/hibmc: Add dp serdes cfg in dp process
>   drm/hisilicon/hibmc: Refactor the member of drm_aux in struct hibmc_dp
>   drm/hisilicon/hibmc: Getting connector info and EDID by using AUX
>     channel
>   drm/hisilicon/hibmc: Add colorbar-cfg feature and its debugfs file
>   drm/hisilicon/hibmc: Enable this hot plug detect of irq feature
>   drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD
>   drm/hisilicon/hibmc: Add vga connector detect functions
> 
>  drivers/gpu/drm/hisilicon/hibmc/Makefile      |   3 +-
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  16 ++-
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  10 +-
>  .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |   2 +
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  91 +++++++++++-
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  36 +++++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  |  97 +++++++++----
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   | 130 +++++++++++++-----
>  .../gpu/drm/hisilicon/hibmc/dp/dp_serdes.c    |  71 ++++++++++
>  .../drm/hisilicon/hibmc/hibmc_drm_debugfs.c   | 104 ++++++++++++++
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    |  78 ++++++++++-
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  87 +++++++++---
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  12 ++
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  |   3 +
>  14 files changed, 638 insertions(+), 102 deletions(-)
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
>  create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
> 
> -- 
> 2.33.0
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function
  2025-03-19 11:54 ` [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Dmitry Baryshkov
@ 2025-03-20  5:12   ` Yongbang Shi
  0 siblings, 0 replies; 24+ messages in thread
From: Yongbang Shi @ 2025-03-20  5:12 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: xinliang.liu, tiantao6, maarten.lankhorst, mripard, tzimmermann,
	airlied, daniel, kong.kongxinwei, liangjian010, chenjianmin,
	lidongming5, libaihan, shenjian15, shaojijie, dri-devel,
	linux-kernel, shiyongbang

> On Wed, Mar 19, 2025 at 11:24:26AM +0800, Yongbang Shi wrote:
>> From: Baihan Li <libaihan@huawei.com>
>>
>> To support DP HPD, edid printing, and colorbar display features based on
>> the Hisislcon DP devices.
>> ---
>> ChangeLog:
>> v6 -> v7:
>>    - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
> Where was that suggestion? Also, would you please implement his feedback
> regarding drm_edid_read()?

Hi Dmitry,
Thanks for yuor mentioned, I misunderstood his suggestion. I will change to drm_edid_read().
Thanks, Baihan.


>>    v6: https://lore.kernel.org/all/20250310040138.2025715-1-shiyongbang@huawei.com/
>> v5 -> v6:
>>    - fix the DP_SERDES_VOL2_PRE0 value after electrical test.
>>    - move the detect_ctx() to the patch 7/9.
>>    - add detect_ctx with 200ms delay, suggested by Dmitry Baryshkov.
>>    v5: https://lore.kernel.org/all/20250307101640.4003229-1-shiyongbang@huawei.com/
>> v4 -> v5:
>>    - add commit log about hibmc_kms_init(), suggested by Dmitry Baryshkov.
>>    - fix the format of block comments, suggested by Dmitry Baryshkov.
>>    - add hibmc_dp_get_serdes_rate_cfg() to correct transferring serdes cfg.
>>    - separate the vga part commit, suggested by Dmitry Baryshkov.
>>    - remove pci_disable_msi() in hibmc_unload()
>>    v4: https://lore.kernel.org/all/20250305112647.2344438-1-shiyongbang@huawei.com/
>> v3 -> v4:
>>    - fix the serdes cfg in hibmc_dp_serdes_set_tx_cfg(), suggested by Dmitry Baryshkov.
>>    - move the dp serdes registers to dp_reg.h, suggested by Dmitry Baryshkov.
>>    - add comments for if-statement of dp_init(), suggested by Dmitry Baryshkov.
>>    - fix the comment log to imperative sentence, suggested by Dmitry Baryshkov.
>>    - add comments in hibmc_control_write(), suggested by Dmitry Baryshkov.
>>    - add link reset of rates and lanes in pre link training process, suggested by Dmitry Baryshkov.
>>    - add vdac detect and connected/disconnected status to enable HPD process, suggested by Dmitry Baryshkov.
>>    - remove a drm_client, suggested by Dmitry Baryshkov.
>>    - fix build errors reported by kernel test robot <lkp@intel.com>
>>      Closes: https://lore.kernel.org/oe-kbuild-all/202502231304.BCzV4Y8D-lkp@intel.com/
>>    v3: https://lore.kernel.org/all/20250222025102.1519798-1-shiyongbang@huawei.com/
>> v2 -> v3:
>>    - restructuring the header p_reg.h, suggested by Dmitry Baryshkov.
>>    - add commit log about dp serdes, suggested by Dmitry Baryshkov.
>>    - return value in hibmc_dp_serdes_init(), suggested by Dmitry Baryshkov.
>>    - add static const in the array of serdes_tx_cfg[], suggested by Dmitry Baryshkov.
>>    - change drm_warn to drm_dbg_dp, suggested by Dmitry Baryshkov.
>>    - add explanations about dp serdes macros, suggested by Dmitry Baryshkov.
>>    - change commit to an imperative sentence, suggested by Dmitry Baryshkov.
>>    - put HIBMC_DP_HOST_SERDES_CTRL in dp_serdes.h, suggested by Dmitry Baryshkov.
>>    - split the patch into two parts, suggested by Dmitry Baryshkov.
>>    - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
>>    - rewrite the commit log, suggested by Dmitry Baryshkov.
>>    - move colorbar debugfs entry to this patch, suggested by Dmitry Baryshkov.
>>    - change binary format to integer format, suggested by Dmitry Baryshkov.
>>    - remove mdelay(100) hpd function in ISR, suggested by Dmitry Baryshkov.
>>    - remove enble_display in ISR, suggested by Dmitry Baryshkov.
>>    - change drm_kms_helper_connector_hotplug_event() to
>>      drm_connector_helper_hpd_irq_event(), suggested by Dmitry Baryshkov.
>>    - move macros to dp_reg.h, suggested by Dmitry Baryshkov.
>>    - remove struct irqs, suggested by Dmitry Baryshkov.
>>    - split this patch into two parts, suggested by Dmitry Baryshkov.
>>    v2: https://lore.kernel.org/all/20250210144959.100551-1-shiyongbang@huawei.com/
>> v1 -> v2:
>>    - splittting the patch and add more detailed the changes in the commit message, suggested by Dmitry Baryshkov.
>>    - changing all names of dp phy to dp serdes.
>>    - deleting type conversion, suggested by Dmitry Baryshkov.
>>    - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
>>    - add colorbar introduction in commit, suggested by Dmitry Baryshkov.
>>    - deleting edid decoder and its debugfs, suggested by Dmitry Baryshkov.
>>    - using debugfs_init() callback, suggested by Dmitry Baryshkov.
>>    - splittting colorbar and debugfs in different patches, suggested by Dmitry Baryshkov.
>>    - optimizing the description in commit message, suggested by Dmitry Baryshkov.
>>    - add mdelay(100) comments, suggested by Dmitry Baryshkov.
>>    - deleting display enable in hpd event, suggested by Dmitry Baryshkov.
>>    v1: https://lore.kernel.org/all/20250127032024.1542219-1-shiyongbang@huawei.com/
>> ---
>>
>> Baihan Li (9):
>>    drm/hisilicon/hibmc: Restructuring the header dp_reg.h
>>    drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage
>>      and pre-emphasis
>>    drm/hisilicon/hibmc: Add dp serdes cfg in dp process
>>    drm/hisilicon/hibmc: Refactor the member of drm_aux in struct hibmc_dp
>>    drm/hisilicon/hibmc: Getting connector info and EDID by using AUX
>>      channel
>>    drm/hisilicon/hibmc: Add colorbar-cfg feature and its debugfs file
>>    drm/hisilicon/hibmc: Enable this hot plug detect of irq feature
>>    drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD
>>    drm/hisilicon/hibmc: Add vga connector detect functions
>>
>>   drivers/gpu/drm/hisilicon/hibmc/Makefile      |   3 +-
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  16 ++-
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h  |  10 +-
>>   .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |   2 +
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    |  91 +++++++++++-
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  36 +++++
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_link.c  |  97 +++++++++----
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   | 130 +++++++++++++-----
>>   .../gpu/drm/hisilicon/hibmc/dp/dp_serdes.c    |  71 ++++++++++
>>   .../drm/hisilicon/hibmc/hibmc_drm_debugfs.c   | 104 ++++++++++++++
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    |  78 ++++++++++-
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  87 +++++++++---
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  12 ++
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c  |   3 +
>>   14 files changed, 638 insertions(+), 102 deletions(-)
>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/dp/dp_serdes.c
>>   create mode 100644 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_debugfs.c
>>
>> -- 
>> 2.33.0
>>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-19  3:24 ` [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel Yongbang Shi
@ 2025-03-20  9:50   ` Jani Nikula
  2025-03-24 12:41     ` Yongbang Shi
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2025-03-20  9:50 UTC (permalink / raw)
  To: Yongbang Shi, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, shiyongbang, libaihan,
	shenjian15, shaojijie, dri-devel, linux-kernel

On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> From: Baihan Li <libaihan@huawei.com>
>
> Add registering drm_aux and use it to get connector edid with drm
> functions. Add ddc channel in connector initialization to put drm_aux
> in drm_connector.
>
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> ChangeLog:
> v6 -> v7:
>   - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula

I don't understand this, and I did not suggest such a thing.

BR,
Jani.


> v5 -> v6:
>   - move the detect_ctx() to the patch 7/9.
> v2 -> v3:
>   - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
> v1 -> v2:
>   - deleting type conversion, suggested by Dmitry Baryshkov.
>   - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
> ---
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  3 +-
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 ++++++++++++++++---
>  .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  5 +++
>  3 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> index ded9e7ce887a..e0bb9b14d9d8 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
> @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp)
>  				 HIBMC_DP_MIN_PULSE_NUM);
>  
>  	dp->aux.transfer = hibmc_dp_aux_xfer;
> -	dp->aux.is_remote = 0;
> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
> +	dp->aux.drm_dev = dp->drm_dev;
>  	drm_dp_aux_init(&dp->aux);
>  	dp->dp_dev->aux = &dp->aux;
>  }
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> index 603d6b198a54..0256724d8b9b 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> @@ -15,11 +15,20 @@
>  
>  static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>  {
> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
> +	const struct drm_edid *drm_edid;
>  	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
> +	if (!dp->aux.name)
> +		return 0;
> +
> +	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
> +
> +	drm_edid_connector_update(connector, drm_edid);
> +
> +	count = drm_edid_connector_add_modes(connector);
> +
> +	drm_edid_free(drm_edid);
>  
>  	return count;
>  }
> @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
>  	.get_modes = hibmc_dp_connector_get_modes,
>  };
>  
> +static int hibmc_dp_late_register(struct drm_connector *connector)
> +{
> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
> +
> +	return drm_dp_aux_register(&dp->aux);
> +}
> +
> +static void hibmc_dp_early_unregister(struct drm_connector *connector)
> +{
> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
> +
> +	drm_dp_aux_unregister(&dp->aux);
> +}
> +
>  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,
> +	.late_register = hibmc_dp_late_register,
> +	.early_unregister = hibmc_dp_early_unregister,
>  };
>  
>  static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
> @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
>  
>  	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
>  
> -	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
> -				 DRM_MODE_CONNECTOR_DisplayPort);
> +	ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs,
> +					  DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc);
>  	if (ret) {
>  		drm_err(dev, "init dp connector failed: %d\n", ret);
>  		return ret;
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> index d982f1e4b958..3ddd71aada66 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
> @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
>  	return container_of(connector, struct hibmc_vdac, connector);
>  }
>  
> +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector)
> +{
> +	return container_of(connector, struct hibmc_dp, connector);
> +}
> +
>  static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev)
>  {
>  	return container_of(dev, struct hibmc_drm_private, dev);

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-20  9:50   ` Jani Nikula
@ 2025-03-24 12:41     ` Yongbang Shi
  2025-03-24 13:33       ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Yongbang Shi @ 2025-03-24 12:41 UTC (permalink / raw)
  To: Jani Nikula, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang


> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>> From: Baihan Li <libaihan@huawei.com>
>>
>> Add registering drm_aux and use it to get connector edid with drm
>> functions. Add ddc channel in connector initialization to put drm_aux
>> in drm_connector.
>>
>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> ChangeLog:
>> v6 -> v7:
>>    - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
> I don't understand this, and I did not suggest such a thing.
>
> BR,
> Jani.
>
Hi Jani,

Is the modification of v8 correct?


>> v5 -> v6:
>>    - move the detect_ctx() to the patch 7/9.
>> v2 -> v3:
>>    - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
>> v1 -> v2:
>>    - deleting type conversion, suggested by Dmitry Baryshkov.
>>    - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  3 +-
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 ++++++++++++++++---
>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  5 +++
>>   3 files changed, 37 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>> index ded9e7ce887a..e0bb9b14d9d8 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>> @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp)
>>   				 HIBMC_DP_MIN_PULSE_NUM);
>>   
>>   	dp->aux.transfer = hibmc_dp_aux_xfer;
>> -	dp->aux.is_remote = 0;
>> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
>> +	dp->aux.drm_dev = dp->drm_dev;
>>   	drm_dp_aux_init(&dp->aux);
>>   	dp->dp_dev->aux = &dp->aux;
>>   }
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> index 603d6b198a54..0256724d8b9b 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> @@ -15,11 +15,20 @@
>>   
>>   static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>>   {
>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>> +	const struct drm_edid *drm_edid;
>>   	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
>> +	if (!dp->aux.name)
>> +		return 0;
>> +
>> +	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>> +
>> +	drm_edid_connector_update(connector, drm_edid);
>> +
>> +	count = drm_edid_connector_add_modes(connector);
>> +
>> +	drm_edid_free(drm_edid);
>>   
>>   	return count;
>>   }
>> @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
>>   	.get_modes = hibmc_dp_connector_get_modes,
>>   };
>>   
>> +static int hibmc_dp_late_register(struct drm_connector *connector)
>> +{
>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>> +
>> +	return drm_dp_aux_register(&dp->aux);
>> +}
>> +
>> +static void hibmc_dp_early_unregister(struct drm_connector *connector)
>> +{
>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>> +
>> +	drm_dp_aux_unregister(&dp->aux);
>> +}
>> +
>>   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,
>> +	.late_register = hibmc_dp_late_register,
>> +	.early_unregister = hibmc_dp_early_unregister,
>>   };
>>   
>>   static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
>> @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
>>   
>>   	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
>>   
>> -	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
>> -				 DRM_MODE_CONNECTOR_DisplayPort);
>> +	ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs,
>> +					  DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc);
>>   	if (ret) {
>>   		drm_err(dev, "init dp connector failed: %d\n", ret);
>>   		return ret;
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> index d982f1e4b958..3ddd71aada66 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>> @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
>>   	return container_of(connector, struct hibmc_vdac, connector);
>>   }
>>   
>> +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector)
>> +{
>> +	return container_of(connector, struct hibmc_dp, connector);
>> +}
>> +
>>   static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev)
>>   {
>>   	return container_of(dev, struct hibmc_drm_private, dev);

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-24 12:41     ` Yongbang Shi
@ 2025-03-24 13:33       ` Jani Nikula
  2025-03-25  3:14         ` Yongbang Shi
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2025-03-24 13:33 UTC (permalink / raw)
  To: Yongbang Shi, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang

On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>> From: Baihan Li <libaihan@huawei.com>
>>>
>>> Add registering drm_aux and use it to get connector edid with drm
>>> functions. Add ddc channel in connector initialization to put drm_aux
>>> in drm_connector.
>>>
>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>> ChangeLog:
>>> v6 -> v7:
>>>    - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>> I don't understand this, and I did not suggest such a thing.
>>
>> BR,
>> Jani.
>>
> Hi Jani,
>
> Is the modification of v8 correct?

I never received that for whatever reason.

>
>
>>> v5 -> v6:
>>>    - move the detect_ctx() to the patch 7/9.
>>> v2 -> v3:
>>>    - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
>>> v1 -> v2:
>>>    - deleting type conversion, suggested by Dmitry Baryshkov.
>>>    - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
>>> ---
>>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  3 +-
>>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 ++++++++++++++++---
>>>   .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  5 +++
>>>   3 files changed, 37 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>>> index ded9e7ce887a..e0bb9b14d9d8 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>>> @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp)
>>>   				 HIBMC_DP_MIN_PULSE_NUM);
>>>   
>>>   	dp->aux.transfer = hibmc_dp_aux_xfer;
>>> -	dp->aux.is_remote = 0;
>>> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
>>> +	dp->aux.drm_dev = dp->drm_dev;
>>>   	drm_dp_aux_init(&dp->aux);
>>>   	dp->dp_dev->aux = &dp->aux;
>>>   }
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>>> index 603d6b198a54..0256724d8b9b 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>>> @@ -15,11 +15,20 @@
>>>   
>>>   static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>>>   {
>>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>>> +	const struct drm_edid *drm_edid;
>>>   	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
>>> +	if (!dp->aux.name)
>>> +		return 0;
>>> +
>>> +	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>>> +
>>> +	drm_edid_connector_update(connector, drm_edid);
>>> +
>>> +	count = drm_edid_connector_add_modes(connector);
>>> +
>>> +	drm_edid_free(drm_edid);
>>>   
>>>   	return count;
>>>   }
>>> @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
>>>   	.get_modes = hibmc_dp_connector_get_modes,
>>>   };
>>>   
>>> +static int hibmc_dp_late_register(struct drm_connector *connector)
>>> +{
>>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>>> +
>>> +	return drm_dp_aux_register(&dp->aux);
>>> +}
>>> +
>>> +static void hibmc_dp_early_unregister(struct drm_connector *connector)
>>> +{
>>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>>> +
>>> +	drm_dp_aux_unregister(&dp->aux);
>>> +}
>>> +
>>>   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,
>>> +	.late_register = hibmc_dp_late_register,
>>> +	.early_unregister = hibmc_dp_early_unregister,
>>>   };
>>>   
>>>   static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
>>> @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
>>>   
>>>   	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
>>>   
>>> -	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
>>> -				 DRM_MODE_CONNECTOR_DisplayPort);
>>> +	ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs,
>>> +					  DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc);
>>>   	if (ret) {
>>>   		drm_err(dev, "init dp connector failed: %d\n", ret);
>>>   		return ret;
>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> index d982f1e4b958..3ddd71aada66 100644
>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>> @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
>>>   	return container_of(connector, struct hibmc_vdac, connector);
>>>   }
>>>   
>>> +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector)
>>> +{
>>> +	return container_of(connector, struct hibmc_dp, connector);
>>> +}
>>> +
>>>   static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev)
>>>   {
>>>   	return container_of(dev, struct hibmc_drm_private, dev);

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-24 13:33       ` Jani Nikula
@ 2025-03-25  3:14         ` Yongbang Shi
  2025-03-26  9:32           ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Yongbang Shi @ 2025-03-25  3:14 UTC (permalink / raw)
  To: Jani Nikula, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang


> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>> From: Baihan Li <libaihan@huawei.com>
>>>>
>>>> Add registering drm_aux and use it to get connector edid with drm
>>>> functions. Add ddc channel in connector initialization to put drm_aux
>>>> in drm_connector.
>>>>
>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> ---
>>>> ChangeLog:
>>>> v6 -> v7:
>>>>     - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>>> I don't understand this, and I did not suggest such a thing.
>>>
>>> BR,
>>> Jani.
>>>
>> Hi Jani,
>>
>> Is the modification of v8 correct?
> I never received that for whatever reason.

Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/


>>
>>>> v5 -> v6:
>>>>     - move the detect_ctx() to the patch 7/9.
>>>> v2 -> v3:
>>>>     - Capitalized EDID and AUX, suggested by Dmitry Baryshkov.
>>>> v1 -> v2:
>>>>     - deleting type conversion, suggested by Dmitry Baryshkov.
>>>>     - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov.
>>>> ---
>>>>    drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c   |  3 +-
>>>>    .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 35 ++++++++++++++++---
>>>>    .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h   |  5 +++
>>>>    3 files changed, 37 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>>>> index ded9e7ce887a..e0bb9b14d9d8 100644
>>>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c
>>>> @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp)
>>>>    				 HIBMC_DP_MIN_PULSE_NUM);
>>>>    
>>>>    	dp->aux.transfer = hibmc_dp_aux_xfer;
>>>> -	dp->aux.is_remote = 0;
>>>> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
>>>> +	dp->aux.drm_dev = dp->drm_dev;
>>>>    	drm_dp_aux_init(&dp->aux);
>>>>    	dp->dp_dev->aux = &dp->aux;
>>>>    }
>>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>>>> index 603d6b198a54..0256724d8b9b 100644
>>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>>>> @@ -15,11 +15,20 @@
>>>>    
>>>>    static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>>>>    {
>>>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>>>> +	const struct drm_edid *drm_edid;
>>>>    	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
>>>> +	if (!dp->aux.name)
>>>> +		return 0;
>>>> +
>>>> +	drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc);
>>>> +
>>>> +	drm_edid_connector_update(connector, drm_edid);
>>>> +
>>>> +	count = drm_edid_connector_add_modes(connector);
>>>> +
>>>> +	drm_edid_free(drm_edid);
>>>>    
>>>>    	return count;
>>>>    }
>>>> @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
>>>>    	.get_modes = hibmc_dp_connector_get_modes,
>>>>    };
>>>>    
>>>> +static int hibmc_dp_late_register(struct drm_connector *connector)
>>>> +{
>>>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>>>> +
>>>> +	return drm_dp_aux_register(&dp->aux);
>>>> +}
>>>> +
>>>> +static void hibmc_dp_early_unregister(struct drm_connector *connector)
>>>> +{
>>>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>>>> +
>>>> +	drm_dp_aux_unregister(&dp->aux);
>>>> +}
>>>> +
>>>>    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,
>>>> +	.late_register = hibmc_dp_late_register,
>>>> +	.early_unregister = hibmc_dp_early_unregister,
>>>>    };
>>>>    
>>>>    static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode)
>>>> @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv)
>>>>    
>>>>    	drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs);
>>>>    
>>>> -	ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs,
>>>> -				 DRM_MODE_CONNECTOR_DisplayPort);
>>>> +	ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs,
>>>> +					  DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc);
>>>>    	if (ret) {
>>>>    		drm_err(dev, "init dp connector failed: %d\n", ret);
>>>>    		return ret;
>>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>>> index d982f1e4b958..3ddd71aada66 100644
>>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h
>>>> @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector)
>>>>    	return container_of(connector, struct hibmc_vdac, connector);
>>>>    }
>>>>    
>>>> +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector)
>>>> +{
>>>> +	return container_of(connector, struct hibmc_dp, connector);
>>>> +}
>>>> +
>>>>    static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev)
>>>>    {
>>>>    	return container_of(dev, struct hibmc_drm_private, dev);

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-25  3:14         ` Yongbang Shi
@ 2025-03-26  9:32           ` Jani Nikula
  2025-03-27  3:00             ` Yongbang Shi
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2025-03-26  9:32 UTC (permalink / raw)
  To: Yongbang Shi, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang

On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>> From: Baihan Li <libaihan@huawei.com>
>>>>>
>>>>> Add registering drm_aux and use it to get connector edid with drm
>>>>> functions. Add ddc channel in connector initialization to put drm_aux
>>>>> in drm_connector.
>>>>>
>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> ---
>>>>> ChangeLog:
>>>>> v6 -> v7:
>>>>>     - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>>>> I don't understand this, and I did not suggest such a thing.
>>>>
>>>> BR,
>>>> Jani.
>>>>
>>> Hi Jani,
>>>
>>> Is the modification of v8 correct?
>> I never received that for whatever reason.
>
> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/

Thanks.

The EDID handling looks fine.

AFAICT you leak dp->aux.name though.


BR,
Jani.



-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-26  9:32           ` Jani Nikula
@ 2025-03-27  3:00             ` Yongbang Shi
  2025-03-27  9:59               ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Yongbang Shi @ 2025-03-27  3:00 UTC (permalink / raw)
  To: Jani Nikula, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang


在 2025/3/26 17:32, Jani Nikula 写道:
> On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>> From: Baihan Li <libaihan@huawei.com>
>>>>>>
>>>>>> Add registering drm_aux and use it to get connector edid with drm
>>>>>> functions. Add ddc channel in connector initialization to put drm_aux
>>>>>> in drm_connector.
>>>>>>
>>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>> ---
>>>>>> ChangeLog:
>>>>>> v6 -> v7:
>>>>>>      - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>>>>> I don't understand this, and I did not suggest such a thing.
>>>>>
>>>>> BR,
>>>>> Jani.
>>>>>
>>>> Hi Jani,
>>>>
>>>> Is the modification of v8 correct?
>>> I never received that for whatever reason.
>> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/
> Thanks.
>
> The EDID handling looks fine.
>
> AFAICT you leak dp->aux.name though.
>
>
> BR,
> Jani.

Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7,
and I deleted it in V8.

Thanks,
Baihan.

>
>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-27  3:00             ` Yongbang Shi
@ 2025-03-27  9:59               ` Jani Nikula
  2025-03-28  6:43                 ` Yongbang Shi
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2025-03-27  9:59 UTC (permalink / raw)
  To: Yongbang Shi, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang

On Thu, 27 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> 在 2025/3/26 17:32, Jani Nikula 写道:
>> On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>> From: Baihan Li <libaihan@huawei.com>
>>>>>>>
>>>>>>> Add registering drm_aux and use it to get connector edid with drm
>>>>>>> functions. Add ddc channel in connector initialization to put drm_aux
>>>>>>> in drm_connector.
>>>>>>>
>>>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>> ---
>>>>>>> ChangeLog:
>>>>>>> v6 -> v7:
>>>>>>>      - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>>>>>> I don't understand this, and I did not suggest such a thing.
>>>>>>
>>>>>> BR,
>>>>>> Jani.
>>>>>>
>>>>> Hi Jani,
>>>>>
>>>>> Is the modification of v8 correct?
>>>> I never received that for whatever reason.
>>> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/
>> Thanks.
>>
>> The EDID handling looks fine.
>>
>> AFAICT you leak dp->aux.name though.
>>
>>
>> BR,
>> Jani.
>
> Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7,
> and I deleted it in V8.

This is in the link you posted:

+	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");



>
> Thanks,
> Baihan.
>
>>
>>

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-27  9:59               ` Jani Nikula
@ 2025-03-28  6:43                 ` Yongbang Shi
  2025-03-28 10:28                   ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Yongbang Shi @ 2025-03-28  6:43 UTC (permalink / raw)
  To: Jani Nikula, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang


> On Thu, 27 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>> 在 2025/3/26 17:32, Jani Nikula 写道:
>>> On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>>> From: Baihan Li <libaihan@huawei.com>
>>>>>>>>
>>>>>>>> Add registering drm_aux and use it to get connector edid with drm
>>>>>>>> functions. Add ddc channel in connector initialization to put drm_aux
>>>>>>>> in drm_connector.
>>>>>>>>
>>>>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>> ---
>>>>>>>> ChangeLog:
>>>>>>>> v6 -> v7:
>>>>>>>>       - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>>>>>>> I don't understand this, and I did not suggest such a thing.
>>>>>>>
>>>>>>> BR,
>>>>>>> Jani.
>>>>>>>
>>>>>> Hi Jani,
>>>>>>
>>>>>> Is the modification of v8 correct?
>>>>> I never received that for whatever reason.
>>>> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/
>>> Thanks.
>>>
>>> The EDID handling looks fine.
>>>
>>> AFAICT you leak dp->aux.name though.
>>>
>>>
>>> BR,
>>> Jani.
>> Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7,
>> and I deleted it in V8.
> This is in the link you posted:
>
> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
>
Hi Jani,

I got it. I think I can change it to devm_kasprintf() in next bug fix patch, is that ok?


>
>> Thanks,
>> Baihan.
>>
>>>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-28  6:43                 ` Yongbang Shi
@ 2025-03-28 10:28                   ` Jani Nikula
  2025-03-28 10:51                     ` Maxime Ripard
  0 siblings, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2025-03-28 10:28 UTC (permalink / raw)
  To: Yongbang Shi, xinliang.liu, tiantao6, maarten.lankhorst, mripard,
	tzimmermann, airlied, daniel, kong.kongxinwei
  Cc: liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel, shiyongbang

On Fri, 28 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>> On Thu, 27 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>> 在 2025/3/26 17:32, Jani Nikula 写道:
>>>> On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>>>> From: Baihan Li <libaihan@huawei.com>
>>>>>>>>>
>>>>>>>>> Add registering drm_aux and use it to get connector edid with drm
>>>>>>>>> functions. Add ddc channel in connector initialization to put drm_aux
>>>>>>>>> in drm_connector.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>>>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>>> ---
>>>>>>>>> ChangeLog:
>>>>>>>>> v6 -> v7:
>>>>>>>>>       - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>>>>>>>> I don't understand this, and I did not suggest such a thing.
>>>>>>>>
>>>>>>>> BR,
>>>>>>>> Jani.
>>>>>>>>
>>>>>>> Hi Jani,
>>>>>>>
>>>>>>> Is the modification of v8 correct?
>>>>>> I never received that for whatever reason.
>>>>> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/
>>>> Thanks.
>>>>
>>>> The EDID handling looks fine.
>>>>
>>>> AFAICT you leak dp->aux.name though.
>>>>
>>>>
>>>> BR,
>>>> Jani.
>>> Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7,
>>> and I deleted it in V8.
>> This is in the link you posted:
>>
>> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
>>
> Hi Jani,
>
> I got it. I think I can change it to devm_kasprintf() in next bug fix patch, is that ok?

Maybe. I don't have the time to look into hibmc details.

BR,
Jani.


>
>
>>
>>> Thanks,
>>> Baihan.
>>>
>>>>

-- 
Jani Nikula, Intel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-28 10:28                   ` Jani Nikula
@ 2025-03-28 10:51                     ` Maxime Ripard
  2025-03-29  6:12                       ` Yongbang Shi
  0 siblings, 1 reply; 24+ messages in thread
From: Maxime Ripard @ 2025-03-28 10:51 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Yongbang Shi, xinliang.liu, tiantao6, maarten.lankhorst,
	tzimmermann, airlied, daniel, kong.kongxinwei, liangjian010,
	chenjianmin, lidongming5, libaihan, shenjian15, shaojijie,
	dri-devel, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]

On Fri, Mar 28, 2025 at 12:28:14PM +0200, Jani Nikula wrote:
> On Fri, 28 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> >> On Thu, 27 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> >>> 在 2025/3/26 17:32, Jani Nikula 写道:
> >>>> On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> >>>>>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> >>>>>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> >>>>>>>>> From: Baihan Li <libaihan@huawei.com>
> >>>>>>>>>
> >>>>>>>>> Add registering drm_aux and use it to get connector edid with drm
> >>>>>>>>> functions. Add ddc channel in connector initialization to put drm_aux
> >>>>>>>>> in drm_connector.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
> >>>>>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> >>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>>>>>> ---
> >>>>>>>>> ChangeLog:
> >>>>>>>>> v6 -> v7:
> >>>>>>>>>       - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
> >>>>>>>> I don't understand this, and I did not suggest such a thing.
> >>>>>>>>
> >>>>>>>> BR,
> >>>>>>>> Jani.
> >>>>>>>>
> >>>>>>> Hi Jani,
> >>>>>>>
> >>>>>>> Is the modification of v8 correct?
> >>>>>> I never received that for whatever reason.
> >>>>> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/
> >>>> Thanks.
> >>>>
> >>>> The EDID handling looks fine.
> >>>>
> >>>> AFAICT you leak dp->aux.name though.
> >>>>
> >>>>
> >>>> BR,
> >>>> Jani.
> >>> Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7,
> >>> and I deleted it in V8.
> >> This is in the link you posted:
> >>
> >> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
> >>
> > Hi Jani,
> >
> > I got it. I think I can change it to devm_kasprintf() in next bug fix patch, is that ok?
> 
> Maybe. I don't have the time to look into hibmc details.

I don't either, but it looks suspicious to me. devm_kasprintf will be
freed when the device will be removed, but the DP Aux bus is probably
staying for a bit longer?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-28 10:51                     ` Maxime Ripard
@ 2025-03-29  6:12                       ` Yongbang Shi
  2025-03-30 16:56                         ` Dmitry Baryshkov
  0 siblings, 1 reply; 24+ messages in thread
From: Yongbang Shi @ 2025-03-29  6:12 UTC (permalink / raw)
  To: Maxime Ripard, Jani Nikula
  Cc: xinliang.liu, tiantao6, maarten.lankhorst, tzimmermann, airlied,
	daniel, kong.kongxinwei, liangjian010, chenjianmin, lidongming5,
	libaihan, shenjian15, shaojijie, dri-devel, linux-kernel,
	shiyongbang

> On Fri, Mar 28, 2025 at 12:28:14PM +0200, Jani Nikula wrote:
>> On Fri, 28 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>> On Thu, 27 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>> 在 2025/3/26 17:32, Jani Nikula 写道:
>>>>>> On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
>>>>>>>>>>> From: Baihan Li <libaihan@huawei.com>
>>>>>>>>>>>
>>>>>>>>>>> Add registering drm_aux and use it to get connector edid with drm
>>>>>>>>>>> functions. Add ddc channel in connector initialization to put drm_aux
>>>>>>>>>>> in drm_connector.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>>>>>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>>>>>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>>>>>>> ---
>>>>>>>>>>> ChangeLog:
>>>>>>>>>>> v6 -> v7:
>>>>>>>>>>>        - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
>>>>>>>>>> I don't understand this, and I did not suggest such a thing.
>>>>>>>>>>
>>>>>>>>>> BR,
>>>>>>>>>> Jani.
>>>>>>>>>>
>>>>>>>>> Hi Jani,
>>>>>>>>>
>>>>>>>>> Is the modification of v8 correct?
>>>>>>>> I never received that for whatever reason.
>>>>>>> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/
>>>>>> Thanks.
>>>>>>
>>>>>> The EDID handling looks fine.
>>>>>>
>>>>>> AFAICT you leak dp->aux.name though.
>>>>>>
>>>>>>
>>>>>> BR,
>>>>>> Jani.
>>>>> Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7,
>>>>> and I deleted it in V8.
>>>> This is in the link you posted:
>>>>
>>>> +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
>>>>
>>> Hi Jani,
>>>
>>> I got it. I think I can change it to devm_kasprintf() in next bug fix patch, is that ok?
>> Maybe. I don't have the time to look into hibmc details.
> I don't either, but it looks suspicious to me. devm_kasprintf will be
> freed when the device will be removed, but the DP Aux bus is probably
> staying for a bit longer?
>
> Maxime

Hi Ripard,

I will bind it to my hibmc device, and aux_unregister is in early_unregister callback of dp's connector_funcs,
which is before the hibmc_pci_remove(), so I think it work good.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel
  2025-03-29  6:12                       ` Yongbang Shi
@ 2025-03-30 16:56                         ` Dmitry Baryshkov
  0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Baryshkov @ 2025-03-30 16:56 UTC (permalink / raw)
  To: Yongbang Shi
  Cc: Maxime Ripard, Jani Nikula, xinliang.liu, tiantao6,
	maarten.lankhorst, tzimmermann, airlied, daniel, kong.kongxinwei,
	liangjian010, chenjianmin, lidongming5, libaihan, shenjian15,
	shaojijie, dri-devel, linux-kernel

On Sat, Mar 29, 2025 at 02:12:56PM +0800, Yongbang Shi wrote:
> > On Fri, Mar 28, 2025 at 12:28:14PM +0200, Jani Nikula wrote:
> > > On Fri, 28 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> > > > > On Thu, 27 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> > > > > > 在 2025/3/26 17:32, Jani Nikula 写道:
> > > > > > > On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> > > > > > > > > On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> > > > > > > > > > > On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote:
> > > > > > > > > > > > From: Baihan Li <libaihan@huawei.com>
> > > > > > > > > > > > 
> > > > > > > > > > > > Add registering drm_aux and use it to get connector edid with drm
> > > > > > > > > > > > functions. Add ddc channel in connector initialization to put drm_aux
> > > > > > > > > > > > in drm_connector.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Baihan Li <libaihan@huawei.com>
> > > > > > > > > > > > Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> > > > > > > > > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > > > > > > > > > ---
> > > > > > > > > > > > ChangeLog:
> > > > > > > > > > > > v6 -> v7:
> > > > > > > > > > > >        - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula
> > > > > > > > > > > I don't understand this, and I did not suggest such a thing.
> > > > > > > > > > > 
> > > > > > > > > > > BR,
> > > > > > > > > > > Jani.
> > > > > > > > > > > 
> > > > > > > > > > Hi Jani,
> > > > > > > > > > 
> > > > > > > > > > Is the modification of v8 correct?
> > > > > > > > > I never received that for whatever reason.
> > > > > > > > Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/
> > > > > > > Thanks.
> > > > > > > 
> > > > > > > The EDID handling looks fine.
> > > > > > > 
> > > > > > > AFAICT you leak dp->aux.name though.
> > > > > > > 
> > > > > > > 
> > > > > > > BR,
> > > > > > > Jani.
> > > > > > Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7,
> > > > > > and I deleted it in V8.
> > > > > This is in the link you posted:
> > > > > 
> > > > > +	dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux");
> > > > > 
> > > > Hi Jani,
> > > > 
> > > > I got it. I think I can change it to devm_kasprintf() in next bug fix patch, is that ok?
> > > Maybe. I don't have the time to look into hibmc details.
> > I don't either, but it looks suspicious to me. devm_kasprintf will be
> > freed when the device will be removed, but the DP Aux bus is probably
> > staying for a bit longer?
> > 
> > Maxime
> 
> Hi Ripard,
> 
> I will bind it to my hibmc device, and aux_unregister is in early_unregister callback of dp's connector_funcs,
> which is before the hibmc_pci_remove(), so I think it work good.

No, DRM connectors are a part of the DRM framework and can potentially
outlive the underlying device.

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2025-03-30 16:56 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-19  3:24 [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 1/9] drm/hisilicon/hibmc: Restructuring the header dp_reg.h Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 2/9] drm/hisilicon/hibmc: Add dp serdes cfg to adjust serdes rate, voltage and pre-emphasis Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 3/9] drm/hisilicon/hibmc: Add dp serdes cfg in dp process Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 4/9] drm/hisilicon/hibmc: Refactor the member of drm_aux in struct hibmc_dp Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 5/9] drm/hisilicon/hibmc: Getting connector info and EDID by using AUX channel Yongbang Shi
2025-03-20  9:50   ` Jani Nikula
2025-03-24 12:41     ` Yongbang Shi
2025-03-24 13:33       ` Jani Nikula
2025-03-25  3:14         ` Yongbang Shi
2025-03-26  9:32           ` Jani Nikula
2025-03-27  3:00             ` Yongbang Shi
2025-03-27  9:59               ` Jani Nikula
2025-03-28  6:43                 ` Yongbang Shi
2025-03-28 10:28                   ` Jani Nikula
2025-03-28 10:51                     ` Maxime Ripard
2025-03-29  6:12                       ` Yongbang Shi
2025-03-30 16:56                         ` Dmitry Baryshkov
2025-03-19  3:24 ` [PATCH v7 drm-dp 6/9] drm/hisilicon/hibmc: Add colorbar-cfg feature and its debugfs file Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 7/9] drm/hisilicon/hibmc: Enable this hot plug detect of irq feature Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 8/9] drm/hisilicon/hibmc: Add MSI irq getting and requesting for HPD Yongbang Shi
2025-03-19  3:24 ` [PATCH v7 drm-dp 9/9] drm/hisilicon/hibmc: Add vga connector detect functions Yongbang Shi
2025-03-19 11:54 ` [PATCH v7 drm-dp 0/9] Add HPD, getting EDID, colorbar features in DP function Dmitry Baryshkov
2025-03-20  5:12   ` Yongbang Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).