On 20.11.2015 16:52, cpaul at redhat.com wrote:
> From: Stephen Chandler Paul <cpaul at 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 at redhat.com>
Yeah, that's something I always wondered a about bit as well.
Debouncing is something very common done in electronics, but as far as I
know the HPD pins don't necessary have an RC circuit so we might need to
handle this case in software here.
A delay of something between 10-30ms between the last HPD interrupt and
further processing of the signal doesn't sounds like such a bad idea.
Retrying on the other hand doesn't necessarily improve the situation
cause the delay introduced by this might not be enough.
So I would rather vote for a fixed delay between an HPD interrupt and
actually starting to process anything.
Regards,
Christian.
> ---
> 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)
