Hi Daniel,
On Tue, Sep 14, 2021 at 04:34:08PM +0200, Daniel Vetter wrote:
On Tue, Sep 14, 2021 at 12:17:24PM +0200, Maxime Ripard wrote:
The drm_helper_hpd_irq_event() documentation states that this function is "useful for drivers which can't or don't track hotplug interrupts for each connector." and that "Drivers which support hotplug interrupts for each connector individually and which have a more fine-grained detect logic should bypass this code and directly call drm_kms_helper_hotplug_event()". This is thus what we ended-up doing.
However, what this actually means, and is further explained in the drm_kms_helper_hotplug_event() documentation, is that drm_kms_helper_hotplug_event() should be called by drivers that can track the connection status change, and if it has changed we should call that function.
This underlying expectation we failed to provide is that the caller of drm_kms_helper_hotplug_event() should call drm_helper_probe_detect() to probe the new status of the connector.
Since we didn't do it, it meant that even though we were sending the notification to user-space and the DRM clients that something changed we never probed or updated our internal connector status ourselves.
This went mostly unnoticed since the detect callback usually doesn't have any side-effect. Also, if we were using the DRM fbdev emulation (which is a DRM client), or any user-space application that can deal with hotplug events, chances are they would react to the hotplug event by probing the connector status eventually.
However, now that we have to enable the scrambler in detect() if it was enabled it has a side effect, and an application such as Kodi or modetest doesn't deal with hotplug events. This resulted with a black screen when Kodi or modetest was running when a screen was disconnected and then reconnected, or switched off and on.
Uh, why are you running this scrambler restore in your probe function? I guess it works, but most drivers that do expensive hotplug restore to handle the "no black screen for replug" use-case handle that in their own dedicated code.
That's what I got from our discussion back in June (I think?). The discussion was about an issue we were having back then where one would disconnect / reconnect the display while it had been setup through SCDC to use the scrambler and higher bit ratio.
During that power cycle (that also happens when you turn a TV off and on), the display would obviously reset its SCDC status. But if there's nothing to handle the uevent, then the same mode remains applied resulting in a blank screen.
If we're not supposed to set the SCDC status again in detect(), how would we deal with this?
But those also tend to have per-output hpd interrupt sources, so maybe that's why?
We do have an per-output HPD interrupt here
Maximey