The use of calls to both drm_helper_hpd_irq_event() and
drm_bridge_hpd_notify() in HPD delayed_work may cause multiple hotplug
uevents and modesets when the bridge connector is used.
Use of drm_helper_hpd_irq_event() cause the internal DRM function
check_connector_changed() to be called, which in turn calls the
connector detect()/force() funcs to detect any connection status or
epoch changes, and when changed trigger a hotplug uevent. For dw-hdmi
connector this also help ensure that EDID and CEC phys addr is updated.
If only a call drm_bridge_hpd_notify() would be used, a custom connector
status/EDID change detection logic needs to be implemented, to fully
match what check_connector_changed() already provides. Update of EDID
and CEC phys addr typically is delayed until userspace trigger a modeset
and fill_modes()/get_modes() ops is called.
The bridge connector detect() func also ensures that any hpd_notify()
funcs are called for all bridges in the chain, so there is not really
any need to have a call to drm_bridge_hpd_notify() here.
With both calls there is two hotplug uevents, two modesets and a total
of four .hpd_notify() calls (using a bridge connector):
dw_hdmi_hardirq(): EVENT=plugout
dw_hdmi_hpd_work()
drm_helper_hpd_irq_event():
dw_hdmi_bridge_hpd_notify(status=2)
[drm:check_connector_changed] [CONNECTOR:46:HDMI-A-1] status updated from
connected to disconnected
[drm:check_connector_changed] [CONNECTOR:46:HDMI-A-1] Changed epoch counter
1 => 2
[drm:drm_sysfs_connector_hotplug_event] [CONNECTOR:46:HDMI-A-1] generating
connector hotplug event
drm_client_hotplug():
[drm:drm_fb_helper_hotplug_event]
[drm:drm_client_modeset_probe]
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:46:HDMI-A-1]
dw_hdmi_bridge_hpd_notify(status=2)
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:46:HDMI-A-1]
disconnected
[drm:drm_edid_connector_update] [CONNECTOR:46:HDMI-A-1] EDID changed, epoch
counter 3
[drm:drm_client_modeset_probe] No connectors reported connected with modes
[drm:drm_client_modeset_probe] [CONNECTOR:46:HDMI-A-1] enabled? no
[drm:drm_client_firmware_config.isra.0] Not using firmware configuration
[drm:drm_client_modeset_probe] picking CRTCs for 3840x2160 config
[drm:drm_client_hotplug] fbdev: ret=0
drm_bridge_hpd_notify():
dw_hdmi_bridge_hpd_notify(status=2)
[drm:drm_sysfs_connector_hotplug_event] [CONNECTOR:46:HDMI-A-1] generating
connector hotplug event
drm_client_hotplug():
[drm:drm_fb_helper_hotplug_event]
[drm:drm_client_modeset_probe]
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:46:HDMI-A-1]
dw_hdmi_bridge_hpd_notify(status=2)
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:46:HDMI-A-1]
disconnected
[drm:drm_client_modeset_probe] No connectors reported connected with modes
[drm:drm_client_modeset_probe] [CONNECTOR:46:HDMI-A-1] enabled? no
[drm:drm_client_firmware_config.isra.0] Not using firmware configuration
[drm:drm_client_modeset_probe] picking CRTCs for 3840x2160 config
[drm:drm_client_hotplug] fbdev: ret=0
Change to only call drm_helper_hpd_irq_event() in HPD delayed_work to
ensure there is only one hotplug uevent and that EDID and CEC phys addr
is updated in a timely manner, independent from userspace having to
react the hotplug uevent.
With only a call the drm_helper_hpd_irq_event() there is only a single
hotplug uevent and only two .hpd_notify() calls:
dw_hdmi_hardirq(): EVENT=plugout
dw_hdmi_hpd_work()
drm_helper_hpd_irq_event():
dw_hdmi_bridge_hpd_notify(status=2)
[drm:check_connector_changed] [CONNECTOR:46:HDMI-A-1] status updated from
connected to disconnected
[drm:check_connector_changed] [CONNECTOR:46:HDMI-A-1] Changed epoch counter
1 => 2
[drm:drm_sysfs_connector_hotplug_event] [CONNECTOR:46:HDMI-A-1] generating
connector hotplug event
drm_client_hotplug():
[drm:drm_fb_helper_hotplug_event]
[drm:drm_client_modeset_probe]
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:46:HDMI-A-1]
dw_hdmi_bridge_hpd_notify(status=2)
[drm:drm_helper_probe_single_connector_modes] [CONNECTOR:46:HDMI-A-1]
disconnected
[drm:drm_edid_connector_update] [CONNECTOR:46:HDMI-A-1] EDID changed, epoch
counter 3
[drm:drm_client_modeset_probe] No connectors reported connected with modes
[drm:drm_client_modeset_probe] [CONNECTOR:46:HDMI-A-1] enabled? no
[drm:drm_client_firmware_config.isra.0] Not using firmware configuration
[drm:drm_client_modeset_probe] picking CRTCs for 3840x2160 config
[drm:drm_client_hotplug] fbdev: ret=0
Signed-off-by: Jonas Karlman <[email protected]>
---
v5: New patch
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 2ea8ce5eca36..d9c9d03f8eff 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3019,14 +3019,28 @@ static irqreturn_t dw_hdmi_hardirq(int irq, void
*dev_id)
static void dw_hdmi_hpd_work(struct work_struct *work)
{
struct dw_hdmi *hdmi = container_of(work, struct dw_hdmi,
hpd_work.work);
- enum drm_connector_status status;
if (WARN_ON(!hdmi->bridge.dev))
return;
+ /*
+ * Notify the DRM core of the HPD event using drm_helper_hpd_irq_event()
+ * instead of drm_bridge_hpd_notify(). This will cause the DRM function
+ * check_connector_changed() to be called, which in turn calls the
+ * connector detect()/force() funcs to detect any connection status or
+ * epoch changes. Something that also triggers EDID and CEC phys address
+ * updates.
+ *
+ * If we were to instead call drm_bridge_hpd_notify() here, we would
+ * have to implement a very similar change detection logic or fully
+ * relay on userspace to react on a hotplug uevent to ensure EDID and
+ * CEC phys address are updated.
+ *
+ * The bridge connector detect() func also ensures that hpd_notify()
+ * funcs are called for all bridges in the chain.
+ */
+
drm_helper_hpd_irq_event(hdmi->bridge.dev);
- status = dw_hdmi_phy_read_hpd(hdmi, hdmi->phy.data);
- drm_bridge_hpd_notify(&hdmi->bridge, status);
}
static const struct dw_hdmi_phy_data dw_hdmi_phys[] = {
--
2.54.0
