Re: [PATCH v6 17/22] drm: bridge: dw_hdmi: Drop call to drm_bridge_hpd_notify()

[Jonas Karlman <jonas@kwiboo.se>]

On 5/16/2026 9:52 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing drm_bridge_hpd_notify() violates the DRM bridge API contract and causes hotplug events to be permanently lost during system suspend.
> --
> 
> commit 6b3491e39082015ab62431ea0c93c74c9344b995
> Author: Jonas Karlman <jonas@kwiboo.se>
> 
> drm: bridge: dw_hdmi: Drop call to drm_bridge_hpd_notify()
> 
> This commit removes the drm_bridge_hpd_notify() call from the dw_hdmi IRQ
> handler, relying instead only on drm_helper_hpd_irq_event() to avoid sending
> duplicate hotplug uevents to userspace.
> 
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index e9c4e24c090ca..6cc7b2a860bd8 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -3101,10 +3101,8 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
>>  			status == connector_status_connected ?
>>  			"plugin" : "plugout");
>>  
>> -		if (hdmi->bridge.dev) {
>> +		if (hdmi->bridge.dev)
>>  			drm_helper_hpd_irq_event(hdmi->bridge.dev);
>> -			drm_bridge_hpd_notify(&hdmi->bridge, status);
>> -		}
>>  	}
> 
> Will removing drm_bridge_hpd_notify() cause hotplug events to be permanently
> lost if they occur during system suspend?

No, drm_mode_config_helper_resume() is called at resume.

> Since the driver advertises DRM_BRIDGE_OP_HPD, does the DRM bridge API
> contract still require it to call drm_bridge_hpd_notify() to properly
> propagate hotplug events to the rest of the bridge chain?

No, hpd_notify() funcs are still called from bridge connector detect().

> Looking at drm_helper_hpd_irq_event(), it aborts early if polling is
> disabled:
> 
> drivers/gpu/drm/drm_probe_helper.c:drm_helper_hpd_irq_event() {
> 	struct drm_connector *connector, *first_changed_connector = NULL;
> 	struct drm_connector_list_iter conn_iter;
> 	int changed = 0;
> 
> 	if (!dev->mode_config.poll_enabled)
> 		return false;
> ...
> }
> 
> During system suspend, drm_kms_helper_poll_disable() sets poll_enabled
> to false. If an HPD interrupt fires while the system is suspended, won't
> drm_helper_hpd_irq_event() silently drop the event without updating the
> connector status or scheduling any deferred work?

No, drm_mode_config_helper_suspend/resume() will call hpd_enable/disable()
to signal when the connector/bridge should start listening for HPD. I.e.
not during suspend.

> Furthermore, when the system resumes, output_poll_execute() explicitly skips
> connectors that use the DRM_CONNECTOR_POLL_HPD flag, and the dw_hdmi resume
> handler does not appear to manually trigger a hotplug event check.
> 
> Could this result in the system being completely unaware of any display
> connections or disconnections that happened while suspended, at least until
> a manual reprobe is forced by userspace?

No, drm_mode_config_helper_resume() is called at resume.

Regards,
Jonas

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic