diff mbox

[4/8] drm/i915: Add Hotplug IRQ Storm detection.

Message ID 1357830166-18049-5-git-send-email-eich@suse.de (mailing list archive)
State New, archived
Headers show

Commit Message

Egbert Eich Jan. 10, 2013, 3:02 p.m. UTC 
Add a hotplug IRQ storm detection (triggered when a hotplug interrupt
fires more than 5 times / sec).
Mask out this specific interrupt and revert to polling on the associated
output.
Rationale:
Despite of the many attempts to fix the problem with noisy hotplug
interrupt lines we are still seeing systems which have issues:
Once cause of noise seems to be bad routing of the hotplug line
on the board: cross talk from other signals seems to cause erronous
hotplug interrupts. This has been documented as an erratum for the
the i945GM chipset and thus hotplug support was disabled for this
chipset model but others seem to have this problem, too.

We have seen this issue on a G35 motherboard for example:
Even different motherboards of the same model seem to behave
differently: while some only see only around 10-100 interrupts/s
others seem to see 5k or more.
We've also observed a dependency on the selected video mode.

Also on certain laptops interrupt noise seems to occur duing
battery charging when the battery is at a certain charge levels.

Thus we add a simple algorithm here that detects an 'interrupt storm'
condition.

Signed-off-by: Egbert Eich <eich@suse.de>
---
 drivers/gpu/drm/i915/i915_irq.c  |   74 ++++++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h |    4 ++
 2 files changed, 67 insertions(+), 11 deletions(-)

Comments

Jesse Barnes March 26, 2013, 7:59 p.m. UTC | #1 
On Thu, 10 Jan 2013 10:02:42 -0500
Egbert Eich <eich@suse.de> wrote:

> +		if (hotplug_trigger & intel_connector->hpd_status_bit) {
> +			if (jiffies > (intel_connector->last_hpd_jiffies + msecs_to_jiffies(1000)) ||
> +			    jiffies < intel_connector->last_hpd_jiffies) {

Can you use time_after() here instead?

Otherwise this looks really nice.  We should get it lots of testing and
soak time.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c0e302e..4e75df0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -285,11 +285,24 @@  static void i915_hotplug_work_func(struct work_struct *work)
 						    hotplug_work);
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_mode_config *mode_config = &dev->mode_config;
+	struct drm_connector *connector;
 	struct intel_encoder *encoder;
+	unsigned long irqflags;
 
 	mutex_lock(&mode_config->mutex);
 	DRM_DEBUG_KMS("running encoder hotplug functions\n");
 
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+	list_for_each_entry(connector, &mode_config->connector_list, head) {
+		struct intel_connector *intel_connector = to_intel_connector(connector);
+		if (intel_connector->hpd_mark_disabled) {
+			intel_connector->hpd_mark_disabled = 0;
+			connector->polled = DRM_CONNECTOR_POLL_CONNECT
+				 | DRM_CONNECTOR_POLL_DISCONNECT;
+		}
+	}
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
 	list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
 		if (encoder->hot_plug)
 			encoder->hot_plug(encoder);
@@ -300,6 +313,36 @@  static void i915_hotplug_work_func(struct work_struct *work)
 	drm_helper_hpd_irq_event(dev);
 }
 
+static inline void hotplug_irq_storm_detect(struct drm_device *dev, u32 hotplug_trigger)
+{
+	struct drm_mode_config *mode_config = &dev->mode_config;
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct drm_connector *connector;
+	unsigned long irqflags;
+
+	spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+	list_for_each_entry(connector, &mode_config->connector_list, head) {
+		struct intel_connector *intel_connector = to_intel_connector(connector);
+
+		if (hotplug_trigger & intel_connector->hpd_status_bit) {
+			if (jiffies > (intel_connector->last_hpd_jiffies + msecs_to_jiffies(1000)) ||
+			    jiffies < intel_connector->last_hpd_jiffies) {
+				intel_connector->last_hpd_jiffies = jiffies;
+				intel_connector->hpd_cnt = 0;
+			} else if (intel_connector->hpd_cnt > 5) {
+				dev_priv->hotplug_supported_mask &= ~intel_connector->hpd_status_bit;
+				intel_connector->hpd_mark_disabled = 1;
+				pr_warn("HPD interrupt storm on connector %s disabling\n",
+					 drm_get_connector_name(connector));
+			} else
+				intel_connector->hpd_cnt++;
+		}
+	}
+
+	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
 /* defined intel_pm.c */
 extern spinlock_t mchdev_lock;
 
@@ -579,13 +622,14 @@  static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		/* Consume port.  Then clear IIR or we'll miss events */
 		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
 			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
-
+			u32 hotplug_trigger = hotplug_status & dev_priv->hotplug_supported_mask;
 			DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
 					 hotplug_status);
-			if (hotplug_status & dev_priv->hotplug_supported_mask)
+			if (hotplug_trigger) {
+				hotplug_irq_storm_detect(dev, hotplug_trigger);
 				queue_work(dev_priv->wq,
 					   &dev_priv->hotplug_work);
-
+			}
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
@@ -609,10 +653,12 @@  static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int pipe;
+	u32 hotplug_trigger = pch_iir & dev_priv->hotplug_supported_mask;
 
-	if (pch_iir & SDE_HOTPLUG_MASK)
+	if (hotplug_trigger) {
+		hotplug_irq_storm_detect(dev, hotplug_trigger);
 		queue_work(dev_priv->wq, &dev_priv->hotplug_work);
-
+	}
 	if (pch_iir & SDE_AUDIO_POWER_MASK)
 		DRM_DEBUG_DRIVER("PCH audio power change on port %d\n",
 				 (pch_iir & SDE_AUDIO_POWER_MASK) >>
@@ -652,10 +698,12 @@  static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
 	int pipe;
+	u32 hotplug_trigger = pch_iir & dev_priv->hotplug_supported_mask;
 
-	if (pch_iir & SDE_HOTPLUG_MASK_CPT)
+	if (hotplug_trigger) {
+		hotplug_irq_storm_detect(dev, hotplug_trigger);
 		queue_work(dev_priv->wq, &dev_priv->hotplug_work);
-
+	}
 	if (pch_iir & SDE_AUDIO_POWER_MASK_CPT)
 		DRM_DEBUG_DRIVER("PCH audio power change on port %d\n",
 				 (pch_iir & SDE_AUDIO_POWER_MASK_CPT) >>
@@ -2368,13 +2416,15 @@  static irqreturn_t i915_irq_handler(int irq, void *arg)
 		if ((I915_HAS_HOTPLUG(dev)) &&
 		    (iir & I915_DISPLAY_PORT_INTERRUPT)) {
 			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+			u32 hotplug_trigger = hotplug_status & dev_priv->hotplug_supported_mask;
 
 			DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
 				  hotplug_status);
-			if (hotplug_status & dev_priv->hotplug_supported_mask)
+			if (hotplug_trigger) {
+				hotplug_irq_storm_detect(dev, hotplug_trigger);
 				queue_work(dev_priv->wq,
 					   &dev_priv->hotplug_work);
-
+			}
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 			POSTING_READ(PORT_HOTPLUG_STAT);
 		}
@@ -2606,13 +2656,15 @@  static irqreturn_t i965_irq_handler(int irq, void *arg)
 		/* Consume port.  Then clear IIR or we'll miss events */
 		if (iir & I915_DISPLAY_PORT_INTERRUPT) {
 			u32 hotplug_status = I915_READ(PORT_HOTPLUG_STAT);
+			u32 hotplug_trigger = hotplug_status & dev_priv->hotplug_supported_mask;
 
 			DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
 				  hotplug_status);
-			if (hotplug_status & dev_priv->hotplug_supported_mask)
+			if (hotplug_trigger) {
+				hotplug_irq_storm_detect(dev, hotplug_trigger);
 				queue_work(dev_priv->wq,
 					   &dev_priv->hotplug_work);
-
+			}
 			I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status);
 			I915_READ(PORT_HOTPLUG_STAT);
 		}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a251036..2d003d9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -194,6 +194,10 @@  struct intel_connector {
 
 	/* hpd status bit for this connector */
 	u32 hpd_status_bit;
+	/* bookkeeping for irq storm detection */
+	int hpd_cnt;
+	unsigned long last_hpd_jiffies;
+	bool hpd_mark_disabled;
 };
 
 struct intel_crtc {