All of lore.kernel.org
 help / color / mirror / Atom feed
From: cpaul@redhat.com
To: "Alex Deucher" <alexander.deucher@amd.com>,
	"Christian König" <christian.koenig@amd.com>,
	"David Airlie" <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Cc: Jerome Glisse <jglisse@redhat.com>,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>
Subject: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt
Date: Fri, 20 Nov 2015 10:52:20 -0500	[thread overview]
Message-ID: <1448034740-30193-1-git-send-email-cpaul@redhat.com> (raw)

From: Stephen Chandler Paul <cpaul@redhat.com>

HPD signals on DVI ports can be fired off before the pins required for
DDC probing actually make contact, due to the pins for HPD making
contact first. This results in a HPD signal being asserted but DDC
probing failing, resulting in hotplugging occasionally failing.

This is somewhat rare on most cards (depending on what angle you plug
the DVI connector in), but on some cards it happens constantly. The
Radeon R5 on the machine used for testing this patch for instance, runs
into this issue just about every time I try to hotplug a DVI monitor and
as a result hotplugging almost never works.

Rescheduling the hotplug work for a second when we run into an HPD
signal with a failing DDC probe usually gives enough time for the rest
of the connector's pins to make contact, and fixes this issue.

Signed-off-by: Stephen Chandler Paul <cpaul@redhat.com>
---
So this one has kind of been a tough sell with Jerome, mostly because it's
somewhat of a hack. Unfortunately however I've managed to find machines where
DVI hotplugging literally doesn't work without a patch like this. We've already
tried a couple of ways of handling the situation of retriggering ddc probes:

* Trying the DDC probe in the radeon_dvi_detect() function multiple times.
* Trying to reschedule the hotplug_work task whenever DDC probing fails on DVI
  but we got a hpd signal (this ended up being a much more complicated patch
  then anticipated)
* Doing what we do right now, which is just triggering userspace to rescan all
  the ports when the hpd signal is asserted by the DVI port but there's no DDC
  probe, and repeating until at least a second passes.

All of these actually work, but I guess it's a question of which one is less of
a hack. If anyone here can think of a cleaner way of handling this feel free to
let me know.

 drivers/gpu/drm/radeon/radeon.h            |  3 +++
 drivers/gpu/drm/radeon/radeon_connectors.c | 20 +++++++++++++++++---
 drivers/gpu/drm/radeon/radeon_irq_kms.c    |  2 ++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index b6cbd81..d63f0fe 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2460,6 +2460,9 @@ struct radeon_device {
 	/* amdkfd interface */
 	struct kfd_dev		*kfd;
 
+	/* last time we received an hpd signal */
+	unsigned long hpd_time;
+
 	struct mutex	mn_lock;
 	DECLARE_HASHTABLE(mn_hash, 7);
 };
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 5a2cafb..4ee9440 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1228,19 +1228,33 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 	const struct drm_encoder_helper_funcs *encoder_funcs;
 	int i, r;
 	enum drm_connector_status ret = connector_status_disconnected;
-	bool dret = false, broken_edid = false;
+	bool dret = false, broken_edid = false, hpd_unchanged;
 
 	r = pm_runtime_get_sync(connector->dev->dev);
 	if (r < 0)
 		return connector_status_disconnected;
 
-	if (!force && radeon_check_hpd_status_unchanged(connector)) {
+	hpd_unchanged = radeon_check_hpd_status_unchanged(connector);
+	if (!force && hpd_unchanged) {
 		ret = connector->status;
 		goto exit;
 	}
 
-	if (radeon_connector->ddc_bus)
+	if (radeon_connector->ddc_bus) {
 		dret = radeon_ddc_probe(radeon_connector, false);
+
+		/* Sometimes the pins required for the DDC probe on DVI
+		 * connectors don't make contact at the same time that the ones
+		 * for HPD do. If the DDC probe fails even though we had an HPD
+		 * signal, signal userspace to try again */
+		if (!dret && !hpd_unchanged &&
+		    connector->status != connector_status_connected &&
+		    time_before(jiffies, rdev->hpd_time + msecs_to_jiffies(1000))) {
+			DRM_DEBUG_KMS("%s: hpd asserted but ddc probe failed, retrying\n",
+				      connector->name);
+			drm_sysfs_hotplug_event(dev);
+		}
+	}
 	if (dret) {
 		radeon_connector->detected_by_load = false;
 		radeon_connector_free_edid(connector);
diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c b/drivers/gpu/drm/radeon/radeon_irq_kms.c
index 171d3e4..579c22c 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -79,6 +79,8 @@ static void radeon_hotplug_work_func(struct work_struct *work)
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct drm_connector *connector;
 
+	rdev->hpd_time = jiffies;
+
 	/* we can race here at startup, some boards seem to trigger
 	 * hotplug irqs when they shouldn't. */
 	if (!rdev->mode_info.mode_config_initialized)
-- 
2.5.0


             reply	other threads:[~2015-11-20 15:52 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-20 15:52 cpaul [this message]
2015-11-21 14:22 ` [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt Christian König
2015-11-21 14:22   ` Christian König
2015-11-21 14:49   ` Daniel Stone
2015-11-21 14:49     ` Daniel Stone
2015-11-21 15:22     ` Christian König
2015-11-21 15:22       ` Christian König
2015-11-23  2:44       ` Lyude
2015-11-23  2:44         ` Lyude
2015-11-23 14:20         ` Deucher, Alexander
2015-11-23 14:20           ` Deucher, Alexander
2015-11-23 15:43           ` Lyude
2015-11-23 15:43             ` Lyude
2015-11-23 17:48             ` Lyude
2015-11-23 17:48               ` Lyude
2015-11-30 15:36           ` Lyude
2015-11-30 15:36             ` Lyude
2015-12-03 23:26           ` [PATCH v2] " cpaul
2015-12-04  8:53             ` Christian König
2015-12-04  8:53               ` Christian König
2015-12-04  9:43               ` Boszormenyi Zoltan
2015-12-04  9:43                 ` Boszormenyi Zoltan
2015-12-04  9:54                 ` Christian König
2015-12-04  9:54                   ` Christian König
2015-12-04 18:09               ` Alex Deucher
2015-12-04 18:09                 ` Alex Deucher
2015-11-23  8:56     ` [PATCH] " Daniel Vetter
2015-11-23  8:55   ` Daniel Vetter
2015-11-23  8:55     ` Daniel Vetter
2015-12-03 14:58 ` Boszormenyi Zoltan
2015-12-03 14:58   ` Boszormenyi Zoltan

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=1448034740-30193-1-git-send-email-cpaul@redhat.com \
    --to=cpaul@redhat.com \
    --cc=airlied@linux.ie \
    --cc=alexander.deucher@amd.com \
    --cc=benjamin.tissoires@redhat.com \
    --cc=christian.koenig@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jglisse@redhat.com \
    --cc=linux-kernel@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.