From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 10/10] drm/i915: Move cached EDID to intel_connector Date: Fri, 19 Oct 2012 10:13:06 -0700 Message-ID: <20121019101306.76df5c12@jbarnes-desktop> References: <26034923ee6f60e29220b662208a7211665fafed.1350644022.git.jani.nikula@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy12-pub.bluehost.com (oproxy12-pub.bluehost.com [50.87.16.10]) by gabe.freedesktop.org (Postfix) with SMTP id 18E3D9EEEB for ; Fri, 19 Oct 2012 10:12:13 -0700 (PDT) In-Reply-To: <26034923ee6f60e29220b662208a7211665fafed.1350644022.git.jani.nikula@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, 19 Oct 2012 14:51:52 +0300 Jani Nikula wrote: > Move the cached EDID from intel_dp and intel_lvds_connector to > intel_connector. Unify cached EDID handling for LVDS and eDP, in > preparation for adding more generic EDID caching later. > > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_dp.c | 61 +++++++++++++++++++++---------------- > drivers/gpu/drm/i915/intel_drv.h | 5 +-- > drivers/gpu/drm/i915/intel_lvds.c | 34 ++++++++++++++------- > 3 files changed, 60 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 8c72477..5b0e4cd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2323,44 +2323,45 @@ g4x_dp_detect(struct intel_dp *intel_dp) > static struct edid * > 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; > + struct intel_connector *intel_connector = to_intel_connector(connector); > > - if (is_edp(intel_dp)) { > - if (!intel_dp->edid) > + /* use cached edid if we have one */ > + if (intel_connector->edid) { > + struct edid *edid; > + int size; > + > + /* invalid edid */ > + if (IS_ERR(intel_connector->edid)) > return NULL; > > - size = (intel_dp->edid->extensions + 1) * EDID_LENGTH; > + size = (intel_connector->edid->extensions + 1) * EDID_LENGTH; > edid = kmalloc(size, GFP_KERNEL); > if (!edid) > return NULL; > > - memcpy(edid, intel_dp->edid, size); > + memcpy(edid, intel_connector->edid, size); > return edid; > } > > - edid = drm_get_edid(connector, adapter); > - return edid; > + return drm_get_edid(connector, adapter); > } > > static int > intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *adapter) > { > - struct intel_dp *intel_dp = intel_attached_dp(connector); > - int ret; > + struct intel_connector *intel_connector = to_intel_connector(connector); > > - 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); > - return intel_dp->edid_mode_count; > + /* use cached edid if we have one */ > + if (intel_connector->edid) { > + /* invalid edid */ > + if (IS_ERR(intel_connector->edid)) > + return 0; > + > + return intel_connector_update_modes(connector, > + intel_connector->edid); > } > > - ret = intel_ddc_get_modes(connector, adapter); > - return ret; > + return intel_ddc_get_modes(connector, adapter); > } > > > @@ -2511,6 +2512,9 @@ intel_dp_destroy(struct drm_connector *connector) > struct drm_device *dev = connector->dev; > struct intel_connector *intel_connector = to_intel_connector(connector); > > + if (!IS_ERR_OR_NULL(intel_connector->edid)) > + kfree(intel_connector->edid); > + > if (intel_dpd_is_edp(dev)) { > intel_panel_destroy_backlight(dev); > intel_panel_fini(&intel_connector->panel); > @@ -2528,7 +2532,6 @@ static void intel_dp_encoder_destroy(struct drm_encoder *encoder) > i2c_del_adapter(&intel_dp->adapter); > drm_encoder_cleanup(encoder); > if (is_edp(intel_dp)) { > - kfree(intel_dp->edid); > cancel_delayed_work_sync(&intel_dp->panel_vdd_work); > ironlake_panel_vdd_off_sync(intel_dp); > } > @@ -2815,13 +2818,17 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port) > ironlake_edp_panel_vdd_on(intel_dp); > edid = drm_get_edid(connector, &intel_dp->adapter); > if (edid) { > - 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; > + if (drm_add_edid_modes(connector, edid)) { > + drm_mode_connector_update_edid_property(connector, edid); > + drm_edid_to_eld(connector, edid); > + } else { > + kfree(edid); > + edid = ERR_PTR(-EINVAL); > + } > + } else { > + edid = ERR_PTR(-ENOENT); > } > + intel_connector->edid = edid; > > /* prefer fixed mode from EDID if available */ > list_for_each_entry(scan, &connector->probed_modes, head) { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index ae0467a..5fa0af8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -186,6 +186,9 @@ struct intel_connector { > > /* Panel info for eDP and LVDS */ > struct intel_panel panel; > + > + /* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */ > + struct edid *edid; > }; > > struct intel_crtc { > @@ -367,8 +370,6 @@ struct intel_dp { > int backlight_off_delay; > struct delayed_work panel_vdd_work; > bool want_panel_vdd; > - struct edid *edid; /* cached EDID for eDP */ > - int edid_mode_count; > struct intel_connector *attached_connector; > }; > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index ced6947..efa62ac 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -45,7 +45,6 @@ struct intel_lvds_connector { > struct intel_connector base; > > struct notifier_block lid_notifier; > - struct edid *edid; > int fitting_mode; > }; > > @@ -461,8 +460,14 @@ static int intel_lvds_get_modes(struct drm_connector *connector) > struct drm_device *dev = connector->dev; > struct drm_display_mode *mode; > > - if (lvds_connector->edid) > - return drm_add_edid_modes(connector, lvds_connector->edid); > + /* use cached edid if we have one */ > + if (lvds_connector->base.edid) { > + /* invalid edid */ > + if (IS_ERR(lvds_connector->base.edid)) > + return 0; > + > + return drm_add_edid_modes(connector, lvds_connector->base.edid); > + } > > mode = drm_mode_duplicate(dev, lvds_connector->base.panel.fixed_mode); > if (mode == NULL) > @@ -554,6 +559,9 @@ static void intel_lvds_destroy(struct drm_connector *connector) > if (lvds_connector->lid_notifier.notifier_call) > acpi_lid_notifier_unregister(&lvds_connector->lid_notifier); > > + if (!IS_ERR_OR_NULL(lvds_connector->base.edid)) > + kfree(lvds_connector->base.edid); > + > intel_panel_destroy_backlight(connector->dev); > intel_panel_fini(&lvds_connector->base.panel); > > @@ -923,6 +931,7 @@ bool intel_lvds_init(struct drm_device *dev) > struct drm_encoder *encoder; > struct drm_display_mode *scan; /* *modes, *bios_mode; */ > struct drm_display_mode *fixed_mode = NULL; > + struct edid *edid; > struct drm_crtc *crtc; > u32 lvds; > int pipe; > @@ -1022,18 +1031,21 @@ bool intel_lvds_init(struct drm_device *dev) > * Attempt to get the fixed panel mode from DDC. Assume that the > * preferred mode is the right one. > */ > - lvds_connector->edid = drm_get_edid(connector, > - intel_gmbus_get_adapter(dev_priv, pin)); > - if (lvds_connector->edid) { > - if (drm_add_edid_modes(connector, lvds_connector->edid)) { > + edid = drm_get_edid(connector, intel_gmbus_get_adapter(dev_priv, pin)); > + if (edid) { > + if (drm_add_edid_modes(connector, edid)) { > drm_mode_connector_update_edid_property(connector, > - lvds_connector->edid); > + edid); > } else { > - kfree(lvds_connector->edid); > - lvds_connector->edid = NULL; > + kfree(edid); > + edid = ERR_PTR(-EINVAL); > } > + } else { > + edid = ERR_PTR(-ENOENT); > } > - if (!lvds_connector->edid) { > + lvds_connector->base.edid = edid; > + > + if (IS_ERR_OR_NULL(edid)) { > /* Didn't get an EDID, so > * Set wide sync ranges so we get all modes > * handed to valid_mode for checking Reviewed-by: Jesse Barnes Looks like 6-10 should also just go upstream, looks like good stuff. -- Jesse Barnes, Intel Open Source Technology Center