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

Sat, 18 Oct 2025 01:46:48 -0700 

On Mon, Sep 22, 2025 at 10:49:40AM +0800, Yongbang Shi wrote:

From: Baihan Li <[email protected]>

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 <[email protected]>
Signed-off-by: Yongbang Shi <[email protected]>
---
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

Reply via email to