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