On Thu, 14 Jun 2012 20:42:15 +0100 Chris Wilson <chris at chris-wilson.co.uk> wrote: > On Thu, 14 Jun 2012 15:28:33 -0400, Jesse Barnes <jbarnes at virtuousgeek.org> wrote: > > They aren't going anywhere, and probing on DDC can cause the panel to > > blank briefly, so read them up front and cache them for later queries. > > > > v2: fix potential NULL derefs in intel_dp_get_edid_modes and > > intel_dp_get_edid (Jani) > > copy full EDID length, including extension blocks (Takashi) > > free EDID on teardown (Takashi) > > v3: malloc a new EDID buffer that's big enough for the memcpy (Chris) > > v4: change handling of NULL EDIDs, just preserve the NULL behavior > > across detects and mode list fetches rather than trying to re-fetch > > the EDID (Chris) > > v5: be glad that Chris is around to remind me to hit C-x C-s before > > committing. > > I do 'git add' reminders as well; by appointment only. > > And now to really pour rain on your fire. Why are we repeating quite > obscure logic from intel_modes.c? Can we not export a function to attach > an edid to a connector and then refactor intel_dp.c to only have a > single edid reader where you could add your magic eDP EDID cache > handling in single place. > > So intel_dp_get_edid_modes calls intel_dp_get_edid() and > intel_connector_attach_edid() rather the single-shot > intel_ddc_get_modes(). I totally agree this can be more generic. However, Daniel wanted something for -fixes that's fairly self-contained. I think we should push this for the current kernel (and potentially stable) since it fixes a bug, and your stuff or Daniel's stuff can go into -next that makes every connector better. -- Jesse Barnes, Intel Open Source Technology Center