Hi Jesse, please find a couple of comments below.
BR,
Jani.
On Tue, 05 Jun 2012, Jesse Barnes <jbarnes at 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.
>
> Signed-off-by: Jesse Barnes <jbarnes at 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);
I suppose it may be justified to use EDID_LENGTH rather than
sizeof(*edid) here, but would it be too paranoid to stick this
somewhere?
BUILD_BUG_ON(sizeof(struct edid) != EDID_LENGTH);
> + return edid;
> + }
> +
> edid = drm_get_edid(connector, adapter);
> - ironlake_edp_panel_vdd_off(intel_dp, false);
> return edid;
> }
>
> @@ -2106,9 +2116,17 @@ intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *ada
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> int ret;
>
> - ironlake_edp_panel_vdd_on(intel_dp);
> + if (is_edp(intel_dp)) {
> + drm_mode_connector_update_edid_property(connector,
> + intel_dp->edid);
> + ret = drm_add_edid_modes(connector, intel_dp->edid);
> + drm_edid_to_eld(connector,
> + intel_dp->edid);
> + connector->display_info.raw_edid = NULL;
> + return intel_dp->edid_mode_count;
> + }
> +
> ret = intel_ddc_get_modes(connector, adapter);
> - ironlake_edp_panel_vdd_off(intel_dp, false);
> return ret;
> }
>
> @@ -2479,11 +2497,14 @@ 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;
> struct edp_power_seq cur, vbt;
> u32 pp_on, pp_off, pp_div;
> + struct edid *edid;
>
> pp_on = I915_READ(PCH_PP_ON_DELAYS);
> pp_off = I915_READ(PCH_PP_OFF_DELAYS);
> @@ -2551,9 +2572,19 @@ intel_dp_init(struct drm_device *dev, int output_reg)
> intel_dp_destroy(&intel_connector->base);
> return;
> }
> - }
>
> - intel_dp_i2c_init(intel_dp, intel_connector, name);
> + ironlake_edp_panel_vdd_on(intel_dp);
> + edid = drm_get_edid(connector, &intel_dp->adapter);
> + if (edid) {
This if condition makes me believe it's possible edid == NULL, but if
that happens, I think the additions to intel_dp_get_edid() and
intel_dp_get_edid_modes() would oops.
> + drm_mode_connector_update_edid_property(connector,
> + edid);
> + intel_dp->edid_mode_count =
> + drm_add_edid_modes(connector, edid);
> + drm_edid_to_eld(connector, edid);
> + intel_dp->edid = edid;
> + }
> + ironlake_edp_panel_vdd_off(intel_dp, false);
> + }
>
> intel_encoder->hot_plug = intel_dp_hot_plug;
>
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
