1. Patchew
  2. linux
  3. [PATCH v4 drm-dp 00/11] Fix hibmc driver bugs
  4. View patch

[PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq

Yongbang Shi posted 11 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Yongbang Shi 1 month, 3 weeks ago 
From: Baihan Li <libaihan@huawei.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.

Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
Signed-off-by: Baihan Li <libaihan@huawei.com>
Signed-off-by: Yongbang Shi <shiyongbang@huawei.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
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Dmitry Baryshkov 1 month, 3 weeks ago 
On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
> From: Baihan Li <libaihan@huawei.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.

> 
> Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
> Signed-off-by: Baihan Li <libaihan@huawei.com>
> Signed-off-by: Yongbang Shi <shiyongbang@huawei.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
> 

-- 
With best wishes
Dmitry
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Yongbang Shi 1 month, 3 weeks ago 
> On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
>> From: Baihan Li <libaihan@huawei.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@huawei.com>
>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.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
>>
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Dmitry Baryshkov 1 month, 2 weeks ago 
On Thu, Aug 14, 2025 at 08:19:41PM +0800, Yongbang Shi wrote:
> 
> > On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
> > > From: Baihan Li <libaihan@huawei.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.

Can you perform a normal DP detection: read DPCD and check that there is
a DPRX attached and that it's either non-branch device or it has one or
more sinks?

> 
> Thanks,
> Baihan Li!
> 
> 
> > > Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
> > > Signed-off-by: Baihan Li <libaihan@huawei.com>
> > > Signed-off-by: Yongbang Shi <shiyongbang@huawei.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
> > > 

-- 
With best wishes
Dmitry
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Yongbang Shi 3 weeks, 2 days ago 
> On Thu, Aug 14, 2025 at 08:19:41PM +0800, Yongbang Shi wrote:
>>> On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
>>>> From: Baihan Li <libaihan@huawei.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.
> Can you perform a normal DP detection: read DPCD and check that there is
> a DPRX attached and that it's either non-branch device or it has one or
> more sinks?

I'm very sorry that I didn't get the last sentence's asking before.
It's a non-branch device. We just connect a DP monitor.

Thanks,
Baihan Li!

>> Thanks,
>> Baihan Li!
>>
>>
>>>> Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.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
>>>>
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Dmitry Baryshkov 3 weeks, 2 days ago 
On Thu, Sep 11, 2025 at 05:32:40PM +0800, Yongbang Shi wrote:
> 
> > On Thu, Aug 14, 2025 at 08:19:41PM +0800, Yongbang Shi wrote:
> > > > On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
> > > > > From: Baihan Li <libaihan@huawei.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.
> > Can you perform a normal DP detection: read DPCD and check that there is
> > a DPRX attached and that it's either non-branch device or it has one or
> > more sinks?
> 
> I'm very sorry that I didn't get the last sentence's asking before.
> It's a non-branch device. We just connect a DP monitor.

Somebody might connect a different configuration than the one that you
are using.

-- 
With best wishes
Dmitry
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Yongbang Shi 3 weeks, 1 day ago 
> On Thu, Sep 11, 2025 at 05:32:40PM +0800, Yongbang Shi wrote:
>>> On Thu, Aug 14, 2025 at 08:19:41PM +0800, Yongbang Shi wrote:
>>>>> On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
>>>>>> From: Baihan Li <libaihan@huawei.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.
>>> Can you perform a normal DP detection: read DPCD and check that there is
>>> a DPRX attached and that it's either non-branch device or it has one or
>>> more sinks?
>> I'm very sorry that I didn't get the last sentence's asking before.
>> It's a non-branch device. We just connect a DP monitor.
> Somebody might connect a different configuration than the one that you
> are using.

Okay, I can add the check drm_dp_is_branch() in the DP's detect_ctx() to
intercept branch devices, is that good?

Thanks,
Baihan Li
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Dmitry Baryshkov 3 weeks, 1 day ago 
On Fri, Sep 12, 2025 at 09:23:05AM +0800, Yongbang Shi wrote:
> 
> > On Thu, Sep 11, 2025 at 05:32:40PM +0800, Yongbang Shi wrote:
> > > > On Thu, Aug 14, 2025 at 08:19:41PM +0800, Yongbang Shi wrote:
> > > > > > On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
> > > > > > > From: Baihan Li <libaihan@huawei.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.
> > > > Can you perform a normal DP detection: read DPCD and check that there is
> > > > a DPRX attached and that it's either non-branch device or it has one or
> > > > more sinks?
> > > I'm very sorry that I didn't get the last sentence's asking before.
> > > It's a non-branch device. We just connect a DP monitor.
> > Somebody might connect a different configuration than the one that you
> > are using.
> 
> Okay, I can add the check drm_dp_is_branch() in the DP's detect_ctx() to
> intercept branch devices, is that good?

My suggestion is to implement DP detection in the way it's done by other
DP drivers.

-- 
With best wishes
Dmitry
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Yongbang Shi 2 weeks, 5 days ago 
> On Fri, Sep 12, 2025 at 09:23:05AM +0800, Yongbang Shi wrote:
>>> On Thu, Sep 11, 2025 at 05:32:40PM +0800, Yongbang Shi wrote:
>>>>> On Thu, Aug 14, 2025 at 08:19:41PM +0800, Yongbang Shi wrote:
>>>>>>> On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
>>>>>>>> From: Baihan Li <libaihan@huawei.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.
>>>>> Can you perform a normal DP detection: read DPCD and check that there is
>>>>> a DPRX attached and that it's either non-branch device or it has one or
>>>>> more sinks?
>>>> I'm very sorry that I didn't get the last sentence's asking before.
>>>> It's a non-branch device. We just connect a DP monitor.
>>> Somebody might connect a different configuration than the one that you
>>> are using.
>> Okay, I can add the check drm_dp_is_branch() in the DP's detect_ctx() to
>> intercept branch devices, is that good?
> My suggestion is to implement DP detection in the way it's done by other
> DP drivers.

Okay, I will reference the code from other manufacturer ways.

Thanks,
Baihan.
Re: [PATCH v4 drm-dp 02/11] drm/hisilicon/hibmc: fix dp probabilistical detect errors after HPD irq
Posted by Yongbang Shi 1 month, 2 weeks ago 
> On Thu, Aug 14, 2025 at 08:19:41PM +0800, Yongbang Shi wrote:
>>> On Wed, Aug 13, 2025 at 05:42:29PM +0800, Yongbang Shi wrote:
>>>> From: Baihan Li <libaihan@huawei.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.
> Can you perform a normal DP detection: read DPCD and check that there is
> a DPRX attached and that it's either non-branch device or it has one or
> more sinks?

Alright! I will add read_dpcd_caps() and read_sink_count_cap() here to detect DP.

Thanks for your tips!


>> Thanks,
>> Baihan Li!
>>
>>
>>>> Fixes: 3c7623fb5bb6 ("drm/hisilicon/hibmc: Enable this hot plug detect of irq feature")
>>>> Signed-off-by: Baihan Li <libaihan@huawei.com>
>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.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
>>>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.