[PATCH v2 10.1/14] drm/i915: Reset power sequencer pipe tracking when disp2d is off

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

The power sequencer loses its state when the disp2d power well is down.
Clear the dev_priv->pps_pipe tracking so that the power sequencer state
gets reinitialized the next time it's needed.

v2: Fix the pps_mutex vs. power_domain mutex deadlock by taking power
    domain reference first

Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
Imre noticed my previouys attempt was utter garbage. Let's try again. Again maybe
we want to squash with 10/14 or at least move the edp_pps_{lock,unlock}() introduction
there to avoid churn.

The alternative of trying to plumb all the power domain calls out from under
pps_mutex seemed a bit too nasty to me, so I opted for this approach instead.

 drivers/gpu/drm/i915/intel_dp.c  | 157 +++++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h |   1 +
 drivers/gpu/drm/i915/intel_pm.c  |   2 +
 3 files changed, 106 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index d6fe1a2..3468315 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -389,6 +389,67 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp)
 						      &power_seq);
 }
 
+void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = dev_priv->dev;
+	struct intel_encoder *encoder;
+
+	if (WARN_ON(!IS_VALLEYVIEW(dev)))
+		return;
+
+	/*
+	 * We can't grab pps_mutex here due to deadlock with power_domain
+	 * mutex when power_domain functions are called while holding pps_mutex.
+	 * That also means that in order to use pps_pipe the code needs to
+	 * hold both a power domain reference and pps_mutex, and the power domain
+	 * reference get/put must be done while _not_ holding pps_mutex.
+	 * edp_pps_{lock,unlock}() do these steps in the correct order, so one
+	 * should use them always.
+	 */
+
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
+		struct intel_dp *intel_dp;
+
+		if (encoder->type != INTEL_OUTPUT_EDP)
+			continue;
+
+		intel_dp = enc_to_intel_dp(&encoder->base);
+		intel_dp->pps_pipe = INVALID_PIPE;
+	}
+}
+
+static void edp_pps_lock(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *encoder = &intel_dig_port->base;
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum intel_display_power_domain power_domain;
+
+	/*
+	 * See vlv_power_sequencer_reset() why we need
+	 * a power domain reference here.
+	 */
+	power_domain = intel_display_port_power_domain(encoder);
+	intel_display_power_get(dev_priv, power_domain);
+
+	mutex_lock(&dev_priv->pps_mutex);
+}
+
+static void edp_pps_unlock(struct intel_dp *intel_dp)
+{
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct intel_encoder *encoder = &intel_dig_port->base;
+	struct drm_device *dev = encoder->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum intel_display_power_domain power_domain;
+
+	mutex_unlock(&dev_priv->pps_mutex);
+
+	power_domain = intel_display_port_power_domain(encoder);
+	intel_display_power_put(dev_priv, power_domain);
+}
+
 static u32 _pp_ctrl_reg(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -425,7 +486,7 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
 	if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART)
 		return 0;
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 
 	pipe = vlv_power_sequencer_pipe(intel_dp);
 
@@ -439,7 +500,7 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code,
 	I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
 	msleep(intel_dp->panel_power_cycle_delay);
 
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 
 	return 0;
 }
@@ -458,17 +519,13 @@ static bool edp_have_panel_vdd(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
-	enum intel_display_power_domain power_domain;
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	return intel_display_power_enabled(dev_priv, power_domain) &&
-	       (I915_READ(_pp_ctrl_reg(intel_dp)) & EDP_FORCE_VDD) != 0;
+	return I915_READ(_pp_ctrl_reg(intel_dp)) & EDP_FORCE_VDD;
 }
 
+
 static void
 intel_dp_check_edp(struct intel_dp *intel_dp)
 {
@@ -614,7 +671,7 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	bool has_aux_irq = HAS_AUX_IRQ(dev);
 	bool vdd;
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 
 	/*
 	 * We will be called with VDD already enabled for dpcd/edid/oui reads.
@@ -731,7 +788,7 @@ out:
 	if (vdd)
 		edp_panel_vdd_off(intel_dp, false);
 
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 
 	return ret;
 }
@@ -1312,16 +1369,14 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
 
 void intel_edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
-	struct drm_i915_private *dev_priv =
-		intel_dp_to_dev(intel_dp)->dev_private;
 	bool vdd;
 
 	if (!is_edp(intel_dp))
 		return;
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 	vdd = edp_panel_vdd_on(intel_dp);
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 
 	WARN(!vdd, "eDP VDD already requested on\n");
 }
@@ -1370,13 +1425,11 @@ static void edp_panel_vdd_work(struct work_struct *__work)
 {
 	struct intel_dp *intel_dp = container_of(to_delayed_work(__work),
 						 struct intel_dp, panel_vdd_work);
-	struct drm_i915_private *dev_priv =
-		intel_dp_to_dev(intel_dp)->dev_private;
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 	if (!intel_dp->want_panel_vdd)
 		edp_panel_vdd_off_sync(intel_dp);
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 }
 
 static void edp_panel_vdd_schedule_off(struct intel_dp *intel_dp)
@@ -1414,15 +1467,12 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
 
 static void intel_edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync)
 {
-	struct drm_i915_private *dev_priv =
-		intel_dp_to_dev(intel_dp)->dev_private;
-
 	if (!is_edp(intel_dp))
 		return;
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 	edp_panel_vdd_off(intel_dp, sync);
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 }
 
 void intel_edp_panel_on(struct intel_dp *intel_dp)
@@ -1437,7 +1487,7 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
 
 	DRM_DEBUG_KMS("Turn eDP power on\n");
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 
 	if (edp_have_panel_power(intel_dp)) {
 		DRM_DEBUG_KMS("eDP power already on\n");
@@ -1472,7 +1522,7 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
 	}
 
  out:
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 }
 
 void intel_edp_panel_off(struct intel_dp *intel_dp)
@@ -1490,7 +1540,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
 
 	DRM_DEBUG_KMS("Turn eDP power off\n");
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 
 	WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
 
@@ -1514,7 +1564,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp)
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain);
 
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 }
 
 void intel_edp_backlight_on(struct intel_dp *intel_dp)
@@ -1540,7 +1590,7 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
 	 */
 	wait_backlight_on(intel_dp);
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 
 	pp = ironlake_get_pp_control(intel_dp);
 	pp |= EDP_BLC_ENABLE;
@@ -1550,7 +1600,7 @@ void intel_edp_backlight_on(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 }
 
 void intel_edp_backlight_off(struct intel_dp *intel_dp)
@@ -1563,7 +1613,7 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
 	if (!is_edp(intel_dp))
 		return;
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 
 	DRM_DEBUG_KMS("\n");
 	pp = ironlake_get_pp_control(intel_dp);
@@ -1574,7 +1624,7 @@ void intel_edp_backlight_off(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 
 	intel_dp->last_backlight_off = jiffies;
 	edp_wait_backlight_off(intel_dp);
@@ -2290,18 +2340,19 @@ static void vlv_steal_power_sequencer(struct drm_device *dev,
 
 	list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) {
 		struct intel_dp *intel_dp;
+		enum port port;
 
 		if (encoder->type != INTEL_OUTPUT_EDP)
 			continue;
 
 		intel_dp = enc_to_intel_dp(&encoder->base);
+		port = dp_to_dig_port(intel_dp)->port;
 
 		if (intel_dp->pps_pipe != pipe)
 			continue;
 
 		DRM_DEBUG_KMS("stealing pipe %c power sequencer from port %c\n",
-			      pipe_name(pipe),
-			      port_name(dp_to_dig_port(intel_dp)->port));
+			      pipe_name(pipe), port_name(port));
 
 		/* make sure vdd is off before we steal it */
 		edp_panel_vdd_off_sync(intel_dp);
@@ -2377,9 +2428,9 @@ static void vlv_pre_enable_dp(struct intel_encoder *encoder)
 	mutex_unlock(&dev_priv->dpio_lock);
 
 	if (is_edp(intel_dp)) {
-		mutex_lock(&dev_priv->pps_mutex);
+		edp_pps_lock(intel_dp);
 		vlv_init_panel_power_sequencer(intel_dp);
-		mutex_unlock(&dev_priv->pps_mutex);
+		edp_pps_unlock(intel_dp);
 	}
 
 	intel_enable_dp(encoder);
@@ -2477,9 +2528,9 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder)
 	mutex_unlock(&dev_priv->dpio_lock);
 
 	if (is_edp(intel_dp)) {
-		mutex_lock(&dev_priv->pps_mutex);
+		edp_pps_lock(intel_dp);
 		vlv_init_panel_power_sequencer(intel_dp);
-		mutex_unlock(&dev_priv->pps_mutex);
+		edp_pps_unlock(intel_dp);
 	}
 
 	intel_enable_dp(encoder);
@@ -4219,17 +4270,16 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	drm_dp_aux_unregister(&intel_dp->aux);
 	intel_dp_mst_encoder_cleanup(intel_dig_port);
 	drm_encoder_cleanup(encoder);
 	if (is_edp(intel_dp)) {
 		cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
-		mutex_lock(&dev_priv->pps_mutex);
+		edp_pps_lock(intel_dp);
 		edp_panel_vdd_off_sync(intel_dp);
-		mutex_unlock(&dev_priv->pps_mutex);
+		edp_pps_unlock(intel_dp);
+
 		if (intel_dp->edp_notifier.notifier_call) {
 			unregister_reboot_notifier(&intel_dp->edp_notifier);
 			intel_dp->edp_notifier.notifier_call = NULL;
@@ -4241,15 +4291,13 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 static void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = dev->dev_private;
 
 	if (!is_edp(intel_dp))
 		return;
 
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 	edp_panel_vdd_off_sync(intel_dp);
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 }
 
 static void intel_dp_encoder_reset(struct drm_encoder *encoder)
@@ -4727,9 +4775,10 @@ void intel_edp_panel_vdd_sanitize(struct intel_encoder *intel_encoder)
 	if (intel_encoder->type != INTEL_OUTPUT_EDP)
 		return;
 
-	mutex_lock(&dev_priv->pps_mutex);
-
 	intel_dp = enc_to_intel_dp(&intel_encoder->base);
+
+	edp_pps_lock(intel_dp);
+
 	if (!edp_have_panel_vdd(intel_dp))
 		goto out;
 	/*
@@ -4744,7 +4793,7 @@ void intel_edp_panel_vdd_sanitize(struct intel_encoder *intel_encoder)
 
 	edp_panel_vdd_schedule_off(intel_dp);
  out:
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 }
 
 static bool intel_edp_init_connector(struct intel_dp *intel_dp,
@@ -4786,9 +4835,9 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	}
 
 	/* We now know it's not a ghost, init power sequence regs. */
-	mutex_lock(&dev_priv->pps_mutex);
+	edp_pps_lock(intel_dp);
 	intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq);
-	mutex_unlock(&dev_priv->pps_mutex);
+	edp_pps_unlock(intel_dp);
 
 	mutex_lock(&dev->mode_config.mutex);
 	edid = drm_get_edid(connector, &intel_dp->aux.ddc);
@@ -4922,14 +4971,14 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	}
 
 	if (is_edp(intel_dp)) {
-		mutex_lock(&dev_priv->pps_mutex);
+		edp_pps_lock(intel_dp);
 		if (IS_VALLEYVIEW(dev)) {
 			vlv_initial_power_sequencer_setup(intel_dp);
 		} else {
 			intel_dp_init_panel_power_timestamps(intel_dp);
 			intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
 		}
-		mutex_unlock(&dev_priv->pps_mutex);
+		edp_pps_unlock(intel_dp);
 	}
 
 	intel_dp_aux_init(intel_dp, intel_connector);
@@ -4945,9 +4994,9 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 		drm_dp_aux_unregister(&intel_dp->aux);
 		if (is_edp(intel_dp)) {
 			cancel_delayed_work_sync(&intel_dp->panel_vdd_work);
-			mutex_lock(&dev_priv->pps_mutex);
+			edp_pps_lock(intel_dp);
 			edp_panel_vdd_off_sync(intel_dp);
-			mutex_unlock(&dev_priv->pps_mutex);
+			edp_pps_unlock(intel_dp);
 		}
 		drm_connector_unregister(connector);
 		drm_connector_cleanup(connector);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 849a447..32627ae 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -943,6 +943,7 @@ void intel_dp_mst_suspend(struct drm_device *dev);
 void intel_dp_mst_resume(struct drm_device *dev);
 int intel_dp_max_link_bw(struct intel_dp *intel_dp);
 void intel_dp_hot_plug(struct intel_encoder *intel_encoder);
+void vlv_power_sequencer_reset(struct drm_i915_private *dev_priv);
 /* intel_dp_mst.c */
 int intel_dp_mst_encoder_init(struct intel_digital_port *intel_dig_port, int conn_id);
 void intel_dp_mst_encoder_cleanup(struct intel_digital_port *intel_dig_port);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 39dd066..d5ffda9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6328,6 +6328,8 @@ static void vlv_display_power_well_disable(struct drm_i915_private *dev_priv,
 	spin_unlock_irq(&dev_priv->irq_lock);
 
 	vlv_set_power_well(dev_priv, power_well, false);
+
+	vlv_power_sequencer_reset(dev_priv);
 }
 
 static void vlv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv,
-- 
1.8.5.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux