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

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

[cpaul]

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

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

[Christian König]

On 20.11.2015 16:52, cpaul@redhat.com wrote:
> 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>

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)

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

[Christian König]

On 20.11.2015 16:52, cpaul@redhat.com wrote:
> 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>

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)

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Stone]

Hi,

On 21 November 2015 at 14:22, Christian König <christian.koenig@amd.com> wrote:
> On 20.11.2015 16:52, cpaul@redhat.com wrote:
>> 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>
>
>
> 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.

Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g. those
on marginal power) which send you HPD storms as well. But DP relies on
'short HPD' pulses which can be as brief as 2ms. So attempting to
totally debounce all HPD won't work.

Cheers,
Daniel

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

[Daniel Stone]

Hi,

On 21 November 2015 at 14:22, Christian König <christian.koenig@amd.com> wrote:
> On 20.11.2015 16:52, cpaul@redhat.com wrote:
>> 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>
>
>
> 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.

Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g. those
on marginal power) which send you HPD storms as well. But DP relies on
'short HPD' pulses which can be as brief as 2ms. So attempting to
totally debounce all HPD won't work.

Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Christian König]

On 21.11.2015 15:49, Daniel Stone wrote:
> Hi,
>
> On 21 November 2015 at 14:22, Christian König <christian.koenig@amd.com> wrote:
>> On 20.11.2015 16:52, cpaul@redhat.com wrote:
>>> 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>
>>
>> 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.
> Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g. those
> on marginal power) which send you HPD storms as well. But DP relies on
> 'short HPD' pulses which can be as brief as 2ms. So attempting to
> totally debounce all HPD won't work.
Well the discussion so far was about HPD on DVI only.

I'm not so deep into DP, but why should it uses HPD pulses of less than 2ms?

Regards,
Christian.

>
> Cheers,
> Daniel

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

[Christian König]

On 21.11.2015 15:49, Daniel Stone wrote:
> Hi,
>
> On 21 November 2015 at 14:22, Christian König <christian.koenig@amd.com> wrote:
>> On 20.11.2015 16:52, cpaul@redhat.com wrote:
>>> 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>
>>
>> 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.
> Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g. those
> on marginal power) which send you HPD storms as well. But DP relies on
> 'short HPD' pulses which can be as brief as 2ms. So attempting to
> totally debounce all HPD won't work.
Well the discussion so far was about HPD on DVI only.

I'm not so deep into DP, but why should it uses HPD pulses of less than 2ms?

Regards,
Christian.

>
> Cheers,
> Daniel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lyude]

On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> On 21.11.2015 15:49, Daniel Stone wrote:
> > Hi,
> > 
> > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > .com> wrote:
> > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > 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>
> > > 
> > > 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.
Unfortunately the delay needed to make hotplugging work on the system
mentioned in the commit log can actually be over 700ms.
> > > 
> > > Retrying on the other hand doesn't necessarily improve the
> > > situation cause
> > > the delay introduced by this might not be enough.
Yeah, but I would think it would make sense to retry here so long as we
back off after a certain time. This would also have the benefit of
skipping this delay on systems that don't need it.
> > > 
> > > So I would rather vote for a fixed delay between an HPD interrupt
> > > and
> > > actually starting to process anything.
> > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > those
> > on marginal power) which send you HPD storms as well. But DP relies
> > on
> > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > totally debounce all HPD won't work.
> Well the discussion so far was about HPD on DVI only.
> 
> I'm not so deep into DP, but why should it uses HPD pulses of less
> than 2ms?
This is part of the DP spec iirc. This being said though, the issue
here with the HPD signal coming before the connector is ready only
happens on DVI. I haven't ever run into this issue with any HDMI cables
or DP cables, so I'm against imposing this on all connectors.

One of the solutions I've been thinking about with this: In
radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
probe until at least a second has passed, after which we back off and
assume the port is disconnected.
> 
> Regards,
> Christian.
> 
> > 
> > Cheers,
> > Daniel
> 
-- 
Cheers,
	Lyude

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

[Lyude]

On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> On 21.11.2015 15:49, Daniel Stone wrote:
> > Hi,
> > 
> > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > .com> wrote:
> > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > 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>
> > > 
> > > 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.
Unfortunately the delay needed to make hotplugging work on the system
mentioned in the commit log can actually be over 700ms.
> > > 
> > > Retrying on the other hand doesn't necessarily improve the
> > > situation cause
> > > the delay introduced by this might not be enough.
Yeah, but I would think it would make sense to retry here so long as we
back off after a certain time. This would also have the benefit of
skipping this delay on systems that don't need it.
> > > 
> > > So I would rather vote for a fixed delay between an HPD interrupt
> > > and
> > > actually starting to process anything.
> > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > those
> > on marginal power) which send you HPD storms as well. But DP relies
> > on
> > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > totally debounce all HPD won't work.
> Well the discussion so far was about HPD on DVI only.
> 
> I'm not so deep into DP, but why should it uses HPD pulses of less
> than 2ms?
This is part of the DP spec iirc. This being said though, the issue
here with the HPD signal coming before the connector is ready only
happens on DVI. I haven't ever run into this issue with any HDMI cables
or DP cables, so I'm against imposing this on all connectors.

One of the solutions I've been thinking about with this: In
radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
probe until at least a second has passed, after which we back off and
assume the port is disconnected.
> 
> Regards,
> Christian.
> 
> > 
> > Cheers,
> > Daniel
> 
-- 
Cheers,
	Lyude

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Sat, Nov 21, 2015 at 03:22:07PM +0100, Christian König wrote:
> On 20.11.2015 16:52, cpaul@redhat.com wrote:
> >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>
> 
> 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.

Might be interesting to compare what we do in i915 in latest kernels:

- in the hpd handler we check the hpd status bits before probing the edid.
  This will take care of slow unplug case where we get the hpd
  interrupt/status change, but i2c is still working.

- but because crap screens (or whatever it is) if hpd is _not_ set we
  retest the status for 30ms (and bail out as soon as hpd status is
  asserted). This seems to be enough time to settle all screens.

Note that this is in intel_hdmi.c since native dvi ports on intel hw also
always supported hdmi features. Not sure whether we could extract this
into some kind of helper and share.
-Daniel


> 
> 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)
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Daniel Vetter]

On Sat, Nov 21, 2015 at 03:22:07PM +0100, Christian König wrote:
> On 20.11.2015 16:52, cpaul@redhat.com wrote:
> >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>
> 
> 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.

Might be interesting to compare what we do in i915 in latest kernels:

- in the hpd handler we check the hpd status bits before probing the edid.
  This will take care of slow unplug case where we get the hpd
  interrupt/status change, but i2c is still working.

- but because crap screens (or whatever it is) if hpd is _not_ set we
  retest the status for 30ms (and bail out as soon as hpd status is
  asserted). This seems to be enough time to settle all screens.

Note that this is in intel_hdmi.c since native dvi ports on intel hw also
always supported hdmi features. Not sure whether we could extract this
into some kind of helper and share.
-Daniel


> 
> 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)
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Sat, Nov 21, 2015 at 02:49:20PM +0000, Daniel Stone wrote:
> Hi,
> 
> On 21 November 2015 at 14:22, Christian König <christian.koenig@amd.com> wrote:
> > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> >> 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>
> >
> >
> > 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.
> 
> Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g. those
> on marginal power) which send you HPD storms as well. But DP relies on
> 'short HPD' pulses which can be as brief as 2ms. So attempting to
> totally debounce all HPD won't work.

In our experience in i915 DP hpd works perfectly on all machines. At least
we never had to add timeout hacks to settle hpd, storm handling code isn't
enabled for DP either and we always checked hpd status bits to decide
whether something is connected or not.

hdmi/dvi is the troublemaker which needs settling time and storm handling.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Deucher, Alexander]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4066 bytes --]

> -----Original Message-----
> From: Lyude [mailto:cpaul@redhat.com]
> Sent: Sunday, November 22, 2015 9:45 PM
> To: Koenig, Christian; Daniel Stone
> Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> Jerome Glisse; Benjamin Tissoires
> Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> got an HPD interrupt
> 
> On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > On 21.11.2015 15:49, Daniel Stone wrote:
> > > Hi,
> > >
> > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > .com> wrote:
> > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > 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>
> > > >
> > > > 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.
> Unfortunately the delay needed to make hotplugging work on the system
> mentioned in the commit log can actually be over 700ms.
> > > >
> > > > Retrying on the other hand doesn't necessarily improve the
> > > > situation cause
> > > > the delay introduced by this might not be enough.
> Yeah, but I would think it would make sense to retry here so long as we
> back off after a certain time. This would also have the benefit of
> skipping this delay on systems that don't need it.
> > > >
> > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > and
> > > > actually starting to process anything.
> > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > those
> > > on marginal power) which send you HPD storms as well. But DP relies
> > > on
> > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > totally debounce all HPD won't work.
> > Well the discussion so far was about HPD on DVI only.
> >
> > I'm not so deep into DP, but why should it uses HPD pulses of less
> > than 2ms?
> This is part of the DP spec iirc. This being said though, the issue
> here with the HPD signal coming before the connector is ready only
> happens on DVI. I haven't ever run into this issue with any HDMI cables
> or DP cables, so I'm against imposing this on all connectors.
> 
> One of the solutions I've been thinking about with this: In
> radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> probe until at least a second has passed, after which we back off and
> assume the port is disconnected.

FWIW, there are registers to adjust how long the hpd needs to be asserted before the hpd connection and short pulse interrupts are triggered.  See DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write the default value, but it might be better to RMW the value in case there is a special golden value set by the vbios at init time.

Alex

> >
> > Regards,
> > Christian.
> >
> > >
> > > Cheers,
> > > Daniel
> >
> --
> Cheers,
> 	Lyude

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

[Deucher, Alexander]

> -----Original Message-----
> From: Lyude [mailto:cpaul@redhat.com]
> Sent: Sunday, November 22, 2015 9:45 PM
> To: Koenig, Christian; Daniel Stone
> Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> Jerome Glisse; Benjamin Tissoires
> Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> got an HPD interrupt
> 
> On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > On 21.11.2015 15:49, Daniel Stone wrote:
> > > Hi,
> > >
> > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > .com> wrote:
> > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > 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>
> > > >
> > > > 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.
> Unfortunately the delay needed to make hotplugging work on the system
> mentioned in the commit log can actually be over 700ms.
> > > >
> > > > Retrying on the other hand doesn't necessarily improve the
> > > > situation cause
> > > > the delay introduced by this might not be enough.
> Yeah, but I would think it would make sense to retry here so long as we
> back off after a certain time. This would also have the benefit of
> skipping this delay on systems that don't need it.
> > > >
> > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > and
> > > > actually starting to process anything.
> > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > those
> > > on marginal power) which send you HPD storms as well. But DP relies
> > > on
> > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > totally debounce all HPD won't work.
> > Well the discussion so far was about HPD on DVI only.
> >
> > I'm not so deep into DP, but why should it uses HPD pulses of less
> > than 2ms?
> This is part of the DP spec iirc. This being said though, the issue
> here with the HPD signal coming before the connector is ready only
> happens on DVI. I haven't ever run into this issue with any HDMI cables
> or DP cables, so I'm against imposing this on all connectors.
> 
> One of the solutions I've been thinking about with this: In
> radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> probe until at least a second has passed, after which we back off and
> assume the port is disconnected.

FWIW, there are registers to adjust how long the hpd needs to be asserted before the hpd connection and short pulse interrupts are triggered.  See DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write the default value, but it might be better to RMW the value in case there is a special golden value set by the vbios at init time.

Alex

> >
> > Regards,
> > Christian.
> >
> > >
> > > Cheers,
> > > Daniel
> >
> --
> Cheers,
> 	Lyude

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lyude]

On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: Lyude [mailto:cpaul@redhat.com]
> > Sent: Sunday, November 22, 2015 9:45 PM
> > To: Koenig, Christian; Daniel Stone
> > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> > Jerome Glisse; Benjamin Tissoires
> > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > got an HPD interrupt
> > 
> > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > Hi,
> > > > 
> > > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > > .com> wrote:
> > > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > > 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>
> > > > > 
> > > > > 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.
> > Unfortunately the delay needed to make hotplugging work on the system
> > mentioned in the commit log can actually be over 700ms.
> > > > > 
> > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > situation cause
> > > > > the delay introduced by this might not be enough.
> > Yeah, but I would think it would make sense to retry here so long as we
> > back off after a certain time. This would also have the benefit of
> > skipping this delay on systems that don't need it.
> > > > > 
> > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > and
> > > > > actually starting to process anything.
> > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > those
> > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > on
> > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > totally debounce all HPD won't work.
> > > Well the discussion so far was about HPD on DVI only.
> > > 
> > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > than 2ms?
> > This is part of the DP spec iirc. This being said though, the issue
> > here with the HPD signal coming before the connector is ready only
> > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > or DP cables, so I'm against imposing this on all connectors.
> > 
> > One of the solutions I've been thinking about with this: In
> > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > probe until at least a second has passed, after which we back off and
> > assume the port is disconnected.
> 
> FWIW, there are registers to adjust how long the hpd needs to be asserted
> before the hpd connection and short pulse interrupts are triggered.  See
> DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write
> the default value, but it might be better to RMW the value in case there is a
> special golden value set by the vbios at init time.
> 
This sounds like a much better idea. I'm going to try to come up with a patch
that adjusts these by default for DVI ports.
> Alex
> 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > 
> > > > Cheers,
> > > > Daniel
> > > 
> > --
> > Cheers,
> > 	Lyude
> 
-- 
Cheers,
	Lyude

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

[Lyude]

On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: Lyude [mailto:cpaul@redhat.com]
> > Sent: Sunday, November 22, 2015 9:45 PM
> > To: Koenig, Christian; Daniel Stone
> > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> > Jerome Glisse; Benjamin Tissoires
> > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > got an HPD interrupt
> > 
> > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > Hi,
> > > > 
> > > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > > .com> wrote:
> > > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > > 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>
> > > > > 
> > > > > 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.
> > Unfortunately the delay needed to make hotplugging work on the system
> > mentioned in the commit log can actually be over 700ms.
> > > > > 
> > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > situation cause
> > > > > the delay introduced by this might not be enough.
> > Yeah, but I would think it would make sense to retry here so long as we
> > back off after a certain time. This would also have the benefit of
> > skipping this delay on systems that don't need it.
> > > > > 
> > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > and
> > > > > actually starting to process anything.
> > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > those
> > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > on
> > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > totally debounce all HPD won't work.
> > > Well the discussion so far was about HPD on DVI only.
> > > 
> > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > than 2ms?
> > This is part of the DP spec iirc. This being said though, the issue
> > here with the HPD signal coming before the connector is ready only
> > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > or DP cables, so I'm against imposing this on all connectors.
> > 
> > One of the solutions I've been thinking about with this: In
> > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > probe until at least a second has passed, after which we back off and
> > assume the port is disconnected.
> 
> FWIW, there are registers to adjust how long the hpd needs to be asserted
> before the hpd connection and short pulse interrupts are triggered.  See
> DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write
> the default value, but it might be better to RMW the value in case there is a
> special golden value set by the vbios at init time.
> 
This sounds like a much better idea. I'm going to try to come up with a patch
that adjusts these by default for DVI ports.
> Alex
> 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > 
> > > > Cheers,
> > > > Daniel
> > > 
> > --
> > Cheers,
> > 	Lyude
> 
-- 
Cheers,
	Lyude

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lyude]

On Mon, 2015-11-23 at 10:43 -0500, Lyude wrote:
> On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote:
> > > -----Original Message-----
> > > From: Lyude [mailto:cpaul@redhat.com]
> > > Sent: Sunday, November 22, 2015 9:45 PM
> > > To: Koenig, Christian; Daniel Stone
> > > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing
> > > List;
> > > Jerome Glisse; Benjamin Tissoires
> > > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > > got an HPD interrupt
> > > 
> > > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > > Hi,
> > > > > 
> > > > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > > > .com> wrote:
> > > > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > > > 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>
> > > > > > 
> > > > > > 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.
> > > Unfortunately the delay needed to make hotplugging work on the system
> > > mentioned in the commit log can actually be over 700ms.
> > > > > > 
> > > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > > situation cause
> > > > > > the delay introduced by this might not be enough.
> > > Yeah, but I would think it would make sense to retry here so long as we
> > > back off after a certain time. This would also have the benefit of
> > > skipping this delay on systems that don't need it.
> > > > > > 
> > > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > > and
> > > > > > actually starting to process anything.
> > > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > > those
> > > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > > on
> > > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > > totally debounce all HPD won't work.
> > > > Well the discussion so far was about HPD on DVI only.
> > > > 
> > > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > > than 2ms?
> > > This is part of the DP spec iirc. This being said though, the issue
> > > here with the HPD signal coming before the connector is ready only
> > > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > > or DP cables, so I'm against imposing this on all connectors.
> > > 
> > > One of the solutions I've been thinking about with this: In
> > > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > > probe until at least a second has passed, after which we back off and
> > > assume the port is disconnected.
> > 
> > FWIW, there are registers to adjust how long the hpd needs to be asserted
> > before the hpd connection and short pulse interrupts are triggered.  See
> > DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write
> > the default value, but it might be better to RMW the value in case there is
> > a
> > special golden value set by the vbios at init time.
> > 
> This sounds like a much better idea. I'm going to try to come up with a patch
> that adjusts these by default for DVI ports.

Ignore this; we can't get a long enough delay using those registers to fix the
issue so we'll still have to handle this in software.
> > Alex
> > 
> > > > 
> > > > Regards,
> > > > Christian.
> > > > 
> > > > > 
> > > > > Cheers,
> > > > > Daniel
> > > > 
> > > --
> > > Cheers,
> > > 	Lyude
> > 
-- 
Cheers,
	Lyude

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

[Lyude]

On Mon, 2015-11-23 at 10:43 -0500, Lyude wrote:
> On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote:
> > > -----Original Message-----
> > > From: Lyude [mailto:cpaul@redhat.com]
> > > Sent: Sunday, November 22, 2015 9:45 PM
> > > To: Koenig, Christian; Daniel Stone
> > > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing
> > > List;
> > > Jerome Glisse; Benjamin Tissoires
> > > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > > got an HPD interrupt
> > > 
> > > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > > Hi,
> > > > > 
> > > > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > > > .com> wrote:
> > > > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > > > 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>
> > > > > > 
> > > > > > 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.
> > > Unfortunately the delay needed to make hotplugging work on the system
> > > mentioned in the commit log can actually be over 700ms.
> > > > > > 
> > > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > > situation cause
> > > > > > the delay introduced by this might not be enough.
> > > Yeah, but I would think it would make sense to retry here so long as we
> > > back off after a certain time. This would also have the benefit of
> > > skipping this delay on systems that don't need it.
> > > > > > 
> > > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > > and
> > > > > > actually starting to process anything.
> > > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > > those
> > > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > > on
> > > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > > totally debounce all HPD won't work.
> > > > Well the discussion so far was about HPD on DVI only.
> > > > 
> > > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > > than 2ms?
> > > This is part of the DP spec iirc. This being said though, the issue
> > > here with the HPD signal coming before the connector is ready only
> > > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > > or DP cables, so I'm against imposing this on all connectors.
> > > 
> > > One of the solutions I've been thinking about with this: In
> > > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > > probe until at least a second has passed, after which we back off and
> > > assume the port is disconnected.
> > 
> > FWIW, there are registers to adjust how long the hpd needs to be asserted
> > before the hpd connection and short pulse interrupts are triggered.  See
> > DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write
> > the default value, but it might be better to RMW the value in case there is
> > a
> > special golden value set by the vbios at init time.
> > 
> This sounds like a much better idea. I'm going to try to come up with a patch
> that adjusts these by default for DVI ports.

Ignore this; we can't get a long enough delay using those registers to fix the
issue so we'll still have to handle this in software.
> > Alex
> > 
> > > > 
> > > > Regards,
> > > > Christian.
> > > > 
> > > > > 
> > > > > Cheers,
> > > > > Daniel
> > > > 
> > > --
> > > Cheers,
> > > 	Lyude
> > 
-- 
Cheers,
	Lyude

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Lyude]

Ping
Do we have any consensus on the best way of handling this situation in the
driver?

On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: Lyude [mailto:cpaul@redhat.com]
> > Sent: Sunday, November 22, 2015 9:45 PM
> > To: Koenig, Christian; Daniel Stone
> > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> > Jerome Glisse; Benjamin Tissoires
> > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > got an HPD interrupt
> > 
> > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > Hi,
> > > > 
> > > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > > .com> wrote:
> > > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > > 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>
> > > > > 
> > > > > 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.
> > Unfortunately the delay needed to make hotplugging work on the system
> > mentioned in the commit log can actually be over 700ms.
> > > > > 
> > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > situation cause
> > > > > the delay introduced by this might not be enough.
> > Yeah, but I would think it would make sense to retry here so long as we
> > back off after a certain time. This would also have the benefit of
> > skipping this delay on systems that don't need it.
> > > > > 
> > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > and
> > > > > actually starting to process anything.
> > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > those
> > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > on
> > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > totally debounce all HPD won't work.
> > > Well the discussion so far was about HPD on DVI only.
> > > 
> > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > than 2ms?
> > This is part of the DP spec iirc. This being said though, the issue
> > here with the HPD signal coming before the connector is ready only
> > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > or DP cables, so I'm against imposing this on all connectors.
> > 
> > One of the solutions I've been thinking about with this: In
> > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > probe until at least a second has passed, after which we back off and
> > assume the port is disconnected.
> 
> FWIW, there are registers to adjust how long the hpd needs to be asserted
> before the hpd connection and short pulse interrupts are triggered.  See
> DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write
> the default value, but it might be better to RMW the value in case there is a
> special golden value set by the vbios at init time.
> 
> Alex
> 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > 
> > > > Cheers,
> > > > Daniel
> > > 
> > --
> > Cheers,
> > 	Lyude
> 
-- 
Cheers,
	Lyude

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

[Lyude]

Ping
Do we have any consensus on the best way of handling this situation in the
driver?

On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: Lyude [mailto:cpaul@redhat.com]
> > Sent: Sunday, November 22, 2015 9:45 PM
> > To: Koenig, Christian; Daniel Stone
> > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> > Jerome Glisse; Benjamin Tissoires
> > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > got an HPD interrupt
> > 
> > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > Hi,
> > > > 
> > > > On 21 November 2015 at 14:22, Christian König <christian.koenig@amd
> > > > .com> wrote:
> > > > > On 20.11.2015 16:52, cpaul@redhat.com wrote:
> > > > > > 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>
> > > > > 
> > > > > 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.
> > Unfortunately the delay needed to make hotplugging work on the system
> > mentioned in the commit log can actually be over 700ms.
> > > > > 
> > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > situation cause
> > > > > the delay introduced by this might not be enough.
> > Yeah, but I would think it would make sense to retry here so long as we
> > back off after a certain time. This would also have the benefit of
> > skipping this delay on systems that don't need it.
> > > > > 
> > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > and
> > > > > actually starting to process anything.
> > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > those
> > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > on
> > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > totally debounce all HPD won't work.
> > > Well the discussion so far was about HPD on DVI only.
> > > 
> > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > than 2ms?
> > This is part of the DP spec iirc. This being said though, the issue
> > here with the HPD signal coming before the connector is ready only
> > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > or DP cables, so I'm against imposing this on all connectors.
> > 
> > One of the solutions I've been thinking about with this: In
> > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > probe until at least a second has passed, after which we back off and
> > assume the port is disconnected.
> 
> FWIW, there are registers to adjust how long the hpd needs to be asserted
> before the hpd connection and short pulse interrupts are triggered.  See
> DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write
> the default value, but it might be better to RMW the value in case there is a
> special golden value set by the vbios at init time.
> 
> Alex
> 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > 
> > > > Cheers,
> > > > Daniel
> > > 
> > --
> > Cheers,
> > 	Lyude
> 
-- 
Cheers,
	Lyude

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [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@redhat.com írta:
> 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)

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

Re: [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@redhat.com írta:
> 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)

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[cpaul]

From: Lyude <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: Lyude <cpaul@redhat.com>
---
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(&rdev->dp_work);
 	if (queue_hotplug)
-		schedule_work(&rdev->hotplug_work);
+		schedule_delayed_work(&rdev->hotplug_work, 0);
 	if (queue_reset) {
 		rdev->needs_reset = true;
 		wake_up_all(&rdev->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(&rdev->dp_work);
 	if (queue_hotplug)
-		schedule_work(&rdev->hotplug_work);
+		schedule_delayed_work(&rdev->hotplug_work, 0);
 	if (queue_hdmi)
 		schedule_work(&rdev->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(&rdev->hotplug_work);
+		schedule_delayed_work(&rdev->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(&rdev->hotplug_work);
+		schedule_delayed_work(&rdev->hotplug_work, 0);
 	if (queue_hdmi)
 		schedule_work(&rdev->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 && radeon_check_hpd_status_unchanged(connector)) {
 		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, try again later */
+		if (!dret && !force &&
+		    connector->status != connector_status_connected) {
+			DRM_DEBUG_KMS("hpd detected without ddc, retrying in 1 second\n");
+			radeon_connector->detected_hpd_without_ddc = true;
+			schedule_delayed_work(&rdev->hotplug_work,
+					      msecs_to_jiffies(1000));
+			goto exit;
+		}
+	}
 	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..979f3bf 100644
--- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
@@ -74,7 +74,7 @@ irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg)
 static void radeon_hotplug_work_func(struct work_struct *work)
 {
 	struct radeon_device *rdev = container_of(work, struct radeon_device,
-						  hotplug_work);
+						  hotplug_work.work);
 	struct drm_device *dev = rdev->ddev;
 	struct drm_mode_config *mode_config = &dev->mode_config;
 	struct drm_connector *connector;
@@ -302,7 +302,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
 		}
 	}
 
-	INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
+	INIT_DELAYED_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
 	INIT_WORK(&rdev->dp_work, radeon_dp_work_func);
 	INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi);
 
@@ -310,7 +310,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
 	r = drm_irq_install(rdev->ddev, rdev->ddev->pdev->irq);
 	if (r) {
 		rdev->irq.installed = false;
-		flush_work(&rdev->hotplug_work);
+		flush_delayed_work(&rdev->hotplug_work);
 		return r;
 	}
 
@@ -333,7 +333,7 @@ void radeon_irq_kms_fini(struct radeon_device *rdev)
 		rdev->irq.installed = false;
 		if (rdev->msi_enabled)
 			pci_disable_msi(rdev->pdev);
-		flush_work(&rdev->hotplug_work);
+		flush_delayed_work(&rdev->hotplug_work);
 	}
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
index 457b026..0aeb3ee 100644
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -553,6 +553,7 @@ struct radeon_connector {
 	void *con_priv;
 	bool dac_load_detect;
 	bool detected_by_load; /* if the connection status was determined by load */
+	bool detected_hpd_without_ddc; /* if an HPD signal was detected on DVI, but ddc probing failed */
 	uint16_t connector_object_id;
 	struct radeon_hpd hpd;
 	struct radeon_router router;
diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
index 97a9048..6244f4e 100644
--- a/drivers/gpu/drm/radeon/rs600.c
+++ b/drivers/gpu/drm/radeon/rs600.c
@@ -813,7 +813,7 @@ int rs600_irq_process(struct radeon_device *rdev)
 		status = rs600_irq_ack(rdev);
 	}
 	if (queue_hotplug)
-		schedule_work(&rdev->hotplug_work);
+		schedule_delayed_work(&rdev->hotplug_work, 0);
 	if (queue_hdmi)
 		schedule_work(&rdev->audio_work);
 	if (rdev->msi_enabled) {
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 07037e3..fb1a7ec 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -6848,7 +6848,7 @@ restart_ih:
 	if (queue_dp)
 		schedule_work(&rdev->dp_work);
 	if (queue_hotplug)
-		schedule_work(&rdev->hotplug_work);
+		schedule_delayed_work(&rdev->hotplug_work, 0);
 	if (queue_thermal && rdev->pm.dpm_enabled)
 		schedule_work(&rdev->pm.dpm.thermal.work);
 	rdev->ih.rptr = rptr;
-- 
2.5.0

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

[Christian König]

On 04.12.2015 00:26, cpaul@redhat.com wrote:
> From: Lyude <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: Lyude <cpaul@redhat.com>

I find a second a bit long, but if it works so what?

Looks sane enough to me, patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

> ---
> 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(&rdev->dp_work);
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_reset) {
>   		rdev->needs_reset = true;
>   		wake_up_all(&rdev->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(&rdev->dp_work);
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_hdmi)
>   		schedule_work(&rdev->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(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->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(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_hdmi)
>   		schedule_work(&rdev->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 && radeon_check_hpd_status_unchanged(connector)) {
>   		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, try again later */
> +		if (!dret && !force &&
> +		    connector->status != connector_status_connected) {
> +			DRM_DEBUG_KMS("hpd detected without ddc, retrying in 1 second\n");
> +			radeon_connector->detected_hpd_without_ddc = true;
> +			schedule_delayed_work(&rdev->hotplug_work,
> +					      msecs_to_jiffies(1000));
> +			goto exit;
> +		}
> +	}
>   	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..979f3bf 100644
> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> @@ -74,7 +74,7 @@ irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg)
>   static void radeon_hotplug_work_func(struct work_struct *work)
>   {
>   	struct radeon_device *rdev = container_of(work, struct radeon_device,
> -						  hotplug_work);
> +						  hotplug_work.work);
>   	struct drm_device *dev = rdev->ddev;
>   	struct drm_mode_config *mode_config = &dev->mode_config;
>   	struct drm_connector *connector;
> @@ -302,7 +302,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>   		}
>   	}
>   
> -	INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
> +	INIT_DELAYED_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
>   	INIT_WORK(&rdev->dp_work, radeon_dp_work_func);
>   	INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi);
>   
> @@ -310,7 +310,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>   	r = drm_irq_install(rdev->ddev, rdev->ddev->pdev->irq);
>   	if (r) {
>   		rdev->irq.installed = false;
> -		flush_work(&rdev->hotplug_work);
> +		flush_delayed_work(&rdev->hotplug_work);
>   		return r;
>   	}
>   
> @@ -333,7 +333,7 @@ void radeon_irq_kms_fini(struct radeon_device *rdev)
>   		rdev->irq.installed = false;
>   		if (rdev->msi_enabled)
>   			pci_disable_msi(rdev->pdev);
> -		flush_work(&rdev->hotplug_work);
> +		flush_delayed_work(&rdev->hotplug_work);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
> index 457b026..0aeb3ee 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -553,6 +553,7 @@ struct radeon_connector {
>   	void *con_priv;
>   	bool dac_load_detect;
>   	bool detected_by_load; /* if the connection status was determined by load */
> +	bool detected_hpd_without_ddc; /* if an HPD signal was detected on DVI, but ddc probing failed */
>   	uint16_t connector_object_id;
>   	struct radeon_hpd hpd;
>   	struct radeon_router router;
> diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
> index 97a9048..6244f4e 100644
> --- a/drivers/gpu/drm/radeon/rs600.c
> +++ b/drivers/gpu/drm/radeon/rs600.c
> @@ -813,7 +813,7 @@ int rs600_irq_process(struct radeon_device *rdev)
>   		status = rs600_irq_ack(rdev);
>   	}
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_hdmi)
>   		schedule_work(&rdev->audio_work);
>   	if (rdev->msi_enabled) {
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index 07037e3..fb1a7ec 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -6848,7 +6848,7 @@ restart_ih:
>   	if (queue_dp)
>   		schedule_work(&rdev->dp_work);
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_thermal && rdev->pm.dpm_enabled)
>   		schedule_work(&rdev->pm.dpm.thermal.work);
>   	rdev->ih.rptr = rptr;

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

[Christian König]

On 04.12.2015 00:26, cpaul@redhat.com wrote:
> From: Lyude <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: Lyude <cpaul@redhat.com>

I find a second a bit long, but if it works so what?

Looks sane enough to me, patch is Reviewed-by: Christian König 
<christian.koenig@amd.com>

> ---
> 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(&rdev->dp_work);
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_reset) {
>   		rdev->needs_reset = true;
>   		wake_up_all(&rdev->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(&rdev->dp_work);
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_hdmi)
>   		schedule_work(&rdev->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(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->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(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_hdmi)
>   		schedule_work(&rdev->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 && radeon_check_hpd_status_unchanged(connector)) {
>   		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, try again later */
> +		if (!dret && !force &&
> +		    connector->status != connector_status_connected) {
> +			DRM_DEBUG_KMS("hpd detected without ddc, retrying in 1 second\n");
> +			radeon_connector->detected_hpd_without_ddc = true;
> +			schedule_delayed_work(&rdev->hotplug_work,
> +					      msecs_to_jiffies(1000));
> +			goto exit;
> +		}
> +	}
>   	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..979f3bf 100644
> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
> @@ -74,7 +74,7 @@ irqreturn_t radeon_driver_irq_handler_kms(int irq, void *arg)
>   static void radeon_hotplug_work_func(struct work_struct *work)
>   {
>   	struct radeon_device *rdev = container_of(work, struct radeon_device,
> -						  hotplug_work);
> +						  hotplug_work.work);
>   	struct drm_device *dev = rdev->ddev;
>   	struct drm_mode_config *mode_config = &dev->mode_config;
>   	struct drm_connector *connector;
> @@ -302,7 +302,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>   		}
>   	}
>   
> -	INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
> +	INIT_DELAYED_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
>   	INIT_WORK(&rdev->dp_work, radeon_dp_work_func);
>   	INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi);
>   
> @@ -310,7 +310,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>   	r = drm_irq_install(rdev->ddev, rdev->ddev->pdev->irq);
>   	if (r) {
>   		rdev->irq.installed = false;
> -		flush_work(&rdev->hotplug_work);
> +		flush_delayed_work(&rdev->hotplug_work);
>   		return r;
>   	}
>   
> @@ -333,7 +333,7 @@ void radeon_irq_kms_fini(struct radeon_device *rdev)
>   		rdev->irq.installed = false;
>   		if (rdev->msi_enabled)
>   			pci_disable_msi(rdev->pdev);
> -		flush_work(&rdev->hotplug_work);
> +		flush_delayed_work(&rdev->hotplug_work);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h
> index 457b026..0aeb3ee 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -553,6 +553,7 @@ struct radeon_connector {
>   	void *con_priv;
>   	bool dac_load_detect;
>   	bool detected_by_load; /* if the connection status was determined by load */
> +	bool detected_hpd_without_ddc; /* if an HPD signal was detected on DVI, but ddc probing failed */
>   	uint16_t connector_object_id;
>   	struct radeon_hpd hpd;
>   	struct radeon_router router;
> diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
> index 97a9048..6244f4e 100644
> --- a/drivers/gpu/drm/radeon/rs600.c
> +++ b/drivers/gpu/drm/radeon/rs600.c
> @@ -813,7 +813,7 @@ int rs600_irq_process(struct radeon_device *rdev)
>   		status = rs600_irq_ack(rdev);
>   	}
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_hdmi)
>   		schedule_work(&rdev->audio_work);
>   	if (rdev->msi_enabled) {
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index 07037e3..fb1a7ec 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -6848,7 +6848,7 @@ restart_ih:
>   	if (queue_dp)
>   		schedule_work(&rdev->dp_work);
>   	if (queue_hotplug)
> -		schedule_work(&rdev->hotplug_work);
> +		schedule_delayed_work(&rdev->hotplug_work, 0);
>   	if (queue_thermal && rdev->pm.dpm_enabled)
>   		schedule_work(&rdev->pm.dpm.thermal.work);
>   	rdev->ih.rptr = rptr;

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

[Boszormenyi Zoltan]

2015-12-04 09:53 keltezéssel, Christian König írta:
> On 04.12.2015 00:26, cpaul@redhat.com wrote:
>> From: Lyude <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: Lyude <cpaul@redhat.com>
>
> I find a second a bit long, but if it works so what?
>
> Looks sane enough to me, patch is Reviewed-by: Christian König <christian.koenig@amd.com>

Does this patch help in case of the Radeon chip only has HDMI and DP outputs
exposed (Zotac ZBOXNANO-AQ01) but used with DVI or VGA monitors with
converter cables? We have some problems with such scenarios that sounds
eerily similar to this description.

Inquiry-by: Zoltán Böszörményi <zboszor@pr.hu>
;-)

Thanks in advance.

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

[Boszormenyi Zoltan]

2015-12-04 09:53 keltezéssel, Christian König írta:
> On 04.12.2015 00:26, cpaul@redhat.com wrote:
>> From: Lyude <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: Lyude <cpaul@redhat.com>
>
> I find a second a bit long, but if it works so what?
>
> Looks sane enough to me, patch is Reviewed-by: Christian König <christian.koenig@amd.com>

Does this patch help in case of the Radeon chip only has HDMI and DP outputs
exposed (Zotac ZBOXNANO-AQ01) but used with DVI or VGA monitors with
converter cables? We have some problems with such scenarios that sounds
eerily similar to this description.

Inquiry-by: Zoltán Böszörményi <zboszor@pr.hu>
;-)

Thanks in advance.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Christian König]

On 04.12.2015 10:43, Boszormenyi Zoltan wrote:
> 2015-12-04 09:53 keltezéssel, Christian König írta:
>> On 04.12.2015 00:26, cpaul@redhat.com wrote:
>>> From: Lyude <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: Lyude <cpaul@redhat.com>
>> I find a second a bit long, but if it works so what?
>>
>> Looks sane enough to me, patch is Reviewed-by: Christian König <christian.koenig@amd.com>
> Does this patch help in case of the Radeon chip only has HDMI and DP outputs
> exposed (Zotac ZBOXNANO-AQ01) but used with DVI or VGA monitors with
> converter cables? We have some problems with such scenarios that sounds
> eerily similar to this description.

No, at least active converter cables are a completely different case.

Regards,
Christian.

>
> Inquiry-by: Zoltán Böszörményi <zboszor@pr.hu>
> ;-)
>
> Thanks in advance.
>

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

[Christian König]

On 04.12.2015 10:43, Boszormenyi Zoltan wrote:
> 2015-12-04 09:53 keltezéssel, Christian König írta:
>> On 04.12.2015 00:26, cpaul@redhat.com wrote:
>>> From: Lyude <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: Lyude <cpaul@redhat.com>
>> I find a second a bit long, but if it works so what?
>>
>> Looks sane enough to me, patch is Reviewed-by: Christian König <christian.koenig@amd.com>
> Does this patch help in case of the Radeon chip only has HDMI and DP outputs
> exposed (Zotac ZBOXNANO-AQ01) but used with DVI or VGA monitors with
> converter cables? We have some problems with such scenarios that sounds
> eerily similar to this description.

No, at least active converter cables are a completely different case.

Regards,
Christian.

>
> Inquiry-by: Zoltán Böszörményi <zboszor@pr.hu>
> ;-)
>
> Thanks in advance.
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Alex Deucher]

On Fri, Dec 4, 2015 at 3:53 AM, Christian König
<christian.koenig@amd.com> wrote:
> On 04.12.2015 00:26, cpaul@redhat.com wrote:
>>
>> From: Lyude <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: Lyude <cpaul@redhat.com>
>
>
> I find a second a bit long, but if it works so what?
>
> Looks sane enough to me, patch is Reviewed-by: Christian König
> <christian.koenig@amd.com>

Applied.  thanks!

Alex

>
>
>> ---
>> 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(&rdev->dp_work);
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_reset) {
>>                 rdev->needs_reset = true;
>>                 wake_up_all(&rdev->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(&rdev->dp_work);
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_hdmi)
>>                 schedule_work(&rdev->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(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->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(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_hdmi)
>>                 schedule_work(&rdev->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 && radeon_check_hpd_status_unchanged(connector)) {
>>                 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, try again later */
>> +               if (!dret && !force &&
>> +                   connector->status != connector_status_connected) {
>> +                       DRM_DEBUG_KMS("hpd detected without ddc, retrying
>> in 1 second\n");
>> +                       radeon_connector->detected_hpd_without_ddc = true;
>> +                       schedule_delayed_work(&rdev->hotplug_work,
>> +                                             msecs_to_jiffies(1000));
>> +                       goto exit;
>> +               }
>> +       }
>>         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..979f3bf 100644
>> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> @@ -74,7 +74,7 @@ irqreturn_t radeon_driver_irq_handler_kms(int irq, void
>> *arg)
>>   static void radeon_hotplug_work_func(struct work_struct *work)
>>   {
>>         struct radeon_device *rdev = container_of(work, struct
>> radeon_device,
>> -                                                 hotplug_work);
>> +                                                 hotplug_work.work);
>>         struct drm_device *dev = rdev->ddev;
>>         struct drm_mode_config *mode_config = &dev->mode_config;
>>         struct drm_connector *connector;
>> @@ -302,7 +302,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>>                 }
>>         }
>>   -     INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
>> +       INIT_DELAYED_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
>>         INIT_WORK(&rdev->dp_work, radeon_dp_work_func);
>>         INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi);
>>   @@ -310,7 +310,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>>         r = drm_irq_install(rdev->ddev, rdev->ddev->pdev->irq);
>>         if (r) {
>>                 rdev->irq.installed = false;
>> -               flush_work(&rdev->hotplug_work);
>> +               flush_delayed_work(&rdev->hotplug_work);
>>                 return r;
>>         }
>>   @@ -333,7 +333,7 @@ void radeon_irq_kms_fini(struct radeon_device *rdev)
>>                 rdev->irq.installed = false;
>>                 if (rdev->msi_enabled)
>>                         pci_disable_msi(rdev->pdev);
>> -               flush_work(&rdev->hotplug_work);
>> +               flush_delayed_work(&rdev->hotplug_work);
>>         }
>>   }
>>   diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
>> b/drivers/gpu/drm/radeon/radeon_mode.h
>> index 457b026..0aeb3ee 100644
>> --- a/drivers/gpu/drm/radeon/radeon_mode.h
>> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
>> @@ -553,6 +553,7 @@ struct radeon_connector {
>>         void *con_priv;
>>         bool dac_load_detect;
>>         bool detected_by_load; /* if the connection status was determined
>> by load */
>> +       bool detected_hpd_without_ddc; /* if an HPD signal was detected on
>> DVI, but ddc probing failed */
>>         uint16_t connector_object_id;
>>         struct radeon_hpd hpd;
>>         struct radeon_router router;
>> diff --git a/drivers/gpu/drm/radeon/rs600.c
>> b/drivers/gpu/drm/radeon/rs600.c
>> index 97a9048..6244f4e 100644
>> --- a/drivers/gpu/drm/radeon/rs600.c
>> +++ b/drivers/gpu/drm/radeon/rs600.c
>> @@ -813,7 +813,7 @@ int rs600_irq_process(struct radeon_device *rdev)
>>                 status = rs600_irq_ack(rdev);
>>         }
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_hdmi)
>>                 schedule_work(&rdev->audio_work);
>>         if (rdev->msi_enabled) {
>> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
>> index 07037e3..fb1a7ec 100644
>> --- a/drivers/gpu/drm/radeon/si.c
>> +++ b/drivers/gpu/drm/radeon/si.c
>> @@ -6848,7 +6848,7 @@ restart_ih:
>>         if (queue_dp)
>>                 schedule_work(&rdev->dp_work);
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_thermal && rdev->pm.dpm_enabled)
>>                 schedule_work(&rdev->pm.dpm.thermal.work);
>>         rdev->ih.rptr = rptr;
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Alex Deucher]

On Fri, Dec 4, 2015 at 3:53 AM, Christian König
<christian.koenig@amd.com> wrote:
> On 04.12.2015 00:26, cpaul@redhat.com wrote:
>>
>> From: Lyude <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: Lyude <cpaul@redhat.com>
>
>
> I find a second a bit long, but if it works so what?
>
> Looks sane enough to me, patch is Reviewed-by: Christian König
> <christian.koenig@amd.com>

Applied.  thanks!

Alex

>
>
>> ---
>> 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(&rdev->dp_work);
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_reset) {
>>                 rdev->needs_reset = true;
>>                 wake_up_all(&rdev->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(&rdev->dp_work);
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_hdmi)
>>                 schedule_work(&rdev->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(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->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(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_hdmi)
>>                 schedule_work(&rdev->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 && radeon_check_hpd_status_unchanged(connector)) {
>>                 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, try again later */
>> +               if (!dret && !force &&
>> +                   connector->status != connector_status_connected) {
>> +                       DRM_DEBUG_KMS("hpd detected without ddc, retrying
>> in 1 second\n");
>> +                       radeon_connector->detected_hpd_without_ddc = true;
>> +                       schedule_delayed_work(&rdev->hotplug_work,
>> +                                             msecs_to_jiffies(1000));
>> +                       goto exit;
>> +               }
>> +       }
>>         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..979f3bf 100644
>> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> @@ -74,7 +74,7 @@ irqreturn_t radeon_driver_irq_handler_kms(int irq, void
>> *arg)
>>   static void radeon_hotplug_work_func(struct work_struct *work)
>>   {
>>         struct radeon_device *rdev = container_of(work, struct
>> radeon_device,
>> -                                                 hotplug_work);
>> +                                                 hotplug_work.work);
>>         struct drm_device *dev = rdev->ddev;
>>         struct drm_mode_config *mode_config = &dev->mode_config;
>>         struct drm_connector *connector;
>> @@ -302,7 +302,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>>                 }
>>         }
>>   -     INIT_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
>> +       INIT_DELAYED_WORK(&rdev->hotplug_work, radeon_hotplug_work_func);
>>         INIT_WORK(&rdev->dp_work, radeon_dp_work_func);
>>         INIT_WORK(&rdev->audio_work, r600_audio_update_hdmi);
>>   @@ -310,7 +310,7 @@ int radeon_irq_kms_init(struct radeon_device *rdev)
>>         r = drm_irq_install(rdev->ddev, rdev->ddev->pdev->irq);
>>         if (r) {
>>                 rdev->irq.installed = false;
>> -               flush_work(&rdev->hotplug_work);
>> +               flush_delayed_work(&rdev->hotplug_work);
>>                 return r;
>>         }
>>   @@ -333,7 +333,7 @@ void radeon_irq_kms_fini(struct radeon_device *rdev)
>>                 rdev->irq.installed = false;
>>                 if (rdev->msi_enabled)
>>                         pci_disable_msi(rdev->pdev);
>> -               flush_work(&rdev->hotplug_work);
>> +               flush_delayed_work(&rdev->hotplug_work);
>>         }
>>   }
>>   diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
>> b/drivers/gpu/drm/radeon/radeon_mode.h
>> index 457b026..0aeb3ee 100644
>> --- a/drivers/gpu/drm/radeon/radeon_mode.h
>> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
>> @@ -553,6 +553,7 @@ struct radeon_connector {
>>         void *con_priv;
>>         bool dac_load_detect;
>>         bool detected_by_load; /* if the connection status was determined
>> by load */
>> +       bool detected_hpd_without_ddc; /* if an HPD signal was detected on
>> DVI, but ddc probing failed */
>>         uint16_t connector_object_id;
>>         struct radeon_hpd hpd;
>>         struct radeon_router router;
>> diff --git a/drivers/gpu/drm/radeon/rs600.c
>> b/drivers/gpu/drm/radeon/rs600.c
>> index 97a9048..6244f4e 100644
>> --- a/drivers/gpu/drm/radeon/rs600.c
>> +++ b/drivers/gpu/drm/radeon/rs600.c
>> @@ -813,7 +813,7 @@ int rs600_irq_process(struct radeon_device *rdev)
>>                 status = rs600_irq_ack(rdev);
>>         }
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_hdmi)
>>                 schedule_work(&rdev->audio_work);
>>         if (rdev->msi_enabled) {
>> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
>> index 07037e3..fb1a7ec 100644
>> --- a/drivers/gpu/drm/radeon/si.c
>> +++ b/drivers/gpu/drm/radeon/si.c
>> @@ -6848,7 +6848,7 @@ restart_ih:
>>         if (queue_dp)
>>                 schedule_work(&rdev->dp_work);
>>         if (queue_hotplug)
>> -               schedule_work(&rdev->hotplug_work);
>> +               schedule_delayed_work(&rdev->hotplug_work, 0);
>>         if (queue_thermal && rdev->pm.dpm_enabled)
>>                 schedule_work(&rdev->pm.dpm.thermal.work);
>>         rdev->ih.rptr = rptr;
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel