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.
>
> 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
>
--
With best wishes
Dmitry
