Re: [PATCH] drm/i915: cache the EDID for eDP panels

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 14 Jun 2012 20:42:15 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Thu, 14 Jun 2012 15:28:33 -0400, Jesse Barnes <jbarnes@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