All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
@ 2018-08-03 18:10 Jan-Marek Glogowski
  2018-08-04  1:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Jan-Marek Glogowski @ 2018-08-03 18:10 UTC (permalink / raw)
  To: intel-gfx

The same patch and various dmesg output is attached to
https://bugs.freedesktop.org/show_bug.cgi?id=107446 "Monitor on 2nd DisplayPort is not initialized"

I have no idea, if this patch is correct, but it fixes my problem of the "bricked" monitor.

Jan-Marek

---

From edf159ae0426d9c90bf974629d83dc50c002bc52 Mon Sep 17 00:00:00 2001
From: Jan-Marek Glogowski <glogow@fbihome.de>
Date: Fri, 3 Aug 2018 15:30:55 +0000
Subject: [PATCH] drm/i915: Re-apply "Perform link quality check
 unconditionally during long pulse"

This re-applies the workaround for "some DP sinks, [which] are a
little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
quality check unconditionally during long pulse").
It makes the secondary AOC E2460P monitor connected via DP to an
acer Veriton N4640G usable again.

This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
DP link retraining into the ->post_hotplug() hook")

Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
---
 drivers/gpu/drm/i915/intel_dp.c | 46 ++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 153342c..796c2a2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4294,18 +4294,6 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
 	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
 }

-/*
- * If display is now connected check links status,
- * there has been known issues of link loss triggering
- * long pulse.
- *
- * Some sinks (eg. ASUS PB287Q) seem to perform some
- * weird HPD ping pong during modesets. So we can apparently
- * end up with HPD going low during a modeset, and then
- * going back up soon after. And once that happens we must
- * retrain the link to get a picture. That's in case no
- * userspace component reacted to intermittent HPD dip.
- */
 int intel_dp_retrain_link(struct intel_encoder *encoder,
 			  struct drm_modeset_acquire_ctx *ctx)
 {
@@ -4322,10 +4310,12 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 	if (!connector || connector->base.status != connector_status_connected)
 		return 0;

-	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
-			       ctx);
-	if (ret)
-		return ret;
+	if (ctx) {
+		ret = drm_modeset_lock(
+			&dev_priv->drm.mode_config.connection_mutex, ctx);
+		if (ret)
+			return ret;
+	}

 	conn_state = connector->base.state;

@@ -4333,9 +4323,11 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 	if (!crtc)
 		return 0;

-	ret = drm_modeset_lock(&crtc->base.mutex, ctx);
-	if (ret)
-		return ret;
+	if (ctx) {
+		ret = drm_modeset_lock(&crtc->base.mutex, ctx);
+		if (ret)
+			return ret;
+	}

 	crtc_state = to_intel_crtc_state(crtc->base.state);

@@ -4854,6 +4846,22 @@ intel_dp_long_pulse(struct intel_connector *connector)
 		 */
 		status = connector_status_disconnected;
 		goto out;
+	} else {
+		/*
+		 * If display is now connected check links status,
+		 * there has been known issues of link loss triggering
+		 * long pulse.
+		 *
+		 * Some sinks (eg. ASUS PB287Q) seem to perform some
+		 * weird HPD ping pong during modesets. So we can apparently
+		 * end up with HPD going low during a modeset, and then
+		 * going back up soon after. And once that happens we must
+		 * retrain the link to get a picture. That's in case no
+		 * userspace component reacted to intermittent HPD dip.
+		 */
+		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+
+		intel_dp_retrain_link(encoder, NULL);
 	}

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

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-03 18:10 [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Jan-Marek Glogowski
@ 2018-08-04  1:36 ` Patchwork
  2018-08-06 10:25   ` [PATCH] v2 " Jan-Marek Glogowski
  2018-08-04  1:53 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Patchwork @ 2018-08-04  1:36 UTC (permalink / raw)
  To: Jan-Marek Glogowski; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
URL   : https://patchwork.freedesktop.org/series/47694/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9586f1b2fc58 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
-:8: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#8: 
https://bugs.freedesktop.org/show_bug.cgi?id=107446 "Monitor on 2nd DisplayPort is not initialized"

-:46: CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#46: FILE: drivers/gpu/drm/i915/intel_dp.c:4353:
+		ret = drm_modeset_lock(

-:91: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

total: 1 errors, 1 warnings, 1 checks, 70 lines checked

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-03 18:10 [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Jan-Marek Glogowski
  2018-08-04  1:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-08-04  1:53 ` Patchwork
  2018-08-06 16:34 ` ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev2) Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-04  1:53 UTC (permalink / raw)
  To: Jan-Marek Glogowski; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
URL   : https://patchwork.freedesktop.org/series/47694/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4617 -> Patchwork_9853 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47694/revisions/1/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      fi-cfl-guc:         PASS -> DMESG-FAIL

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_guc:
      fi-skl-guc:         NOTRUN -> DMESG-WARN (fdo#107175, fdo#107258)

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         NOTRUN -> DMESG-FAIL (fdo#107174)

    igt@drv_selftest@live_workarounds:
      {fi-cfl-8109u}:     PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362, fdo#103191)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-kbl-7560u:       INCOMPLETE -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7567u:       DMESG-FAIL (fdo#106947, fdo#106560) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-FAIL (fdo#102614, fdo#106103) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-byt-clapper}:   FAIL (fdo#107362, fdo#103191) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

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

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175
  fdo#107258 https://bugs.freedesktop.org/show_bug.cgi?id=107258
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (51 -> 47) ==

  Additional (1): fi-skl-guc 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4617 -> Patchwork_9853

  CI_DRM_4617: d9f22d63ed928e975312a8a90f7b95157a41f7e0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4587: 5d78c73d871525ec9caecd88ad7d9abe36637314 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9853: 9586f1b2fc585b3e83d1b4e5e1bca8056568647f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9586f1b2fc58 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9853/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] v2 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-04  1:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
@ 2018-08-06 10:25   ` Jan-Marek Glogowski
  2018-08-07 19:33     ` Lyude Paul
  0 siblings, 1 reply; 17+ messages in thread
From: Jan-Marek Glogowski @ 2018-08-06 10:25 UTC (permalink / raw)
  To: intel-gfx

This re-applies the workaround for "some DP sinks, [which] are a
little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
quality check unconditionally during long pulse").
It makes the secondary AOC E2460P monitor connected via DP to an
acer Veriton N4640G usable again.

This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
DP link retraining into the ->post_hotplug() hook")

Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
---
 drivers/gpu/drm/i915/intel_dp.c | 46 ++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8e0e14b..7e6f8a5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
 	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
 }

-/*
- * If display is now connected check links status,
- * there has been known issues of link loss triggering
- * long pulse.
- *
- * Some sinks (eg. ASUS PB287Q) seem to perform some
- * weird HPD ping pong during modesets. So we can apparently
- * end up with HPD going low during a modeset, and then
- * going back up soon after. And once that happens we must
- * retrain the link to get a picture. That's in case no
- * userspace component reacted to intermittent HPD dip.
- */
 int intel_dp_retrain_link(struct intel_encoder *encoder,
 			  struct drm_modeset_acquire_ctx *ctx)
 {
@@ -4361,10 +4349,12 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 	if (!connector || connector->base.status != connector_status_connected)
 		return 0;

-	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
-			       ctx);
-	if (ret)
-		return ret;
+	if (ctx) {
+		ret = drm_modeset_lock
+			(&dev_priv->drm.mode_config.connection_mutex, ctx);
+		if (ret)
+			return ret;
+	}

 	conn_state = connector->base.state;

@@ -4372,9 +4362,11 @@ int intel_dp_retrain_link(struct intel_encoder *encoder,
 	if (!crtc)
 		return 0;

-	ret = drm_modeset_lock(&crtc->base.mutex, ctx);
-	if (ret)
-		return ret;
+	if (ctx) {
+		ret = drm_modeset_lock(&crtc->base.mutex, ctx);
+		if (ret)
+			return ret;
+	}

 	crtc_state = to_intel_crtc_state(crtc->base.state);

@@ -4982,6 +4974,22 @@ intel_dp_long_pulse(struct intel_connector *connector)
 		 */
 		status = connector_status_disconnected;
 		goto out;
+	} else {
+		/*
+		 * If display is now connected check links status,
+		 * there has been known issues of link loss triggering
+		 * long pulse.
+		 *
+		 * Some sinks (eg. ASUS PB287Q) seem to perform some
+		 * weird HPD ping pong during modesets. So we can apparently
+		 * end up with HPD going low during a modeset, and then
+		 * going back up soon after. And once that happens we must
+		 * retrain the link to get a picture. That's in case no
+		 * userspace component reacted to intermittent HPD dip.
+		 */
+		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+
+		intel_dp_retrain_link(encoder, NULL);
 	}

 	/*
-- 
2.1.4


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

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

* ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev2)
  2018-08-03 18:10 [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Jan-Marek Glogowski
  2018-08-04  1:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
  2018-08-04  1:53 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-08-06 16:34 ` Patchwork
  2018-08-06 19:27 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-06 16:34 UTC (permalink / raw)
  To: Jan-Marek Glogowski; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev2)
URL   : https://patchwork.freedesktop.org/series/47694/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4621 -> Patchwork_9861 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47694/revisions/2/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-bxt-dsi:         PASS -> DMESG-FAIL (fdo#106560)
      fi-kbl-7567u:       PASS -> DMESG-FAIL (fdo#106947, fdo#106560)

    igt@drv_selftest@live_workarounds:
      fi-cnl-psr:         PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-skl-guc:         PASS -> FAIL (fdo#103191)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@kms_chamelium@dp-crc-fast:
      fi-kbl-7500u:       FAIL (fdo#103841) -> PASS

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   FAIL (fdo#103167) -> PASS

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

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362


== Participating hosts (53 -> 48) ==

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


== Build changes ==

    * Linux: CI_DRM_4621 -> Patchwork_9861

  CI_DRM_4621: b35c6b4ccdc1e9b386d5679282123099cf83adf1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4587: 5d78c73d871525ec9caecd88ad7d9abe36637314 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9861: 6e96f6b9a9cb0e8d4a3826b549272716d806841f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6e96f6b9a9cb v2 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9861/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev2)
  2018-08-03 18:10 [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Jan-Marek Glogowski
                   ` (2 preceding siblings ...)
  2018-08-06 16:34 ` ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev2) Patchwork
@ 2018-08-06 19:27 ` Patchwork
  2018-08-08 23:17 ` ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev3) Patchwork
  2018-08-09  2:39 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-06 19:27 UTC (permalink / raw)
  To: Jan-Marek Glogowski; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev2)
URL   : https://patchwork.freedesktop.org/series/47694/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4621_full -> Patchwork_9861_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9861_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9861_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_9861_full:

  === IGT changes ===

    ==== Warnings ====

    igt@pm_rc6_residency@rc6-accuracy:
      shard-kbl:          SKIP -> PASS
      shard-snb:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_parallel@fds:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@kms_setmode@basic:
      shard-hsw:          PASS -> FAIL (fdo#99912)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_workarounds:
      shard-kbl:          DMESG-FAIL (fdo#107292) -> PASS

    igt@gem_eio@reset-stress:
      shard-hsw:          FAIL (fdo#107500) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-glk:          FAIL (fdo#105363) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107500 https://bugs.freedesktop.org/show_bug.cgi?id=107500
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4621 -> Patchwork_9861

  CI_DRM_4621: b35c6b4ccdc1e9b386d5679282123099cf83adf1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4587: 5d78c73d871525ec9caecd88ad7d9abe36637314 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9861: 6e96f6b9a9cb0e8d4a3826b549272716d806841f @ 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_9861/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] v2 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-06 10:25   ` [PATCH] v2 " Jan-Marek Glogowski
@ 2018-08-07 19:33     ` Lyude Paul
  2018-08-07 22:26       ` Jan-Marek Glogowski
  0 siblings, 1 reply; 17+ messages in thread
From: Lyude Paul @ 2018-08-07 19:33 UTC (permalink / raw)
  To: Jan-Marek Glogowski, intel-gfx

On Mon, 2018-08-06 at 12:25 +0200, Jan-Marek Glogowski wrote:
> This re-applies the workaround for "some DP sinks, [which] are a
> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> quality check unconditionally during long pulse").
> It makes the secondary AOC E2460P monitor connected via DP to an
> acer Veriton N4640G usable again.
> 
> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> DP link retraining into the ->post_hotplug() hook")
> 
> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 46 ++++++++++++++++++++++++------------
> -----
>  1 file changed, 27 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 8e0e14b..7e6f8a5 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
> *intel_dp)
>  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
>  }
> 
> -/*
> - * If display is now connected check links status,
> - * there has been known issues of link loss triggering
> - * long pulse.
> - *
> - * Some sinks (eg. ASUS PB287Q) seem to perform some
> - * weird HPD ping pong during modesets. So we can apparently
> - * end up with HPD going low during a modeset, and then
> - * going back up soon after. And once that happens we must
> - * retrain the link to get a picture. That's in case no
> - * userspace component reacted to intermittent HPD dip.
> - */
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>  			  struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -4361,10 +4349,12 @@ int intel_dp_retrain_link(struct intel_encoder
> *encoder,
>  	if (!connector || connector->base.status !=
> connector_status_connected)
>  		return 0;
> 
> -	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> -			       ctx);
> -	if (ret)
> -		return ret;
> +	if (ctx) {
> +		ret = drm_modeset_lock
> +			(&dev_priv->drm.mode_config.connection_mutex, ctx);
> +		if (ret)
> +			return ret;
> +	}
...why are we skipping locking anything if there isn't a
drm_modeset_acquire_ctx passed to us? And additionally, why wouldn't there be
an acquisition context passed to us? We always need to be holding the
connection mutex and the respective CRTC lock whenever we're retraining.

> 
>  	conn_state = connector->base.state;
> 
> @@ -4372,9 +4362,11 @@ int intel_dp_retrain_link(struct intel_encoder
> *encoder,
>  	if (!crtc)
>  		return 0;
> 
> -	ret = drm_modeset_lock(&crtc->base.mutex, ctx);
> -	if (ret)
> -		return ret;
> +	if (ctx) {
> +		ret = drm_modeset_lock(&crtc->base.mutex, ctx);
> +		if (ret)
> +			return ret;
> +	}
> 
>  	crtc_state = to_intel_crtc_state(crtc->base.state);
> 
> @@ -4982,6 +4974,22 @@ intel_dp_long_pulse(struct intel_connector
> *connector)
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> +	} else {
> +		/*
> +		 * If display is now connected check links status,
> +		 * there has been known issues of link loss triggering
> +		 * long pulse.
> +		 *
> +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
> +		 * weird HPD ping pong during modesets. So we can apparently
> +		 * end up with HPD going low during a modeset, and then
> +		 * going back up soon after. And once that happens we must
> +		 * retrain the link to get a picture. That's in case no
> +		 * userspace component reacted to intermittent HPD dip.
> +		 */
> +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
> +
> +		intel_dp_retrain_link(encoder, NULL);
>  	}
> 
>  	/*
-- 
Cheers,
	Lyude Paul

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

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

* Re: [PATCH] v2 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-07 19:33     ` Lyude Paul
@ 2018-08-07 22:26       ` Jan-Marek Glogowski
  2018-08-07 22:46         ` Lyude Paul
  0 siblings, 1 reply; 17+ messages in thread
From: Jan-Marek Glogowski @ 2018-08-07 22:26 UTC (permalink / raw)
  To: Lyude Paul, intel-gfx

Am August 7, 2018 7:33:12 PM UTC schrieb Lyude Paul <lyude@redhat.com>:
>On Mon, 2018-08-06 at 12:25 +0200, Jan-Marek Glogowski wrote:
>> This re-applies the workaround for "some DP sinks, [which] are a
>> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
>> quality check unconditionally during long pulse").
>> It makes the secondary AOC E2460P monitor connected via DP to an
>> acer Veriton N4640G usable again.
>> 
>> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
>> DP link retraining into the ->post_hotplug() hook")
>> 
>> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 46
>++++++++++++++++++++++++------------
>> -----
>>  1 file changed, 27 insertions(+), 19 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 8e0e14b..7e6f8a5 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
>> *intel_dp)
>>  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
>>  }
>> 
>> -/*
>> - * If display is now connected check links status,
>> - * there has been known issues of link loss triggering
>> - * long pulse.
>> - *
>> - * Some sinks (eg. ASUS PB287Q) seem to perform some
>> - * weird HPD ping pong during modesets. So we can apparently
>> - * end up with HPD going low during a modeset, and then
>> - * going back up soon after. And once that happens we must
>> - * retrain the link to get a picture. That's in case no
>> - * userspace component reacted to intermittent HPD dip.
>> - */
>>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>>  			  struct drm_modeset_acquire_ctx *ctx)
>>  {
>> @@ -4361,10 +4349,12 @@ int intel_dp_retrain_link(struct
>intel_encoder
>> *encoder,
>>  	if (!connector || connector->base.status !=
>> connector_status_connected)
>>  		return 0;
>> 
>> -	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
>> -			       ctx);
>> -	if (ret)
>> -		return ret;
>> +	if (ctx) {
>> +		ret = drm_modeset_lock
>> +			(&dev_priv->drm.mode_config.connection_mutex, ctx);
>> +		if (ret)
>> +			return ret;
>> +	}
>...why are we skipping locking anything if there isn't a
>drm_modeset_acquire_ctx passed to us? And additionally, why wouldn't
>there be
>an acquisition context passed to us? We always need to be holding the
>connection mutex and the respective CRTC lock whenever we're
>retraining.

The original patch added all the locking to intel_dp_retrain_link. intel_dp_long_pulse is just called from intel_dp_detect, which takes the drm_modeset_lock depending on a valid crtc. I just assumed this is still sufficient, so for this case we don't need to acquire the lock again. If this is not sufficient, we can forward the ctx, state etc. from intel_dp_detect. I didn't check, if these values are the same values that intel_dp_retrain_link collects using different calls.

There is also the "WARN_ON(!drm_modeset_is_locked(&dev_priv->drm.mode_config.connection_mutex));" in intel_dp_long_pulse, which suggests we should also already have the connection_mutex when calling intel_dp_retrain_link later.

All my assumptions are based on reading the original patch and a little bit of the surrounding code.

This patch is just a minimal change, which "works for me". Not sure if all the locking should be moved back to the retrain call sites, to make it more explicit?

>> 
>>  	conn_state = connector->base.state;
>> 
>> @@ -4372,9 +4362,11 @@ int intel_dp_retrain_link(struct intel_encoder
>> *encoder,
>>  	if (!crtc)
>>  		return 0;
>> 
>> -	ret = drm_modeset_lock(&crtc->base.mutex, ctx);
>> -	if (ret)
>> -		return ret;
>> +	if (ctx) {
>> +		ret = drm_modeset_lock(&crtc->base.mutex, ctx);
>> +		if (ret)
>> +			return ret;
>> +	}
>> 
>>  	crtc_state = to_intel_crtc_state(crtc->base.state);
>> 
>> @@ -4982,6 +4974,22 @@ intel_dp_long_pulse(struct intel_connector
>> *connector)
>>  		 */
>>  		status = connector_status_disconnected;
>>  		goto out;
>> +	} else {
>> +		/*
>> +		 * If display is now connected check links status,
>> +		 * there has been known issues of link loss triggering
>> +		 * long pulse.
>> +		 *
>> +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
>> +		 * weird HPD ping pong during modesets. So we can apparently
>> +		 * end up with HPD going low during a modeset, and then
>> +		 * going back up soon after. And once that happens we must
>> +		 * retrain the link to get a picture. That's in case no
>> +		 * userspace component reacted to intermittent HPD dip.
>> +		 */
>> +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
>> >base;
>> +
>> +		intel_dp_retrain_link(encoder, NULL);
>>  	}
>> 
>>  	/*

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

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

* Re: [PATCH] v2 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-07 22:26       ` Jan-Marek Glogowski
@ 2018-08-07 22:46         ` Lyude Paul
  2018-08-08  8:53           ` [PATCH] v3 " Jan-Marek Glogowski
  0 siblings, 1 reply; 17+ messages in thread
From: Lyude Paul @ 2018-08-07 22:46 UTC (permalink / raw)
  To: Jan-Marek Glogowski, intel-gfx

On Tue, 2018-08-07 at 22:26 +0000, Jan-Marek Glogowski wrote:
> Am August 7, 2018 7:33:12 PM UTC schrieb Lyude Paul <lyude@redhat.com>:
> > On Mon, 2018-08-06 at 12:25 +0200, Jan-Marek Glogowski wrote:
> > > This re-applies the workaround for "some DP sinks, [which] are a
> > > little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> > > quality check unconditionally during long pulse").
> > > It makes the secondary AOC E2460P monitor connected via DP to an
> > > acer Veriton N4640G usable again.
> > > 
> > > This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> > > DP link retraining into the ->post_hotplug() hook")
> > > 
> > > Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 46
> > 
> > ++++++++++++++++++++++++------------
> > > -----
> > >  1 file changed, 27 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 8e0e14b..7e6f8a5 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
> > > *intel_dp)
> > >  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
> > >  }
> > > 
> > > -/*
> > > - * If display is now connected check links status,
> > > - * there has been known issues of link loss triggering
> > > - * long pulse.
> > > - *
> > > - * Some sinks (eg. ASUS PB287Q) seem to perform some
> > > - * weird HPD ping pong during modesets. So we can apparently
> > > - * end up with HPD going low during a modeset, and then
> > > - * going back up soon after. And once that happens we must
> > > - * retrain the link to get a picture. That's in case no
> > > - * userspace component reacted to intermittent HPD dip.
> > > - */
> > >  int intel_dp_retrain_link(struct intel_encoder *encoder,
> > >  			  struct drm_modeset_acquire_ctx *ctx)
> > >  {
> > > @@ -4361,10 +4349,12 @@ int intel_dp_retrain_link(struct
> > 
> > intel_encoder
> > > *encoder,
> > >  	if (!connector || connector->base.status !=
> > > connector_status_connected)
> > >  		return 0;
> > > 
> > > -	ret = drm_modeset_lock(&dev_priv->drm.mode_config.connection_mutex,
> > > -			       ctx);
> > > -	if (ret)
> > > -		return ret;
> > > +	if (ctx) {
> > > +		ret = drm_modeset_lock
> > > +			(&dev_priv->drm.mode_config.connection_mutex, ctx);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > 
> > ...why are we skipping locking anything if there isn't a
> > drm_modeset_acquire_ctx passed to us? And additionally, why wouldn't
> > there be
> > an acquisition context passed to us? We always need to be holding the
> > connection mutex and the respective CRTC lock whenever we're
> > retraining.
> 
> The original patch added all the locking to intel_dp_retrain_link.
> intel_dp_long_pulse is just called from intel_dp_detect, which takes the
> drm_modeset_lock depending on a valid crtc. I just assumed this is still
> sufficient, so for this case we don't need to acquire the lock again. If
> this is not sufficient, we can forward the ctx, state etc. from
> intel_dp_detect. I didn't check, if these values are the same values that
> intel_dp_retrain_link collects using different calls.

Since intel_dp_retrain_link() is meant to be called from pretty much anywhere
with an acquisition context, you want to assume that at the start of the
function there's no locks being held at all even that isn't currently the case
in the code (likewise, we have plans to call this function from other places
soon). As well, it's perfectly OK to grab a modeset lock twice in a row, as
long as it is only ever dropped once. That's why we made ww_mutex in the first
place!

That being said: we /cannot/ perform link training if the CRTC is not locked,
same for the connector lock. Things will break very badly if we're in the
middle of retraining a DP link if something else tries to change the display
state at the same time. I think the original code did this as well, but had
the locking in different spots. If it didn't then that code was definitely
broken, and was probably why we changed it in the first place.

> 
> There is also the "WARN_ON(!drm_modeset_is_locked(&dev_priv-
> >drm.mode_config.connection_mutex));" in intel_dp_long_pulse, which suggests
> we should also already have the connection_mutex when calling
> intel_dp_retrain_link later.
I wouldn't worry about that one unless you get a warning :).
intel_dp_long_pulse() is pretty much exclusively meant for handling long pulse
HPD events, so it's not really meant to be used outside of that context. That
is just there in case that changes someday in the future, at which point we
can add the appropriate locking.

> 
> All my assumptions are based on reading the original patch and a little bit
> of the surrounding code.
> 
> This patch is just a minimal change, which "works for me". Not sure if all
> the locking should be moved back to the retrain call sites, to make it more
> explicit?
(more comments down below...)
> 
> > > 
> > >  	conn_state = connector->base.state;
> > > 
> > > @@ -4372,9 +4362,11 @@ int intel_dp_retrain_link(struct intel_encoder
> > > *encoder,
> > >  	if (!crtc)
> > >  		return 0;
> > > 
> > > -	ret = drm_modeset_lock(&crtc->base.mutex, ctx);
> > > -	if (ret)
> > > -		return ret;
> > > +	if (ctx) {
> > > +		ret = drm_modeset_lock(&crtc->base.mutex, ctx);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > 
> > >  	crtc_state = to_intel_crtc_state(crtc->base.state);
> > > 
> > > @@ -4982,6 +4974,22 @@ intel_dp_long_pulse(struct intel_connector
> > > *connector)
> > >  		 */
> > >  		status = connector_status_disconnected;
> > >  		goto out;
> > > +	} else {
> > > +		/*
> > > +		 * If display is now connected check links status,
> > > +		 * there has been known issues of link loss triggering
> > > +		 * long pulse.
> > > +		 *
> > > +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
> > > +		 * weird HPD ping pong during modesets. So we can apparently
> > > +		 * end up with HPD going low during a modeset, and then
> > > +		 * going back up soon after. And once that happens we must
> > > +		 * retrain the link to get a picture. That's in case no
> > > +		 * userspace component reacted to intermittent HPD dip.
> > > +		 */
> > > +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> > > > base;
> > > 
> > > +
> > > +		intel_dp_retrain_link(encoder, NULL);
So, the only valid thing for you to do here is to make it so that we pass ctx
to intel_dp_long_pulse() so that it can grab the locks it needs for retraining
under the context that's being used for the connector probing.

> > >  	}
> > > 
> > >  	/*
> 
> 
-- 
Cheers,
	Lyude Paul

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

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

* [PATCH] v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-07 22:46         ` Lyude Paul
@ 2018-08-08  8:53           ` Jan-Marek Glogowski
  2018-08-13 16:35             ` Lyude Paul
  2018-08-16 18:03             ` Manasi Navare
  0 siblings, 2 replies; 17+ messages in thread
From: Jan-Marek Glogowski @ 2018-08-08  8:53 UTC (permalink / raw)
  To: Lyude Paul, intel-gfx

This re-applies the workaround for "some DP sinks, [which] are a
little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
quality check unconditionally during long pulse").
It makes the secondary AOC E2460P monitor connected via DP to an
acer Veriton N4640G usable again.

This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
DP link retraining into the ->post_hotplug() hook")

Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
---
 drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8e0e14b..22b2452 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
 	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
 }

-/*
- * If display is now connected check links status,
- * there has been known issues of link loss triggering
- * long pulse.
- *
- * Some sinks (eg. ASUS PB287Q) seem to perform some
- * weird HPD ping pong during modesets. So we can apparently
- * end up with HPD going low during a modeset, and then
- * going back up soon after. And once that happens we must
- * retrain the link to get a picture. That's in case no
- * userspace component reacted to intermittent HPD dip.
- */
 int intel_dp_retrain_link(struct intel_encoder *encoder,
 			  struct drm_modeset_acquire_ctx *ctx)
 {
@@ -4923,7 +4911,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 }

 static int
-intel_dp_long_pulse(struct intel_connector *connector)
+intel_dp_long_pulse(struct intel_connector *connector,
+		    struct drm_modeset_acquire_ctx *ctx)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
 	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
@@ -4982,6 +4971,22 @@ intel_dp_long_pulse(struct intel_connector *connector)
 		 */
 		status = connector_status_disconnected;
 		goto out;
+	} else {
+		/*
+		 * If display is now connected check links status,
+		 * there has been known issues of link loss triggering
+		 * long pulse.
+		 *
+		 * Some sinks (eg. ASUS PB287Q) seem to perform some
+		 * weird HPD ping pong during modesets. So we can apparently
+		 * end up with HPD going low during a modeset, and then
+		 * going back up soon after. And once that happens we must
+		 * retrain the link to get a picture. That's in case no
+		 * userspace component reacted to intermittent HPD dip.
+		 */
+		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
+
+		intel_dp_retrain_link(encoder, ctx);
 	}

 	/*
@@ -5043,7 +5048,7 @@ intel_dp_detect(struct drm_connector *connector,
 				return ret;
 		}

-		status = intel_dp_long_pulse(intel_dp->attached_connector);
+		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
 	}

 	intel_dp->detect_done = false;
-- 
2.1.4

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

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

* ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev3)
  2018-08-03 18:10 [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Jan-Marek Glogowski
                   ` (3 preceding siblings ...)
  2018-08-06 19:27 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-08-08 23:17 ` Patchwork
  2018-08-09  2:39 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-08 23:17 UTC (permalink / raw)
  To: Jan-Marek Glogowski; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev3)
URL   : https://patchwork.freedesktop.org/series/47694/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4635 -> Patchwork_9894 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47694/revisions/3/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    {igt@amdgpu/amd_prime@amd-to-i915}:
      {fi-kbl-8809g}:     NOTRUN -> FAIL (fdo#107341)

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       PASS -> DMESG-FAIL (fdo#106560, fdo#106947)

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362)

    
    ==== Possible fixes ====

    {igt@amdgpu/amd_basic@userptr}:
      {fi-kbl-8809g}:     INCOMPLETE (fdo#107402) -> PASS

    igt@drv_selftest@live_hangcheck:
      fi-cnl-psr:         DMESG-FAIL (fdo#106560) -> PASS
      fi-skl-guc:         DMESG-FAIL (fdo#107174) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b:
      {fi-byt-clapper}:   FAIL (fdo#107362) -> PASS

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

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107341 https://bugs.freedesktop.org/show_bug.cgi?id=107341
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402


== Participating hosts (50 -> 47) ==

  Additional (1): fi-hsw-peppy 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4635 -> Patchwork_9894

  CI_DRM_4635: a9f34f7e3bab765e2b1320a1b154a76be38602a7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4589: 779d2d42f9db6a2797d1ef50036af6fac4e62e73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9894: c8a6ddc5b49269876fdbda9f2d58f28d4694070a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

c8a6ddc5b492 v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9894/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev3)
  2018-08-03 18:10 [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Jan-Marek Glogowski
                   ` (4 preceding siblings ...)
  2018-08-08 23:17 ` ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev3) Patchwork
@ 2018-08-09  2:39 ` Patchwork
  5 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2018-08-09  2:39 UTC (permalink / raw)
  To: Jan-Marek Glogowski; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev3)
URL   : https://patchwork.freedesktop.org/series/47694/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4635_full -> Patchwork_9894_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9894_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9894_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_9894_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@other-init-3:
      shard-kbl:          PASS -> SKIP +5

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-kbl:          PASS -> DMESG-WARN (fdo#106684)

    igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
      shard-glk:          PASS -> FAIL (fdo#104873)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    igt@perf@enable-disable:
      shard-kbl:          PASS -> DMESG-FAIL (fdo#106064, fdo#106684)

    igt@perf_pmu@rc6-runtime-pm:
      shard-glk:          PASS -> FAIL (fdo#105010)

    
    ==== Possible fixes ====

    igt@gem_exec_await@wide-contexts:
      shard-apl:          FAIL (fdo#105900, fdo#106680) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS

    
  fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
  fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105900 https://bugs.freedesktop.org/show_bug.cgi?id=105900
  fdo#106064 https://bugs.freedesktop.org/show_bug.cgi?id=106064
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#106684 https://bugs.freedesktop.org/show_bug.cgi?id=106684
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4635 -> Patchwork_9894

  CI_DRM_4635: a9f34f7e3bab765e2b1320a1b154a76be38602a7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4589: 779d2d42f9db6a2797d1ef50036af6fac4e62e73 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9894: c8a6ddc5b49269876fdbda9f2d58f28d4694070a @ 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_9894/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-08  8:53           ` [PATCH] v3 " Jan-Marek Glogowski
@ 2018-08-13 16:35             ` Lyude Paul
  2018-08-16 18:03             ` Manasi Navare
  1 sibling, 0 replies; 17+ messages in thread
From: Lyude Paul @ 2018-08-13 16:35 UTC (permalink / raw)
  To: Jan-Marek Glogowski, intel-gfx

On Wed, 2018-08-08 at 10:53 +0200, Jan-Marek Glogowski wrote:
> This re-applies the workaround for "some DP sinks, [which] are a
> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> quality check unconditionally during long pulse").
> It makes the secondary AOC E2460P monitor connected via DP to an
> acer Veriton N4640G usable again.
> 
> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> DP link retraining into the ->post_hotplug() hook")
> 
> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
Just as a heads up I haven't forgotten about this! Coincidentally; it turns
out a real life friend of mine is seeing an issue that is very likely fixed by
this patch :), so I am going to have them do some checks for me to make sure
this fix is OK (it looks fine to me, but you can't be too careful)

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 8e0e14b..22b2452 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
> *intel_dp)
>  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
>  }
> 
> -/*
> - * If display is now connected check links status,
> - * there has been known issues of link loss triggering
> - * long pulse.
> - *
> - * Some sinks (eg. ASUS PB287Q) seem to perform some
> - * weird HPD ping pong during modesets. So we can apparently
> - * end up with HPD going low during a modeset, and then
> - * going back up soon after. And once that happens we must
> - * retrain the link to get a picture. That's in case no
> - * userspace component reacted to intermittent HPD dip.
> - */
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>  			  struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -4923,7 +4911,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  }
> 
>  static int
> -intel_dp_long_pulse(struct intel_connector *connector)
> +intel_dp_long_pulse(struct intel_connector *connector,
> +		    struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> @@ -4982,6 +4971,22 @@ intel_dp_long_pulse(struct intel_connector
> *connector)
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> +	} else {
> +		/*
> +		 * If display is now connected check links status,
> +		 * there has been known issues of link loss triggering
> +		 * long pulse.
> +		 *
> +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
> +		 * weird HPD ping pong during modesets. So we can apparently
> +		 * end up with HPD going low during a modeset, and then
> +		 * going back up soon after. And once that happens we must
> +		 * retrain the link to get a picture. That's in case no
> +		 * userspace component reacted to intermittent HPD dip.
> +		 */
> +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)-
> >base;
> +
> +		intel_dp_retrain_link(encoder, ctx);
>  	}
> 
>  	/*
> @@ -5043,7 +5048,7 @@ intel_dp_detect(struct drm_connector *connector,
>  				return ret;
>  		}
> 
> -		status = intel_dp_long_pulse(intel_dp->attached_connector);
> +		status = intel_dp_long_pulse(intel_dp->attached_connector,
> ctx);
>  	}
> 
>  	intel_dp->detect_done = false;
-- 
Cheers,
	Lyude Paul

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

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

* Re: [PATCH] v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-08  8:53           ` [PATCH] v3 " Jan-Marek Glogowski
  2018-08-13 16:35             ` Lyude Paul
@ 2018-08-16 18:03             ` Manasi Navare
  2018-08-16 19:12               ` Jan-Marek Glogowski
  2018-08-17 11:12               ` Jan-Marek Glogowski
  1 sibling, 2 replies; 17+ messages in thread
From: Manasi Navare @ 2018-08-16 18:03 UTC (permalink / raw)
  To: Jan-Marek Glogowski; +Cc: intel-gfx

On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote:
> This re-applies the workaround for "some DP sinks, [which] are a
> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> quality check unconditionally during long pulse").
> It makes the secondary AOC E2460P monitor connected via DP to an
> acer Veriton N4640G usable again.

Would be nice to add in the commit message that this sink sends a
long pulse to indicate link loss, hence check link status during long pulse.

If there is a FDO bug associated with this you could also add the
Bugzilla link before Sign off tag.
But other than that this looks good to me.

Manasi

> 
> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> DP link retraining into the ->post_hotplug() hook")
> 
> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 33 +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8e0e14b..22b2452 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp *intel_dp)
>  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
>  }
> 
> -/*
> - * If display is now connected check links status,
> - * there has been known issues of link loss triggering
> - * long pulse.
> - *
> - * Some sinks (eg. ASUS PB287Q) seem to perform some
> - * weird HPD ping pong during modesets. So we can apparently
> - * end up with HPD going low during a modeset, and then
> - * going back up soon after. And once that happens we must
> - * retrain the link to get a picture. That's in case no
> - * userspace component reacted to intermittent HPD dip.
> - */
>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>  			  struct drm_modeset_acquire_ctx *ctx)
>  {
> @@ -4923,7 +4911,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  }
> 
>  static int
> -intel_dp_long_pulse(struct intel_connector *connector)
> +intel_dp_long_pulse(struct intel_connector *connector,
> +		    struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> @@ -4982,6 +4971,22 @@ intel_dp_long_pulse(struct intel_connector *connector)
>  		 */
>  		status = connector_status_disconnected;
>  		goto out;
> +	} else {
> +		/*
> +		 * If display is now connected check links status,
> +		 * there has been known issues of link loss triggering
> +		 * long pulse.
> +		 *
> +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
> +		 * weird HPD ping pong during modesets. So we can apparently
> +		 * end up with HPD going low during a modeset, and then
> +		 * going back up soon after. And once that happens we must
> +		 * retrain the link to get a picture. That's in case no
> +		 * userspace component reacted to intermittent HPD dip.
> +		 */
> +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> +
> +		intel_dp_retrain_link(encoder, ctx);
>  	}
> 
>  	/*
> @@ -5043,7 +5048,7 @@ intel_dp_detect(struct drm_connector *connector,
>  				return ret;
>  		}
> 
> -		status = intel_dp_long_pulse(intel_dp->attached_connector);
> +		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
>  	}
> 
>  	intel_dp->detect_done = false;
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-16 18:03             ` Manasi Navare
@ 2018-08-16 19:12               ` Jan-Marek Glogowski
  2018-08-17 11:12               ` Jan-Marek Glogowski
  1 sibling, 0 replies; 17+ messages in thread
From: Jan-Marek Glogowski @ 2018-08-16 19:12 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

Am August 16, 2018 6:03:50 PM UTC schrieb Manasi Navare <manasi.d.navare@intel.com>:
>On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote:
>> This re-applies the workaround for "some DP sinks, [which] are a
>> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
>> quality check unconditionally during long pulse").
>> It makes the secondary AOC E2460P monitor connected via DP to an
>> acer Veriton N4640G usable again.
>
>Would be nice to add in the commit message that this sink sends a
>long pulse to indicate link loss, hence check link status during long
>pulse.

I have no idea, if this is happening. I just found the monitor wasn't working with the current kernel, when I was trying to debug an other bug I ran into while backporting DRM 4.15 to 4.4. I did a bisect while at it. Please read the FDO bug for more info.

>If there is a FDO bug associated with this you could also add the
>Bugzilla link before Sign off tag.
>But other than that this looks good to me.

FDO bug: https://bugs.freedesktop.org/show_bug.cgi?id=107446

>
>Manasi
>
>> 
>> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
>> DP link retraining into the ->post_hotplug() hook")
>> 
>> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 33
>+++++++++++++++++++--------------
>>  1 file changed, 19 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>b/drivers/gpu/drm/i915/intel_dp.c
>> index 8e0e14b..22b2452 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
>*intel_dp)
>>  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
>>  }
>> 
>> -/*
>> - * If display is now connected check links status,
>> - * there has been known issues of link loss triggering
>> - * long pulse.
>> - *
>> - * Some sinks (eg. ASUS PB287Q) seem to perform some
>> - * weird HPD ping pong during modesets. So we can apparently
>> - * end up with HPD going low during a modeset, and then
>> - * going back up soon after. And once that happens we must
>> - * retrain the link to get a picture. That's in case no
>> - * userspace component reacted to intermittent HPD dip.
>> - */
>>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>>  			  struct drm_modeset_acquire_ctx *ctx)
>>  {
>> @@ -4923,7 +4911,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>>  }
>> 
>>  static int
>> -intel_dp_long_pulse(struct intel_connector *connector)
>> +intel_dp_long_pulse(struct intel_connector *connector,
>> +		    struct drm_modeset_acquire_ctx *ctx)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>  	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
>> @@ -4982,6 +4971,22 @@ intel_dp_long_pulse(struct intel_connector
>*connector)
>>  		 */
>>  		status = connector_status_disconnected;
>>  		goto out;
>> +	} else {
>> +		/*
>> +		 * If display is now connected check links status,
>> +		 * there has been known issues of link loss triggering
>> +		 * long pulse.
>> +		 *
>> +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
>> +		 * weird HPD ping pong during modesets. So we can apparently
>> +		 * end up with HPD going low during a modeset, and then
>> +		 * going back up soon after. And once that happens we must
>> +		 * retrain the link to get a picture. That's in case no
>> +		 * userspace component reacted to intermittent HPD dip.
>> +		 */
>> +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> +
>> +		intel_dp_retrain_link(encoder, ctx);
>>  	}
>> 
>>  	/*
>> @@ -5043,7 +5048,7 @@ intel_dp_detect(struct drm_connector
>*connector,
>>  				return ret;
>>  		}
>> 
>> -		status = intel_dp_long_pulse(intel_dp->attached_connector);
>> +		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
>>  	}
>> 
>>  	intel_dp->detect_done = false;
>> -- 
>> 2.1.4
>> 

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

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

* [PATCH] v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-16 18:03             ` Manasi Navare
  2018-08-16 19:12               ` Jan-Marek Glogowski
@ 2018-08-17 11:12               ` Jan-Marek Glogowski
  2018-08-17 20:33                 ` Lyude Paul
  1 sibling, 1 reply; 17+ messages in thread
From: Jan-Marek Glogowski @ 2018-08-17 11:12 UTC (permalink / raw)
  To: Manasi Navare; +Cc: intel-gfx

Resend, as this was in my sent-mail folder, but it doesn't appear in the list archive…

Am August 16, 2018 6:03:50 PM UTC schrieb Manasi Navare <manasi.d.navare@intel.com>:
>On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote:
>> This re-applies the workaround for "some DP sinks, [which] are a
>> little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
>> quality check unconditionally during long pulse").
>> It makes the secondary AOC E2460P monitor connected via DP to an
>> acer Veriton N4640G usable again.
>
>Would be nice to add in the commit message that this sink sends a
>long pulse to indicate link loss, hence check link status during long
>pulse.

I have no idea, if this is happening. I just found the monitor wasn't working with the current
kernel, when I was trying to debug an other bug I ran into while backporting DRM 4.15 to 4.4. I did
a bisect while at it. Please read the FDO bug for more info.

>If there is a FDO bug associated with this you could also add the
>Bugzilla link before Sign off tag.
>But other than that this looks good to me.

FDO bug: https://bugs.freedesktop.org/show_bug.cgi?id=107446

Jan-Marek

>
>Manasi
>
>> 
>> This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
>> DP link retraining into the ->post_hotplug() hook")
>> 
>> Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 33
>+++++++++++++++++++--------------
>>  1 file changed, 19 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>b/drivers/gpu/drm/i915/intel_dp.c
>> index 8e0e14b..22b2452 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
>*intel_dp)
>>  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
>>  }
>> 
>> -/*
>> - * If display is now connected check links status,
>> - * there has been known issues of link loss triggering
>> - * long pulse.
>> - *
>> - * Some sinks (eg. ASUS PB287Q) seem to perform some
>> - * weird HPD ping pong during modesets. So we can apparently
>> - * end up with HPD going low during a modeset, and then
>> - * going back up soon after. And once that happens we must
>> - * retrain the link to get a picture. That's in case no
>> - * userspace component reacted to intermittent HPD dip.
>> - */
>>  int intel_dp_retrain_link(struct intel_encoder *encoder,
>>  			  struct drm_modeset_acquire_ctx *ctx)
>>  {
>> @@ -4923,7 +4911,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>>  }
>> 
>>  static int
>> -intel_dp_long_pulse(struct intel_connector *connector)
>> +intel_dp_long_pulse(struct intel_connector *connector,
>> +		    struct drm_modeset_acquire_ctx *ctx)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>>  	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
>> @@ -4982,6 +4971,22 @@ intel_dp_long_pulse(struct intel_connector
>*connector)
>>  		 */
>>  		status = connector_status_disconnected;
>>  		goto out;
>> +	} else {
>> +		/*
>> +		 * If display is now connected check links status,
>> +		 * there has been known issues of link loss triggering
>> +		 * long pulse.
>> +		 *
>> +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
>> +		 * weird HPD ping pong during modesets. So we can apparently
>> +		 * end up with HPD going low during a modeset, and then
>> +		 * going back up soon after. And once that happens we must
>> +		 * retrain the link to get a picture. That's in case no
>> +		 * userspace component reacted to intermittent HPD dip.
>> +		 */
>> +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> +
>> +		intel_dp_retrain_link(encoder, ctx);
>>  	}
>> 
>>  	/*
>> @@ -5043,7 +5048,7 @@ intel_dp_detect(struct drm_connector
>*connector,
>>  				return ret;
>>  		}
>> 
>> -		status = intel_dp_long_pulse(intel_dp->attached_connector);
>> +		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
>>  	}
>> 
>>  	intel_dp->detect_done = false;
>> -- 
>> 2.1.4
>> 

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

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

* Re: [PATCH] v3 drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse"
  2018-08-17 11:12               ` Jan-Marek Glogowski
@ 2018-08-17 20:33                 ` Lyude Paul
  0 siblings, 0 replies; 17+ messages in thread
From: Lyude Paul @ 2018-08-17 20:33 UTC (permalink / raw)
  To: Jan-Marek Glogowski, Manasi Navare; +Cc: intel-gfx

On Fri, 2018-08-17 at 13:12 +0200, Jan-Marek Glogowski wrote:
> Resend, as this was in my sent-mail folder, but it doesn't appear in the list
> archive…
> 
> Am August 16, 2018 6:03:50 PM UTC schrieb Manasi Navare <
> manasi.d.navare@intel.com>:
> > On Wed, Aug 08, 2018 at 10:53:35AM +0200, Jan-Marek Glogowski wrote:
> > > This re-applies the workaround for "some DP sinks, [which] are a
> > > little nuts" from commit 1a36147bb939 ("drm/i915: Perform link
> > > quality check unconditionally during long pulse").
> > > It makes the secondary AOC E2460P monitor connected via DP to an
> > > acer Veriton N4640G usable again.
> > 
> > Would be nice to add in the commit message that this sink sends a
> > long pulse to indicate link loss, hence check link status during long
> > pulse.
> 
> I have no idea, if this is happening. I just found the monitor wasn't working
> with the current
> kernel, when I was trying to debug an other bug I ran into while backporting
> DRM 4.15 to 4.4. I did
> a bisect while at it. Please read the FDO bug for more info.
I think you misunderstood-I believe mdnavare was asking if you could put the
comment that was on 1a36147bb939 originally in your patch

> 
> > If there is a FDO bug associated with this you could also add the
> > Bugzilla link before Sign off tag.
> > But other than that this looks good to me.
> 
> FDO bug: https://bugs.freedesktop.org/show_bug.cgi?id=107446
> 
> Jan-Marek
> 
> > 
> > Manasi
> > 
> > > 
> > > This hunk was dropped in commit c85d200e8321 ("drm/i915: Move SST
> > > DP link retraining into the ->post_hotplug() hook")
> > > 
> > > Signed-off-by: Jan-Marek Glogowski <glogow@fbihome.de>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 33
> > 
> > +++++++++++++++++++--------------
> > >  1 file changed, 19 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > 
> > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 8e0e14b..22b2452 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4333,18 +4333,6 @@ intel_dp_needs_link_retrain(struct intel_dp
> > 
> > *intel_dp)
> > >  	return !drm_dp_channel_eq_ok(link_status, intel_dp->lane_count);
> > >  }
> > > 
> > > -/*
> > > - * If display is now connected check links status,
> > > - * there has been known issues of link loss triggering
> > > - * long pulse.
> > > - *
> > > - * Some sinks (eg. ASUS PB287Q) seem to perform some
> > > - * weird HPD ping pong during modesets. So we can apparently
> > > - * end up with HPD going low during a modeset, and then
> > > - * going back up soon after. And once that happens we must
> > > - * retrain the link to get a picture. That's in case no
> > > - * userspace component reacted to intermittent HPD dip.
> > > - */
> > >  int intel_dp_retrain_link(struct intel_encoder *encoder,
> > >  			  struct drm_modeset_acquire_ctx *ctx)
> > >  {
> > > @@ -4923,7 +4911,8 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> > >  }
> > > 
> > >  static int
> > > -intel_dp_long_pulse(struct intel_connector *connector)
> > > +intel_dp_long_pulse(struct intel_connector *connector,
> > > +		    struct drm_modeset_acquire_ctx *ctx)
> > >  {
> > >  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> > >  	struct intel_dp *intel_dp = intel_attached_dp(&connector->base);
> > > @@ -4982,6 +4971,22 @@ intel_dp_long_pulse(struct intel_connector
> > 
> > *connector)
> > >  		 */
> > >  		status = connector_status_disconnected;
> > >  		goto out;
> > > +	} else {
> > > +		/*
> > > +		 * If display is now connected check links status,
> > > +		 * there has been known issues of link loss triggering
> > > +		 * long pulse.
> > > +		 *
> > > +		 * Some sinks (eg. ASUS PB287Q) seem to perform some
> > > +		 * weird HPD ping pong during modesets. So we can apparently
> > > +		 * end up with HPD going low during a modeset, and then
> > > +		 * going back up soon after. And once that happens we must
> > > +		 * retrain the link to get a picture. That's in case no
> > > +		 * userspace component reacted to intermittent HPD dip.
> > > +		 */
> > > +		struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
> > > +
> > > +		intel_dp_retrain_link(encoder, ctx);
> > >  	}
> > > 
> > >  	/*
> > > @@ -5043,7 +5048,7 @@ intel_dp_detect(struct drm_connector
> > 
> > *connector,
> > >  				return ret;
> > >  		}
> > > 
> > > -		status = intel_dp_long_pulse(intel_dp->attached_connector);
> > > +		status = intel_dp_long_pulse(intel_dp->attached_connector, ctx);
> > >  	}
> > > 
> > >  	intel_dp->detect_done = false;
> > > -- 
> > > 2.1.4
> > > 
> 
> 

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

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

end of thread, other threads:[~2018-08-17 20:33 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-03 18:10 [PATCH] drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" Jan-Marek Glogowski
2018-08-04  1:36 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2018-08-06 10:25   ` [PATCH] v2 " Jan-Marek Glogowski
2018-08-07 19:33     ` Lyude Paul
2018-08-07 22:26       ` Jan-Marek Glogowski
2018-08-07 22:46         ` Lyude Paul
2018-08-08  8:53           ` [PATCH] v3 " Jan-Marek Glogowski
2018-08-13 16:35             ` Lyude Paul
2018-08-16 18:03             ` Manasi Navare
2018-08-16 19:12               ` Jan-Marek Glogowski
2018-08-17 11:12               ` Jan-Marek Glogowski
2018-08-17 20:33                 ` Lyude Paul
2018-08-04  1:53 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-08-06 16:34 ` ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev2) Patchwork
2018-08-06 19:27 ` ✓ Fi.CI.IGT: " Patchwork
2018-08-08 23:17 ` ✓ Fi.CI.BAT: success for drm/i915: Re-apply "Perform link quality check, unconditionally during long pulse" (rev3) Patchwork
2018-08-09  2:39 ` ✓ 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.