Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1283451 > unrolled thread

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

Started bycpaul@redhat.com
First post2015-12-04 00:30 +0100
Last post2015-12-04 19:20 +0100
Articles 4 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt cpaul@redhat.com - 2015-12-04 00:30 +0100
    Re: [PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we  got an HPD interrupt Christian König <christian.koenig@amd.com> - 2015-12-04 10:00 +0100
      Re: [PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we  got an HPD interrupt Boszormenyi Zoltan <zboszor@pr.hu> - 2015-12-04 10:50 +0100
      Re: [PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we  got an HPD interrupt Alex Deucher <alexdeucher@gmail.com> - 2015-12-04 19:20 +0100

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

Fromcpaul@redhat.com
Date2015-12-04 00:30 +0100
Subject[PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt
Message-ID<qBLu2-3ct-15@gated-at.bofh.it>
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

FromChristian König <christian.koenig@amd.com>
Date2015-12-04 10:00 +0100
SubjectRe: [PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt
Message-ID<qBUnF-nU-9@gated-at.bofh.it>
In reply to#1283451
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;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromBoszormenyi Zoltan <zboszor@pr.hu>
Date2015-12-04 10:50 +0100
SubjectRe: [PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt
Message-ID<qBVa2-W5-9@gated-at.bofh.it>
In reply to#1283655
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

FromAlex Deucher <alexdeucher@gmail.com>
Date2015-12-04 19:20 +0100
SubjectRe: [PATCH v2] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt
Message-ID<qC37z-6ec-15@gated-at.bofh.it>
In reply to#1283655
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web