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 <[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;
> }
Yes
>
> 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
> > >
--
With best wishes
Dmitry
