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

[Takashi Iwai <tiwai@suse.de>]

At Tue,  5 Jun 2012 16:54:27 -0400,
Jesse Barnes 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.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   43 +++++++++++++++++++++++++++++++++------
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9b2effc..e47ae37 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -32,6 +32,7 @@
>  #include "drm.h"
>  #include "drm_crtc.h"
>  #include "drm_crtc_helper.h"
> +#include "drm_edid.h"
>  #include "intel_drv.h"
>  #include "i915_drm.h"
>  #include "i915_drv.h"
> @@ -67,6 +68,8 @@ struct intel_dp {
>  	struct drm_display_mode *panel_fixed_mode;  /* for eDP */
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
> +	struct edid *edid; /* cached EDID for eDP */
> +	int edid_mode_count;
>  };
>  
>  /**
> @@ -2094,9 +2097,16 @@ intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	struct edid	*edid;
>  
> -	ironlake_edp_panel_vdd_on(intel_dp);
> +	if (is_edp(intel_dp)) {
> +		edid = kmalloc(EDID_LENGTH, GFP_KERNEL);
> +		if (!edid)
> +			return NULL;
> +
> +		memcpy(edid, intel_dp->edid, EDID_LENGTH);
> +		return edid;

It might be bigger than EDID_LENGTH when it contains extensions.
Better to create a helper function like drm_copy_edid()?

Also, there is no kfree() corresponding to intel_dp->edid, so a small
memory leak when unloading the module.  (Ditto for intel_lvds.c, BTW)


Takashi