Caching of EDID for X server to decrease startup time of X server

[Sharma, Shashank]

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 at lists.freedesktop.org] On Behalf Of Russell King - ARM Linux
Sent: Monday, August 10, 2015 8:17 PM
To: Thierry Reding
Cc: Paul Menzel; dri-devel at lists.freedesktop.org
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 at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel