Hi Paul, Neil, > Am 03.03.2022 um 18:20 schrieb Paul Cercueil <paul@xxxxxxxxxxxxxxx>: > > Hi Nikolaus, > > [snip] > >>> Well he said "the Ingenic DRM core" aka ingenic-drm-drv.c. You do have access to the main drm_device in the ingenic_drm_bind() function, so you can add it there (with a cleanup function calling drm_kms_helper_poll_fini() registered with drmm_add_action_or_reset()). >> Well, do you really want to mix HPD detection between connector, Synopsys bridge and Ingenic DRM core? These are independent... >> Or should be accessed only through the bridge chain pointers. >> IMHO we should keep separate functions separate. > > The drm_kms_helper_poll_init() just says "this DRM device may have connectors that need to be polled" so it very well fits inside the main driver, IMHO. As far as I understand, it has the side-effect to always set dev->mode_config.poll_enabled and schedule_delayed_work() for all devices. I am not sure if this is intended for arbitrary ingenic-drm devices. But you know better than me. Hm. But wait, I think I now finally remember why I have proposed it the way it is! It is always better to go back to requirements and find the least invasive solution. - HPD IRQ works and calls dw_hdmi_irq() [as can be shown by adding printk()] - it is just that the udevd is only notified if poll_enabled = true (but no polling takes place!). An earlier version (v4) to fix this proposed to add an explicit call to drm_kms_helper_hotplug_event() in dw_hdmi_irq() but that was rejected a while ago because drm_helper_hpd_irq_event() will already call it: https://www.spinics.net/lists/dri-devel/msg316846.html Since this did not take into account that dev->mode_config.poll_enabled must be set true, I then proposed the enable_poll() mechanism just to set this bit for the ingenic-dw-hdmi specialization. So a HPD event is delivered to the dw-hdmi driver as dw_hdmi_irq() and that calls drm_helper_hpd_irq_event() but not drm_kms_helper_hotplug_event() and user-space is not getting aware. It is all a hack because we mix the dw-hdmi driver which originally did register its own connector with an explicit connector... In summary I now thing that the v4 patch is the simplest and least invasive solution. We neither have to introduce a dw_hdmi_enable_poll() function or call drm_kms_helper_poll_init() anywhere. It is just a single line to add to dw-hdmi. And neither touches ingenic-dw-hdmi nor ingenic-drm-drv. So let's go back to v4 version (just modify commit message to better describe why we have to call drm_kms_helper_hotplug_event() explicitly) and forget about alternatives. BR, Nikolaus
