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

Jesse Barnes
2012-06-14T18:55:49Z

On Thu, 14 Jun 2012 19:37:32 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Thu, 14 Jun 2012 14:30:52 -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)
> >
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=46856
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > ---
> > @@ -2121,10 +2124,19 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> > {
> > struct intel_dp *intel_dp = intel_attached_dp(connector);
> > struct edid *edid;
> > + int size;
> > +
> > + if (is_edp(intel_dp) && intel_dp->edid) {
> > + size = (intel_dp->edid->extensions + 1) * EDID_LENGTH;
> > + edid = kmalloc(size, GFP_KERNEL);
> > + if (!edid)
> > + return NULL;
> > +
> > + memcpy(edid, intel_dp->edid, size);
> > + return edid;
> > + }
> >
> > - ironlake_edp_panel_vdd_on(intel_dp);
> > edid = drm_get_edid(connector, adapter);
> > - ironlake_edp_panel_vdd_off(intel_dp, false);
> > return edid;
>
> This makes me a little uneasy, as the wording here makes it sound like
> it is possible for us to initiate a DP aux xfer without raising VDD on
> the eDP panel. Having made the decision to retreive the EDID for a panel
> only once, we should use that result for all future queries i.e. if it
> was NULL during init, assume it will be NULL now as well. (I hope nobody
> brings out an eDP-to-HDMI dongle!)

Oh good call. Now that I'm checking for whether we have a cached edid
I should leave the aux toggling in...

Thanks,
Jesse

--
Jesse Barnes, Intel Open Source Technology Center