[PATCH v6 drm-dp 0/4] Fix hibmc driver bugs

[PATCH v6 drm-dp 0/4] Fix hibmc driver bugs

[Yongbang Shi]

From: Baihan Li <libaihan@huawei.com>

There are some bugfix for hibmc-drm driver.
---
ChangeLog:
v5 -> v6:
  - use HPD status in DP detect_ctx(), suggested by Dmitry Baryshkov.
v4 -> v5:
  - Because some of patches are applied, this series only contains the rest of them.
  - fix the commit and DP detect_ctx(), suggested by Dmitry Baryshkov.
  - fix bugfix commit ID, suggested by Dmitry Baryshkov.
  - remove the 08/11 patch, I'll add in next series.
  - combined 9 and 11 patch together, suggested by Dmitry Baryshkov.
v3 -> v4:
  - remove link training process in hibmc_dp_detect(), suggested by Dmitry Baryshkov.
  - remove if (dev->registered), suggested by Dmitry Baryshkov.
  - remove non-related changes, suggested by Dmitry Baryshkov.
  - Remove the clock check, suggested by Dmitry Baryshkov.
  - ( I'll add them in next series after redesigning this part)
  - add KVM edid in commit message, suggested by Dmitry Baryshkov.
  - fix magic values, suggested by Dmitry Baryshkov.
  - fix the commit subjects, suggested by Dmitry Baryshkov.
v2 -> v3:
  - fix hibmc_connector_get_modes() and hibmc_vdac_detect() to realize BMC KVM, suggested by Dmitry Baryshkov.
  - fix the issue commit ID, suggested by Dmitry Baryshkov.
  - split into 2 commits, suggested by Dmitry Baryshkov.
  - add more comments in commit log, suggested by Dmitry Baryshkov.
---

Baihan Li (4):
  drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD
    irq
  drm/hisilicon/hibmc: add dp mode valid check
  drm/hisilicon/hibmc: fix no showing problem with loading hibmc
    manually
  drm/hisilicon/hibmc: Adding reset colorbar cfg in dp init.

 .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  2 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    | 31 ++++++++++++++++--
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  9 ++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h   |  3 ++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 32 +++++++++++++++++--
 5 files changed, 73 insertions(+), 4 deletions(-)

-- 
2.33.0

[PATCH v6 drm-dp 1/4] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq

[Yongbang Shi]

From: Baihan Li <libaihan@huawei.com>

The issue is that drm_connector_helper_detect_from_ddc() returns wrong
status when plugging or unplugging the monitor. Use HPD pin status in
DP's detect_ctx() for real physcal monitor in/out, and keep using
detect_frome_ddc() if it's the first time to call detect because of
insmoding driver.

Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
---
ChangeLog:
v5 -> v6:
  - use HPD status in DP detect_ctx(), suggested by Dmitry Baryshkov.
v4 -> v5:
  - fix the commit message and DP detect_ctx(), suggested by Dmitry Baryshkov.
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c     | 12 ++++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h     |  7 +++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h    |  3 +++
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 13 +++++++++++--
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index 8f0daec7d174..4d8d3e4d4f84 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -2,6 +2,7 @@
 // Copyright (c) 2024 Hisilicon Limited.
 
 #include <linux/io.h>
+#include <linux/iopoll.h>
 #include <linux/delay.h>
 #include "dp_config.h"
 #include "dp_comm.h"
@@ -305,3 +306,14 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg)
 	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);
 }
+
+void hibmc_dp_update_hpd_status(struct hibmc_dp *dp)
+{
+	int status;
+
+	readl_poll_timeout(dp->dp_dev->base + HIBMC_DP_HPD_STATUS, status,
+			   FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) != dp->hpd_status,
+			   1000, 100000); /* DP spec says 100ms */
+
+	dp->hpd_status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status);
+}
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
index 665f5b166dfb..8348ad9e34a8 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -14,6 +14,11 @@
 
 struct hibmc_dp_dev;
 
+enum hibmc_hpd_status {
+	HIBMC_HPD_OUT,
+	HIBMC_HPD_IN,
+};
+
 enum hibmc_dp_cbar_pattern {
 	CBAR_COLOR_BAR,
 	CBAR_WHITE,
@@ -50,6 +55,7 @@ struct hibmc_dp {
 	struct drm_dp_aux aux;
 	struct hibmc_dp_cbar_cfg cfg;
 	u32 irq_status;
+	int hpd_status;
 };
 
 int hibmc_dp_hw_init(struct hibmc_dp *dp);
@@ -60,5 +66,6 @@ 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);
+void hibmc_dp_update_hpd_status(struct hibmc_dp *dp);
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
index 394b1e933c3a..64306abcd986 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
@@ -24,6 +24,9 @@
 #define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
 #define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
 
+#define HIBMC_DP_HPD_STATUS			0x98
+#define HIBMC_DP_HPD_CUR_STATE		GENMASK(7, 4)
+
 #define HIBMC_DP_PHYIF_CTRL0			0xa0
 #define HIBMC_DP_CFG_SCRAMBLE_EN		BIT(0)
 #define HIBMC_DP_CFG_PAT_SEL			GENMASK(7, 4)
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
index d06832e62e96..48c9c97eef0e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -34,9 +34,16 @@ static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
 static int hibmc_dp_detect(struct drm_connector *connector,
 			   struct drm_modeset_acquire_ctx *ctx, bool force)
 {
-	mdelay(200);
+	struct hibmc_dp *dp = to_hibmc_dp(connector);
+
+	/* if no HPD just probe DDC */
+	if (!dp->irq_status)
+		return drm_connector_helper_detect_from_ddc(connector, ctx, force);
 
-	return drm_connector_helper_detect_from_ddc(connector, ctx, force);
+	if (dp->hpd_status == HIBMC_HPD_IN)
+		return connector_status_connected;
+
+	return connector_status_disconnected;
 }
 
 static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
@@ -128,6 +135,8 @@ irqreturn_t hibmc_dp_hpd_isr(int irq, void *arg)
 		hibmc_dp_reset_link(&priv->dp);
 	}
 
+	hibmc_dp_update_hpd_status(&priv->dp);
+
 	if (dev->registered)
 		drm_connector_helper_hpd_irq_event(&priv->dp.connector);
 
-- 
2.33.0

[PATCH v6 drm-dp 2/4] drm/hisilicon/hibmc: add dp mode valid check

[Yongbang Shi]

From: Baihan Li <libaihan@huawei.com>

If DP is connected, check the DP BW in mode_valid_ctx() to ensure
that DP's link rate supports high-resolution data transmission.

Fixes: 0ab6ea261c1f ("drm/hisilicon/hibmc: add dp module in hibmc")
Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
ChangeLog:
v4 -> v5:
  - fix bugfix commit ID, suggested by Dmitry Baryshkov.
---
 .../gpu/drm/hisilicon/hibmc/dp/dp_config.h    |  2 ++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c    | 10 ++++++++++
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h    |  2 ++
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c    | 19 +++++++++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
index 08f9e1caf7fc..efb30a758475 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_config.h
@@ -17,5 +17,7 @@
 #define HIBMC_DP_LINK_RATE_CAL		27
 #define HIBMC_DP_SYNC_DELAY(lanes)	((lanes) == 0x2 ? 86 : 46)
 #define HIBMC_DP_INT_ENABLE		0xc
+/* HIBMC_DP_LINK_RATE_CAL * 10000 * 80% = 216000 */
+#define DP_MODE_VALI_CAL		216000
 
 #endif
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index 4d8d3e4d4f84..f275d4c14819 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -264,6 +264,16 @@ void hibmc_dp_reset_link(struct hibmc_dp *dp)
 	dp->dp_dev->link.status.channel_equalized = false;
 }
 
+u8 hibmc_dp_get_link_rate(struct hibmc_dp *dp)
+{
+	return dp->dp_dev->link.cap.link_rate;
+}
+
+u8 hibmc_dp_get_lanes(struct hibmc_dp *dp)
+{
+	return dp->dp_dev->link.cap.lanes;
+}
+
 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 8348ad9e34a8..1d5e6a1d8e76 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
@@ -67,5 +67,7 @@ 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);
 void hibmc_dp_update_hpd_status(struct hibmc_dp *dp);
+u8 hibmc_dp_get_link_rate(struct hibmc_dp *dp);
+u8 hibmc_dp_get_lanes(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 48c9c97eef0e..9a80d8047291 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
@@ -12,6 +12,7 @@
 
 #include "hibmc_drm_drv.h"
 #include "dp/dp_hw.h"
+#include "dp/dp_config.h"
 
 #define DP_MASKED_SINK_HPD_PLUG_INT	BIT(2)
 
@@ -46,9 +47,27 @@ static int hibmc_dp_detect(struct drm_connector *connector,
 	return connector_status_disconnected;
 }
 
+static int hibmc_dp_mode_valid(struct drm_connector *connector,
+			       const struct drm_display_mode *mode,
+			       struct drm_modeset_acquire_ctx *ctx,
+			       enum drm_mode_status *status)
+{
+	struct hibmc_dp *dp = to_hibmc_dp(connector);
+	u64 cur_val, max_val;
+
+	/* check DP link BW */
+	cur_val = (u64)mode->clock * HIBMC_DP_BPP;
+	max_val = (u64)hibmc_dp_get_link_rate(dp) * DP_MODE_VALI_CAL * hibmc_dp_get_lanes(dp);
+
+	*status = cur_val > max_val ? MODE_CLOCK_HIGH : MODE_OK;
+
+	return 0;
+}
+
 static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
 	.get_modes = hibmc_dp_connector_get_modes,
 	.detect_ctx = hibmc_dp_detect,
+	.mode_valid_ctx = hibmc_dp_mode_valid,
 };
 
 static int hibmc_dp_late_register(struct drm_connector *connector)
-- 
2.33.0

[PATCH v6 drm-dp 3/4] drm/hisilicon/hibmc: fix no showing problem with loading hibmc manually

[Yongbang Shi]

From: Baihan Li <libaihan@huawei.com>

When using command rmmod and insmod, there is no showing in second time
insmoding. Because DP controller won't send HPD signals, if connection
doesn't change or controller isn't reset. So add reset before unreset
in hibmc_dp_hw_init().

And also need to move the HDCP cfg after DP controller de-resets, so
that HDCP configuration takes effect.

Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
ChangeLog:
v4 -> v5:
  - combined 9 and 11 patch together, suggested by Dmitry Baryshkov.
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index f275d4c14819..6d3777dfd76e 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -177,13 +177,16 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
 	dp_dev->link.cap.lanes = 0x2;
 	dp_dev->link.cap.link_rate = DP_LINK_BW_8_1;
 
-	/* hdcp data */
-	writel(HIBMC_DP_HDCP, dp_dev->base + HIBMC_DP_HDCP_CFG);
 	/* int init */
 	writel(0, dp_dev->base + HIBMC_DP_INTR_ENABLE);
 	writel(HIBMC_DP_INT_RST, dp_dev->base + HIBMC_DP_INTR_ORIGINAL_STATUS);
 	/* rst */
+	writel(0, dp_dev->base + HIBMC_DP_DPTX_RST_CTRL);
+	usleep_range(30, 50);
+	/* de-rst */
 	writel(HIBMC_DP_DPTX_RST, dp_dev->base + HIBMC_DP_DPTX_RST_CTRL);
+	/* hdcp data */
+	writel(HIBMC_DP_HDCP, dp_dev->base + HIBMC_DP_HDCP_CFG);
 	/* clock enable */
 	writel(HIBMC_DP_CLK_EN, dp_dev->base + HIBMC_DP_DPTX_CLK_CTRL);
 
-- 
2.33.0

[PATCH v6 drm-dp 4/4] drm/hisilicon/hibmc: Adding reset colorbar cfg in dp init.

[Yongbang Shi]

From: Baihan Li <libaihan@huawei.com>

Add colorbar disable operation before reset chontroller, to make sure
colorbar status is clear in the DP init, so if rmmod the driver and the
previous colorbar configuration will not affect the next time insmod the
driver.

Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
index 6d3777dfd76e..26641b4a0a21 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
@@ -180,6 +180,8 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp)
 	/* int init */
 	writel(0, dp_dev->base + HIBMC_DP_INTR_ENABLE);
 	writel(HIBMC_DP_INT_RST, dp_dev->base + HIBMC_DP_INTR_ORIGINAL_STATUS);
+	/* clr colorbar */
+	writel(0, dp_dev->base + HIBMC_DP_COLOR_BAR_CTRL);
 	/* rst */
 	writel(0, dp_dev->base + HIBMC_DP_DPTX_RST_CTRL);
 	usleep_range(30, 50);
-- 
2.33.0

Re: [PATCH v6 drm-dp 1/4] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq

[Dmitry Baryshkov]

On Mon, Sep 22, 2025 at 10:49:40AM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@huawei.com>
> 
> The issue is that drm_connector_helper_detect_from_ddc() returns wrong
> status when plugging or unplugging the monitor. Use HPD pin status in
> DP's detect_ctx() for real physcal monitor in/out, and keep using
> detect_frome_ddc() if it's the first time to call detect because of
> insmoding driver.

If I understand correct, this is not quite right. Consider DP-to-HDMI or
DP-to-DVI dongle being plugged without an actual monitor and then the
monitor being plugged later on.

> 
> Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> ---
> ChangeLog:
> v5 -> v6:
>   - use HPD status in DP detect_ctx(), suggested by Dmitry Baryshkov.
> v4 -> v5:
>   - fix the commit message and DP detect_ctx(), suggested by Dmitry Baryshkov.
> ---
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c     | 12 ++++++++++++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h     |  7 +++++++
>  drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h    |  3 +++
>  drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 13 +++++++++++--
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> index 8f0daec7d174..4d8d3e4d4f84 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> @@ -2,6 +2,7 @@
>  // Copyright (c) 2024 Hisilicon Limited.
>  
>  #include <linux/io.h>
> +#include <linux/iopoll.h>
>  #include <linux/delay.h>
>  #include "dp_config.h"
>  #include "dp_comm.h"
> @@ -305,3 +306,14 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg)
>  	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);
>  }
> +
> +void hibmc_dp_update_hpd_status(struct hibmc_dp *dp)
> +{
> +	int status;
> +
> +	readl_poll_timeout(dp->dp_dev->base + HIBMC_DP_HPD_STATUS, status,
> +			   FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) != dp->hpd_status,
> +			   1000, 100000); /* DP spec says 100ms */
> +
> +	dp->hpd_status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status);
> +}
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> index 665f5b166dfb..8348ad9e34a8 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> @@ -14,6 +14,11 @@
>  
>  struct hibmc_dp_dev;
>  
> +enum hibmc_hpd_status {
> +	HIBMC_HPD_OUT,
> +	HIBMC_HPD_IN,
> +};
> +
>  enum hibmc_dp_cbar_pattern {
>  	CBAR_COLOR_BAR,
>  	CBAR_WHITE,
> @@ -50,6 +55,7 @@ struct hibmc_dp {
>  	struct drm_dp_aux aux;
>  	struct hibmc_dp_cbar_cfg cfg;
>  	u32 irq_status;
> +	int hpd_status;
>  };
>  
>  int hibmc_dp_hw_init(struct hibmc_dp *dp);
> @@ -60,5 +66,6 @@ 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);
> +void hibmc_dp_update_hpd_status(struct hibmc_dp *dp);
>  
>  #endif
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> index 394b1e933c3a..64306abcd986 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> @@ -24,6 +24,9 @@
>  #define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
>  #define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
>  
> +#define HIBMC_DP_HPD_STATUS			0x98
> +#define HIBMC_DP_HPD_CUR_STATE		GENMASK(7, 4)
> +
>  #define HIBMC_DP_PHYIF_CTRL0			0xa0
>  #define HIBMC_DP_CFG_SCRAMBLE_EN		BIT(0)
>  #define HIBMC_DP_CFG_PAT_SEL			GENMASK(7, 4)
> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> index d06832e62e96..48c9c97eef0e 100644
> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> @@ -34,9 +34,16 @@ static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>  static int hibmc_dp_detect(struct drm_connector *connector,
>  			   struct drm_modeset_acquire_ctx *ctx, bool force)
>  {
> -	mdelay(200);
> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
> +
> +	/* if no HPD just probe DDC */
> +	if (!dp->irq_status)
> +		return drm_connector_helper_detect_from_ddc(connector, ctx, force);
>  
> -	return drm_connector_helper_detect_from_ddc(connector, ctx, force);
> +	if (dp->hpd_status == HIBMC_HPD_IN)
> +		return connector_status_connected;
> +
> +	return connector_status_disconnected;
>  }
>  
>  static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
> @@ -128,6 +135,8 @@ irqreturn_t hibmc_dp_hpd_isr(int irq, void *arg)
>  		hibmc_dp_reset_link(&priv->dp);
>  	}
>  
> +	hibmc_dp_update_hpd_status(&priv->dp);
> +
>  	if (dev->registered)
>  		drm_connector_helper_hpd_irq_event(&priv->dp.connector);
>  
> -- 
> 2.33.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v6 drm-dp 1/4] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq

[Yongbang Shi]

> On Mon, Sep 22, 2025 at 10:49:40AM +0800, Yongbang Shi wrote:
>> From: Baihan Li <libaihan@huawei.com>
>>
>> The issue is that drm_connector_helper_detect_from_ddc() returns wrong
>> status when plugging or unplugging the monitor. Use HPD pin status in
>> DP's detect_ctx() for real physcal monitor in/out, and keep using
>> detect_frome_ddc() if it's the first time to call detect because of
>> insmoding driver.
> If I understand correct, this is not quite right. Consider DP-to-HDMI or
> DP-to-DVI dongle being plugged without an actual monitor and then the
> monitor being plugged later on.

Hi Dmitry,
Thanks for your correction. So your point is that if the HPD comes in from a dongle.
Is that okay to add those checks at the end?

         ret = drm_dp_read_dpcd_caps(dp_dev->aux, dp_dev->dpcd);
	if (ret)
		return connector_status_disconnected;

	if (!drm_dp_is_branch(dpcd))
		return connector_status_connected;

	if (drm_dp_read_sink_count_cap(connector, dp_dev->dpcd, &dp_dev->desc) &&
	    dp_dev->dpcd[DP_DOWNSTREAM_PORT_0] & DP_DS_PORT_HPD) {
		ret = drm_dp_read_sink_count(dp_dev->dpcd);
		if (ret > 0)
			return connector_status_connected;
	}

Thanks，
Baihan


>> Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
>> ---
>> ChangeLog:
>> v5 -> v6:
>>    - use HPD status in DP detect_ctx(), suggested by Dmitry Baryshkov.
>> v4 -> v5:
>>    - fix the commit message and DP detect_ctx(), suggested by Dmitry Baryshkov.
>> ---
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c     | 12 ++++++++++++
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h     |  7 +++++++
>>   drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h    |  3 +++
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 13 +++++++++++--
>>   4 files changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
>> index 8f0daec7d174..4d8d3e4d4f84 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
>> @@ -2,6 +2,7 @@
>>   // Copyright (c) 2024 Hisilicon Limited.
>>   
>>   #include <linux/io.h>
>> +#include <linux/iopoll.h>
>>   #include <linux/delay.h>
>>   #include "dp_config.h"
>>   #include "dp_comm.h"
>> @@ -305,3 +306,14 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg)
>>   	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);
>>   }
>> +
>> +void hibmc_dp_update_hpd_status(struct hibmc_dp *dp)
>> +{
>> +	int status;
>> +
>> +	readl_poll_timeout(dp->dp_dev->base + HIBMC_DP_HPD_STATUS, status,
>> +			   FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) != dp->hpd_status,
>> +			   1000, 100000); /* DP spec says 100ms */
>> +
>> +	dp->hpd_status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status);
>> +}
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
>> index 665f5b166dfb..8348ad9e34a8 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
>> @@ -14,6 +14,11 @@
>>   
>>   struct hibmc_dp_dev;
>>   
>> +enum hibmc_hpd_status {
>> +	HIBMC_HPD_OUT,
>> +	HIBMC_HPD_IN,
>> +};
>> +
>>   enum hibmc_dp_cbar_pattern {
>>   	CBAR_COLOR_BAR,
>>   	CBAR_WHITE,
>> @@ -50,6 +55,7 @@ struct hibmc_dp {
>>   	struct drm_dp_aux aux;
>>   	struct hibmc_dp_cbar_cfg cfg;
>>   	u32 irq_status;
>> +	int hpd_status;
>>   };
>>   
>>   int hibmc_dp_hw_init(struct hibmc_dp *dp);
>> @@ -60,5 +66,6 @@ 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);
>> +void hibmc_dp_update_hpd_status(struct hibmc_dp *dp);
>>   
>>   #endif
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
>> index 394b1e933c3a..64306abcd986 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
>> @@ -24,6 +24,9 @@
>>   #define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
>>   #define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
>>   
>> +#define HIBMC_DP_HPD_STATUS			0x98
>> +#define HIBMC_DP_HPD_CUR_STATE		GENMASK(7, 4)
>> +
>>   #define HIBMC_DP_PHYIF_CTRL0			0xa0
>>   #define HIBMC_DP_CFG_SCRAMBLE_EN		BIT(0)
>>   #define HIBMC_DP_CFG_PAT_SEL			GENMASK(7, 4)
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> index d06832e62e96..48c9c97eef0e 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> @@ -34,9 +34,16 @@ static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
>>   static int hibmc_dp_detect(struct drm_connector *connector,
>>   			   struct drm_modeset_acquire_ctx *ctx, bool force)
>>   {
>> -	mdelay(200);
>> +	struct hibmc_dp *dp = to_hibmc_dp(connector);
>> +
>> +	/* if no HPD just probe DDC */
>> +	if (!dp->irq_status)
>> +		return drm_connector_helper_detect_from_ddc(connector, ctx, force);
>>   
>> -	return drm_connector_helper_detect_from_ddc(connector, ctx, force);
>> +	if (dp->hpd_status == HIBMC_HPD_IN)
>> +		return connector_status_connected;
>> +
>> +	return connector_status_disconnected;
>>   }
>>   
>>   static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
>> @@ -128,6 +135,8 @@ irqreturn_t hibmc_dp_hpd_isr(int irq, void *arg)
>>   		hibmc_dp_reset_link(&priv->dp);
>>   	}
>>   
>> +	hibmc_dp_update_hpd_status(&priv->dp);
>> +
>>   	if (dev->registered)
>>   		drm_connector_helper_hpd_irq_event(&priv->dp.connector);
>>   
>> -- 
>> 2.33.0
>>

Re: [PATCH v6 drm-dp 1/4] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq

[Dmitry Baryshkov]

On Tue, Sep 23, 2025 at 10:57:56AM +0800, Yongbang Shi wrote:
> > On Mon, Sep 22, 2025 at 10:49:40AM +0800, Yongbang Shi wrote:
> > > From: Baihan Li <libaihan@huawei.com>
> > > 
> > > The issue is that drm_connector_helper_detect_from_ddc() returns wrong
> > > status when plugging or unplugging the monitor. Use HPD pin status in
> > > DP's detect_ctx() for real physcal monitor in/out, and keep using
> > > detect_frome_ddc() if it's the first time to call detect because of
> > > insmoding driver.
> > If I understand correct, this is not quite right. Consider DP-to-HDMI or
> > DP-to-DVI dongle being plugged without an actual monitor and then the
> > monitor being plugged later on.
> 
> Hi Dmitry,
> Thanks for your correction. So your point is that if the HPD comes in from a dongle.
> Is that okay to add those checks at the end?
> 
>         ret = drm_dp_read_dpcd_caps(dp_dev->aux, dp_dev->dpcd);
> 	if (ret)
> 		return connector_status_disconnected;
> 
> 	if (!drm_dp_is_branch(dpcd))
> 		return connector_status_connected;
> 
> 	if (drm_dp_read_sink_count_cap(connector, dp_dev->dpcd, &dp_dev->desc) &&
> 	    dp_dev->dpcd[DP_DOWNSTREAM_PORT_0] & DP_DS_PORT_HPD) {
> 		ret = drm_dp_read_sink_count(dp_dev->dpcd);
> 		if (ret > 0)
> 			return connector_status_connected;
> 	}

Yes

> 
> Thanks，
> Baihan
> 
> 
> > > Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
> > > Signed-off-by: Baihan Li <libaihan@huawei.com>
> > > Signed-off-by: Yongbang Shi <shiyongbang@huawei.com>
> > > ---
> > > ChangeLog:
> > > v5 -> v6:
> > >    - use HPD status in DP detect_ctx(), suggested by Dmitry Baryshkov.
> > > v4 -> v5:
> > >    - fix the commit message and DP detect_ctx(), suggested by Dmitry Baryshkov.
> > > ---
> > >   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c     | 12 ++++++++++++
> > >   drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h     |  7 +++++++
> > >   drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h    |  3 +++
> > >   drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 13 +++++++++++--
> > >   4 files changed, 33 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> > > index 8f0daec7d174..4d8d3e4d4f84 100644
> > > --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c
> > > @@ -2,6 +2,7 @@
> > >   // Copyright (c) 2024 Hisilicon Limited.
> > >   #include <linux/io.h>
> > > +#include <linux/iopoll.h>
> > >   #include <linux/delay.h>
> > >   #include "dp_config.h"
> > >   #include "dp_comm.h"
> > > @@ -305,3 +306,14 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg)
> > >   	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);
> > >   }
> > > +
> > > +void hibmc_dp_update_hpd_status(struct hibmc_dp *dp)
> > > +{
> > > +	int status;
> > > +
> > > +	readl_poll_timeout(dp->dp_dev->base + HIBMC_DP_HPD_STATUS, status,
> > > +			   FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) != dp->hpd_status,
> > > +			   1000, 100000); /* DP spec says 100ms */
> > > +
> > > +	dp->hpd_status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status);
> > > +}
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> > > index 665f5b166dfb..8348ad9e34a8 100644
> > > --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
> > > @@ -14,6 +14,11 @@
> > >   struct hibmc_dp_dev;
> > > +enum hibmc_hpd_status {
> > > +	HIBMC_HPD_OUT,
> > > +	HIBMC_HPD_IN,
> > > +};
> > > +
> > >   enum hibmc_dp_cbar_pattern {
> > >   	CBAR_COLOR_BAR,
> > >   	CBAR_WHITE,
> > > @@ -50,6 +55,7 @@ struct hibmc_dp {
> > >   	struct drm_dp_aux aux;
> > >   	struct hibmc_dp_cbar_cfg cfg;
> > >   	u32 irq_status;
> > > +	int hpd_status;
> > >   };
> > >   int hibmc_dp_hw_init(struct hibmc_dp *dp);
> > > @@ -60,5 +66,6 @@ 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);
> > > +void hibmc_dp_update_hpd_status(struct hibmc_dp *dp);
> > >   #endif
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> > > index 394b1e933c3a..64306abcd986 100644
> > > --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_reg.h
> > > @@ -24,6 +24,9 @@
> > >   #define HIBMC_DP_CFG_AUX_READY_DATA_BYTE	GENMASK(16, 12)
> > >   #define HIBMC_DP_CFG_AUX			GENMASK(24, 17)
> > > +#define HIBMC_DP_HPD_STATUS			0x98
> > > +#define HIBMC_DP_HPD_CUR_STATE		GENMASK(7, 4)
> > > +
> > >   #define HIBMC_DP_PHYIF_CTRL0			0xa0
> > >   #define HIBMC_DP_CFG_SCRAMBLE_EN		BIT(0)
> > >   #define HIBMC_DP_CFG_PAT_SEL			GENMASK(7, 4)
> > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> > > index d06832e62e96..48c9c97eef0e 100644
> > > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> > > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
> > > @@ -34,9 +34,16 @@ static int hibmc_dp_connector_get_modes(struct drm_connector *connector)
> > >   static int hibmc_dp_detect(struct drm_connector *connector,
> > >   			   struct drm_modeset_acquire_ctx *ctx, bool force)
> > >   {
> > > -	mdelay(200);
> > > +	struct hibmc_dp *dp = to_hibmc_dp(connector);
> > > +
> > > +	/* if no HPD just probe DDC */
> > > +	if (!dp->irq_status)
> > > +		return drm_connector_helper_detect_from_ddc(connector, ctx, force);
> > > -	return drm_connector_helper_detect_from_ddc(connector, ctx, force);
> > > +	if (dp->hpd_status == HIBMC_HPD_IN)
> > > +		return connector_status_connected;
> > > +
> > > +	return connector_status_disconnected;
> > >   }
> > >   static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
> > > @@ -128,6 +135,8 @@ irqreturn_t hibmc_dp_hpd_isr(int irq, void *arg)
> > >   		hibmc_dp_reset_link(&priv->dp);
> > >   	}
> > > +	hibmc_dp_update_hpd_status(&priv->dp);
> > > +
> > >   	if (dev->registered)
> > >   		drm_connector_helper_hpd_irq_event(&priv->dp.connector);
> > > -- 
> > > 2.33.0
> > > 

-- 
With best wishes
Dmitry