All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume
@ 2020-10-06 18:58 Ville Syrjala
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init Ville Syrjala
                   ` (12 more replies)
  0 siblings, 13 replies; 25+ messages in thread
From: Ville Syrjala @ 2020-10-06 18:58 UTC (permalink / raw)
  To: intel-gfx

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

Currently we call .hpd_irq_setup() directly just before display
resume, and follow it with another call via intel_hpd_init()
just afterwards. Assuming the hpd pins are marked as enabled
during the open-coded call these two things do exactly the
same thing (ie. enable HPD interrupts). Which even makes sense
since we definitely need working HPD interrupts for MST sideband
during the display resume.

So let's just nuke the open-coded call and move the intel_hpd_init()
call earlier.

If we really would like to prevent all *long* HPD processing during
display resume we'd need some kind of software mechanism to simply
ignore all long HPDs. Currently we appear to have that just for
fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
right all the time I guess that's mostly sufficient.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c |  8 +-------
 drivers/gpu/drm/i915/i915_drv.c              | 16 ++--------------
 2 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 907e1d155443..87df7167433f 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5036,18 +5036,12 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 		intel_pps_unlock_regs_wa(dev_priv);
 		intel_modeset_init_hw(dev_priv);
 		intel_init_clock_gating(dev_priv);
-
-		spin_lock_irq(&dev_priv->irq_lock);
-		if (dev_priv->display.hpd_irq_setup)
-			dev_priv->display.hpd_irq_setup(dev_priv);
-		spin_unlock_irq(&dev_priv->irq_lock);
+		intel_hpd_init(dev_priv);
 
 		ret = __intel_display_resume(dev, state, ctx);
 		if (ret)
 			drm_err(&dev_priv->drm,
 				"Restoring old state failed with %i\n", ret);
-
-		intel_hpd_init(dev_priv);
 	}
 
 	drm_atomic_state_put(state);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ebc15066d108..b2a050d916e3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1226,26 +1226,14 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	intel_modeset_init_hw(dev_priv);
 	intel_init_clock_gating(dev_priv);
+	intel_hpd_init(dev_priv);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.hpd_irq_setup)
-		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
-
+	/* MST sideband requires HPD interrupts enabled */
 	intel_dp_mst_resume(dev_priv);
-
 	intel_display_resume(dev);
 
 	drm_kms_helper_poll_enable(dev);
 
-	/*
-	 * ... but also need to make sure that hotplug processing
-	 * doesn't cause havoc. Like in the driver load code we don't
-	 * bother with the tiny race here where we might lose hotplug
-	 * notifications.
-	 * */
-	intel_hpd_init(dev_priv);
-
 	intel_opregion_resume(dev_priv);
 
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
-- 
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] 25+ messages in thread

* [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
@ 2020-10-06 18:58 ` Ville Syrjala
  2020-10-12 20:16   ` Imre Deak
  2020-10-19 15:39   ` Imre Deak
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 3/3] drm/i915: Refactor .hpd_irq_setup() calls a bit Ville Syrjala
                   ` (11 subsequent siblings)
  12 siblings, 2 replies; 25+ messages in thread
From: Ville Syrjala @ 2020-10-06 18:58 UTC (permalink / raw)
  To: intel-gfx

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

LSPCON requires HPD detection logic to be enabled when we call
its .reset() hook during resume, to check the live state of the
pin. To that end let's reorder the resume sequence such that
we do HPD init before the encoder reset.

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b2a050d916e3..66ddd4161885 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1213,21 +1213,20 @@ static int i915_drm_resume(struct drm_device *dev)
 	 * GPU will hang. i915_gem_init_hw() will initiate batches to
 	 * update/restore the context.
 	 *
-	 * drm_mode_config_reset() needs AUX interrupts.
-	 *
 	 * Modeset enabling in intel_modeset_init_hw() also needs working
 	 * interrupts.
 	 */
 	intel_runtime_pm_enable_interrupts(dev_priv);
 
-	drm_mode_config_reset(dev);
-
 	i915_gem_resume(dev_priv);
 
 	intel_modeset_init_hw(dev_priv);
 	intel_init_clock_gating(dev_priv);
 	intel_hpd_init(dev_priv);
 
+	/* May need AUX interrupts and HPD detection enabled */
+	drm_mode_config_reset(dev);
+
 	/* MST sideband requires HPD interrupts enabled */
 	intel_dp_mst_resume(dev_priv);
 	intel_display_resume(dev);
-- 
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] 25+ messages in thread

* [Intel-gfx] [PATCH 3/3] drm/i915: Refactor .hpd_irq_setup() calls a bit
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init Ville Syrjala
@ 2020-10-06 18:58 ` Ville Syrjala
  2020-10-12 20:22   ` Imre Deak
  2020-10-06 19:27 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Reorder hpd init vs. display resume Patchwork
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 25+ messages in thread
From: Ville Syrjala @ 2020-10-06 18:58 UTC (permalink / raw)
  To: intel-gfx

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

Add a small wrapper for .hpd_irq_setup() which does the
"do we even have the hook?" and "are display interrupts enabled?"
checks.

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

diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 5c58c1ed6493..6110d71b4f14 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -213,6 +213,12 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv)
 	}
 }
 
+static void intel_hpd_irq_setup(struct drm_i915_private *i915)
+{
+	if (i915->display_irqs_enabled && i915->display.hpd_irq_setup)
+		i915->display.hpd_irq_setup(i915);
+}
+
 static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
 {
 	struct drm_i915_private *dev_priv =
@@ -248,8 +254,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
 			dev_priv->hotplug.stats[pin].state = HPD_ENABLED;
 	}
 
-	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup)
-		dev_priv->display.hpd_irq_setup(dev_priv);
+	intel_hpd_irq_setup(dev_priv);
 
 	spin_unlock_irq(&dev_priv->irq_lock);
 
@@ -556,8 +561,8 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
 	 * Disable any IRQs that storms were detected on. Polling enablement
 	 * happens later in our hotplug work.
 	 */
-	if (storm_detected && dev_priv->display_irqs_enabled)
-		dev_priv->display.hpd_irq_setup(dev_priv);
+	if (storm_detected)
+		intel_hpd_irq_setup(dev_priv);
 	spin_unlock(&dev_priv->irq_lock);
 
 	/*
@@ -602,12 +607,9 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
 	 * Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked checks happy.
 	 */
-	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
-		spin_lock_irq(&dev_priv->irq_lock);
-		if (dev_priv->display_irqs_enabled)
-			dev_priv->display.hpd_irq_setup(dev_priv);
-		spin_unlock_irq(&dev_priv->irq_lock);
-	}
+	spin_lock_irq(&dev_priv->irq_lock);
+	intel_hpd_irq_setup(dev_priv);
+	spin_unlock_irq(&dev_priv->irq_lock);
 }
 
 static void i915_hpd_poll_init_work(struct work_struct *work)
-- 
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] 25+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init Ville Syrjala
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 3/3] drm/i915: Refactor .hpd_irq_setup() calls a bit Ville Syrjala
@ 2020-10-06 19:27 ` Patchwork
  2020-10-07 11:06 ` [Intel-gfx] [PATCH 1/3] " Ville Syrjälä
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-06 19:27 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx


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

== Series Details ==

Series: series starting with [1/3] drm/i915: Reorder hpd init vs. display resume
URL   : https://patchwork.freedesktop.org/series/82417/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9104 -> Patchwork_18641
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-cml-u2:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-cml-u2/igt@gem_exec_suspend@basic-s3.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-cml-u2/igt@gem_exec_suspend@basic-s3.html
    - fi-cml-s:           [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-cml-s/igt@gem_exec_suspend@basic-s3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-cml-s/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_pm_rpm@basic-rte:
    - fi-bsw-kefka:       [PASS][5] -> [FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-bsw-kefka/igt@i915_pm_rpm@basic-rte.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-bsw-kefka/igt@i915_pm_rpm@basic-rte.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-cml-s:           [PASS][7] -> [INCOMPLETE][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-cml-s/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-cml-s/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> [FAIL][9]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-bsw-kefka/igt@runner@aborted.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@core_hotunplug@unbind-rebind}:
    - fi-bsw-kefka:       [PASS][10] -> [INCOMPLETE][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-bsw-kefka/igt@core_hotunplug@unbind-rebind.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-bsw-kefka/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3:
    - {fi-ehl-1}:         [PASS][12] -> [INCOMPLETE][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-ehl-1/igt@gem_exec_suspend@basic-s3.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-ehl-1/igt@gem_exec_suspend@basic-s3.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-kbl-r:           [PASS][14] -> [INCOMPLETE][15] ([i915#155])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-kbl-r/igt@gem_exec_suspend@basic-s3.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-kbl-r/igt@gem_exec_suspend@basic-s3.html
    - fi-tgl-u2:          [PASS][16] -> [INCOMPLETE][17] ([i915#1602])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html
    - fi-icl-y:           [PASS][18] -> [INCOMPLETE][19] ([i915#1185])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-icl-y/igt@gem_exec_suspend@basic-s3.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-icl-y/igt@gem_exec_suspend@basic-s3.html
    - fi-icl-u2:          [PASS][20] -> [INCOMPLETE][21] ([i915#1185] / [i915#263])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-icl-u2/igt@gem_exec_suspend@basic-s3.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-icl-u2/igt@gem_exec_suspend@basic-s3.html
    - fi-skl-6600u:       [PASS][22] -> [INCOMPLETE][23] ([i915#198])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-skl-6600u/igt@gem_exec_suspend@basic-s3.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-skl-6600u/igt@gem_exec_suspend@basic-s3.html

  
#### Possible fixes ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-byt-j1900:       [DMESG-WARN][24] ([i915#1982]) -> [PASS][25] +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-byt-j1900/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Warnings ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [DMESG-WARN][26] ([i915#1982]) -> [SKIP][27] ([fdo#109271])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][28] ([i915#62] / [i915#92]) -> [DMESG-WARN][29] ([i915#62] / [i915#92] / [i915#95]) +1 similar issue
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-kbl-x1275:       [DMESG-WARN][30] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][31] ([i915#62] / [i915#92]) +6 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9104/fi-kbl-x1275/igt@kms_force_connector_basic@prune-stale-modes.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18641/fi-kbl-x1275/igt@kms_force_connector_basic@prune-stale-modes.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1185]: https://gitlab.freedesktop.org/drm/intel/issues/1185
  [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
  [i915#1602]: https://gitlab.freedesktop.org/drm/intel/issues/1602
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#263]: https://gitlab.freedesktop.org/drm/intel/issues/263
  [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 (45 -> 39)
------------------------------

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


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

  * Linux: CI_DRM_9104 -> Patchwork_18641

  CI-20190529: 20190529
  CI_DRM_9104: 9cca7a33b0ebfaa5e0e86098b38eb7508097936a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5802: 0e4fbc60ca5ad6585e642d2ddf8313f3c738426e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18641: b4a7c0d3262015b9f52330cf3ebda023ce481d01 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b4a7c0d32620 drm/i915: Refactor .hpd_irq_setup() calls a bit
1d45fb3bb190 drm/i915: Do drm_mode_config_reset() after HPD init
b9de42a8fa89 drm/i915: Reorder hpd init vs. display resume

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 9828 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] 25+ messages in thread

* Re: [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (2 preceding siblings ...)
  2020-10-06 19:27 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Reorder hpd init vs. display resume Patchwork
@ 2020-10-07 11:06 ` Ville Syrjälä
  2020-10-07 19:22 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2020-10-07 11:06 UTC (permalink / raw)
  To: intel-gfx; +Cc: Daniel Vetter

On Tue, Oct 06, 2020 at 09:58:07PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we call .hpd_irq_setup() directly just before display
> resume, and follow it with another call via intel_hpd_init()
> just afterwards. Assuming the hpd pins are marked as enabled
> during the open-coded call these two things do exactly the
> same thing (ie. enable HPD interrupts). Which even makes sense
> since we definitely need working HPD interrupts for MST sideband
> during the display resume.
> 
> So let's just nuke the open-coded call and move the intel_hpd_init()
> call earlier.
> 
> If we really would like to prevent all *long* HPD processing during
> display resume we'd need some kind of software mechanism to simply
> ignore all long HPDs. Currently we appear to have that just for
> fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
> right all the time I guess that's mostly sufficient.

Daniel suggested including the archaeological record here.
This is what I was planning to amend here:

"For a bit of history on this, we first got a mechanism to block
hotplug processing during suspend in commit 15239099d7a7 ("drm/i915: 
enable irqs earlier when resuming") on account of moving the irq enable 
earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper: 
Fix hpd vs. initial config races") because the fdev initial config
got pushed to a later point. The second ad-hoc hpd_irq_setup() for
resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST 
support (v0.7)") to be able to do MST sideband during resume. And
finally we got a partial resurrection of the hpd blocking
mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
processing during suspend"), but this time it only prevent fbdev
from handling the hpd while resuming."

But looks like I was far too optimistic and this did in fact blow
up in CI. So I'll need to do some actual work to figure out what's
going on...

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c |  8 +-------
>  drivers/gpu/drm/i915/i915_drv.c              | 16 ++--------------
>  2 files changed, 3 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 907e1d155443..87df7167433f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5036,18 +5036,12 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
>  		intel_pps_unlock_regs_wa(dev_priv);
>  		intel_modeset_init_hw(dev_priv);
>  		intel_init_clock_gating(dev_priv);
> -
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		if (dev_priv->display.hpd_irq_setup)
> -			dev_priv->display.hpd_irq_setup(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> +		intel_hpd_init(dev_priv);
>  
>  		ret = __intel_display_resume(dev, state, ctx);
>  		if (ret)
>  			drm_err(&dev_priv->drm,
>  				"Restoring old state failed with %i\n", ret);
> -
> -		intel_hpd_init(dev_priv);
>  	}
>  
>  	drm_atomic_state_put(state);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ebc15066d108..b2a050d916e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1226,26 +1226,14 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	intel_modeset_init_hw(dev_priv);
>  	intel_init_clock_gating(dev_priv);
> +	intel_hpd_init(dev_priv);
>  
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.hpd_irq_setup)
> -		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -
> +	/* MST sideband requires HPD interrupts enabled */
>  	intel_dp_mst_resume(dev_priv);
> -
>  	intel_display_resume(dev);
>  
>  	drm_kms_helper_poll_enable(dev);
>  
> -	/*
> -	 * ... but also need to make sure that hotplug processing
> -	 * doesn't cause havoc. Like in the driver load code we don't
> -	 * bother with the tiny race here where we might lose hotplug
> -	 * notifications.
> -	 * */
> -	intel_hpd_init(dev_priv);
> -
>  	intel_opregion_resume(dev_priv);
>  
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> -- 
> 2.26.2

-- 
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] 25+ messages in thread

* [Intel-gfx] [PATCH v2 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (3 preceding siblings ...)
  2020-10-07 11:06 ` [Intel-gfx] [PATCH 1/3] " Ville Syrjälä
@ 2020-10-07 19:22 ` Ville Syrjala
  2020-10-07 22:15   ` Lyude Paul
                     ` (2 more replies)
  2020-10-08 11:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev2) Patchwork
                   ` (7 subsequent siblings)
  12 siblings, 3 replies; 25+ messages in thread
From: Ville Syrjala @ 2020-10-07 19:22 UTC (permalink / raw)
  To: intel-gfx

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

Currently we call .hpd_irq_setup() directly just before display
resume, and follow it with another call via intel_hpd_init()
just afterwards. Assuming the hpd pins are marked as enabled
during the open-coded call these two things do exactly the
same thing (ie. enable HPD interrupts). Which even makes sense
since we definitely need working HPD interrupts for MST sideband
during the display resume.

So let's nuke the open-coded call and move the intel_hpd_init()
call earlier. However we need to leave the poll_init_work stuff
behind after the display resume as that will trigger display
detection while we're resuming. We don't want that trampling over
the display resume process. To make this a bit more symmetric
we turn this into a intel_hpd_poll_{enable,disable}() pair.
So we end up with the following transformation:
intel_hpd_poll_init() -> intel_hpd_poll_enable()
lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
.hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()

If we really would like to prevent all *long* HPD processing during
display resume we'd need some kind of software mechanism to simply
ignore all long HPDs. Currently we appear to have that just for
fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
right all the time I guess that's mostly sufficient.

For a bit of history on this, we first got a mechanism to block
hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
enable irqs earlier when resuming") on account of moving the irq enable
earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
Fix hpd vs. initial config races") because the fdev initial config
got pushed to a later point. The second ad-hoc hpd_irq_setup() for
resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
support (v0.7)") to be able to do MST sideband during the resume.
And finally we got a partial resurrection of the hpd blocking
mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
processing during suspend"), but this time it only prevent fbdev
from handling hpd while resuming.

v2: Leave the poll_init_work behind

Cc: Lyude Paul <lyude@redhat.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
 .../drm/i915/display/intel_display_power.c    |  3 +-
 drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
 drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
 5 files changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 907e1d155443..0d5607ae97c4 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5036,18 +5036,15 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 		intel_pps_unlock_regs_wa(dev_priv);
 		intel_modeset_init_hw(dev_priv);
 		intel_init_clock_gating(dev_priv);
-
-		spin_lock_irq(&dev_priv->irq_lock);
-		if (dev_priv->display.hpd_irq_setup)
-			dev_priv->display.hpd_irq_setup(dev_priv);
-		spin_unlock_irq(&dev_priv->irq_lock);
+		intel_hpd_init(dev_priv);
+		intel_hpd_poll_disable(dev_priv);
 
 		ret = __intel_display_resume(dev, state, ctx);
 		if (ret)
 			drm_err(&dev_priv->drm,
 				"Restoring old state failed with %i\n", ret);
 
-		intel_hpd_init(dev_priv);
+		intel_hpd_poll_disable(dev_priv);
 	}
 
 	drm_atomic_state_put(state);
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 7277e58b01f1..20ddc54298cb 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1424,6 +1424,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
 		return;
 
 	intel_hpd_init(dev_priv);
+	intel_hpd_poll_disable(dev_priv);
 
 	/* Re-enable the ADPA, if we have one */
 	for_each_intel_encoder(&dev_priv->drm, encoder) {
@@ -1449,7 +1450,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
 
 	/* Prevent us from re-enabling polling on accident in late suspend */
 	if (!dev_priv->drm.dev->power.is_suspended)
-		intel_hpd_poll_init(dev_priv);
+		intel_hpd_poll_enable(dev_priv);
 }
 
 static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 5c58c1ed6493..30bd4c86d146 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -584,7 +584,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
  * This is a separate step from interrupt enabling to simplify the locking rules
  * in the driver load and resume code.
  *
- * Also see: intel_hpd_poll_init(), which enables connector polling
+ * Also see: intel_hpd_poll_enable() and intel_hpd_poll_disable().
  */
 void intel_hpd_init(struct drm_i915_private *dev_priv)
 {
@@ -595,9 +595,6 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
 		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
 	}
 
-	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
-	schedule_work(&dev_priv->hotplug.poll_init_work);
-
 	/*
 	 * Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked checks happy.
@@ -654,12 +651,12 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
 }
 
 /**
- * intel_hpd_poll_init - enables/disables polling for connectors with hpd
+ * intel_hpd_poll_enable - enable polling for connectors with hpd
  * @dev_priv: i915 device instance
  *
- * This function enables polling for all connectors, regardless of whether or
- * not they support hotplug detection. Under certain conditions HPD may not be
- * functional. On most Intel GPUs, this happens when we enter runtime suspend.
+ * This function enables polling for all connectors which support HPD.
+ * Under certain conditions HPD may not be functional. On most Intel GPUs,
+ * this happens when we enter runtime suspend.
  * On Valleyview and Cherryview systems, this also happens when we shut off all
  * of the powerwells.
  *
@@ -667,9 +664,9 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
  * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
  * worker.
  *
- * Also see: intel_hpd_init(), which restores hpd handling.
+ * Also see: intel_hpd_init() and intel_hpd_poll_disable().
  */
-void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
+void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
 {
 	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
 
@@ -682,6 +679,31 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
 	schedule_work(&dev_priv->hotplug.poll_init_work);
 }
 
+/**
+ * intel_hpd_poll_disable - disable polling for connectors with hpd
+ * @dev_priv: i915 device instance
+ *
+ * This function disables polling for all connectors which support HPD.
+ * Under certain conditions HPD may not be functional. On most Intel GPUs,
+ * this happens when we enter runtime suspend.
+ * On Valleyview and Cherryview systems, this also happens when we shut off all
+ * of the powerwells.
+ *
+ * Since this function can get called in contexts where we're already holding
+ * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
+ * worker.
+ *
+ * Also used during driver init to initialize connector->polled
+ * appropriately for all connectors.
+ *
+ * Also see: intel_hpd_init() and intel_hpd_poll_enable().
+ */
+void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
+{
+	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
+	schedule_work(&dev_priv->hotplug.poll_init_work);
+}
+
 void intel_hpd_init_work(struct drm_i915_private *dev_priv)
 {
 	INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work,
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
index a704d7c94d16..b87e95d606e6 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.h
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
@@ -14,7 +14,8 @@ struct intel_digital_port;
 struct intel_encoder;
 enum port;
 
-void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
+void intel_hpd_poll_enable(struct drm_i915_private *dev_priv);
+void intel_hpd_poll_disable(struct drm_i915_private *dev_priv);
 enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
 					       struct intel_connector *connector);
 void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ebc15066d108..3fc7b996fc48 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1226,26 +1226,15 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	intel_modeset_init_hw(dev_priv);
 	intel_init_clock_gating(dev_priv);
+	intel_hpd_init(dev_priv);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.hpd_irq_setup)
-		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
-
+	/* MST sideband requires HPD interrupts enabled */
 	intel_dp_mst_resume(dev_priv);
-
 	intel_display_resume(dev);
 
+	intel_hpd_poll_disable(dev_priv);
 	drm_kms_helper_poll_enable(dev);
 
-	/*
-	 * ... but also need to make sure that hotplug processing
-	 * doesn't cause havoc. Like in the driver load code we don't
-	 * bother with the tiny race here where we might lose hotplug
-	 * notifications.
-	 * */
-	intel_hpd_init(dev_priv);
-
 	intel_opregion_resume(dev_priv);
 
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
@@ -1557,7 +1546,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	assert_forcewakes_inactive(&dev_priv->uncore);
 
 	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
-		intel_hpd_poll_init(dev_priv);
+		intel_hpd_poll_enable(dev_priv);
 
 	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
 	return 0;
@@ -1602,8 +1591,10 @@ static int intel_runtime_resume(struct device *kdev)
 	 * power well, so hpd is reinitialized from there. For
 	 * everyone else do it here.
 	 */
-	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
+	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
 		intel_hpd_init(dev_priv);
+		intel_hpd_poll_disable(dev_priv);
+	}
 
 	intel_enable_ipc(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] 25+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-07 19:22 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
@ 2020-10-07 22:15   ` Lyude Paul
  2020-10-08  8:19     ` Ville Syrjälä
  2020-10-12 19:36   ` Imre Deak
  2020-10-13 18:11   ` [Intel-gfx] [PATCH v3 " Ville Syrjala
  2 siblings, 1 reply; 25+ messages in thread
From: Lyude Paul @ 2020-10-07 22:15 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Wed, 2020-10-07 at 22:22 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we call .hpd_irq_setup() directly just before display
> resume, and follow it with another call via intel_hpd_init()
> just afterwards. Assuming the hpd pins are marked as enabled
> during the open-coded call these two things do exactly the
> same thing (ie. enable HPD interrupts). Which even makes sense
> since we definitely need working HPD interrupts for MST sideband
> during the display resume.
> 
> So let's nuke the open-coded call and move the intel_hpd_init()
> call earlier. However we need to leave the poll_init_work stuff
> behind after the display resume as that will trigger display
> detection while we're resuming. We don't want that trampling over
> the display resume process. To make this a bit more symmetric
> we turn this into a intel_hpd_poll_{enable,disable}() pair.
> So we end up with the following transformation:
> intel_hpd_poll_init() -> intel_hpd_poll_enable()
> lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
> .hpd_irq_setup()+resume+intel_hpd_init() ->
> intel_hpd_init()+resume+intel_hpd_poll_disable()
> 
> If we really would like to prevent all *long* HPD processing during
> display resume we'd need some kind of software mechanism to simply
> ignore all long HPDs. Currently we appear to have that just for
> fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
> right all the time I guess that's mostly sufficient.
> 
> For a bit of history on this, we first got a mechanism to block
> hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
> enable irqs earlier when resuming") on account of moving the irq enable
> earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
> Fix hpd vs. initial config races") because the fdev initial config
> got pushed to a later point. The second ad-hoc hpd_irq_setup() for
> resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
> support (v0.7)") to be able to do MST sideband during the resume.
> And finally we got a partial resurrection of the hpd blocking
> mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
> processing during suspend"), but this time it only prevent fbdev
> from handling hpd while resuming.
> 
> v2: Leave the poll_init_work behind
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
>  .../drm/i915/display/intel_display_power.c    |  3 +-
>  drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
>  drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
>  5 files changed, 46 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 907e1d155443..0d5607ae97c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5036,18 +5036,15 @@ void intel_finish_reset(struct drm_i915_private
> *dev_priv)
>  		intel_pps_unlock_regs_wa(dev_priv);
>  		intel_modeset_init_hw(dev_priv);
>  		intel_init_clock_gating(dev_priv);
> -
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		if (dev_priv->display.hpd_irq_setup)
> -			dev_priv->display.hpd_irq_setup(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> +		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
>  
>  		ret = __intel_display_resume(dev, state, ctx);
>  		if (ret)
>  			drm_err(&dev_priv->drm,
>  				"Restoring old state failed with %i\n", ret);
>  
> -		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);

Looks like you're calling intel_hpd_poll_disable() twice here, is this
intentional?

>  	}
>  
>  	drm_atomic_state_put(state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 7277e58b01f1..20ddc54298cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1424,6 +1424,7 @@ static void vlv_display_power_well_init(struct
> drm_i915_private *dev_priv)
>  		return;
>  
>  	intel_hpd_init(dev_priv);
> +	intel_hpd_poll_disable(dev_priv);
>  
>  	/* Re-enable the ADPA, if we have one */
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
> @@ -1449,7 +1450,7 @@ static void vlv_display_power_well_deinit(struct
> drm_i915_private *dev_priv)
>  
>  	/* Prevent us from re-enabling polling on accident in late suspend */
>  	if (!dev_priv->drm.dev->power.is_suspended)
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  }
>  
>  static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 5c58c1ed6493..30bd4c86d146 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -584,7 +584,7 @@ void intel_hpd_irq_handler(struct drm_i915_private
> *dev_priv,
>   * This is a separate step from interrupt enabling to simplify the locking
> rules
>   * in the driver load and resume code.
>   *
> - * Also see: intel_hpd_poll_init(), which enables connector polling
> + * Also see: intel_hpd_poll_enable() and intel_hpd_poll_disable().
>   */
>  void intel_hpd_init(struct drm_i915_private *dev_priv)
>  {
> @@ -595,9 +595,6 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>  		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
>  	}
>  
> -	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> -	schedule_work(&dev_priv->hotplug.poll_init_work);
> -
>  	/*
>  	 * Interrupt setup is already guaranteed to be single-threaded, this is
>  	 * just to make the assert_spin_locked checks happy.
> @@ -654,12 +651,12 @@ static void i915_hpd_poll_init_work(struct work_struct
> *work)
>  }
>  
>  /**
> - * intel_hpd_poll_init - enables/disables polling for connectors with hpd
> + * intel_hpd_poll_enable - enable polling for connectors with hpd
>   * @dev_priv: i915 device instance
>   *
> - * This function enables polling for all connectors, regardless of whether or
> - * not they support hotplug detection. Under certain conditions HPD may not
> be
> - * functional. On most Intel GPUs, this happens when we enter runtime
> suspend.
> + * This function enables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
>   * On Valleyview and Cherryview systems, this also happens when we shut off
> all
>   * of the powerwells.
>   *
> @@ -667,9 +664,9 @@ static void i915_hpd_poll_init_work(struct work_struct
> *work)
>   * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
>   * worker.
>   *
> - * Also see: intel_hpd_init(), which restores hpd handling.
> + * Also see: intel_hpd_init() and intel_hpd_poll_disable().
>   */
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
>  {
>  	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
>  
> @@ -682,6 +679,31 @@ void intel_hpd_poll_init(struct drm_i915_private
> *dev_priv)
>  	schedule_work(&dev_priv->hotplug.poll_init_work);
>  }
>  
> +/**
> + * intel_hpd_poll_disable - disable polling for connectors with hpd
> + * @dev_priv: i915 device instance
> + *
> + * This function disables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
> + * On Valleyview and Cherryview systems, this also happens when we shut off
> all
> + * of the powerwells.
> + *
> + * Since this function can get called in contexts where we're already holding
> + * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
> + * worker.
> + *
> + * Also used during driver init to initialize connector->polled
> + * appropriately for all connectors.
> + *
> + * Also see: intel_hpd_init() and intel_hpd_poll_enable().
> + */
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
> +{
> +	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> +	schedule_work(&dev_priv->hotplug.poll_init_work);
> +}
> +
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>  {
>  	INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h
> b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index a704d7c94d16..b87e95d606e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -14,7 +14,8 @@ struct intel_digital_port;
>  struct intel_encoder;
>  enum port;
>  
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv);
>  enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
>  					       struct intel_connector
> *connector);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ebc15066d108..3fc7b996fc48 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1226,26 +1226,15 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	intel_modeset_init_hw(dev_priv);
>  	intel_init_clock_gating(dev_priv);
> +	intel_hpd_init(dev_priv);
>  
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.hpd_irq_setup)
> -		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -
> +	/* MST sideband requires HPD interrupts enabled */
>  	intel_dp_mst_resume(dev_priv);
> -
>  	intel_display_resume(dev);
>  
> +	intel_hpd_poll_disable(dev_priv);
>  	drm_kms_helper_poll_enable(dev);
>  
> -	/*
> -	 * ... but also need to make sure that hotplug processing
> -	 * doesn't cause havoc. Like in the driver load code we don't
> -	 * bother with the tiny race here where we might lose hotplug
> -	 * notifications.
> -	 * */
> -	intel_hpd_init(dev_priv);
> -
>  	intel_opregion_resume(dev_priv);
>  
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> @@ -1557,7 +1546,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  	assert_forcewakes_inactive(&dev_priv->uncore);
>  
>  	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  
>  	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
>  	return 0;
> @@ -1602,8 +1591,10 @@ static int intel_runtime_resume(struct device *kdev)
>  	 * power well, so hpd is reinitialized from there. For
>  	 * everyone else do it here.
>  	 */
> -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
>  		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
> +	}
>  
>  	intel_enable_ipc(dev_priv);
>  
-- 
Sincerely,
      Lyude Paul (she/her)
      Software Engineer at Red Hat

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

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

* Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-07 22:15   ` Lyude Paul
@ 2020-10-08  8:19     ` Ville Syrjälä
  0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2020-10-08  8:19 UTC (permalink / raw)
  To: Lyude Paul; +Cc: intel-gfx

On Wed, Oct 07, 2020 at 06:15:47PM -0400, Lyude Paul wrote:
> On Wed, 2020-10-07 at 22:22 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we call .hpd_irq_setup() directly just before display
> > resume, and follow it with another call via intel_hpd_init()
> > just afterwards. Assuming the hpd pins are marked as enabled
> > during the open-coded call these two things do exactly the
> > same thing (ie. enable HPD interrupts). Which even makes sense
> > since we definitely need working HPD interrupts for MST sideband
> > during the display resume.
> > 
> > So let's nuke the open-coded call and move the intel_hpd_init()
> > call earlier. However we need to leave the poll_init_work stuff
> > behind after the display resume as that will trigger display
> > detection while we're resuming. We don't want that trampling over
> > the display resume process. To make this a bit more symmetric
> > we turn this into a intel_hpd_poll_{enable,disable}() pair.
> > So we end up with the following transformation:
> > intel_hpd_poll_init() -> intel_hpd_poll_enable()
> > lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
> > .hpd_irq_setup()+resume+intel_hpd_init() ->
> > intel_hpd_init()+resume+intel_hpd_poll_disable()
> > 
> > If we really would like to prevent all *long* HPD processing during
> > display resume we'd need some kind of software mechanism to simply
> > ignore all long HPDs. Currently we appear to have that just for
> > fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
> > right all the time I guess that's mostly sufficient.
> > 
> > For a bit of history on this, we first got a mechanism to block
> > hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
> > enable irqs earlier when resuming") on account of moving the irq enable
> > earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
> > Fix hpd vs. initial config races") because the fdev initial config
> > got pushed to a later point. The second ad-hoc hpd_irq_setup() for
> > resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
> > support (v0.7)") to be able to do MST sideband during the resume.
> > And finally we got a partial resurrection of the hpd blocking
> > mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
> > processing during suspend"), but this time it only prevent fbdev
> > from handling hpd while resuming.
> > 
> > v2: Leave the poll_init_work behind
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
> >  .../drm/i915/display/intel_display_power.c    |  3 +-
> >  drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
> >  drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
> >  drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
> >  5 files changed, 46 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 907e1d155443..0d5607ae97c4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -5036,18 +5036,15 @@ void intel_finish_reset(struct drm_i915_private
> > *dev_priv)
> >  		intel_pps_unlock_regs_wa(dev_priv);
> >  		intel_modeset_init_hw(dev_priv);
> >  		intel_init_clock_gating(dev_priv);
> > -
> > -		spin_lock_irq(&dev_priv->irq_lock);
> > -		if (dev_priv->display.hpd_irq_setup)
> > -			dev_priv->display.hpd_irq_setup(dev_priv);
> > -		spin_unlock_irq(&dev_priv->irq_lock);
> > +		intel_hpd_init(dev_priv);
> > +		intel_hpd_poll_disable(dev_priv);
> >  
> >  		ret = __intel_display_resume(dev, state, ctx);
> >  		if (ret)
> >  			drm_err(&dev_priv->drm,
> >  				"Restoring old state failed with %i\n", ret);
> >  
> > -		intel_hpd_init(dev_priv);
> > +		intel_hpd_poll_disable(dev_priv);
> 
> Looks like you're calling intel_hpd_poll_disable() twice here, is this
> intentional?

No, just a failure to follow my own rules. Thanks for spotting it.

-- 
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] 25+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev2)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (4 preceding siblings ...)
  2020-10-07 19:22 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
@ 2020-10-08 11:39 ` Patchwork
  2020-10-08 12:00 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-08 11:39 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev2)
URL   : https://patchwork.freedesktop.org/series/82417/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9bfa4d97c74e drm/i915: Reorder hpd init vs. display resume
-:26: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#26: 
.hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()

-:166: WARNING:TYPO_SPELLING: 'seperate' may be misspelled - perhaps 'separate'?
#166: FILE: drivers/gpu/drm/i915/display/intel_hotplug.c:693:
+ * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate

total: 0 errors, 2 warnings, 0 checks, 168 lines checked
f5532005bb88 drm/i915: Do drm_mode_config_reset() after HPD init
2bcc60547bc5 drm/i915: Refactor .hpd_irq_setup() calls a bit


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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev2)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (5 preceding siblings ...)
  2020-10-08 11:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev2) Patchwork
@ 2020-10-08 12:00 ` Patchwork
  2020-10-08 17:02 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3) Patchwork
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-08 12:00 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


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

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev2)
URL   : https://patchwork.freedesktop.org/series/82417/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9111 -> Patchwork_18650
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

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

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - {fi-tgl-dsi}:       [DMESG-WARN][3] ([i915#1982] / [k.org#205379]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9111/fi-tgl-dsi/igt@i915_module_load@reload.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18650/fi-tgl-dsi/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [DMESG-WARN][5] ([i915#1982]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9111/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18650/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.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
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (15 -> 12)
------------------------------

  Missing    (3): fi-byt-clapper fi-byt-squawks fi-bsw-cyan 


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

  * Linux: CI_DRM_9111 -> Patchwork_18650

  CI-20190529: 20190529
  CI_DRM_9111: 4ebfe1a6dfa98f91aeec86210071e9d9034ffbef @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5804: a15f8da169c7ab32db77aca7ae2b26c616c9edef @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18650: 2bcc60547bc5b60392c3d6040c702e4fa5363c76 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2bcc60547bc5 drm/i915: Refactor .hpd_irq_setup() calls a bit
f5532005bb88 drm/i915: Do drm_mode_config_reset() after HPD init
9bfa4d97c74e drm/i915: Reorder hpd init vs. display resume

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 3636 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] 25+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (6 preceding siblings ...)
  2020-10-08 12:00 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-10-08 17:02 ` Patchwork
  2020-10-08 17:24 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-08 17:02 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3)
URL   : https://patchwork.freedesktop.org/series/82417/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
f5192fdf6756 drm/i915: Reorder hpd init vs. display resume
-:26: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#26: 
.hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()

-:166: WARNING:TYPO_SPELLING: 'seperate' may be misspelled - perhaps 'separate'?
#166: FILE: drivers/gpu/drm/i915/display/intel_hotplug.c:693:
+ * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate

total: 0 errors, 2 warnings, 0 checks, 168 lines checked
d3ea7f9670c6 drm/i915: Do drm_mode_config_reset() after HPD init
90e7c6ff4798 drm/i915: Refactor .hpd_irq_setup() calls a bit


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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (7 preceding siblings ...)
  2020-10-08 17:02 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3) Patchwork
@ 2020-10-08 17:24 ` Patchwork
  2020-10-08 19:06 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-08 17:24 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


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

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3)
URL   : https://patchwork.freedesktop.org/series/82417/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9113 -> Patchwork_18657
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

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

  
#### Possible fixes ####

  * {igt@core_hotunplug@unbind-rebind}:
    - fi-icl-y:           [DMESG-WARN][3] ([i915#1982]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-icl-y/igt@core_hotunplug@unbind-rebind.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-icl-y/igt@core_hotunplug@unbind-rebind.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-skl-lmem:        [INCOMPLETE][5] ([i915#198]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-skl-lmem/igt@gem_exec_suspend@basic-s3.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-skl-lmem/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_busy@basic@flip:
    - {fi-tgl-dsi}:       [DMESG-WARN][7] ([i915#1982]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-tgl-dsi/igt@kms_busy@basic@flip.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-tgl-dsi/igt@kms_busy@basic@flip.html
    - fi-kbl-x1275:       [DMESG-WARN][9] ([i915#62] / [i915#92] / [i915#95]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-kbl-x1275/igt@kms_busy@basic@flip.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-kbl-x1275/igt@kms_busy@basic@flip.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-icl-u2:          [DMESG-WARN][11] ([i915#1982]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@vgem_basic@unload:
    - fi-skl-guc:         [DMESG-WARN][13] ([i915#2203]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-skl-guc/igt@vgem_basic@unload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-skl-guc/igt@vgem_basic@unload.html
    - fi-kbl-x1275:       [DMESG-WARN][15] ([i915#62] / [i915#92]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-kbl-x1275/igt@vgem_basic@unload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-kbl-x1275/igt@vgem_basic@unload.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([i915#62] / [i915#92]) -> [DMESG-WARN][18] ([i915#1982] / [i915#62] / [i915#92])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html

  * 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_9113/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_flip@basic-flip-vs-modeset@a-dp1:
    - fi-kbl-x1275:       [DMESG-WARN][21] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][22] ([i915#62] / [i915#92]) +5 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset@a-dp1.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset@a-dp1.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c:
    - fi-kbl-x1275:       [DMESG-WARN][23] ([i915#62] / [i915#92]) -> [DMESG-WARN][24] ([i915#62] / [i915#92] / [i915#95]) +5 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/fi-kbl-x1275/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-c.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/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#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [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 (45 -> 39)
------------------------------

  Additional (1): fi-cfl-8109u 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_9113 -> Patchwork_18657

  CI-20190529: 20190529
  CI_DRM_9113: 412ff15f2b9a97bd0ab32f562ecb7efc84837881 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5805: 9ce50ffed89a46fa1bc98ee2cfe2271c49801079 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18657: 90e7c6ff4798e69f07c568b8269a49b0c193d4fc @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

90e7c6ff4798 drm/i915: Refactor .hpd_irq_setup() calls a bit
d3ea7f9670c6 drm/i915: Do drm_mode_config_reset() after HPD init
f5192fdf6756 drm/i915: Reorder hpd init vs. display resume

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 8785 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] 25+ messages in thread

* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (8 preceding siblings ...)
  2020-10-08 17:24 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-10-08 19:06 ` Patchwork
  2020-10-13 18:19 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4) Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-08 19:06 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


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

== Series Details ==

Series: series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3)
URL   : https://patchwork.freedesktop.org/series/82417/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9113_full -> Patchwork_18657_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_18657_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18657_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_18657_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup@uc:
    - shard-hsw:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-hsw8/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup@uc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup@uc.html

  * igt@gem_wait@write-wait@vcs0:
    - shard-snb:          NOTRUN -> [FAIL][3] +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-snb7/igt@gem_wait@write-wait@vcs0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_gttfill@engines@rcs0:
    - shard-glk:          [PASS][4] -> [DMESG-WARN][5] ([i915#118] / [i915#95])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-glk7/igt@gem_exec_gttfill@engines@rcs0.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-glk6/igt@gem_exec_gttfill@engines@rcs0.html

  * igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup@wc:
    - shard-hsw:          [PASS][6] -> [FAIL][7] ([i915#1888]) +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-hsw8/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup@wc.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-hsw6/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy-gup@wc.html

  * igt@gem_userptr_blits@unsync-unmap-cycles:
    - shard-skl:          [PASS][8] -> [TIMEOUT][9] ([i915#2424])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl7/igt@gem_userptr_blits@unsync-unmap-cycles.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl1/igt@gem_userptr_blits@unsync-unmap-cycles.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-skl:          [PASS][10] -> [DMESG-WARN][11] ([i915#1436] / [i915#716])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl2/igt@gen9_exec_parse@allowed-single.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl7/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-skl:          [PASS][12] -> [FAIL][13] ([i915#454])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl7/igt@i915_pm_dc@dc6-psr.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl1/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_cursor_edge_walk@pipe-b-64x64-left-edge:
    - shard-glk:          [PASS][14] -> [DMESG-WARN][15] ([i915#1982])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-glk6/igt@kms_cursor_edge_walk@pipe-b-64x64-left-edge.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-glk1/igt@kms_cursor_edge_walk@pipe-b-64x64-left-edge.html

  * igt@kms_flip@2x-flip-vs-panning@ab-vga1-hdmi-a1:
    - shard-hsw:          [PASS][16] -> [DMESG-WARN][17] ([i915#1982]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-hsw2/igt@kms_flip@2x-flip-vs-panning@ab-vga1-hdmi-a1.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-hsw6/igt@kms_flip@2x-flip-vs-panning@ab-vga1-hdmi-a1.html

  * igt@kms_flip@flip-vs-blocking-wf-vblank@a-edp1:
    - shard-skl:          [PASS][18] -> [DMESG-WARN][19] ([i915#1982]) +3 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl7/igt@kms_flip@flip-vs-blocking-wf-vblank@a-edp1.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl7/igt@kms_flip@flip-vs-blocking-wf-vblank@a-edp1.html

  * igt@kms_flip@flip-vs-suspend@a-dp1:
    - shard-kbl:          [PASS][20] -> [INCOMPLETE][21] ([i915#155])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-kbl7/igt@kms_flip@flip-vs-suspend@a-dp1.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-kbl2/igt@kms_flip@flip-vs-suspend@a-dp1.html

  * igt@kms_flip@flip-vs-suspend@b-hdmi-a1:
    - shard-hsw:          [PASS][22] -> [INCOMPLETE][23] ([i915#2055])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-hsw1/igt@kms_flip@flip-vs-suspend@b-hdmi-a1.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-hsw6/igt@kms_flip@flip-vs-suspend@b-hdmi-a1.html

  * igt@kms_flip@plain-flip-ts-check-interruptible@c-edp1:
    - shard-skl:          [PASS][24] -> [FAIL][25] ([i915#2122]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl3/igt@kms_flip@plain-flip-ts-check-interruptible@c-edp1.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl3/igt@kms_flip@plain-flip-ts-check-interruptible@c-edp1.html

  * igt@kms_frontbuffer_tracking@psr-slowdraw:
    - shard-tglb:         [PASS][26] -> [DMESG-WARN][27] ([i915#1982])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-tglb1/igt@kms_frontbuffer_tracking@psr-slowdraw.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-tglb1/igt@kms_frontbuffer_tracking@psr-slowdraw.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][28] -> [FAIL][29] ([i915#1188])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl3/igt@kms_hdr@bpc-switch-suspend.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl3/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][30] -> [FAIL][31] ([fdo#108145] / [i915#265])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl9/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl5/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping:
    - shard-iclb:         [PASS][32] -> [DMESG-WARN][33] ([i915#1982])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-iclb7/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-iclb3/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [PASS][34] -> [SKIP][35] ([fdo#109441]) +2 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-iclb6/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@kms_setmode@basic:
    - shard-glk:          [PASS][36] -> [FAIL][37] ([i915#31])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-glk4/igt@kms_setmode@basic.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-glk8/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * {igt@gem_exec_capture@pi@rcs0}:
    - shard-skl:          [INCOMPLETE][38] -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl2/igt@gem_exec_capture@pi@rcs0.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl3/igt@gem_exec_capture@pi@rcs0.html
    - shard-glk:          [INCOMPLETE][40] -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-glk1/igt@gem_exec_capture@pi@rcs0.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-glk4/igt@gem_exec_capture@pi@rcs0.html

  * igt@gem_mmap_gtt@basic-small-bo:
    - shard-hsw:          [INCOMPLETE][42] -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-hsw2/igt@gem_mmap_gtt@basic-small-bo.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-hsw2/igt@gem_mmap_gtt@basic-small-bo.html

  * igt@i915_pm_rpm@system-suspend:
    - shard-skl:          [INCOMPLETE][44] ([i915#151]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl1/igt@i915_pm_rpm@system-suspend.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl4/igt@i915_pm_rpm@system-suspend.html

  * igt@i915_selftest@live@gt_heartbeat:
    - shard-skl:          [DMESG-FAIL][46] ([i915#541]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl3/igt@i915_selftest@live@gt_heartbeat.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl6/igt@i915_selftest@live@gt_heartbeat.html

  * {igt@kms_async_flips@async-flip-with-page-flip-events}:
    - shard-glk:          [FAIL][48] ([i915#2521]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-glk3/igt@kms_async_flips@async-flip-with-page-flip-events.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-glk4/igt@kms_async_flips@async-flip-with-page-flip-events.html

  * igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge:
    - shard-glk:          [DMESG-WARN][50] ([i915#1982]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-glk1/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-glk2/igt@kms_cursor_edge_walk@pipe-b-128x128-bottom-edge.html

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-hsw:          [FAIL][52] ([i915#2370]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-hsw8/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-hsw1/igt@kms_cursor_legacy@cursor-vs-flip-varying-size.html

  * igt@kms_flip@2x-dpms-vs-vblank-race@ab-vga1-hdmi-a1:
    - shard-hsw:          [DMESG-WARN][54] ([i915#1982]) -> [PASS][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-hsw6/igt@kms_flip@2x-dpms-vs-vblank-race@ab-vga1-hdmi-a1.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-hsw2/igt@kms_flip@2x-dpms-vs-vblank-race@ab-vga1-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@fbc-farfromfence:
    - shard-glk:          [FAIL][56] ([i915#49]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-glk8/igt@kms_frontbuffer_tracking@fbc-farfromfence.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-glk7/igt@kms_frontbuffer_tracking@fbc-farfromfence.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-tglb:         [DMESG-WARN][58] ([i915#1982]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-skl:          [DMESG-WARN][60] ([i915#1982]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl8/igt@kms_plane@plane-position-covered-pipe-b-planes.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl7/igt@kms_plane@plane-position-covered-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][62] ([fdo#108145] / [i915#265]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][64] ([fdo#109441]) -> [PASS][65] +2 similar issues
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-iclb1/igt@kms_psr@psr2_cursor_render.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@prime_vgem@sync@rcs0:
    - shard-iclb:         [INCOMPLETE][66] ([i915#409]) -> [PASS][67]
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-iclb3/igt@prime_vgem@sync@rcs0.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-iclb2/igt@prime_vgem@sync@rcs0.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc5-psr:
    - shard-tglb:         [DMESG-WARN][68] ([i915#2411]) -> [FAIL][69] ([i915#1899])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-tglb6/igt@i915_pm_dc@dc5-psr.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-tglb7/igt@i915_pm_dc@dc5-psr.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [WARN][70] ([i915#1515]) -> [FAIL][71] ([i915#1515])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-iclb3/igt@i915_pm_rc6_residency@rc6-idle.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-iclb2/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_content_protection@legacy:
    - shard-apl:          [FAIL][72] ([fdo#110321] / [fdo#110336] / [i915#1635]) -> [TIMEOUT][73] ([i915#1319] / [i915#1635])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-apl4/igt@kms_content_protection@legacy.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-apl7/igt@kms_content_protection@legacy.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          [DMESG-WARN][74] ([i915#1982]) -> [DMESG-FAIL][75] ([i915#1982])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@runner@aborted:
    - shard-skl:          [FAIL][76] ([i915#2439]) -> [FAIL][77] ([i915#1436])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9113/shard-skl2/igt@runner@aborted.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18657/shard-skl7/igt@runner@aborted.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#151]: https://gitlab.freedesktop.org/drm/intel/issues/151
  [i915#1515]: https://gitlab.freedesktop.org/drm/intel/issues/1515
  [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#1899]: https://gitlab.freedesktop.org/drm/intel/issues/1899
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2055]: https://gitlab.freedesktop.org/drm/intel/issues/2055
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2370]: https://gitlab.freedesktop.org/drm/intel/issues/2370
  [i915#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#2424]: https://gitlab.freedesktop.org/drm/intel/issues/2424
  [i915#2439]: https://gitlab.freedesktop.org/drm/intel/issues/2439
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#409]: https://gitlab.freedesktop.org/drm/intel/issues/409
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


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

  No changes in participating hosts


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

  * Linux: CI_DRM_9113 -> Patchwork_18657

  CI-20190529: 20190529
  CI_DRM_9113: 412ff15f2b9a97bd0ab32f562ecb7efc84837881 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5805: 9ce50ffed89a46fa1bc98ee2cfe2271c49801079 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18657: 90e7c6ff4798e69f07c568b8269a49b0c193d4fc @ 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_18657/index.html

[-- Attachment #1.2: Type: text/html, Size: 20696 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] 25+ messages in thread

* Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-07 19:22 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
  2020-10-07 22:15   ` Lyude Paul
@ 2020-10-12 19:36   ` Imre Deak
  2020-10-13 13:39     ` Ville Syrjälä
  2020-10-13 18:11   ` [Intel-gfx] [PATCH v3 " Ville Syrjala
  2 siblings, 1 reply; 25+ messages in thread
From: Imre Deak @ 2020-10-12 19:36 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Wed, Oct 07, 2020 at 10:22:41PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we call .hpd_irq_setup() directly just before display
> resume, and follow it with another call via intel_hpd_init()
> just afterwards. Assuming the hpd pins are marked as enabled
> during the open-coded call these two things do exactly the
> same thing (ie. enable HPD interrupts). Which even makes sense
> since we definitely need working HPD interrupts for MST sideband
> during the display resume.
> 
> So let's nuke the open-coded call and move the intel_hpd_init()
> call earlier. However we need to leave the poll_init_work stuff
> behind after the display resume as that will trigger display
> detection while we're resuming. We don't want that trampling over
> the display resume process. To make this a bit more symmetric
> we turn this into a intel_hpd_poll_{enable,disable}() pair.
> So we end up with the following transformation:
> intel_hpd_poll_init() -> intel_hpd_poll_enable()
> lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
> .hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()
> 
> If we really would like to prevent all *long* HPD processing during
> display resume we'd need some kind of software mechanism to simply
> ignore all long HPDs. Currently we appear to have that just for
> fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
> right all the time I guess that's mostly sufficient.
> 
> For a bit of history on this, we first got a mechanism to block
> hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
> enable irqs earlier when resuming") on account of moving the irq enable
> earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
> Fix hpd vs. initial config races") because the fdev initial config
> got pushed to a later point. The second ad-hoc hpd_irq_setup() for
> resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
> support (v0.7)") to be able to do MST sideband during the resume.
> And finally we got a partial resurrection of the hpd blocking
> mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
> processing during suspend"), but this time it only prevent fbdev
> from handling hpd while resuming.
> 
> v2: Leave the poll_init_work behind
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
>  .../drm/i915/display/intel_display_power.c    |  3 +-
>  drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
>  drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
>  5 files changed, 46 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 907e1d155443..0d5607ae97c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5036,18 +5036,15 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
>  		intel_pps_unlock_regs_wa(dev_priv);
>  		intel_modeset_init_hw(dev_priv);
>  		intel_init_clock_gating(dev_priv);
> -
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		if (dev_priv->display.hpd_irq_setup)
> -			dev_priv->display.hpd_irq_setup(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> +		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
>  
>  		ret = __intel_display_resume(dev, state, ctx);
>  		if (ret)
>  			drm_err(&dev_priv->drm,
>  				"Restoring old state failed with %i\n", ret);
>  
> -		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);

This call is the needed one (to re-probe the connectors) and the above
call is not.

>  	}
>  
>  	drm_atomic_state_put(state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 7277e58b01f1..20ddc54298cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1424,6 +1424,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>  		return;
>  
>  	intel_hpd_init(dev_priv);
> +	intel_hpd_poll_disable(dev_priv);
>  
>  	/* Re-enable the ADPA, if we have one */
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
> @@ -1449,7 +1450,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
>  
>  	/* Prevent us from re-enabling polling on accident in late suspend */
>  	if (!dev_priv->drm.dev->power.is_suspended)
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  }
>  
>  static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 5c58c1ed6493..30bd4c86d146 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -584,7 +584,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>   * This is a separate step from interrupt enabling to simplify the locking rules
>   * in the driver load and resume code.
>   *
> - * Also see: intel_hpd_poll_init(), which enables connector polling
> + * Also see: intel_hpd_poll_enable() and intel_hpd_poll_disable().
>   */
>  void intel_hpd_init(struct drm_i915_private *dev_priv)
>  {
> @@ -595,9 +595,6 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>  		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
>  	}
>  
> -	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> -	schedule_work(&dev_priv->hotplug.poll_init_work);
> -
>  	/*
>  	 * Interrupt setup is already guaranteed to be single-threaded, this is
>  	 * just to make the assert_spin_locked checks happy.
> @@ -654,12 +651,12 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
>  }
>  
>  /**
> - * intel_hpd_poll_init - enables/disables polling for connectors with hpd
> + * intel_hpd_poll_enable - enable polling for connectors with hpd
>   * @dev_priv: i915 device instance
>   *
> - * This function enables polling for all connectors, regardless of whether or
> - * not they support hotplug detection. Under certain conditions HPD may not be
> - * functional. On most Intel GPUs, this happens when we enter runtime suspend.
> + * This function enables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
>   * On Valleyview and Cherryview systems, this also happens when we shut off all
>   * of the powerwells.
>   *
> @@ -667,9 +664,9 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
>   * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
>   * worker.
>   *
> - * Also see: intel_hpd_init(), which restores hpd handling.
> + * Also see: intel_hpd_init() and intel_hpd_poll_disable().
>   */
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
>  {
>  	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
>  
> @@ -682,6 +679,31 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
>  	schedule_work(&dev_priv->hotplug.poll_init_work);
>  }
>  
> +/**
> + * intel_hpd_poll_disable - disable polling for connectors with hpd
> + * @dev_priv: i915 device instance
> + *
> + * This function disables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
> + * On Valleyview and Cherryview systems, this also happens when we shut off all
> + * of the powerwells.
> + *
> + * Since this function can get called in contexts where we're already holding
> + * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
> + * worker.
> + *
> + * Also used during driver init to initialize connector->polled
> + * appropriately for all connectors.
> + *
> + * Also see: intel_hpd_init() and intel_hpd_poll_enable().
> + */
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
> +{
> +	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> +	schedule_work(&dev_priv->hotplug.poll_init_work);
> +}
> +
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>  {
>  	INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index a704d7c94d16..b87e95d606e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -14,7 +14,8 @@ struct intel_digital_port;
>  struct intel_encoder;
>  enum port;
>  
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv);
>  enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
>  					       struct intel_connector *connector);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ebc15066d108..3fc7b996fc48 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1226,26 +1226,15 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	intel_modeset_init_hw(dev_priv);
>  	intel_init_clock_gating(dev_priv);
> +	intel_hpd_init(dev_priv);
>  
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.hpd_irq_setup)
> -		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -
> +	/* MST sideband requires HPD interrupts enabled */

The above is a comment for intel_hpd_init().

intel_modeset_init() also calls intel_hpd_init(), looks like that
guarantees the explicit connector probing during driver loading. Do we
need to call intel_hpd_poll_disable() somewhere on that path too? (Maybe
only from i915_driver_register().)

>  	intel_dp_mst_resume(dev_priv);
> -
>  	intel_display_resume(dev);
>  
> +	intel_hpd_poll_disable(dev_priv);
>  	drm_kms_helper_poll_enable(dev);
>  
> -	/*
> -	 * ... but also need to make sure that hotplug processing
> -	 * doesn't cause havoc. Like in the driver load code we don't
> -	 * bother with the tiny race here where we might lose hotplug
> -	 * notifications.
> -	 * */
> -	intel_hpd_init(dev_priv);
> -
>  	intel_opregion_resume(dev_priv);
>  
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> @@ -1557,7 +1546,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  	assert_forcewakes_inactive(&dev_priv->uncore);
>  
>  	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  
>  	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
>  	return 0;
> @@ -1602,8 +1591,10 @@ static int intel_runtime_resume(struct device *kdev)
>  	 * power well, so hpd is reinitialized from there. For
>  	 * everyone else do it here.
>  	 */
> -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
>  		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
> +	}
>  
>  	intel_enable_ipc(dev_priv);
>  
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init Ville Syrjala
@ 2020-10-12 20:16   ` Imre Deak
  2020-10-19 15:39   ` Imre Deak
  1 sibling, 0 replies; 25+ messages in thread
From: Imre Deak @ 2020-10-12 20:16 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Tue, Oct 06, 2020 at 09:58:08PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> LSPCON requires HPD detection logic to be enabled when we call
> its .reset() hook during resume, to check the live state of the
> pin. To that end let's reorder the resume sequence such that
> we do HPD init before the encoder reset.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b2a050d916e3..66ddd4161885 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1213,21 +1213,20 @@ static int i915_drm_resume(struct drm_device *dev)
>  	 * GPU will hang. i915_gem_init_hw() will initiate batches to
>  	 * update/restore the context.
>  	 *
> -	 * drm_mode_config_reset() needs AUX interrupts.
> -	 *
>  	 * Modeset enabling in intel_modeset_init_hw() also needs working
>  	 * interrupts.
>  	 */
>  	intel_runtime_pm_enable_interrupts(dev_priv);
>  
> -	drm_mode_config_reset(dev);
> -
>  	i915_gem_resume(dev_priv);
>  
>  	intel_modeset_init_hw(dev_priv);
>  	intel_init_clock_gating(dev_priv);
>  	intel_hpd_init(dev_priv);
>  
> +	/* May need AUX interrupts and HPD detection enabled */
> +	drm_mode_config_reset(dev);

lspcon_resume() could be called both by the above and during connector
detection. Delaying hotplug detection until after intel_display_resume()
would be the ideal solution, but until that we could avoid this race by
taking connection_mutex.

> +
>  	/* MST sideband requires HPD interrupts enabled */
>  	intel_dp_mst_resume(dev_priv);
>  	intel_display_resume(dev);
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 3/3] drm/i915: Refactor .hpd_irq_setup() calls a bit
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 3/3] drm/i915: Refactor .hpd_irq_setup() calls a bit Ville Syrjala
@ 2020-10-12 20:22   ` Imre Deak
  0 siblings, 0 replies; 25+ messages in thread
From: Imre Deak @ 2020-10-12 20:22 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Tue, Oct 06, 2020 at 09:58:09PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add a small wrapper for .hpd_irq_setup() which does the
> "do we even have the hook?" and "are display interrupts enabled?"
> checks.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_hotplug.c | 22 +++++++++++---------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 5c58c1ed6493..6110d71b4f14 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -213,6 +213,12 @@ intel_hpd_irq_storm_switch_to_polling(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> +static void intel_hpd_irq_setup(struct drm_i915_private *i915)
> +{
> +	if (i915->display_irqs_enabled && i915->display.hpd_irq_setup)
> +		i915->display.hpd_irq_setup(i915);
> +}
> +
>  static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  {
>  	struct drm_i915_private *dev_priv =
> @@ -248,8 +254,7 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work)
>  			dev_priv->hotplug.stats[pin].state = HPD_ENABLED;
>  	}
>  
> -	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup)
> -		dev_priv->display.hpd_irq_setup(dev_priv);
> +	intel_hpd_irq_setup(dev_priv);
>  
>  	spin_unlock_irq(&dev_priv->irq_lock);
>  
> @@ -556,8 +561,8 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>  	 * Disable any IRQs that storms were detected on. Polling enablement
>  	 * happens later in our hotplug work.
>  	 */
> -	if (storm_detected && dev_priv->display_irqs_enabled)
> -		dev_priv->display.hpd_irq_setup(dev_priv);
> +	if (storm_detected)
> +		intel_hpd_irq_setup(dev_priv);
>  	spin_unlock(&dev_priv->irq_lock);
>  
>  	/*
> @@ -602,12 +607,9 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>  	 * Interrupt setup is already guaranteed to be single-threaded, this is
>  	 * just to make the assert_spin_locked checks happy.
>  	 */
> -	if (dev_priv->display_irqs_enabled && dev_priv->display.hpd_irq_setup) {
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		if (dev_priv->display_irqs_enabled)
> -			dev_priv->display.hpd_irq_setup(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> -	}
> +	spin_lock_irq(&dev_priv->irq_lock);
> +	intel_hpd_irq_setup(dev_priv);
> +	spin_unlock_irq(&dev_priv->irq_lock);
>  }
>  
>  static void i915_hpd_poll_init_work(struct work_struct *work)
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v2 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-12 19:36   ` Imre Deak
@ 2020-10-13 13:39     ` Ville Syrjälä
  0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2020-10-13 13:39 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Mon, Oct 12, 2020 at 10:36:45PM +0300, Imre Deak wrote:
> On Wed, Oct 07, 2020 at 10:22:41PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Currently we call .hpd_irq_setup() directly just before display
> > resume, and follow it with another call via intel_hpd_init()
> > just afterwards. Assuming the hpd pins are marked as enabled
> > during the open-coded call these two things do exactly the
> > same thing (ie. enable HPD interrupts). Which even makes sense
> > since we definitely need working HPD interrupts for MST sideband
> > during the display resume.
> > 
> > So let's nuke the open-coded call and move the intel_hpd_init()
> > call earlier. However we need to leave the poll_init_work stuff
> > behind after the display resume as that will trigger display
> > detection while we're resuming. We don't want that trampling over
> > the display resume process. To make this a bit more symmetric
> > we turn this into a intel_hpd_poll_{enable,disable}() pair.
> > So we end up with the following transformation:
> > intel_hpd_poll_init() -> intel_hpd_poll_enable()
> > lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
> > .hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()
> > 
> > If we really would like to prevent all *long* HPD processing during
> > display resume we'd need some kind of software mechanism to simply
> > ignore all long HPDs. Currently we appear to have that just for
> > fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
> > right all the time I guess that's mostly sufficient.
> > 
> > For a bit of history on this, we first got a mechanism to block
> > hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
> > enable irqs earlier when resuming") on account of moving the irq enable
> > earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
> > Fix hpd vs. initial config races") because the fdev initial config
> > got pushed to a later point. The second ad-hoc hpd_irq_setup() for
> > resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
> > support (v0.7)") to be able to do MST sideband during the resume.
> > And finally we got a partial resurrection of the hpd blocking
> > mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
> > processing during suspend"), but this time it only prevent fbdev
> > from handling hpd while resuming.
> > 
> > v2: Leave the poll_init_work behind
> > 
> > Cc: Lyude Paul <lyude@redhat.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
> >  .../drm/i915/display/intel_display_power.c    |  3 +-
> >  drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
> >  drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
> >  drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
> >  5 files changed, 46 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 907e1d155443..0d5607ae97c4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -5036,18 +5036,15 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
> >  		intel_pps_unlock_regs_wa(dev_priv);
> >  		intel_modeset_init_hw(dev_priv);
> >  		intel_init_clock_gating(dev_priv);
> > -
> > -		spin_lock_irq(&dev_priv->irq_lock);
> > -		if (dev_priv->display.hpd_irq_setup)
> > -			dev_priv->display.hpd_irq_setup(dev_priv);
> > -		spin_unlock_irq(&dev_priv->irq_lock);
> > +		intel_hpd_init(dev_priv);
> > +		intel_hpd_poll_disable(dev_priv);
> >  
> >  		ret = __intel_display_resume(dev, state, ctx);
> >  		if (ret)
> >  			drm_err(&dev_priv->drm,
> >  				"Restoring old state failed with %i\n", ret);
> >  
> > -		intel_hpd_init(dev_priv);
> > +		intel_hpd_poll_disable(dev_priv);
> 
> This call is the needed one (to re-probe the connectors) and the above
> call is not.
> 
> >  	}
> >  
> >  	drm_atomic_state_put(state);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > index 7277e58b01f1..20ddc54298cb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > @@ -1424,6 +1424,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
> >  		return;
> >  
> >  	intel_hpd_init(dev_priv);
> > +	intel_hpd_poll_disable(dev_priv);
> >  
> >  	/* Re-enable the ADPA, if we have one */
> >  	for_each_intel_encoder(&dev_priv->drm, encoder) {
> > @@ -1449,7 +1450,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
> >  
> >  	/* Prevent us from re-enabling polling on accident in late suspend */
> >  	if (!dev_priv->drm.dev->power.is_suspended)
> > -		intel_hpd_poll_init(dev_priv);
> > +		intel_hpd_poll_enable(dev_priv);
> >  }
> >  
> >  static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > index 5c58c1ed6493..30bd4c86d146 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> > @@ -584,7 +584,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> >   * This is a separate step from interrupt enabling to simplify the locking rules
> >   * in the driver load and resume code.
> >   *
> > - * Also see: intel_hpd_poll_init(), which enables connector polling
> > + * Also see: intel_hpd_poll_enable() and intel_hpd_poll_disable().
> >   */
> >  void intel_hpd_init(struct drm_i915_private *dev_priv)
> >  {
> > @@ -595,9 +595,6 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
> >  		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
> >  	}
> >  
> > -	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> > -	schedule_work(&dev_priv->hotplug.poll_init_work);
> > -
> >  	/*
> >  	 * Interrupt setup is already guaranteed to be single-threaded, this is
> >  	 * just to make the assert_spin_locked checks happy.
> > @@ -654,12 +651,12 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
> >  }
> >  
> >  /**
> > - * intel_hpd_poll_init - enables/disables polling for connectors with hpd
> > + * intel_hpd_poll_enable - enable polling for connectors with hpd
> >   * @dev_priv: i915 device instance
> >   *
> > - * This function enables polling for all connectors, regardless of whether or
> > - * not they support hotplug detection. Under certain conditions HPD may not be
> > - * functional. On most Intel GPUs, this happens when we enter runtime suspend.
> > + * This function enables polling for all connectors which support HPD.
> > + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> > + * this happens when we enter runtime suspend.
> >   * On Valleyview and Cherryview systems, this also happens when we shut off all
> >   * of the powerwells.
> >   *
> > @@ -667,9 +664,9 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
> >   * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
> >   * worker.
> >   *
> > - * Also see: intel_hpd_init(), which restores hpd handling.
> > + * Also see: intel_hpd_init() and intel_hpd_poll_disable().
> >   */
> > -void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
> > +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
> >  {
> >  	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
> >  
> > @@ -682,6 +679,31 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
> >  	schedule_work(&dev_priv->hotplug.poll_init_work);
> >  }
> >  
> > +/**
> > + * intel_hpd_poll_disable - disable polling for connectors with hpd
> > + * @dev_priv: i915 device instance
> > + *
> > + * This function disables polling for all connectors which support HPD.
> > + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> > + * this happens when we enter runtime suspend.
> > + * On Valleyview and Cherryview systems, this also happens when we shut off all
> > + * of the powerwells.
> > + *
> > + * Since this function can get called in contexts where we're already holding
> > + * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
> > + * worker.
> > + *
> > + * Also used during driver init to initialize connector->polled
> > + * appropriately for all connectors.
> > + *
> > + * Also see: intel_hpd_init() and intel_hpd_poll_enable().
> > + */
> > +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
> > +{
> > +	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> > +	schedule_work(&dev_priv->hotplug.poll_init_work);
> > +}
> > +
> >  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
> >  {
> >  	INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work,
> > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > index a704d7c94d16..b87e95d606e6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> > @@ -14,7 +14,8 @@ struct intel_digital_port;
> >  struct intel_encoder;
> >  enum port;
> >  
> > -void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> > +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv);
> > +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv);
> >  enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
> >  					       struct intel_connector *connector);
> >  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index ebc15066d108..3fc7b996fc48 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1226,26 +1226,15 @@ static int i915_drm_resume(struct drm_device *dev)
> >  
> >  	intel_modeset_init_hw(dev_priv);
> >  	intel_init_clock_gating(dev_priv);
> > +	intel_hpd_init(dev_priv);
> >  
> > -	spin_lock_irq(&dev_priv->irq_lock);
> > -	if (dev_priv->display.hpd_irq_setup)
> > -		dev_priv->display.hpd_irq_setup(dev_priv);
> > -	spin_unlock_irq(&dev_priv->irq_lock);
> > -
> > +	/* MST sideband requires HPD interrupts enabled */
> 
> The above is a comment for intel_hpd_init().

I meant it more as "here be the point where we start to depend on HPD
interrupts".

> 
> intel_modeset_init() also calls intel_hpd_init(), looks like that
> guarantees the explicit connector probing during driver loading. Do we
> need to call intel_hpd_poll_disable() somewhere on that path too? (Maybe
> only from i915_driver_register().)

Argh. Must have been some rebase failure. Even reflog remembers me
adding that call, but apparently it never made it into the version I
posted.

> 
> >  	intel_dp_mst_resume(dev_priv);
> > -
> >  	intel_display_resume(dev);
> >  
> > +	intel_hpd_poll_disable(dev_priv);
> >  	drm_kms_helper_poll_enable(dev);
> >  
> > -	/*
> > -	 * ... but also need to make sure that hotplug processing
> > -	 * doesn't cause havoc. Like in the driver load code we don't
> > -	 * bother with the tiny race here where we might lose hotplug
> > -	 * notifications.
> > -	 * */
> > -	intel_hpd_init(dev_priv);
> > -
> >  	intel_opregion_resume(dev_priv);
> >  
> >  	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> > @@ -1557,7 +1546,7 @@ static int intel_runtime_suspend(struct device *kdev)
> >  	assert_forcewakes_inactive(&dev_priv->uncore);
> >  
> >  	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> > -		intel_hpd_poll_init(dev_priv);
> > +		intel_hpd_poll_enable(dev_priv);
> >  
> >  	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
> >  	return 0;
> > @@ -1602,8 +1591,10 @@ static int intel_runtime_resume(struct device *kdev)
> >  	 * power well, so hpd is reinitialized from there. For
> >  	 * everyone else do it here.
> >  	 */
> > -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> > +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
> >  		intel_hpd_init(dev_priv);
> > +		intel_hpd_poll_disable(dev_priv);
> > +	}
> >  
> >  	intel_enable_ipc(dev_priv);
> >  
> > -- 
> > 2.26.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] 25+ messages in thread

* [Intel-gfx] [PATCH v3 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-07 19:22 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
  2020-10-07 22:15   ` Lyude Paul
  2020-10-12 19:36   ` Imre Deak
@ 2020-10-13 18:11   ` Ville Syrjala
  2020-10-19 15:38     ` Imre Deak
  2020-10-19 16:52     ` Lyude Paul
  2 siblings, 2 replies; 25+ messages in thread
From: Ville Syrjala @ 2020-10-13 18:11 UTC (permalink / raw)
  To: intel-gfx

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

Currently we call .hpd_irq_setup() directly just before display
resume, and follow it with another call via intel_hpd_init()
just afterwards. Assuming the hpd pins are marked as enabled
during the open-coded call these two things do exactly the
same thing (ie. enable HPD interrupts). Which even makes sense
since we definitely need working HPD interrupts for MST sideband
during the display resume.

So let's nuke the open-coded call and move the intel_hpd_init()
call earlier. However we need to leave the poll_init_work stuff
behind after the display resume as that will trigger display
detection while we're resuming. We don't want that trampling over
the display resume process. To make this a bit more symmetric
we turn this into a intel_hpd_poll_{enable,disable}() pair.
So we end up with the following transformation:
intel_hpd_poll_init() -> intel_hpd_poll_enable()
lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
.hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()

If we really would like to prevent all *long* HPD processing during
display resume we'd need some kind of software mechanism to simply
ignore all long HPDs. Currently we appear to have that just for
fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
right all the time I guess that's mostly sufficient.

For a bit of history on this, we first got a mechanism to block
hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
enable irqs earlier when resuming") on account of moving the irq enable
earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
Fix hpd vs. initial config races") because the fdev initial config
got pushed to a later point. The second ad-hoc hpd_irq_setup() for
resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
support (v0.7)") to be able to do MST sideband during the resume.
And finally we got a partial resurrection of the hpd blocking
mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
processing during suspend"), but this time it only prevent fbdev
from handling hpd while resuming.

v2: Leave the poll_init_work behind
v3: Remove the extra intel_hpd_poll_disable() from display reset (Lyude)
    Add the missing intel_hpd_poll_disable() to display init (Imre)

Cc: Lyude Paul <lyude@redhat.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
 .../drm/i915/display/intel_display_power.c    |  3 +-
 drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
 drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
 drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
 5 files changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 907e1d155443..3be0d531f96c 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5036,18 +5036,14 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
 		intel_pps_unlock_regs_wa(dev_priv);
 		intel_modeset_init_hw(dev_priv);
 		intel_init_clock_gating(dev_priv);
-
-		spin_lock_irq(&dev_priv->irq_lock);
-		if (dev_priv->display.hpd_irq_setup)
-			dev_priv->display.hpd_irq_setup(dev_priv);
-		spin_unlock_irq(&dev_priv->irq_lock);
+		intel_hpd_init(dev_priv);
 
 		ret = __intel_display_resume(dev, state, ctx);
 		if (ret)
 			drm_err(&dev_priv->drm,
 				"Restoring old state failed with %i\n", ret);
 
-		intel_hpd_init(dev_priv);
+		intel_hpd_poll_disable(dev_priv);
 	}
 
 	drm_atomic_state_put(state);
@@ -18257,6 +18253,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
 
 	/* Only enable hotplug handling once the fbdev is fully set up. */
 	intel_hpd_init(i915);
+	intel_hpd_poll_disable(i915);
 
 	intel_init_ipc(i915);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 7277e58b01f1..20ddc54298cb 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1424,6 +1424,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
 		return;
 
 	intel_hpd_init(dev_priv);
+	intel_hpd_poll_disable(dev_priv);
 
 	/* Re-enable the ADPA, if we have one */
 	for_each_intel_encoder(&dev_priv->drm, encoder) {
@@ -1449,7 +1450,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
 
 	/* Prevent us from re-enabling polling on accident in late suspend */
 	if (!dev_priv->drm.dev->power.is_suspended)
-		intel_hpd_poll_init(dev_priv);
+		intel_hpd_poll_enable(dev_priv);
 }
 
 static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
index 5c58c1ed6493..30bd4c86d146 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
@@ -584,7 +584,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
  * This is a separate step from interrupt enabling to simplify the locking rules
  * in the driver load and resume code.
  *
- * Also see: intel_hpd_poll_init(), which enables connector polling
+ * Also see: intel_hpd_poll_enable() and intel_hpd_poll_disable().
  */
 void intel_hpd_init(struct drm_i915_private *dev_priv)
 {
@@ -595,9 +595,6 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
 		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
 	}
 
-	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
-	schedule_work(&dev_priv->hotplug.poll_init_work);
-
 	/*
 	 * Interrupt setup is already guaranteed to be single-threaded, this is
 	 * just to make the assert_spin_locked checks happy.
@@ -654,12 +651,12 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
 }
 
 /**
- * intel_hpd_poll_init - enables/disables polling for connectors with hpd
+ * intel_hpd_poll_enable - enable polling for connectors with hpd
  * @dev_priv: i915 device instance
  *
- * This function enables polling for all connectors, regardless of whether or
- * not they support hotplug detection. Under certain conditions HPD may not be
- * functional. On most Intel GPUs, this happens when we enter runtime suspend.
+ * This function enables polling for all connectors which support HPD.
+ * Under certain conditions HPD may not be functional. On most Intel GPUs,
+ * this happens when we enter runtime suspend.
  * On Valleyview and Cherryview systems, this also happens when we shut off all
  * of the powerwells.
  *
@@ -667,9 +664,9 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
  * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
  * worker.
  *
- * Also see: intel_hpd_init(), which restores hpd handling.
+ * Also see: intel_hpd_init() and intel_hpd_poll_disable().
  */
-void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
+void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
 {
 	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
 
@@ -682,6 +679,31 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
 	schedule_work(&dev_priv->hotplug.poll_init_work);
 }
 
+/**
+ * intel_hpd_poll_disable - disable polling for connectors with hpd
+ * @dev_priv: i915 device instance
+ *
+ * This function disables polling for all connectors which support HPD.
+ * Under certain conditions HPD may not be functional. On most Intel GPUs,
+ * this happens when we enter runtime suspend.
+ * On Valleyview and Cherryview systems, this also happens when we shut off all
+ * of the powerwells.
+ *
+ * Since this function can get called in contexts where we're already holding
+ * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
+ * worker.
+ *
+ * Also used during driver init to initialize connector->polled
+ * appropriately for all connectors.
+ *
+ * Also see: intel_hpd_init() and intel_hpd_poll_enable().
+ */
+void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
+{
+	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
+	schedule_work(&dev_priv->hotplug.poll_init_work);
+}
+
 void intel_hpd_init_work(struct drm_i915_private *dev_priv)
 {
 	INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work,
diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
index a704d7c94d16..b87e95d606e6 100644
--- a/drivers/gpu/drm/i915/display/intel_hotplug.h
+++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
@@ -14,7 +14,8 @@ struct intel_digital_port;
 struct intel_encoder;
 enum port;
 
-void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
+void intel_hpd_poll_enable(struct drm_i915_private *dev_priv);
+void intel_hpd_poll_disable(struct drm_i915_private *dev_priv);
 enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
 					       struct intel_connector *connector);
 void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ebc15066d108..3fc7b996fc48 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1226,26 +1226,15 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	intel_modeset_init_hw(dev_priv);
 	intel_init_clock_gating(dev_priv);
+	intel_hpd_init(dev_priv);
 
-	spin_lock_irq(&dev_priv->irq_lock);
-	if (dev_priv->display.hpd_irq_setup)
-		dev_priv->display.hpd_irq_setup(dev_priv);
-	spin_unlock_irq(&dev_priv->irq_lock);
-
+	/* MST sideband requires HPD interrupts enabled */
 	intel_dp_mst_resume(dev_priv);
-
 	intel_display_resume(dev);
 
+	intel_hpd_poll_disable(dev_priv);
 	drm_kms_helper_poll_enable(dev);
 
-	/*
-	 * ... but also need to make sure that hotplug processing
-	 * doesn't cause havoc. Like in the driver load code we don't
-	 * bother with the tiny race here where we might lose hotplug
-	 * notifications.
-	 * */
-	intel_hpd_init(dev_priv);
-
 	intel_opregion_resume(dev_priv);
 
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
@@ -1557,7 +1546,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	assert_forcewakes_inactive(&dev_priv->uncore);
 
 	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
-		intel_hpd_poll_init(dev_priv);
+		intel_hpd_poll_enable(dev_priv);
 
 	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
 	return 0;
@@ -1602,8 +1591,10 @@ static int intel_runtime_resume(struct device *kdev)
 	 * power well, so hpd is reinitialized from there. For
 	 * everyone else do it here.
 	 */
-	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
+	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
 		intel_hpd_init(dev_priv);
+		intel_hpd_poll_disable(dev_priv);
+	}
 
 	intel_enable_ipc(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] 25+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (9 preceding siblings ...)
  2020-10-08 19:06 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2020-10-13 18:19 ` Patchwork
  2020-10-13 18:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2020-10-14 14:14 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-13 18:19 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4)
URL   : https://patchwork.freedesktop.org/series/82417/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
706cef4f9041 drm/i915: Reorder hpd init vs. display resume
-:26: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#26: 
.hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()

-:176: WARNING:TYPO_SPELLING: 'seperate' may be misspelled - perhaps 'separate'?
#176: FILE: drivers/gpu/drm/i915/display/intel_hotplug.c:693:
+ * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate

total: 0 errors, 2 warnings, 0 checks, 174 lines checked
ff2305070287 drm/i915: Do drm_mode_config_reset() after HPD init
44633a798306 drm/i915: Refactor .hpd_irq_setup() calls a bit


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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (10 preceding siblings ...)
  2020-10-13 18:19 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4) Patchwork
@ 2020-10-13 18:45 ` Patchwork
  2020-10-14 14:14 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-13 18:45 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx


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

== Series Details ==

Series: series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4)
URL   : https://patchwork.freedesktop.org/series/82417/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9136 -> Patchwork_18690
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@double-flink:
    - fi-tgl-y:           [PASS][1] -> [DMESG-WARN][2] ([i915#402]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-tgl-y/igt@gem_flink_basic@double-flink.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-tgl-y/igt@gem_flink_basic@double-flink.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-tgl-u2:          [PASS][5] -> [INCOMPLETE][6] ([i915#2557])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-tgl-u2/igt@i915_selftest@live@gt_heartbeat.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-tgl-u2/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-bsw-n3050:       [PASS][7] -> [DMESG-WARN][8] ([i915#1982])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - fi-icl-u2:          [PASS][9] -> [DMESG-WARN][10] ([i915#1982])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  * igt@kms_psr@primary_page_flip:
    - fi-tgl-y:           [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-tgl-y/igt@kms_psr@primary_page_flip.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-tgl-y/igt@kms_psr@primary_page_flip.html

  
#### Possible fixes ####

  * igt@gem_flink_basic@flink-lifetime:
    - fi-tgl-y:           [DMESG-WARN][13] ([i915#402]) -> [PASS][14] +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-tgl-y/igt@gem_flink_basic@flink-lifetime.html

  * igt@i915_module_load@reload:
    - {fi-tgl-dsi}:       [DMESG-WARN][15] ([i915#1982] / [k.org#205379]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-tgl-dsi/igt@i915_module_load@reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-tgl-dsi/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-n3050:       [DMESG-WARN][17] ([i915#1982]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-bsw-n3050/igt@i915_pm_rpm@basic-pci-d3-state.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-bsw-n3050/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_busy@basic@flip:
    - {fi-kbl-7560u}:     [DMESG-WARN][19] ([i915#1982]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-kbl-7560u/igt@kms_busy@basic@flip.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-kbl-7560u/igt@kms_busy@basic@flip.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - fi-icl-u2:          [DMESG-WARN][21] ([i915#1982]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-tgl-y:           [DMESG-WARN][23] ([i915#1982]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-tgl-y/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-tgl-y/igt@kms_pipe_crc_basic@read-crc-pipe-b.html

  
#### Warnings ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-tgl-y:           [DMESG-WARN][25] ([i915#2411] / [i915#402]) -> [DMESG-WARN][26] ([i915#2411])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/fi-tgl-y/igt@gem_exec_suspend@basic-s3.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/fi-tgl-y/igt@gem_exec_suspend@basic-s3.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#2411]: https://gitlab.freedesktop.org/drm/intel/issues/2411
  [i915#2557]: https://gitlab.freedesktop.org/drm/intel/issues/2557
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [k.org#205379]: https://bugzilla.kernel.org/show_bug.cgi?id=205379


Participating hosts (47 -> 41)
------------------------------

  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_9136 -> Patchwork_18690

  CI-20190529: 20190529
  CI_DRM_9136: 29eb1a8ba2cc0d14d3cae7213f9cdaaa13f3dd99 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5813: d4e6dd955a1dad02271aa41c9389f5097ee17765 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18690: 44633a798306c7dbf6d66491ebd44d0b8d3c9a5a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

44633a798306 drm/i915: Refactor .hpd_irq_setup() calls a bit
ff2305070287 drm/i915: Do drm_mode_config_reset() after HPD init
706cef4f9041 drm/i915: Reorder hpd init vs. display resume

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 8308 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] 25+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4)
  2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
                   ` (11 preceding siblings ...)
  2020-10-13 18:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-10-14 14:14 ` Patchwork
  12 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2020-10-14 14:14 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx


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

== Series Details ==

Series: series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4)
URL   : https://patchwork.freedesktop.org/series/82417/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9136_full -> Patchwork_18690_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@gem_exec_capture@pi@vecs0}:
    - shard-glk:          NOTRUN -> [INCOMPLETE][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-glk4/igt@gem_exec_capture@pi@vecs0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@feature_discovery@psr2:
    - shard-iclb:         [PASS][2] -> [SKIP][3] ([i915#658])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-iclb2/igt@feature_discovery@psr2.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-iclb1/igt@feature_discovery@psr2.html

  * igt@gem_userptr_blits@unsync-unmap-cycles:
    - shard-skl:          [PASS][4] -> [TIMEOUT][5] ([i915#2424])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl1/igt@gem_userptr_blits@unsync-unmap-cycles.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl5/igt@gem_userptr_blits@unsync-unmap-cycles.html

  * igt@i915_suspend@forcewake:
    - shard-skl:          [PASS][6] -> [INCOMPLETE][7] ([i915#636])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl3/igt@i915_suspend@forcewake.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl4/igt@i915_suspend@forcewake.html

  * igt@kms_cursor_edge_walk@pipe-b-256x256-left-edge:
    - shard-skl:          [PASS][8] -> [DMESG-WARN][9] ([i915#1982]) +9 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl5/igt@kms_cursor_edge_walk@pipe-b-256x256-left-edge.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl3/igt@kms_cursor_edge_walk@pipe-b-256x256-left-edge.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-glk:          [PASS][10] -> [FAIL][11] ([i915#72])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-glk4/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-glk8/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html

  * igt@kms_flip@2x-flip-vs-suspend-interruptible@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][12] -> [DMESG-WARN][13] ([i915#1982])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-glk4/igt@kms_flip@2x-flip-vs-suspend-interruptible@ab-hdmi-a1-hdmi-a2.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-glk9/igt@kms_flip@2x-flip-vs-suspend-interruptible@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1:
    - shard-kbl:          [PASS][14] -> [DMESG-WARN][15] ([i915#1982])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-kbl4/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-kbl6/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
    - shard-skl:          [PASS][16] -> [FAIL][17] ([i915#2122]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl1/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl4/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-wc:
    - shard-tglb:         [PASS][18] -> [DMESG-WARN][19] ([i915#1982]) +2 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-wc.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-mmap-wc.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [PASS][20] -> [FAIL][21] ([i915#1188])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl2/igt@kms_hdr@bpc-switch-dpms.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl10/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence:
    - shard-skl:          [PASS][22] -> [FAIL][23] ([i915#53])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl4/igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl9/igt@kms_pipe_crc_basic@read-crc-pipe-c-frame-sequence.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][24] -> [FAIL][25] ([fdo#108145] / [i915#265])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [PASS][26] -> [SKIP][27] ([fdo#109441]) +2 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-iclb2/igt@kms_psr@psr2_suspend.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-iclb8/igt@kms_psr@psr2_suspend.html

  * igt@kms_setmode@basic:
    - shard-skl:          [PASS][28] -> [FAIL][29] ([i915#31])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl7/igt@kms_setmode@basic.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl3/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * {igt@gem_exec_capture@pi@rcs0}:
    - shard-glk:          [INCOMPLETE][30] ([i915#2553]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-glk6/igt@gem_exec_capture@pi@rcs0.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-glk4/igt@gem_exec_capture@pi@rcs0.html

  * igt@gem_exec_fence@parallel@rcs0:
    - shard-tglb:         [FAIL][32] ([i915#1893]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-tglb6/igt@gem_exec_fence@parallel@rcs0.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-tglb3/igt@gem_exec_fence@parallel@rcs0.html

  * igt@gem_exec_whisper@basic-queues-all:
    - shard-glk:          [DMESG-WARN][34] ([i915#118] / [i915#95]) -> [PASS][35] +1 similar issue
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-glk1/igt@gem_exec_whisper@basic-queues-all.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-glk3/igt@gem_exec_whisper@basic-queues-all.html

  * {igt@kms_async_flips@async-flip-with-page-flip-events}:
    - shard-kbl:          [FAIL][36] ([i915#2521]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-kbl2/igt@kms_async_flips@async-flip-with-page-flip-events.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-kbl2/igt@kms_async_flips@async-flip-with-page-flip-events.html

  * igt@kms_big_fb@linear-32bpp-rotate-180:
    - shard-skl:          [DMESG-WARN][38] ([i915#1982]) -> [PASS][39] +9 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl5/igt@kms_big_fb@linear-32bpp-rotate-180.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl3/igt@kms_big_fb@linear-32bpp-rotate-180.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
    - shard-skl:          [FAIL][40] ([i915#2346]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl4/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html

  * igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1:
    - shard-skl:          [FAIL][42] ([i915#2122]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl9/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl1/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-plflip-blt:
    - shard-tglb:         [FAIL][44] -> [PASS][45] +3 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-plflip-blt.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-tglb:         [FAIL][46] ([i915#2416]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-kbl:          [DMESG-WARN][48] ([i915#165] / [i915#78]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-kbl2/igt@kms_hdr@bpc-switch-dpms.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-kbl2/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - shard-skl:          [FAIL][50] ([i915#53]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl2/igt@kms_pipe_crc_basic@read-crc-pipe-a.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl2/igt@kms_pipe_crc_basic@read-crc-pipe-a.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][52] ([fdo#108145] / [i915#265]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl2/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [SKIP][54] ([fdo#109441]) -> [PASS][55] +1 similar issue
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-iclb4/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [FAIL][56] ([i915#1635] / [i915#31]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-apl8/igt@kms_setmode@basic.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-apl1/igt@kms_setmode@basic.html
    - shard-glk:          [FAIL][58] ([i915#31]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-glk8/igt@kms_setmode@basic.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-glk7/igt@kms_setmode@basic.html

  * igt@sysfs_heartbeat_interval@mixed@vcs1:
    - shard-kbl:          [INCOMPLETE][60] ([i915#1731]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-kbl4/igt@sysfs_heartbeat_interval@mixed@vcs1.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-kbl6/igt@sysfs_heartbeat_interval@mixed@vcs1.html

  
#### Warnings ####

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][62] ([i915#658]) -> [SKIP][63] ([i915#588])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-iclb4/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [FAIL][64] ([i915#1515]) -> [WARN][65] ([i915#1515])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-iclb5/igt@i915_pm_rc6_residency@rc6-idle.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-iclb5/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy:
    - shard-skl:          [DMESG-FAIL][66] ([i915#1982]) -> [DMESG-WARN][67] ([i915#1982])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9136/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18690/shard-skl4/igt@kms_cursor_legacy@flip-vs-cursor-crc-legacy.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1515]: https://gitlab.freedesktop.org/drm/intel/issues/1515
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
  [i915#1731]: https://gitlab.freedesktop.org/drm/intel/issues/1731
  [i915#1893]: https://gitlab.freedesktop.org/drm/intel/issues/1893
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2416]: https://gitlab.freedesktop.org/drm/intel/issues/2416
  [i915#2424]: https://gitlab.freedesktop.org/drm/intel/issues/2424
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#2553]: https://gitlab.freedesktop.org/drm/intel/issues/2553
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
  [i915#53]: https://gitlab.freedesktop.org/drm/intel/issues/53
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#636]: https://gitlab.freedesktop.org/drm/intel/issues/636
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
  [i915#78]: https://gitlab.freedesktop.org/drm/intel/issues/78
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


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

  Missing    (1): pig-snb-2600 


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

  * Linux: CI_DRM_9136 -> Patchwork_18690

  CI-20190529: 20190529
  CI_DRM_9136: 29eb1a8ba2cc0d14d3cae7213f9cdaaa13f3dd99 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5813: d4e6dd955a1dad02271aa41c9389f5097ee17765 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18690: 44633a798306c7dbf6d66491ebd44d0b8d3c9a5a @ 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_18690/index.html

[-- Attachment #1.2: Type: text/html, Size: 18026 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] 25+ messages in thread

* Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-13 18:11   ` [Intel-gfx] [PATCH v3 " Ville Syrjala
@ 2020-10-19 15:38     ` Imre Deak
  2020-10-19 16:52     ` Lyude Paul
  1 sibling, 0 replies; 25+ messages in thread
From: Imre Deak @ 2020-10-19 15:38 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Tue, Oct 13, 2020 at 09:11:37PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we call .hpd_irq_setup() directly just before display
> resume, and follow it with another call via intel_hpd_init()
> just afterwards. Assuming the hpd pins are marked as enabled
> during the open-coded call these two things do exactly the
> same thing (ie. enable HPD interrupts). Which even makes sense
> since we definitely need working HPD interrupts for MST sideband
> during the display resume.
> 
> So let's nuke the open-coded call and move the intel_hpd_init()
> call earlier. However we need to leave the poll_init_work stuff
> behind after the display resume as that will trigger display
> detection while we're resuming. We don't want that trampling over
> the display resume process. To make this a bit more symmetric
> we turn this into a intel_hpd_poll_{enable,disable}() pair.
> So we end up with the following transformation:
> intel_hpd_poll_init() -> intel_hpd_poll_enable()
> lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
> .hpd_irq_setup()+resume+intel_hpd_init() -> intel_hpd_init()+resume+intel_hpd_poll_disable()
> 
> If we really would like to prevent all *long* HPD processing during
> display resume we'd need some kind of software mechanism to simply
> ignore all long HPDs. Currently we appear to have that just for
> fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
> right all the time I guess that's mostly sufficient.
> 
> For a bit of history on this, we first got a mechanism to block
> hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
> enable irqs earlier when resuming") on account of moving the irq enable
> earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
> Fix hpd vs. initial config races") because the fdev initial config
> got pushed to a later point. The second ad-hoc hpd_irq_setup() for
> resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
> support (v0.7)") to be able to do MST sideband during the resume.
> And finally we got a partial resurrection of the hpd blocking
> mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
> processing during suspend"), but this time it only prevent fbdev
> from handling hpd while resuming.
> 
> v2: Leave the poll_init_work behind
> v3: Remove the extra intel_hpd_poll_disable() from display reset (Lyude)
>     Add the missing intel_hpd_poll_disable() to display init (Imre)
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
>  .../drm/i915/display/intel_display_power.c    |  3 +-
>  drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
>  drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
>  5 files changed, 46 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 907e1d155443..3be0d531f96c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5036,18 +5036,14 @@ void intel_finish_reset(struct drm_i915_private *dev_priv)
>  		intel_pps_unlock_regs_wa(dev_priv);
>  		intel_modeset_init_hw(dev_priv);
>  		intel_init_clock_gating(dev_priv);
> -
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		if (dev_priv->display.hpd_irq_setup)
> -			dev_priv->display.hpd_irq_setup(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> +		intel_hpd_init(dev_priv);
>  
>  		ret = __intel_display_resume(dev, state, ctx);
>  		if (ret)
>  			drm_err(&dev_priv->drm,
>  				"Restoring old state failed with %i\n", ret);
>  
> -		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
>  	}
>  
>  	drm_atomic_state_put(state);
> @@ -18257,6 +18253,7 @@ int intel_modeset_init(struct drm_i915_private *i915)
>  
>  	/* Only enable hotplug handling once the fbdev is fully set up. */
>  	intel_hpd_init(i915);
> +	intel_hpd_poll_disable(i915);
>  
>  	intel_init_ipc(i915);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 7277e58b01f1..20ddc54298cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1424,6 +1424,7 @@ static void vlv_display_power_well_init(struct drm_i915_private *dev_priv)
>  		return;
>  
>  	intel_hpd_init(dev_priv);
> +	intel_hpd_poll_disable(dev_priv);
>  
>  	/* Re-enable the ADPA, if we have one */
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
> @@ -1449,7 +1450,7 @@ static void vlv_display_power_well_deinit(struct drm_i915_private *dev_priv)
>  
>  	/* Prevent us from re-enabling polling on accident in late suspend */
>  	if (!dev_priv->drm.dev->power.is_suspended)
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  }
>  
>  static void vlv_display_power_well_enable(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 5c58c1ed6493..30bd4c86d146 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -584,7 +584,7 @@ void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
>   * This is a separate step from interrupt enabling to simplify the locking rules
>   * in the driver load and resume code.
>   *
> - * Also see: intel_hpd_poll_init(), which enables connector polling
> + * Also see: intel_hpd_poll_enable() and intel_hpd_poll_disable().
>   */
>  void intel_hpd_init(struct drm_i915_private *dev_priv)
>  {
> @@ -595,9 +595,6 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>  		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
>  	}
>  
> -	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> -	schedule_work(&dev_priv->hotplug.poll_init_work);
> -
>  	/*
>  	 * Interrupt setup is already guaranteed to be single-threaded, this is
>  	 * just to make the assert_spin_locked checks happy.
> @@ -654,12 +651,12 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
>  }
>  
>  /**
> - * intel_hpd_poll_init - enables/disables polling for connectors with hpd
> + * intel_hpd_poll_enable - enable polling for connectors with hpd
>   * @dev_priv: i915 device instance
>   *
> - * This function enables polling for all connectors, regardless of whether or
> - * not they support hotplug detection. Under certain conditions HPD may not be
> - * functional. On most Intel GPUs, this happens when we enter runtime suspend.
> + * This function enables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
>   * On Valleyview and Cherryview systems, this also happens when we shut off all
>   * of the powerwells.
>   *
> @@ -667,9 +664,9 @@ static void i915_hpd_poll_init_work(struct work_struct *work)
>   * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
>   * worker.
>   *
> - * Also see: intel_hpd_init(), which restores hpd handling.
> + * Also see: intel_hpd_init() and intel_hpd_poll_disable().
>   */
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
>  {
>  	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
>  
> @@ -682,6 +679,31 @@ void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
>  	schedule_work(&dev_priv->hotplug.poll_init_work);
>  }
>  
> +/**
> + * intel_hpd_poll_disable - disable polling for connectors with hpd
> + * @dev_priv: i915 device instance
> + *
> + * This function disables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
> + * On Valleyview and Cherryview systems, this also happens when we shut off all
> + * of the powerwells.
> + *
> + * Since this function can get called in contexts where we're already holding
> + * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
> + * worker.
> + *
> + * Also used during driver init to initialize connector->polled
> + * appropriately for all connectors.
> + *
> + * Also see: intel_hpd_init() and intel_hpd_poll_enable().
> + */
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
> +{
> +	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> +	schedule_work(&dev_priv->hotplug.poll_init_work);
> +}
> +
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>  {
>  	INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index a704d7c94d16..b87e95d606e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -14,7 +14,8 @@ struct intel_digital_port;
>  struct intel_encoder;
>  enum port;
>  
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv);
>  enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder *encoder,
>  					       struct intel_connector *connector);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index ebc15066d108..3fc7b996fc48 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1226,26 +1226,15 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	intel_modeset_init_hw(dev_priv);
>  	intel_init_clock_gating(dev_priv);
> +	intel_hpd_init(dev_priv);
>  
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.hpd_irq_setup)
> -		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -
> +	/* MST sideband requires HPD interrupts enabled */
>  	intel_dp_mst_resume(dev_priv);
> -
>  	intel_display_resume(dev);
>  
> +	intel_hpd_poll_disable(dev_priv);
>  	drm_kms_helper_poll_enable(dev);
>  
> -	/*
> -	 * ... but also need to make sure that hotplug processing
> -	 * doesn't cause havoc. Like in the driver load code we don't
> -	 * bother with the tiny race here where we might lose hotplug
> -	 * notifications.
> -	 * */
> -	intel_hpd_init(dev_priv);
> -
>  	intel_opregion_resume(dev_priv);
>  
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> @@ -1557,7 +1546,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  	assert_forcewakes_inactive(&dev_priv->uncore);
>  
>  	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  
>  	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
>  	return 0;
> @@ -1602,8 +1591,10 @@ static int intel_runtime_resume(struct device *kdev)
>  	 * power well, so hpd is reinitialized from there. For
>  	 * everyone else do it here.
>  	 */
> -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
>  		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
> +	}
>  
>  	intel_enable_ipc(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	[flat|nested] 25+ messages in thread

* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init
  2020-10-06 18:58 ` [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init Ville Syrjala
  2020-10-12 20:16   ` Imre Deak
@ 2020-10-19 15:39   ` Imre Deak
  2020-10-19 15:58     ` Ville Syrjälä
  1 sibling, 1 reply; 25+ messages in thread
From: Imre Deak @ 2020-10-19 15:39 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Tue, Oct 06, 2020 at 09:58:08PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> LSPCON requires HPD detection logic to be enabled when we call
> its .reset() hook during resume, to check the live state of the
> pin. To that end let's reorder the resume sequence such that
> we do HPD init before the encoder reset.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b2a050d916e3..66ddd4161885 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1213,21 +1213,20 @@ static int i915_drm_resume(struct drm_device *dev)
>  	 * GPU will hang. i915_gem_init_hw() will initiate batches to
>  	 * update/restore the context.
>  	 *
> -	 * drm_mode_config_reset() needs AUX interrupts.
> -	 *
>  	 * Modeset enabling in intel_modeset_init_hw() also needs working
>  	 * interrupts.
>  	 */
>  	intel_runtime_pm_enable_interrupts(dev_priv);
>  
> -	drm_mode_config_reset(dev);
> -
>  	i915_gem_resume(dev_priv);
>  
>  	intel_modeset_init_hw(dev_priv);
>  	intel_init_clock_gating(dev_priv);
>  	intel_hpd_init(dev_priv);
>  
> +	/* May need AUX interrupts and HPD detection enabled */
> +	drm_mode_config_reset(dev);
> +
>  	/* MST sideband requires HPD interrupts enabled */
>  	intel_dp_mst_resume(dev_priv);
>  	intel_display_resume(dev);
> -- 
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init
  2020-10-19 15:39   ` Imre Deak
@ 2020-10-19 15:58     ` Ville Syrjälä
  0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2020-10-19 15:58 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Mon, Oct 19, 2020 at 06:39:59PM +0300, Imre Deak wrote:
> On Tue, Oct 06, 2020 at 09:58:08PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > LSPCON requires HPD detection logic to be enabled when we call
> > its .reset() hook during resume, to check the live state of the
> > pin. To that end let's reorder the resume sequence such that
> > we do HPD init before the encoder reset.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>

I think I'll just drop this patch entirely. AFAICS no longer needed
once the lspcon resume is moved out of .reset().

> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index b2a050d916e3..66ddd4161885 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1213,21 +1213,20 @@ static int i915_drm_resume(struct drm_device *dev)
> >  	 * GPU will hang. i915_gem_init_hw() will initiate batches to
> >  	 * update/restore the context.
> >  	 *
> > -	 * drm_mode_config_reset() needs AUX interrupts.
> > -	 *
> >  	 * Modeset enabling in intel_modeset_init_hw() also needs working
> >  	 * interrupts.
> >  	 */
> >  	intel_runtime_pm_enable_interrupts(dev_priv);
> >  
> > -	drm_mode_config_reset(dev);
> > -
> >  	i915_gem_resume(dev_priv);
> >  
> >  	intel_modeset_init_hw(dev_priv);
> >  	intel_init_clock_gating(dev_priv);
> >  	intel_hpd_init(dev_priv);
> >  
> > +	/* May need AUX interrupts and HPD detection enabled */
> > +	drm_mode_config_reset(dev);
> > +
> >  	/* MST sideband requires HPD interrupts enabled */
> >  	intel_dp_mst_resume(dev_priv);
> >  	intel_display_resume(dev);
> > -- 
> > 2.26.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] 25+ messages in thread

* Re: [Intel-gfx] [PATCH v3 1/3] drm/i915: Reorder hpd init vs. display resume
  2020-10-13 18:11   ` [Intel-gfx] [PATCH v3 " Ville Syrjala
  2020-10-19 15:38     ` Imre Deak
@ 2020-10-19 16:52     ` Lyude Paul
  1 sibling, 0 replies; 25+ messages in thread
From: Lyude Paul @ 2020-10-19 16:52 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Reviewed-by: Lyude Paul <lyude@redhat.com>

On Tue, 2020-10-13 at 21:11 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we call .hpd_irq_setup() directly just before display
> resume, and follow it with another call via intel_hpd_init()
> just afterwards. Assuming the hpd pins are marked as enabled
> during the open-coded call these two things do exactly the
> same thing (ie. enable HPD interrupts). Which even makes sense
> since we definitely need working HPD interrupts for MST sideband
> during the display resume.
> 
> So let's nuke the open-coded call and move the intel_hpd_init()
> call earlier. However we need to leave the poll_init_work stuff
> behind after the display resume as that will trigger display
> detection while we're resuming. We don't want that trampling over
> the display resume process. To make this a bit more symmetric
> we turn this into a intel_hpd_poll_{enable,disable}() pair.
> So we end up with the following transformation:
> intel_hpd_poll_init() -> intel_hpd_poll_enable()
> lone intel_hpd_init() -> intel_hpd_init()+intel_hpd_poll_disable()
> .hpd_irq_setup()+resume+intel_hpd_init() ->
> intel_hpd_init()+resume+intel_hpd_poll_disable()
> 
> If we really would like to prevent all *long* HPD processing during
> display resume we'd need some kind of software mechanism to simply
> ignore all long HPDs. Currently we appear to have that just for
> fbdev via ifbdev->hpd_suspended. Since we aren't exploding left and
> right all the time I guess that's mostly sufficient.
> 
> For a bit of history on this, we first got a mechanism to block
> hotplug processing during suspend in commit 15239099d7a7 ("drm/i915:
> enable irqs earlier when resuming") on account of moving the irq enable
> earlier. This then got removed in commit 50c3dc970a09 ("drm/fb-helper:
> Fix hpd vs. initial config races") because the fdev initial config
> got pushed to a later point. The second ad-hoc hpd_irq_setup() for
> resume was added in commit 0e32b39ceed6 ("drm/i915: add DP 1.2 MST
> support (v0.7)") to be able to do MST sideband during the resume.
> And finally we got a partial resurrection of the hpd blocking
> mechanism in commit e8a8fedd57fd ("drm/i915: Block fbdev HPD
> processing during suspend"), but this time it only prevent fbdev
> from handling hpd while resuming.
> 
> v2: Leave the poll_init_work behind
> v3: Remove the extra intel_hpd_poll_disable() from display reset (Lyude)
>     Add the missing intel_hpd_poll_disable() to display init (Imre)
> 
> Cc: Lyude Paul <lyude@redhat.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  9 ++--
>  .../drm/i915/display/intel_display_power.c    |  3 +-
>  drivers/gpu/drm/i915/display/intel_hotplug.c  | 42 ++++++++++++++-----
>  drivers/gpu/drm/i915/display/intel_hotplug.h  |  3 +-
>  drivers/gpu/drm/i915/i915_drv.c               | 23 ++++------
>  5 files changed, 46 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 907e1d155443..3be0d531f96c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5036,18 +5036,14 @@ void intel_finish_reset(struct drm_i915_private
> *dev_priv)
>  		intel_pps_unlock_regs_wa(dev_priv);
>  		intel_modeset_init_hw(dev_priv);
>  		intel_init_clock_gating(dev_priv);
> -
> -		spin_lock_irq(&dev_priv->irq_lock);
> -		if (dev_priv->display.hpd_irq_setup)
> -			dev_priv->display.hpd_irq_setup(dev_priv);
> -		spin_unlock_irq(&dev_priv->irq_lock);
> +		intel_hpd_init(dev_priv);
>  
>  		ret = __intel_display_resume(dev, state, ctx);
>  		if (ret)
>  			drm_err(&dev_priv->drm,
>  				"Restoring old state failed with %i\n", ret);
>  
> -		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
>  	}
>  
>  	drm_atomic_state_put(state);
> @@ -18257,6 +18253,7 @@ int intel_modeset_init(struct drm_i915_private
> *i915)
>  
>  	/* Only enable hotplug handling once the fbdev is fully set up. */
>  	intel_hpd_init(i915);
> +	intel_hpd_poll_disable(i915);
>  
>  	intel_init_ipc(i915);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
> b/drivers/gpu/drm/i915/display/intel_display_power.c
> index 7277e58b01f1..20ddc54298cb 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> @@ -1424,6 +1424,7 @@ static void vlv_display_power_well_init(struct
> drm_i915_private *dev_priv)
>  		return;
>  
>  	intel_hpd_init(dev_priv);
> +	intel_hpd_poll_disable(dev_priv);
>  
>  	/* Re-enable the ADPA, if we have one */
>  	for_each_intel_encoder(&dev_priv->drm, encoder) {
> @@ -1449,7 +1450,7 @@ static void vlv_display_power_well_deinit(struct
> drm_i915_private *dev_priv)
>  
>  	/* Prevent us from re-enabling polling on accident in late suspend */
>  	if (!dev_priv->drm.dev->power.is_suspended)
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  }
>  
>  static void vlv_display_power_well_enable(struct drm_i915_private
> *dev_priv,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c
> b/drivers/gpu/drm/i915/display/intel_hotplug.c
> index 5c58c1ed6493..30bd4c86d146 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c
> @@ -584,7 +584,7 @@ void intel_hpd_irq_handler(struct drm_i915_private
> *dev_priv,
>   * This is a separate step from interrupt enabling to simplify the locking
> rules
>   * in the driver load and resume code.
>   *
> - * Also see: intel_hpd_poll_init(), which enables connector polling
> + * Also see: intel_hpd_poll_enable() and intel_hpd_poll_disable().
>   */
>  void intel_hpd_init(struct drm_i915_private *dev_priv)
>  {
> @@ -595,9 +595,6 @@ void intel_hpd_init(struct drm_i915_private *dev_priv)
>  		dev_priv->hotplug.stats[i].state = HPD_ENABLED;
>  	}
>  
> -	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> -	schedule_work(&dev_priv->hotplug.poll_init_work);
> -
>  	/*
>  	 * Interrupt setup is already guaranteed to be single-threaded, this
> is
>  	 * just to make the assert_spin_locked checks happy.
> @@ -654,12 +651,12 @@ static void i915_hpd_poll_init_work(struct work_struct
> *work)
>  }
>  
>  /**
> - * intel_hpd_poll_init - enables/disables polling for connectors with hpd
> + * intel_hpd_poll_enable - enable polling for connectors with hpd
>   * @dev_priv: i915 device instance
>   *
> - * This function enables polling for all connectors, regardless of whether
> or
> - * not they support hotplug detection. Under certain conditions HPD may not
> be
> - * functional. On most Intel GPUs, this happens when we enter runtime
> suspend.
> + * This function enables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
>   * On Valleyview and Cherryview systems, this also happens when we shut off
> all
>   * of the powerwells.
>   *
> @@ -667,9 +664,9 @@ static void i915_hpd_poll_init_work(struct work_struct
> *work)
>   * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
>   * worker.
>   *
> - * Also see: intel_hpd_init(), which restores hpd handling.
> + * Also see: intel_hpd_init() and intel_hpd_poll_disable().
>   */
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv)
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv)
>  {
>  	WRITE_ONCE(dev_priv->hotplug.poll_enabled, true);
>  
> @@ -682,6 +679,31 @@ void intel_hpd_poll_init(struct drm_i915_private
> *dev_priv)
>  	schedule_work(&dev_priv->hotplug.poll_init_work);
>  }
>  
> +/**
> + * intel_hpd_poll_disable - disable polling for connectors with hpd
> + * @dev_priv: i915 device instance
> + *
> + * This function disables polling for all connectors which support HPD.
> + * Under certain conditions HPD may not be functional. On most Intel GPUs,
> + * this happens when we enter runtime suspend.
> + * On Valleyview and Cherryview systems, this also happens when we shut off
> all
> + * of the powerwells.
> + *
> + * Since this function can get called in contexts where we're already
> holding
> + * dev->mode_config.mutex, we do the actual hotplug enabling in a seperate
> + * worker.
> + *
> + * Also used during driver init to initialize connector->polled
> + * appropriately for all connectors.
> + *
> + * Also see: intel_hpd_init() and intel_hpd_poll_enable().
> + */
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv)
> +{
> +	WRITE_ONCE(dev_priv->hotplug.poll_enabled, false);
> +	schedule_work(&dev_priv->hotplug.poll_init_work);
> +}
> +
>  void intel_hpd_init_work(struct drm_i915_private *dev_priv)
>  {
>  	INIT_DELAYED_WORK(&dev_priv->hotplug.hotplug_work,
> diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.h
> b/drivers/gpu/drm/i915/display/intel_hotplug.h
> index a704d7c94d16..b87e95d606e6 100644
> --- a/drivers/gpu/drm/i915/display/intel_hotplug.h
> +++ b/drivers/gpu/drm/i915/display/intel_hotplug.h
> @@ -14,7 +14,8 @@ struct intel_digital_port;
>  struct intel_encoder;
>  enum port;
>  
> -void intel_hpd_poll_init(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_enable(struct drm_i915_private *dev_priv);
> +void intel_hpd_poll_disable(struct drm_i915_private *dev_priv);
>  enum intel_hotplug_state intel_encoder_hotplug(struct intel_encoder
> *encoder,
>  					       struct intel_connector
> *connector);
>  void intel_hpd_irq_handler(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index ebc15066d108..3fc7b996fc48 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1226,26 +1226,15 @@ static int i915_drm_resume(struct drm_device *dev)
>  
>  	intel_modeset_init_hw(dev_priv);
>  	intel_init_clock_gating(dev_priv);
> +	intel_hpd_init(dev_priv);
>  
> -	spin_lock_irq(&dev_priv->irq_lock);
> -	if (dev_priv->display.hpd_irq_setup)
> -		dev_priv->display.hpd_irq_setup(dev_priv);
> -	spin_unlock_irq(&dev_priv->irq_lock);
> -
> +	/* MST sideband requires HPD interrupts enabled */
>  	intel_dp_mst_resume(dev_priv);
> -
>  	intel_display_resume(dev);
>  
> +	intel_hpd_poll_disable(dev_priv);
>  	drm_kms_helper_poll_enable(dev);
>  
> -	/*
> -	 * ... but also need to make sure that hotplug processing
> -	 * doesn't cause havoc. Like in the driver load code we don't
> -	 * bother with the tiny race here where we might lose hotplug
> -	 * notifications.
> -	 * */
> -	intel_hpd_init(dev_priv);
> -
>  	intel_opregion_resume(dev_priv);
>  
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false);
> @@ -1557,7 +1546,7 @@ static int intel_runtime_suspend(struct device *kdev)
>  	assert_forcewakes_inactive(&dev_priv->uncore);
>  
>  	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> -		intel_hpd_poll_init(dev_priv);
> +		intel_hpd_poll_enable(dev_priv);
>  
>  	drm_dbg_kms(&dev_priv->drm, "Device suspended\n");
>  	return 0;
> @@ -1602,8 +1591,10 @@ static int intel_runtime_resume(struct device *kdev)
>  	 * power well, so hpd is reinitialized from there. For
>  	 * everyone else do it here.
>  	 */
> -	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv))
> +	if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
>  		intel_hpd_init(dev_priv);
> +		intel_hpd_poll_disable(dev_priv);
> +	}
>  
>  	intel_enable_ipc(dev_priv);
>  
-- 
Cheers,
	Lyude Paul (she/her)
	Software Engineer at Red Hat

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

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

end of thread, other threads:[~2020-10-19 16:52 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 18:58 [Intel-gfx] [PATCH 1/3] drm/i915: Reorder hpd init vs. display resume Ville Syrjala
2020-10-06 18:58 ` [Intel-gfx] [PATCH 2/3] drm/i915: Do drm_mode_config_reset() after HPD init Ville Syrjala
2020-10-12 20:16   ` Imre Deak
2020-10-19 15:39   ` Imre Deak
2020-10-19 15:58     ` Ville Syrjälä
2020-10-06 18:58 ` [Intel-gfx] [PATCH 3/3] drm/i915: Refactor .hpd_irq_setup() calls a bit Ville Syrjala
2020-10-12 20:22   ` Imre Deak
2020-10-06 19:27 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [1/3] drm/i915: Reorder hpd init vs. display resume Patchwork
2020-10-07 11:06 ` [Intel-gfx] [PATCH 1/3] " Ville Syrjälä
2020-10-07 19:22 ` [Intel-gfx] [PATCH v2 " Ville Syrjala
2020-10-07 22:15   ` Lyude Paul
2020-10-08  8:19     ` Ville Syrjälä
2020-10-12 19:36   ` Imre Deak
2020-10-13 13:39     ` Ville Syrjälä
2020-10-13 18:11   ` [Intel-gfx] [PATCH v3 " Ville Syrjala
2020-10-19 15:38     ` Imre Deak
2020-10-19 16:52     ` Lyude Paul
2020-10-08 11:39 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev2) Patchwork
2020-10-08 12:00 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-10-08 17:02 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v2,1/3] drm/i915: Reorder hpd init vs. display resume (rev3) Patchwork
2020-10-08 17:24 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-10-08 19:06 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-10-13 18:19 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v3,1/3] drm/i915: Reorder hpd init vs. display resume (rev4) Patchwork
2020-10-13 18:45 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-10-14 14:14 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

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.