intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gen4: Fix interrupt setup ordering
@ 2010-05-27 21:26 Adam Jackson
  2010-06-03 21:31 ` Adam Jackson
  0 siblings, 1 reply; 3+ messages in thread
From: Adam Jackson @ 2010-05-27 21:26 UTC (permalink / raw)
  To: intel-gfx

Unmask, then enable interrupts, then enable interrupt sources; matches
PCH ordering.  The old way (sources, enable, unmask) gives a window
during which interrupt conditions would appear in ISR but would never
reach IIR and thus never raise an IRQ.  Since interrupts only trigger
on rising edges in ISR, this would lead to conditions where (for
example) output hotplugging would never fire an interrupt because it
was already stuck on in ISR.

Also, since we know IIR and PIPExSTAT have been cleared during
irq_preinstall, don't clear them again during irq_postinstall, nothing
good can come of that.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/i915/i915_irq.c |   50 ++++++++++++++++++---------------------
 1 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a7e4b1f..f9d2cc0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1402,29 +1402,10 @@ int i915_driver_irq_postinstall(struct drm_device *dev)
 	dev_priv->pipestat[1] = 0;
 
 	if (I915_HAS_HOTPLUG(dev)) {
-		u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
-
-		/* Note HDMI and DP share bits */
-		if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
-			hotplug_en |= HDMIB_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & HDMIC_HOTPLUG_INT_STATUS)
-			hotplug_en |= HDMIC_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & HDMID_HOTPLUG_INT_STATUS)
-			hotplug_en |= HDMID_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS)
-			hotplug_en |= SDVOC_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS)
-			hotplug_en |= SDVOB_HOTPLUG_INT_EN;
-		if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS)
-			hotplug_en |= CRT_HOTPLUG_INT_EN;
-		/* Ignore TV since it's buggy */
-
-		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
-
 		/* Enable in IER... */
 		enable_mask |= I915_DISPLAY_PORT_INTERRUPT;
 		/* and unmask in IMR */
-		i915_enable_irq(dev_priv, I915_DISPLAY_PORT_INTERRUPT);
+		dev_priv->irq_mask_reg &= ~I915_DISPLAY_PORT_INTERRUPT;
 	}
 
 	/*
@@ -1442,16 +1423,31 @@ int i915_driver_irq_postinstall(struct drm_device *dev)
 	}
 	I915_WRITE(EMR, error_mask);
 
-	/* Disable pipe interrupt enables, clear pending pipe status */
-	I915_WRITE(PIPEASTAT, I915_READ(PIPEASTAT) & 0x8000ffff);
-	I915_WRITE(PIPEBSTAT, I915_READ(PIPEBSTAT) & 0x8000ffff);
-	/* Clear pending interrupt status */
-	I915_WRITE(IIR, I915_READ(IIR));
-
-	I915_WRITE(IER, enable_mask);
 	I915_WRITE(IMR, dev_priv->irq_mask_reg);
+	I915_WRITE(IER, enable_mask);
 	(void) I915_READ(IER);
 
+	if (I915_HAS_HOTPLUG(dev)) {
+		u32 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
+
+		/* Note HDMI and DP share bits */
+		if (dev_priv->hotplug_supported_mask & HDMIB_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMIB_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & HDMIC_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMIC_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & HDMID_HOTPLUG_INT_STATUS)
+			hotplug_en |= HDMID_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & SDVOC_HOTPLUG_INT_STATUS)
+			hotplug_en |= SDVOC_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & SDVOB_HOTPLUG_INT_STATUS)
+			hotplug_en |= SDVOB_HOTPLUG_INT_EN;
+		if (dev_priv->hotplug_supported_mask & CRT_HOTPLUG_INT_STATUS)
+			hotplug_en |= CRT_HOTPLUG_INT_EN;
+		/* Ignore TV since it's buggy */
+
+		I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
+	}
+
 	opregion_enable_asle(dev);
 
 	return 0;
-- 
1.7.0.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915/gen4: Fix interrupt setup ordering
  2010-05-27 21:26 [PATCH] drm/i915/gen4: Fix interrupt setup ordering Adam Jackson
@ 2010-06-03 21:31 ` Adam Jackson
  2010-06-04 18:01   ` Eric Anholt
  0 siblings, 1 reply; 3+ messages in thread
From: Adam Jackson @ 2010-06-03 21:31 UTC (permalink / raw)
  To: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 928 bytes --]

On Thu, 2010-05-27 at 17:26 -0400, Adam Jackson wrote:
> Unmask, then enable interrupts, then enable interrupt sources; matches
> PCH ordering.  The old way (sources, enable, unmask) gives a window
> during which interrupt conditions would appear in ISR but would never
> reach IIR and thus never raise an IRQ.  Since interrupts only trigger
> on rising edges in ISR, this would lead to conditions where (for
> example) output hotplugging would never fire an interrupt because it
> was already stuck on in ISR.
> 
> Also, since we know IIR and PIPExSTAT have been cleared during
> irq_preinstall, don't clear them again during irq_postinstall, nothing
> good can come of that.
> 
> Signed-off-by: Adam Jackson <ajax@redhat.com>

Just to follow up, I've tested this (combined with the already-merged
VGA paranoia patch) on GM45.  Output hotplug still works and I'm no
longer seeing stuck bits in ISR.

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/i915/gen4: Fix interrupt setup ordering
  2010-06-03 21:31 ` Adam Jackson
@ 2010-06-04 18:01   ` Eric Anholt
  0 siblings, 0 replies; 3+ messages in thread
From: Eric Anholt @ 2010-06-04 18:01 UTC (permalink / raw)
  To: Adam Jackson, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1241 bytes --]

On Thu, 03 Jun 2010 17:31:22 -0400, Adam Jackson <ajax@redhat.com> wrote:
> On Thu, 2010-05-27 at 17:26 -0400, Adam Jackson wrote:
> > Unmask, then enable interrupts, then enable interrupt sources; matches
> > PCH ordering.  The old way (sources, enable, unmask) gives a window
> > during which interrupt conditions would appear in ISR but would never
> > reach IIR and thus never raise an IRQ.  Since interrupts only trigger
> > on rising edges in ISR, this would lead to conditions where (for
> > example) output hotplugging would never fire an interrupt because it
> > was already stuck on in ISR.
> > 
> > Also, since we know IIR and PIPExSTAT have been cleared during
> > irq_preinstall, don't clear them again during irq_postinstall, nothing
> > good can come of that.
> > 
> > Signed-off-by: Adam Jackson <ajax@redhat.com>
> 
> Just to follow up, I've tested this (combined with the already-merged
> VGA paranoia patch) on GM45.  Output hotplug still works and I'm no
> longer seeing stuck bits in ISR.

Thanks for the followup.  It really helps me to know what people have
tested on, and whether they were fixing a bug due to code inspection or
due to actually solving a problem they were hitting on hardware.

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-06-04 18:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-27 21:26 [PATCH] drm/i915/gen4: Fix interrupt setup ordering Adam Jackson
2010-06-03 21:31 ` Adam Jackson
2010-06-04 18:01   ` Eric Anholt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).