All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot
@ 2020-10-01 15:16 Ville Syrjala
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 2/6] drm/i915: Add an encoder .shutdown() hook Ville Syrjala
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Ville Syrjala @ 2020-10-01 15:16 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson

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

Implement the pci .shutdown() hook in order to quiesce the
hardware prior to reboot. The main purpose here is to turn
all displays off. Some displays/other drivers tend to get
confused if the state after reboot isn't exactly as they
expected.

One specific example was the Dell UP2414Q in MST mode.
It would require me to pull the power cord after a reboot
or else it would just not come back to life. Sadly I don't
have that at hand anymore so not sure if it's still
misbehaving without the graceful shutdown, or if we
managed to fix something else since I last tested it.

For good measure we do a gem suspend as well, so that
we match the suspend flow more closely. Also stopping
all DMA and whatnot is probably a good idea for kexec.
I would expect that some kind of GT reset happens on
normal reboot so probably not totally necessary there.

v2: Use the pci .shutdown() hook instead of a reboot notifier (Lukas)
    Do the gem suspend for kexec (Chris)

Cc: Lukas Wunner <lukas@wunner.de>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 16 ++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_pci.c |  8 ++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 45e719c79183..062b61ebd9c4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1036,6 +1036,22 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
 	drm_modeset_unlock_all(dev);
 }
 
+void i915_driver_shutdown(struct drm_i915_private *i915)
+{
+	i915_gem_suspend(i915);
+
+	drm_kms_helper_poll_disable(&i915->drm);
+
+	drm_atomic_helper_shutdown(&i915->drm);
+
+	intel_dp_mst_suspend(i915);
+
+	intel_runtime_pm_disable_interrupts(i915);
+	intel_hpd_cancel_work(i915);
+
+	intel_suspend_encoders(i915);
+}
+
 static bool suspend_to_idle(struct drm_i915_private *dev_priv)
 {
 #if IS_ENABLED(CONFIG_ACPI_SLEEP)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index eef9a821c49c..9c2672c56cc1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1779,6 +1779,7 @@ extern const struct dev_pm_ops i915_pm_ops;
 
 int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
 void i915_driver_remove(struct drm_i915_private *i915);
+void i915_driver_shutdown(struct drm_i915_private *i915);
 
 int i915_resume_switcheroo(struct drm_i915_private *i915);
 int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state);
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 366ddfc8df6b..249730561b6c 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -1090,11 +1090,19 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 }
 
+static void i915_pci_shutdown(struct pci_dev *pdev)
+{
+	struct drm_i915_private *i915 = pci_get_drvdata(pdev);
+
+	i915_driver_shutdown(i915);
+}
+
 static struct pci_driver i915_pci_driver = {
 	.name = DRIVER_NAME,
 	.id_table = pciidlist,
 	.probe = i915_pci_probe,
 	.remove = i915_pci_remove,
+	.shutdown = i915_pci_shutdown,
 	.driver.pm = &i915_pm_ops,
 };
 
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v2 2/6] drm/i915: Add an encoder .shutdown() hook
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
@ 2020-10-01 15:16 ` Ville Syrjala
  2020-10-06  9:15   ` Jani Nikula
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the " Ville Syrjala
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2020-10-01 15:16 UTC (permalink / raw)
  To: intel-gfx

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

Add a new encoder hook .shutdown() which will get called at the end
of the pci .shutdown() hook. We shall use this to deal with the
panel power cycle delay issues.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h |  5 +++++
 drivers/gpu/drm/i915/i915_drv.c                    | 13 +++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index d5dc18cb8c39..6f3e3d756383 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -199,6 +199,11 @@ struct intel_encoder {
 	 * device interrupts are disabled.
 	 */
 	void (*suspend)(struct intel_encoder *);
+	/*
+	 * Called during system reboot/shutdown after all the
+	 * encoders have been disabled and suspended.
+	 */
+	void (*shutdown)(struct intel_encoder *encoder);
 	enum hpd_pin hpd_pin;
 	enum intel_display_power_domain power_domain;
 	/* for communication with audio component; protected by av_mutex */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 062b61ebd9c4..d38fceb239ac 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1036,6 +1036,18 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
 	drm_modeset_unlock_all(dev);
 }
 
+static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
+{
+	struct drm_device *dev = &dev_priv->drm;
+	struct intel_encoder *encoder;
+
+	drm_modeset_lock_all(dev);
+	for_each_intel_encoder(dev, encoder)
+		if (encoder->shutdown)
+			encoder->shutdown(encoder);
+	drm_modeset_unlock_all(dev);
+}
+
 void i915_driver_shutdown(struct drm_i915_private *i915)
 {
 	i915_gem_suspend(i915);
@@ -1050,6 +1062,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
 	intel_hpd_cancel_work(i915);
 
 	intel_suspend_encoders(i915);
+	intel_shutdown_encoders(i915);
 }
 
 static bool suspend_to_idle(struct drm_i915_private *dev_priv)
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the .shutdown() hook
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 2/6] drm/i915: Add an encoder .shutdown() hook Ville Syrjala
@ 2020-10-01 15:16 ` Ville Syrjala
  2020-10-06  9:29   ` Jani Nikula
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 4/6] drm/i915: Wait for eDP panel power cycle delay on reboot on all platforms Ville Syrjala
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2020-10-01 15:16 UTC (permalink / raw)
  To: intel-gfx

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

Currently VLV/CHV use a reboot notifier to make sure the panel
power cycle delay isn't violated across a system reboot. Replace
that with the new encoder .shutdown() hook.

And let's also stop overriding the power cycle delay with the
max value. No idea why the current code does that. The already
programmed delay should be correct.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_types.h    |  2 -
 drivers/gpu/drm/i915/display/intel_dp.c       | 58 +++++--------------
 2 files changed, 14 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 6f3e3d756383..9b9ed1a2f412 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1319,8 +1319,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.
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 70e0b85442f9..e0f2e9236785 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -28,7 +28,6 @@
 #include <linux/export.h>
 #include <linux/i2c.h>
 #include <linux/notifier.h>
-#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 
@@ -1191,41 +1190,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_i915_private *dev_priv = dp_to_i915(intel_dp);
-	intel_wakeref_t wakeref;
-
-	if (!intel_dp_is_edp(intel_dp) || code != SYS_RESTART)
-		return 0;
-
-	with_pps_lock(intel_dp, wakeref) {
-		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-			enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
-			i915_reg_t pp_ctrl_reg, pp_div_reg;
-			u32 pp_div;
-
-			pp_ctrl_reg = PP_CONTROL(pipe);
-			pp_div_reg  = PP_DIVISOR(pipe);
-			pp_div = intel_de_read(dev_priv, pp_div_reg);
-			pp_div &= PP_REFERENCE_DIVIDER_MASK;
-
-			/* 0x1F write to PP_DIV_REG sets max cycle delay */
-			intel_de_write(dev_priv, pp_div_reg, pp_div | 0x1F);
-			intel_de_write(dev_priv, pp_ctrl_reg,
-				       PANEL_UNLOCK_REGS);
-			msleep(intel_dp->panel_power_cycle_delay);
-		}
-	}
-
-	return 0;
-}
-
 static bool edp_have_panel_power(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -6690,11 +6654,6 @@ void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
 		 */
 		with_pps_lock(intel_dp, wakeref)
 			edp_panel_vdd_off_sync(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);
@@ -6725,6 +6684,18 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
 		edp_panel_vdd_off_sync(intel_dp);
 }
 
+static void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
+{
+	struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
+	intel_wakeref_t wakeref;
+
+	if (!intel_dp_is_edp(intel_dp))
+		return;
+
+	with_pps_lock(intel_dp, wakeref)
+		wait_panel_power_cycle(intel_dp);
+}
+
 static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -7838,9 +7809,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	mutex_unlock(&dev->mode_config.mutex);
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
-		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
@@ -8061,6 +8029,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	intel_encoder->get_config = intel_dp_get_config;
 	intel_encoder->update_pipe = intel_panel_update_backlight;
 	intel_encoder->suspend = intel_dp_encoder_suspend;
+	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
+		intel_encoder->shutdown = intel_dp_encoder_shutdown;
 	if (IS_CHERRYVIEW(dev_priv)) {
 		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = chv_pre_enable_dp;
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v2 4/6] drm/i915: Wait for eDP panel power cycle delay on reboot on all platforms
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 2/6] drm/i915: Add an encoder .shutdown() hook Ville Syrjala
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the " Ville Syrjala
@ 2020-10-01 15:16 ` Ville Syrjala
  2020-10-06  9:30   ` Jani Nikula
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 5/6] drm/i915: Wait for LVDS panel power cycle delay on reboot Ville Syrjala
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2020-10-01 15:16 UTC (permalink / raw)
  To: intel-gfx

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

Extend the eDP panel power cycle delay wait on reboot handling
to cover all platforms. No reason to think that VLV/CHV are
in any way special since the documentation states that the
hardware power cycle delay goes back to its default value on
reset, and that may not be enough for all panels.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c | 1 +
 drivers/gpu/drm/i915/display/intel_dp.c  | 5 ++---
 drivers/gpu/drm/i915/display/intel_dp.h  | 1 +
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 5742394c8292..e3fcd2591a6c 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -5175,6 +5175,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	encoder->get_hw_state = intel_ddi_get_hw_state;
 	encoder->get_config = intel_ddi_get_config;
 	encoder->suspend = intel_dp_encoder_suspend;
+	encoder->shutdown = intel_dp_encoder_shutdown;
 	encoder->get_power_domains = intel_ddi_get_power_domains;
 
 	encoder->type = INTEL_OUTPUT_DDI;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index e0f2e9236785..3a14a003b4c9 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6684,7 +6684,7 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
 		edp_panel_vdd_off_sync(intel_dp);
 }
 
-static void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
+void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
 	intel_wakeref_t wakeref;
@@ -8029,8 +8029,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	intel_encoder->get_config = intel_dp_get_config;
 	intel_encoder->update_pipe = intel_panel_update_backlight;
 	intel_encoder->suspend = intel_dp_encoder_suspend;
-	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		intel_encoder->shutdown = intel_dp_encoder_shutdown;
+	intel_encoder->shutdown = intel_dp_encoder_shutdown;
 	if (IS_CHERRYVIEW(dev_priv)) {
 		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
 		intel_encoder->pre_enable = chv_pre_enable_dp;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 66854aab9887..7466498d4c01 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -57,6 +57,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
 					   bool enable);
 void intel_dp_encoder_reset(struct drm_encoder *encoder);
 void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
+void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder);
 void intel_dp_encoder_flush_work(struct drm_encoder *encoder);
 int intel_dp_compute_config(struct intel_encoder *encoder,
 			    struct intel_crtc_state *pipe_config,
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v2 5/6] drm/i915: Wait for LVDS panel power cycle delay on reboot
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
                   ` (2 preceding siblings ...)
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 4/6] drm/i915: Wait for eDP panel power cycle delay on reboot on all platforms Ville Syrjala
@ 2020-10-01 15:16 ` Ville Syrjala
  2020-10-06  9:31   ` Jani Nikula
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 6/6] drm/i915: Wait for VLV/CHV/BXT/GLK DSI " Ville Syrjala
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2020-10-01 15:16 UTC (permalink / raw)
  To: intel-gfx

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

Just like with eDP let's wait for the power sequencer power
cycle delay before we reboot the machine, as otherwise we
can't guarantee the panel's minimum power cycle delay will
be respected.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_lvds.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
index e65c2de522c3..c6c7c0b9989b 100644
--- a/drivers/gpu/drm/i915/display/intel_lvds.c
+++ b/drivers/gpu/drm/i915/display/intel_lvds.c
@@ -371,6 +371,15 @@ static void pch_post_disable_lvds(struct intel_atomic_state *state,
 	intel_disable_lvds(state, encoder, old_crtc_state, old_conn_state);
 }
 
+static void intel_lvds_shutdown(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+
+	if (intel_de_wait_for_clear(dev_priv, PP_STATUS(0), PP_CYCLE_DELAY_ACTIVE, 5000))
+		drm_err(&dev_priv->drm,
+			"timed out waiting for panel power cycle delay\n");
+}
+
 static enum drm_mode_status
 intel_lvds_mode_valid(struct drm_connector *connector,
 		      struct drm_display_mode *mode)
@@ -897,6 +906,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
 	intel_encoder->get_config = intel_lvds_get_config;
 	intel_encoder->update_pipe = intel_panel_update_backlight;
+	intel_encoder->shutdown = intel_lvds_shutdown;
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
 
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [Intel-gfx] [PATCH v2 6/6] drm/i915: Wait for VLV/CHV/BXT/GLK DSI panel power cycle delay on reboot
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
                   ` (3 preceding siblings ...)
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 5/6] drm/i915: Wait for LVDS panel power cycle delay on reboot Ville Syrjala
@ 2020-10-01 15:16 ` Ville Syrjala
  2020-10-06  9:31   ` Jani Nikula
  2020-10-01 15:36 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/6] drm/i915: Shut down displays gracefully " Patchwork
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjala @ 2020-10-01 15:16 UTC (permalink / raw)
  To: intel-gfx

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

As with eDP and LVDS we should also respect the power cycle
delay on DSI panels. We are not using the power sequencer
for these, and we have no optimizations around the sleep
duration, so we just msleep() the whole thing away.

Note that the ICL+ DSI code doesn't seem to have any power
off/power cycle delay handling whatsoever. The only thing it
handles is the power on delay. As that looks pretty busted
in general I won't bother dealing with it for the time being.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/vlv_dsi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 5e5522923b1e..d52f9c177908 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -985,6 +985,13 @@ static void intel_dsi_post_disable(struct intel_atomic_state *state,
 	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
 }
 
+static void intel_dsi_shutdown(struct intel_encoder *encoder)
+{
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
+
+	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
+}
+
 static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 				   enum pipe *pipe)
 {
@@ -1843,6 +1850,7 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
 	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
 	intel_encoder->get_config = intel_dsi_get_config;
 	intel_encoder->update_pipe = intel_panel_update_backlight;
+	intel_encoder->shutdown = intel_dsi_shutdown;
 
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
 
-- 
2.26.2

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/6] drm/i915: Shut down displays gracefully on reboot
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
                   ` (4 preceding siblings ...)
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 6/6] drm/i915: Wait for VLV/CHV/BXT/GLK DSI " Ville Syrjala
@ 2020-10-01 15:36 ` Patchwork
  2020-10-01 15:57 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-10-01 15:36 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/6] drm/i915: Shut down displays gracefully on reboot
URL   : https://patchwork.freedesktop.org/series/82308/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_reset.c:1312:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gvt/mmio.c:290:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1440:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1494:15: warning: memset with byte count of 16777216
+./include/linux/seqlock.h:752:24: warning: trying to copy expression type 31
+./include/linux/seqlock.h:778:16: warning: trying to copy expression type 31
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


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

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/6] drm/i915: Shut down displays gracefully on reboot
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
                   ` (5 preceding siblings ...)
  2020-10-01 15:36 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/6] drm/i915: Shut down displays gracefully " Patchwork
@ 2020-10-01 15:57 ` Patchwork
  2020-10-01 17:28 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-10-01 15:57 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 7219 bytes --]

== Series Details ==

Series: series starting with [v2,1/6] drm/i915: Shut down displays gracefully on reboot
URL   : https://patchwork.freedesktop.org/series/82308/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9089 -> Patchwork_18607
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/index.html

Known issues
------------

  Here are the changes found in Patchwork_18607 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic@flip:
    - fi-kbl-x1275:       [PASS][1] -> [DMESG-WARN][2] ([i915#62] / [i915#92] / [i915#95]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-kbl-x1275/igt@kms_busy@basic@flip.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-kbl-x1275/igt@kms_busy@basic@flip.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1:
    - fi-icl-u2:          [PASS][3] -> [DMESG-WARN][4] ([i915#1982]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html

  * igt@vgem_basic@unload:
    - fi-skl-guc:         [PASS][5] -> [DMESG-WARN][6] ([i915#2203])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-skl-guc/igt@vgem_basic@unload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-skl-guc/igt@vgem_basic@unload.html

  
#### Possible fixes ####

  * {igt@core_hotunplug@unbind-rebind}:
    - fi-kbl-x1275:       [DMESG-WARN][7] ([i915#62] / [i915#92]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-kbl-x1275/igt@core_hotunplug@unbind-rebind.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-kbl-x1275/igt@core_hotunplug@unbind-rebind.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       [DMESG-WARN][9] ([i915#1982]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-byt-j1900/igt@i915_pm_rpm@basic-pci-d3-state.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-byt-j1900/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-bsw-n3050:       [DMESG-WARN][11] ([i915#1982]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
    - {fi-kbl-7560u}:     [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-kbl-7560u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-kbl-7560u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
    - fi-icl-u2:          [DMESG-WARN][15] ([i915#1982]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
    - fi-bsw-kefka:       [DMESG-WARN][17] ([i915#1982]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [DMESG-FAIL][19] ([i915#62]) -> [DMESG-FAIL][20] ([i915#62] / [i915#95])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-skl-6700k2:      [INCOMPLETE][21] ([i915#2203]) -> [DMESG-WARN][22] ([i915#2203])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-skl-6700k2/igt@kms_chamelium@common-hpd-after-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-skl-6700k2/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][23] ([i915#62] / [i915#92]) -> [DMESG-WARN][24] ([i915#62] / [i915#92] / [i915#95]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
    - fi-kbl-x1275:       [DMESG-WARN][25] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][26] ([i915#62] / [i915#92]) +3 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (43 -> 39)
------------------------------

  Additional (2): fi-cfl-8109u fi-skl-6600u 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_9089 -> Patchwork_18607

  CI-20190529: 20190529
  CI_DRM_9089: cbf909a385ce3d076ce67c5b01ab2f55ffde0186 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5796: 19ae9421a5af7b03a1c9a577c57f2cf8b16a0116 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18607: 2ffa054283d9f6ae926419ca94290a5b57929168 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2ffa054283d9 drm/i915: Wait for VLV/CHV/BXT/GLK DSI panel power cycle delay on reboot
7d17e7ca254e drm/i915: Wait for LVDS panel power cycle delay on reboot
f36090807c49 drm/i915: Wait for eDP panel power cycle delay on reboot on all platforms
7f0733de843f drm/i915: Replace the VLV/CHV eDP reboot notifier with the .shutdown() hook
e7319f669336 drm/i915: Add an encoder .shutdown() hook
312cea57e527 drm/i915: Shut down displays gracefully on reboot

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/index.html

[-- Attachment #1.2: Type: text/html, Size: 9586 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [v2,1/6] drm/i915: Shut down displays gracefully on reboot
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
                   ` (6 preceding siblings ...)
  2020-10-01 15:57 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-10-01 17:28 ` Patchwork
  2020-10-06  9:14 ` [Intel-gfx] [PATCH v2 1/6] " Jani Nikula
  2020-10-06  9:58 ` Chris Wilson
  9 siblings, 0 replies; 19+ messages in thread
From: Patchwork @ 2020-10-01 17:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 13113 bytes --]

== Series Details ==

Series: series starting with [v2,1/6] drm/i915: Shut down displays gracefully on reboot
URL   : https://patchwork.freedesktop.org/series/82308/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9089_full -> Patchwork_18607_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_18607_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18607_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_18607_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ctx_isolation@preservation-s3@bcs0:
    - shard-iclb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-iclb1/igt@gem_ctx_isolation@preservation-s3@bcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-iclb3/igt@gem_ctx_isolation@preservation-s3@bcs0.html

  
Known issues
------------

  Here are the changes found in Patchwork_18607_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_reloc@basic-many-active@rcs0:
    - shard-glk:          [PASS][3] -> [FAIL][4] ([i915#2389]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-glk8/igt@gem_exec_reloc@basic-many-active@rcs0.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-glk9/igt@gem_exec_reloc@basic-many-active@rcs0.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-skl:          [PASS][5] -> [TIMEOUT][6] ([i915#1958] / [i915#2424])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl10/igt@gem_userptr_blits@sync-unmap-cycles.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl6/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@kms_cursor_edge_walk@pipe-b-64x64-right-edge:
    - shard-glk:          [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-glk2/igt@kms_cursor_edge_walk@pipe-b-64x64-right-edge.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-glk5/igt@kms_cursor_edge_walk@pipe-b-64x64-right-edge.html
    - shard-skl:          [PASS][9] -> [DMESG-WARN][10] ([i915#1982]) +12 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl1/igt@kms_cursor_edge_walk@pipe-b-64x64-right-edge.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl2/igt@kms_cursor_edge_walk@pipe-b-64x64-right-edge.html

  * igt@kms_flip@blocking-wf_vblank@a-edp1:
    - shard-tglb:         [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-tglb5/igt@kms_flip@blocking-wf_vblank@a-edp1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-tglb8/igt@kms_flip@blocking-wf_vblank@a-edp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [PASS][13] -> [DMESG-WARN][14] ([i915#180])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-iclb:         [PASS][15] -> [DMESG-WARN][16] ([i915#1982])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-iclb3/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-iclb6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145] / [i915#265])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][19] -> [DMESG-FAIL][20] ([fdo#108145] / [i915#1982])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_primary_blt:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-iclb2/igt@kms_psr@psr2_primary_blt.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-iclb7/igt@kms_psr@psr2_primary_blt.html

  * igt@sysfs_heartbeat_interval@mixed@rcs0:
    - shard-skl:          [PASS][23] -> [FAIL][24] ([i915#1731])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl1/igt@sysfs_heartbeat_interval@mixed@rcs0.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl2/igt@sysfs_heartbeat_interval@mixed@rcs0.html

  
#### Possible fixes ####

  * igt@gem_exec_whisper@basic-fds-forked-all:
    - shard-glk:          [DMESG-WARN][25] ([i915#118] / [i915#95]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-glk8/igt@gem_exec_whisper@basic-fds-forked-all.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-glk9/igt@gem_exec_whisper@basic-fds-forked-all.html

  * igt@i915_selftest@live@execlists:
    - shard-skl:          [INCOMPLETE][27] ([CI#80]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl8/igt@i915_selftest@live@execlists.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl7/igt@i915_selftest@live@execlists.html

  * {igt@kms_async_flips@async-flip-with-page-flip-events}:
    - shard-glk:          [FAIL][29] ([i915#2521]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-glk5/igt@kms_async_flips@async-flip-with-page-flip-events.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-glk8/igt@kms_async_flips@async-flip-with-page-flip-events.html
    - shard-tglb:         [FAIL][31] ([i915#2521]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-tglb2/igt@kms_async_flips@async-flip-with-page-flip-events.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-tglb1/igt@kms_async_flips@async-flip-with-page-flip-events.html

  * igt@kms_cursor_crc@pipe-b-cursor-size-change:
    - shard-skl:          [FAIL][33] ([i915#54]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl2/igt@kms_cursor_crc@pipe-b-cursor-size-change.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl5/igt@kms_cursor_crc@pipe-b-cursor-size-change.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - shard-skl:          [FAIL][35] -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl5/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
    - shard-skl:          [FAIL][37] ([i915#2346]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html

  * igt@kms_flip@2x-flip-vs-suspend@bc-vga1-hdmi-a1:
    - shard-hsw:          [INCOMPLETE][39] ([i915#2055]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-hsw7/igt@kms_flip@2x-flip-vs-suspend@bc-vga1-hdmi-a1.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-hsw8/igt@kms_flip@2x-flip-vs-suspend@bc-vga1-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-pgflip-blt:
    - shard-skl:          [DMESG-WARN][41] ([i915#1982]) -> [PASS][42] +5 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl7/igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-pgflip-blt.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl3/igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-pgflip-blt.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][43] ([fdo#108145] / [i915#265]) -> [PASS][44] +2 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-iclb:         [DMESG-WARN][45] ([i915#1982]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-iclb3/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-iclb1/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [SKIP][47] ([fdo#109441]) -> [PASS][48] +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-iclb1/igt@kms_psr@psr2_primary_mmap_cpu.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][49] ([i915#180]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-kbl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][51] ([fdo#109642] / [fdo#111068]) -> [SKIP][52] ([i915#1911])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9089/shard-iclb8/igt@kms_psr2_su@page_flip.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/shard-iclb2/igt@kms_psr2_su@page_flip.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [CI#80]: https://gitlab.freedesktop.org/gfx-ci/i915-infra/issues/80
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1731]: https://gitlab.freedesktop.org/drm/intel/issues/1731
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1911]: https://gitlab.freedesktop.org/drm/intel/issues/1911
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2055]: https://gitlab.freedesktop.org/drm/intel/issues/2055
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2389]: https://gitlab.freedesktop.org/drm/intel/issues/2389
  [i915#2424]: https://gitlab.freedesktop.org/drm/intel/issues/2424
  [i915#2469]: https://gitlab.freedesktop.org/drm/intel/issues/2469
  [i915#2476]: https://gitlab.freedesktop.org/drm/intel/issues/2476
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_9089 -> Patchwork_18607

  CI-20190529: 20190529
  CI_DRM_9089: cbf909a385ce3d076ce67c5b01ab2f55ffde0186 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5796: 19ae9421a5af7b03a1c9a577c57f2cf8b16a0116 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18607: 2ffa054283d9f6ae926419ca94290a5b57929168 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18607/index.html

[-- Attachment #1.2: Type: text/html, Size: 14949 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
                   ` (7 preceding siblings ...)
  2020-10-01 17:28 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2020-10-06  9:14 ` Jani Nikula
  2020-10-06  9:58 ` Chris Wilson
  9 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2020-10-06  9:14 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx; +Cc: Chris Wilson

On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Implement the pci .shutdown() hook in order to quiesce the
> hardware prior to reboot. The main purpose here is to turn
> all displays off. Some displays/other drivers tend to get
> confused if the state after reboot isn't exactly as they
> expected.
>
> One specific example was the Dell UP2414Q in MST mode.
> It would require me to pull the power cord after a reboot
> or else it would just not come back to life. Sadly I don't
> have that at hand anymore so not sure if it's still
> misbehaving without the graceful shutdown, or if we
> managed to fix something else since I last tested it.
>
> For good measure we do a gem suspend as well, so that
> we match the suspend flow more closely. Also stopping
> all DMA and whatnot is probably a good idea for kexec.
> I would expect that some kind of GT reset happens on
> normal reboot so probably not totally necessary there.
>
> v2: Use the pci .shutdown() hook instead of a reboot notifier (Lukas)
>     Do the gem suspend for kexec (Chris)
>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 16 ++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/i915_pci.c |  8 ++++++++
>  3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 45e719c79183..062b61ebd9c4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1036,6 +1036,22 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  	drm_modeset_unlock_all(dev);
>  }
>  
> +void i915_driver_shutdown(struct drm_i915_private *i915)
> +{
> +	i915_gem_suspend(i915);
> +
> +	drm_kms_helper_poll_disable(&i915->drm);
> +
> +	drm_atomic_helper_shutdown(&i915->drm);
> +
> +	intel_dp_mst_suspend(i915);
> +
> +	intel_runtime_pm_disable_interrupts(i915);
> +	intel_hpd_cancel_work(i915);
> +
> +	intel_suspend_encoders(i915);

I wish we could directly split this to gem and display parts.

*shrug*

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +}
> +
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)
>  {
>  #if IS_ENABLED(CONFIG_ACPI_SLEEP)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index eef9a821c49c..9c2672c56cc1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1779,6 +1779,7 @@ extern const struct dev_pm_ops i915_pm_ops;
>  
>  int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
>  void i915_driver_remove(struct drm_i915_private *i915);
> +void i915_driver_shutdown(struct drm_i915_private *i915);
>  
>  int i915_resume_switcheroo(struct drm_i915_private *i915);
>  int i915_suspend_switcheroo(struct drm_i915_private *i915, pm_message_t state);
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 366ddfc8df6b..249730561b6c 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -1090,11 +1090,19 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	return 0;
>  }
>  
> +static void i915_pci_shutdown(struct pci_dev *pdev)
> +{
> +	struct drm_i915_private *i915 = pci_get_drvdata(pdev);
> +
> +	i915_driver_shutdown(i915);
> +}
> +
>  static struct pci_driver i915_pci_driver = {
>  	.name = DRIVER_NAME,
>  	.id_table = pciidlist,
>  	.probe = i915_pci_probe,
>  	.remove = i915_pci_remove,
> +	.shutdown = i915_pci_shutdown,
>  	.driver.pm = &i915_pm_ops,
>  };

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 2/6] drm/i915: Add an encoder .shutdown() hook
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 2/6] drm/i915: Add an encoder .shutdown() hook Ville Syrjala
@ 2020-10-06  9:15   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2020-10-06  9:15 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a new encoder hook .shutdown() which will get called at the end
> of the pci .shutdown() hook. We shall use this to deal with the
> panel power cycle delay issues.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display_types.h |  5 +++++
>  drivers/gpu/drm/i915/i915_drv.c                    | 13 +++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index d5dc18cb8c39..6f3e3d756383 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -199,6 +199,11 @@ struct intel_encoder {
>  	 * device interrupts are disabled.
>  	 */
>  	void (*suspend)(struct intel_encoder *);
> +	/*
> +	 * Called during system reboot/shutdown after all the
> +	 * encoders have been disabled and suspended.
> +	 */
> +	void (*shutdown)(struct intel_encoder *encoder);
>  	enum hpd_pin hpd_pin;
>  	enum intel_display_power_domain power_domain;
>  	/* for communication with audio component; protected by av_mutex */
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 062b61ebd9c4..d38fceb239ac 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1036,6 +1036,18 @@ static void intel_suspend_encoders(struct drm_i915_private *dev_priv)
>  	drm_modeset_unlock_all(dev);
>  }
>  
> +static void intel_shutdown_encoders(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = &dev_priv->drm;
> +	struct intel_encoder *encoder;
> +
> +	drm_modeset_lock_all(dev);
> +	for_each_intel_encoder(dev, encoder)
> +		if (encoder->shutdown)
> +			encoder->shutdown(encoder);
> +	drm_modeset_unlock_all(dev);
> +}
> +
>  void i915_driver_shutdown(struct drm_i915_private *i915)
>  {
>  	i915_gem_suspend(i915);
> @@ -1050,6 +1062,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)
>  	intel_hpd_cancel_work(i915);
>  
>  	intel_suspend_encoders(i915);
> +	intel_shutdown_encoders(i915);
>  }
>  
>  static bool suspend_to_idle(struct drm_i915_private *dev_priv)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the .shutdown() hook
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the " Ville Syrjala
@ 2020-10-06  9:29   ` Jani Nikula
  2020-10-06 13:43     ` Ville Syrjälä
  0 siblings, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2020-10-06  9:29 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently VLV/CHV use a reboot notifier to make sure the panel
> power cycle delay isn't violated across a system reboot. Replace
> that with the new encoder .shutdown() hook.
>
> And let's also stop overriding the power cycle delay with the
> max value. No idea why the current code does that. The already
> programmed delay should be correct.

I kind of have a little uneasy feeling about conflating these two
changes together. I think both are objectively good changes, just not
necessarily at once.

ISTR setting the max delay was, perhaps, somehow related to the hardware
losing its marbles after power is cut, effectively not ensuring any of
the delays at power-on. So it's possible we set the max here to account
for that. Maybe. ;)

Anyway,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

on the whole.

I'm leaving it up to you, but personally I'd lean towards switching
edp_notify_handler() to use wait_panel_power_cycle(intel_dp) first in a
separate patch, to help with potential bisect results, and then doing
the rest.

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  2 -
>  drivers/gpu/drm/i915/display/intel_dp.c       | 58 +++++--------------
>  2 files changed, 14 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 6f3e3d756383..9b9ed1a2f412 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1319,8 +1319,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.
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 70e0b85442f9..e0f2e9236785 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -28,7 +28,6 @@
>  #include <linux/export.h>
>  #include <linux/i2c.h>
>  #include <linux/notifier.h>
> -#include <linux/reboot.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  
> @@ -1191,41 +1190,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_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	intel_wakeref_t wakeref;
> -
> -	if (!intel_dp_is_edp(intel_dp) || code != SYS_RESTART)
> -		return 0;
> -
> -	with_pps_lock(intel_dp, wakeref) {
> -		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> -			enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> -			i915_reg_t pp_ctrl_reg, pp_div_reg;
> -			u32 pp_div;
> -
> -			pp_ctrl_reg = PP_CONTROL(pipe);
> -			pp_div_reg  = PP_DIVISOR(pipe);
> -			pp_div = intel_de_read(dev_priv, pp_div_reg);
> -			pp_div &= PP_REFERENCE_DIVIDER_MASK;
> -
> -			/* 0x1F write to PP_DIV_REG sets max cycle delay */
> -			intel_de_write(dev_priv, pp_div_reg, pp_div | 0x1F);
> -			intel_de_write(dev_priv, pp_ctrl_reg,
> -				       PANEL_UNLOCK_REGS);
> -			msleep(intel_dp->panel_power_cycle_delay);
> -		}
> -	}
> -
> -	return 0;
> -}
> -
>  static bool edp_have_panel_power(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -6690,11 +6654,6 @@ void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
>  		 */
>  		with_pps_lock(intel_dp, wakeref)
>  			edp_panel_vdd_off_sync(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);
> @@ -6725,6 +6684,18 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>  		edp_panel_vdd_off_sync(intel_dp);
>  }
>  
> +static void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
> +{
> +	struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
> +	intel_wakeref_t wakeref;
> +
> +	if (!intel_dp_is_edp(intel_dp))
> +		return;
> +
> +	with_pps_lock(intel_dp, wakeref)
> +		wait_panel_power_cycle(intel_dp);
> +}
> +
>  static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -7838,9 +7809,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>  	mutex_unlock(&dev->mode_config.mutex);
>  
>  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> -		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
> @@ -8061,6 +8029,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  	intel_encoder->get_config = intel_dp_get_config;
>  	intel_encoder->update_pipe = intel_panel_update_backlight;
>  	intel_encoder->suspend = intel_dp_encoder_suspend;
> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> +		intel_encoder->shutdown = intel_dp_encoder_shutdown;
>  	if (IS_CHERRYVIEW(dev_priv)) {
>  		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
>  		intel_encoder->pre_enable = chv_pre_enable_dp;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 4/6] drm/i915: Wait for eDP panel power cycle delay on reboot on all platforms
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 4/6] drm/i915: Wait for eDP panel power cycle delay on reboot on all platforms Ville Syrjala
@ 2020-10-06  9:30   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2020-10-06  9:30 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Extend the eDP panel power cycle delay wait on reboot handling
> to cover all platforms. No reason to think that VLV/CHV are
> in any way special since the documentation states that the
> hardware power cycle delay goes back to its default value on
> reset, and that may not be enough for all panels.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c | 1 +
>  drivers/gpu/drm/i915/display/intel_dp.c  | 5 ++---
>  drivers/gpu/drm/i915/display/intel_dp.h  | 1 +
>  3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 5742394c8292..e3fcd2591a6c 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -5175,6 +5175,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	encoder->get_hw_state = intel_ddi_get_hw_state;
>  	encoder->get_config = intel_ddi_get_config;
>  	encoder->suspend = intel_dp_encoder_suspend;
> +	encoder->shutdown = intel_dp_encoder_shutdown;
>  	encoder->get_power_domains = intel_ddi_get_power_domains;
>  
>  	encoder->type = INTEL_OUTPUT_DDI;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index e0f2e9236785..3a14a003b4c9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6684,7 +6684,7 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>  		edp_panel_vdd_off_sync(intel_dp);
>  }
>  
> -static void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
> +void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
>  	intel_wakeref_t wakeref;
> @@ -8029,8 +8029,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  	intel_encoder->get_config = intel_dp_get_config;
>  	intel_encoder->update_pipe = intel_panel_update_backlight;
>  	intel_encoder->suspend = intel_dp_encoder_suspend;
> -	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -		intel_encoder->shutdown = intel_dp_encoder_shutdown;
> +	intel_encoder->shutdown = intel_dp_encoder_shutdown;
>  	if (IS_CHERRYVIEW(dev_priv)) {
>  		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
>  		intel_encoder->pre_enable = chv_pre_enable_dp;
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index 66854aab9887..7466498d4c01 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -57,6 +57,7 @@ void intel_dp_sink_set_decompression_state(struct intel_dp *intel_dp,
>  					   bool enable);
>  void intel_dp_encoder_reset(struct drm_encoder *encoder);
>  void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder);
> +void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder);
>  void intel_dp_encoder_flush_work(struct drm_encoder *encoder);
>  int intel_dp_compute_config(struct intel_encoder *encoder,
>  			    struct intel_crtc_state *pipe_config,

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 5/6] drm/i915: Wait for LVDS panel power cycle delay on reboot
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 5/6] drm/i915: Wait for LVDS panel power cycle delay on reboot Ville Syrjala
@ 2020-10-06  9:31   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2020-10-06  9:31 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Just like with eDP let's wait for the power sequencer power
> cycle delay before we reboot the machine, as otherwise we
> can't guarantee the panel's minimum power cycle delay will
> be respected.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_lvds.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index e65c2de522c3..c6c7c0b9989b 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -371,6 +371,15 @@ static void pch_post_disable_lvds(struct intel_atomic_state *state,
>  	intel_disable_lvds(state, encoder, old_crtc_state, old_conn_state);
>  }
>  
> +static void intel_lvds_shutdown(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +
> +	if (intel_de_wait_for_clear(dev_priv, PP_STATUS(0), PP_CYCLE_DELAY_ACTIVE, 5000))

I guess we'll be seeing reports if we hit the 5s delay. :|

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +		drm_err(&dev_priv->drm,
> +			"timed out waiting for panel power cycle delay\n");
> +}
> +
>  static enum drm_mode_status
>  intel_lvds_mode_valid(struct drm_connector *connector,
>  		      struct drm_display_mode *mode)
> @@ -897,6 +906,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
>  	intel_encoder->get_hw_state = intel_lvds_get_hw_state;
>  	intel_encoder->get_config = intel_lvds_get_config;
>  	intel_encoder->update_pipe = intel_panel_update_backlight;
> +	intel_encoder->shutdown = intel_lvds_shutdown;
>  	intel_connector->get_hw_state = intel_connector_get_hw_state;
>  
>  	intel_connector_attach_encoder(intel_connector, intel_encoder);

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 6/6] drm/i915: Wait for VLV/CHV/BXT/GLK DSI panel power cycle delay on reboot
  2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 6/6] drm/i915: Wait for VLV/CHV/BXT/GLK DSI " Ville Syrjala
@ 2020-10-06  9:31   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2020-10-06  9:31 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> As with eDP and LVDS we should also respect the power cycle
> delay on DSI panels. We are not using the power sequencer
> for these, and we have no optimizations around the sleep
> duration, so we just msleep() the whole thing away.
>
> Note that the ICL+ DSI code doesn't seem to have any power
> off/power cycle delay handling whatsoever. The only thing it
> handles is the power on delay. As that looks pretty busted
> in general I won't bother dealing with it for the time being.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Better than nothing.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/vlv_dsi.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 5e5522923b1e..d52f9c177908 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -985,6 +985,13 @@ static void intel_dsi_post_disable(struct intel_atomic_state *state,
>  	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
>  }
>  
> +static void intel_dsi_shutdown(struct intel_encoder *encoder)
> +{
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> +
> +	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
> +}
> +
>  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  				   enum pipe *pipe)
>  {
> @@ -1843,6 +1850,7 @@ void vlv_dsi_init(struct drm_i915_private *dev_priv)
>  	intel_encoder->get_hw_state = intel_dsi_get_hw_state;
>  	intel_encoder->get_config = intel_dsi_get_config;
>  	intel_encoder->update_pipe = intel_panel_update_backlight;
> +	intel_encoder->shutdown = intel_dsi_shutdown;
>  
>  	intel_connector->get_hw_state = intel_connector_get_hw_state;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot
  2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
                   ` (8 preceding siblings ...)
  2020-10-06  9:14 ` [Intel-gfx] [PATCH v2 1/6] " Jani Nikula
@ 2020-10-06  9:58 ` Chris Wilson
  9 siblings, 0 replies; 19+ messages in thread
From: Chris Wilson @ 2020-10-06  9:58 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Quoting Ville Syrjala (2020-10-01 16:16:35)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Implement the pci .shutdown() hook in order to quiesce the
> hardware prior to reboot. The main purpose here is to turn
> all displays off. Some displays/other drivers tend to get
> confused if the state after reboot isn't exactly as they
> expected.
> 
> One specific example was the Dell UP2414Q in MST mode.
> It would require me to pull the power cord after a reboot
> or else it would just not come back to life. Sadly I don't
> have that at hand anymore so not sure if it's still
> misbehaving without the graceful shutdown, or if we
> managed to fix something else since I last tested it.
> 
> For good measure we do a gem suspend as well, so that
> we match the suspend flow more closely. Also stopping
> all DMA and whatnot is probably a good idea for kexec.
> I would expect that some kind of GT reset happens on
> normal reboot so probably not totally necessary there.

Note that we should do a reset just in case kexec [whatever comes next]
fails or forgets, depending on your level of paranoia. So device shutdown
is called from the reboot/halt pathways, bypassing the usual suspend/remove.

> v2: Use the pci .shutdown() hook instead of a reboot notifier (Lukas)
>     Do the gem suspend for kexec (Chris)

We could do with a little bit more force than i915_gem_suspend, but baby
steps. I was trying to think how we could squeeze a kexec test into CI.
I don't know kexec well enough to devise a robust igt.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the .shutdown() hook
  2020-10-06  9:29   ` Jani Nikula
@ 2020-10-06 13:43     ` Ville Syrjälä
  2020-10-06 18:13       ` Jani Nikula
  0 siblings, 1 reply; 19+ messages in thread
From: Ville Syrjälä @ 2020-10-06 13:43 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Oct 06, 2020 at 12:29:22PM +0300, Jani Nikula wrote:
> On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently VLV/CHV use a reboot notifier to make sure the panel
> > power cycle delay isn't violated across a system reboot. Replace
> > that with the new encoder .shutdown() hook.
> >
> > And let's also stop overriding the power cycle delay with the
> > max value. No idea why the current code does that. The already
> > programmed delay should be correct.
> 
> I kind of have a little uneasy feeling about conflating these two
> changes together. I think both are objectively good changes, just not
> necessarily at once.
> 
> ISTR setting the max delay was, perhaps, somehow related to the hardware
> losing its marbles after power is cut, effectively not ensuring any of
> the delays at power-on. So it's possible we set the max here to account
> for that. Maybe. ;)
> 
> Anyway,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> on the whole.
> 
> I'm leaving it up to you, but personally I'd lean towards switching
> edp_notify_handler() to use wait_panel_power_cycle(intel_dp) first in a
> separate patch, to help with potential bisect results, and then doing
> the rest.

I don't think it would be quite that simple. We'd have to also toss
in some combination of panel_off() and vdd_off_sync() in there,
depending on whether the panel power is currently enabled or not.
Otherwise the bookkeeping needed by wait_panel_power_cycle() isn't
going to be up to date.

> 
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  .../drm/i915/display/intel_display_types.h    |  2 -
> >  drivers/gpu/drm/i915/display/intel_dp.c       | 58 +++++--------------
> >  2 files changed, 14 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 6f3e3d756383..9b9ed1a2f412 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1319,8 +1319,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.
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 70e0b85442f9..e0f2e9236785 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -28,7 +28,6 @@
> >  #include <linux/export.h>
> >  #include <linux/i2c.h>
> >  #include <linux/notifier.h>
> > -#include <linux/reboot.h>
> >  #include <linux/slab.h>
> >  #include <linux/types.h>
> >  
> > @@ -1191,41 +1190,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_i915_private *dev_priv = dp_to_i915(intel_dp);
> > -	intel_wakeref_t wakeref;
> > -
> > -	if (!intel_dp_is_edp(intel_dp) || code != SYS_RESTART)
> > -		return 0;
> > -
> > -	with_pps_lock(intel_dp, wakeref) {
> > -		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > -			enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> > -			i915_reg_t pp_ctrl_reg, pp_div_reg;
> > -			u32 pp_div;
> > -
> > -			pp_ctrl_reg = PP_CONTROL(pipe);
> > -			pp_div_reg  = PP_DIVISOR(pipe);
> > -			pp_div = intel_de_read(dev_priv, pp_div_reg);
> > -			pp_div &= PP_REFERENCE_DIVIDER_MASK;
> > -
> > -			/* 0x1F write to PP_DIV_REG sets max cycle delay */
> > -			intel_de_write(dev_priv, pp_div_reg, pp_div | 0x1F);
> > -			intel_de_write(dev_priv, pp_ctrl_reg,
> > -				       PANEL_UNLOCK_REGS);
> > -			msleep(intel_dp->panel_power_cycle_delay);
> > -		}
> > -	}
> > -
> > -	return 0;
> > -}
> > -
> >  static bool edp_have_panel_power(struct intel_dp *intel_dp)
> >  {
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > @@ -6690,11 +6654,6 @@ void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
> >  		 */
> >  		with_pps_lock(intel_dp, wakeref)
> >  			edp_panel_vdd_off_sync(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);
> > @@ -6725,6 +6684,18 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
> >  		edp_panel_vdd_off_sync(intel_dp);
> >  }
> >  
> > +static void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
> > +{
> > +	struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
> > +	intel_wakeref_t wakeref;
> > +
> > +	if (!intel_dp_is_edp(intel_dp))
> > +		return;
> > +
> > +	with_pps_lock(intel_dp, wakeref)
> > +		wait_panel_power_cycle(intel_dp);
> > +}
> > +
> >  static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
> >  {
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > @@ -7838,9 +7809,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >  	mutex_unlock(&dev->mode_config.mutex);
> >  
> >  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > -		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
> > @@ -8061,6 +8029,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> >  	intel_encoder->get_config = intel_dp_get_config;
> >  	intel_encoder->update_pipe = intel_panel_update_backlight;
> >  	intel_encoder->suspend = intel_dp_encoder_suspend;
> > +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> > +		intel_encoder->shutdown = intel_dp_encoder_shutdown;
> >  	if (IS_CHERRYVIEW(dev_priv)) {
> >  		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
> >  		intel_encoder->pre_enable = chv_pre_enable_dp;
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

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

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the .shutdown() hook
  2020-10-06 13:43     ` Ville Syrjälä
@ 2020-10-06 18:13       ` Jani Nikula
  2020-10-06 18:33         ` Ville Syrjälä
  0 siblings, 1 reply; 19+ messages in thread
From: Jani Nikula @ 2020-10-06 18:13 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Tue, 06 Oct 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Oct 06, 2020 at 12:29:22PM +0300, Jani Nikula wrote:
>> On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > Currently VLV/CHV use a reboot notifier to make sure the panel
>> > power cycle delay isn't violated across a system reboot. Replace
>> > that with the new encoder .shutdown() hook.
>> >
>> > And let's also stop overriding the power cycle delay with the
>> > max value. No idea why the current code does that. The already
>> > programmed delay should be correct.
>> 
>> I kind of have a little uneasy feeling about conflating these two
>> changes together. I think both are objectively good changes, just not
>> necessarily at once.
>> 
>> ISTR setting the max delay was, perhaps, somehow related to the hardware
>> losing its marbles after power is cut, effectively not ensuring any of
>> the delays at power-on. So it's possible we set the max here to account
>> for that. Maybe. ;)
>> 
>> Anyway,
>> 
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> on the whole.
>> 
>> I'm leaving it up to you, but personally I'd lean towards switching
>> edp_notify_handler() to use wait_panel_power_cycle(intel_dp) first in a
>> separate patch, to help with potential bisect results, and then doing
>> the rest.
>
> I don't think it would be quite that simple. We'd have to also toss
> in some combination of panel_off() and vdd_off_sync() in there,
> depending on whether the panel power is currently enabled or not.
> Otherwise the bookkeeping needed by wait_panel_power_cycle() isn't
> going to be up to date.

Oh? So the calls via encoder hooks actually ensure that?

In that case, push away!

BR,
Jani.



>
>> 
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  .../drm/i915/display/intel_display_types.h    |  2 -
>> >  drivers/gpu/drm/i915/display/intel_dp.c       | 58 +++++--------------
>> >  2 files changed, 14 insertions(+), 46 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > index 6f3e3d756383..9b9ed1a2f412 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > @@ -1319,8 +1319,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.
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 70e0b85442f9..e0f2e9236785 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -28,7 +28,6 @@
>> >  #include <linux/export.h>
>> >  #include <linux/i2c.h>
>> >  #include <linux/notifier.h>
>> > -#include <linux/reboot.h>
>> >  #include <linux/slab.h>
>> >  #include <linux/types.h>
>> >  
>> > @@ -1191,41 +1190,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_i915_private *dev_priv = dp_to_i915(intel_dp);
>> > -	intel_wakeref_t wakeref;
>> > -
>> > -	if (!intel_dp_is_edp(intel_dp) || code != SYS_RESTART)
>> > -		return 0;
>> > -
>> > -	with_pps_lock(intel_dp, wakeref) {
>> > -		if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>> > -			enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
>> > -			i915_reg_t pp_ctrl_reg, pp_div_reg;
>> > -			u32 pp_div;
>> > -
>> > -			pp_ctrl_reg = PP_CONTROL(pipe);
>> > -			pp_div_reg  = PP_DIVISOR(pipe);
>> > -			pp_div = intel_de_read(dev_priv, pp_div_reg);
>> > -			pp_div &= PP_REFERENCE_DIVIDER_MASK;
>> > -
>> > -			/* 0x1F write to PP_DIV_REG sets max cycle delay */
>> > -			intel_de_write(dev_priv, pp_div_reg, pp_div | 0x1F);
>> > -			intel_de_write(dev_priv, pp_ctrl_reg,
>> > -				       PANEL_UNLOCK_REGS);
>> > -			msleep(intel_dp->panel_power_cycle_delay);
>> > -		}
>> > -	}
>> > -
>> > -	return 0;
>> > -}
>> > -
>> >  static bool edp_have_panel_power(struct intel_dp *intel_dp)
>> >  {
>> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>> > @@ -6690,11 +6654,6 @@ void intel_dp_encoder_flush_work(struct drm_encoder *encoder)
>> >  		 */
>> >  		with_pps_lock(intel_dp, wakeref)
>> >  			edp_panel_vdd_off_sync(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);
>> > @@ -6725,6 +6684,18 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder)
>> >  		edp_panel_vdd_off_sync(intel_dp);
>> >  }
>> >  
>> > +static void intel_dp_encoder_shutdown(struct intel_encoder *intel_encoder)
>> > +{
>> > +	struct intel_dp *intel_dp = enc_to_intel_dp(intel_encoder);
>> > +	intel_wakeref_t wakeref;
>> > +
>> > +	if (!intel_dp_is_edp(intel_dp))
>> > +		return;
>> > +
>> > +	with_pps_lock(intel_dp, wakeref)
>> > +		wait_panel_power_cycle(intel_dp);
>> > +}
>> > +
>> >  static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
>> >  {
>> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>> > @@ -7838,9 +7809,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> >  	mutex_unlock(&dev->mode_config.mutex);
>> >  
>> >  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>> > -		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
>> > @@ -8061,6 +8029,8 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>> >  	intel_encoder->get_config = intel_dp_get_config;
>> >  	intel_encoder->update_pipe = intel_panel_update_backlight;
>> >  	intel_encoder->suspend = intel_dp_encoder_suspend;
>> > +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>> > +		intel_encoder->shutdown = intel_dp_encoder_shutdown;
>> >  	if (IS_CHERRYVIEW(dev_priv)) {
>> >  		intel_encoder->pre_pll_enable = chv_dp_pre_pll_enable;
>> >  		intel_encoder->pre_enable = chv_pre_enable_dp;
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 19+ messages in thread
* Re: [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the .shutdown() hook
  2020-10-06 18:13       ` Jani Nikula
@ 2020-10-06 18:33         ` Ville Syrjälä
  0 siblings, 0 replies; 19+ messages in thread
From: Ville Syrjälä @ 2020-10-06 18:33 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Tue, Oct 06, 2020 at 09:13:32PM +0300, Jani Nikula wrote:
> On Tue, 06 Oct 2020, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Oct 06, 2020 at 12:29:22PM +0300, Jani Nikula wrote:
> >> On Thu, 01 Oct 2020, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > Currently VLV/CHV use a reboot notifier to make sure the panel
> >> > power cycle delay isn't violated across a system reboot. Replace
> >> > that with the new encoder .shutdown() hook.
> >> >
> >> > And let's also stop overriding the power cycle delay with the
> >> > max value. No idea why the current code does that. The already
> >> > programmed delay should be correct.
> >> 
> >> I kind of have a little uneasy feeling about conflating these two
> >> changes together. I think both are objectively good changes, just not
> >> necessarily at once.
> >> 
> >> ISTR setting the max delay was, perhaps, somehow related to the hardware
> >> losing its marbles after power is cut, effectively not ensuring any of
> >> the delays at power-on. So it's possible we set the max here to account
> >> for that. Maybe. ;)
> >> 
> >> Anyway,
> >> 
> >> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> >> 
> >> on the whole.
> >> 
> >> I'm leaving it up to you, but personally I'd lean towards switching
> >> edp_notify_handler() to use wait_panel_power_cycle(intel_dp) first in a
> >> separate patch, to help with potential bisect results, and then doing
> >> the rest.
> >
> > I don't think it would be quite that simple. We'd have to also toss
> > in some combination of panel_off() and vdd_off_sync() in there,
> > depending on whether the panel power is currently enabled or not.
> > Otherwise the bookkeeping needed by wait_panel_power_cycle() isn't
> > going to be up to date.
> 
> Oh? So the calls via encoder hooks actually ensure that?

1. drm_atomic_helper_shutdown()
   -> panel_off() as needed via the normal crtc disable sequence
2. mst suspend + interrupts off + cancel hpd stuff
   -> hopefully nothing can re-enable vdd after this point
3. encoder.suspend()
   -> vdd_off_sync() to really turn vdd off if it's still lingering
4. encoder.shutdown()
   -> wait_panel_power_cycle()

Hopefully it even works like that :P

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

^ permalink raw reply	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2020-10-06 18:33 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-01 15:16 [Intel-gfx] [PATCH v2 1/6] drm/i915: Shut down displays gracefully on reboot Ville Syrjala
2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 2/6] drm/i915: Add an encoder .shutdown() hook Ville Syrjala
2020-10-06  9:15   ` Jani Nikula
2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 3/6] drm/i915: Replace the VLV/CHV eDP reboot notifier with the " Ville Syrjala
2020-10-06  9:29   ` Jani Nikula
2020-10-06 13:43     ` Ville Syrjälä
2020-10-06 18:13       ` Jani Nikula
2020-10-06 18:33         ` Ville Syrjälä
2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 4/6] drm/i915: Wait for eDP panel power cycle delay on reboot on all platforms Ville Syrjala
2020-10-06  9:30   ` Jani Nikula
2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 5/6] drm/i915: Wait for LVDS panel power cycle delay on reboot Ville Syrjala
2020-10-06  9:31   ` Jani Nikula
2020-10-01 15:16 ` [Intel-gfx] [PATCH v2 6/6] drm/i915: Wait for VLV/CHV/BXT/GLK DSI " Ville Syrjala
2020-10-06  9:31   ` Jani Nikula
2020-10-01 15:36 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/6] drm/i915: Shut down displays gracefully " Patchwork
2020-10-01 15:57 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-10-01 17:28 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-10-06  9:14 ` [Intel-gfx] [PATCH v2 1/6] " Jani Nikula
2020-10-06  9:58 ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.