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

Jani Nikula
2012-06-15T10:49:40Z

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