[10/10] drm/i915: Move cached EDID to intel_connector

Commit Message

Jani Nikula Oct. 19, 2012, 11:51 a.m. UTC

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 <[e-mail]>
---
 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(-)

Comments

Jesse Barnes Oct. 19, 2012, 5:13 p.m. UTC | #1

On Fri, 19 Oct 2012 14:51:52 +0300
Jani Nikula <[e-mail]> 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 <[e-mail]>
> ---
>  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 <[e-mail]>

Looks like 6-10 should also just go upstream, looks like good stuff.

Patch

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