[Intel-gfx] [PATCH] drm/i915: propagate errors from intel_dp_init_connector

Paulo Zanoni przanoni at gmail.com
Mon Jun 10 19:19:45 CEST 2013


From: Paulo Zanoni <paulo.r.zanoni at intel.com>

In case we detect a "ghost eDP", intel_dp_init_connector was freeing
both the connector and encoder and then returning. On Haswell,
intel_ddi_init was trying to use the freed encoder on the HDMI
initialization path since the following commit:

commit 21a8e6a4853b2ed39fa4c5188a710f2cf1b92026
Author: Daniel Vetter <daniel.vetter at ffwll.ch>
Date:   Wed Apr 10 23:28:35 2013 +0200
    drm/i915: don't setup hdmi for port D edp in ddi_init

So on this patch we change intel_dp_init_connector from void to bool,
check its return value on its callers and leave the resource-freeing
task to its callers (since the callers are the ones who allocate the
resources). With this change it will be easier to see that
intel_dp_init_connector can fail, so I hope we won't have similar
regressions in the future.

Also include a small change to remove extra "{" and "}" on nearby
code.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 10 +++++++---
 drivers/gpu/drm/i915/intel_dp.c  | 15 +++++++++------
 drivers/gpu/drm/i915/intel_drv.h |  3 ++-
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 224ce25..4e0cf37 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1356,14 +1356,18 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
 	intel_encoder->cloneable = false;
 	intel_encoder->hot_plug = intel_ddi_hot_plug;
 
-	intel_dp_init_connector(intel_dig_port, dp_connector);
+	/* May fail if it's an eDP connector that's disconnected. */
+	if (!intel_dp_init_connector(intel_dig_port, dp_connector)) {
+		intel_ddi_destroy(encoder);
+		intel_dp_destroy(&dp_connector->base);
+		return;
+	}
 
 	if (intel_encoder->type != INTEL_OUTPUT_EDP) {
 		hdmi_connector = kzalloc(sizeof(struct intel_connector),
 					 GFP_KERNEL);
-		if (!hdmi_connector) {
+		if (!hdmi_connector)
 			return;
-		}
 
 		intel_dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
 		intel_hdmi_init_connector(intel_dig_port, hdmi_connector);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0de82bb..ec23b3d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2687,7 +2687,7 @@ done:
 	return 0;
 }
 
-static void
+void
 intel_dp_destroy(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
@@ -2961,7 +2961,7 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev,
 		      I915_READ(pp_div_reg));
 }
 
-void
+bool
 intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 			struct intel_connector *intel_connector)
 {
@@ -3095,9 +3095,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		} else {
 			/* if this fails, presume the device is a ghost */
 			DRM_INFO("failed to retrieve link info, disabling eDP\n");
-			intel_dp_encoder_destroy(&intel_encoder->base);
-			intel_dp_destroy(connector);
-			return;
+			return false;
 		}
 
 		/* We now know it's not a ghost, init power sequence regs. */
@@ -3152,6 +3150,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		u32 temp = I915_READ(PEG_BAND_GAP_DATA);
 		I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
 	}
+
+	return true;
 }
 
 void
@@ -3197,5 +3197,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder->cloneable = false;
 	intel_encoder->hot_plug = intel_dp_hot_plug;
 
-	intel_dp_init_connector(intel_dig_port, intel_connector);
+	if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
+		intel_dp_encoder_destroy(encoder);
+		intel_dp_destroy(&intel_connector->base);
+	}
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b9e250c..8a7ad99 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -569,9 +569,10 @@ extern bool intel_lvds_init(struct drm_device *dev);
 extern bool intel_is_dual_link_lvds(struct drm_device *dev);
 extern void intel_dp_init(struct drm_device *dev, int output_reg,
 			  enum port port);
-extern void intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
+extern bool intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 				    struct intel_connector *intel_connector);
 extern void intel_dp_init_link_config(struct intel_dp *intel_dp);
+extern void intel_dp_destroy(struct drm_connector *connector);
 extern void intel_dp_start_link_train(struct intel_dp *intel_dp);
 extern void intel_dp_complete_link_train(struct intel_dp *intel_dp);
 extern void intel_dp_stop_link_train(struct intel_dp *intel_dp);
-- 
1.8.1.2




More information about the Intel-gfx mailing list