From: Daniel Vetter <daniel@ffwll.ch>
To: Egbert Eich <eich@suse.de>
Cc: intel-gfx@lists.freedesktop.org,
Chris Wilson <chris.wilson@intel.com>,
Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection (v2)
Date: Sun, 3 Mar 2013 19:07:43 +0100 [thread overview]
Message-ID: <20130303180743.GO9021@phenom.ffwll.local> (raw)
In-Reply-To: <1362043155-20747-1-git-send-email-eich@suse.de>
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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
next prev parent reply other threads:[~2013-03-03 18:05 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-10 15:02 [PATCH 0/8] Detect and deal with Interrupt 'Storms' from noisy Hotplug Lines Egbert Eich
2013-01-10 15:02 ` [PATCH 1/8] drm/i915: Remove pch_rq_mask from struct drm_i915_private Egbert Eich
2013-01-11 20:13 ` Daniel Vetter
2013-01-10 15:02 ` [PATCH 2/8] drm/i915: Set hotplug_supported_flag for all chipset generations Egbert Eich
2013-03-26 19:51 ` Jesse Barnes
2013-01-10 15:02 ` [PATCH 3/8] drm/i915: Add hpd status bit to struct intel_connector Egbert Eich
2013-03-26 19:53 ` Jesse Barnes
2013-01-10 15:02 ` [PATCH 4/8] drm/i915: Add Hotplug IRQ Storm detection Egbert Eich
2013-03-26 19:59 ` Jesse Barnes
2013-01-10 15:02 ` [PATCH 5/8] drm/i915: Move hotplug interrupt enable for i915/i965/valleyview into a separate function Egbert Eich
2013-03-26 20:04 ` Jesse Barnes
2013-01-10 15:02 ` [PATCH 6/8] drm/i915: Only enable hotplug irq when needed on Ironlake and later chips Egbert Eich
2013-01-10 15:02 ` [PATCH 7/8] drm/i915: When detecting a hotplug IRQ storm disable respective IRQs Egbert Eich
2013-01-10 15:02 ` [PATCH 8/8] drm/i915: Add Reenable Timer to turn Hotplug Detection back on Egbert Eich
2013-01-11 20:34 ` [PATCH 0/8] Detect and deal with Interrupt 'Storms' from noisy Hotplug Lines Daniel Vetter
2013-01-17 14:01 ` Egbert Eich
2013-01-17 14:45 ` Daniel Vetter
2013-01-22 13:22 ` Egbert Eich
2013-01-22 13:48 ` Daniel Vetter
2013-01-22 15:11 ` Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 01/12] DRM/i915: Remove valleyview_hpd_irq_setup Egbert Eich
2013-03-26 20:06 ` Jesse Barnes
2013-02-25 17:06 ` [PATCH v.2 02/12] DRM/I915: Add enum hpd_pin to intel_encoder Egbert Eich
2013-03-26 20:07 ` Jesse Barnes
2013-02-25 17:06 ` [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders Egbert Eich
2013-02-28 0:12 ` Chris Wilson
2013-02-28 9:17 ` [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders (v2) Egbert Eich
2013-03-26 20:08 ` [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders Jesse Barnes
2013-02-25 17:06 ` [PATCH v.2 04/12] DRM/i915: Remove i965_hpd_irq_setup Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 05/12] DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private Egbert Eich
2013-03-26 21:06 ` Daniel Vetter
2013-03-27 15:08 ` Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection Egbert Eich
2013-02-28 0:30 ` Chris Wilson
2013-02-28 9:19 ` [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection (v2) Egbert Eich
2013-03-03 18:07 ` Daniel Vetter [this message]
2013-03-05 7:38 ` [PATCH v.3 06/12] DRM/i915: Add HPD IRQ storm detection (v3) Egbert Eich
2013-03-05 7:48 ` [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on (v2) Egbert Eich
2013-03-05 10:28 ` Ville Syrjälä
2013-03-05 12:26 ` [PATCH v.3 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on (v3) Egbert Eich
2013-03-05 7:55 ` [PATCH v.2 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v2) Egbert Eich
2013-03-05 13:00 ` [PATCH v.3 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v3) Egbert Eich
2013-03-05 14:52 ` Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 07/12] DRM/i915: (re)init HPD interrupt storm statistics Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 08/12] DRM/i915: Treat hpd_irq_setup() for ironake and older generations the same way Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 09/12] DRM/i915: Disable HPD interrupt on pin when irq storm is detected Egbert Eich
2013-03-05 12:34 ` [PATCH v.2 09/12] DRM/i915: Disable HPD interrupt on pin when irq storm is detected (v2) Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on Egbert Eich
2013-03-27 15:12 ` Daniel Vetter
2013-02-25 17:06 ` [PATCH v.2 11/12] DRM/i915: Add bit field to record which pins have received HPD events Egbert Eich
2013-02-25 17:06 ` [PATCH v.2 12/12] DRM/i915: Only reprobe display on encoder which has received an HPD event Egbert Eich
2013-03-05 14:18 ` [PATCH v.3 " Egbert Eich
2013-02-28 0:46 ` [PATCH v.2 00/12] Detect and deal with Interrupt 'Storms' from noisy Hotplug Lines Chris Wilson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130303180743.GO9021@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=chris.wilson@intel.com \
--cc=eich@suse.de \
--cc=intel-gfx@lists.freedesktop.org \
--cc=rodrigo.vivi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox