We have already added a patch, in I915 driver, which caches the EDID, in connector, and releases it only after a hot unplug. The patch is under review, but may be the same logic can be reused here. The problem is, every time we have a connector->detect() call, most of the driver re-read the EDID, which is not an efficient design. connector->detect() gets called many times during bootup, even without a hotplug/unplug, which causes many EDID reads for HDMI/DP. Ideally we should read EDID only when there is a hotplug, and release the cached EDID when there is a hot-unplug. The method can go like this: 1. Cache the EDID in connector->edid, when there is really a hot plug-in interrupt (encoder->hot_plug() function can be used for the same) 2. Do not read the EDID again, in any connector->detect() functions, just used the cached EDID from connector->edid for all decision making. 3. Release/Free the EDID from connecotor->edid, when there is really a hot-unplug (again encoder->hot_plug() function can be used) If we can maintain this state machine, there won't be any extra EDID read at all. Hope this was not complete waste of your time :) Regards Shashank -----Original Message----- From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Russell King - ARM Linux Sent: Monday, August 10, 2015 8:17 PM To: Thierry Reding Cc: Paul Menzel; dri-devel@xxxxxxxxxxxxxxxxxxxxx Subject: Re: Caching of EDID for X server to decrease startup time of X server On Mon, Aug 10, 2015 at 03:01:49PM +0200, Thierry Reding wrote: > On Sat, Aug 08, 2015 at 10:18:57PM +0200, Paul Menzel wrote: > > Is it correct, that EDID is probed for both outputs? Is that necessary? > > I assume during startup the Linux kernel for example already > > collected this information and if no change of monitor/output is > > detected, the EDID should be the same, right? > > > > Is that already implement and I just need to apply the correct > > configuration? > > > > Any suggestions on how to decrease the startup time for the X server > > are much appreciated. > > Russell and Sascha were discussing this kind of caching in the i.MX > driver recently. Adding both for visibility. Also not trimming the > quote in case they don't have the original. > > It sounds like this could be useful to have in the core. Yes, as I mentioned in the discussion between Sascha and myself, I think this should be implemented in the DRM core rather than each and every driver. My reasoning is that the pretense for caching the EDID is basically one of performance: we don't want to keep on performing the time consuming I2C bus cycles to read it. However, drm_helper_probe_single_connector_modes_merge_bits() itself can be very expensive - it can call out to drm_load_edid_firmware(), which can involve requesting EDID firmware - which results in calling out to userspace. So, if we're concerned about performance, I think it would be better to solve all the performance problems in this path in a generic way which covers both issues. > As I understand > it, hotplug detection is pretty well specified for more modern display > interfaces (like HDMI and DisplayPort), so I think caching of this > sort could work for those. However, I think some older interfaces such > as VGA (or perhaps even DVI as well) don't have reliable hotplug > detection and hence would need to be able to force reading the EDID. Yes, I think I've seen code in DRM which does hotplug-detection-by-EDID. Those are normally doing so in their ->detect callback, and using the presence of EDID to report whether there's a device connected. I'd suggest these remain separate from this caching as it's serving a different purpose. > Still, perhaps a connector flag could be introduced to enable caching > on a per-connector basis, and then we should be able to deal with this > in the DRM core, rather than have per-driver quirks. I agree, and I think a lot of it can be handled entirely within DRM for most cases if we: - arrange for a new connector "modes_invalid" flag which is set initially - arrange for drm_helper_probe_single_connector_modes_merge_bits to test this flag to determine whether it needs to re-read the modes and/or load EDID. - have drm_helper_probe_single_connector_modes_merge_bits clear this flag if caching is enabled and we successfully read the modes from the connector and/or loaded EDID. - set this flag whenever we see the connector transition from a non-connected state to a connected state. I'm trying not to talk myself into writing another patch at the moment... my latency for getting development work into the kernel seems to be getting on for 2 years judging by the state of my dwhdmi audio and CEC patches, though I'm pretty sure that's just because they're fairly low-priority for me. :( -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
