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

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 15 Jun 2012, Chris Wilson <chris@chris-wilson.co.uk> 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.
>
> Jesse's patch revamped. Gotta love those display_info.raw_edid = NULL!
> ---
>  drivers/gpu/drm/i915/intel_dp.c    |   48 +++++++++++++++++++++++++++---------
>  drivers/gpu/drm/i915/intel_drv.h   |    1 +
>  drivers/gpu/drm/i915/intel_modes.c |   24 ++++++++++++------
>  3 files changed, 54 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index eb57ec7..207e25f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -67,6 +67,7 @@ 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 for eDP */
>  };
>  
>  /**
> @@ -2095,26 +2096,50 @@ g4x_dp_detect(struct intel_dp *intel_dp)
>  }
>  
>  static struct edid *
> -intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
> +_drm_edid_duplicate(struct edid *edid)
> +{
> +	struct edid *copy;
> +	int size;
> +
> +	if (edid == NULL)
> +		return NULL;
> +
> +	size = EDID_LENGTH * (1 + edid->extensions);
> +	*copy = kmalloc(size, GFP_KERNEL);

You may want to remove that '*' there...

Jani.

> +	if (copy)
> +		memcpy(copy, edid, size);
> +
> +	return copy;
> +}
> +
> +static struct edid *
> +intel_dp_get_edid(struct drm_connector *connector)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct edid	*edid;
> +	struct edid *edid;
> +
> +	if (is_edp(intel_dp))
> +		return _drm_edid_duplicate(intel_dp->edid);
>  
>  	ironlake_edp_panel_vdd_on(intel_dp);
> -	edid = drm_get_edid(connector, adapter);
> +	edid = drm_get_edid(connector, &intel_dp->adapter);
>  	ironlake_edp_panel_vdd_off(intel_dp, false);
> +
>  	return edid;
>  }
>  
>  static int
> -intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *adapter)
> +intel_dp_get_edid_modes(struct drm_connector *connector)
>  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	int	ret;
> +	struct edid *edid;
> +	int ret;
> +
> +	edid = intel_dp_get_edid(intel_dp);
> +	ret = intel_connector_attach_edid(connector, edid);
> +	connector->display_info.raw_edid = NULL;
> +	kfree(edid);
>  
> -	ironlake_edp_panel_vdd_on(intel_dp);
> -	ret = intel_ddc_get_modes(connector, adapter);
> -	ironlake_edp_panel_vdd_off(intel_dp, false);
>  	return ret;
>  }
>  
> @@ -2172,7 +2197,7 @@ static int intel_dp_get_modes(struct drm_connector *connector)
>  	/* We should parse the EDID data and find out if it has an audio sink
>  	 */
>  
> -	ret = intel_dp_get_edid_modes(connector, &intel_dp->adapter);
> +	ret = intel_dp_get_edid_modes(connector);
>  	if (ret) {
>  		if (is_edp(intel_dp) && !intel_dp->panel_fixed_mode) {
>  			struct drm_display_mode *newmode;
> @@ -2485,6 +2510,8 @@ intel_dp_init(struct drm_device *dev, int output_reg)
>  			break;
>  	}
>  
> +	intel_dp_i2c_init(intel_dp, intel_connector, name);
> +
>  	/* Cache some DPCD data in the eDP case */
>  	if (is_edp(intel_dp)) {
>  		bool ret;
> @@ -2543,6 +2570,7 @@ intel_dp_init(struct drm_device *dev, int output_reg)
>  
>  		ironlake_edp_panel_vdd_on(intel_dp);
>  		ret = intel_dp_get_dpcd(intel_dp);
> +		intel_dp->edid = drm_get_edid(connector, &intel_dp->adapter);
>  		ironlake_edp_panel_vdd_off(intel_dp, false);
>  
>  		if (ret) {
> @@ -2559,8 +2587,6 @@ intel_dp_init(struct drm_device *dev, int output_reg)
>  		}
>  	}
>  
> -	intel_dp_i2c_init(intel_dp, intel_connector, name);
> -
>  	intel_encoder->hot_plug = intel_dp_hot_plug;
>  
>  	if (is_edp(intel_dp)) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3e09188..316413d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -336,6 +336,7 @@ struct intel_fbc_work {
>  
>  int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
>  extern bool intel_ddc_probe(struct intel_encoder *intel_encoder, int ddc_bus);
> +int intel_connector_attach_edid(struct drm_connector *connector, struct edid *edid);
>  
>  extern void intel_attach_force_audio_property(struct drm_connector *connector);
>  extern void intel_attach_broadcast_rgb_property(struct drm_connector *connector);
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index d67ec3a..089ed8b 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -60,6 +60,18 @@ bool intel_ddc_probe(struct intel_encoder *intel_encoder, int ddc_bus)
>  			    msgs, 2) == 2;
>  }
>  
> +int intel_connector_attach_edid(struct drm_connector *connector,
> +				struct edid *edid)
> +{
> +	int ret;
> +
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	ret = drm_add_edid_modes(connector, edid);
> +	drm_edid_to_eld(connector, edid);
> +
> +	return ret;
> +}
> +
>  /**
>   * intel_ddc_get_modes - get modelist from monitor
>   * @connector: DRM connector device to use
> @@ -71,16 +83,12 @@ int intel_ddc_get_modes(struct drm_connector *connector,
>  			struct i2c_adapter *adapter)
>  {
>  	struct edid *edid;
> -	int ret = 0;
> +	int ret;
>  
>  	edid = drm_get_edid(connector, adapter);
> -	if (edid) {
> -		drm_mode_connector_update_edid_property(connector, edid);
> -		ret = drm_add_edid_modes(connector, edid);
> -		drm_edid_to_eld(connector, edid);
> -		connector->display_info.raw_edid = NULL;
> -		kfree(edid);
> -	}
> +	ret = intel_connector_attach_edid(connector, edid);
> +	connector->display_info.raw_edid = NULL;
> +	kfree(edid);
>  
>  	return ret;
>  }
> -- 
> 1.7.10
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx