| Message ID | 1362043155-20747-1-git-send-email-eich@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 28, 2013 at 04:19:15AM -0500, Egbert Eich wrote: > Add a hotplug IRQ storm detection (triggered when a hotplug interrupt > fires more than 5 times / sec). > 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. > > v2: Fixed comment. > > Signed-off-by: Egbert Eich <eich@suse.de> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_drv.h | 9 +++++ > drivers/gpu/drm/i915/i915_irq.c | 63 +++++++++++++++++++++++++++++++------- > 2 files changed, 60 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index d8604a6..6ca742d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1059,6 +1059,15 @@ typedef struct drm_i915_private { > /* Old dri1 support infrastructure, beware the dragons ya fools entering > * here! */ > struct i915_dri1_state dri1; > + struct { > + unsigned long hpd_last_jiffies; > + int hpd_cnt; > + enum { > + HPD_ENABLED = 0, > + HPD_DISABLED = 1, > + HPD_MARK_DISABLED = 2 > + } hpd_mark; > + } hpd_stats[HPD_NUM_PINS]; It's still hidden in the dri1 dungeon ;-) Imo this would fit nicely next to the existing hotplug work and state tracking stuff. -Daniel > } drm_i915_private_t; > > /* Iterate over initialised rings */ > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index c40c7cc..24cb6ed 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -578,6 +578,34 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, > queue_work(dev_priv->wq, &dev_priv->rps.work); > } > > +static inline void hotplug_irq_storm_detect(struct drm_device *dev, > + u32 hotplug_trigger, > + const u32 *hpd) > +{ > + drm_i915_private_t *dev_priv = dev->dev_private; > + unsigned long irqflags; > + int i; > + > + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); > + > + for (i = 1; i < HPD_NUM_PINS; i++) { > + if ((hpd[i] & hotplug_trigger) && > + dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { > + if (jiffies > (dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)) || > + jiffies < dev_priv->hpd_stats[i].hpd_last_jiffies) { > + dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; > + dev_priv->hpd_stats[i].hpd_cnt = 0; > + } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { > + dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; > + DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); > + } else > + dev_priv->hpd_stats[i].hpd_cnt++; > + } > + } > + > + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); > +} > + > static void gmbus_irq_handler(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; > @@ -646,13 +674,15 @@ 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 & HOTPLUG_INT_STATUS_I915; > > DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", > hotplug_status); > - if (hotplug_status & HOTPLUG_INT_STATUS_I915) > + if (hotplug_trigger) { > + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915); > queue_work(dev_priv->wq, > &dev_priv->hotplug_work); > - > + } > I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status); > I915_READ(PORT_HOTPLUG_STAT); > } > @@ -676,10 +706,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 & SDE_HOTPLUG_MASK; > > - if (pch_iir & SDE_HOTPLUG_MASK) > + if (hotplug_trigger) { > + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_ibx); > 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) >> > @@ -722,10 +754,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 & SDE_HOTPLUG_MASK_CPT; > > - if (pch_iir & SDE_HOTPLUG_MASK_CPT) > + if (hotplug_trigger) { > + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_cpt); > 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) >> > @@ -2468,13 +2502,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 & HOTPLUG_INT_STATUS_I915; > > DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", > hotplug_status); > - if (hotplug_status & HOTPLUG_INT_STATUS_I915) > + if (hotplug_trigger) { > + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915); > queue_work(dev_priv->wq, > &dev_priv->hotplug_work); > - > + } > I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status); > POSTING_READ(PORT_HOTPLUG_STAT); > } > @@ -2701,15 +2737,18 @@ 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 & (IS_G4X(dev) ? > + HOTPLUG_INT_STATUS_G4X : > + HOTPLUG_INT_STATUS_I965); > > DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", > hotplug_status); > - if (hotplug_status & (IS_G4X(dev) ? > - HOTPLUG_INT_STATUS_G4X : > - HOTPLUG_INT_STATUS_I965)) > + if (hotplug_trigger) { > + hotplug_irq_storm_detect(dev, hotplug_trigger, > + IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i965); > queue_work(dev_priv->wq, > &dev_priv->hotplug_work); > - > + } > I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status); > I915_READ(PORT_HOTPLUG_STAT); > } > -- > 1.7.7 >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d8604a6..6ca742d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1059,6 +1059,15 @@ typedef struct drm_i915_private { /* Old dri1 support infrastructure, beware the dragons ya fools entering * here! */ struct i915_dri1_state dri1; + struct { + unsigned long hpd_last_jiffies; + int hpd_cnt; + enum { + HPD_ENABLED = 0, + HPD_DISABLED = 1, + HPD_MARK_DISABLED = 2 + } hpd_mark; + } hpd_stats[HPD_NUM_PINS]; } drm_i915_private_t; /* Iterate over initialised rings */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index c40c7cc..24cb6ed 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -578,6 +578,34 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv, queue_work(dev_priv->wq, &dev_priv->rps.work); } +static inline void hotplug_irq_storm_detect(struct drm_device *dev, + u32 hotplug_trigger, + const u32 *hpd) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + unsigned long irqflags; + int i; + + spin_lock_irqsave(&dev_priv->irq_lock, irqflags); + + for (i = 1; i < HPD_NUM_PINS; i++) { + if ((hpd[i] & hotplug_trigger) && + dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) { + if (jiffies > (dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)) || + jiffies < dev_priv->hpd_stats[i].hpd_last_jiffies) { + dev_priv->hpd_stats[i].hpd_last_jiffies = jiffies; + dev_priv->hpd_stats[i].hpd_cnt = 0; + } else if (dev_priv->hpd_stats[i].hpd_cnt > 5) { + dev_priv->hpd_stats[i].hpd_mark = HPD_MARK_DISABLED; + DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i); + } else + dev_priv->hpd_stats[i].hpd_cnt++; + } + } + + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags); +} + static void gmbus_irq_handler(struct drm_device *dev) { struct drm_i915_private *dev_priv = (drm_i915_private_t *) dev->dev_private; @@ -646,13 +674,15 @@ 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 & HOTPLUG_INT_STATUS_I915; DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & HOTPLUG_INT_STATUS_I915) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915); queue_work(dev_priv->wq, &dev_priv->hotplug_work); - + } I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status); I915_READ(PORT_HOTPLUG_STAT); } @@ -676,10 +706,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 & SDE_HOTPLUG_MASK; - if (pch_iir & SDE_HOTPLUG_MASK) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_ibx); 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) >> @@ -722,10 +754,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 & SDE_HOTPLUG_MASK_CPT; - if (pch_iir & SDE_HOTPLUG_MASK_CPT) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_cpt); 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) >> @@ -2468,13 +2502,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 & HOTPLUG_INT_STATUS_I915; DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & HOTPLUG_INT_STATUS_I915) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915); queue_work(dev_priv->wq, &dev_priv->hotplug_work); - + } I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status); POSTING_READ(PORT_HOTPLUG_STAT); } @@ -2701,15 +2737,18 @@ 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 & (IS_G4X(dev) ? + HOTPLUG_INT_STATUS_G4X : + HOTPLUG_INT_STATUS_I965); DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n", hotplug_status); - if (hotplug_status & (IS_G4X(dev) ? - HOTPLUG_INT_STATUS_G4X : - HOTPLUG_INT_STATUS_I965)) + if (hotplug_trigger) { + hotplug_irq_storm_detect(dev, hotplug_trigger, + IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i965); queue_work(dev_priv->wq, &dev_priv->hotplug_work); - + } I915_WRITE(PORT_HOTPLUG_STAT, hotplug_status); I915_READ(PORT_HOTPLUG_STAT); }