* [PATCH v.2 01/12] DRM/i915: Remove valleyview_hpd_irq_setup.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
@ 2013-02-25 17:06 ` 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
` (11 subsequent siblings)
12 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
It's basically identical to i915_hpd_irq_setup().
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 26 +-------------------------
1 files changed, 1 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2cd97d1..5fd3267 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2113,30 +2113,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
return 0;
}
-static void valleyview_hpd_irq_setup(struct drm_device *dev)
-{
- drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
-
- /* Note HDMI and DP share bits */
- if (dev_priv->hotplug_supported_mask & PORTB_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTC_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTD_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTD_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_I915)
- hotplug_en |= SDVOC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_I915)
- hotplug_en |= SDVOB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
- hotplug_en |= CRT_HOTPLUG_INT_EN;
- hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
- }
-
- I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
-}
-
static void valleyview_irq_uninstall(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
@@ -2835,7 +2811,7 @@ void intel_irq_init(struct drm_device *dev)
dev->driver->irq_uninstall = valleyview_irq_uninstall;
dev->driver->enable_vblank = valleyview_enable_vblank;
dev->driver->disable_vblank = valleyview_disable_vblank;
- dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup;
+ dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
/* Share pre & uninstall handlers with ILK/SNB */
dev->driver->irq_handler = ivybridge_irq_handler;
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 01/12] DRM/i915: Remove valleyview_hpd_irq_setup.
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
0 siblings, 0 replies; 54+ messages in thread
From: Jesse Barnes @ 2013-03-26 20:06 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, 25 Feb 2013 12:06:48 -0500
Egbert Eich <eich@suse.de> wrote:
> It's basically identical to i915_hpd_irq_setup().
>
> Signed-off-by: Egbert Eich <eich@suse.de>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 26 +-------------------------
> 1 files changed, 1 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 2cd97d1..5fd3267 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2113,30 +2113,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
> return 0;
> }
>
> -static void valleyview_hpd_irq_setup(struct drm_device *dev)
> -{
> - drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> - u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
> -
> - /* Note HDMI and DP share bits */
> - if (dev_priv->hotplug_supported_mask & PORTB_HOTPLUG_INT_STATUS)
> - hotplug_en |= PORTB_HOTPLUG_INT_EN;
> - if (dev_priv->hotplug_supported_mask & PORTC_HOTPLUG_INT_STATUS)
> - hotplug_en |= PORTC_HOTPLUG_INT_EN;
> - if (dev_priv->hotplug_supported_mask & PORTD_HOTPLUG_INT_STATUS)
> - hotplug_en |= PORTD_HOTPLUG_INT_EN;
> - if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_I915)
> - hotplug_en |= SDVOC_HOTPLUG_INT_EN;
> - if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_I915)
> - hotplug_en |= SDVOB_HOTPLUG_INT_EN;
> - if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
> - hotplug_en |= CRT_HOTPLUG_INT_EN;
> - hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
> - }
> -
> - I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
> -}
> -
> static void valleyview_irq_uninstall(struct drm_device *dev)
> {
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> @@ -2835,7 +2811,7 @@ void intel_irq_init(struct drm_device *dev)
> dev->driver->irq_uninstall = valleyview_irq_uninstall;
> dev->driver->enable_vblank = valleyview_enable_vblank;
> dev->driver->disable_vblank = valleyview_disable_vblank;
> - dev_priv->display.hpd_irq_setup = valleyview_hpd_irq_setup;
> + dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
> } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
> /* Share pre & uninstall handlers with ILK/SNB */
> dev->driver->irq_handler = ivybridge_irq_handler;
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v.2 02/12] DRM/I915: Add enum hpd_pin to intel_encoder.
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-02-25 17:06 ` 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
` (10 subsequent siblings)
12 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
To clean up hotplug support we add a new enum to intel_encoder:
enum hpd_pin. It allows the encoder to request a hpd line but leave
the details which IRQ is responsible on which chipset generation
to i915_irq.c.
This way requesting hotplug support will become really simple on
the encoder/connector level.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 13 +++++++++++++
drivers/gpu/drm/i915/intel_crt.c | 2 ++
drivers/gpu/drm/i915/intel_dp.c | 4 ++++
drivers/gpu/drm/i915/intel_drv.h | 1 +
drivers/gpu/drm/i915/intel_hdmi.c | 4 ++++
drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
6 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e95337c..316747a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -86,6 +86,19 @@ enum port {
};
#define port_name(p) ((p) + 'A')
+enum hpd_pin {
+ HPD_NONE = 0,
+ HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
+ HPD_TV = HPD_NONE, /* TV is known to be unreliable */
+ HPD_CRT,
+ HPD_SDVO_B,
+ HPD_SDVO_C,
+ HPD_PORT_B,
+ HPD_PORT_C,
+ HPD_PORT_D,
+ HPD_NUM_PINS
+};
+
#define I915_GEM_GPU_DOMAINS \
(I915_GEM_DOMAIN_RENDER | \
I915_GEM_DOMAIN_SAMPLER | \
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index cfc9687..de43cab 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -777,6 +777,8 @@ void intel_crt_init(struct drm_device *dev)
crt->base.disable = intel_disable_crt;
crt->base.enable = intel_enable_crt;
+ if (I915_HAS_HOTPLUG(dev))
+ crt->base.hpd_pin = HPD_CRT;
if (HAS_DDI(dev))
crt->base.get_hw_state = intel_ddi_get_hw_state;
else
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 7b8bfe8..549933a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2835,18 +2835,22 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
/* Set up the DDC bus. */
switch (port) {
case PORT_A:
+ intel_encoder->hpd_pin = HPD_PORT_A;
name = "DPDDC-A";
break;
case PORT_B:
dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS;
+ intel_encoder->hpd_pin = HPD_PORT_B;
name = "DPDDC-B";
break;
case PORT_C:
dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS;
+ intel_encoder->hpd_pin = HPD_PORT_C;
name = "DPDDC-C";
break;
case PORT_D:
dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS;
+ intel_encoder->hpd_pin = HPD_PORT_D;
name = "DPDDC-D";
break;
default:
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 005a91f..c9003bd 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -168,6 +168,7 @@ struct intel_encoder {
* it is connected to in the pipe parameter. */
bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
int crtc_mask;
+ enum hpd_pin hpd_pin;
};
struct intel_panel {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 5a6138c..8cd6bc1 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1022,17 +1022,21 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
switch (port) {
case PORT_B:
intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
+ intel_encoder->hpd_pin = HPD_PORT_B;
dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS;
break;
case PORT_C:
intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
+ intel_encoder->hpd_pin = HPD_PORT_C;
dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS;
break;
case PORT_D:
intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
+ intel_encoder->hpd_pin = HPD_PORT_D;
dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS;
break;
case PORT_A:
+ intel_encoder->hpd_pin = HPD_PORT_A;
/* Internal port only for eDP. */
default:
BUG();
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f01063a..cf57705 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2780,6 +2780,9 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
SDVOB_HOTPLUG_INT_STATUS_I915 : SDVOC_HOTPLUG_INT_STATUS_I915;
}
+ if (intel_sdvo->hotplug_active)
+ intel_encoder->hpd_pin = HPD_SDVO_B ? HPD_SDVO_B : HPD_SDVO_C;
+
drm_encoder_helper_add(&intel_encoder->base, &intel_sdvo_helper_funcs);
intel_encoder->disable = intel_disable_sdvo;
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 02/12] DRM/I915: Add enum hpd_pin to intel_encoder.
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
0 siblings, 0 replies; 54+ messages in thread
From: Jesse Barnes @ 2013-03-26 20:07 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, 25 Feb 2013 12:06:49 -0500
Egbert Eich <eich@suse.de> wrote:
> To clean up hotplug support we add a new enum to intel_encoder:
> enum hpd_pin. It allows the encoder to request a hpd line but leave
> the details which IRQ is responsible on which chipset generation
> to i915_irq.c.
> This way requesting hotplug support will become really simple on
> the encoder/connector level.
>
> Signed-off-by: Egbert Eich <eich@suse.de>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 13 +++++++++++++
> drivers/gpu/drm/i915/intel_crt.c | 2 ++
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> drivers/gpu/drm/i915/intel_drv.h | 1 +
> drivers/gpu/drm/i915/intel_hdmi.c | 4 ++++
> drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> 6 files changed, 27 insertions(+), 0 deletions(-)
Even better than the last one I suppose...
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders.
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-02-25 17:06 ` [PATCH v.2 02/12] DRM/I915: Add enum hpd_pin to intel_encoder Egbert Eich
@ 2013-02-25 17:06 ` Egbert Eich
2013-02-28 0:12 ` Chris Wilson
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
` (9 subsequent siblings)
12 siblings, 2 replies; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
This allows to enable HPD interrupts for individual pins to only receive
hotplug events from lines which are connected and working.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 160 ++++++++++++++++++++++++--------------
drivers/gpu/drm/i915/i915_reg.h | 32 ++++++++-
2 files changed, 132 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5fd3267..306cd79 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -36,6 +36,68 @@
#include "i915_trace.h"
#include "intel_drv.h"
+static const u32 hpd_ibx[] = {
+ 0, /* HPD_NONE */
+ SDE_CRT_HOTPLUG, /* HPD_CRT */
+ SDE_SDVOB_HOTPLUG, /* HPD_SDVO_B */
+ 0, /* HPD_SDVO_C */
+ SDE_PORTB_HOTPLUG, /* HPD_PORT_B */
+ SDE_PORTC_HOTPLUG, /* HPD_PORT_C */
+ SDE_PORTD_HOTPLUG /* HPD_PORT_D */
+};
+
+static const u32 hpd_cpt[] = {
+ 0, /* HPD_NONE */
+ SDE_CRT_HOTPLUG_CPT, /* HPD_CRT */
+ 0, /* HPD_SDVO_B */
+ 0, /* HPD_SDVO_C */
+ SDE_PORTB_HOTPLUG_CPT, /* HPD_PORT_B */
+ SDE_PORTC_HOTPLUG_CPT, /* HPD_PORT_C */
+ SDE_PORTD_HOTPLUG_CPT /* HPD_PORT_D */
+};
+
+static const u32 hpd_mask_i915[] = {
+ 0, /* HPD_NONE */
+ CRT_HOTPLUG_INT_EN, /* HPD_CRT */
+ SDVOB_HOTPLUG_INT_EN, /* HPD_SDVO_B */
+ SDVOC_HOTPLUG_INT_EN, /* HPD_SDVO_C */
+ PORTB_HOTPLUG_INT_EN, /* HPD_PORT_B */
+ PORTC_HOTPLUG_INT_EN, /* HPD_PORT_C */
+ PORTD_HOTPLUG_INT_EN /* HPD_PORT_D */
+};
+
+static const u32 hpd_status_gen4[] = {
+ 0, /* HPD_NONE */
+ CRT_HOTPLUG_INT_STATUS, /* HPD_CRT */
+ SDVOB_HOTPLUG_INT_STATUS_G4X, /* HPD_SDVO_B */
+ SDVOC_HOTPLUG_INT_STATUS_G4X, /* HPD_SDVO_C */
+ PORTB_HOTPLUG_INT_STATUS, /* HPD_PORT_B */
+ PORTC_HOTPLUG_INT_STATUS, /* HPD_PORT_C */
+ PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */
+};
+
+static const u32 hpd_status_i965[] = {
+ 0, /* HPD_NONE */
+ CRT_HOTPLUG_INT_STATUS, /* HPD_CRT */
+ SDVOB_HOTPLUG_INT_STATUS_I965, /* HPD_SDVO_B */
+ SDVOC_HOTPLUG_INT_STATUS_I965, /* HPD_SDVO_C */
+ PORTB_HOTPLUG_INT_STATUS, /* HPD_PORT_B */
+ PORTC_HOTPLUG_INT_STATUS, /* HPD_PORT_C */
+ PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */
+};
+
+static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
+ 0, /* HPD_NONE */
+ CRT_HOTPLUG_INT_STATUS, /* HPD_CRT */
+ SDVOB_HOTPLUG_INT_STATUS_I915, /* HPD_SDVO_B */
+ SDVOC_HOTPLUG_INT_STATUS_I915, /* HPD_SDVO_C */
+ PORTB_HOTPLUG_INT_STATUS, /* HPD_PORT_B */
+ PORTC_HOTPLUG_INT_STATUS, /* HPD_PORT_C */
+ PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */
+};
+
+
+
/* For display hotplug interrupt */
static void
ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
@@ -596,7 +658,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
- if (hotplug_status & dev_priv->hotplug_supported_mask)
+ if (hotplug_status & HOTPLUG_INT_STATUS_I915)
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
@@ -1940,17 +2002,21 @@ static void ibx_enable_hotplug(struct drm_device *dev)
static void ibx_irq_postinstall(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- u32 mask;
-
- if (HAS_PCH_IBX(dev))
- mask = SDE_HOTPLUG_MASK |
- SDE_GMBUS |
- SDE_AUX_MASK;
- else
- mask = SDE_HOTPLUG_MASK_CPT |
- SDE_GMBUS_CPT |
- SDE_AUX_MASK_CPT;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *intel_encoder;
+ u32 mask = I915_READ(SDEIER);
+ if (HAS_PCH_IBX(dev)) {
+ mask &= ~SDE_HOTPLUG_MASK;
+ list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
+ mask |= hpd_ibx[intel_encoder->hpd_pin];
+ mask |= SDE_GMBUS | SDE_AUX_MASK;
+ } else {
+ mask &= ~SDE_HOTPLUG_MASK_CPT;
+ list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
+ mask |= hpd_cpt[intel_encoder->hpd_pin];
+ mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
+ }
I915_WRITE(SDEIIR, I915_READ(SDEIIR));
I915_WRITE(SDEIMR, ~mask);
I915_WRITE(SDEIER, mask);
@@ -2360,26 +2426,16 @@ static int i915_irq_postinstall(struct drm_device *dev)
static void i915_hpd_irq_setup(struct drm_device *dev)
{
- drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- u32 hotplug_en;
-
if (I915_HAS_HOTPLUG(dev)) {
- hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *encoder;
+ u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
- if (dev_priv->hotplug_supported_mask & PORTB_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTC_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTD_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTD_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_I915)
- hotplug_en |= SDVOC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_I915)
- hotplug_en |= SDVOB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
- hotplug_en |= CRT_HOTPLUG_INT_EN;
- hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
- }
+ hotplug_en &= ~HOTPLUG_INT_EN_MASK;
+ list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
+ hotplug_en |= hpd_mask_i915[encoder->hpd_pin];
+ hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
/* Ignore TV since it's buggy */
@@ -2443,7 +2499,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
- if (hotplug_status & dev_priv->hotplug_supported_mask)
+ if (hotplug_status & HOTPLUG_INT_STATUS_I915)
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
@@ -2596,38 +2652,22 @@ static int i965_irq_postinstall(struct drm_device *dev)
static void i965_hpd_irq_setup(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *encoder;
u32 hotplug_en;
/* Note HDMI and DP share hotplug bits */
hotplug_en = 0;
- if (dev_priv->hotplug_supported_mask & PORTB_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTC_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTD_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTD_HOTPLUG_INT_EN;
- if (IS_G4X(dev)) {
- if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_G4X)
- hotplug_en |= SDVOC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_G4X)
- hotplug_en |= SDVOB_HOTPLUG_INT_EN;
- } else {
- if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_I965)
- hotplug_en |= SDVOC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_I965)
- hotplug_en |= SDVOB_HOTPLUG_INT_EN;
- }
- if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
- hotplug_en |= CRT_HOTPLUG_INT_EN;
-
- /* Programming the CRT detection parameters tends
- to generate a spurious hotplug event about three
- seconds later. So just do it once.
- */
- if (IS_G4X(dev))
- hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
- hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
- }
+ list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
+ /* enable bits are the same for all generations */
+ hotplug_en |= hpd_mask_i915[encoder->hpd_pin];
+ /* Programming the CRT detection parameters tends
+ to generate a spurious hotplug event about three
+ seconds later. So just do it once.
+ */
+ if (IS_G4X(dev))
+ hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
+ hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
/* Ignore TV since it's buggy */
@@ -2690,7 +2730,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
- if (hotplug_status & dev_priv->hotplug_supported_mask)
+ if (hotplug_status & (IS_G4X(dev) ?
+ HOTPLUG_INT_STATUS_G4X :
+ HOTPLUG_INT_STATUS_I965))
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 527b664..c3d2c01 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1637,6 +1637,12 @@
#define SDVOC_HOTPLUG_INT_EN (1 << 25)
#define TV_HOTPLUG_INT_EN (1 << 18)
#define CRT_HOTPLUG_INT_EN (1 << 9)
+#define HOTPLUG_INT_EN_MASK (PORTB_HOTPLUG_INT_EN | \
+ PORTC_HOTPLUG_INT_EN | \
+ PORTD_HOTPLUG_INT_EN | \
+ SDVOC_HOTPLUG_INT_EN | \
+ SDVOB_HOTPLUG_INT_EN | \
+ CRT_HOTPLUG_INT_EN)
#define CRT_HOTPLUG_FORCE_DETECT (1 << 3)
#define CRT_HOTPLUG_ACTIVATION_PERIOD_32 (0 << 8)
/* must use period 64 on GM45 according to docs */
@@ -1675,6 +1681,26 @@
#define SDVOB_HOTPLUG_INT_STATUS_I965 (3 << 2)
#define SDVOC_HOTPLUG_INT_STATUS_I915 (1 << 7)
#define SDVOB_HOTPLUG_INT_STATUS_I915 (1 << 6)
+#define HOTPLUG_INT_STATUS_G4X (CRT_HOTPLUG_INT_STATUS | \
+ SDVOB_HOTPLUG_INT_STATUS_G4X | \
+ SDVOC_HOTPLUG_INT_STATUS_G4X | \
+ PORTB_HOTPLUG_INT_STATUS | \
+ PORTC_HOTPLUG_INT_STATUS | \
+ PORTD_HOTPLUG_INT_STATUS)
+
+#define HOTPLUG_INT_STATUS_I965 (CRT_HOTPLUG_INT_STATUS | \
+ SDVOB_HOTPLUG_INT_STATUS_I965 | \
+ SDVOC_HOTPLUG_INT_STATUS_I965 | \
+ PORTB_HOTPLUG_INT_STATUS | \
+ PORTC_HOTPLUG_INT_STATUS | \
+ PORTD_HOTPLUG_INT_STATUS)
+
+#define HOTPLUG_INT_STATUS_I915 (CRT_HOTPLUG_INT_STATUS | \
+ SDVOB_HOTPLUG_INT_STATUS_I915 | \
+ SDVOC_HOTPLUG_INT_STATUS_I915 | \
+ PORTB_HOTPLUG_INT_STATUS | \
+ PORTC_HOTPLUG_INT_STATUS | \
+ PORTD_HOTPLUG_INT_STATUS)
/* SDVO port control */
#define SDVOB 0x61140
@@ -3508,7 +3534,11 @@
#define SDE_PORTC_HOTPLUG (1 << 9)
#define SDE_PORTB_HOTPLUG (1 << 8)
#define SDE_SDVOB_HOTPLUG (1 << 6)
-#define SDE_HOTPLUG_MASK (0xf << 8)
+#define SDE_HOTPLUG_MASK (SDE_CRT_HOTPLUG | \
+ SDE_SDVOB_HOTPLUG | \
+ SDE_PORTB_HOTPLUG | \
+ SDE_PORTC_HOTPLUG | \
+ SDE_PORTD_HOTPLUG)
#define SDE_TRANSB_CRC_DONE (1 << 5)
#define SDE_TRANSB_CRC_ERR (1 << 4)
#define SDE_TRANSB_FIFO_UNDER (1 << 3)
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders.
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
1 sibling, 1 reply; 54+ messages in thread
From: Chris Wilson @ 2013-02-28 0:12 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, Feb 25, 2013 at 12:06:50PM -0500, Egbert Eich wrote:
> This allows to enable HPD interrupts for individual pins to only receive
> hotplug events from lines which are connected and working.
>
> Signed-off-by: Egbert Eich <eich@suse.de>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 160 ++++++++++++++++++++++++--------------
> drivers/gpu/drm/i915/i915_reg.h | 32 ++++++++-
> 2 files changed, 132 insertions(+), 60 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 5fd3267..306cd79 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -36,6 +36,68 @@
> #include "i915_trace.h"
> #include "intel_drv.h"
>
> +static const u32 hpd_ibx[] = {
> + 0, /* HPD_NONE */
> + SDE_CRT_HOTPLUG, /* HPD_CRT */
> + SDE_SDVOB_HOTPLUG, /* HPD_SDVO_B */
> + 0, /* HPD_SDVO_C */
> + SDE_PORTB_HOTPLUG, /* HPD_PORT_B */
> + SDE_PORTC_HOTPLUG, /* HPD_PORT_C */
> + SDE_PORTD_HOTPLUG /* HPD_PORT_D */
> +};
Trivial bikeshed, these would look neater as
static const u32 hpd_ibx[] = {
[HPD_CRT] = SDE_CRTC_HOTPLUG,
[HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
[HPD_PORT_B] = SDE_PORTB_HOTPLUG,
[HPD_PORT_C] = SDE_PORTC_HOTPLUG,
[HPD_PORT_D] = SDE_PORTD_HOTPLUG,
};
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 54+ messages in thread* [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders (v2)
2013-02-28 0:12 ` Chris Wilson
@ 2013-02-28 9:17 ` Egbert Eich
0 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-02-28 9:17 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
This allows to enable HPD interrupts for individual pins to only receive
hotplug events from lines which are connected and working.
v2: Restructured initailization of const arrays following a suggstion
by Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Egbert Eich <eich@suse.de>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_irq.c | 151 ++++++++++++++++++++++++---------------
drivers/gpu/drm/i915/i915_reg.h | 32 ++++++++-
2 files changed, 123 insertions(+), 60 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5fd3267..7bdc90c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -36,6 +36,59 @@
#include "i915_trace.h"
#include "intel_drv.h"
+static const u32 hpd_ibx[] = {
+ [HPD_CRT] = SDE_CRT_HOTPLUG,
+ [HPD_SDVO_B] = SDE_SDVOB_HOTPLUG,
+ [HPD_PORT_B] = SDE_PORTB_HOTPLUG,
+ [HPD_PORT_C] = SDE_PORTC_HOTPLUG,
+ [HPD_PORT_D] = SDE_PORTD_HOTPLUG
+};
+
+static const u32 hpd_cpt[] = {
+ [HPD_CRT] = SDE_CRT_HOTPLUG_CPT,
+ [HPD_PORT_B] = SDE_PORTB_HOTPLUG_CPT,
+ [HPD_PORT_C] = SDE_PORTC_HOTPLUG_CPT,
+ [HPD_PORT_D] = SDE_PORTD_HOTPLUG_CPT
+};
+
+static const u32 hpd_mask_i915[] = {
+ [HPD_CRT] = CRT_HOTPLUG_INT_EN,
+ [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_EN,
+ [HPD_SDVO_C] = SDVOC_HOTPLUG_INT_EN,
+ [HPD_PORT_B] = PORTB_HOTPLUG_INT_EN,
+ [HPD_PORT_C] = PORTC_HOTPLUG_INT_EN,
+ [HPD_PORT_D] = PORTD_HOTPLUG_INT_EN
+};
+
+static const u32 hpd_status_gen4[] = {
+ [HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
+ [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_G4X,
+ [HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_G4X,
+ [HPD_PORT_B] = PORTB_HOTPLUG_INT_STATUS,
+ [HPD_PORT_C] = PORTC_HOTPLUG_INT_STATUS,
+ [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
+};
+
+static const u32 hpd_status_i965[] = {
+ [HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
+ [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I965,
+ [HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I965,
+ [HPD_PORT_B] = PORTB_HOTPLUG_INT_STATUS,
+ [HPD_PORT_C] = PORTC_HOTPLUG_INT_STATUS,
+ [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
+};
+
+static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
+ [HPD_CRT] = CRT_HOTPLUG_INT_STATUS,
+ [HPD_SDVO_B] = SDVOB_HOTPLUG_INT_STATUS_I915,
+ [HPD_SDVO_C] = SDVOC_HOTPLUG_INT_STATUS_I915,
+ [HPD_PORT_B] = PORTB_HOTPLUG_INT_STATUS,
+ [HPD_PORT_C] = PORTC_HOTPLUG_INT_STATUS,
+ [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
+};
+
+
+
/* For display hotplug interrupt */
static void
ironlake_enable_display_irq(drm_i915_private_t *dev_priv, u32 mask)
@@ -596,7 +649,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
- if (hotplug_status & dev_priv->hotplug_supported_mask)
+ if (hotplug_status & HOTPLUG_INT_STATUS_I915)
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
@@ -1940,17 +1993,21 @@ static void ibx_enable_hotplug(struct drm_device *dev)
static void ibx_irq_postinstall(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- u32 mask;
-
- if (HAS_PCH_IBX(dev))
- mask = SDE_HOTPLUG_MASK |
- SDE_GMBUS |
- SDE_AUX_MASK;
- else
- mask = SDE_HOTPLUG_MASK_CPT |
- SDE_GMBUS_CPT |
- SDE_AUX_MASK_CPT;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *intel_encoder;
+ u32 mask = I915_READ(SDEIER);
+ if (HAS_PCH_IBX(dev)) {
+ mask &= ~SDE_HOTPLUG_MASK;
+ list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
+ mask |= hpd_ibx[intel_encoder->hpd_pin];
+ mask |= SDE_GMBUS | SDE_AUX_MASK;
+ } else {
+ mask &= ~SDE_HOTPLUG_MASK_CPT;
+ list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
+ mask |= hpd_cpt[intel_encoder->hpd_pin];
+ mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
+ }
I915_WRITE(SDEIIR, I915_READ(SDEIIR));
I915_WRITE(SDEIMR, ~mask);
I915_WRITE(SDEIER, mask);
@@ -2360,26 +2417,16 @@ static int i915_irq_postinstall(struct drm_device *dev)
static void i915_hpd_irq_setup(struct drm_device *dev)
{
- drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- u32 hotplug_en;
-
if (I915_HAS_HOTPLUG(dev)) {
- hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *encoder;
+ u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
- if (dev_priv->hotplug_supported_mask & PORTB_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTC_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTD_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTD_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_I915)
- hotplug_en |= SDVOC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_I915)
- hotplug_en |= SDVOB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
- hotplug_en |= CRT_HOTPLUG_INT_EN;
- hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
- }
+ hotplug_en &= ~HOTPLUG_INT_EN_MASK;
+ list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
+ hotplug_en |= hpd_mask_i915[encoder->hpd_pin];
+ hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
/* Ignore TV since it's buggy */
@@ -2443,7 +2490,7 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
- if (hotplug_status & dev_priv->hotplug_supported_mask)
+ if (hotplug_status & HOTPLUG_INT_STATUS_I915)
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
@@ -2596,38 +2643,22 @@ static int i965_irq_postinstall(struct drm_device *dev)
static void i965_hpd_irq_setup(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct intel_encoder *encoder;
u32 hotplug_en;
/* Note HDMI and DP share hotplug bits */
hotplug_en = 0;
- if (dev_priv->hotplug_supported_mask & PORTB_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTB_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTC_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & PORTD_HOTPLUG_INT_STATUS)
- hotplug_en |= PORTD_HOTPLUG_INT_EN;
- if (IS_G4X(dev)) {
- if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_G4X)
- hotplug_en |= SDVOC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_G4X)
- hotplug_en |= SDVOB_HOTPLUG_INT_EN;
- } else {
- if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS_I965)
- hotplug_en |= SDVOC_HOTPLUG_INT_EN;
- if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS_I965)
- hotplug_en |= SDVOB_HOTPLUG_INT_EN;
- }
- if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS) {
- hotplug_en |= CRT_HOTPLUG_INT_EN;
-
- /* Programming the CRT detection parameters tends
- to generate a spurious hotplug event about three
- seconds later. So just do it once.
- */
- if (IS_G4X(dev))
- hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
- hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
- }
+ list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
+ /* enable bits are the same for all generations */
+ hotplug_en |= hpd_mask_i915[encoder->hpd_pin];
+ /* Programming the CRT detection parameters tends
+ to generate a spurious hotplug event about three
+ seconds later. So just do it once.
+ */
+ if (IS_G4X(dev))
+ hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
+ hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
/* Ignore TV since it's buggy */
@@ -2690,7 +2721,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
- if (hotplug_status & dev_priv->hotplug_supported_mask)
+ if (hotplug_status & (IS_G4X(dev) ?
+ HOTPLUG_INT_STATUS_G4X :
+ HOTPLUG_INT_STATUS_I965))
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 527b664..c3d2c01 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1637,6 +1637,12 @@
#define SDVOC_HOTPLUG_INT_EN (1 << 25)
#define TV_HOTPLUG_INT_EN (1 << 18)
#define CRT_HOTPLUG_INT_EN (1 << 9)
+#define HOTPLUG_INT_EN_MASK (PORTB_HOTPLUG_INT_EN | \
+ PORTC_HOTPLUG_INT_EN | \
+ PORTD_HOTPLUG_INT_EN | \
+ SDVOC_HOTPLUG_INT_EN | \
+ SDVOB_HOTPLUG_INT_EN | \
+ CRT_HOTPLUG_INT_EN)
#define CRT_HOTPLUG_FORCE_DETECT (1 << 3)
#define CRT_HOTPLUG_ACTIVATION_PERIOD_32 (0 << 8)
/* must use period 64 on GM45 according to docs */
@@ -1675,6 +1681,26 @@
#define SDVOB_HOTPLUG_INT_STATUS_I965 (3 << 2)
#define SDVOC_HOTPLUG_INT_STATUS_I915 (1 << 7)
#define SDVOB_HOTPLUG_INT_STATUS_I915 (1 << 6)
+#define HOTPLUG_INT_STATUS_G4X (CRT_HOTPLUG_INT_STATUS | \
+ SDVOB_HOTPLUG_INT_STATUS_G4X | \
+ SDVOC_HOTPLUG_INT_STATUS_G4X | \
+ PORTB_HOTPLUG_INT_STATUS | \
+ PORTC_HOTPLUG_INT_STATUS | \
+ PORTD_HOTPLUG_INT_STATUS)
+
+#define HOTPLUG_INT_STATUS_I965 (CRT_HOTPLUG_INT_STATUS | \
+ SDVOB_HOTPLUG_INT_STATUS_I965 | \
+ SDVOC_HOTPLUG_INT_STATUS_I965 | \
+ PORTB_HOTPLUG_INT_STATUS | \
+ PORTC_HOTPLUG_INT_STATUS | \
+ PORTD_HOTPLUG_INT_STATUS)
+
+#define HOTPLUG_INT_STATUS_I915 (CRT_HOTPLUG_INT_STATUS | \
+ SDVOB_HOTPLUG_INT_STATUS_I915 | \
+ SDVOC_HOTPLUG_INT_STATUS_I915 | \
+ PORTB_HOTPLUG_INT_STATUS | \
+ PORTC_HOTPLUG_INT_STATUS | \
+ PORTD_HOTPLUG_INT_STATUS)
/* SDVO port control */
#define SDVOB 0x61140
@@ -3508,7 +3534,11 @@
#define SDE_PORTC_HOTPLUG (1 << 9)
#define SDE_PORTB_HOTPLUG (1 << 8)
#define SDE_SDVOB_HOTPLUG (1 << 6)
-#define SDE_HOTPLUG_MASK (0xf << 8)
+#define SDE_HOTPLUG_MASK (SDE_CRT_HOTPLUG | \
+ SDE_SDVOB_HOTPLUG | \
+ SDE_PORTB_HOTPLUG | \
+ SDE_PORTC_HOTPLUG | \
+ SDE_PORTD_HOTPLUG)
#define SDE_TRANSB_CRC_DONE (1 << 5)
#define SDE_TRANSB_CRC_ERR (1 << 4)
#define SDE_TRANSB_FIFO_UNDER (1 << 3)
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v.2 03/12] DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in encoders.
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-03-26 20:08 ` Jesse Barnes
1 sibling, 0 replies; 54+ messages in thread
From: Jesse Barnes @ 2013-03-26 20:08 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, 25 Feb 2013 12:06:50 -0500
Egbert Eich <eich@suse.de> wrote:
> This allows to enable HPD interrupts for individual pins to only receive
> hotplug events from lines which are connected and working.
>
> Signed-off-by: Egbert Eich <eich@suse.de>
> ---
In reviewing the old one, I thought tables might be nice, but on
seeing it I'm not sure. I think they're about the same. So I'm ok
with either.
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v.2 04/12] DRM/i915: Remove i965_hpd_irq_setup.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (2 preceding siblings ...)
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-25 17:06 ` 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
` (8 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
After
"Convert HPD interrupts to make use of HPD pin assignment in encoders."
This function is now basically the same as i915_hpd_irq_setup().
Consolidating both functions in one requires one more check for
I915_HAS_HOTPLUG(dev) in the i965 code path and one more check for
IS_G4X(dev) in the i915 code path. These are considered harmless.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 53 +++++++++++++-------------------------
1 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 306cd79..c6ae7ae 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2424,25 +2424,6 @@ static int i915_irq_postinstall(struct drm_device *dev)
return 0;
}
-static void i915_hpd_irq_setup(struct drm_device *dev)
-{
- if (I915_HAS_HOTPLUG(dev)) {
- drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
- struct drm_mode_config *mode_config = &dev->mode_config;
- struct intel_encoder *encoder;
- u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
-
- hotplug_en &= ~HOTPLUG_INT_EN_MASK;
- list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
- hotplug_en |= hpd_mask_i915[encoder->hpd_pin];
- hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
-
- /* Ignore TV since it's buggy */
-
- I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
- }
-}
-
static irqreturn_t i915_irq_handler(int irq, void *arg)
{
struct drm_device *dev = (struct drm_device *) arg;
@@ -2649,29 +2630,31 @@ static int i965_irq_postinstall(struct drm_device *dev)
return 0;
}
-static void i965_hpd_irq_setup(struct drm_device *dev)
+static void i915_hpd_irq_setup(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
struct drm_mode_config *mode_config = &dev->mode_config;
struct intel_encoder *encoder;
u32 hotplug_en;
- /* Note HDMI and DP share hotplug bits */
- hotplug_en = 0;
- list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
+ if (I915_HAS_HOTPLUG(dev)) {
+ hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+ hotplug_en &= ~HOTPLUG_INT_EN_MASK;
+ /* Note HDMI and DP share hotplug bits */
/* enable bits are the same for all generations */
- hotplug_en |= hpd_mask_i915[encoder->hpd_pin];
- /* Programming the CRT detection parameters tends
- to generate a spurious hotplug event about three
- seconds later. So just do it once.
- */
- if (IS_G4X(dev))
- hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
- hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
-
- /* Ignore TV since it's buggy */
+ list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
+ hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
+ /* Programming the CRT detection parameters tends
+ to generate a spurious hotplug event about three
+ seconds later. So just do it once.
+ */
+ if (IS_G4X(dev))
+ hotplug_en |= CRT_HOTPLUG_ACTIVATION_PERIOD_64;
+ hotplug_en |= CRT_HOTPLUG_VOLTAGE_COMPARE_50;
- I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
+ /* Ignore TV since it's buggy */
+ I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
+ }
}
static irqreturn_t i965_irq_handler(int irq, void *arg)
@@ -2886,7 +2869,7 @@ void intel_irq_init(struct drm_device *dev)
dev->driver->irq_postinstall = i965_irq_postinstall;
dev->driver->irq_uninstall = i965_irq_uninstall;
dev->driver->irq_handler = i965_irq_handler;
- dev_priv->display.hpd_irq_setup = i965_hpd_irq_setup;
+ dev_priv->display.hpd_irq_setup = i915_hpd_irq_setup;
}
dev->driver->enable_vblank = i915_enable_vblank;
dev->driver->disable_vblank = i915_disable_vblank;
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.2 05/12] DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (3 preceding siblings ...)
2013-02-25 17:06 ` [PATCH v.2 04/12] DRM/i915: Remove i965_hpd_irq_setup Egbert Eich
@ 2013-02-25 17:06 ` Egbert Eich
2013-03-26 21:06 ` Daniel Vetter
2013-02-25 17:06 ` [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection Egbert Eich
` (7 subsequent siblings)
12 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
Now since we have replaced the bits to show interest in hotplug IRQs
we can go and nuke the 'hotplug_supported_mask'.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 1 -
drivers/gpu/drm/i915/intel_crt.c | 2 --
drivers/gpu/drm/i915/intel_dp.c | 3 ---
drivers/gpu/drm/i915/intel_hdmi.c | 3 ---
drivers/gpu/drm/i915/intel_sdvo.c | 9 +++------
5 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 316747a..d8604a6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -922,7 +922,6 @@ typedef struct drm_i915_private {
u32 irq_mask;
u32 gt_irq_mask;
- u32 hotplug_supported_mask;
struct work_struct hotplug_work;
bool enable_hotplug_processing;
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index de43cab..5654583 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -800,8 +800,6 @@ void intel_crt_init(struct drm_device *dev)
*/
crt->force_hotplug_required = 0;
- dev_priv->hotplug_supported_mask |= CRT_HOTPLUG_INT_STATUS;
-
/*
* TODO: find a proper way to discover whether we need to set the the
* polarity and link reversal bits or not, instead of relying on the
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 549933a..5a77d40 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2839,17 +2839,14 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
name = "DPDDC-A";
break;
case PORT_B:
- dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS;
intel_encoder->hpd_pin = HPD_PORT_B;
name = "DPDDC-B";
break;
case PORT_C:
- dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS;
intel_encoder->hpd_pin = HPD_PORT_C;
name = "DPDDC-C";
break;
case PORT_D:
- dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS;
intel_encoder->hpd_pin = HPD_PORT_D;
name = "DPDDC-D";
break;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8cd6bc1..e48452a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1023,17 +1023,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
case PORT_B:
intel_hdmi->ddc_bus = GMBUS_PORT_DPB;
intel_encoder->hpd_pin = HPD_PORT_B;
- dev_priv->hotplug_supported_mask |= PORTB_HOTPLUG_INT_STATUS;
break;
case PORT_C:
intel_hdmi->ddc_bus = GMBUS_PORT_DPC;
intel_encoder->hpd_pin = HPD_PORT_C;
- dev_priv->hotplug_supported_mask |= PORTC_HOTPLUG_INT_STATUS;
break;
case PORT_D:
intel_hdmi->ddc_bus = GMBUS_PORT_DPD;
intel_encoder->hpd_pin = HPD_PORT_D;
- dev_priv->hotplug_supported_mask |= PORTD_HOTPLUG_INT_STATUS;
break;
case PORT_A:
intel_encoder->hpd_pin = HPD_PORT_A;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index cf57705..d362dd5 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2780,6 +2780,9 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
SDVOB_HOTPLUG_INT_STATUS_I915 : SDVOC_HOTPLUG_INT_STATUS_I915;
}
+ /* Only enable the hotplug irq if we need it, to work around noisy
+ * hotplug lines.
+ */
if (intel_sdvo->hotplug_active)
intel_encoder->hpd_pin = HPD_SDVO_B ? HPD_SDVO_B : HPD_SDVO_C;
@@ -2811,12 +2814,6 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
*/
intel_sdvo->base.cloneable = false;
- /* Only enable the hotplug irq if we need it, to work around noisy
- * hotplug lines.
- */
- if (intel_sdvo->hotplug_active)
- dev_priv->hotplug_supported_mask |= hotplug_mask;
-
intel_sdvo_select_ddc_bus(dev_priv, intel_sdvo, sdvo_reg);
/* Set the input timing to the screen. Assume always input 0. */
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 05/12] DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private.
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
0 siblings, 1 reply; 54+ messages in thread
From: Daniel Vetter @ 2013-03-26 21:06 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, Feb 25, 2013 at 12:06:52PM -0500, Egbert Eich wrote:
> Now since we have replaced the bits to show interest in hotplug IRQs
> we can go and nuke the 'hotplug_supported_mask'.
>
> Signed-off-by: Egbert Eich <eich@suse.de>
I've applied your patch up to this one. Patch four needed some manual
convincing to fit correctly, but the new hpd infrastructure is now in
place.
To get the actual hpd irq storm detection going I think it'd be good to
resend the remaining patches on top of latest drm-intel-nightly. I'll try
to yell at people a bit harder to review things more timely this time
around.
Thanks for the patches.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v.2 05/12] DRM/i915: Get rid if the 'hotplug_supported_mask' in struct drm_i915_private.
2013-03-26 21:06 ` Daniel Vetter
@ 2013-03-27 15:08 ` Egbert Eich
0 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-03-27 15:08 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Egbert Eich, intel-gfx, Chris Wilson, Rodrigo Vivi
Daniel Vetter writes:
> On Mon, Feb 25, 2013 at 12:06:52PM -0500, Egbert Eich wrote:
> > Now since we have replaced the bits to show interest in hotplug IRQs
> > we can go and nuke the 'hotplug_supported_mask'.
> >
> > Signed-off-by: Egbert Eich <eich@suse.de>
>
> I've applied your patch up to this one. Patch four needed some manual
> convincing to fit correctly, but the new hpd infrastructure is now in
> place.
>
> To get the actual hpd irq storm detection going I think it'd be good to
> resend the remaining patches on top of latest drm-intel-nightly. I'll try
> to yell at people a bit harder to review things more timely this time
> around.
>
> Thanks for the patches.
Thanks guys for reviewing the patches. I will try to spare some time tonight
to look into the issues that came up.
I got dragged into other tasks as well so I didn't even find the time to
follow up on the patches myself.
For now I need to go back and debug yet another issue with SDVO on GM45 :(
Cheers,
Egbert.
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (4 preceding siblings ...)
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-02-25 17:06 ` Egbert Eich
2013-02-28 0:30 ` Chris Wilson
2013-02-25 17:06 ` [PATCH v.2 07/12] DRM/i915: (re)init HPD interrupt storm statistics Egbert Eich
` (6 subsequent siblings)
12 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
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_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];
} 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 c6ae7ae..08a0934 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -587,6 +587,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;
@@ -655,13 +683,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);
}
@@ -685,10 +715,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) >>
@@ -731,10 +763,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) >>
@@ -2477,13 +2511,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);
}
@@ -2710,15 +2746,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
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection.
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
0 siblings, 1 reply; 54+ messages in thread
From: Chris Wilson @ 2013-02-28 0:30 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, Feb 25, 2013 at 12:06:53PM -0500, Egbert Eich wrote:
> 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.
Hmm, not convinced that just marking it as HPD_MARK_DISABLED does that.
It certainly prevents the irq from sending the uevent, but we have not
yet disabled the irq or enabled drm_kms_helper.poll for this connector.
Though userspace polling is unaffected.
> 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.
Looks sane.
>
> Signed-off-by: Egbert Eich <eich@suse.de>
> ---
> 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];
Please don't put it below dri1 -- as it may then be confused with our
legacy burden.
> } 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 c6ae7ae..08a0934 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -587,6 +587,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) {
if (time_is_after(dev_priv->hpd_stats[i].hpd_last_jiffies + msecs_to_jiffies(1000)))
Though it would be simpler if we just stored the adjusted
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);
> +}
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 54+ messages in thread* [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection (v2)
2013-02-28 0:30 ` Chris Wilson
@ 2013-02-28 9:19 ` Egbert Eich
2013-03-03 18:07 ` Daniel Vetter
0 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-28 9:19 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
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];
} 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
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection (v2)
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
2013-03-05 7:38 ` [PATCH v.3 06/12] DRM/i915: Add HPD IRQ storm detection (v3) Egbert Eich
0 siblings, 1 reply; 54+ messages in thread
From: Daniel Vetter @ 2013-03-03 18:07 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 54+ messages in thread* [PATCH v.3 06/12] DRM/i915: Add HPD IRQ storm detection (v3)
2013-03-03 18:07 ` Daniel Vetter
@ 2013-03-05 7:38 ` 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 7:55 ` [PATCH v.2 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v2) Egbert Eich
0 siblings, 2 replies; 54+ messages in thread
From: Egbert Eich @ 2013-03-05 7:38 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
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.
v3: Reordered drm_i915_private: moved hpd state tracking to hotplug work stuff.
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..296278f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -924,6 +924,15 @@ typedef struct drm_i915_private {
struct work_struct hotplug_work;
bool enable_hotplug_processing;
+ 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];
int num_pipe;
int num_pch_pll;
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
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on (v2).
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 ` Egbert Eich
2013-03-05 10:28 ` Ville Syrjälä
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
1 sibling, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-03-05 7:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
We disable hoptplug detection when we encounter a hotplug event
storm. Still hotplug detection is required on some outputs (like
Display Port). The interrupt storm may be only temporary (on certain
Dell Laptops for instance it happens at certain charging states of
the system). Thus we enable it after a certain grace period (2 minutes).
Should the interrupt storm persist it will be detected immediately
and it will be disabled again.
v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_irq.c | 53 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 296278f..1fb7c44 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -933,6 +933,8 @@ typedef struct drm_i915_private {
HPD_MARK_DISABLED = 2
} hpd_mark;
} hpd_stats[HPD_NUM_PINS];
+#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
+ struct timer_list hotplug_reenable_timer;
int num_pipe;
int num_pch_pll;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d11534c..c688b27 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -367,8 +367,11 @@ static void i915_hotplug_work_func(struct work_struct *work)
connector_disabled = true;
}
}
- if (connector_disabled)
+ if (connector_disabled) {
drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
+ mod_timer(&dev_priv->hotplug_reenable_timer,
+ jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
+ }
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -2244,6 +2247,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
for_each_pipe(pipe)
I915_WRITE(PIPESTAT(pipe), 0xffff);
@@ -2265,6 +2270,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
I915_WRITE(HWSTAM, 0xffffffff);
I915_WRITE(DEIMR, 0xffffffff);
@@ -2603,6 +2610,8 @@ static void i915_irq_uninstall(struct drm_device * dev)
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
int pipe;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
if (I915_HAS_HOTPLUG(dev)) {
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2851,6 +2860,8 @@ static void i965_irq_uninstall(struct drm_device * dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2866,6 +2877,44 @@ static void i965_irq_uninstall(struct drm_device * dev)
I915_WRITE(IIR, I915_READ(IIR));
}
+static void i915_reenable_hotplug_timer_func(unsigned long data)
+{
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *)data;
+ struct drm_device *dev = dev_priv->dev;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ unsigned long irqflags;
+ int i;
+
+ spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ for (i = 1; i < HPD_NUM_PINS; i++) {
+ if (dev_priv->hpd_stats[i].hpd_mark == HPD_MARK_DISABLED) {
+ struct drm_connector *connector;
+
+ dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+
+ if (intel_connector->encoder->hpd_pin == i) {
+ if (connector->polled != intel_connector->polled)
+ DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
+ drm_get_connector_name(connector));
+ connector->polled = intel_connector->polled;
+ if (!connector->polled)
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+ }
+ }
+
+ if (IS_HASWELL(dev) ||
+ IS_IVYBRIDGE(dev) ||
+ (HAS_PCH_SPLIT(dev)))
+ ibx_hpd_irq_setup(dev);
+ else
+ i915_hpd_irq_setup(dev);
+ }
+ }
+ spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
void intel_irq_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2878,6 +2927,8 @@ void intel_irq_init(struct drm_device *dev)
setup_timer(&dev_priv->gpu_error.hangcheck_timer,
i915_hangcheck_elapsed,
(unsigned long) dev);
+ setup_timer(&dev_priv->hotplug_reenable_timer, i915_reenable_hotplug_timer_func,
+ (unsigned long) dev_priv);
pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on (v2).
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
0 siblings, 1 reply; 54+ messages in thread
From: Ville Syrjälä @ 2013-03-05 10:28 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Tue, Mar 05, 2013 at 02:48:49AM -0500, Egbert Eich wrote:
> We disable hoptplug detection when we encounter a hotplug event
> storm. Still hotplug detection is required on some outputs (like
> Display Port). The interrupt storm may be only temporary (on certain
> Dell Laptops for instance it happens at certain charging states of
> the system). Thus we enable it after a certain grace period (2 minutes).
> Should the interrupt storm persist it will be detected immediately
> and it will be disabled again.
>
> v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker.
>
> Signed-off-by: Egbert Eich <eich@suse.de>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_irq.c | 53 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 54 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 296278f..1fb7c44 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -933,6 +933,8 @@ typedef struct drm_i915_private {
> HPD_MARK_DISABLED = 2
> } hpd_mark;
> } hpd_stats[HPD_NUM_PINS];
> +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
> + struct timer_list hotplug_reenable_timer;
>
> int num_pipe;
> int num_pch_pll;
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index d11534c..c688b27 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -367,8 +367,11 @@ static void i915_hotplug_work_func(struct work_struct *work)
> connector_disabled = true;
> }
> }
> - if (connector_disabled)
> + if (connector_disabled) {
> drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
> + mod_timer(&dev_priv->hotplug_reenable_timer,
> + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
> + }
>
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> @@ -2244,6 +2247,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> for_each_pipe(pipe)
> I915_WRITE(PIPESTAT(pipe), 0xffff);
>
> @@ -2265,6 +2270,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> I915_WRITE(HWSTAM, 0xffffffff);
>
> I915_WRITE(DEIMR, 0xffffffff);
> @@ -2603,6 +2610,8 @@ static void i915_irq_uninstall(struct drm_device * dev)
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> int pipe;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> if (I915_HAS_HOTPLUG(dev)) {
> I915_WRITE(PORT_HOTPLUG_EN, 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
> @@ -2851,6 +2860,8 @@ static void i965_irq_uninstall(struct drm_device * dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> I915_WRITE(PORT_HOTPLUG_EN, 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>
> @@ -2866,6 +2877,44 @@ static void i965_irq_uninstall(struct drm_device * dev)
> I915_WRITE(IIR, I915_READ(IIR));
> }
>
> +static void i915_reenable_hotplug_timer_func(unsigned long data)
> +{
> + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data;
> + struct drm_device *dev = dev_priv->dev;
> + struct drm_mode_config *mode_config = &dev->mode_config;
> + unsigned long irqflags;
> + int i;
> +
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + for (i = 1; i < HPD_NUM_PINS; i++) {
Maybe include a small comment to avoid people thinking the 'i = 1' is
a typo.
> + if (dev_priv->hpd_stats[i].hpd_mark == HPD_MARK_DISABLED) {
This is getting quite deep. This could help a bit:
if (dev_priv->hpd_stats[i].hpd_mark != HPD_MARK_DISABLED)
continue;
> + struct drm_connector *connector;
> +
> + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
> + list_for_each_entry(connector, &mode_config->connector_list, head) {
> + struct intel_connector *intel_connector = to_intel_connector(connector);
> +
> + if (intel_connector->encoder->hpd_pin == i) {
> + if (connector->polled != intel_connector->polled)
> + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
> + drm_get_connector_name(connector));
> + connector->polled = intel_connector->polled;
> + if (!connector->polled)
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> + }
> + }
> +
> + if (IS_HASWELL(dev) ||
> + IS_IVYBRIDGE(dev) ||
> + (HAS_PCH_SPLIT(dev)))
IS_HASWELL() and IS_IVYBRIDGE() are superfluous.
> + ibx_hpd_irq_setup(dev);
> + else
> + i915_hpd_irq_setup(dev);
> + }
> + }
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> +}
> +
> void intel_irq_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2878,6 +2927,8 @@ void intel_irq_init(struct drm_device *dev)
> setup_timer(&dev_priv->gpu_error.hangcheck_timer,
> i915_hangcheck_elapsed,
> (unsigned long) dev);
> + setup_timer(&dev_priv->hotplug_reenable_timer, i915_reenable_hotplug_timer_func,
> + (unsigned long) dev_priv);
>
> pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
>
> --
> 1.7.7
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 54+ messages in thread* [PATCH v.3 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on (v3).
2013-03-05 10:28 ` Ville Syrjälä
@ 2013-03-05 12:26 ` Egbert Eich
0 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-03-05 12:26 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
We disable hoptplug detection when we encounter a hotplug event
storm. Still hotplug detection is required on some outputs (like
Display Port). The interrupt storm may be only temporary (on certain
Dell Laptops for instance it happens at certain charging states of
the system). Thus we enable it after a certain grace period (2 minutes).
Should the interrupt storm persist it will be detected immediately
and it will be disabled again.
v2: Reordered drm_i915_private: moved hotplug_reenable_timer to hpd state tracker.
v3: Clarified loop start value,
Removed superfluous test for Ivybridge and Haswell,
Restructured loop to avoid deep nesting (all suggested by Ville Syrjälä)
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_irq.c | 53 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 296278f..1fb7c44 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -933,6 +933,8 @@ typedef struct drm_i915_private {
HPD_MARK_DISABLED = 2
} hpd_mark;
} hpd_stats[HPD_NUM_PINS];
+#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
+ struct timer_list hotplug_reenable_timer;
int num_pipe;
int num_pch_pll;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d11534c..76903af 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -367,8 +367,11 @@ static void i915_hotplug_work_func(struct work_struct *work)
connector_disabled = true;
}
}
- if (connector_disabled)
+ if (connector_disabled) {
drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
+ mod_timer(&dev_priv->hotplug_reenable_timer,
+ jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
+ }
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -2244,6 +2247,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
for_each_pipe(pipe)
I915_WRITE(PIPESTAT(pipe), 0xffff);
@@ -2265,6 +2270,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
I915_WRITE(HWSTAM, 0xffffffff);
I915_WRITE(DEIMR, 0xffffffff);
@@ -2603,6 +2610,8 @@ static void i915_irq_uninstall(struct drm_device * dev)
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
int pipe;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
if (I915_HAS_HOTPLUG(dev)) {
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2851,6 +2860,8 @@ static void i965_irq_uninstall(struct drm_device * dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2866,6 +2877,44 @@ static void i965_irq_uninstall(struct drm_device * dev)
I915_WRITE(IIR, I915_READ(IIR));
}
+static void i915_reenable_hotplug_timer_func(unsigned long data)
+{
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *)data;
+ struct drm_device *dev = dev_priv->dev;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ unsigned long irqflags;
+ int i;
+
+ spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ for (i = (HPD_NONE + 1); i < HPD_NUM_PINS; i++) {
+ struct drm_connector *connector;
+
+ if (dev_priv->hpd_stats[i].hpd_mark != HPD_MARK_DISABLED)
+ continue;
+
+ dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
+
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+
+ if (intel_connector->encoder->hpd_pin == i) {
+ if (connector->polled != intel_connector->polled)
+ DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
+ drm_get_connector_name(connector));
+ connector->polled = intel_connector->polled;
+ if (!connector->polled)
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+ }
+ }
+
+ if (HAS_PCH_SPLIT(dev))
+ ibx_hpd_irq_setup(dev);
+ else
+ i915_hpd_irq_setup(dev);
+ }
+ spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
void intel_irq_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2878,6 +2927,8 @@ void intel_irq_init(struct drm_device *dev)
setup_timer(&dev_priv->gpu_error.hangcheck_timer,
i915_hangcheck_elapsed,
(unsigned long) dev);
+ setup_timer(&dev_priv->hotplug_reenable_timer, i915_reenable_hotplug_timer_func,
+ (unsigned long) dev_priv);
pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
--
1.7.7
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v.2 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v2)
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 7:55 ` 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
1 sibling, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-03-05 7:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
This way it is possible to limit 're'-detect() of displays to connectors
which have received an HPD event.
v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++
2 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1fb7c44..d48d1e6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -933,6 +933,7 @@ typedef struct drm_i915_private {
HPD_MARK_DISABLED = 2
} hpd_mark;
} hpd_stats[HPD_NUM_PINS];
+ u32 hpd_event_bits;
#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
struct timer_list hotplug_reenable_timer;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index c688b27..223813f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -344,6 +344,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
struct drm_connector *connector;
unsigned long irqflags;
bool connector_disabled = false;
+ u32 hpd_event_bits;
/* HPD irq before everything is fully set up. */
if (!dev_priv->enable_hotplug_processing)
@@ -353,6 +354,9 @@ static void i915_hotplug_work_func(struct work_struct *work)
DRM_DEBUG_KMS("running encoder hotplug functions\n");
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+ hpd_event_bits = dev_priv->hpd_event_bits;
+ dev_priv->hpd_event_bits = 0;
list_for_each_entry(connector, &mode_config->connector_list, head) {
intel_connector = to_intel_connector(connector);
intel_encoder = intel_connector->encoder;
@@ -366,6 +370,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
| DRM_CONNECTOR_POLL_DISCONNECT;
connector_disabled = true;
}
+ if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
+ DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
+ drm_get_connector_name(connector), intel_encoder->hpd_pin);
+ }
}
if (connector_disabled) {
drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
@@ -620,12 +628,14 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev,
for (i = 1; i < HPD_NUM_PINS; i++) {
if ((hpd[i] & hotplug_trigger) &&
dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) {
+ dev_priv->hpd_event_bits |= (1 << i);
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;
+ dev_priv->hpd_event_bits &= ~(1 << i);
DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i);
ret = true;
} else
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.3 11/12] DRM/i915: Add bit field to record which pins have received HPD events (v3)
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 ` Egbert Eich
2013-03-05 14:52 ` Egbert Eich
0 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-03-05 13:00 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
This way it is possible to limit 're'-detect() of displays to connectors
which have received an HPD event.
v2: Reordered drm_i915_private: Move hpd_event_bits to hpd state tracking.
v3: Fix patch.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 47 ++++++++++++++++++++++++++++++++++-----
2 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1fb7c44..d48d1e6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -933,6 +933,7 @@ typedef struct drm_i915_private {
HPD_MARK_DISABLED = 2
} hpd_mark;
} hpd_stats[HPD_NUM_PINS];
+ u32 hpd_event_bits;
#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
struct timer_list hotplug_reenable_timer;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index fd68b21..9e8d5b4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -330,6 +330,24 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
crtc);
}
+/**
+ * drm_helper_hpd_irq_single_connector_event() - call this function with mode_config.mutex lock held
+ */
+
+static int intel_hpd_irq_single_connector_event(struct drm_device *dev, struct drm_connector *connector)
+{
+ enum drm_connector_status old_status;
+
+ old_status = connector->status;
+
+ connector->status = connector->funcs->detect(connector, false);
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
+ connector->base.id,
+ drm_get_connector_name(connector),
+ old_status, connector->status);
+ return (old_status != connector->status);
+}
+
/*
* Handle hotplug events outside the interrupt handler proper.
*/
@@ -344,6 +362,8 @@ static void i915_hotplug_work_func(struct work_struct *work)
struct drm_connector *connector;
unsigned long irqflags;
bool connector_disabled = false;
+ bool changed = false;
+ u32 hpd_event_bits;
/* HPD irq before everything is fully set up. */
if (!dev_priv->enable_hotplug_processing)
@@ -353,6 +373,9 @@ static void i915_hotplug_work_func(struct work_struct *work)
DRM_DEBUG_KMS("running encoder hotplug functions\n");
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+ hpd_event_bits = dev_priv->hpd_event_bits;
+ dev_priv->hpd_event_bits = 0;
list_for_each_entry(connector, &mode_config->connector_list, head) {
intel_connector = to_intel_connector(connector);
intel_encoder = intel_connector->encoder;
@@ -366,6 +389,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
| DRM_CONNECTOR_POLL_DISCONNECT;
connector_disabled = true;
}
+ if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
+ DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
+ drm_get_connector_name(connector), intel_encoder->hpd_pin);
+ }
}
if (connector_disabled) {
drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
@@ -375,14 +402,20 @@ static void i915_hotplug_work_func(struct work_struct *work)
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
- list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- if (intel_encoder->hot_plug)
- intel_encoder->hot_plug(intel_encoder);
-
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ intel_connector = to_intel_connector(connector);
+ intel_encoder = intel_connector->encoder;
+ if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
+ if (intel_encoder->hot_plug)
+ intel_encoder->hot_plug(intel_encoder);
+ if (intel_hpd_irq_single_connector_event(dev, connector))
+ changed = true;
+ }
+ }
mutex_unlock(&mode_config->mutex);
- /* Just fire off a uevent and let userspace tell us what to do */
- drm_helper_hpd_irq_event(dev);
+ if (changed)
+ drm_kms_helper_hotplug_event(dev);
}
static void ironlake_handle_rps_change(struct drm_device *dev)
@@ -620,12 +653,14 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev,
for (i = 1; i < HPD_NUM_PINS; i++) {
if ((hpd[i] & hotplug_trigger) &&
dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) {
+ dev_priv->hpd_event_bits |= (1 << i);
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;
+ dev_priv->hpd_event_bits &= ~(1 << i);
DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i);
ret = true;
} else
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v.2 07/12] DRM/i915: (re)init HPD interrupt storm statistics.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (5 preceding siblings ...)
2013-02-25 17:06 ` [PATCH v.2 06/12] DRM/i915: Add HPD IRQ storm detection Egbert Eich
@ 2013-02-25 17:06 ` 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
` (5 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
When an encoder is shared on several connectors there is only
one hotplug line, thus this line needs to be shared among these
connectors.
If HPD detect only works reliably on a subset of those connectors,
we want to poll the others. Thus we need to make sure that storm
detection doesn't mess up the settings for those connectors.
Therefore we store the settings in the intel_connector struct and
restore them from there.
If nothing is set but the encoder has a hpd_pin set we assume this
connector is hotplug capable.
On init/reset we make sure the polled state of the connectors
is (re)set to the default value, the HPD interrupts are marked
enabled.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 13 +++++++++++++
drivers/gpu/drm/i915/intel_crt.c | 6 ++----
drivers/gpu/drm/i915/intel_dp.c | 1 -
drivers/gpu/drm/i915/intel_drv.h | 4 ++++
drivers/gpu/drm/i915/intel_hdmi.c | 1 -
drivers/gpu/drm/i915/intel_sdvo.c | 5 ++---
drivers/gpu/drm/i915/intel_tv.c | 2 +-
7 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 08a0934..d1ccff7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2918,7 +2918,20 @@ void intel_irq_init(struct drm_device *dev)
void intel_hpd_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ struct drm_connector *connector;
+ int i;
+ for (i = 1; i < HPD_NUM_PINS; i++) {
+ dev_priv->hpd_stats[i].hpd_cnt = 0;
+ dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
+ }
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+ connector->polled = intel_connector->polled;
+ if (!connector->polled && I915_HAS_HOTPLUG(dev) && intel_connector->encoder->hpd_pin > HPD_NONE)
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+ }
if (dev_priv->display.hpd_irq_setup)
dev_priv->display.hpd_irq_setup(dev);
}
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 5654583..4318f3e 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -790,10 +790,8 @@ void intel_crt_init(struct drm_device *dev)
drm_sysfs_connector_add(connector);
- if (I915_HAS_HOTPLUG(dev))
- connector->polled = DRM_CONNECTOR_POLL_HPD;
- else
- connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+ if (!I915_HAS_HOTPLUG(dev))
+ intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT;
/*
* Configure the automatic hotplug detection stuff
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5a77d40..66134d1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2816,7 +2816,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
drm_connector_init(dev, connector, &intel_dp_connector_funcs, type);
drm_connector_helper_add(connector, &intel_dp_connector_helper_funcs);
- connector->polled = DRM_CONNECTOR_POLL_HPD;
connector->interlace_allowed = true;
connector->doublescan_allowed = 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c9003bd..142f40a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -198,6 +198,10 @@ struct intel_connector {
/* Cached EDID for eDP and LVDS. May hold ERR_PTR for invalid EDID. */
struct edid *edid;
+
+ /* since POLL and HPD connectors may use the same HPD line keep the native
+ state of connector->polled in case hotplug storm detection changes it */
+ u8 polled;
};
struct intel_crtc {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e48452a..82c9b9b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1015,7 +1015,6 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
DRM_MODE_CONNECTOR_HDMIA);
drm_connector_helper_add(connector, &intel_hdmi_connector_helper_funcs);
- connector->polled = DRM_CONNECTOR_POLL_HPD;
connector->interlace_allowed = 1;
connector->doublescan_allowed = 0;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index d362dd5..30635c6 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2270,7 +2270,6 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
connector = &intel_connector->base;
if (intel_sdvo_get_hotplug_support(intel_sdvo) &
intel_sdvo_connector->output_flag) {
- connector->polled = DRM_CONNECTOR_POLL_HPD;
intel_sdvo->hotplug_active |= intel_sdvo_connector->output_flag;
/* Some SDVO devices have one-shot hotplug interrupts.
* Ensure that they get re-enabled when an interrupt happens.
@@ -2278,7 +2277,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
intel_sdvo_enable_hotplug(intel_encoder);
} else {
- connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
+ intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
}
encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
connector->connector_type = DRM_MODE_CONNECTOR_DVID;
@@ -2347,7 +2346,7 @@ intel_sdvo_analog_init(struct intel_sdvo *intel_sdvo, int device)
intel_connector = &intel_sdvo_connector->base;
connector = &intel_connector->base;
- connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+ intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT;
encoder->encoder_type = DRM_MODE_ENCODER_DAC;
connector->connector_type = DRM_MODE_CONNECTOR_VGA;
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index 984a113..a5a3fc0 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1613,7 +1613,7 @@ intel_tv_init(struct drm_device *dev)
*
* More recent chipsets favour HDMI rather than integrated S-Video.
*/
- connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+ intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT;
drm_connector_init(dev, connector, &intel_tv_connector_funcs,
DRM_MODE_CONNECTOR_SVIDEO);
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.2 08/12] DRM/i915: Treat hpd_irq_setup() for ironake and older generations the same way.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (6 preceding siblings ...)
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 ` 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
` (4 subsequent siblings)
12 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
When switching to enabling HPD IRQs only for lines where needed and supported
this will ensure that the right lines will be enabled on all generations when
intel_hpd_init() is called.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d1ccff7..e0cf629 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2033,7 +2033,7 @@ static void ibx_enable_hotplug(struct drm_device *dev)
I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
}
-static void ibx_irq_postinstall(struct drm_device *dev)
+static void ibx_hpd_irq_setup(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
struct drm_mode_config *mode_config = &dev->mode_config;
@@ -2055,8 +2055,6 @@ static void ibx_irq_postinstall(struct drm_device *dev)
I915_WRITE(SDEIMR, ~mask);
I915_WRITE(SDEIER, mask);
POSTING_READ(SDEIER);
-
- ibx_enable_hotplug(dev);
}
static int ironlake_irq_postinstall(struct drm_device *dev)
@@ -2094,7 +2092,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
I915_WRITE(GTIER, render_irqs);
POSTING_READ(GTIER);
- ibx_irq_postinstall(dev);
+ ibx_enable_hotplug(dev);
if (IS_IRONLAKE_M(dev)) {
/* Clear & enable PCU event interrupts */
@@ -2140,7 +2138,7 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
I915_WRITE(GTIER, render_irqs);
POSTING_READ(GTIER);
- ibx_irq_postinstall(dev);
+ ibx_enable_hotplug(dev);
return 0;
}
@@ -2884,6 +2882,7 @@ void intel_irq_init(struct drm_device *dev)
dev->driver->irq_uninstall = ironlake_irq_uninstall;
dev->driver->enable_vblank = ivybridge_enable_vblank;
dev->driver->disable_vblank = ivybridge_disable_vblank;
+ dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
} else if (HAS_PCH_SPLIT(dev)) {
dev->driver->irq_handler = ironlake_irq_handler;
dev->driver->irq_preinstall = ironlake_irq_preinstall;
@@ -2891,6 +2890,7 @@ void intel_irq_init(struct drm_device *dev)
dev->driver->irq_uninstall = ironlake_irq_uninstall;
dev->driver->enable_vblank = ironlake_enable_vblank;
dev->driver->disable_vblank = ironlake_disable_vblank;
+ dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
} else {
if (INTEL_INFO(dev)->gen == 2) {
dev->driver->irq_preinstall = i8xx_irq_preinstall;
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.2 09/12] DRM/i915: Disable HPD interrupt on pin when irq storm is detected.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (7 preceding siblings ...)
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 ` 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
` (3 subsequent siblings)
12 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
This patch disables hotplug interrupts if an 'interrupt storm'
has been detected.
Noise on the interrupt line renders the hotplug interrupt useless:
each hotplug event causes the devices to be rescanned which will
will only increase the system load.
Thus disable the hotplug interrupts and fall back to periodic
device polling.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++++++++++++++++++++---------
1 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index e0cf629..8878fdd 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -96,7 +96,8 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
PORTD_HOTPLUG_INT_STATUS /* HPD_PORT_D */
};
-
+static void ibx_hpd_irq_setup(struct drm_device *dev);
+static void i915_hpd_irq_setup(struct drm_device *dev);
/* For display hotplug interrupt */
static void
@@ -347,7 +348,11 @@ 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 intel_encoder *encoder;
+ struct intel_connector *intel_connector;
+ struct intel_encoder *intel_encoder;
+ struct drm_connector *connector;
+ unsigned long irqflags;
+ bool connector_disabled = false;
/* HPD irq before everything is fully set up. */
if (!dev_priv->enable_hotplug_processing)
@@ -356,9 +361,29 @@ static void i915_hotplug_work_func(struct work_struct *work)
mutex_lock(&mode_config->mutex);
DRM_DEBUG_KMS("running encoder hotplug functions\n");
- list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
- if (encoder->hot_plug)
- encoder->hot_plug(encoder);
+ spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ intel_connector = to_intel_connector(connector);
+ intel_encoder = intel_connector->encoder;
+ if (intel_encoder->hpd_pin > HPD_NONE &&
+ dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED &&
+ connector->polled == DRM_CONNECTOR_POLL_HPD) {
+ pr_warn("HPD interrupt storm detected on connector %s disabling\n",
+ drm_get_connector_name(connector));
+ dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED;
+ connector->polled = DRM_CONNECTOR_POLL_CONNECT
+ | DRM_CONNECTOR_POLL_DISCONNECT;
+ connector_disabled = true;
+ }
+ }
+ if (connector_disabled)
+ drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
+
+ spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+ list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
+ if (intel_encoder->hot_plug)
+ intel_encoder->hot_plug(encoder);
mutex_unlock(&mode_config->mutex);
@@ -587,13 +612,14 @@ 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,
+static inline bool 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;
+ bool ret = false;
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
@@ -607,12 +633,15 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev,
} 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);
+ ret = true;
} else
dev_priv->hpd_stats[i].hpd_cnt++;
}
}
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+ return ret;
}
static void gmbus_irq_handler(struct drm_device *dev)
@@ -688,7 +717,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915);
+ if( hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915))
+ i915_hpd_irq_setup(dev);
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
}
@@ -718,7 +748,8 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK;
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_ibx);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_ibx))
+ ibx_hpd_irq_setup(dev);
queue_work(dev_priv->wq, &dev_priv->hotplug_work);
}
if (pch_iir & SDE_AUDIO_POWER_MASK)
@@ -766,7 +797,8 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_cpt);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_cpt))
+ ibx_hpd_irq_setup(dev);
queue_work(dev_priv->wq, &dev_priv->hotplug_work);
}
if (pch_iir & SDE_AUDIO_POWER_MASK_CPT)
@@ -2043,12 +2075,14 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
if (HAS_PCH_IBX(dev)) {
mask &= ~SDE_HOTPLUG_MASK;
list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- mask |= hpd_ibx[intel_encoder->hpd_pin];
+ if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+ mask |= hpd_ibx[intel_encoder->hpd_pin];
mask |= SDE_GMBUS | SDE_AUX_MASK;
} else {
mask &= ~SDE_HOTPLUG_MASK_CPT;
list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- mask |= hpd_cpt[intel_encoder->hpd_pin];
+ if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+ mask |= hpd_cpt[intel_encoder->hpd_pin];
mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
}
I915_WRITE(SDEIIR, I915_READ(SDEIIR));
@@ -2514,7 +2548,8 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915))
+ i915_hpd_irq_setup(dev);
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
}
@@ -2668,7 +2703,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
struct drm_mode_config *mode_config = &dev->mode_config;
- struct intel_encoder *encoder;
+ struct intel_encoder *intel_encoder;
u32 hotplug_en;
if (I915_HAS_HOTPLUG(dev)) {
@@ -2677,7 +2712,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
/* Note HDMI and DP share hotplug bits */
/* enable bits are the same for all generations */
list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
+ if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+ hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
/* Programming the CRT detection parameters tends
to generate a spurious hotplug event about three
seconds later. So just do it once.
@@ -2751,8 +2787,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger,
- IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i965);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger,
+ IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i965))
+ i915_hpd_irq_setup(dev);
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
}
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.2 09/12] DRM/i915: Disable HPD interrupt on pin when irq storm is detected (v2)
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 ` Egbert Eich
0 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-03-05 12:34 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
This patch disables hotplug interrupts if an 'interrupt storm'
has been detected.
Noise on the interrupt line renders the hotplug interrupt useless:
each hotplug event causes the devices to be rescanned which will
will only increase the system load.
Thus disable the hotplug interrupts and fall back to periodic
device polling.
v2: Fixed cleanup typo.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 69 ++++++++++++++++++++++++++++++---------
1 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d9c99d7..0abef6a 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -87,7 +87,8 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */
[HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
};
-
+static void ibx_hpd_irq_setup(struct drm_device *dev);
+static void i915_hpd_irq_setup(struct drm_device *dev);
/* For display hotplug interrupt */
static void
@@ -338,7 +339,11 @@ 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 intel_encoder *encoder;
+ struct intel_connector *intel_connector;
+ struct intel_encoder *intel_encoder;
+ struct drm_connector *connector;
+ unsigned long irqflags;
+ bool connector_disabled = false;
/* HPD irq before everything is fully set up. */
if (!dev_priv->enable_hotplug_processing)
@@ -347,9 +352,29 @@ static void i915_hotplug_work_func(struct work_struct *work)
mutex_lock(&mode_config->mutex);
DRM_DEBUG_KMS("running encoder hotplug functions\n");
- list_for_each_entry(encoder, &mode_config->encoder_list, base.head)
- if (encoder->hot_plug)
- encoder->hot_plug(encoder);
+ spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ intel_connector = to_intel_connector(connector);
+ intel_encoder = intel_connector->encoder;
+ if (intel_encoder->hpd_pin > HPD_NONE &&
+ dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_MARK_DISABLED &&
+ connector->polled == DRM_CONNECTOR_POLL_HPD) {
+ pr_warn("HPD interrupt storm detected on connector %s disabling\n",
+ drm_get_connector_name(connector));
+ dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark = HPD_DISABLED;
+ connector->polled = DRM_CONNECTOR_POLL_CONNECT
+ | DRM_CONNECTOR_POLL_DISCONNECT;
+ connector_disabled = true;
+ }
+ }
+ if (connector_disabled)
+ drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
+
+ spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+ list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
+ if (intel_encoder->hot_plug)
+ intel_encoder->hot_plug(intel_encoder);
mutex_unlock(&mode_config->mutex);
@@ -578,13 +603,14 @@ 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,
+static inline bool 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;
+ bool ret = false;
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
@@ -598,12 +624,15 @@ static inline void hotplug_irq_storm_detect(struct drm_device *dev,
} 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);
+ ret = true;
} else
dev_priv->hpd_stats[i].hpd_cnt++;
}
}
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+
+ return ret;
}
static void gmbus_irq_handler(struct drm_device *dev)
@@ -679,7 +708,8 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915);
+ if( hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915))
+ i915_hpd_irq_setup(dev);
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
}
@@ -709,7 +739,8 @@ static void ibx_irq_handler(struct drm_device *dev, u32 pch_iir)
u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK;
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_ibx);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_ibx))
+ ibx_hpd_irq_setup(dev);
queue_work(dev_priv->wq, &dev_priv->hotplug_work);
}
if (pch_iir & SDE_AUDIO_POWER_MASK)
@@ -757,7 +788,8 @@ static void cpt_irq_handler(struct drm_device *dev, u32 pch_iir)
u32 hotplug_trigger = pch_iir & SDE_HOTPLUG_MASK_CPT;
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_cpt);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_cpt))
+ ibx_hpd_irq_setup(dev);
queue_work(dev_priv->wq, &dev_priv->hotplug_work);
}
if (pch_iir & SDE_AUDIO_POWER_MASK_CPT)
@@ -2034,12 +2066,14 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
if (HAS_PCH_IBX(dev)) {
mask &= ~SDE_HOTPLUG_MASK;
list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- mask |= hpd_ibx[intel_encoder->hpd_pin];
+ if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+ mask |= hpd_ibx[intel_encoder->hpd_pin];
mask |= SDE_GMBUS | SDE_AUX_MASK;
} else {
mask &= ~SDE_HOTPLUG_MASK_CPT;
list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- mask |= hpd_cpt[intel_encoder->hpd_pin];
+ if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+ mask |= hpd_cpt[intel_encoder->hpd_pin];
mask |= SDE_GMBUS_CPT | SDE_AUX_MASK_CPT;
}
I915_WRITE(SDEIIR, I915_READ(SDEIIR));
@@ -2505,7 +2539,8 @@ static irqreturn_t i915_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger, hpd_status_i915))
+ i915_hpd_irq_setup(dev);
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
}
@@ -2659,7 +2694,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
{
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
struct drm_mode_config *mode_config = &dev->mode_config;
- struct intel_encoder *encoder;
+ struct intel_encoder *intel_encoder;
u32 hotplug_en;
if (I915_HAS_HOTPLUG(dev)) {
@@ -2668,7 +2703,8 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
/* Note HDMI and DP share hotplug bits */
/* enable bits are the same for all generations */
list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
+ if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark == HPD_ENABLED)
+ hotplug_en |= hpd_mask_i915[intel_encoder->hpd_pin];
/* Programming the CRT detection parameters tends
to generate a spurious hotplug event about three
seconds later. So just do it once.
@@ -2742,8 +2778,9 @@ static irqreturn_t i965_irq_handler(int irq, void *arg)
DRM_DEBUG_DRIVER("hotplug event received, stat 0x%08x\n",
hotplug_status);
if (hotplug_trigger) {
- hotplug_irq_storm_detect(dev, hotplug_trigger,
- IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i965);
+ if (hotplug_irq_storm_detect(dev, hotplug_trigger,
+ IS_G4X(dev) ? hpd_status_gen4 : hpd_status_i965))
+ i915_hpd_irq_setup(dev);
queue_work(dev_priv->wq,
&dev_priv->hotplug_work);
}
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread
* [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (8 preceding siblings ...)
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-02-25 17:06 ` 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
` (2 subsequent siblings)
12 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
We disable hoptplug detection when we encounter a hotplug event
storm. Still hotplug detection is required on some outputs (like
Display Port). The interrupt storm may be only temporary (on certain
Dell Laptops for instance it happens at certain charging states of
the system). Thus we enable it after a certain grace period (2 minutes).
Should the interrupt storm persist it will be detected immediately
and it will be disabled again.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +
drivers/gpu/drm/i915/i915_irq.c | 53 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 54 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6ca742d..58bee7a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1068,6 +1068,8 @@ typedef struct drm_i915_private {
HPD_MARK_DISABLED = 2
} hpd_mark;
} hpd_stats[HPD_NUM_PINS];
+#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
+ struct timer_list hotplug_reenable_timer;
} 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 8878fdd..ba598e3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -376,8 +376,11 @@ static void i915_hotplug_work_func(struct work_struct *work)
connector_disabled = true;
}
}
- if (connector_disabled)
+ if (connector_disabled) {
drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
+ mod_timer(&dev_priv->hotplug_reenable_timer,
+ jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
+ }
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
@@ -2253,6 +2256,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
for_each_pipe(pipe)
I915_WRITE(PIPESTAT(pipe), 0xffff);
@@ -2274,6 +2279,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
I915_WRITE(HWSTAM, 0xffffffff);
I915_WRITE(DEIMR, 0xffffffff);
@@ -2612,6 +2619,8 @@ static void i915_irq_uninstall(struct drm_device * dev)
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
int pipe;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
if (I915_HAS_HOTPLUG(dev)) {
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2860,6 +2869,8 @@ static void i965_irq_uninstall(struct drm_device * dev)
if (!dev_priv)
return;
+ del_timer_sync(&dev_priv->hotplug_reenable_timer);
+
I915_WRITE(PORT_HOTPLUG_EN, 0);
I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
@@ -2875,6 +2886,44 @@ static void i965_irq_uninstall(struct drm_device * dev)
I915_WRITE(IIR, I915_READ(IIR));
}
+static void i915_reenable_hotplug_timer_func(unsigned long data)
+{
+ drm_i915_private_t *dev_priv = (drm_i915_private_t *)data;
+ struct drm_device *dev = dev_priv->dev;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ unsigned long irqflags;
+ int i;
+
+ spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+ for (i = 1; i < HPD_NUM_PINS; i++) {
+ if (dev_priv->hpd_stats[i].hpd_mark == HPD_MARK_DISABLED) {
+ struct drm_connector *connector;
+
+ dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ struct intel_connector *intel_connector = to_intel_connector(connector);
+
+ if (intel_connector->encoder->hpd_pin == i) {
+ if (connector->polled != intel_connector->polled)
+ DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
+ drm_get_connector_name(connector));
+ connector->polled = intel_connector->polled;
+ if (!connector->polled)
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+ }
+ }
+
+ if (IS_HASWELL(dev) ||
+ IS_IVYBRIDGE(dev) ||
+ (HAS_PCH_SPLIT(dev)))
+ ibx_hpd_irq_setup(dev);
+ else
+ i915_hpd_irq_setup(dev);
+ }
+ }
+ spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
+}
+
void intel_irq_init(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -2887,6 +2936,8 @@ void intel_irq_init(struct drm_device *dev)
setup_timer(&dev_priv->gpu_error.hangcheck_timer,
i915_hangcheck_elapsed,
(unsigned long) dev);
+ setup_timer(&dev_priv->hotplug_reenable_timer, i915_reenable_hotplug_timer_func,
+ (unsigned long) dev_priv);
pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on.
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
0 siblings, 0 replies; 54+ messages in thread
From: Daniel Vetter @ 2013-03-27 15:12 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, Feb 25, 2013 at 12:06:57PM -0500, Egbert Eich wrote:
> We disable hoptplug detection when we encounter a hotplug event
> storm. Still hotplug detection is required on some outputs (like
> Display Port). The interrupt storm may be only temporary (on certain
> Dell Laptops for instance it happens at certain charging states of
> the system). Thus we enable it after a certain grace period (2 minutes).
> Should the interrupt storm persist it will be detected immediately
> and it will be disabled again.
>
> Signed-off-by: Egbert Eich <eich@suse.de>
Oops, just noticed that you have an ibx_hpd_irq_setup patch, too ;-) I
think I'll go with since that one's just gotten a bit a beating in review
from Ville. Hopfully that's not too ugly to rebase over ...
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +
> drivers/gpu/drm/i915/i915_irq.c | 53 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 54 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6ca742d..58bee7a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1068,6 +1068,8 @@ typedef struct drm_i915_private {
> HPD_MARK_DISABLED = 2
> } hpd_mark;
> } hpd_stats[HPD_NUM_PINS];
> +#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
> + struct timer_list hotplug_reenable_timer;
> } 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 8878fdd..ba598e3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -376,8 +376,11 @@ static void i915_hotplug_work_func(struct work_struct *work)
> connector_disabled = true;
> }
> }
> - if (connector_disabled)
> + if (connector_disabled) {
> drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
> + mod_timer(&dev_priv->hotplug_reenable_timer,
> + jiffies + msecs_to_jiffies(I915_REENABLE_HOTPLUG_DELAY));
> + }
>
> spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>
> @@ -2253,6 +2256,8 @@ static void valleyview_irq_uninstall(struct drm_device *dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> for_each_pipe(pipe)
> I915_WRITE(PIPESTAT(pipe), 0xffff);
>
> @@ -2274,6 +2279,8 @@ static void ironlake_irq_uninstall(struct drm_device *dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> I915_WRITE(HWSTAM, 0xffffffff);
>
> I915_WRITE(DEIMR, 0xffffffff);
> @@ -2612,6 +2619,8 @@ static void i915_irq_uninstall(struct drm_device * dev)
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
> int pipe;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> if (I915_HAS_HOTPLUG(dev)) {
> I915_WRITE(PORT_HOTPLUG_EN, 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
> @@ -2860,6 +2869,8 @@ static void i965_irq_uninstall(struct drm_device * dev)
> if (!dev_priv)
> return;
>
> + del_timer_sync(&dev_priv->hotplug_reenable_timer);
> +
> I915_WRITE(PORT_HOTPLUG_EN, 0);
> I915_WRITE(PORT_HOTPLUG_STAT, I915_READ(PORT_HOTPLUG_STAT));
>
> @@ -2875,6 +2886,44 @@ static void i965_irq_uninstall(struct drm_device * dev)
> I915_WRITE(IIR, I915_READ(IIR));
> }
>
> +static void i915_reenable_hotplug_timer_func(unsigned long data)
> +{
> + drm_i915_private_t *dev_priv = (drm_i915_private_t *)data;
> + struct drm_device *dev = dev_priv->dev;
> + struct drm_mode_config *mode_config = &dev->mode_config;
> + unsigned long irqflags;
> + int i;
> +
> + spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
> + for (i = 1; i < HPD_NUM_PINS; i++) {
> + if (dev_priv->hpd_stats[i].hpd_mark == HPD_MARK_DISABLED) {
> + struct drm_connector *connector;
> +
> + dev_priv->hpd_stats[i].hpd_mark = HPD_ENABLED;
> + list_for_each_entry(connector, &mode_config->connector_list, head) {
> + struct intel_connector *intel_connector = to_intel_connector(connector);
> +
> + if (intel_connector->encoder->hpd_pin == i) {
> + if (connector->polled != intel_connector->polled)
> + DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n",
> + drm_get_connector_name(connector));
> + connector->polled = intel_connector->polled;
> + if (!connector->polled)
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> + }
> + }
> +
> + if (IS_HASWELL(dev) ||
> + IS_IVYBRIDGE(dev) ||
> + (HAS_PCH_SPLIT(dev)))
> + ibx_hpd_irq_setup(dev);
> + else
> + i915_hpd_irq_setup(dev);
> + }
> + }
> + spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
> +}
> +
> void intel_irq_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -2887,6 +2936,8 @@ void intel_irq_init(struct drm_device *dev)
> setup_timer(&dev_priv->gpu_error.hangcheck_timer,
> i915_hangcheck_elapsed,
> (unsigned long) dev);
> + setup_timer(&dev_priv->hotplug_reenable_timer, i915_reenable_hotplug_timer_func,
> + (unsigned long) dev_priv);
>
> pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
>
> --
> 1.7.7
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v.2 11/12] DRM/i915: Add bit field to record which pins have received HPD events.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (9 preceding siblings ...)
2013-02-25 17:06 ` [PATCH v.2 10/12] DRM/i915: Add Reenable Timer to turn Hotplug Detection back on Egbert Eich
@ 2013-02-25 17:06 ` 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-02-28 0:46 ` [PATCH v.2 00/12] Detect and deal with Interrupt 'Storms' from noisy Hotplug Lines Chris Wilson
12 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
This way it is possible to limit 're'-detect() of displays to connectors
which have received an HPD event.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_irq.c | 10 ++++++++++
2 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 58bee7a..cd6391c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1068,6 +1068,7 @@ typedef struct drm_i915_private {
HPD_MARK_DISABLED = 2
} hpd_mark;
} hpd_stats[HPD_NUM_PINS];
+ u32 hpd_event_bits;
#define I915_REENABLE_HOTPLUG_DELAY (2*60*1000)
struct timer_list hotplug_reenable_timer;
} drm_i915_private_t;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ba598e3..6e7b0b5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -353,6 +353,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
struct drm_connector *connector;
unsigned long irqflags;
bool connector_disabled = false;
+ u32 hpd_event_bits;
/* HPD irq before everything is fully set up. */
if (!dev_priv->enable_hotplug_processing)
@@ -362,6 +363,9 @@ static void i915_hotplug_work_func(struct work_struct *work)
DRM_DEBUG_KMS("running encoder hotplug functions\n");
spin_lock_irqsave(&dev_priv->irq_lock, irqflags);
+
+ hpd_event_bits = dev_priv->hpd_event_bits;
+ dev_priv->hpd_event_bits = 0;
list_for_each_entry(connector, &mode_config->connector_list, head) {
intel_connector = to_intel_connector(connector);
intel_encoder = intel_connector->encoder;
@@ -375,6 +379,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
| DRM_CONNECTOR_POLL_DISCONNECT;
connector_disabled = true;
}
+ if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
+ DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
+ drm_get_connector_name(connector), intel_encoder->hpd_pin);
+ }
}
if (connector_disabled) {
drm_kms_helper_poll_enable(dev); /* if there were no outputs to poll, poll is disabled */
@@ -629,12 +637,14 @@ static inline bool hotplug_irq_storm_detect(struct drm_device *dev,
for (i = 1; i < HPD_NUM_PINS; i++) {
if ((hpd[i] & hotplug_trigger) &&
dev_priv->hpd_stats[i].hpd_mark == HPD_ENABLED) {
+ dev_priv->hpd_event_bits |= (1 << i);
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;
+ dev_priv->hpd_event_bits &= ~(1 << i);
DRM_DEBUG_KMS("HPD interrupt storm detected on PIN %d\n", i);
ret = true;
} else
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.2 12/12] DRM/i915: Only reprobe display on encoder which has received an HPD event.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (10 preceding siblings ...)
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 ` 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
12 siblings, 1 reply; 54+ messages in thread
From: Egbert Eich @ 2013-02-25 17:06 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
Instead of calling into the DRM helper layer to poll all connectors for
changes in connected displays probe only those connectors which have
received a hotplug event.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 37 +++++++++++++++++++++++++++++++------
1 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6e7b0b5..6c96d71 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -339,6 +339,24 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
crtc);
}
+/**
+ * drm_helper_hpd_irq_single_connector_event() - call this function with mode_config.mutex lock held
+ */
+
+static int intel_hpd_irq_single_connector_event(struct drm_device *dev, struct drm_connector *connector)
+{
+ enum drm_connector_status old_status;
+
+ old_status = connector->status;
+
+ connector->status = connector->funcs->detect(connector, false);
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
+ connector->base.id,
+ drm_get_connector_name(connector),
+ old_status, connector->status);
+ return (old_status != connector->status);
+}
+
/*
* Handle hotplug events outside the interrupt handler proper.
*/
@@ -353,6 +371,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
struct drm_connector *connector;
unsigned long irqflags;
bool connector_disabled = false;
+ bool changed = false;
u32 hpd_event_bits;
/* HPD irq before everything is fully set up. */
@@ -392,14 +411,20 @@ static void i915_hotplug_work_func(struct work_struct *work)
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
- list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- if (intel_encoder->hot_plug)
- intel_encoder->hot_plug(encoder);
-
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ intel_connector = to_intel_connector(connector);
+ intel_encoder = intel_connector->encoder;
+ if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
+ if (intel_encoder->hot_plug)
+ intel_encoder->hot_plug(intel_encoder);
+ if (intel_hpd_irq_single_connector_event(dev, connector))
+ changed = true;
+ }
+ }
mutex_unlock(&mode_config->mutex);
- /* Just fire off a uevent and let userspace tell us what to do */
- drm_helper_hpd_irq_event(dev);
+ if (changed)
+ drm_kms_helper_hotplug_event(dev);
}
static void ironlake_handle_rps_change(struct drm_device *dev)
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread* [PATCH v.3 12/12] DRM/i915: Only reprobe display on encoder which has received an HPD event.
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 ` Egbert Eich
0 siblings, 0 replies; 54+ messages in thread
From: Egbert Eich @ 2013-03-05 14:18 UTC (permalink / raw)
To: intel-gfx; +Cc: Egbert Eich, Chris Wilson, Rodrigo Vivi
Instead of calling into the DRM helper layer to poll all connectors for
changes in connected displays probe only those connectors which have
received a hotplug event.
Signed-off-by: Egbert Eich <eich@suse.de>
---
drivers/gpu/drm/i915/i915_irq.c | 37 +++++++++++++++++++++++++++++++------
1 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d65d76a..9e8d5b4 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -330,6 +330,24 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
crtc);
}
+/**
+ * drm_helper_hpd_irq_single_connector_event() - call this function with mode_config.mutex lock held
+ */
+
+static int intel_hpd_irq_single_connector_event(struct drm_device *dev, struct drm_connector *connector)
+{
+ enum drm_connector_status old_status;
+
+ old_status = connector->status;
+
+ connector->status = connector->funcs->detect(connector, false);
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
+ connector->base.id,
+ drm_get_connector_name(connector),
+ old_status, connector->status);
+ return (old_status != connector->status);
+}
+
/*
* Handle hotplug events outside the interrupt handler proper.
*/
@@ -344,6 +362,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
struct drm_connector *connector;
unsigned long irqflags;
bool connector_disabled = false;
+ bool changed = false;
u32 hpd_event_bits;
/* HPD irq before everything is fully set up. */
@@ -383,14 +402,20 @@ static void i915_hotplug_work_func(struct work_struct *work)
spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
- list_for_each_entry(intel_encoder, &mode_config->encoder_list, base.head)
- if (intel_encoder->hot_plug)
- intel_encoder->hot_plug(intel_encoder);
-
+ list_for_each_entry(connector, &mode_config->connector_list, head) {
+ intel_connector = to_intel_connector(connector);
+ intel_encoder = intel_connector->encoder;
+ if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
+ if (intel_encoder->hot_plug)
+ intel_encoder->hot_plug(intel_encoder);
+ if (intel_hpd_irq_single_connector_event(dev, connector))
+ changed = true;
+ }
+ }
mutex_unlock(&mode_config->mutex);
- /* Just fire off a uevent and let userspace tell us what to do */
- drm_helper_hpd_irq_event(dev);
+ if (changed)
+ drm_kms_helper_hotplug_event(dev);
}
static void ironlake_handle_rps_change(struct drm_device *dev)
--
1.7.7
^ permalink raw reply related [flat|nested] 54+ messages in thread
* Re: [PATCH v.2 00/12] Detect and deal with Interrupt 'Storms' from noisy Hotplug Lines.
2013-02-25 17:06 ` [PATCH v.2 00/12] " Egbert Eich
` (11 preceding siblings ...)
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-02-28 0:46 ` Chris Wilson
12 siblings, 0 replies; 54+ messages in thread
From: Chris Wilson @ 2013-02-28 0:46 UTC (permalink / raw)
To: Egbert Eich; +Cc: intel-gfx, Chris Wilson, Rodrigo Vivi
On Mon, Feb 25, 2013 at 12:06:47PM -0500, Egbert Eich wrote:
> I've reworked my 'hotplug interrupt storm detection'-patches and
> included most of Daniel's suggestions.
> I've looked into adding EDID caching but since this requires some
> larger scale changes and some changes outside of the Intel driver
> it seemed to be a good idea to propose those changes at a later time.
>
> Egbert Eich (12):
> DRM/i915: Remove valleyview_hpd_irq_setup.
> DRM/I915: Add enum hpd_pin to intel_encoder.
> DRM/i915: Convert HPD interrupts to make use of HPD pin assignment in
> encoders.
> DRM/i915: Remove i965_hpd_irq_setup.
> DRM/i915: Get rid if the 'hotplug_supported_mask' in struct
> drm_i915_private.
> DRM/i915: Add HPD IRQ storm detection.
> DRM/i915: (re)init HPD interrupt storm statistics.
> DRM/i915: Treat hpd_irq_setup() for ironake and older generations the
> same way.
> DRM/i915: Disable HPD interrupt on pin when irq storm is detected.
> DRM/i915: Add Reenable Timer to turn Hotplug Detection back on.
> DRM/i915: Add bit field to record which pins have received HPD
> events.
> DRM/i915: Only reprobe display on encoder which has received an HPD
> event.
I'm happy with this series. It seems to do everything that we need to
enable/disable hpd, and tidies up hotplug detection in the process.
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Didn't spot anything wrong in the first pass, but I'd like to let the
details soak in a bit before r-b.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 54+ messages in thread