[PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt

[Boszormenyi Zoltan]

2015-11-20 16:52 keltezéssel, cpaul at redhat.com írta:
> 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>
> ---
> 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)

Does this patch help in case I use a DP-2-DVI-D or HDMI-2-DVI-D converter cable?

Thanks in advance,
Zoltán Böszörményi