[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1dd93bb7-4f67-4b9b-8b6a-d7c5c77cf807@huawei.com>
Date: Thu, 14 Aug 2025 20:19:41 +0800
From: Yongbang Shi <shiyongbang@...wei.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
CC: <xinliang.liu@...aro.org>, <tiantao6@...ilicon.com>,
<maarten.lankhorst@...ux.intel.com>, <mripard@...nel.org>,
<tzimmermann@...e.de>, <airlied@...il.com>, <daniel@...ll.ch>,
<kong.kongxinwei@...ilicon.com>, <liangjian010@...wei.com>,
<chenjianmin@...wei.com>, <fengsheng5@...wei.com>, <libaihan@...wei.com>,
<shenjian15@...wei.com>, <shaojijie@...wei.com>,
<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
<shiyongbang@...wei.com>
Subject: Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp
probabilistical detect errors after HPD irq
> On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
>> From: Baihan Li <libaihan@...wei.com>
>>
>> The debouncing when HPD pulled out still remains sometimes, 200ms still can
>> not ensure helper_detect() is correct. So add a flag to hold the sink
>> status, and changed detect_ctx() functions by using flag to check status.
> THis doesn't explain what is wrong with
> drm_connector_helper_detect_from_ddc(). In the end, this function
> doesn't use the HPD pin.
I'm sorry about the misunderstanding.
The issue is that after plugging or unplugging the monitor, the driver takes no action sometimes
even though an interrupt is triggered. The root cause is that drm_connector_helper_detect_from_ddc()
still returns connected status when the monitor is unplugged.
And I will fix the way in the end.
Thanks,
Baihan Li!
>> Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
>> Signed-off-by: Baihan Li <libaihan@...wei.com>
>> Signed-off-by: Yongbang Shi <shiyongbang@...wei.com>
>> ---
>> ChangeLog:
>> v3 -> v4:
>> - remove link training process in hibmc_dp_detect(), suggested by Dmitry Baryshkov.
>> - remove if (dev->registered), suggested by Dmitry Baryshkov.
>> ---
>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h | 1 +
>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 19 ++++++++++++-------
>> 2 files changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
>> index 665f5b166dfb..68867475508c 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_hw.h
>> @@ -50,6 +50,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);
>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> index d06832e62e96..ded38530ecda 100644
>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c
>> @@ -34,9 +34,12 @@ 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);
>>
>> - return drm_connector_helper_detect_from_ddc(connector, ctx, force);
>> + if (dp->hpd_status)
>> + return connector_status_connected;
>> + else
>> + return connector_status_disconnected;
>> }
>>
>> static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = {
>> @@ -115,21 +118,23 @@ 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);
>> + struct hibmc_dp *dp = &priv->dp;
>> int idx;
>>
>> if (!drm_dev_enter(dev, &idx))
>> return -ENODEV;
>>
>> - if (priv->dp.irq_status & DP_MASKED_SINK_HPD_PLUG_INT) {
>> + if (((dp->irq_status & DP_MASKED_SINK_HPD_PLUG_INT) && !dp->hpd_status)) {
>> drm_dbg_dp(&priv->dev, "HPD IN isr occur!\n");
>> - hibmc_dp_hpd_cfg(&priv->dp);
>> + hibmc_dp_hpd_cfg(dp);
>> + dp->hpd_status = 1;
>> } else {
>> drm_dbg_dp(&priv->dev, "HPD OUT isr occur!\n");
>> - hibmc_dp_reset_link(&priv->dp);
>> + hibmc_dp_reset_link(dp);
>> + dp->hpd_status = 0;
>> }
>>
>> - if (dev->registered)
>> - drm_connector_helper_hpd_irq_event(&priv->dp.connector);
>> + drm_connector_helper_hpd_irq_event(&priv->dp.connector);
>>
>> drm_dev_exit(idx);
>>
>> --
>> 2.33.0
>>
Powered by blists - more mailing lists