[PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt
From: Lyude <cp...@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: Lyude
---
Sending this version of the patch because Jerome says this will probably be the
least controversial of the potential fixes. Instead of sending userspace tons of
hotplug events, we just reschedule the hotplug work.
drivers/gpu/drm/radeon/cik.c | 2 +-
drivers/gpu/drm/radeon/evergreen.c | 2 +-
drivers/gpu/drm/radeon/r100.c | 2 +-
drivers/gpu/drm/radeon/r600.c | 2 +-
drivers/gpu/drm/radeon/radeon.h| 2 +-
drivers/gpu/drm/radeon/radeon_connectors.c | 21 -
drivers/gpu/drm/radeon/radeon_irq_kms.c| 8
drivers/gpu/drm/radeon/radeon_mode.h | 1 +
drivers/gpu/drm/radeon/rs600.c | 2 +-
drivers/gpu/drm/radeon/si.c| 2 +-
10 files changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 248953d..6801a0c 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -8472,7 +8472,7 @@ restart_ih:
if (queue_dp)
schedule_work(>dp_work);
if (queue_hotplug)
- schedule_work(>hotplug_work);
+ schedule_delayed_work(>hotplug_work, 0);
if (queue_reset) {
rdev->needs_reset = true;
wake_up_all(>fence_queue);
diff --git a/drivers/gpu/drm/radeon/evergreen.c
b/drivers/gpu/drm/radeon/evergreen.c
index 0acde19..f8e4986 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -5368,7 +5368,7 @@ restart_ih:
if (queue_dp)
schedule_work(>dp_work);
if (queue_hotplug)
- schedule_work(>hotplug_work);
+ schedule_delayed_work(>hotplug_work, 0);
if (queue_hdmi)
schedule_work(>audio_work);
if (queue_thermal && rdev->pm.dpm_enabled)
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 238b13f..2df3c86 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -806,7 +806,7 @@ int r100_irq_process(struct radeon_device *rdev)
status = r100_irq_ack(rdev);
}
if (queue_hotplug)
- schedule_work(>hotplug_work);
+ schedule_delayed_work(>hotplug_work, 0);
if (rdev->msi_enabled) {
switch (rdev->family) {
case CHIP_RS400:
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 4ea5b10..cc2fdf0 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -4276,7 +4276,7 @@ restart_ih:
WREG32(IH_RB_RPTR, rptr);
}
if (queue_hotplug)
- schedule_work(>hotplug_work);
+ schedule_delayed_work(>hotplug_work, 0);
if (queue_hdmi)
schedule_work(>audio_work);
if (queue_thermal && rdev->pm.dpm_enabled)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index b6cbd81..87db649 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -2414,7 +2414,7 @@ struct radeon_device {
struct r600_ih ih; /* r6/700 interrupt ring */
struct radeon_rlc rlc;
struct radeon_mec mec;
- struct work_struct hotplug_work;
+ struct delayed_work hotplug_work;
struct work_struct dp_work;
struct work_struct audio_work;
int num_crtc; /* number of crtcs */
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 5a2cafb..340f3f5 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1234,13 +1234,32 @@ radeon_dvi_detect(struct drm_connector *connector, bool
force)
if (r < 0)
return connector_status_disconnected;
+ if (radeon_connector->detected_hpd_without_ddc) {
+ force = true;
+ radeon_connector->detected_hpd_without_ddc = false;
+ }
+
if (!force &
[PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt
From: Stephen Chandler Paul <cp...@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
---
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 mutexmn_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 = >mode_config;
struct drm_connector *connector;
+
[PATCH] DRM - radeon: Don't link train DisplayPort on HPD until we get the dpcd
From: Stephen Chandler Paul <cp...@redhat.com>
Most of the time this isn't an issue since hotplugging an adaptor will
trigger a crtc mode change which in turn, causes the driver to probe
every DisplayPort for a dpcd. However, in cases where hotplugging
doesn't cause a mode change (specifically when one unplugs a monitor
from a DisplayPort connector, then plugs that same monitor back in
seconds later on the same port without any other monitors connected), we
never probe for the dpcd before starting the initial link training. What
happens from there looks like this:
- GPU has only one monitor connected. It's connected via
DisplayPort, and does not go through an adaptor of any sort.
- User unplugs DisplayPort connector from GPU.
- Change in HPD is detected by the driver, we probe every
DisplayPort for a possible connection.
- Probe the port the user originally had the monitor connected
on for it's dpcd. This fails, and we clear the first (and only
the first) byte of the dpcd to indicate we no longer have a
dpcd for this port.
- User plugs the previously disconnected monitor back into the
same DisplayPort.
- radeon_connector_hotplug() is called before everyone else,
and tries to handle the link training. Since only the first
byte of the dpcd is zeroed, the driver is able to complete
link training but does so against the wrong dpcd, causing it
to initialize the link with the wrong settings.
- Display stays blank (usually), dpcd is probed after the
initial link training, and the driver prints no obvious
messages to the log.
In theory, since only one byte of the dpcd is chopped off (specifically,
the byte that contains the revision information for DisplayPort), it's
not entirely impossible that this bug may not show on certain monitors.
For instance, the only reason this bug was visible on my ASUS PB238
monitor was due to the fact that this monitor using the enhanced framing
symbol sequence, the flag for which is ignored if the radeon driver
thinks that the DisplayPort version is below 1.1.
Signed-off-by: Stephen Chandler Paul
Reviewed-by: Jerome Glisse
---
drivers/gpu/drm/radeon/radeon_connectors.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 94b21ae..5a2cafb 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -95,6 +95,11 @@ void radeon_connector_hotplug(struct drm_connector
*connector)
if (!radeon_hpd_sense(rdev, radeon_connector->hpd.hpd))
{
drm_helper_connector_dpms(connector,
DRM_MODE_DPMS_OFF);
} else if
(radeon_dp_needs_link_train(radeon_connector)) {
+ /* Don't try to start link training before we
+* have the dpcd */
+ if (!radeon_dp_getdpcd(radeon_connector))
+ return;
+
/* set it to OFF so that
drm_helper_connector_dpms()
* won't return immediately since the current
state
* is ON at this point.
--
2.4.3
