All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: [PATCH 22/24] drm/i915: unify GT/PM irq postinstall code
Date: Wed, 12 Jun 2013 13:37:24 +0200	[thread overview]
Message-ID: <1371037046-3732-23-git-send-email-daniel.vetter@ffwll.ch> (raw)
In-Reply-To: <1371037046-3732-1-git-send-email-daniel.vetter@ffwll.ch>

Again extract a common helper. For the postinstall hook things are a
bit more complicated since we have more cases on ilk-hsw/vlv here.

But since vlv was clearly broken by failing to initialize
dev_priv->gt_irq_mask correclty (mayb that explains the strange
RING_IMR clearing in the preinstall hook?) clearly justified the
shared code.

Also kill the PMIER setting in the async rps enable work. I should
have been save, but also clearly looked rather fragile.

With this we now have the usual interrupt register sequence for GT/PM
irq registers:

- IER is setup once with all the interrupts we ever need in the
  postinstall hook and never touched again. Exceptions are SDEIER,
  which is touched in the preinstall hook (when the irq handler isn't
  enabled) and then only from the irq handler. And DEIER/VLV_IER with
  is used in the irq handler but also written to once in the
  postinstall hook. But since that write is essentially what enables
  the interrupt and we should always have MSI interrupts we should be
  save. In case we ever have non-MSI interrupts we'd be screwed.

- IIR is cleared in the postinstall hook before we enable/unmask the
  respective interrupt sources. Hence we can't steal an interrupt
  event an accidentally trigger the spurious interrupt logic in the
  core kernel.

- IMR regs are (usually) all masked off. Those are the only regs
  changed at runtime, which is all protected by dev_priv->irq_lock.

This unification also kills the cargo-culted read-modify-write PM
register setup for VECS. Interrupt setup is done without userspace
being able to interfere, so we better know what values we want to put
into those registers. RMW cycles otoh are really good at papering over
races, until stuff magically blows up and no one has a clue why.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 91 +++++++++++++++++++----------------------
 drivers/gpu/drm/i915/intel_pm.c |  4 --
 2 files changed, 41 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 954d8f8..1fb49f7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2616,6 +2616,44 @@ static void ibx_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(SDEIMR, ~mask);
 }
 
+static void gen5_gt_irq_postinstall(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 pm_irqs, gt_irqs;
+
+	pm_irqs = gt_irqs = 0;
+
+	dev_priv->gt_irq_mask = ~0;
+	if (HAS_L3_GPU_CACHE(dev)) {
+		dev_priv->gt_irq_mask = ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
+		gt_irqs |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
+	}
+
+	gt_irqs |= GT_RENDER_USER_INTERRUPT;
+	if (IS_GEN5(dev)) {
+		gt_irqs |= GT_RENDER_PIPECTL_NOTIFY_INTERRUPT |
+			   ILK_BSD_USER_INTERRUPT;
+	} else {
+		gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
+	}
+
+	I915_WRITE(GTIIR, I915_READ(GTIIR));
+	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
+	I915_WRITE(GTIER, gt_irqs);
+	POSTING_READ(GTIER);
+
+	if (HAS_VEBOX(dev))
+		pm_irqs |= PM_VEBOX_USER_INTERRUPT;
+	if (INTEL_INFO(dev)->gen >= 6)
+		pm_irqs |= GEN6_PM_RPS_EVENTS;
+
+	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+	I915_WRITE(GEN6_PMIMR, 0xffffffff);
+	I915_WRITE(GEN6_PMIER, pm_irqs);
+	POSTING_READ(GEN6_PMIER);
+
+}
+
 static int ironlake_irq_postinstall(struct drm_device *dev)
 {
 	unsigned long irqflags;
@@ -2626,7 +2664,6 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE |
 			   DE_AUX_CHANNEL_A | DE_PIPEB_FIFO_UNDERRUN |
 			   DE_PIPEA_FIFO_UNDERRUN | DE_POISON;
-	u32 gt_irqs;
 
 	dev_priv->irq_mask = ~display_mask;
 
@@ -2637,21 +2674,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 			  DE_PIPEA_VBLANK | DE_PIPEB_VBLANK | DE_PCU_EVENT);
 	POSTING_READ(DEIER);
 
-	dev_priv->gt_irq_mask = ~0;
-
-	I915_WRITE(GTIIR, I915_READ(GTIIR));
-	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
-
-	gt_irqs = GT_RENDER_USER_INTERRUPT;
-
-	if (IS_GEN6(dev))
-		gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
-	else
-		gt_irqs |= GT_RENDER_PIPECTL_NOTIFY_INTERRUPT |
-			   ILK_BSD_USER_INTERRUPT;
-
-	I915_WRITE(GTIER, gt_irqs);
-	POSTING_READ(GTIER);
+	gen5_gt_irq_postinstall(dev);
 
 	ibx_irq_postinstall(dev);
 
@@ -2680,8 +2703,6 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 		DE_PLANEA_FLIP_DONE_IVB |
 		DE_AUX_CHANNEL_A_IVB |
 		DE_ERR_INT_IVB;
-	u32 pm_irqs = GEN6_PM_RPS_EVENTS;
-	u32 gt_irqs;
 
 	dev_priv->irq_mask = ~display_mask;
 
@@ -2696,30 +2717,7 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 		   DE_PIPEA_VBLANK_IVB);
 	POSTING_READ(DEIER);
 
-	dev_priv->gt_irq_mask = ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
-
-	I915_WRITE(GTIIR, I915_READ(GTIIR));
-	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
-
-	gt_irqs = GT_RENDER_USER_INTERRUPT | GT_BSD_USER_INTERRUPT |
-		  GT_BLT_USER_INTERRUPT | GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
-	I915_WRITE(GTIER, gt_irqs);
-	POSTING_READ(GTIER);
-
-	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
-	if (HAS_VEBOX(dev))
-		pm_irqs |= PM_VEBOX_USER_INTERRUPT;
-
-	/* Our enable/disable rps functions may touch these registers so
-	 * make sure to set a known state for only the non-RPS bits.
-	 * The RMW is extra paranoia since this should be called after being set
-	 * to a known state in preinstall.
-	 * */
-	I915_WRITE(GEN6_PMIMR,
-		   (I915_READ(GEN6_PMIMR) | ~GEN6_PM_RPS_EVENTS) & ~pm_irqs);
-	I915_WRITE(GEN6_PMIER,
-		   (I915_READ(GEN6_PMIER) & GEN6_PM_RPS_EVENTS) | pm_irqs);
-	POSTING_READ(GEN6_PMIER);
+	gen5_gt_irq_postinstall(dev);
 
 	ibx_irq_postinstall(dev);
 
@@ -2729,7 +2727,6 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 static int valleyview_irq_postinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
-	u32 gt_irqs;
 	u32 enable_mask;
 	u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV;
 	unsigned long irqflags;
@@ -2769,13 +2766,7 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(VLV_IIR, 0xffffffff);
 	I915_WRITE(VLV_IIR, 0xffffffff);
 
-	I915_WRITE(GTIIR, I915_READ(GTIIR));
-	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
-
-	gt_irqs = GT_RENDER_USER_INTERRUPT | GT_BSD_USER_INTERRUPT |
-		GT_BLT_USER_INTERRUPT;
-	I915_WRITE(GTIER, gt_irqs);
-	POSTING_READ(GTIER);
+	gen5_gt_irq_postinstall(dev);
 
 	/* ack & enable invalid PTE error interrupts */
 #if 0 /* FIXME: add support to irq handler for checking these bits */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 6a5cfb5..1f3dcdc 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3312,8 +3312,6 @@ static void gen6_enable_rps(struct drm_device *dev)
 
 	gen6_set_rps(dev_priv->dev, (gt_perf_status & 0xff00) >> 8);
 
-	/* requires MSI enabled */
-	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
 	spin_lock_irq(&dev_priv->irq_lock);
 	/* FIXME: Our interrupt enabling sequence is bonghits.
 	 * dev_priv->rps.pm_iir really should be 0 here. */
@@ -3583,8 +3581,6 @@ static void valleyview_enable_rps(struct drm_device *dev)
 
 	valleyview_set_rps(dev_priv->dev, rpe);
 
-	/* requires MSI enabled */
-	I915_WRITE(GEN6_PMIER, GEN6_PM_RPS_EVENTS);
 	spin_lock_irq(&dev_priv->irq_lock);
 	WARN_ON(dev_priv->rps.pm_iir != 0);
 	I915_WRITE(GEN6_PMIMR, 0);
-- 
1.8.1.4

  parent reply	other threads:[~2013-06-12 11:38 UTC|newest]

Thread overview: 86+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-12 11:37 [PATCH 00/24] irq locking review Daniel Vetter
2013-06-12 11:37 ` [PATCH 01/24] drm/i915: fix locking around ironlake_enable|disable_display_irq Daniel Vetter
2013-06-25 12:26   ` [PATCH] " Daniel Vetter
2013-06-26 21:15     ` Paulo Zanoni
2013-06-27 10:37       ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 02/24] drm/i915: close tiny race in the ilk pcu even interrupt setup Daniel Vetter
2013-06-26 21:20   ` Paulo Zanoni
2013-06-12 11:37 ` [PATCH 03/24] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-26 21:44   ` Paulo Zanoni
2013-06-12 11:37 ` [PATCH 04/24] drm/i915: s/hotplug_irq_storm_detect/intel_hpd_irq_handler/ Daniel Vetter
2013-06-12 14:22   ` Egbert Eich
2013-06-12 11:37 ` [PATCH 05/24] drm/i915: fold the hpd_irq_setup call into intel_hpd_irq_handler Daniel Vetter
     [not found]   ` <20920.34096.704203.67316@linux-qknr.site>
2013-06-12 15:00     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 06/24] drm/i915: fold the queue_work " Daniel Vetter
2013-06-12 14:29   ` Egbert Eich
2013-06-12 11:37 ` [PATCH 07/24] drm/i915: fold the no-irq check " Daniel Vetter
2013-06-12 14:33   ` Egbert Eich
2013-06-26 22:35     ` Paulo Zanoni
2013-06-27 10:39       ` Daniel Vetter
2013-06-27 11:44       ` [PATCH 1/8] drm/i915: fix locking around ironlake_enable|disable_display_irq Daniel Vetter
2013-06-27 11:44         ` [PATCH 2/8] drm/i915: close tiny race in the ilk pcu even interrupt setup Daniel Vetter
2013-06-27 11:45         ` [PATCH 3/8] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-27 11:45         ` [PATCH 4/8] drm/i915: s/hotplug_irq_storm_detect/intel_hpd_irq_handler/ Daniel Vetter
2013-06-27 11:45         ` [PATCH 5/8] drm/i915: fold the hpd_irq_setup call into intel_hpd_irq_handler Daniel Vetter
2013-06-27 11:45         ` [PATCH 6/8] drm/i915: fold the queue_work " Daniel Vetter
2013-06-27 12:13           ` Chris Wilson
2013-06-27 11:45         ` [PATCH 7/8] drm/i915: fold the no-irq check " Daniel Vetter
2013-06-27 12:14           ` Chris Wilson
2013-06-27 11:45         ` [PATCH 8/8] drm/i915: fix hpd interrupt register locking Daniel Vetter
2013-06-27 14:41           ` Paulo Zanoni
2013-06-27 15:52             ` [PATCH 1/6] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-27 15:52               ` [PATCH 2/6] drm/i915: s/hotplug_irq_storm_detect/intel_hpd_irq_handler/ Daniel Vetter
2013-06-27 15:52               ` [PATCH 3/6] drm/i915: fold the hpd_irq_setup call into intel_hpd_irq_handler Daniel Vetter
2013-06-27 15:52               ` [PATCH 4/6] drm/i915: fold the queue_work " Daniel Vetter
2013-06-27 15:52               ` [PATCH 5/6] drm/i915: fold the no-irq check " Daniel Vetter
2013-06-27 15:52               ` [PATCH 6/6] drm/i915: fix hpd interrupt register locking Daniel Vetter
2013-07-04 19:22               ` [PATCH 1/6] drm/i915: assert_spin_locked for pipestat interrupt enable/disable Daniel Vetter
2013-06-27 17:44             ` [PATCH 8/8] drm/i915: fix hpd interrupt register locking Daniel Vetter
2013-06-12 11:37 ` [PATCH 08/24] " Daniel Vetter
2013-06-12 14:59   ` Egbert Eich
2013-06-12 15:10     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 09/24] drm/i915: extract ibx_display_interrupt_update Daniel Vetter
2013-06-25 12:27   ` [PATCH] " Daniel Vetter
2013-06-12 11:37 ` [PATCH 10/24] drm/i915: remove SERR_INT clearing in the postinstall hook Daniel Vetter
2013-06-27 19:34   ` Paulo Zanoni
2013-07-04 19:49     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 11/24] drm/i915: improve SERR_INT clearing for fifo underrun reporting Daniel Vetter
2013-06-27 20:19   ` Paulo Zanoni
2013-07-04 19:55     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 12/24] drm/i915: improve GEN7_ERR_INT " Daniel Vetter
2013-06-12 11:37 ` [PATCH 13/24] drm/i915: kill lpt pch transcoder->crtc mapping code for fifo underruns Daniel Vetter
2013-06-12 13:04   ` Paulo Zanoni
2013-06-12 14:46     ` [PATCH] " Daniel Vetter
2013-06-27 20:45       ` Paulo Zanoni
2013-07-04 20:41         ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 14/24] drm/i915: irq handlers don't need interrupt-safe spinlocks Daniel Vetter
2013-06-25 12:27   ` [PATCH] " Daniel Vetter
2013-06-27 21:14     ` Paulo Zanoni
2013-06-27 22:40       ` Daniel Vetter
2013-06-28 16:57         ` Paulo Zanoni
2013-06-12 11:37 ` [PATCH 15/24] drm/i915: streamline hsw_pm_irq_handler Daniel Vetter
2013-06-25 12:28   ` [PATCH] " Daniel Vetter
2013-06-12 11:37 ` [PATCH 16/24] drm/i915: queue work outside spinlock in hsw_pm_irq_handler Daniel Vetter
2013-06-12 11:37 ` [PATCH 17/24] drm/i915: kill dev_priv->rps.lock Daniel Vetter
2013-06-28  3:35   ` Ben Widawsky
2013-06-28  3:35   ` Ben Widawsky
2013-06-12 11:37 ` [PATCH 18/24] drm/i915: unify ring irq refcounts (again) Daniel Vetter
2013-06-28 17:24   ` Ben Widawsky
2013-07-04 20:52     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 19/24] drm/i915: don't enable PM_VEBOX_CS_ERROR_INTERRUPT Daniel Vetter
2013-06-12 17:13   ` Ben Widawsky
2013-06-12 17:18     ` Daniel Vetter
2013-06-12 18:19       ` Ben Widawsky
2013-06-12 18:32         ` Daniel Vetter
2013-06-12 18:51           ` Ben Widawsky
2013-06-28 17:25           ` Ben Widawsky
2013-06-12 11:37 ` [PATCH 20/24] drm/i915: kill bogus GTIIR clearing in vlv_preinstall hook Daniel Vetter
2013-06-28 17:01   ` Ben Widawsky
2013-07-04 20:56     ` Daniel Vetter
2013-06-12 11:37 ` [PATCH 21/24] drm/i915: unify PM interrupt preinstall sequence Daniel Vetter
2013-06-28 17:26   ` Ben Widawsky
2013-07-04 21:03     ` Daniel Vetter
2013-06-12 11:37 ` Daniel Vetter [this message]
2013-06-12 11:37 ` [PATCH 23/24] drm/i915: extract rps interrupt enable/disable helpers Daniel Vetter
2013-06-12 11:37 ` [PATCH 24/24] drm/i915: simplify rps interrupt enabling/disabling sequence Daniel Vetter
2013-06-12 22:32   ` Ben Widawsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1371037046-3732-23-git-send-email-daniel.vetter@ffwll.ch \
    --to=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.