All of lore.kernel.org
 help / color / mirror / Atom feed
From: ville.syrjala@linux.intel.com
To: intel-gfx@lists.freedesktop.org
Cc: stable@vger.kernel.org, Manasi Navare <manasi.d.navare@intel.com>,
	Palmer Dabbelt <palmer@dabbelt.com>
Subject: [PATCH v3] drm/i915: Perform link quality check unconditionally during long pulse
Date: Wed, 12 Apr 2017 22:30:17 +0300	[thread overview]
Message-ID: <20170412193017.21029-1-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20170216153007.14868-1-ville.syrjala@linux.intel.com>

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

Apparently some DP sinks are a little nuts and cause HPD to drop
intermittently during modesets. This happens eg. on an ASUS PB287Q.
In oder to recover from this we can't really use the previous
connector status to determine if the link needs retraining, so let's
just ignore that piece of information and do the retrain
unconditionally. We do of course still check whether the link is
supposed to be running or not.

To actually get read out the EDID and update things properly we
also need to nuke the goto out added by commit 7d23e3c37bb3
("drm/i915: Cleaning up intel_dp_hpd_pulse"). I'm actually not sure
why that was there. Perhaps to avoid an EDID read if the connector
status didn't appear to change, but that sort of thing is quite racy
and would have failed anyway if we failed to keep up with the
hotplugs (if we missed the HPD down in between two HPD ups). And
now that we take this codepath unconditionally we definitely need
to drop the goto as otherwise we would never do the EDID read.

v2: Drop the goto that made us skip EDID reads entirely. Doh!
v3: Rebase due to locking changes
    s/apparely/apparently/ in the comment (Chris)

Cc: stable@vger.kernel.org
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Reported-by: Palmer Dabbelt <palmer@dabbelt.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99766
References: https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 16b7bf7af537..09601a22d3cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4685,9 +4685,20 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		 */
 		status = connector_status_disconnected;
 		goto out;
-	} else if (connector->status == connector_status_connected) {
+	} else {
+		/*
+		 * If display is now connected check links status,
+		 * there has been known issues of link loss triggerring
+		 * 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.
+		 */
 		intel_dp_check_link_status(intel_dp);
-		goto out;
 	}
 
 	/*
-- 
2.10.2

WARNING: multiple messages have this Message-ID (diff)
From: ville.syrjala@linux.intel.com
To: intel-gfx@lists.freedesktop.org
Cc: Palmer Dabbelt <palmer@dabbelt.com>, stable@vger.kernel.org
Subject: [PATCH v3] drm/i915: Perform link quality check unconditionally during long pulse
Date: Wed, 12 Apr 2017 22:30:17 +0300	[thread overview]
Message-ID: <20170412193017.21029-1-ville.syrjala@linux.intel.com> (raw)
In-Reply-To: <20170216153007.14868-1-ville.syrjala@linux.intel.com>

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

Apparently some DP sinks are a little nuts and cause HPD to drop
intermittently during modesets. This happens eg. on an ASUS PB287Q.
In oder to recover from this we can't really use the previous
connector status to determine if the link needs retraining, so let's
just ignore that piece of information and do the retrain
unconditionally. We do of course still check whether the link is
supposed to be running or not.

To actually get read out the EDID and update things properly we
also need to nuke the goto out added by commit 7d23e3c37bb3
("drm/i915: Cleaning up intel_dp_hpd_pulse"). I'm actually not sure
why that was there. Perhaps to avoid an EDID read if the connector
status didn't appear to change, but that sort of thing is quite racy
and would have failed anyway if we failed to keep up with the
hotplugs (if we missed the HPD down in between two HPD ups). And
now that we take this codepath unconditionally we definitely need
to drop the goto as otherwise we would never do the EDID read.

v2: Drop the goto that made us skip EDID reads entirely. Doh!
v3: Rebase due to locking changes
    s/apparely/apparently/ in the comment (Chris)

Cc: stable@vger.kernel.org
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Reported-by: Palmer Dabbelt <palmer@dabbelt.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99766
References: https://lists.freedesktop.org/archives/intel-gfx/2017-February/119779.html
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_dp.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 16b7bf7af537..09601a22d3cc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4685,9 +4685,20 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		 */
 		status = connector_status_disconnected;
 		goto out;
-	} else if (connector->status == connector_status_connected) {
+	} else {
+		/*
+		 * If display is now connected check links status,
+		 * there has been known issues of link loss triggerring
+		 * 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.
+		 */
 		intel_dp_check_link_status(intel_dp);
-		goto out;
 	}
 
 	/*
-- 
2.10.2

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

  parent reply	other threads:[~2017-04-12 19:30 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-10 22:44 [PATCH] drm/i915: Fix DisplayPort Hotplug Palmer Dabbelt
2017-02-14  8:22 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-02-14  8:54   ` Saarinen, Jani
2017-02-14 15:01 ` [PATCH] " Ville Syrjälä
2017-02-14 18:48   ` Palmer Dabbelt
2017-02-14 19:00     ` Ville Syrjälä
2017-02-16  2:58       ` Palmer Dabbelt
2017-02-16 15:26         ` Ville Syrjälä
2017-02-16 15:30           ` [PATCH] drm/i915: Perform link quality check unconditionally during long pulse ville.syrjala
2017-02-16 15:30             ` ville.syrjala
2017-02-16 15:39             ` Palmer Dabbelt
2017-02-16 15:39               ` Palmer Dabbelt
2017-02-16 15:49               ` Ville Syrjälä
2017-02-16 15:49                 ` Ville Syrjälä
2017-02-16 17:07             ` [Intel-gfx] " Manasi Navare
2017-02-16 17:07               ` Manasi Navare
2017-02-16 17:18               ` Ville Syrjälä
2017-02-16 17:18                 ` Ville Syrjälä
2017-02-16 17:24                 ` [Intel-gfx] " Manasi Navare
2017-02-16 17:24                   ` Manasi Navare
2017-02-16 17:46                   ` [Intel-gfx] " Ville Syrjälä
2017-02-16 17:46                     ` Ville Syrjälä
2017-02-23  4:00                     ` [Intel-gfx] " Palmer Dabbelt
2017-02-23  4:00                       ` Palmer Dabbelt
2017-02-23  9:22                       ` [Intel-gfx] " Ville Syrjälä
2017-02-23  9:22                         ` Ville Syrjälä
2017-03-13 20:53             ` [PATCH v2] " ville.syrjala
2017-03-13 20:53               ` ville.syrjala
2017-03-13 21:20               ` [Intel-gfx] " Chris Wilson
2017-03-13 21:20                 ` Chris Wilson
2017-03-13 23:09               ` Manasi Navare
2017-03-13 23:09                 ` Manasi Navare
2017-03-14 10:15                 ` Ville Syrjälä
2017-03-14 10:15                   ` Ville Syrjälä
2017-04-12 19:30             ` ville.syrjala [this message]
2017-04-12 19:30               ` [PATCH v3] " ville.syrjala
2017-04-13 12:27               ` Ville Syrjälä
2017-04-13 12:27                 ` Ville Syrjälä
2017-02-16 19:52 ` ✓ Fi.CI.BAT: success for drm/i915: Fix DisplayPort Hotplug (rev2) Patchwork
2017-03-13 21:47 ` ✓ Fi.CI.BAT: success for drm/i915: Fix DisplayPort Hotplug (rev3) Patchwork
2017-04-12 19:47 ` ✓ Fi.CI.BAT: success for drm/i915: Fix DisplayPort Hotplug (rev4) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170412193017.21029-1-ville.syrjala@linux.intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=manasi.d.navare@intel.com \
    --cc=palmer@dabbelt.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.