[PATCH 00/18] [RFC] Introduce the Haswell VECS

[PATCH 00/18] [RFC] Introduce the Haswell VECS

[Ben Widawsky]

The VECS is a new command streamer introduced in Haswell which allows
offloading of video post processing to a new engine called the VEBOX.
Like other rings, it is up to userspace to take advantage of it. Also
like the other rings, the primary enabling is turning on interrupts,
adding the new semaphores, and other basic bits. Somewhat more
complicated than the other rings, the VECS has its interrupts in the
PMIMR, so as a result, I did a bunch of interrupt naming cleanups
because I thought it made more sense.

Coming separately will be the basic i-g-t test cases. I still have some
work to do on those, but figure that the review can go on in parallel.

In terms of the work history, these started as patches by me, finished by
Haihao, and then cleaned up and rebased by me. I tried to leave credit to
Haihao where applicable, but I rewrote a lot of his stuff, so blame me if
something is bad.

Ben Widawsky (14):
  drm/i915: Comments for semaphore clarification
  drm/i915: Semaphore MBOX update generalization
  drm/i915: Introduce VECS: the 4th ring
  drm/i915: Add VECS semaphore bits
  drm/i915: Rename ring flush functions
  drm/i915: Vebox ringbuffer init
  drm/i915: Create a more generic pm handler for hsw+
  drm/i915: make PM interrupt writes non-destructive
  drm/i915: Create an ivybridge_irq_preinstall
  drm/i915: Add PM regs to pre install
  drm/i915: Convert irq_refounct to struct
  drm/i915: consolidate interrupt naming scheme
  drm/i915: vebox interrupt get/put
  drm/i915: Enable vebox interrupts

Xiang, Haihao (4):
  drm/i915: add HAS_VEBOX
  drm/i915: add VEBOX into debugfs
  drm/i915: add I915_EXEC_VEBOX to i915_gem_do_execbuffer()
  drm/i915: add I915_PARAM_HAS_VEBOX to i915_getparam

 drivers/gpu/drm/i915/i915_debugfs.c        |  13 ++
 drivers/gpu/drm/i915/i915_dma.c            |   5 +-
 drivers/gpu/drm/i915/i915_drv.c            |   2 +
 drivers/gpu/drm/i915/i915_drv.h            |   3 +
 drivers/gpu/drm/i915/i915_gem.c            |   8 ++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   9 ++
 drivers/gpu/drm/i915/i915_irq.c            | 153 +++++++++++++++++------
 drivers/gpu/drm/i915/i915_reg.h            | 148 ++++++++++++-----------
 drivers/gpu/drm/i915/intel_pm.c            |   8 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 188 +++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  13 +-
 include/uapi/drm/i915_drm.h                |   3 +-
 12 files changed, 383 insertions(+), 170 deletions(-)

-- 
1.8.0

[PATCH 01/18] drm/i915: Comments for semaphore clarification

[Ben Widawsky]

Semaphores are tied very closely to the rings in the GPU. Trivial patch
adds comments to the existing code so that when we add new rings we can
include comments there as well. It also helps distinguish the ring to
semaphore mailbox interactions by using the ringname in the semaphore
data structures.

This patch should have no functional impact.

A subset of this patch was:
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++---------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9118bd1..f82755e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -252,12 +252,12 @@
 #define  MI_SEMAPHORE_UPDATE	    (1<<21)
 #define  MI_SEMAPHORE_COMPARE	    (1<<20)
 #define  MI_SEMAPHORE_REGISTER	    (1<<18)
-#define  MI_SEMAPHORE_SYNC_RV	    (2<<16)
-#define  MI_SEMAPHORE_SYNC_RB	    (0<<16)
-#define  MI_SEMAPHORE_SYNC_VR	    (0<<16)
-#define  MI_SEMAPHORE_SYNC_VB	    (2<<16)
-#define  MI_SEMAPHORE_SYNC_BR	    (2<<16)
-#define  MI_SEMAPHORE_SYNC_BV	    (0<<16)
+#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS wait for BCS  (BRSYNC) */
+#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS wait for VCS  (VRSYNC) */
+#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS wait for RCS  (RVSYNC) */
+#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS wait for BCS  (BVSYNC) */
+#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS wait for VCS  (VBSYNC) */
+#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS wait for RCS  (RBSYNC) */
 #define  MI_SEMAPHORE_SYNC_INVALID  (1<<0)
 /*
  * 3D instructions used by the kernel
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a035ac2..423948f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1503,9 +1503,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->irq_enable_mask = GT_USER_INTERRUPT;
 		ring->get_seqno = gen6_ring_get_seqno;
 		ring->sync_to = gen6_ring_sync;
-		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID;
-		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_RV;
-		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_RB;
+		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
+		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
+		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
 		ring->signal_mbox[0] = GEN6_VRSYNC;
 		ring->signal_mbox[1] = GEN6_BRSYNC;
 	} else if (IS_GEN5(dev)) {
@@ -1639,9 +1639,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->irq_put = gen6_ring_put_irq;
 		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 		ring->sync_to = gen6_ring_sync;
-		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_VR;
-		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_INVALID;
-		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_VB;
+		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
+		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
+		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
 		ring->signal_mbox[0] = GEN6_RVSYNC;
 		ring->signal_mbox[1] = GEN6_BVSYNC;
 	} else {
@@ -1684,9 +1684,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	ring->irq_put = gen6_ring_put_irq;
 	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 	ring->sync_to = gen6_ring_sync;
-	ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_BR;
-	ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_BV;
-	ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
+	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
+	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
 	ring->signal_mbox[0] = GEN6_RBSYNC;
 	ring->signal_mbox[1] = GEN6_VBSYNC;
 	ring->init = init_ring_common;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 5af65b8..df1a0a2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -89,7 +89,7 @@ struct  intel_ring_buffer {
 				   struct intel_ring_buffer *to,
 				   u32 seqno);
 
-	u32		semaphore_register[3]; /*our mbox written by others */
+	u32		semaphore_register[I915_NUM_RINGS]; /*our mbox written by others */
 	u32		signal_mbox[2]; /* mboxes this ring signals to */
 	/**
 	 * List of objects currently involved in rendering from the
-- 
1.8.0

[PATCH 02/18] drm/i915: Semaphore MBOX update generalization

[Ben Widawsky]

This replaces the existing MBOX update code with a more generalized
calculation for emitting mbox updates. We also create a sentinel for
doing the updates so we can more abstractly deal with the rings.

When doing MBOX updates the code must be aware of the /other/ rings.
Until now the platforms which supported semaphores had a fixed number of
rings and so it made sense for the code to be very specialized
(hardcoded).

The patch does contain a functional change, but should have no
behavioral changes.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_reg.h         |  1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 40 ++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f82755e..d220410 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -438,6 +438,7 @@
 #define GEN6_VRSYNC (RING_SYNC_1(GEN6_BSD_RING_BASE))
 #define GEN6_VBSYNC (RING_SYNC_0(GEN6_BSD_RING_BASE))
 #define GEN6_BRSYNC (RING_SYNC_0(BLT_RING_BASE))
+#define GEN6_NOSYNC 0
 #define GEN6_BVSYNC (RING_SYNC_1(BLT_RING_BASE))
 #define RING_MAX_IDLE(base)	((base)+0x54)
 #define RING_HWS_PGA(base)	((base)+0x80)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 423948f..ace1176 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -558,12 +558,14 @@ update_mboxes(struct intel_ring_buffer *ring,
 	    u32 seqno,
 	    u32 mmio_offset)
 {
+#define MBOX_UPDATE_DWORDS 4
 	intel_ring_emit(ring, MI_SEMAPHORE_MBOX |
 			      MI_SEMAPHORE_GLOBAL_GTT |
 			      MI_SEMAPHORE_REGISTER |
 			      MI_SEMAPHORE_UPDATE);
 	intel_ring_emit(ring, seqno);
 	intel_ring_emit(ring, mmio_offset);
+	intel_ring_emit(ring, MI_NOOP);
 }
 
 /**
@@ -579,21 +581,26 @@ static int
 gen6_add_request(struct intel_ring_buffer *ring,
 		 u32 *seqno)
 {
-	u32 mbox1_reg;
-	u32 mbox2_reg;
-	int ret;
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_ring_buffer *useless;
+	int i, ret;
 
-	ret = intel_ring_begin(ring, 10);
+	ret = intel_ring_begin(ring, ((I915_NUM_RINGS-1) *
+				      MBOX_UPDATE_DWORDS) +
+				      4);
 	if (ret)
 		return ret;
-
-	mbox1_reg = ring->signal_mbox[0];
-	mbox2_reg = ring->signal_mbox[1];
+#undef MBOX_UPDATE_DWORDS
 
 	*seqno = i915_gem_next_request_seqno(ring);
 
-	update_mboxes(ring, *seqno, mbox1_reg);
-	update_mboxes(ring, *seqno, mbox2_reg);
+	for_each_ring(useless, dev_priv, i) {
+		u32 mbox_reg = ring->signal_mbox[i];
+		if (mbox_reg != GEN6_NOSYNC)
+			update_mboxes(ring, *seqno, mbox_reg);
+	}
+
 	intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
 	intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT);
 	intel_ring_emit(ring, *seqno);
@@ -1506,8 +1513,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
 		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
-		ring->signal_mbox[0] = GEN6_VRSYNC;
-		ring->signal_mbox[1] = GEN6_BRSYNC;
+		ring->signal_mbox[RCS] = GEN6_NOSYNC;
+		ring->signal_mbox[VCS] = GEN6_VRSYNC;
+		ring->signal_mbox[BCS] = GEN6_BRSYNC;
 	} else if (IS_GEN5(dev)) {
 		ring->add_request = pc_render_add_request;
 		ring->flush = gen4_render_ring_flush;
@@ -1642,8 +1650,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
 		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
-		ring->signal_mbox[0] = GEN6_RVSYNC;
-		ring->signal_mbox[1] = GEN6_BVSYNC;
+		ring->signal_mbox[RCS] = GEN6_RVSYNC;
+		ring->signal_mbox[VCS] = GEN6_NOSYNC;
+		ring->signal_mbox[BCS] = GEN6_BVSYNC;
 	} else {
 		ring->mmio_base = BSD_RING_BASE;
 		ring->flush = bsd_ring_flush;
@@ -1687,8 +1696,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
 	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
 	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
-	ring->signal_mbox[0] = GEN6_RBSYNC;
-	ring->signal_mbox[1] = GEN6_VBSYNC;
+	ring->signal_mbox[RCS] = GEN6_RBSYNC;
+	ring->signal_mbox[VCS] = GEN6_VBSYNC;
+	ring->signal_mbox[BCS] = GEN6_NOSYNC;
 	ring->init = init_ring_common;
 
 	return intel_init_ring_buffer(dev, ring);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index df1a0a2..d8eecb1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -90,7 +90,7 @@ struct  intel_ring_buffer {
 				   u32 seqno);
 
 	u32		semaphore_register[I915_NUM_RINGS]; /*our mbox written by others */
-	u32		signal_mbox[2]; /* mboxes this ring signals to */
+	u32		signal_mbox[I915_NUM_RINGS]; /* mboxes this ring signals to */
 	/**
 	 * List of objects currently involved in rendering from the
 	 * ringbuffer.
-- 
1.8.0

[PATCH 03/18] drm/i915: Introduce VECS: the 4th ring

[Ben Widawsky]

The video enhancement command streamer is a new ring on HSW which does
what it sounds like it does. This patch provides the most minimal
inception of the ring.

In order to support a new ring, we need to bump the number. The patch
may look trivial to the untrained eye, but bumping the number of rings
is a bit scary. As such the patch is not terribly useful by itself, but
a pretty nice place to find issues during a bisection.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
 drivers/gpu/drm/i915/intel_ringbuffer.h | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ace1176..299fb0c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -866,6 +866,8 @@ void intel_ring_setup_status_page(struct intel_ring_buffer *ring)
 		case VCS:
 			mmio = BSD_HWS_PGA_GEN7;
 			break;
+		case VECS:
+			BUG();
 		}
 	} else if (IS_GEN6(ring->dev)) {
 		mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d8eecb1..3f7530e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -32,8 +32,9 @@ struct  intel_ring_buffer {
 		RCS = 0x0,
 		VCS,
 		BCS,
+		VECS,
 	} id;
-#define I915_NUM_RINGS 3
+#define I915_NUM_RINGS 4
 	u32		mmio_base;
 	void		__iomem *virtual_start;
 	struct		drm_device *dev;
-- 
1.8.0

[PATCH 04/18] drm/i915: Add VECS semaphore bits

[Ben Widawsky]

Like the other rings, the VECS supports semaphores. The semaphore stuff
is a bit wonky so this patch on it's own should be nice for review.

This patch should have no functional impact.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_reg.h         | 40 ++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_ringbuffer.c |  9 +++++---
 2 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d220410..62bad81 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -252,13 +252,19 @@
 #define  MI_SEMAPHORE_UPDATE	    (1<<21)
 #define  MI_SEMAPHORE_COMPARE	    (1<<20)
 #define  MI_SEMAPHORE_REGISTER	    (1<<18)
-#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS wait for BCS  (BRSYNC) */
-#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS wait for VCS  (VRSYNC) */
-#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS wait for RCS  (RVSYNC) */
-#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS wait for BCS  (BVSYNC) */
-#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS wait for VCS  (VBSYNC) */
-#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS wait for RCS  (RBSYNC) */
-#define  MI_SEMAPHORE_SYNC_INVALID  (1<<0)
+#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS  wait for RCS  (RVSYNC) */
+#define  MI_SEMAPHORE_SYNC_VER	    (1<<16) /* VECS wait for RCS  (RVESYNC) */
+#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS  wait for RCS  (RBSYNC) */
+#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS  wait for VCS  (VBSYNC) */
+#define  MI_SEMAPHORE_SYNC_VEV	    (1<<16) /* VECS wait for VCS  (VVESYNC) */
+#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS  wait for VCS  (VRSYNC) */
+#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS  wait for BCS  (BRSYNC) */
+#define  MI_SEMAPHORE_SYNC_VEB	    (1<<16) /* VECS wait for BCS  (BVESYNC) */
+#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS  wait for BCS  (BVSYNC) */
+#define  MI_SEMAPHORE_SYNC_BVE	    (0<<16) /* BCS  wait for VECS (VEBSYNC) */
+#define  MI_SEMAPHORE_SYNC_VVE	    (1<<16) /* VCS  wait for VECS (VEVSYNC) */
+#define  MI_SEMAPHORE_SYNC_RVE	    (2<<16) /* RCS  wait for VECS (VERSYNC) */
+#define  MI_SEMAPHORE_SYNC_INVALID  (3<<16)
 /*
  * 3D instructions used by the kernel
  */
@@ -426,6 +432,7 @@
 #define RENDER_RING_BASE	0x02000
 #define BSD_RING_BASE		0x04000
 #define GEN6_BSD_RING_BASE	0x12000
+#define VEBOX_RING_BASE		0x1a000
 #define BLT_RING_BASE		0x22000
 #define RING_TAIL(base)		((base)+0x30)
 #define RING_HEAD(base)		((base)+0x34)
@@ -433,13 +440,20 @@
 #define RING_CTL(base)		((base)+0x3c)
 #define RING_SYNC_0(base)	((base)+0x40)
 #define RING_SYNC_1(base)	((base)+0x44)
-#define GEN6_RVSYNC (RING_SYNC_0(RENDER_RING_BASE))
-#define GEN6_RBSYNC (RING_SYNC_1(RENDER_RING_BASE))
-#define GEN6_VRSYNC (RING_SYNC_1(GEN6_BSD_RING_BASE))
-#define GEN6_VBSYNC (RING_SYNC_0(GEN6_BSD_RING_BASE))
-#define GEN6_BRSYNC (RING_SYNC_0(BLT_RING_BASE))
+#define RING_SYNC_2(base)	((base)+0x48)
+#define GEN6_RVSYNC	(RING_SYNC_0(RENDER_RING_BASE))
+#define GEN6_RBSYNC	(RING_SYNC_1(RENDER_RING_BASE))
+#define GEN6_RVESYNC	(RING_SYNC_2(RENDER_RING_BASE))
+#define GEN6_VBSYNC	(RING_SYNC_0(GEN6_BSD_RING_BASE))
+#define GEN6_VRSYNC	(RING_SYNC_1(GEN6_BSD_RING_BASE))
+#define GEN6_VVESYNC	(RING_SYNC_2(GEN6_BSD_RING_BASE))
+#define GEN6_BRSYNC	(RING_SYNC_0(BLT_RING_BASE))
+#define GEN6_BVSYNC	(RING_SYNC_1(BLT_RING_BASE))
+#define GEN6_BVESYNC	(RING_SYNC_2(BLT_RING_BASE))
+#define GEN6_VEBSYNC	(RING_SYNC_0(VEBOX_RING_BASE))
+#define GEN6_VERSYNC	(RING_SYNC_1(VEBOX_RING_BASE))
+#define GEN6_VEVSYNC	(RING_SYNC_2(VEBOX_RING_BASE))
 #define GEN6_NOSYNC 0
-#define GEN6_BVSYNC (RING_SYNC_1(BLT_RING_BASE))
 #define RING_MAX_IDLE(base)	((base)+0x54)
 #define RING_HWS_PGA(base)	((base)+0x80)
 #define RING_HWS_PGA_GEN6(base)	((base)+0x2080)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 299fb0c..2d0c3bb 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -633,9 +633,6 @@ gen6_ring_sync(struct intel_ring_buffer *waiter,
 	 */
 	seqno -= 1;
 
-	WARN_ON(signaller->semaphore_register[waiter->id] ==
-		MI_SEMAPHORE_SYNC_INVALID);
-
 	ret = intel_ring_begin(waiter, 4);
 	if (ret)
 		return ret;
@@ -1515,9 +1512,11 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
 		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
+		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_RVE;
 		ring->signal_mbox[RCS] = GEN6_NOSYNC;
 		ring->signal_mbox[VCS] = GEN6_VRSYNC;
 		ring->signal_mbox[BCS] = GEN6_BRSYNC;
+		ring->signal_mbox[VECS] = GEN6_VERSYNC;
 	} else if (IS_GEN5(dev)) {
 		ring->add_request = pc_render_add_request;
 		ring->flush = gen4_render_ring_flush;
@@ -1652,9 +1651,11 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
 		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
 		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
+		ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_VEV;
 		ring->signal_mbox[RCS] = GEN6_RVSYNC;
 		ring->signal_mbox[VCS] = GEN6_NOSYNC;
 		ring->signal_mbox[BCS] = GEN6_BVSYNC;
+		ring->signal_mbox[VECS] = GEN6_VEVSYNC;
 	} else {
 		ring->mmio_base = BSD_RING_BASE;
 		ring->flush = bsd_ring_flush;
@@ -1698,9 +1699,11 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
 	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
 	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_BVE;
 	ring->signal_mbox[RCS] = GEN6_RBSYNC;
 	ring->signal_mbox[VCS] = GEN6_VBSYNC;
 	ring->signal_mbox[BCS] = GEN6_NOSYNC;
+	ring->signal_mbox[VECS] = GEN6_VEBSYNC;
 	ring->init = init_ring_common;
 
 	return intel_init_ring_buffer(dev, ring);
-- 
1.8.0

[PATCH 05/18] drm/i915: Rename ring flush functions

[Ben Widawsky]

Historically we considered the render ring to have special flush
semantics and everything else to fall under a more general umbrella.
Probably by coincidence more than anything we decided to make the bsd
ring have the default *other* flush. As the new vebox ring exposes, the
bsd ring is actually the weird one. Doing this allows us to call
gen6_ring_flush for the vebox because calling blt_ring_flush would be
weird...

This patch should have no functional change.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2d0c3bb..11f3698 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1390,8 +1390,8 @@ static void gen6_bsd_ring_write_tail(struct intel_ring_buffer *ring,
 		   _MASKED_BIT_DISABLE(GEN6_BSD_SLEEP_MSG_DISABLE));
 }
 
-static int gen6_ring_flush(struct intel_ring_buffer *ring,
-			   u32 invalidate, u32 flush)
+static int gen6_bsd_ring_flush(struct intel_ring_buffer *ring,
+			       u32 invalidate, u32 flush)
 {
 	uint32_t cmd;
 	int ret;
@@ -1462,8 +1462,8 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
 
 /* Blitter support (SandyBridge+) */
 
-static int blt_ring_flush(struct intel_ring_buffer *ring,
-			  u32 invalidate, u32 flush)
+static int gen6_ring_flush(struct intel_ring_buffer *ring,
+			   u32 invalidate, u32 flush)
 {
 	uint32_t cmd;
 	int ret;
@@ -1640,7 +1640,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		/* gen6 bsd needs a special wa for tail updates */
 		if (IS_GEN6(dev))
 			ring->write_tail = gen6_bsd_ring_write_tail;
-		ring->flush = gen6_ring_flush;
+		ring->flush = gen6_bsd_ring_flush;
 		ring->add_request = gen6_add_request;
 		ring->get_seqno = gen6_ring_get_seqno;
 		ring->irq_enable_mask = GEN6_BSD_USER_INTERRUPT;
@@ -1688,7 +1688,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 
 	ring->mmio_base = BLT_RING_BASE;
 	ring->write_tail = ring_write_tail;
-	ring->flush = blt_ring_flush;
+	ring->flush = gen6_ring_flush;
 	ring->add_request = gen6_add_request;
 	ring->get_seqno = gen6_ring_get_seqno;
 	ring->irq_enable_mask = GEN6_BLITTER_USER_INTERRUPT;
-- 
1.8.0

[PATCH 06/18] drm/i915: add HAS_VEBOX

[Ben Widawsky]

From: "Xiang, Haihao" <haihao.xiang@intel.com>

The flag will be useful to help share code between IVB, and HSW as the
programming is similar in many places with this as one of the major
differences.

Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
[Commit message + small fix by]
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c | 2 +-
 drivers/gpu/drm/i915/i915_drv.c | 2 ++
 drivers/gpu/drm/i915/i915_drv.h | 3 +++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 1eea5be..80511fc 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1442,7 +1442,7 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
 #define DEV_INFO_FLAG(name) info->name ? #name "," : ""
 #define DEV_INFO_SEP ,
 	DRM_DEBUG_DRIVER("i915 device info: gen=%i, pciid=0x%04x flags="
-			 "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
+			 "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
 			 info->gen,
 			 dev_priv->dev->pdev->device,
 			 DEV_INFO_FLAGS);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f8ba5fe..106becf5c 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -287,6 +287,7 @@ static const struct intel_device_info intel_haswell_d_info = {
 	.has_blt_ring = 1,
 	.has_llc = 1,
 	.has_force_wake = 1,
+	.has_vebox_ring = 1,
 };
 
 static const struct intel_device_info intel_haswell_m_info = {
@@ -296,6 +297,7 @@ static const struct intel_device_info intel_haswell_m_info = {
 	.has_blt_ring = 1,
 	.has_llc = 1,
 	.has_force_wake = 1,
+	.has_vebox_ring = 1,
 };
 
 static const struct pci_device_id pciidlist[] = {		/* aka */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f316916..c103b9d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -311,6 +311,7 @@ struct drm_i915_gt_funcs {
 	DEV_INFO_FLAG(supports_tv) DEV_INFO_SEP \
 	DEV_INFO_FLAG(has_bsd_ring) DEV_INFO_SEP \
 	DEV_INFO_FLAG(has_blt_ring) DEV_INFO_SEP \
+	DEV_INFO_FLAG(has_vebox_ring) DEV_INFO_SEP \
 	DEV_INFO_FLAG(has_llc)
 
 struct intel_device_info {
@@ -339,6 +340,7 @@ struct intel_device_info {
 	u8 has_bsd_ring:1;
 	u8 has_blt_ring:1;
 	u8 has_llc:1;
+	u8 has_vebox_ring:1;
 };
 
 #define I915_PPGTT_PD_ENTRIES 512
@@ -1177,6 +1179,7 @@ struct drm_i915_file_private {
 
 #define HAS_BSD(dev)            (INTEL_INFO(dev)->has_bsd_ring)
 #define HAS_BLT(dev)            (INTEL_INFO(dev)->has_blt_ring)
+#define HAS_VEBOX(dev)          (INTEL_INFO(dev)->has_vebox_ring)
 #define HAS_LLC(dev)            (INTEL_INFO(dev)->has_llc)
 #define I915_NEED_GFX_HWS(dev)	(INTEL_INFO(dev)->need_gfx_hws)
 
-- 
1.8.0

[PATCH 07/18] drm/i915: Vebox ringbuffer init

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem.c         |  8 ++++++++
 drivers/gpu/drm/i915/i915_reg.h         |  1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 34 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cdcf19d..2ebaa6f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3923,6 +3923,12 @@ i915_gem_init_hw(struct drm_device *dev)
 			goto cleanup_bsd_ring;
 	}
 
+	if (HAS_VEBOX(dev)) {
+		ret = intel_init_vebox_ring_buffer(dev);
+		if (ret)
+			goto cleanup_vebox_ring;
+	}
+
 	dev_priv->next_seqno = 1;
 
 	/*
@@ -3934,6 +3940,8 @@ i915_gem_init_hw(struct drm_device *dev)
 
 	return 0;
 
+cleanup_vebox_ring:
+	intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
 cleanup_bsd_ring:
 	intel_cleanup_ring_buffer(&dev_priv->ring[VCS]);
 cleanup_render_ring:
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 62bad81..21f5d61 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -465,6 +465,7 @@
 #define DONE_REG		0x40b0
 #define BSD_HWS_PGA_GEN7	(0x04180)
 #define BLT_HWS_PGA_GEN7	(0x04280)
+#define VEBOX_HWS_PGA_GEN7	(0x04380)
 #define RING_ACTHD(base)	((base)+0x74)
 #define RING_NOPID(base)	((base)+0x94)
 #define RING_IMR(base)		((base)+0xa8)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 11f3698..af90257 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -864,7 +864,8 @@ void intel_ring_setup_status_page(struct intel_ring_buffer *ring)
 			mmio = BSD_HWS_PGA_GEN7;
 			break;
 		case VECS:
-			BUG();
+			mmio = VEBOX_HWS_PGA_GEN7;
+			break;
 		}
 	} else if (IS_GEN6(ring->dev)) {
 		mmio = RING_HWS_PGA_GEN6(ring->mmio_base);
@@ -1709,6 +1710,37 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	return intel_init_ring_buffer(dev, ring);
 }
 
+int intel_init_vebox_ring_buffer(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = dev->dev_private;
+	struct intel_ring_buffer *ring = &dev_priv->ring[VECS];
+
+	ring->name = "video enhancement ring";
+	ring->id = VECS;
+
+	ring->mmio_base = VEBOX_RING_BASE;
+	ring->write_tail = ring_write_tail;
+	ring->flush = gen6_ring_flush;
+	ring->add_request = gen6_add_request;
+	ring->get_seqno = gen6_ring_get_seqno;
+	ring->irq_enable_mask = 0;
+	ring->irq_get = NULL;
+	ring->irq_put = NULL;
+	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
+	ring->sync_to = gen6_ring_sync;
+	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VER;
+	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_VEV;
+	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VEB;
+	ring->semaphore_register[VECS] = MI_SEMAPHORE_SYNC_INVALID;
+	ring->signal_mbox[RCS] = GEN6_RVESYNC;
+	ring->signal_mbox[VCS] = GEN6_VVESYNC;
+	ring->signal_mbox[BCS] = GEN6_BVESYNC;
+	ring->signal_mbox[VECS] = GEN6_NOSYNC;
+	ring->init = init_ring_common;
+
+	return intel_init_ring_buffer(dev, ring);
+}
+
 int
 intel_ring_flush_all_caches(struct intel_ring_buffer *ring)
 {
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 3f7530e..eb2dbd6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -213,6 +213,7 @@ int intel_ring_invalidate_all_caches(struct intel_ring_buffer *ring);
 int intel_init_render_ring_buffer(struct drm_device *dev);
 int intel_init_bsd_ring_buffer(struct drm_device *dev);
 int intel_init_blt_ring_buffer(struct drm_device *dev);
+int intel_init_vebox_ring_buffer(struct drm_device *dev);
 
 u32 intel_ring_get_active_head(struct intel_ring_buffer *ring);
 void intel_ring_setup_status_page(struct intel_ring_buffer *ring);
-- 
1.8.0

[PATCH 08/18] drm/i915: Create a more generic pm handler for hsw+

[Ben Widawsky]

HSW has some special requirements for the VEBOX. Splitting out the
interrupt handler will make the code a bit nicer and less error prone
when we begin to handle those.

The slight functional change in this patch (queueing work while holding
the spinlock) is intentional as it makes a subsequent patch a bit nicer.
The change should also only effect HSW platforms.

Based on patches from:
CC: Haihao Xiang <haihao.xiang@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2604867..d5aef6c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -500,6 +500,7 @@ static void snb_gt_irq_handler(struct drm_device *dev,
 		ivybridge_handle_parity_error(dev);
 }
 
+/* Legacy way of handling PM interrupts */
 static void gen6_queue_rps_work(struct drm_i915_private *dev_priv,
 				u32 pm_iir)
 {
@@ -524,6 +525,31 @@ static void gen6_queue_rps_work(struct drm_i915_private *dev_priv,
 	queue_work(dev_priv->wq, &dev_priv->rps.work);
 }
 
+/* Unlike gen6_queue_rps_work() from which this function is originally derived,
+ * we must be able to deal with other PM interrupts. This is complicated because
+ * of the way in which we use the masks to defer the RPS work (which for
+ * posterity is necessary because of forcewake).
+ */
+static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv,
+			       u32 pm_iir)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev_priv->rps.lock, flags);
+	dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS;
+	if (dev_priv->rps.pm_iir) {
+		I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir);
+		/* We never want to mask useful interrupts. (also posting read) */
+		WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_DEFERRED_EVENTS);
+		/* TODO: if queue_work is slow, move it out of the spinlock */
+		queue_work(dev_priv->wq, &dev_priv->rps.work);
+	}
+	spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
+
+	if (pm_iir & ~GEN6_PM_DEFERRED_EVENTS)
+		DRM_ERROR("Unexpected PM interrupted\n");
+}
+
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
@@ -731,7 +757,9 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 
 	pm_iir = I915_READ(GEN6_PMIIR);
 	if (pm_iir) {
-		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
+		if (IS_HASWELL(dev))
+			hsw_pm_irq_handler(dev_priv, pm_iir);
+		else if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 		I915_WRITE(GEN6_PMIIR, pm_iir);
 		ret = IRQ_HANDLED;
-- 
1.8.0

[PATCH 09/18] drm/i915: make PM interrupt writes non-destructive

[Ben Widawsky]

PM interrupts have an expanded role on HSW. It helps route the EBOX
interrupts. This patch is necessary to make the existing code which
touches the mask, and enable registers more friendly to other code paths
that also will need these registers.

v2: Shouldn't destroy PMIIR or PMIMR VEBOX interrupt state in
enable/disable rps functions (Haihao)

CC: Xiang, Haihao <haihao.xiang@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c | 17 +++++++++--------
 drivers/gpu/drm/i915/i915_reg.h |  2 +-
 drivers/gpu/drm/i915/intel_pm.c |  8 ++++----
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d5aef6c..5533e55 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -372,10 +372,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	pm_iir = dev_priv->rps.pm_iir;
 	dev_priv->rps.pm_iir = 0;
 	pm_imr = I915_READ(GEN6_PMIMR);
-	I915_WRITE(GEN6_PMIMR, 0);
+	/* Make sure not to corrupt PMIMR state used by ringbuffer code */
+	I915_WRITE(GEN6_PMIMR, pm_imr & ~GEN6_PM_RPS_EVENTS);
 	spin_unlock_irq(&dev_priv->rps.lock);
 
-	if ((pm_iir & GEN6_PM_DEFERRED_EVENTS) == 0)
+	if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
 		return;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
@@ -536,17 +537,17 @@ static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv,
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev_priv->rps.lock, flags);
-	dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS;
+	dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS;
 	if (dev_priv->rps.pm_iir) {
 		I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir);
 		/* We never want to mask useful interrupts. (also posting read) */
-		WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_DEFERRED_EVENTS);
+		WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
 		/* TODO: if queue_work is slow, move it out of the spinlock */
 		queue_work(dev_priv->wq, &dev_priv->rps.work);
 	}
 	spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
 
-	if (pm_iir & ~GEN6_PM_DEFERRED_EVENTS)
+	if (pm_iir & ~GEN6_PM_RPS_EVENTS)
 		DRM_ERROR("Unexpected PM interrupted\n");
 }
 
@@ -619,7 +620,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
 			blc_event = true;
 
-		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
+		if (pm_iir & GEN6_PM_RPS_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 
 		I915_WRITE(GTIIR, gt_iir);
@@ -759,7 +760,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	if (pm_iir) {
 		if (IS_HASWELL(dev))
 			hsw_pm_irq_handler(dev_priv, pm_iir);
-		else if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
+		else if (pm_iir & GEN6_PM_RPS_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 		I915_WRITE(GEN6_PMIIR, pm_iir);
 		ret = IRQ_HANDLED;
@@ -841,7 +842,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	if (IS_GEN5(dev) &&  de_iir & DE_PCU_EVENT)
 		ironlake_handle_rps_change(dev);
 
-	if (IS_GEN6(dev) && pm_iir & GEN6_PM_DEFERRED_EVENTS)
+	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
 		gen6_queue_rps_work(dev_priv, pm_iir);
 
 	/* should clear PCH hotplug event before clear CPU irq */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 21f5d61..70f9ad7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4269,7 +4269,7 @@
 #define  GEN6_PM_RP_DOWN_THRESHOLD		(1<<4)
 #define  GEN6_PM_RP_UP_EI_EXPIRED		(1<<2)
 #define  GEN6_PM_RP_DOWN_EI_EXPIRED		(1<<1)
-#define  GEN6_PM_DEFERRED_EVENTS		(GEN6_PM_RP_UP_THRESHOLD | \
+#define  GEN6_PM_RPS_EVENTS			(GEN6_PM_RP_UP_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8f15616..5477454 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2354,7 +2354,7 @@ static void gen6_disable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
 	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
-	I915_WRITE(GEN6_PMIER, 0);
+	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) & ~GEN6_PM_RPS_EVENTS);
 	/* Complete PM interrupt masking here doesn't race with the rps work
 	 * item again unmasking PM interrupts because that is using a different
 	 * register (PMIMR) to mask PM interrupts. The only risk is in leaving
@@ -2364,7 +2364,7 @@ static void gen6_disable_rps(struct drm_device *dev)
 	dev_priv->rps.pm_iir = 0;
 	spin_unlock_irq(&dev_priv->rps.lock);
 
-	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & ~GEN6_PM_RPS_EVENTS);
 }
 
 int intel_enable_rc6(const struct drm_device *dev)
@@ -2518,10 +2518,10 @@ 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, GEN6_PM_DEFERRED_EVENTS);
+	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
 	spin_lock_irq(&dev_priv->rps.lock);
 	WARN_ON(dev_priv->rps.pm_iir != 0);
-	I915_WRITE(GEN6_PMIMR, 0);
+	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & GEN6_PM_RPS_EVENTS);
 	spin_unlock_irq(&dev_priv->rps.lock);
 	/* enable all PM interrupts */
 	I915_WRITE(GEN6_PMINTRMSK, 0);
-- 
1.8.0

[PATCH 10/18] drm/i915: Create an ivybridge_irq_preinstall

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5533e55..d882d41 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1810,6 +1810,31 @@ static void ironlake_irq_preinstall(struct drm_device *dev)
 	POSTING_READ(SDEIER);
 }
 
+static void ivybridge_irq_preinstall(struct drm_device *dev)
+{
+	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
+
+	atomic_set(&dev_priv->irq_received, 0);
+
+	I915_WRITE(HWSTAM, 0xeffe);
+
+	/* XXX hotplug from PCH */
+
+	I915_WRITE(DEIMR, 0xffffffff);
+	I915_WRITE(DEIER, 0x0);
+	POSTING_READ(DEIER);
+
+	/* and GT */
+	I915_WRITE(GTIMR, 0xffffffff);
+	I915_WRITE(GTIER, 0x0);
+	POSTING_READ(GTIER);
+
+	/* south display irq */
+	I915_WRITE(SDEIMR, 0xffffffff);
+	I915_WRITE(SDEIER, 0x0);
+	POSTING_READ(SDEIER);
+}
+
 static void valleyview_irq_preinstall(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
@@ -2733,9 +2758,8 @@ void intel_irq_init(struct drm_device *dev)
 		dev->driver->enable_vblank = valleyview_enable_vblank;
 		dev->driver->disable_vblank = valleyview_disable_vblank;
 	} else if (IS_IVYBRIDGE(dev)) {
-		/* Share pre & uninstall handlers with ILK/SNB */
 		dev->driver->irq_handler = ivybridge_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_preinstall;
+		dev->driver->irq_preinstall = ivybridge_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
 		dev->driver->enable_vblank = ivybridge_enable_vblank;
@@ -2743,7 +2767,7 @@ void intel_irq_init(struct drm_device *dev)
 	} else if (IS_HASWELL(dev)) {
 		/* Share interrupts handling with IVB */
 		dev->driver->irq_handler = ivybridge_irq_handler;
-		dev->driver->irq_preinstall = ironlake_irq_preinstall;
+		dev->driver->irq_preinstall = ivybridge_irq_preinstall;
 		dev->driver->irq_postinstall = ivybridge_irq_postinstall;
 		dev->driver->irq_uninstall = ironlake_irq_uninstall;
 		dev->driver->enable_vblank = ivybridge_enable_vblank;
-- 
1.8.0

[PATCH 11/18] drm/i915: Add PM regs to pre install

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d882d41..4ac4522 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1833,6 +1833,11 @@ static void ivybridge_irq_preinstall(struct drm_device *dev)
 	I915_WRITE(SDEIMR, 0xffffffff);
 	I915_WRITE(SDEIER, 0x0);
 	POSTING_READ(SDEIER);
+
+	/* Power management */
+	I915_WRITE(GEN6_PMIMR, 0xffffffff);
+	I915_WRITE(GEN6_PMIER, 0x0);
+	POSTING_READ(GEN6_PMIER);
 }
 
 static void valleyview_irq_preinstall(struct drm_device *dev)
-- 
1.8.0

[PATCH 12/18] drm/i915: Convert irq_refounct to struct

[Ben Widawsky]

It's overkill on older gens, but it's useful for newer gens.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 16 ++++++++--------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  4 +++-
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index af90257..9cfe633 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -743,7 +743,7 @@ gen5_ring_get_irq(struct intel_ring_buffer *ring)
 		return false;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (ring->irq_refcount++ == 0) {
+	if (ring->irq_refcount.gt++ == 0) {
 		dev_priv->gt_irq_mask &= ~ring->irq_enable_mask;
 		I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
 		POSTING_READ(GTIMR);
@@ -761,7 +761,7 @@ gen5_ring_put_irq(struct intel_ring_buffer *ring)
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (--ring->irq_refcount == 0) {
+	if (--ring->irq_refcount.gt == 0) {
 		dev_priv->gt_irq_mask |= ring->irq_enable_mask;
 		I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
 		POSTING_READ(GTIMR);
@@ -780,7 +780,7 @@ i9xx_ring_get_irq(struct intel_ring_buffer *ring)
 		return false;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (ring->irq_refcount++ == 0) {
+	if (ring->irq_refcount.gt++ == 0) {
 		dev_priv->irq_mask &= ~ring->irq_enable_mask;
 		I915_WRITE(IMR, dev_priv->irq_mask);
 		POSTING_READ(IMR);
@@ -798,7 +798,7 @@ i9xx_ring_put_irq(struct intel_ring_buffer *ring)
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (--ring->irq_refcount == 0) {
+	if (--ring->irq_refcount.gt == 0) {
 		dev_priv->irq_mask |= ring->irq_enable_mask;
 		I915_WRITE(IMR, dev_priv->irq_mask);
 		POSTING_READ(IMR);
@@ -817,7 +817,7 @@ i8xx_ring_get_irq(struct intel_ring_buffer *ring)
 		return false;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (ring->irq_refcount++ == 0) {
+	if (ring->irq_refcount.gt++ == 0) {
 		dev_priv->irq_mask &= ~ring->irq_enable_mask;
 		I915_WRITE16(IMR, dev_priv->irq_mask);
 		POSTING_READ16(IMR);
@@ -835,7 +835,7 @@ i8xx_ring_put_irq(struct intel_ring_buffer *ring)
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (--ring->irq_refcount == 0) {
+	if (--ring->irq_refcount.gt == 0) {
 		dev_priv->irq_mask |= ring->irq_enable_mask;
 		I915_WRITE16(IMR, dev_priv->irq_mask);
 		POSTING_READ16(IMR);
@@ -933,7 +933,7 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring)
 	gen6_gt_force_wake_get(dev_priv);
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (ring->irq_refcount++ == 0) {
+	if (ring->irq_refcount.gt++ == 0) {
 		if (HAS_L3_GPU_CACHE(dev) && ring->id == RCS)
 			I915_WRITE_IMR(ring, ~(ring->irq_enable_mask |
 						GEN6_RENDER_L3_PARITY_ERROR));
@@ -956,7 +956,7 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring)
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	if (--ring->irq_refcount == 0) {
+	if (--ring->irq_refcount.gt == 0) {
 		if (HAS_L3_GPU_CACHE(dev) && ring->id == RCS)
 			I915_WRITE_IMR(ring, ~GEN6_RENDER_L3_PARITY_ERROR);
 		else
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index eb2dbd6..b2fe5b4 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -57,7 +57,9 @@ struct  intel_ring_buffer {
 	 */
 	u32		last_retired_head;
 
-	u32		irq_refcount;		/* protected by dev_priv->irq_lock */
+	struct {
+		u32	gt;
+	} irq_refcount;	/* protected by dev_priv->irq_lock */
 	u32		irq_enable_mask;	/* bitmask to enable ring interrupt */
 	u32		trace_irq_seqno;
 	u32		sync_seqno[I915_NUM_RINGS-1];
-- 
1.8.0

[PATCH 13/18] drm/i915: consolidate interrupt naming scheme

[Ben Widawsky]

The motivation here is we're going to add some new interrupt definitions
and handling outside of the GT interrupts which is all we've managed so
far (with some RPS exceptions). By consolidating the names in the future
we can make thing a bit cleaner as we don't need to define register
names twice, and we can leverage pretty decent overlap in HW registers
since ILK.

To explain briefly what is in the comments: there are two sets of
interrupt masking/enabling registers. At least so far, the definitions
of the two sets overlap. The old code setup distinct names for
interrupts in each set, ie. one for global, and one for ring. This made
things confusing when using the wrong defines in the wrong places.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c         |  58 +++++++++---------
 drivers/gpu/drm/i915/i915_reg.h         | 101 ++++++++++++++------------------
 drivers/gpu/drm/i915/intel_ringbuffer.c |  17 +++---
 3 files changed, 79 insertions(+), 97 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4ac4522..d3838c6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -438,7 +438,7 @@ static void ivybridge_parity_work(struct work_struct *work)
 	I915_WRITE(GEN7_MISCCPCTL, misccpctl);
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	dev_priv->gt_irq_mask &= ~GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
+	dev_priv->gt_irq_mask &= ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
 
@@ -470,7 +470,7 @@ static void ivybridge_handle_parity_error(struct drm_device *dev)
 		return;
 
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
-	dev_priv->gt_irq_mask |= GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
+	dev_priv->gt_irq_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
 	spin_unlock_irqrestore(&dev_priv->irq_lock, flags);
 
@@ -482,22 +482,21 @@ static void snb_gt_irq_handler(struct drm_device *dev,
 			       u32 gt_iir)
 {
 
-	if (gt_iir & (GEN6_RENDER_USER_INTERRUPT |
-		      GEN6_RENDER_PIPE_CONTROL_NOTIFY_INTERRUPT))
+	if (gt_iir & (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
 		notify_ring(dev, &dev_priv->ring[RCS]);
-	if (gt_iir & GEN6_BSD_USER_INTERRUPT)
+	if (gt_iir & GT_BSD_USER_INTERRUPT)
 		notify_ring(dev, &dev_priv->ring[VCS]);
-	if (gt_iir & GEN6_BLITTER_USER_INTERRUPT)
+	if (gt_iir & GT_BLT_USER_INTERRUPT)
 		notify_ring(dev, &dev_priv->ring[BCS]);
 
-	if (gt_iir & (GT_GEN6_BLT_CS_ERROR_INTERRUPT |
-		      GT_GEN6_BSD_CS_ERROR_INTERRUPT |
-		      GT_RENDER_CS_ERROR_INTERRUPT)) {
+	if (gt_iir & (GT_BLT_CS_ERROR_INTERRUPT |
+		      GT_BSD_CS_ERROR_INTERRUPT |
+		      GT_RENDER_MASTER_ERROR_INTERRUPT)) {
 		DRM_ERROR("GT error interrupt 0x%08x\n", gt_iir);
 		i915_handle_error(dev, false);
 	}
 
-	if (gt_iir & GT_GEN7_L3_PARITY_ERROR_INTERRUPT)
+	if (gt_iir & GT_RENDER_L3_PARITY_ERROR_INTERRUPT)
 		ivybridge_handle_parity_error(dev);
 }
 
@@ -776,9 +775,9 @@ static void ilk_gt_irq_handler(struct drm_device *dev,
 			       struct drm_i915_private *dev_priv,
 			       u32 gt_iir)
 {
-	if (gt_iir & (GT_USER_INTERRUPT | GT_PIPE_NOTIFY))
+	if (gt_iir & (GT_RENDER_USER_INTERRUPT | GT_RENDER_PIPECTL_NOTIFY_INTERRUPT))
 		notify_ring(dev, &dev_priv->ring[RCS]);
-	if (gt_iir & GT_BSD_USER_INTERRUPT)
+	if (gt_iir & ILK_BSD_USER_INTERRUPT)
 		notify_ring(dev, &dev_priv->ring[VCS]);
 }
 
@@ -1898,7 +1897,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 	/* enable kind of interrupts always enabled */
 	u32 display_mask = DE_MASTER_IRQ_CONTROL | DE_GSE | DE_PCH_EVENT |
 			   DE_PLANEA_FLIP_DONE | DE_PLANEB_FLIP_DONE;
-	u32 render_irqs;
+	u32 gt_irqs;
 	u32 hotplug_mask;
 
 	dev_priv->irq_mask = ~display_mask;
@@ -1914,17 +1913,14 @@ static int ironlake_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(GTIIR, I915_READ(GTIIR));
 	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
 
+	gt_irqs = GT_RENDER_USER_INTERRUPT;
+
 	if (IS_GEN6(dev))
-		render_irqs =
-			GT_USER_INTERRUPT |
-			GEN6_BSD_USER_INTERRUPT |
-			GEN6_BLITTER_USER_INTERRUPT;
+		gt_irqs |= GT_BLT_USER_INTERRUPT | GT_BSD_USER_INTERRUPT;
 	else
-		render_irqs =
-			GT_USER_INTERRUPT |
-			GT_PIPE_NOTIFY |
-			GT_BSD_USER_INTERRUPT;
-	I915_WRITE(GTIER, render_irqs);
+		gt_irqs |= GT_RENDER_PIPECTL_NOTIFY_INTERRUPT | ILK_BSD_USER_INTERRUPT;
+
+	I915_WRITE(GTIER, gt_irqs);
 	POSTING_READ(GTIER);
 
 	if (HAS_PCH_CPT(dev)) {
@@ -1968,7 +1964,7 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 		DE_PLANEC_FLIP_DONE_IVB |
 		DE_PLANEB_FLIP_DONE_IVB |
 		DE_PLANEA_FLIP_DONE_IVB;
-	u32 render_irqs;
+	u32 gt_irqs;
 	u32 hotplug_mask;
 
 	dev_priv->irq_mask = ~display_mask;
@@ -1983,14 +1979,14 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 		   DE_PIPEA_VBLANK_IVB);
 	POSTING_READ(DEIER);
 
-	dev_priv->gt_irq_mask = ~GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
+	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);
 
-	render_irqs = GT_USER_INTERRUPT | GEN6_BSD_USER_INTERRUPT |
-		GEN6_BLITTER_USER_INTERRUPT | GT_GEN7_L3_PARITY_ERROR_INTERRUPT;
-	I915_WRITE(GTIER, render_irqs);
+	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);
 
 	hotplug_mask = (SDE_CRT_HOTPLUG_CPT |
@@ -2012,10 +2008,10 @@ 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 hotplug_en = I915_READ(PORT_HOTPLUG_EN);
 	u32 pipestat_enable = PLANE_FLIP_DONE_INT_EN_VLV;
-	u32 render_irqs;
 	u16 msid;
 
 	enable_mask = I915_DISPLAY_PORT_INTERRUPT;
@@ -2058,9 +2054,9 @@ static int valleyview_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(GTIIR, I915_READ(GTIIR));
 	I915_WRITE(GTIMR, dev_priv->gt_irq_mask);
 
-	render_irqs = GT_USER_INTERRUPT | GEN6_BSD_USER_INTERRUPT |
-		GEN6_BLITTER_USER_INTERRUPT;
-	I915_WRITE(GTIER, render_irqs);
+	gt_irqs = GT_RENDER_USER_INTERRUPT | GT_BSD_USER_INTERRUPT |
+		GT_BLT_USER_INTERRUPT;
+	I915_WRITE(GTIER, gt_irqs);
 	POSTING_READ(GTIER);
 
 	/* ack & enable invalid PTE error interrupts */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 70f9ad7..b83928d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -574,24 +574,6 @@
 #define VLV_IIR		0x1820a4
 #define VLV_IMR		0x1820a8
 #define VLV_ISR		0x1820ac
-#define   I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
-#define   I915_DISPLAY_PORT_INTERRUPT			(1<<17)
-#define   I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
-#define   I915_GMCH_THERMAL_SENSOR_EVENT_INTERRUPT	(1<<14) /* p-state */
-#define   I915_HWB_OOM_INTERRUPT			(1<<13)
-#define   I915_SYNC_STATUS_INTERRUPT			(1<<12)
-#define   I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT	(1<<11)
-#define   I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT	(1<<10)
-#define   I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT	(1<<9)
-#define   I915_DISPLAY_PLANE_C_FLIP_PENDING_INTERRUPT	(1<<8)
-#define   I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT		(1<<7)
-#define   I915_DISPLAY_PIPE_A_EVENT_INTERRUPT		(1<<6)
-#define   I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT		(1<<5)
-#define   I915_DISPLAY_PIPE_B_EVENT_INTERRUPT		(1<<4)
-#define   I915_DEBUG_INTERRUPT				(1<<2)
-#define   I915_USER_INTERRUPT				(1<<1)
-#define   I915_ASLE_INTERRUPT				(1<<0)
-#define   I915_BSD_USER_INTERRUPT                      (1<<25)
 #define EIR		0x020b0
 #define EMR		0x020b4
 #define ESR		0x020b8
@@ -702,28 +684,6 @@
 #define CACHE_MODE_1		0x7004 /* IVB+ */
 #define   PIXEL_SUBSPAN_COLLECT_OPT_DISABLE (1<<6)
 
-/* GEN6 interrupt control
- * Note that the per-ring interrupt bits do alias with the global interrupt bits
- * in GTIMR. */
-#define GEN6_RENDER_HWSTAM	0x2098
-#define GEN6_RENDER_IMR		0x20a8
-#define   GEN6_RENDER_CONTEXT_SWITCH_INTERRUPT		(1 << 8)
-#define   GEN6_RENDER_PPGTT_PAGE_FAULT			(1 << 7)
-#define   GEN6_RENDER_TIMEOUT_COUNTER_EXPIRED		(1 << 6)
-#define   GEN6_RENDER_L3_PARITY_ERROR			(1 << 5)
-#define   GEN6_RENDER_PIPE_CONTROL_NOTIFY_INTERRUPT	(1 << 4)
-#define   GEN6_RENDER_COMMAND_PARSER_MASTER_ERROR	(1 << 3)
-#define   GEN6_RENDER_SYNC_STATUS			(1 << 2)
-#define   GEN6_RENDER_DEBUG_INTERRUPT			(1 << 1)
-#define   GEN6_RENDER_USER_INTERRUPT			(1 << 0)
-
-#define GEN6_BLITTER_HWSTAM	0x22098
-#define GEN6_BLITTER_IMR	0x220a8
-#define   GEN6_BLITTER_MI_FLUSH_DW_NOTIFY_INTERRUPT	(1 << 26)
-#define   GEN6_BLITTER_COMMAND_PARSER_MASTER_ERROR	(1 << 25)
-#define   GEN6_BLITTER_SYNC_STATUS			(1 << 24)
-#define   GEN6_BLITTER_USER_INTERRUPT			(1 << 22)
-
 #define GEN6_BLITTER_ECOSKPD	0x221d0
 #define   GEN6_BLITTER_LOCK_SHIFT			16
 #define   GEN6_BLITTER_FBC_NOTIFY			(1<<3)
@@ -734,9 +694,49 @@
 #define   GEN6_BSD_SLEEP_INDICATOR	(1 << 3)
 #define   GEN6_BSD_GO_INDICATOR		(1 << 4)
 
-#define GEN6_BSD_HWSTAM			0x12098
-#define GEN6_BSD_IMR			0x120a8
-#define   GEN6_BSD_USER_INTERRUPT	(1 << 12)
+/* On modern GEN architectures interrupt control consists of two sets
+ * of registers. The first set pertains to the ring generating the
+ * interrupt. The second control is for the functional block generating the
+ * interrupt. These are PM, GT, DE, etc.
+ *
+ * Luckily *knocks on wood* all the ring interrupt bits match up with the
+ * GT interrupt bits, so we don't need to duplicate the defines.
+ *
+ * These defines should cover us well from SNB->HSW with minor exceptions
+ * it can also work on ILK.
+ */
+#define GT_BLT_FLUSHDW_NOTIFY_INTERRUPT		(1 << 26)
+#define GT_BLT_CS_ERROR_INTERRUPT		(1 << 25)
+#define GT_BLT_USER_INTERRUPT			(1 << 22)
+#define GT_BSD_CS_ERROR_INTERRUPT		(1 << 15)
+#define GT_BSD_USER_INTERRUPT			(1 << 12)
+#define GT_RENDER_L3_PARITY_ERROR_INTERRUPT	(1 <<  5) /* !snb */
+#define GT_RENDER_PIPECTL_NOTIFY_INTERRUPT	(1 <<  4)
+#define GT_RENDER_MASTER_ERROR_INTERRUPT	(1 <<  3)
+#define GT_RENDER_SYNC_STATUS_INTERRUPT		(1 <<  2)
+#define GT_RENDER_DEBUG_INTERRUPT		(1 <<  1)
+#define GT_RENDER_USER_INTERRUPT		(1 <<  0)
+
+/* These are all the "old" interrupts */
+#define ILK_BSD_USER_INTERRUPT				(1<<5)
+#define I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
+#define I915_DISPLAY_PORT_INTERRUPT			(1<<17)
+#define I915_RENDER_COMMAND_PARSER_ERROR_INTERRUPT	(1<<15)
+#define I915_GMCH_THERMAL_SENSOR_EVENT_INTERRUPT	(1<<14) /* p-state */
+#define I915_HWB_OOM_INTERRUPT				(1<<13)
+#define I915_SYNC_STATUS_INTERRUPT			(1<<12)
+#define I915_DISPLAY_PLANE_A_FLIP_PENDING_INTERRUPT	(1<<11)
+#define I915_DISPLAY_PLANE_B_FLIP_PENDING_INTERRUPT	(1<<10)
+#define I915_OVERLAY_PLANE_FLIP_PENDING_INTERRUPT	(1<<9)
+#define I915_DISPLAY_PLANE_C_FLIP_PENDING_INTERRUPT	(1<<8)
+#define I915_DISPLAY_PIPE_A_VBLANK_INTERRUPT		(1<<7)
+#define I915_DISPLAY_PIPE_A_EVENT_INTERRUPT		(1<<6)
+#define I915_DISPLAY_PIPE_B_VBLANK_INTERRUPT		(1<<5)
+#define I915_DISPLAY_PIPE_B_EVENT_INTERRUPT		(1<<4)
+#define I915_DEBUG_INTERRUPT				(1<<2)
+#define I915_USER_INTERRUPT				(1<<1)
+#define I915_ASLE_INTERRUPT				(1<<0)
+#define I915_BSD_USER_INTERRUPT				(1 << 25)
 
 #define GEN6_BSD_RNCID			0x12198
 
@@ -3431,21 +3431,6 @@
 #define DEIIR   0x44008
 #define DEIER   0x4400c
 
-/* GT interrupt.
- * Note that for gen6+ the ring-specific interrupt bits do alias with the
- * corresponding bits in the per-ring interrupt control registers. */
-#define GT_GEN6_BLT_FLUSHDW_NOTIFY_INTERRUPT	(1 << 26)
-#define GT_GEN6_BLT_CS_ERROR_INTERRUPT		(1 << 25)
-#define GT_GEN6_BLT_USER_INTERRUPT		(1 << 22)
-#define GT_GEN6_BSD_CS_ERROR_INTERRUPT		(1 << 15)
-#define GT_GEN6_BSD_USER_INTERRUPT		(1 << 12)
-#define GT_BSD_USER_INTERRUPT			(1 << 5) /* ilk only */
-#define GT_GEN7_L3_PARITY_ERROR_INTERRUPT	(1 << 5)
-#define GT_PIPE_NOTIFY				(1 << 4)
-#define GT_RENDER_CS_ERROR_INTERRUPT		(1 << 3)
-#define GT_SYNC_STATUS				(1 << 2)
-#define GT_USER_INTERRUPT			(1 << 0)
-
 #define GTISR   0x44010
 #define GTIMR   0x44014
 #define GTIIR   0x44018
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 9cfe633..33b176a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -540,7 +540,7 @@ static int init_render_ring(struct intel_ring_buffer *ring)
 		I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
 
 	if (HAS_L3_GPU_CACHE(dev))
-		I915_WRITE_IMR(ring, ~GEN6_RENDER_L3_PARITY_ERROR);
+		I915_WRITE_IMR(ring, ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
 
 	return ret;
 }
@@ -936,7 +936,7 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring)
 	if (ring->irq_refcount.gt++ == 0) {
 		if (HAS_L3_GPU_CACHE(dev) && ring->id == RCS)
 			I915_WRITE_IMR(ring, ~(ring->irq_enable_mask |
-						GEN6_RENDER_L3_PARITY_ERROR));
+					       GT_RENDER_L3_PARITY_ERROR_INTERRUPT));
 		else
 			I915_WRITE_IMR(ring, ~ring->irq_enable_mask);
 		dev_priv->gt_irq_mask &= ~ring->irq_enable_mask;
@@ -958,7 +958,7 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring)
 	spin_lock_irqsave(&dev_priv->irq_lock, flags);
 	if (--ring->irq_refcount.gt == 0) {
 		if (HAS_L3_GPU_CACHE(dev) && ring->id == RCS)
-			I915_WRITE_IMR(ring, ~GEN6_RENDER_L3_PARITY_ERROR);
+			I915_WRITE_IMR(ring, ~GT_RENDER_L3_PARITY_ERROR_INTERRUPT);
 		else
 			I915_WRITE_IMR(ring, ~0);
 		dev_priv->gt_irq_mask |= ring->irq_enable_mask;
@@ -1507,7 +1507,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 			ring->flush = gen6_render_ring_flush;
 		ring->irq_get = gen6_ring_get_irq;
 		ring->irq_put = gen6_ring_put_irq;
-		ring->irq_enable_mask = GT_USER_INTERRUPT;
+		ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
 		ring->get_seqno = gen6_ring_get_seqno;
 		ring->sync_to = gen6_ring_sync;
 		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
@@ -1524,7 +1524,8 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 		ring->get_seqno = pc_render_get_seqno;
 		ring->irq_get = gen5_ring_get_irq;
 		ring->irq_put = gen5_ring_put_irq;
-		ring->irq_enable_mask = GT_USER_INTERRUPT | GT_PIPE_NOTIFY;
+		ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT |
+					GT_RENDER_PIPECTL_NOTIFY_INTERRUPT;
 	} else {
 		ring->add_request = i9xx_add_request;
 		if (INTEL_INFO(dev)->gen < 4)
@@ -1644,7 +1645,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->flush = gen6_bsd_ring_flush;
 		ring->add_request = gen6_add_request;
 		ring->get_seqno = gen6_ring_get_seqno;
-		ring->irq_enable_mask = GEN6_BSD_USER_INTERRUPT;
+		ring->irq_enable_mask = GT_BSD_USER_INTERRUPT;
 		ring->irq_get = gen6_ring_get_irq;
 		ring->irq_put = gen6_ring_put_irq;
 		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
@@ -1663,7 +1664,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 		ring->add_request = i9xx_add_request;
 		ring->get_seqno = ring_get_seqno;
 		if (IS_GEN5(dev)) {
-			ring->irq_enable_mask = GT_BSD_USER_INTERRUPT;
+			ring->irq_enable_mask = ILK_BSD_USER_INTERRUPT;
 			ring->irq_get = gen5_ring_get_irq;
 			ring->irq_put = gen5_ring_put_irq;
 		} else {
@@ -1692,7 +1693,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 	ring->flush = gen6_ring_flush;
 	ring->add_request = gen6_add_request;
 	ring->get_seqno = gen6_ring_get_seqno;
-	ring->irq_enable_mask = GEN6_BLITTER_USER_INTERRUPT;
+	ring->irq_enable_mask = GT_BLT_USER_INTERRUPT;
 	ring->irq_get = gen6_ring_get_irq;
 	ring->irq_put = gen6_ring_put_irq;
 	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
-- 
1.8.0

[PATCH 14/18] drm/i915: vebox interrupt get/put

[Ben Widawsky]

v2: Use the correct lock to protect PM interrupt regs, this was
accidentally lost from earlier (Haihao)
Fix return types (Ben)

CC: Xiang, Haihao <haihao.xiang@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 46 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |  5 ++--
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 33b176a..5f03abc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -970,6 +970,48 @@ gen6_ring_put_irq(struct intel_ring_buffer *ring)
 	gen6_gt_force_wake_put(dev_priv);
 }
 
+static bool
+hsw_vebox_get_irq(struct intel_ring_buffer *ring)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned long flags;
+
+	if (!dev->irq_enabled)
+		return false;
+
+	spin_lock_irqsave(&dev_priv->rps.lock, flags);
+	if (ring->irq_refcount.pm++ == 0) {
+		u32 pm_imr = I915_READ(GEN6_PMIMR);
+		I915_WRITE_IMR(ring, ~ring->irq_enable_mask);
+		I915_WRITE(GEN6_PMIMR, pm_imr & ~ring->irq_enable_mask);
+		POSTING_READ(GEN6_PMIMR);
+	}
+	spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
+
+	return true;
+}
+
+static void
+hsw_vebox_put_irq(struct intel_ring_buffer *ring)
+{
+	struct drm_device *dev = ring->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned long flags;
+
+	if (!dev->irq_enabled)
+		return;
+
+	spin_lock_irqsave(&dev_priv->rps.lock, flags);
+	if (--ring->irq_refcount.pm == 0) {
+		u32 pm_imr = I915_READ(GEN6_PMIMR);
+		I915_WRITE_IMR(ring, ~0);
+		I915_WRITE(GEN6_PMIMR, pm_imr | ring->irq_enable_mask);
+		POSTING_READ(GEN6_PMIMR);
+	}
+	spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
+}
+
 static int
 i965_dispatch_execbuffer(struct intel_ring_buffer *ring,
 			 u32 offset, u32 length,
@@ -1725,8 +1767,8 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	ring->add_request = gen6_add_request;
 	ring->get_seqno = gen6_ring_get_seqno;
 	ring->irq_enable_mask = 0;
-	ring->irq_get = NULL;
-	ring->irq_put = NULL;
+	ring->irq_get = hsw_vebox_get_irq;
+	ring->irq_put = hsw_vebox_put_irq;
 	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
 	ring->sync_to = gen6_ring_sync;
 	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VER;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b2fe5b4..2a2bd20 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -58,8 +58,9 @@ struct  intel_ring_buffer {
 	u32		last_retired_head;
 
 	struct {
-		u32	gt;
-	} irq_refcount;	/* protected by dev_priv->irq_lock */
+		u32	gt; /*  protected by dev_priv->irq_lock */
+		u32	pm; /*  protected by dev_priv->rps.lock (sucks) */
+	} irq_refcount;
 	u32		irq_enable_mask;	/* bitmask to enable ring interrupt */
 	u32		trace_irq_seqno;
 	u32		sync_seqno[I915_NUM_RINGS-1];
-- 
1.8.0

[PATCH 15/18] drm/i915: Enable vebox interrupts

[Ben Widawsky]

Similar to a patch originally written by:

v2: Reversed the meanings of masked and enabled (Haihao)
Made non-destructive writes in case enable/disabler rps runs first
(Haihao)

CC: Xiang, Haihao <haihao.xiang@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c         | 25 +++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_reg.h         |  3 +++
 drivers/gpu/drm/i915/intel_ringbuffer.c |  2 +-
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d3838c6..7714ca6 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -546,8 +546,15 @@ static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv,
 	}
 	spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
 
-	if (pm_iir & ~GEN6_PM_RPS_EVENTS)
-		DRM_ERROR("Unexpected PM interrupted\n");
+	if (pm_iir & ~GEN6_PM_RPS_EVENTS) {
+		if (pm_iir & PM_VEBOX_USER_INTERRUPT)
+			notify_ring(dev_priv->dev, &dev_priv->ring[VECS]);
+
+		if (pm_iir & PM_VEBOX_CS_ERROR_INTERRUPT) {
+			DRM_ERROR("PM error interrupt 0x%08x\n", pm_iir);
+			i915_handle_error(dev_priv->dev, false);
+		}
+	}
 }
 
 static irqreturn_t valleyview_irq_handler(int irq, void *arg)
@@ -2000,6 +2007,20 @@ static int ivybridge_irq_postinstall(struct drm_device *dev)
 	I915_WRITE(SDEIER, hotplug_mask);
 	POSTING_READ(SDEIER);
 
+	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+	if (HAS_VEBOX(dev)) {
+		u32 pm_irqs, pmier, pmimr;
+		pm_irqs = PM_VEBOX_USER_INTERRUPT | PM_VEBOX_CS_ERROR_INTERRUPT;
+
+		/* Our enable/disable rps functions may touch these registers so
+		 * make sure to set a known state for only the non-RPS bits. */
+		pmier = (I915_READ(GEN6_PMIER) & GEN6_PM_RPS_EVENTS) | pm_irqs;
+		pmimr = (I915_READ(GEN6_PMIMR) | ~GEN6_PM_RPS_EVENTS) & ~pm_irqs;
+		I915_WRITE(GEN6_PMIMR, pmimr);
+		I915_WRITE(GEN6_PMIER, pmier);
+	}
+	POSTING_READ(GEN6_PMIER);
+
 	ironlake_enable_pch_hotplug(dev);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b83928d..885f481 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -717,6 +717,9 @@
 #define GT_RENDER_DEBUG_INTERRUPT		(1 <<  1)
 #define GT_RENDER_USER_INTERRUPT		(1 <<  0)
 
+#define PM_VEBOX_CS_ERROR_INTERRUPT		(1 << 12) /* hsw+ */
+#define PM_VEBOX_USER_INTERRUPT			(1 << 10) /* hsw+ */
+
 /* These are all the "old" interrupts */
 #define ILK_BSD_USER_INTERRUPT				(1<<5)
 #define I915_PIPE_CONTROL_NOTIFY_INTERRUPT		(1<<18)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 5f03abc..9ec00a9 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1766,7 +1766,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
 	ring->flush = gen6_ring_flush;
 	ring->add_request = gen6_add_request;
 	ring->get_seqno = gen6_ring_get_seqno;
-	ring->irq_enable_mask = 0;
+	ring->irq_enable_mask = PM_VEBOX_USER_INTERRUPT | PM_VEBOX_CS_ERROR_INTERRUPT;
 	ring->irq_get = hsw_vebox_get_irq;
 	ring->irq_put = hsw_vebox_put_irq;
 	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
-- 
1.8.0

[PATCH 16/18] drm/i915: add VEBOX into debugfs

[Ben Widawsky]

From: "Xiang, Haihao" <haihao.xiang@intel.com>

Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
[Order changed, and modified by]
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 09cd7d7..e842dea 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -375,6 +375,17 @@ static int i915_gem_request_info(struct seq_file *m, void *data)
 		}
 		count++;
 	}
+	if (!list_empty(&dev_priv->ring[VECS].request_list)) {
+		seq_printf(m, "VEBOX requests:\n");
+		list_for_each_entry(gem_request,
+				    &dev_priv->ring[VECS].request_list,
+				    list) {
+			seq_printf(m, "    %d @ %d\n",
+				   gem_request->seqno,
+				   (int) (jiffies - gem_request->emitted_jiffies));
+		}
+		count++;
+	}
 	mutex_unlock(&dev->struct_mutex);
 
 	if (count == 0)
@@ -566,6 +577,7 @@ static const char *ring_str(int ring)
 	case RCS: return "render";
 	case VCS: return "bsd";
 	case BCS: return "blt";
+	case VECS: return "vebox";
 	default: return "";
 	}
 }
@@ -2037,6 +2049,7 @@ static struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_hws", i915_hws_info, 0, (void *)RCS},
 	{"i915_gem_hws_blt", i915_hws_info, 0, (void *)BCS},
 	{"i915_gem_hws_bsd", i915_hws_info, 0, (void *)VCS},
+	{"i915_gem_hsw_vebox", i915_hws_info, 0, (void *)VECS},
 	{"i915_rstdby_delays", i915_rstdby_delays, 0},
 	{"i915_cur_delayinfo", i915_cur_delayinfo, 0},
 	{"i915_delayfreq_table", i915_delayfreq_table, 0},
-- 
1.8.0

[PATCH 17/18] drm/i915: add I915_EXEC_VEBOX to i915_gem_do_execbuffer()

[Ben Widawsky]

From: "Xiang, Haihao" <haihao.xiang@intel.com>

A user can run batchbuffer via VEBOX ring.

Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
[Order changed by]
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 +++++++++
 include/uapi/drm/i915_drm.h                | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d80e9dd..9ac2b04 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -841,6 +841,15 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			return -EPERM;
 		}
 		break;
+	case I915_EXEC_VEBOX:
+		ring = &dev_priv->ring[VECS];
+		if (ctx_id != 0) {
+			DRM_DEBUG("Ring %s doesn't support contexts\n",
+				  ring->name);
+			return -EPERM;
+		}
+		break;
+
 	default:
 		DRM_DEBUG("execbuf with unknown ring: %d\n",
 			  (int)(args->flags & I915_EXEC_RING_MASK));
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index b746a3c..99454b3 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -653,6 +653,7 @@ struct drm_i915_gem_execbuffer2 {
 #define I915_EXEC_RENDER                 (1<<0)
 #define I915_EXEC_BSD                    (2<<0)
 #define I915_EXEC_BLT                    (3<<0)
+#define I915_EXEC_VEBOX                  (4<<0)
 
 /* Used for switching the constants addressing mode on gen4+ RENDER ring.
  * Gen6+ only supports relative addressing to dynamic state (default) and
-- 
1.8.0

[PATCH 18/18] drm/i915: add I915_PARAM_HAS_VEBOX to i915_getparam

[Ben Widawsky]

From: "Xiang, Haihao" <haihao.xiang@intel.com>

This will let userland only try to use the new ring
when the appropriate kernel is present

Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
[Order changed, and merge conflict resolved by]
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_dma.c | 3 +++
 include/uapi/drm/i915_drm.h     | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 80511fc..742c01e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -984,6 +984,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_BLT:
 		value = intel_ring_initialized(&dev_priv->ring[BCS]);
 		break;
+	case I915_PARAM_HAS_VEBOX:
+		value = intel_ring_initialized(&dev_priv->ring[VECS]);
+		break;
 	case I915_PARAM_HAS_RELAXED_FENCING:
 		value = 1;
 		break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 99454b3..a6006fd 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -305,7 +305,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_WAIT_TIMEOUT	 19
 #define I915_PARAM_HAS_SEMAPHORES	 20
 #define I915_PARAM_HAS_PRIME_VMAP_FLUSH	 21
-#define I915_PARAM_RSVD_FOR_FUTURE_USE	 22
+#define I915_PARAM_HAS_VEBOX		 22
 #define I915_PARAM_HAS_SECURE_BATCHES	 23
 
 typedef struct drm_i915_getparam {
-- 
1.8.0

Re: [PATCH 09/18] drm/i915: make PM interrupt writes non-destructive

[Chris Wilson]

On Tue,  6 Nov 2012 16:25:33 +0000, Ben Widawsky <ben@bwidawsk.net> wrote:
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8f15616..5477454 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2354,7 +2354,7 @@ static void gen6_disable_rps(struct drm_device *dev)
>  	I915_WRITE(GEN6_RC_CONTROL, 0);
>  	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
>  	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
> -	I915_WRITE(GEN6_PMIER, 0);
> +	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) & ~GEN6_PM_RPS_EVENTS);
>  	/* Complete PM interrupt masking here doesn't race with the rps work
>  	 * item again unmasking PM interrupts because that is using a different
>  	 * register (PMIMR) to mask PM interrupts. The only risk is in leaving
> @@ -2364,7 +2364,7 @@ static void gen6_disable_rps(struct drm_device *dev)
>  	dev_priv->rps.pm_iir = 0;
>  	spin_unlock_irq(&dev_priv->rps.lock);
>  
> -	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
> +	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & ~GEN6_PM_RPS_EVENTS);
>  }
>  
>  int intel_enable_rc6(const struct drm_device *dev)
> @@ -2518,10 +2518,10 @@ 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, GEN6_PM_DEFERRED_EVENTS);
> +	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
>  	spin_lock_irq(&dev_priv->rps.lock);
>  	WARN_ON(dev_priv->rps.pm_iir != 0);
> -	I915_WRITE(GEN6_PMIMR, 0);
> +	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & GEN6_PM_RPS_EVENTS);
>  	spin_unlock_irq(&dev_priv->rps.lock);
>  	/* enable all PM interrupts */
>  	I915_WRITE(GEN6_PMINTRMSK, 0);

Totally confused.

IER = IER & ~RPS // only disable the RPS events
IIR = IIR & RPS // clear the asserted RPS bits

IMR != IIR.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 09/18] drm/i915: make PM interrupt writes non-destructive

[Ben Widawsky]

On Wed, 07 Nov 2012 10:17:09 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Tue,  6 Nov 2012 16:25:33 +0000, Ben Widawsky <ben@bwidawsk.net> wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 8f15616..5477454 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -2354,7 +2354,7 @@ static void gen6_disable_rps(struct drm_device *dev)
> >  	I915_WRITE(GEN6_RC_CONTROL, 0);
> >  	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
> >  	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
> > -	I915_WRITE(GEN6_PMIER, 0);
> > +	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) & ~GEN6_PM_RPS_EVENTS);
> >  	/* Complete PM interrupt masking here doesn't race with the rps work
> >  	 * item again unmasking PM interrupts because that is using a different
> >  	 * register (PMIMR) to mask PM interrupts. The only risk is in leaving
> > @@ -2364,7 +2364,7 @@ static void gen6_disable_rps(struct drm_device *dev)
> >  	dev_priv->rps.pm_iir = 0;
> >  	spin_unlock_irq(&dev_priv->rps.lock);
> >  
> > -	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
> > +	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & ~GEN6_PM_RPS_EVENTS);
> >  }
> >  
> >  int intel_enable_rc6(const struct drm_device *dev)
> > @@ -2518,10 +2518,10 @@ 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, GEN6_PM_DEFERRED_EVENTS);
> > +	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
> >  	spin_lock_irq(&dev_priv->rps.lock);
> >  	WARN_ON(dev_priv->rps.pm_iir != 0);
> > -	I915_WRITE(GEN6_PMIMR, 0);
> > +	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & GEN6_PM_RPS_EVENTS);
> >  	spin_unlock_irq(&dev_priv->rps.lock);
> >  	/* enable all PM interrupts */
> >  	I915_WRITE(GEN6_PMINTRMSK, 0);
> 
> Totally confused.
> 
> IER = IER & ~RPS // only disable the RPS events
> IIR = IIR & RPS // clear the asserted RPS bits
> 
> IMR != IIR.
> -Chris
> 

So yes, there is a bug here I think. To make sure you aren't referring
to something else though I'll elaborate, and add this to the commit
message in v2.

At preinstall all interrupts are masked and disabled. That will only
happen at driver load time. Then the above code enable/disable rc6
happens also at resume, so a little more care is taken.

The IMR is only touched by the workqueue, so enable/disable touch IER
and IIR.

Disable (there is a bug here you found):
Disable RPS events, optionally clear IIR RPS interrupts.

+	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & ~GEN6_PM_RPS_EVENTS);
should be either nothing at all (since we clear on enable anyway), or:
+	I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);


Enable (this is okay as it stands, I think):
Enable RPS events, clear IIR RPS interrupts.

+	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
+	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) & GEN6_PM_RPS_EVENTS);

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 00/18] [RFC] Introduce the Haswell VECS

[Ben Widawsky]

The basic usage of this feature is already upstream in libva.

git://anongit.freedesktop.org/vaapi/intel-driver

On Tue,  6 Nov 2012 16:25:24 +0000
Ben Widawsky <ben@bwidawsk.net> wrote:

> The VECS is a new command streamer introduced in Haswell which allows
> offloading of video post processing to a new engine called the VEBOX.
> Like other rings, it is up to userspace to take advantage of it. Also
> like the other rings, the primary enabling is turning on interrupts,
> adding the new semaphores, and other basic bits. Somewhat more
> complicated than the other rings, the VECS has its interrupts in the
> PMIMR, so as a result, I did a bunch of interrupt naming cleanups
> because I thought it made more sense.
> 
> Coming separately will be the basic i-g-t test cases. I still have some
> work to do on those, but figure that the review can go on in parallel.
> 
> In terms of the work history, these started as patches by me, finished by
> Haihao, and then cleaned up and rebased by me. I tried to leave credit to
> Haihao where applicable, but I rewrote a lot of his stuff, so blame me if
> something is bad.
> 
> Ben Widawsky (14):
>   drm/i915: Comments for semaphore clarification
>   drm/i915: Semaphore MBOX update generalization
>   drm/i915: Introduce VECS: the 4th ring
>   drm/i915: Add VECS semaphore bits
>   drm/i915: Rename ring flush functions
>   drm/i915: Vebox ringbuffer init
>   drm/i915: Create a more generic pm handler for hsw+
>   drm/i915: make PM interrupt writes non-destructive
>   drm/i915: Create an ivybridge_irq_preinstall
>   drm/i915: Add PM regs to pre install
>   drm/i915: Convert irq_refounct to struct
>   drm/i915: consolidate interrupt naming scheme
>   drm/i915: vebox interrupt get/put
>   drm/i915: Enable vebox interrupts
> 
> Xiang, Haihao (4):
>   drm/i915: add HAS_VEBOX
>   drm/i915: add VEBOX into debugfs
>   drm/i915: add I915_EXEC_VEBOX to i915_gem_do_execbuffer()
>   drm/i915: add I915_PARAM_HAS_VEBOX to i915_getparam
> 
>  drivers/gpu/drm/i915/i915_debugfs.c        |  13 ++
>  drivers/gpu/drm/i915/i915_dma.c            |   5 +-
>  drivers/gpu/drm/i915/i915_drv.c            |   2 +
>  drivers/gpu/drm/i915/i915_drv.h            |   3 +
>  drivers/gpu/drm/i915/i915_gem.c            |   8 ++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   9 ++
>  drivers/gpu/drm/i915/i915_irq.c            | 153 +++++++++++++++++------
>  drivers/gpu/drm/i915/i915_reg.h            | 148 ++++++++++++-----------
>  drivers/gpu/drm/i915/intel_pm.c            |   8 +-
>  drivers/gpu/drm/i915/intel_ringbuffer.c    | 188 +++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |  13 +-
>  include/uapi/drm/i915_drm.h                |   3 +-
>  12 files changed, 383 insertions(+), 170 deletions(-)
> 



-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 01/18] drm/i915: Comments for semaphore clarification

[Jani Nikula]

On Tue, 06 Nov 2012, Ben Widawsky <ben@bwidawsk.net> wrote:
> Semaphores are tied very closely to the rings in the GPU. Trivial patch
> adds comments to the existing code so that when we add new rings we can
> include comments there as well. It also helps distinguish the ring to
> semaphore mailbox interactions by using the ringname in the semaphore
> data structures.
>
> This patch should have no functional impact.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> A subset of this patch was:
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++------
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +++++++++---------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
>  3 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9118bd1..f82755e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -252,12 +252,12 @@
>  #define  MI_SEMAPHORE_UPDATE	    (1<<21)
>  #define  MI_SEMAPHORE_COMPARE	    (1<<20)
>  #define  MI_SEMAPHORE_REGISTER	    (1<<18)
> -#define  MI_SEMAPHORE_SYNC_RV	    (2<<16)
> -#define  MI_SEMAPHORE_SYNC_RB	    (0<<16)
> -#define  MI_SEMAPHORE_SYNC_VR	    (0<<16)
> -#define  MI_SEMAPHORE_SYNC_VB	    (2<<16)
> -#define  MI_SEMAPHORE_SYNC_BR	    (2<<16)
> -#define  MI_SEMAPHORE_SYNC_BV	    (0<<16)
> +#define  MI_SEMAPHORE_SYNC_RB	    (0<<16) /* RCS wait for BCS  (BRSYNC) */
> +#define  MI_SEMAPHORE_SYNC_RV	    (2<<16) /* RCS wait for VCS  (VRSYNC) */
> +#define  MI_SEMAPHORE_SYNC_VR	    (0<<16) /* VCS wait for RCS  (RVSYNC) */
> +#define  MI_SEMAPHORE_SYNC_VB	    (2<<16) /* VCS wait for BCS  (BVSYNC) */
> +#define  MI_SEMAPHORE_SYNC_BV	    (0<<16) /* BCS wait for VCS  (VBSYNC) */
> +#define  MI_SEMAPHORE_SYNC_BR	    (2<<16) /* BCS wait for RCS  (RBSYNC) */
>  #define  MI_SEMAPHORE_SYNC_INVALID  (1<<0)
>  /*
>   * 3D instructions used by the kernel
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index a035ac2..423948f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1503,9 +1503,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>  		ring->irq_enable_mask = GT_USER_INTERRUPT;
>  		ring->get_seqno = gen6_ring_get_seqno;
>  		ring->sync_to = gen6_ring_sync;
> -		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID;
> -		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_RV;
> -		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_RB;
> +		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
> +		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
> +		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
>  		ring->signal_mbox[0] = GEN6_VRSYNC;
>  		ring->signal_mbox[1] = GEN6_BRSYNC;
>  	} else if (IS_GEN5(dev)) {
> @@ -1639,9 +1639,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>  		ring->irq_put = gen6_ring_put_irq;
>  		ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
>  		ring->sync_to = gen6_ring_sync;
> -		ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_VR;
> -		ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_INVALID;
> -		ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_VB;
> +		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
> +		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
> +		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
>  		ring->signal_mbox[0] = GEN6_RVSYNC;
>  		ring->signal_mbox[1] = GEN6_BVSYNC;
>  	} else {
> @@ -1684,9 +1684,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>  	ring->irq_put = gen6_ring_put_irq;
>  	ring->dispatch_execbuffer = gen6_ring_dispatch_execbuffer;
>  	ring->sync_to = gen6_ring_sync;
> -	ring->semaphore_register[0] = MI_SEMAPHORE_SYNC_BR;
> -	ring->semaphore_register[1] = MI_SEMAPHORE_SYNC_BV;
> -	ring->semaphore_register[2] = MI_SEMAPHORE_SYNC_INVALID;
> +	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
> +	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
> +	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
>  	ring->signal_mbox[0] = GEN6_RBSYNC;
>  	ring->signal_mbox[1] = GEN6_VBSYNC;
>  	ring->init = init_ring_common;
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 5af65b8..df1a0a2 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -89,7 +89,7 @@ struct  intel_ring_buffer {
>  				   struct intel_ring_buffer *to,
>  				   u32 seqno);
>  
> -	u32		semaphore_register[3]; /*our mbox written by others */
> +	u32		semaphore_register[I915_NUM_RINGS]; /*our mbox written by others */
>  	u32		signal_mbox[2]; /* mboxes this ring signals to */
>  	/**
>  	 * List of objects currently involved in rendering from the
> -- 
> 1.8.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/18] drm/i915: Semaphore MBOX update generalization

[Jani Nikula]

On Tue, 06 Nov 2012, Ben Widawsky <ben@bwidawsk.net> wrote:
> This replaces the existing MBOX update code with a more generalized
> calculation for emitting mbox updates. We also create a sentinel for
> doing the updates so we can more abstractly deal with the rings.
>
> When doing MBOX updates the code must be aware of the /other/ rings.
> Until now the platforms which supported semaphores had a fixed number of
> rings and so it made sense for the code to be very specialized
> (hardcoded).
>
> The patch does contain a functional change, but should have no
> behavioral changes.
>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  1 +
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 40 ++++++++++++++++++++-------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 +-
>  3 files changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f82755e..d220410 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -438,6 +438,7 @@
>  #define GEN6_VRSYNC (RING_SYNC_1(GEN6_BSD_RING_BASE))
>  #define GEN6_VBSYNC (RING_SYNC_0(GEN6_BSD_RING_BASE))
>  #define GEN6_BRSYNC (RING_SYNC_0(BLT_RING_BASE))
> +#define GEN6_NOSYNC 0
>  #define GEN6_BVSYNC (RING_SYNC_1(BLT_RING_BASE))
>  #define RING_MAX_IDLE(base)	((base)+0x54)
>  #define RING_HWS_PGA(base)	((base)+0x80)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 423948f..ace1176 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -558,12 +558,14 @@ update_mboxes(struct intel_ring_buffer *ring,
>  	    u32 seqno,
>  	    u32 mmio_offset)
>  {
> +#define MBOX_UPDATE_DWORDS 4
>  	intel_ring_emit(ring, MI_SEMAPHORE_MBOX |
>  			      MI_SEMAPHORE_GLOBAL_GTT |
>  			      MI_SEMAPHORE_REGISTER |
>  			      MI_SEMAPHORE_UPDATE);
>  	intel_ring_emit(ring, seqno);
>  	intel_ring_emit(ring, mmio_offset);
> +	intel_ring_emit(ring, MI_NOOP);

I take it this is the functional change you refer to in the commit
message, but the reason why is missing.

>  }
>  
>  /**
> @@ -579,21 +581,26 @@ static int
>  gen6_add_request(struct intel_ring_buffer *ring,
>  		 u32 *seqno)
>  {
> -	u32 mbox1_reg;
> -	u32 mbox2_reg;
> -	int ret;
> +	struct drm_device *dev = ring->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_ring_buffer *useless;
> +	int i, ret;
>  
> -	ret = intel_ring_begin(ring, 10);
> +	ret = intel_ring_begin(ring, ((I915_NUM_RINGS-1) *
> +				      MBOX_UPDATE_DWORDS) +
> +				      4);

My only (possibly unwarranted) worry about this patch is that this and
the for_each_ring below implicitly expect each ring->signal_mbox to have
*exactly* one GEN6_NOSYNC, and there are no comments or checks about
it. How does it blow up if that is not the case?

Other than the comments above,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>  	if (ret)
>  		return ret;
> -
> -	mbox1_reg = ring->signal_mbox[0];
> -	mbox2_reg = ring->signal_mbox[1];
> +#undef MBOX_UPDATE_DWORDS
>  
>  	*seqno = i915_gem_next_request_seqno(ring);
>  
> -	update_mboxes(ring, *seqno, mbox1_reg);
> -	update_mboxes(ring, *seqno, mbox2_reg);
> +	for_each_ring(useless, dev_priv, i) {
> +		u32 mbox_reg = ring->signal_mbox[i];
> +		if (mbox_reg != GEN6_NOSYNC)
> +			update_mboxes(ring, *seqno, mbox_reg);
> +	}
> +
>  	intel_ring_emit(ring, MI_STORE_DWORD_INDEX);
>  	intel_ring_emit(ring, I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT);
>  	intel_ring_emit(ring, *seqno);
> @@ -1506,8 +1513,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>  		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID;
>  		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV;
>  		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB;
> -		ring->signal_mbox[0] = GEN6_VRSYNC;
> -		ring->signal_mbox[1] = GEN6_BRSYNC;
> +		ring->signal_mbox[RCS] = GEN6_NOSYNC;
> +		ring->signal_mbox[VCS] = GEN6_VRSYNC;
> +		ring->signal_mbox[BCS] = GEN6_BRSYNC;
>  	} else if (IS_GEN5(dev)) {
>  		ring->add_request = pc_render_add_request;
>  		ring->flush = gen4_render_ring_flush;
> @@ -1642,8 +1650,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>  		ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR;
>  		ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID;
>  		ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB;
> -		ring->signal_mbox[0] = GEN6_RVSYNC;
> -		ring->signal_mbox[1] = GEN6_BVSYNC;
> +		ring->signal_mbox[RCS] = GEN6_RVSYNC;
> +		ring->signal_mbox[VCS] = GEN6_NOSYNC;
> +		ring->signal_mbox[BCS] = GEN6_BVSYNC;
>  	} else {
>  		ring->mmio_base = BSD_RING_BASE;
>  		ring->flush = bsd_ring_flush;
> @@ -1687,8 +1696,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>  	ring->semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR;
>  	ring->semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV;
>  	ring->semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID;
> -	ring->signal_mbox[0] = GEN6_RBSYNC;
> -	ring->signal_mbox[1] = GEN6_VBSYNC;
> +	ring->signal_mbox[RCS] = GEN6_RBSYNC;
> +	ring->signal_mbox[VCS] = GEN6_VBSYNC;
> +	ring->signal_mbox[BCS] = GEN6_NOSYNC;
>  	ring->init = init_ring_common;
>  
>  	return intel_init_ring_buffer(dev, ring);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index df1a0a2..d8eecb1 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -90,7 +90,7 @@ struct  intel_ring_buffer {
>  				   u32 seqno);
>  
>  	u32		semaphore_register[I915_NUM_RINGS]; /*our mbox written by others */
> -	u32		signal_mbox[2]; /* mboxes this ring signals to */
> +	u32		signal_mbox[I915_NUM_RINGS]; /* mboxes this ring signals to */
>  	/**
>  	 * List of objects currently involved in rendering from the
>  	 * ringbuffer.
> -- 
> 1.8.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 05/18] drm/i915: Rename ring flush functions

[Jani Nikula]

On Tue, 06 Nov 2012, Ben Widawsky <ben@bwidawsk.net> wrote:
> Historically we considered the render ring to have special flush
> semantics and everything else to fall under a more general umbrella.
> Probably by coincidence more than anything we decided to make the bsd
> ring have the default *other* flush. As the new vebox ring exposes, the
> bsd ring is actually the weird one. Doing this allows us to call
> gen6_ring_flush for the vebox because calling blt_ring_flush would be
> weird...
>
> This patch should have no functional change.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 2d0c3bb..11f3698 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1390,8 +1390,8 @@ static void gen6_bsd_ring_write_tail(struct intel_ring_buffer *ring,
>  		   _MASKED_BIT_DISABLE(GEN6_BSD_SLEEP_MSG_DISABLE));
>  }
>  
> -static int gen6_ring_flush(struct intel_ring_buffer *ring,
> -			   u32 invalidate, u32 flush)
> +static int gen6_bsd_ring_flush(struct intel_ring_buffer *ring,
> +			       u32 invalidate, u32 flush)
>  {
>  	uint32_t cmd;
>  	int ret;
> @@ -1462,8 +1462,8 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring,
>  
>  /* Blitter support (SandyBridge+) */
>  
> -static int blt_ring_flush(struct intel_ring_buffer *ring,
> -			  u32 invalidate, u32 flush)
> +static int gen6_ring_flush(struct intel_ring_buffer *ring,
> +			   u32 invalidate, u32 flush)
>  {
>  	uint32_t cmd;
>  	int ret;
> @@ -1640,7 +1640,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
>  		/* gen6 bsd needs a special wa for tail updates */
>  		if (IS_GEN6(dev))
>  			ring->write_tail = gen6_bsd_ring_write_tail;
> -		ring->flush = gen6_ring_flush;
> +		ring->flush = gen6_bsd_ring_flush;
>  		ring->add_request = gen6_add_request;
>  		ring->get_seqno = gen6_ring_get_seqno;
>  		ring->irq_enable_mask = GEN6_BSD_USER_INTERRUPT;
> @@ -1688,7 +1688,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
>  
>  	ring->mmio_base = BLT_RING_BASE;
>  	ring->write_tail = ring_write_tail;
> -	ring->flush = blt_ring_flush;
> +	ring->flush = gen6_ring_flush;
>  	ring->add_request = gen6_add_request;
>  	ring->get_seqno = gen6_ring_get_seqno;
>  	ring->irq_enable_mask = GEN6_BLITTER_USER_INTERRUPT;
> -- 
> 1.8.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 06/18] drm/i915: add HAS_VEBOX

[Jani Nikula]

On Tue, 06 Nov 2012, Ben Widawsky <ben@bwidawsk.net> wrote:
> From: "Xiang, Haihao" <haihao.xiang@intel.com>
>
> The flag will be useful to help share code between IVB, and HSW as the
> programming is similar in many places with this as one of the major
> differences.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


>
> Signed-off-by: Xiang, Haihao <haihao.xiang@intel.com>
> [Commit message + small fix by]
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 2 +-
>  drivers/gpu/drm/i915/i915_drv.c | 2 ++
>  drivers/gpu/drm/i915/i915_drv.h | 3 +++
>  3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 1eea5be..80511fc 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1442,7 +1442,7 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>  #define DEV_INFO_FLAG(name) info->name ? #name "," : ""
>  #define DEV_INFO_SEP ,
>  	DRM_DEBUG_DRIVER("i915 device info: gen=%i, pciid=0x%04x flags="
> -			 "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
> +			 "%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s",
>  			 info->gen,
>  			 dev_priv->dev->pdev->device,
>  			 DEV_INFO_FLAGS);
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f8ba5fe..106becf5c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -287,6 +287,7 @@ static const struct intel_device_info intel_haswell_d_info = {
>  	.has_blt_ring = 1,
>  	.has_llc = 1,
>  	.has_force_wake = 1,
> +	.has_vebox_ring = 1,
>  };
>  
>  static const struct intel_device_info intel_haswell_m_info = {
> @@ -296,6 +297,7 @@ static const struct intel_device_info intel_haswell_m_info = {
>  	.has_blt_ring = 1,
>  	.has_llc = 1,
>  	.has_force_wake = 1,
> +	.has_vebox_ring = 1,
>  };
>  
>  static const struct pci_device_id pciidlist[] = {		/* aka */
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f316916..c103b9d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -311,6 +311,7 @@ struct drm_i915_gt_funcs {
>  	DEV_INFO_FLAG(supports_tv) DEV_INFO_SEP \
>  	DEV_INFO_FLAG(has_bsd_ring) DEV_INFO_SEP \
>  	DEV_INFO_FLAG(has_blt_ring) DEV_INFO_SEP \
> +	DEV_INFO_FLAG(has_vebox_ring) DEV_INFO_SEP \
>  	DEV_INFO_FLAG(has_llc)
>  
>  struct intel_device_info {
> @@ -339,6 +340,7 @@ struct intel_device_info {
>  	u8 has_bsd_ring:1;
>  	u8 has_blt_ring:1;
>  	u8 has_llc:1;
> +	u8 has_vebox_ring:1;
>  };
>  
>  #define I915_PPGTT_PD_ENTRIES 512
> @@ -1177,6 +1179,7 @@ struct drm_i915_file_private {
>  
>  #define HAS_BSD(dev)            (INTEL_INFO(dev)->has_bsd_ring)
>  #define HAS_BLT(dev)            (INTEL_INFO(dev)->has_blt_ring)
> +#define HAS_VEBOX(dev)          (INTEL_INFO(dev)->has_vebox_ring)
>  #define HAS_LLC(dev)            (INTEL_INFO(dev)->has_llc)
>  #define I915_NEED_GFX_HWS(dev)	(INTEL_INFO(dev)->need_gfx_hws)
>  
> -- 
> 1.8.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/18 v3] drm/i915: make PM interrupt writes non-destructive

[Ben Widawsky]

PM interrupts have an expanded role on HSW. It helps route the EBOX
interrupts. This patch is necessary to make the existing code which
touches the mask, and enable registers more friendly to other code paths
that also will need these registers.

To be more explicit:
At preinstall all interrupts are masked and disabled. This implies that
preinstall should always happen before any enabling/disabling of RPS or
other interrupts.

The PMIMR is touched by the workqueue, so enable/disable touch IER and
IIR. Similarly, the code currently expects IMR has no use outside of the
RPS related interrupts so they unconditionally set 0, or ~0. We could
use IER in the workqueue, and IMR elsewhere, but since the workqueue
use-case is more transient the existing usage makes sense.

Disable RPS events:
IER := IER & ~GEN6_PM_RPS_EVENTS // Disable RPS related interrupts
IIR := GEN6_PM_RPS_EVENTS // Disable any outstanding interrupts

Enable RPS events:
IER := IER | GEN6_PM_RPS_EVENTS // Enable the RPS related interrupts
IIR := GEN6_PM_RPS_EVENTS // Make sure there were no leftover events
(really shouldn't happen)

v2: Shouldn't destroy PMIIR or PMIMR VEBOX interrupt state in
enable/disable rps functions (Haihao)

v3: Bug found by Chris where we were clearing the wrong bits at rps
disable.
    expanded commit message

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c | 17 +++++++++--------
 drivers/gpu/drm/i915/i915_reg.h |  2 +-
 drivers/gpu/drm/i915/intel_pm.c | 10 +++++-----
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 278c0f00..87ec18f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -372,10 +372,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	pm_iir = dev_priv->rps.pm_iir;
 	dev_priv->rps.pm_iir = 0;
 	pm_imr = I915_READ(GEN6_PMIMR);
-	I915_WRITE(GEN6_PMIMR, 0);
+	/* Make sure not to corrupt PMIMR state used by ringbuffer code */
+	I915_WRITE(GEN6_PMIMR, pm_imr & ~GEN6_PM_RPS_EVENTS);
 	spin_unlock_irq(&dev_priv->rps.lock);
 
-	if ((pm_iir & GEN6_PM_DEFERRED_EVENTS) == 0)
+	if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
 		return;
 
 	mutex_lock(&dev_priv->dev->struct_mutex);
@@ -536,17 +537,17 @@ static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv,
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev_priv->rps.lock, flags);
-	dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS;
+	dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS;
 	if (dev_priv->rps.pm_iir) {
 		I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir);
 		/* We never want to mask useful interrupts. (also posting read) */
-		WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_DEFERRED_EVENTS);
+		WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
 		/* TODO: if queue_work is slow, move it out of the spinlock */
 		queue_work(dev_priv->wq, &dev_priv->rps.work);
 	}
 	spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
 
-	if (pm_iir & ~GEN6_PM_DEFERRED_EVENTS)
+	if (pm_iir & ~GEN6_PM_RPS_EVENTS)
 		DRM_ERROR("Unexpected PM interrupted\n");
 }
 
@@ -619,7 +620,7 @@ static irqreturn_t valleyview_irq_handler(DRM_IRQ_ARGS)
 		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
 			blc_event = true;
 
-		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
+		if (pm_iir & GEN6_PM_RPS_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 
 		I915_WRITE(GTIIR, gt_iir);
@@ -755,7 +756,7 @@ static irqreturn_t ivybridge_irq_handler(DRM_IRQ_ARGS)
 	if (pm_iir) {
 		if (IS_HASWELL(dev))
 			hsw_pm_irq_handler(dev_priv, pm_iir);
-		else if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
+		else if (pm_iir & GEN6_PM_RPS_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 		I915_WRITE(GEN6_PMIIR, pm_iir);
 		ret = IRQ_HANDLED;
@@ -845,7 +846,7 @@ static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS)
 	if (IS_GEN5(dev) &&  de_iir & DE_PCU_EVENT)
 		ironlake_handle_rps_change(dev);
 
-	if (IS_GEN6(dev) && pm_iir & GEN6_PM_DEFERRED_EVENTS)
+	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
 		gen6_queue_rps_work(dev_priv, pm_iir);
 
 	/* should clear PCH hotplug event before clear CPU irq */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 81d3068..3ac4b98 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4225,7 +4225,7 @@
 #define  GEN6_PM_RP_DOWN_THRESHOLD		(1<<4)
 #define  GEN6_PM_RP_UP_EI_EXPIRED		(1<<2)
 #define  GEN6_PM_RP_DOWN_EI_EXPIRED		(1<<1)
-#define  GEN6_PM_DEFERRED_EVENTS		(GEN6_PM_RP_UP_THRESHOLD | \
+#define  GEN6_PM_RPS_EVENTS			(GEN6_PM_RP_UP_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a3e4f8b..be8560c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2354,7 +2354,7 @@ static void gen6_disable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
 	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
-	I915_WRITE(GEN6_PMIER, 0);
+	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) & ~GEN6_PM_RPS_EVENTS);
 	/* Complete PM interrupt masking here doesn't race with the rps work
 	 * item again unmasking PM interrupts because that is using a different
 	 * register (PMIMR) to mask PM interrupts. The only risk is in leaving
@@ -2364,7 +2364,7 @@ static void gen6_disable_rps(struct drm_device *dev)
 	dev_priv->rps.pm_iir = 0;
 	spin_unlock_irq(&dev_priv->rps.lock);
 
-	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+	I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
 }
 
 int intel_enable_rc6(const struct drm_device *dev)
@@ -2532,12 +2532,12 @@ 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, GEN6_PM_DEFERRED_EVENTS);
+	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
 	spin_lock_irq(&dev_priv->rps.lock);
 	WARN_ON(dev_priv->rps.pm_iir != 0);
-	I915_WRITE(GEN6_PMIMR, 0);
+	I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
 	spin_unlock_irq(&dev_priv->rps.lock);
-	/* enable all PM interrupts */
+	/* unmask all PM interrupts */
 	I915_WRITE(GEN6_PMINTRMSK, 0);
 
 	gen6_gt_force_wake_put(dev_priv);
-- 
1.8.0

[PATCH 09/18 v4] drm/i915: make PM interrupt writes non-destructive

[Ben Widawsky]

PM interrupts have an expanded role on HSW. It helps route the EBOX
interrupts. This patch is necessary to make the existing code which
touches the mask, and enable registers more friendly to other code paths
that also will need these registers.

To be more explicit:
At preinstall all interrupts are masked and disabled. This implies that
preinstall should always happen before any enabling/disabling of RPS or
other interrupts.

The PMIMR is touched by the workqueue, so enable/disable touch IER and
IIR. Similarly, the code currently expects IMR has no use outside of the
RPS related interrupts so they unconditionally set 0, or ~0. We could
use IER in the workqueue, and IMR elsewhere, but since the workqueue
use-case is more transient the existing usage makes sense.

Disable RPS events:
IER := IER & ~GEN6_PM_RPS_EVENTS // Disable RPS related interrupts
IIR := GEN6_PM_RPS_EVENTS // Disable any outstanding interrupts

Enable RPS events:
IER := IER | GEN6_PM_RPS_EVENTS // Enable the RPS related interrupts
IIR := GEN6_PM_RPS_EVENTS // Make sure there were no leftover events
(really shouldn't happen)

v2: Shouldn't destroy PMIIR or PMIMR VEBOX interrupt state in
enable/disable rps functions (Haihao)

v3: Bug found by Chris where we were clearing the wrong bits at rps
disable.
    expanded commit message

v4: v3 was based off the wrong branch

CC: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_irq.c | 17 +++++++++--------
 drivers/gpu/drm/i915/i915_reg.h |  2 +-
 drivers/gpu/drm/i915/intel_pm.c | 10 +++++-----
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index d5aef6c..5533e55 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -372,10 +372,11 @@ static void gen6_pm_rps_work(struct work_struct *work)
 	pm_iir = dev_priv->rps.pm_iir;
 	dev_priv->rps.pm_iir = 0;
 	pm_imr = I915_READ(GEN6_PMIMR);
-	I915_WRITE(GEN6_PMIMR, 0);
+	/* Make sure not to corrupt PMIMR state used by ringbuffer code */
+	I915_WRITE(GEN6_PMIMR, pm_imr & ~GEN6_PM_RPS_EVENTS);
 	spin_unlock_irq(&dev_priv->rps.lock);
 
-	if ((pm_iir & GEN6_PM_DEFERRED_EVENTS) == 0)
+	if ((pm_iir & GEN6_PM_RPS_EVENTS) == 0)
 		return;
 
 	mutex_lock(&dev_priv->rps.hw_lock);
@@ -536,17 +537,17 @@ static void hsw_pm_irq_handler(struct drm_i915_private *dev_priv,
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev_priv->rps.lock, flags);
-	dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_DEFERRED_EVENTS;
+	dev_priv->rps.pm_iir |= pm_iir & GEN6_PM_RPS_EVENTS;
 	if (dev_priv->rps.pm_iir) {
 		I915_WRITE(GEN6_PMIMR, dev_priv->rps.pm_iir);
 		/* We never want to mask useful interrupts. (also posting read) */
-		WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_DEFERRED_EVENTS);
+		WARN_ON(I915_READ_NOTRACE(GEN6_PMIMR) & ~GEN6_PM_RPS_EVENTS);
 		/* TODO: if queue_work is slow, move it out of the spinlock */
 		queue_work(dev_priv->wq, &dev_priv->rps.work);
 	}
 	spin_unlock_irqrestore(&dev_priv->rps.lock, flags);
 
-	if (pm_iir & ~GEN6_PM_DEFERRED_EVENTS)
+	if (pm_iir & ~GEN6_PM_RPS_EVENTS)
 		DRM_ERROR("Unexpected PM interrupted\n");
 }
 
@@ -619,7 +620,7 @@ static irqreturn_t valleyview_irq_handler(int irq, void *arg)
 		if (pipe_stats[pipe] & PIPE_LEGACY_BLC_EVENT_STATUS)
 			blc_event = true;
 
-		if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
+		if (pm_iir & GEN6_PM_RPS_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 
 		I915_WRITE(GTIIR, gt_iir);
@@ -759,7 +760,7 @@ static irqreturn_t ivybridge_irq_handler(int irq, void *arg)
 	if (pm_iir) {
 		if (IS_HASWELL(dev))
 			hsw_pm_irq_handler(dev_priv, pm_iir);
-		else if (pm_iir & GEN6_PM_DEFERRED_EVENTS)
+		else if (pm_iir & GEN6_PM_RPS_EVENTS)
 			gen6_queue_rps_work(dev_priv, pm_iir);
 		I915_WRITE(GEN6_PMIIR, pm_iir);
 		ret = IRQ_HANDLED;
@@ -841,7 +842,7 @@ static irqreturn_t ironlake_irq_handler(int irq, void *arg)
 	if (IS_GEN5(dev) &&  de_iir & DE_PCU_EVENT)
 		ironlake_handle_rps_change(dev);
 
-	if (IS_GEN6(dev) && pm_iir & GEN6_PM_DEFERRED_EVENTS)
+	if (IS_GEN6(dev) && pm_iir & GEN6_PM_RPS_EVENTS)
 		gen6_queue_rps_work(dev_priv, pm_iir);
 
 	/* should clear PCH hotplug event before clear CPU irq */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 21f5d61..70f9ad7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4269,7 +4269,7 @@
 #define  GEN6_PM_RP_DOWN_THRESHOLD		(1<<4)
 #define  GEN6_PM_RP_UP_EI_EXPIRED		(1<<2)
 #define  GEN6_PM_RP_DOWN_EI_EXPIRED		(1<<1)
-#define  GEN6_PM_DEFERRED_EVENTS		(GEN6_PM_RP_UP_THRESHOLD | \
+#define  GEN6_PM_RPS_EVENTS			(GEN6_PM_RP_UP_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_THRESHOLD | \
 						 GEN6_PM_RP_DOWN_TIMEOUT)
 
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8f15616..79b7af4 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2354,7 +2354,7 @@ static void gen6_disable_rps(struct drm_device *dev)
 	I915_WRITE(GEN6_RC_CONTROL, 0);
 	I915_WRITE(GEN6_RPNSWREQ, 1 << 31);
 	I915_WRITE(GEN6_PMINTRMSK, 0xffffffff);
-	I915_WRITE(GEN6_PMIER, 0);
+	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) & ~GEN6_PM_RPS_EVENTS);
 	/* Complete PM interrupt masking here doesn't race with the rps work
 	 * item again unmasking PM interrupts because that is using a different
 	 * register (PMIMR) to mask PM interrupts. The only risk is in leaving
@@ -2364,7 +2364,7 @@ static void gen6_disable_rps(struct drm_device *dev)
 	dev_priv->rps.pm_iir = 0;
 	spin_unlock_irq(&dev_priv->rps.lock);
 
-	I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR));
+	I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
 }
 
 int intel_enable_rc6(const struct drm_device *dev)
@@ -2518,12 +2518,12 @@ 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, GEN6_PM_DEFERRED_EVENTS);
+	I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS);
 	spin_lock_irq(&dev_priv->rps.lock);
 	WARN_ON(dev_priv->rps.pm_iir != 0);
-	I915_WRITE(GEN6_PMIMR, 0);
+	I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS);
 	spin_unlock_irq(&dev_priv->rps.lock);
-	/* enable all PM interrupts */
+	/* unmask all PM interrupts */
 	I915_WRITE(GEN6_PMINTRMSK, 0);
 
 	rc6vids = 0;
-- 
1.8.0

Re: [PATCH 14/18] drm/i915: vebox interrupt get/put

[Daniel Vetter]

On Tue, Nov 06, 2012 at 04:25:38PM +0000, Ben Widawsky wrote:
> v2: Use the correct lock to protect PM interrupt regs, this was
> accidentally lost from earlier (Haihao)
> Fix return types (Ben)
> 
> CC: Xiang, Haihao <haihao.xiang@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

[snip]

> ---
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index b2fe5b4..2a2bd20 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -58,8 +58,9 @@ struct  intel_ring_buffer {
>  	u32		last_retired_head;
>  
>  	struct {
> -		u32	gt;
> -	} irq_refcount;	/* protected by dev_priv->irq_lock */
> +		u32	gt; /*  protected by dev_priv->irq_lock */
> +		u32	pm; /*  protected by dev_priv->rps.lock (sucks) */
> +	} irq_refcount;

So Ben asked me on internal irc to at least ack the irq stuff in his VECS
patches here as a cheap excuse for why he never updated them. I'm ok with
it on a quick read, the #define unification seems to make tons of sense,
and the locking trick here is imo ok, too. Only bikeshed I could have come
up with is to put gt/pm irq_refcount into a union, since for a given ring
we'll never use both. Maybe also add a bigger comment above the
irq_refcount union/struct to explain clearly wtf is going on here.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch