[PATCH] drm/i915: Shut down displays gracefully on reboot

* [PATCH] drm/i915: Shut down displays gracefully on reboot
@ 2016-08-03 13:36 ville.syrjala
  2016-08-03 13:59 ` ✗ Ro.CI.BAT: failure for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: ville.syrjala @ 2016-08-03 13:36 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Dell UP2414Q doesn't like it when we're driving it in MST mode, and then
reboot the machine. After reboot, the monitor is somehow confused and
refuses to do the payload allocation:
 [drm:drm_dp_mst_allocate_vcpi] initing vcpi for 282 8
 [drm:drm_dp_dpcd_write_payload] status not set after read payload table status 0
and thus we're left with a black screen until the monitor power is
toggled.

It seems that if we shut down things gracefully prior to rebooting, the
monitor doesn't get confused like this. So let's try to shut down all
the displays gracefully on reboot. The downside is that we will
introduce the reboot notifier to all the modesetl locks. So if there's
a locking bug around, we might not be able to reboot the machine
gracefully. sysrq reboot will still work though, as it will not
call the notifiers.

While we do this, we can eliminate the eDP reboot notifier, which is
there to shut down the panel power and make sure the firmware won't
violate the panel power cycle delay post-reboot. Since we're now
shutting eDP down properly, we can mostly just rip out the eDP notifier.
We still need to do the panel power cycle wait though, as that would
normally get postponed until the next modeset. So let's move that part
into intel_panel so that other display types will get the same treatment
as a bonus.

The Dell UP2414Q can often get even more confused, and sometimes what
you have to do is: switch to another input on the monitor, toggle the
monitor power, switch the input back to the original setting. And
sometimes it seems you just have to yank the power cable entirely. I'm
not sure if this reboot notifier might avoid some of these other
failure modes as well, but I'm pretty sure it can't hurt at least.

Cc: Clint Taylor <clinton.a.taylor@intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  2 ++
 drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      | 49 ++----------------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  7 +++---
 drivers/gpu/drm/i915/intel_dsi.c     |  3 ++-
 drivers/gpu/drm/i915/intel_dvo.c     |  2 +-
 drivers/gpu/drm/i915/intel_lvds.c    |  3 ++-
 drivers/gpu/drm/i915/intel_panel.c   | 29 ++++++++++++++++++++-
 8 files changed, 65 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 65ada5d2f88c..f8eb8c7de007 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1727,6 +1727,8 @@ struct intel_wm_config {
 struct drm_i915_private {
 	struct drm_device drm;
 
+	struct notifier_block reboot_notifier;
+
 	struct kmem_cache *objects;
 	struct kmem_cache *vmas;
 	struct kmem_cache *requests;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a8e8cc8dfae9..2192a21aa6c9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -31,6 +31,8 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/vgaarb.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
 #include <drm/drm_edid.h>
 #include <drm/drmP.h>
 #include "intel_drv.h"
@@ -15578,6 +15580,23 @@ fail:
 	drm_modeset_acquire_fini(&ctx);
 }
 
+
+static int intel_reboot_notifier(struct notifier_block *nb,
+				 unsigned long code, void *unused)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(nb, typeof(*dev_priv), reboot_notifier);
+
+	if (code != SYS_RESTART)
+		return 0;
+
+	intel_display_suspend(&dev_priv->drm);
+
+	intel_panel_reboot_notifier(dev_priv);
+
+	return 0;
+}
+
 void intel_modeset_init(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -15706,6 +15725,9 @@ void intel_modeset_init(struct drm_device *dev)
 	 * since the watermark calculation done here will use pstate->fb.
 	 */
 	sanitize_watermarks(dev);
+
+	dev_priv->reboot_notifier.notifier_call = intel_reboot_notifier;
+	register_reboot_notifier(&dev_priv->reboot_notifier);
 }
 
 static void intel_enable_pipe_a(struct drm_device *dev)
@@ -16291,6 +16313,8 @@ void intel_modeset_cleanup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
+	unregister_reboot_notifier(&dev_priv->reboot_notifier);
+
 	intel_disable_gt_powersave(dev_priv);
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 001f74fc0ce5..d8d13a9e33e5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,8 +28,6 @@
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/export.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
@@ -631,42 +629,6 @@ _pp_stat_reg(struct intel_dp *intel_dp)
 	return regs.pp_stat;
 }
 
-/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
-   This function only applicable when panel PM state is not to be tracked */
-static int edp_notify_handler(struct notifier_block *this, unsigned long code,
-			      void *unused)
-{
-	struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
-						 edp_notifier);
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	struct drm_i915_private *dev_priv = to_i915(dev);
-
-	if (!is_edp(intel_dp) || code != SYS_RESTART)
-		return 0;
-
-	pps_lock(intel_dp);
-
-	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-		enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
-		i915_reg_t pp_ctrl_reg, pp_div_reg;
-		u32 pp_div;
-
-		pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
-		pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
-		pp_div = I915_READ(pp_div_reg);
-		pp_div &= PP_REFERENCE_DIVIDER_MASK;
-
-		/* 0x1F write to PP_DIV_REG sets max cycle delay */
-		I915_WRITE(pp_div_reg, pp_div | 0x1F);
-		I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
-		msleep(intel_dp->panel_power_cycle_delay);
-	}
-
-	pps_unlock(intel_dp);
-
-	return 0;
-}
-
 static bool edp_have_panel_power(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -4562,11 +4524,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 		pps_lock(intel_dp);
 		edp_panel_vdd_off_sync(intel_dp);
 		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;
-		}
 	}
 
 	intel_dp_aux_fini(intel_dp);
@@ -5465,9 +5422,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	mutex_unlock(&dev->mode_config.mutex);
 
 	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-		intel_dp->edp_notifier.notifier_call = edp_notify_handler;
-		register_reboot_notifier(&intel_dp->edp_notifier);
-
 		/*
 		 * Figure out the current pipe for the initial backlight setup.
 		 * If the current pipe isn't valid, try the PPS pipe, and if that
@@ -5488,7 +5442,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 			      pipe_name(pipe));
 	}
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode,
+			 intel_dp->panel_power_cycle_delay);
 	intel_connector->panel.backlight.power = intel_edp_backlight_power;
 	intel_panel_setup_backlight(connector, pipe);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 50cdc890d956..92fa66a02ca5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -226,6 +226,7 @@ struct intel_panel {
 	struct drm_display_mode *fixed_mode;
 	struct drm_display_mode *downclock_mode;
 	int fitting_mode;
+	int reboot_power_cycle_delay;
 
 	/* backlight */
 	struct {
@@ -877,8 +878,6 @@ struct intel_dp {
 	unsigned long last_backlight_off;
 	ktime_t panel_power_off_time;
 
-	struct notifier_block edp_notifier;
-
 	/*
 	 * Pipe whose power sequencer is currently locked into
 	 * this port. Only relevant on VLV/CHV.
@@ -1521,8 +1520,10 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv);
 /* intel_panel.c */
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode,
-		     struct drm_display_mode *downclock_mode);
+		     struct drm_display_mode *downclock_mode,
+		     int reboot_power_cycle_delay);
 void intel_panel_fini(struct intel_panel *panel);
+void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv);
 void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode,
 			    struct drm_display_mode *adjusted_mode);
 void intel_pch_panel_fitting(struct intel_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index de8e9fb51595..5b722ec23381 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1596,7 +1596,8 @@ void intel_dsi_init(struct drm_device *dev)
 	connector->display_info.width_mm = fixed_mode->width_mm;
 	connector->display_info.height_mm = fixed_mode->height_mm;
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, NULL);
+	intel_panel_init(&intel_connector->panel, fixed_mode, NULL,
+			 intel_dsi->panel_pwr_cycle_delay);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	intel_dsi_add_properties(intel_connector);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 47bdf9dad0d3..c2c9c922590c 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -547,7 +547,7 @@ void intel_dvo_init(struct drm_device *dev)
 			 */
 			intel_panel_init(&intel_connector->panel,
 					 intel_dvo_get_current_mode(connector),
-					 NULL);
+					 NULL, 0);
 			intel_dvo->panel_wants_dither = true;
 		}
 
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 49550470483e..07d7ac2c1520 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -1120,7 +1120,8 @@ void intel_lvds_init(struct drm_device *dev)
 out:
 	mutex_unlock(&dev->mode_config.mutex);
 
-	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
+	/* FIXME fill in an accurate power cycle delay */
+	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0);
 	intel_panel_setup_backlight(connector, INVALID_PIPE);
 
 	lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 96c65d77e886..fe57a743ad21 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1781,14 +1781,41 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
 	}
 }
 
+/*
+ * Make sure the power cycle delay is respected. The PPS
+ * does supposedly initiate a power cycle on reset, but it
+ * also resets the power cycle delay register value to the
+ * default value, and hence may not wait long enough if the
+ * firmware attempts to power up the panel immediately.
+ * Also eg. DSI doesn't use the PPS.
+ */
+void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = &dev_priv->drm;
+	struct intel_connector *connector;
+	int reboot_power_cycle_delay = 0;
+
+	for_each_intel_connector(dev, connector) {
+		struct intel_panel *panel = &connector->panel;
+
+		reboot_power_cycle_delay = max(reboot_power_cycle_delay,
+					       panel->reboot_power_cycle_delay);
+	}
+
+	if (reboot_power_cycle_delay)
+		msleep(reboot_power_cycle_delay);
+}
+
 int intel_panel_init(struct intel_panel *panel,
 		     struct drm_display_mode *fixed_mode,
-		     struct drm_display_mode *downclock_mode)
+		     struct drm_display_mode *downclock_mode,
+		     int reboot_power_cycle_delay)
 {
 	intel_panel_init_backlight_funcs(panel);
 
 	panel->fixed_mode = fixed_mode;
 	panel->downclock_mode = downclock_mode;
+	panel->reboot_power_cycle_delay = reboot_power_cycle_delay;
 
 	return 0;
 }
-- 
2.7.4

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread