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

[Chris Wilson <chris@chris-wilson.co.uk>]

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!)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre