* [PATCH for drm-misc-fixes v2 0/2] Fix some bugs in the hibmc DP
@ 2026-07-21 12:43 Yongbang Shi
2026-07-21 12:43 ` [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status Yongbang Shi
2026-07-21 12:43 ` [PATCH for drm-misc-fixes v2 2/2] drm/hisilicon/hibmc: Add a flag to indicate whether the OS-side driver has been loaded Yongbang Shi
0 siblings, 2 replies; 6+ messages in thread
From: Yongbang Shi @ 2026-07-21 12:43 UTC (permalink / raw)
To: tzimmermann, dmitry.baryshkov, tiantao6, maarten.lankhorst,
mripard, airlied, daniel, kong.kongxinwei
Cc: liangjian010, chenjianmin, fengsheng5, shiyongbang, helin52,
shenjian15, shaojijie, dri-devel, linux-kernel
From: Lin He <helin52@huawei.com>
Fix some bugs in the hibmc DP driver.
---
ChangeLog:
v1 -> v2:
- More states in HIBMC_DP_HPD_STATUS are added to the
'hibmc_dp_get_hpd_status'.
- The call to `hibmc_set_enable_flag()` has been moved before
`drm_client_setup()`, specifically before the connector detection
triggers the I2C bit operation. (sashiko-bot)
- A read-modify-write operation has been implemented instead of directly
writing to prevent clearing the I2C mask bits. (sashiko-bot)
---
Lin He (2):
drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status
drm/hisilicon/hibmc: Add a flag to indicate whether the OS-side driver
has been loaded
drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h | 1 -
drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 32 ++++++++++++-------
drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 4 ++-
.../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 29 ++++++++++-------
.../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 22 +++++++++++++
.../gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h | 3 ++
6 files changed, 67 insertions(+), 24 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status 2026-07-21 12:43 [PATCH for drm-misc-fixes v2 0/2] Fix some bugs in the hibmc DP Yongbang Shi @ 2026-07-21 12:43 ` Yongbang Shi 2026-08-27 8:16 ` Thomas Zimmermann 2026-07-21 12:43 ` [PATCH for drm-misc-fixes v2 2/2] drm/hisilicon/hibmc: Add a flag to indicate whether the OS-side driver has been loaded Yongbang Shi 1 sibling, 1 reply; 6+ messages in thread From: Yongbang Shi @ 2026-07-21 12:43 UTC (permalink / raw) To: tzimmermann, dmitry.baryshkov, tiantao6, maarten.lankhorst, mripard, airlied, daniel, kong.kongxinwei Cc: liangjian010, chenjianmin, fengsheng5, shiyongbang, helin52, shenjian15, shaojijie, dri-devel, linux-kernel From: Lin He <helin52@huawei.com> To more accurately determine whether the current HPD status matches the interrupt status, the polling mechanism in the lower half of the HPD interrupt (via hibmc_dp_check_hpd_status) has been replaced with directly retrieving the interrupt status in the upper half of the interrupt (via hibmc_dp_get_hpd_status). * Detection and training are not performed if hpd_status is not HPD_IN. * Set the initial status of hpd_status to HPD_OUT. Fixes: 3906e7a3b26d ("drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq") Signed-off-by: Lin He <helin52@huawei.com> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> --- ChangeLog: v1 -> v2: - More states in HIBMC_DP_HPD_STATUS are added to the 'hibmc_dp_get_hpd_status'. --- drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h | 1 - drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 32 ++++++++++++------- drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 4 ++- .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 29 ++++++++++------- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 + 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h index f53dac256ee0..b0e258b9265e 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h @@ -43,7 +43,6 @@ struct hibmc_dp_dev { u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; struct drm_dp_desc desc; bool is_branch; - int hpd_status; void __iomem *serdes_base; }; diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c index d5bd3c45649b..c9a113a1937d 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c @@ -191,6 +191,10 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp) 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); + /* To latch the HPD interrupt, ensuring that DP can support more modes + * within the fbcon framework when connected alone. + */ + msleep(100); return 0; } @@ -322,20 +326,26 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg) writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL); } -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status) +int hibmc_dp_get_hpd_status(struct hibmc_dp *dp) { + int ret = HIBMC_HPD_UNKNOWN; u32 status; - int ret; - ret = readl_poll_timeout(dp->dp_dev->base + HIBMC_DP_HPD_STATUS, status, - FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) == exp_status, - 1000, 100000); /* DP spec says 100ms */ - if (ret) { - drm_dbg_dp(dp->drm_dev, "wait hpd status timeout"); - return false; + status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, + readl(dp->dp_dev->base + HIBMC_DP_HPD_STATUS)); + switch (status) { + case 0: /* idle */ + case 3: /* unplug */ + case 4: /* unplug intermediate */ + ret = HIBMC_HPD_OUT; + break; + case 1: /* plug */ + case 2: /* plug intermediate */ + ret = HIBMC_HPD_IN; + break; + default: + break; } - dp->dp_dev->hpd_status = exp_status; - - return true; + return ret; } diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h index 0f3662d8737e..bcd4e9d155c8 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h @@ -15,6 +15,7 @@ struct hibmc_dp_dev; enum hibmc_hpd_status { + HIBMC_HPD_UNKNOWN, HIBMC_HPD_OUT, HIBMC_HPD_IN, }; @@ -55,6 +56,7 @@ struct hibmc_dp { struct drm_dp_aux aux; struct hibmc_dp_cbar_cfg cfg; u32 irq_status; + int hpd_status; int phys_status; }; @@ -66,7 +68,7 @@ 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); -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status); +int hibmc_dp_get_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); diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c index 2e9403b8bf3c..23716d48149f 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c @@ -63,12 +63,8 @@ static int hibmc_dp_detect(struct drm_connector *connector, struct hibmc_dp_dev *dp_dev = dp->dp_dev; int ret = connector_status_disconnected; - if (dp->irq_status) { - if (dp_dev->hpd_status != HIBMC_HPD_IN) { - ret = connector_status_disconnected; - goto exit; - } - } + if (dp->hpd_status != HIBMC_HPD_IN) + goto exit; if (!hibmc_dp_get_dpcd(dp_dev)) { ret = connector_status_disconnected; @@ -166,6 +162,9 @@ static void hibmc_dp_encoder_enable(struct drm_encoder *drm_encoder, struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder); struct drm_display_mode *mode = &drm_encoder->crtc->state->mode; + if (dp->hpd_status != HIBMC_HPD_IN) + return; + if (hibmc_dp_prepare(dp, mode)) return; @@ -189,24 +188,31 @@ 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, exp_status; + int status = priv->dp.hpd_status; + 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"); + if (status != HIBMC_HPD_IN) { + drm_err(&priv->dev, "HPD status (%d) error", status); + goto exit; + } hibmc_dp_hpd_cfg(&priv->dp); - exp_status = HIBMC_HPD_IN; } else { drm_dbg_dp(&priv->dev, "HPD OUT isr occur!\n"); + if (status != HIBMC_HPD_OUT) { + drm_err(&priv->dev, "HPD status (%d) error", status); + goto exit; + } hibmc_dp_reset_link(&priv->dp); - exp_status = HIBMC_HPD_OUT; } - if (hibmc_dp_check_hpd_status(&priv->dp, exp_status)) - drm_connector_helper_hpd_irq_event(&priv->dp.connector); + drm_connector_helper_hpd_irq_event(&priv->dp.connector); +exit: drm_dev_exit(idx); return IRQ_HANDLED; @@ -223,6 +229,7 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) dp->mmio = priv->mmio; dp->drm_dev = dev; + dp->hpd_status = HIBMC_HPD_OUT; ret = hibmc_dp_hw_init(&priv->dp); if (ret) { diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index 4d85c89f3f88..e5cca7b63b78 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -63,6 +63,7 @@ static irqreturn_t hibmc_dp_interrupt(int irq, void *arg) status = readl(priv->mmio + HIBMC_DP_INTSTAT); if (status) { priv->dp.irq_status = status; + priv->dp.hpd_status = hibmc_dp_get_hpd_status(&priv->dp); writel(status, priv->mmio + HIBMC_DP_INTCLR); return IRQ_WAKE_THREAD; } -- 2.43.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status 2026-07-21 12:43 ` [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status Yongbang Shi @ 2026-08-27 8:16 ` Thomas Zimmermann 2026-08-31 13:40 ` Yongbang Shi 0 siblings, 1 reply; 6+ messages in thread From: Thomas Zimmermann @ 2026-08-27 8:16 UTC (permalink / raw) To: Yongbang Shi, dmitry.baryshkov, tiantao6, maarten.lankhorst, mripard, airlied, daniel, kong.kongxinwei Cc: liangjian010, chenjianmin, fengsheng5, helin52, shenjian15, shaojijie, dri-devel, linux-kernel Hi, you definitely want to go over the Sashiko review for both patches. Am 21.07.26 um 14:43 schrieb Yongbang Shi: > From: Lin He <helin52@huawei.com> > > To more accurately determine whether the current HPD status matches the > interrupt status, the polling mechanism in the lower half of the HPD > interrupt (via hibmc_dp_check_hpd_status) has been replaced with directly > retrieving the interrupt status in the upper half of the interrupt (via > hibmc_dp_get_hpd_status). The upper half might run a lot if the IRQ is shared with other devices. It's usually better to do as little as possible there and keep things in the handler thread. Best regards Thomas > > * Detection and training are not performed if hpd_status is not HPD_IN. > > * Set the initial status of hpd_status to HPD_OUT. > > Fixes: 3906e7a3b26d ("drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq") > Signed-off-by: Lin He <helin52@huawei.com> > Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> > --- > ChangeLog: > v1 -> v2: > - More states in HIBMC_DP_HPD_STATUS are added to the > 'hibmc_dp_get_hpd_status'. > --- > drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h | 1 - > drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 32 ++++++++++++------- > drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 4 ++- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 29 ++++++++++------- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 + > 5 files changed, 43 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h > index f53dac256ee0..b0e258b9265e 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h > @@ -43,7 +43,6 @@ struct hibmc_dp_dev { > u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; > struct drm_dp_desc desc; > bool is_branch; > - int hpd_status; > void __iomem *serdes_base; > }; > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c > index d5bd3c45649b..c9a113a1937d 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c > @@ -191,6 +191,10 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp) > 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); > + /* To latch the HPD interrupt, ensuring that DP can support more modes > + * within the fbcon framework when connected alone. > + */ > + msleep(100); > > return 0; > } > @@ -322,20 +326,26 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg) > writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL); > } > > -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status) > +int hibmc_dp_get_hpd_status(struct hibmc_dp *dp) > { > + int ret = HIBMC_HPD_UNKNOWN; > u32 status; > - int ret; > > - ret = readl_poll_timeout(dp->dp_dev->base + HIBMC_DP_HPD_STATUS, status, > - FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) == exp_status, > - 1000, 100000); /* DP spec says 100ms */ > - if (ret) { > - drm_dbg_dp(dp->drm_dev, "wait hpd status timeout"); > - return false; > + status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, > + readl(dp->dp_dev->base + HIBMC_DP_HPD_STATUS)); > + switch (status) { > + case 0: /* idle */ > + case 3: /* unplug */ > + case 4: /* unplug intermediate */ > + ret = HIBMC_HPD_OUT; > + break; > + case 1: /* plug */ > + case 2: /* plug intermediate */ > + ret = HIBMC_HPD_IN; > + break; > + default: > + break; > } > > - dp->dp_dev->hpd_status = exp_status; > - > - return true; > + return ret; > } > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h > index 0f3662d8737e..bcd4e9d155c8 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h > @@ -15,6 +15,7 @@ > struct hibmc_dp_dev; > > enum hibmc_hpd_status { > + HIBMC_HPD_UNKNOWN, > HIBMC_HPD_OUT, > HIBMC_HPD_IN, > }; > @@ -55,6 +56,7 @@ struct hibmc_dp { > struct drm_dp_aux aux; > struct hibmc_dp_cbar_cfg cfg; > u32 irq_status; > + int hpd_status; > int phys_status; > }; > > @@ -66,7 +68,7 @@ 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); > -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status); > +int hibmc_dp_get_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); > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c > index 2e9403b8bf3c..23716d48149f 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c > @@ -63,12 +63,8 @@ static int hibmc_dp_detect(struct drm_connector *connector, > struct hibmc_dp_dev *dp_dev = dp->dp_dev; > int ret = connector_status_disconnected; > > - if (dp->irq_status) { > - if (dp_dev->hpd_status != HIBMC_HPD_IN) { > - ret = connector_status_disconnected; > - goto exit; > - } > - } > + if (dp->hpd_status != HIBMC_HPD_IN) > + goto exit; > > if (!hibmc_dp_get_dpcd(dp_dev)) { > ret = connector_status_disconnected; > @@ -166,6 +162,9 @@ static void hibmc_dp_encoder_enable(struct drm_encoder *drm_encoder, > struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder); > struct drm_display_mode *mode = &drm_encoder->crtc->state->mode; > > + if (dp->hpd_status != HIBMC_HPD_IN) > + return; > + > if (hibmc_dp_prepare(dp, mode)) > return; > > @@ -189,24 +188,31 @@ 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, exp_status; > + int status = priv->dp.hpd_status; > + 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"); > + if (status != HIBMC_HPD_IN) { > + drm_err(&priv->dev, "HPD status (%d) error", status); > + goto exit; > + } > hibmc_dp_hpd_cfg(&priv->dp); > - exp_status = HIBMC_HPD_IN; > } else { > drm_dbg_dp(&priv->dev, "HPD OUT isr occur!\n"); > + if (status != HIBMC_HPD_OUT) { > + drm_err(&priv->dev, "HPD status (%d) error", status); > + goto exit; > + } > hibmc_dp_reset_link(&priv->dp); > - exp_status = HIBMC_HPD_OUT; > } > > - if (hibmc_dp_check_hpd_status(&priv->dp, exp_status)) > - drm_connector_helper_hpd_irq_event(&priv->dp.connector); > + drm_connector_helper_hpd_irq_event(&priv->dp.connector); > > +exit: > drm_dev_exit(idx); > > return IRQ_HANDLED; > @@ -223,6 +229,7 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) > > dp->mmio = priv->mmio; > dp->drm_dev = dev; > + dp->hpd_status = HIBMC_HPD_OUT; > > ret = hibmc_dp_hw_init(&priv->dp); > if (ret) { > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > index 4d85c89f3f88..e5cca7b63b78 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > @@ -63,6 +63,7 @@ static irqreturn_t hibmc_dp_interrupt(int irq, void *arg) > status = readl(priv->mmio + HIBMC_DP_INTSTAT); > if (status) { > priv->dp.irq_status = status; > + priv->dp.hpd_status = hibmc_dp_get_hpd_status(&priv->dp); > writel(status, priv->mmio + HIBMC_DP_INTCLR); > return IRQ_WAKE_THREAD; > } -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status 2026-08-27 8:16 ` Thomas Zimmermann @ 2026-08-31 13:40 ` Yongbang Shi 2026-09-02 7:32 ` Thomas Zimmermann 0 siblings, 1 reply; 6+ messages in thread From: Yongbang Shi @ 2026-08-31 13:40 UTC (permalink / raw) To: Thomas Zimmermann, dmitry.baryshkov, tiantao (H), maarten.lankhorst, mripard, airlied, daniel, kongxinwei (A) Cc: Liangjian(Jim,Kunpeng Solution Development Dept), Chenjianmin, fengsheng (A), helin (T), shenjian (K), shaojijie, dri-devel, linux-kernel, shiyongbang, helin52 > Hi, > > you definitely want to go over the Sashiko review for both patches. > Yes, Sashiko has raised several historical issues that we've been analyzing recently. The feedback via email and the v3 patch will be ready very soon. Sashiko was recently launched, and I'd like to ask: if we want to reject Sashiko's suggestions, is it sufficient to simply state the reasons for the rejection in our reply email? How can we prevent Sashiko from raising the same issues in the next version's patch? > Am 21.07.26 um 14:43 schrieb Yongbang Shi: >> From: Lin He <helin52@huawei.com> >> >> To more accurately determine whether the current HPD status matches the >> interrupt status, the polling mechanism in the lower half of the HPD >> interrupt (via hibmc_dp_check_hpd_status) has been replaced with directly >> retrieving the interrupt status in the upper half of the interrupt (via >> hibmc_dp_get_hpd_status). > > The upper half might run a lot if the IRQ is shared with other devices. > It's usually better to do as little as possible there and keep things in > the handler thread. > Of course, that's exactly what we did. Thanks, Yongbang. > Best regards > Thomas > >> >> * Detection and training are not performed if hpd_status is not HPD_IN. >> >> * Set the initial status of hpd_status to HPD_OUT. >> >> Fixes: 3906e7a3b26d ("drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq") >> Signed-off-by: Lin He <helin52@huawei.com> >> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >> --- >> ChangeLog: >> v1 -> v2: >> - More states in HIBMC_DP_HPD_STATUS are added to the >> 'hibmc_dp_get_hpd_status'. >> --- >> drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h | 1 - >> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 32 ++++++++++++------- >> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 4 ++- >> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 29 ++++++++++------- >> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 + >> 5 files changed, 43 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >> index f53dac256ee0..b0e258b9265e 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >> @@ -43,7 +43,6 @@ struct hibmc_dp_dev { >> u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; >> struct drm_dp_desc desc; >> bool is_branch; >> - int hpd_status; >> void __iomem *serdes_base; >> }; >> >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >> index d5bd3c45649b..c9a113a1937d 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >> @@ -191,6 +191,10 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp) >> 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); >> + /* To latch the HPD interrupt, ensuring that DP can support more modes >> + * within the fbcon framework when connected alone. >> + */ >> + msleep(100); >> >> return 0; >> } >> @@ -322,20 +326,26 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, const struct hibmc_dp_cbar_cfg *cfg) >> writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + HIBMC_DP_TIMING_SYNC_CTRL); >> } >> >> -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status) >> +int hibmc_dp_get_hpd_status(struct hibmc_dp *dp) >> { >> + int ret = HIBMC_HPD_UNKNOWN; >> u32 status; >> - int ret; >> >> - ret = readl_poll_timeout(dp->dp_dev->base + HIBMC_DP_HPD_STATUS, status, >> - FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) == exp_status, >> - 1000, 100000); /* DP spec says 100ms */ >> - if (ret) { >> - drm_dbg_dp(dp->drm_dev, "wait hpd status timeout"); >> - return false; >> + status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, >> + readl(dp->dp_dev->base + HIBMC_DP_HPD_STATUS)); >> + switch (status) { >> + case 0: /* idle */ >> + case 3: /* unplug */ >> + case 4: /* unplug intermediate */ >> + ret = HIBMC_HPD_OUT; >> + break; >> + case 1: /* plug */ >> + case 2: /* plug intermediate */ >> + ret = HIBMC_HPD_IN; >> + break; >> + default: >> + break; >> } >> >> - dp->dp_dev->hpd_status = exp_status; >> - >> - return true; >> + return ret; >> } >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >> index 0f3662d8737e..bcd4e9d155c8 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >> @@ -15,6 +15,7 @@ >> struct hibmc_dp_dev; >> >> enum hibmc_hpd_status { >> + HIBMC_HPD_UNKNOWN, >> HIBMC_HPD_OUT, >> HIBMC_HPD_IN, >> }; >> @@ -55,6 +56,7 @@ struct hibmc_dp { >> struct drm_dp_aux aux; >> struct hibmc_dp_cbar_cfg cfg; >> u32 irq_status; >> + int hpd_status; >> int phys_status; >> }; >> >> @@ -66,7 +68,7 @@ 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); >> -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status); >> +int hibmc_dp_get_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); >> >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >> index 2e9403b8bf3c..23716d48149f 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >> @@ -63,12 +63,8 @@ static int hibmc_dp_detect(struct drm_connector *connector, >> struct hibmc_dp_dev *dp_dev = dp->dp_dev; >> int ret = connector_status_disconnected; >> >> - if (dp->irq_status) { >> - if (dp_dev->hpd_status != HIBMC_HPD_IN) { >> - ret = connector_status_disconnected; >> - goto exit; >> - } >> - } >> + if (dp->hpd_status != HIBMC_HPD_IN) >> + goto exit; >> >> if (!hibmc_dp_get_dpcd(dp_dev)) { >> ret = connector_status_disconnected; >> @@ -166,6 +162,9 @@ static void hibmc_dp_encoder_enable(struct drm_encoder *drm_encoder, >> struct hibmc_dp *dp = container_of(drm_encoder, struct hibmc_dp, encoder); >> struct drm_display_mode *mode = &drm_encoder->crtc->state->mode; >> >> + if (dp->hpd_status != HIBMC_HPD_IN) >> + return; >> + >> if (hibmc_dp_prepare(dp, mode)) >> return; >> >> @@ -189,24 +188,31 @@ 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, exp_status; >> + int status = priv->dp.hpd_status; >> + 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"); >> + if (status != HIBMC_HPD_IN) { >> + drm_err(&priv->dev, "HPD status (%d) error", status); >> + goto exit; >> + } >> hibmc_dp_hpd_cfg(&priv->dp); >> - exp_status = HIBMC_HPD_IN; >> } else { >> drm_dbg_dp(&priv->dev, "HPD OUT isr occur!\n"); >> + if (status != HIBMC_HPD_OUT) { >> + drm_err(&priv->dev, "HPD status (%d) error", status); >> + goto exit; >> + } >> hibmc_dp_reset_link(&priv->dp); >> - exp_status = HIBMC_HPD_OUT; >> } >> >> - if (hibmc_dp_check_hpd_status(&priv->dp, exp_status)) >> - drm_connector_helper_hpd_irq_event(&priv->dp.connector); >> + drm_connector_helper_hpd_irq_event(&priv->dp.connector); >> >> +exit: >> drm_dev_exit(idx); >> >> return IRQ_HANDLED; >> @@ -223,6 +229,7 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) >> >> dp->mmio = priv->mmio; >> dp->drm_dev = dev; >> + dp->hpd_status = HIBMC_HPD_OUT; >> >> ret = hibmc_dp_hw_init(&priv->dp); >> if (ret) { >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c >> index 4d85c89f3f88..e5cca7b63b78 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c >> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c >> @@ -63,6 +63,7 @@ static irqreturn_t hibmc_dp_interrupt(int irq, void *arg) >> status = readl(priv->mmio + HIBMC_DP_INTSTAT); >> if (status) { >> priv->dp.irq_status = status; >> + priv->dp.hpd_status = hibmc_dp_get_hpd_status(&priv->dp); >> writel(status, priv->mmio + HIBMC_DP_INTCLR); >> return IRQ_WAKE_THREAD; >> } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status 2026-08-31 13:40 ` Yongbang Shi @ 2026-09-02 7:32 ` Thomas Zimmermann 0 siblings, 0 replies; 6+ messages in thread From: Thomas Zimmermann @ 2026-09-02 7:32 UTC (permalink / raw) To: Yongbang Shi, dmitry.baryshkov, tiantao (H), maarten.lankhorst, mripard, airlied, daniel, kongxinwei (A) Cc: Liangjian(Jim,Kunpeng Solution Development Dept), Chenjianmin, fengsheng (A), helin (T), shenjian (K), shaojijie, dri-devel, linux-kernel Hi Am 31.08.26 um 15:40 schrieb Yongbang Shi: >> Hi, >> >> you definitely want to go over the Sashiko review for both patches. >> > > Yes, Sashiko has raised several historical issues that we've been > analyzing recently. The feedback > via email and the v3 patch will be ready very soon. > > Sashiko was recently launched, and I'd like to ask: if we want to > reject Sashiko's suggestions, > is it sufficient to simply state the reasons for the rejection in our > reply email? How can we > prevent Sashiko from raising the same issues in the next version's patch? I don't think it is possible to interact directly with Sashiko. So it can happen that it comes back with the same comments again and again. We (DRM community) treat these reviews as suggestions and the final decision about a change is done among humans. For me, I reply to Sashikos comments once on the mailing list so that it is publicly documented. > > >> Am 21.07.26 um 14:43 schrieb Yongbang Shi: >>> From: Lin He <helin52@huawei.com> >>> >>> To more accurately determine whether the current HPD status matches the >>> interrupt status, the polling mechanism in the lower half of the HPD >>> interrupt (via hibmc_dp_check_hpd_status) has been replaced with >>> directly >>> retrieving the interrupt status in the upper half of the interrupt (via >>> hibmc_dp_get_hpd_status). >> >> The upper half might run a lot if the IRQ is shared with other devices. >> It's usually better to do as little as possible there and keep things in >> the handler thread. >> > > Of course, that's exactly what we did. I don't understand. With this patch, the driver does more work in the top half. No? Best regards Thomas > > Thanks, > Yongbang. > >> Best regards >> Thomas >> >>> >>> * Detection and training are not performed if hpd_status is not HPD_IN. >>> >>> * Set the initial status of hpd_status to HPD_OUT. >>> >>> Fixes: 3906e7a3b26d ("drm/hisilicon/hibmc: fix dp probabilistical >>> detect errors after HPD irq") >>> Signed-off-by: Lin He <helin52@huawei.com> >>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >>> --- >>> ChangeLog: >>> v1 -> v2: >>> - More states in HIBMC_DP_HPD_STATUS are added to the >>> 'hibmc_dp_get_hpd_status'. >>> --- >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h | 1 - >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c | 32 >>> ++++++++++++------- >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 4 ++- >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 29 ++++++++++------- >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 1 + >>> 5 files changed, 43 insertions(+), 24 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >>> b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >>> index f53dac256ee0..b0e258b9265e 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_comm.h >>> @@ -43,7 +43,6 @@ struct hibmc_dp_dev { >>> u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS]; >>> struct drm_dp_desc desc; >>> bool is_branch; >>> - int hpd_status; >>> void __iomem *serdes_base; >>> }; >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >>> b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >>> index d5bd3c45649b..c9a113a1937d 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.c >>> @@ -191,6 +191,10 @@ int hibmc_dp_hw_init(struct hibmc_dp *dp) >>> 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); >>> + /* To latch the HPD interrupt, ensuring that DP can support >>> more modes >>> + * within the fbcon framework when connected alone. >>> + */ >>> + msleep(100); >>> return 0; >>> } >>> @@ -322,20 +326,26 @@ void hibmc_dp_set_cbar(struct hibmc_dp *dp, >>> const struct hibmc_dp_cbar_cfg *cfg) >>> writel(HIBMC_DP_SYNC_EN_MASK, dp_dev->base + >>> HIBMC_DP_TIMING_SYNC_CTRL); >>> } >>> -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status) >>> +int hibmc_dp_get_hpd_status(struct hibmc_dp *dp) >>> { >>> + int ret = HIBMC_HPD_UNKNOWN; >>> u32 status; >>> - int ret; >>> - ret = readl_poll_timeout(dp->dp_dev->base + >>> HIBMC_DP_HPD_STATUS, status, >>> - FIELD_GET(HIBMC_DP_HPD_CUR_STATE, status) == >>> exp_status, >>> - 1000, 100000); /* DP spec says 100ms */ >>> - if (ret) { >>> - drm_dbg_dp(dp->drm_dev, "wait hpd status timeout"); >>> - return false; >>> + status = FIELD_GET(HIBMC_DP_HPD_CUR_STATE, >>> + readl(dp->dp_dev->base + HIBMC_DP_HPD_STATUS)); >>> + switch (status) { >>> + case 0: /* idle */ >>> + case 3: /* unplug */ >>> + case 4: /* unplug intermediate */ >>> + ret = HIBMC_HPD_OUT; >>> + break; >>> + case 1: /* plug */ >>> + case 2: /* plug intermediate */ >>> + ret = HIBMC_HPD_IN; >>> + break; >>> + default: >>> + break; >>> } >>> - dp->dp_dev->hpd_status = exp_status; >>> - >>> - return true; >>> + return ret; >>> } >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >>> b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >>> index 0f3662d8737e..bcd4e9d155c8 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h >>> @@ -15,6 +15,7 @@ >>> struct hibmc_dp_dev; >>> enum hibmc_hpd_status { >>> + HIBMC_HPD_UNKNOWN, >>> HIBMC_HPD_OUT, >>> HIBMC_HPD_IN, >>> }; >>> @@ -55,6 +56,7 @@ struct hibmc_dp { >>> struct drm_dp_aux aux; >>> struct hibmc_dp_cbar_cfg cfg; >>> u32 irq_status; >>> + int hpd_status; >>> int phys_status; >>> }; >>> @@ -66,7 +68,7 @@ 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); >>> -bool hibmc_dp_check_hpd_status(struct hibmc_dp *dp, int exp_status); >>> +int hibmc_dp_get_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); >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>> index 2e9403b8bf3c..23716d48149f 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>> @@ -63,12 +63,8 @@ static int hibmc_dp_detect(struct drm_connector >>> *connector, >>> struct hibmc_dp_dev *dp_dev = dp->dp_dev; >>> int ret = connector_status_disconnected; >>> - if (dp->irq_status) { >>> - if (dp_dev->hpd_status != HIBMC_HPD_IN) { >>> - ret = connector_status_disconnected; >>> - goto exit; >>> - } >>> - } >>> + if (dp->hpd_status != HIBMC_HPD_IN) >>> + goto exit; >>> if (!hibmc_dp_get_dpcd(dp_dev)) { >>> ret = connector_status_disconnected; >>> @@ -166,6 +162,9 @@ static void hibmc_dp_encoder_enable(struct >>> drm_encoder *drm_encoder, >>> struct hibmc_dp *dp = container_of(drm_encoder, struct >>> hibmc_dp, encoder); >>> struct drm_display_mode *mode = &drm_encoder->crtc->state->mode; >>> + if (dp->hpd_status != HIBMC_HPD_IN) >>> + return; >>> + >>> if (hibmc_dp_prepare(dp, mode)) >>> return; >>> @@ -189,24 +188,31 @@ 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, exp_status; >>> + int status = priv->dp.hpd_status; >>> + 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"); >>> + if (status != HIBMC_HPD_IN) { >>> + drm_err(&priv->dev, "HPD status (%d) error", status); >>> + goto exit; >>> + } >>> hibmc_dp_hpd_cfg(&priv->dp); >>> - exp_status = HIBMC_HPD_IN; >>> } else { >>> drm_dbg_dp(&priv->dev, "HPD OUT isr occur!\n"); >>> + if (status != HIBMC_HPD_OUT) { >>> + drm_err(&priv->dev, "HPD status (%d) error", status); >>> + goto exit; >>> + } >>> hibmc_dp_reset_link(&priv->dp); >>> - exp_status = HIBMC_HPD_OUT; >>> } >>> - if (hibmc_dp_check_hpd_status(&priv->dp, exp_status)) >>> - drm_connector_helper_hpd_irq_event(&priv->dp.connector); >>> + drm_connector_helper_hpd_irq_event(&priv->dp.connector); >>> +exit: >>> drm_dev_exit(idx); >>> return IRQ_HANDLED; >>> @@ -223,6 +229,7 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) >>> dp->mmio = priv->mmio; >>> dp->drm_dev = dev; >>> + dp->hpd_status = HIBMC_HPD_OUT; >>> ret = hibmc_dp_hw_init(&priv->dp); >>> if (ret) { >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c >>> b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c >>> index 4d85c89f3f88..e5cca7b63b78 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c >>> @@ -63,6 +63,7 @@ static irqreturn_t hibmc_dp_interrupt(int irq, >>> void *arg) >>> status = readl(priv->mmio + HIBMC_DP_INTSTAT); >>> if (status) { >>> priv->dp.irq_status = status; >>> + priv->dp.hpd_status = hibmc_dp_get_hpd_status(&priv->dp); >>> writel(status, priv->mmio + HIBMC_DP_INTCLR); >>> return IRQ_WAKE_THREAD; >>> } >> > -- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH for drm-misc-fixes v2 2/2] drm/hisilicon/hibmc: Add a flag to indicate whether the OS-side driver has been loaded 2026-07-21 12:43 [PATCH for drm-misc-fixes v2 0/2] Fix some bugs in the hibmc DP Yongbang Shi 2026-07-21 12:43 ` [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status Yongbang Shi @ 2026-07-21 12:43 ` Yongbang Shi 1 sibling, 0 replies; 6+ messages in thread From: Yongbang Shi @ 2026-07-21 12:43 UTC (permalink / raw) To: tzimmermann, dmitry.baryshkov, tiantao6, maarten.lankhorst, mripard, airlied, daniel, kong.kongxinwei Cc: liangjian010, chenjianmin, fengsheng5, shiyongbang, helin52, shenjian15, shaojijie, dri-devel, linux-kernel From: Lin He <helin52@huawei.com> Add a flag to indicate whether the OS-side driver has been loaded to prevent the BMC from enabling DP if the driver is not loaded, which could lead to system failure in handling interrupts and generate error messages like: irq xx: nobody cared (try booting with the "irqpoll" option) ... Call Trace: <TRQ> ... Fixes: 0ab6ea261c1f ("drm/hisilicon/hibmc: add dp module in hibmc") Signed-off-by: Lin He <helin52@huawei.com> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> --- ChangeLog: v1 -> v2: - The call to `hibmc_set_enable_flag()` has been moved before `drm_client_setup()`, specifically before the connector detection triggers the I2C bit operation. (sashiko-bot) - A read-modify-write operation has been implemented instead of directly writing to prevent clearing the I2C mask bits. (sashiko-bot) --- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 21 +++++++++++++++++++ .../gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h | 3 +++ 2 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index e5cca7b63b78..53c02f8b8841 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -424,6 +424,24 @@ static int hibmc_load(struct drm_device *dev) return ret; } +static inline void hibmc_set_enable_flag(struct hibmc_drm_private *priv) +{ + u32 value; + + value = readl(priv->mmio + HIBMC_ENABLE_FLAG); + value |= HIBMC_ENABLE_STATE; + writel(value, priv->mmio + HIBMC_ENABLE_FLAG); +} + +static inline void hibmc_set_disable_flag(struct hibmc_drm_private *priv) +{ + u32 value; + + value = readl(priv->mmio + HIBMC_ENABLE_FLAG); + value &= ~HIBMC_ENABLE_STATE; + writel(value, priv->mmio + HIBMC_ENABLE_FLAG); +} + static int hibmc_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) { @@ -466,6 +484,8 @@ static int hibmc_pci_probe(struct pci_dev *pdev, goto err_unload; } + hibmc_set_enable_flag(priv); + drm_client_setup(dev, NULL); return 0; @@ -480,6 +500,7 @@ static void hibmc_pci_remove(struct pci_dev *pdev) { struct drm_device *dev = pci_get_drvdata(pdev); + hibmc_set_disable_flag(to_hibmc_drm_private(dev)); drm_dev_unregister(dev); hibmc_unload(dev); } diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h index 17b30c393b10..b95add5b253c 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_regs.h @@ -200,5 +200,8 @@ #define HIBMC_CRT_PALETTE 0x80C00 +#define HIBMC_ENABLE_FLAG 0x802A4 +#define HIBMC_ENABLE_STATE BIT(2) + #define HIBMC_FIELD(field, value) (field(value) & field##_MASK) #endif -- 2.43.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-02 7:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-07-21 12:43 [PATCH for drm-misc-fixes v2 0/2] Fix some bugs in the hibmc DP Yongbang Shi 2026-07-21 12:43 ` [PATCH for drm-misc-fixes v2 1/2] drm/hisilicon/hibmc: Modify the method of obtaining the hpd_status Yongbang Shi 2026-08-27 8:16 ` Thomas Zimmermann 2026-08-31 13:40 ` Yongbang Shi 2026-09-02 7:32 ` Thomas Zimmermann 2026-07-21 12:43 ` [PATCH for drm-misc-fixes v2 2/2] drm/hisilicon/hibmc: Add a flag to indicate whether the OS-side driver has been loaded Yongbang Shi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®