On Fri, 15 Jun 2012 10:34:22 +0300 Jani Nikula <jani.nikula at linux.intel.com> wrote: > On Thu, 14 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. > > > > 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) > > v4: change handling of NULL EDIDs, just preserve the NULL behavior > > across detects and mode list fetches rather than trying to re-fetch > > the EDID (Chris) > > v5: be glad that Chris is around to remind me to hit C-x C-s before > > committing. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=46856 > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 49 ++++++++++++++++++++++++++++++++++----- > > 1 file changed, 43 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 6538c46..69d2f0c 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; > > }; > > > > /** > > @@ -2121,10 +2124,22 @@ 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)) { > > + if (!intel_dp->edid) > > + return NULL; > > + > > + 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; > > } > > > > @@ -2134,9 +2149,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); > > Hi Jesse, I'm sure you meant to do *something* with that return value. I > presume it should be equal to intel_dp->edid_mode_count, but is it > possible it's not? Is it better to return ret or > intel_dp->edid_mode_count from this function? > Heh there's always something... but yeah it should be equal to the mode_count; I should either drop the ret= or use it. Maybe as a cleanup on top since I'm tired of respinning this one? :p -- Jesse Barnes, Intel Open Source Technology Center
