Hi, On Tue, Apr 5, 2022 at 5:54 AM Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote: > > 3. For DP and eDP HPD means something a little different. Essentially > > there are two concepts: a) is a display physically connected and b) is > > the display powered up and ready. For DP, the two are really tied > > together. From the kernel's point of view you never "power down" a DP > > display and you can't detect that it's physically connected until it's > > ready. Said another way, on you tie "is a display there" to the HPD > > line and the moment a display is there it's ready for you to do AUX > > transfers. For eDP, in the lowest power state of a display it _won't_ > > assert its "HPD" signal. However, it's still physically present. For > > eDP you simply have to _assume_ it's present without any actual proof > > since you can't get proof until you power it up. Thus for eDP, you > > report that the display is there as soon as we're asked. We can't > > _talk_ to the display yet, though. So in get_modes() we need to be > > able to power the display on enough to talk over the AUX channel to > > it. As part of this, we wait for the signal named "HPD" which really > > means "panel finished powering on" in this context. > > > > NOTE: for aux transfer, we don't have the _display_ pipe and clocks > > running. We only have enough stuff running to do the AUX transfer. > > We're not clocking out pixels. We haven't fully powered on the > > display. The AUX transfer is designed to be something that can be done > > early _before_ you turn on the display. > > > > > > OK, so basically that was a longwinded way of saying: yes, we could > > avoid the AUX transfer in probe, but we can't wait all the way to > > enable. We have to be able to transfer in get_modes(). If you think > > that's helpful I think it'd be a pretty easy patch to write even if it > > would look a tad bit awkward IMO. Let me know if you want me to post > > it up. > > I think it would be a good idea. At least it will allow us to judge, > which is the more correct way. I'm still happy to prototype this, but the more I think about it the more it feels like a workaround for the Qualcomm driver. The eDP panel driver is actually given a pointer to the AUX bus at probe time. It's really weird to say that we can't do a transfer on it yet... As you said, this is a little sideband bus. It should be able to be used without all the full blown infra of the rest of the driver. > And I also think it might help the ti,sn65dsi86 driver, as it won't > have to ensure that gpio is available during the AUX bus probe. The ti,sn65dsi86 GPIO issue has been solved for a while, though so not sure why we need to do something there? I'm also unclear how it would have helped. In this discussion, we've agreed that the panel driver would still acquire resources during its probe time and the only thing that would be delayed would be the first AUX transfer. The GPIO is a resource here and it's ideal to acquire it at probe time so we could EPROBE_DEFER if needed. > BTW, another random idea, before you start coding. > > We have the bridge's hpd_notify call. Currently it is called only by > the means of drm_bridge_connector's HPD mechanism, tied to the bridge > registering as DRM_BRIDGE_OP_HPD. > It looks to me like it might be a perfect fit for the first aux-bus > related reads. > > We'd need to trigger it manually once and tie it to the new > drm_panel_funcs callback, which in turn would probe the aux bus, > create backlight, etc. > > Regarding the Sankeerth's patch. I have been comparing it with the > hpd_event_thread()'s calls. > It looks to me like we should reuse dp_display_config_hpd() > /EV_HPD_INIT_SETUP and maybe others. > > What I'm trying to say is that if we split AUX probing and first AUX > transfers, it would be possible to reuse a significant part of MSM DP > HPD machine rather than hacking around it and replicating it manually. I'm not sure I completely understand, but I'm pretty wary here. It's my assertion that all of the current "HPD" infrastructure in DRM all relates to the physical presence of the panel. If you start implementing these functions for eDP I think you're going to confuse the heck out of everything. The kernel will think that this is a display that's sometimes not there. Whenever the display is powered off then HPD will be low and it will look like there's no display. Nothing will ever try to power it on because it looks like there's no display. I think your idea is to "trigger once" at bootup and then it all magically works, right? ...but what about after bootup? If you turn the display off for whatever reason (modeset or you simply close the lid of your laptop because you're using an external display) and then you want to use the eDP display again, how do you kickstart the process another time? You can't reboot, and when the display is off the HPD line is low. I can't say it enough times, HPD on eDP _does not mean hot plug detect_. The panel is always there. HPD is really a "panel ready / panel notify" signal for eDP. That's fully what its function is. -Doug
