[v2,3/3] drm/i915: move gen8 irq shifts to intel_lrc.c
diff mbox

Message ID 20180308234629.27021-3-daniele.ceraolospurio@intel.com
State New
Headers show

Commit Message

Daniele Ceraolo Spurio March 8, 2018, 11:46 p.m. UTC
The only usage outside the intel_lrc.c file is in the ringbuffer
init, but the irq mask calculated there is then overwritten for
all engines that have a non-zero shift, so we can drop it.

This change is not aimed at code saving but at removing from
intel_engines information that does not apply to all gens that have
the engine. When checking without the temporary WARN_ON, code size
is basically unchanged:

add/remove: 1/0 grow/shrink: 3/4 up/down: 70/-67 (3)
Function                                     old     new   delta
logical_ring_setup                           315     343     +28
irq_shifts                                     -      28     +28
intel_init_render_ring_buffer                258     268     +10
reset_common_ring                            704     708      +4
intel_engine_init_cmd_parser                1064    1058      -6
intel_engines_init_mmio                     1264    1256      -8
intel_ring_default_vfuncs                    584     563     -21
intel_engines                                224     192     -32
Total: Before=1479719, After=1479722, chg +0.00%

Suggested-by: Michel Thierry <michel.thierry@intel.com>
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_engine_cs.c  | 10 ----------
 drivers/gpu/drm/i915/intel_lrc.c        | 22 +++++++++++++++++++---
 drivers/gpu/drm/i915/intel_ringbuffer.c |  4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |  1 -
 4 files changed, 21 insertions(+), 16 deletions(-)

Comments

Chris Wilson March 9, 2018, 12:31 a.m. UTC | #1
Quoting Daniele Ceraolo Spurio (2018-03-08 23:46:29)
> The only usage outside the intel_lrc.c file is in the ringbuffer
> init, but the irq mask calculated there is then overwritten for
> all engines that have a non-zero shift, so we can drop it.
> 
> This change is not aimed at code saving but at removing from
> intel_engines information that does not apply to all gens that have
> the engine. When checking without the temporary WARN_ON, code size
> is basically unchanged:
> 
> add/remove: 1/0 grow/shrink: 3/4 up/down: 70/-67 (3)
> Function                                     old     new   delta
> logical_ring_setup                           315     343     +28
> irq_shifts                                     -      28     +28
> intel_init_render_ring_buffer                258     268     +10
> reset_common_ring                            704     708      +4
> intel_engine_init_cmd_parser                1064    1058      -6
> intel_engines_init_mmio                     1264    1256      -8
> intel_ring_default_vfuncs                    584     563     -21
> intel_engines                                224     192     -32
> Total: Before=1479719, After=1479722, chg +0.00%
> 
> Suggested-by: Michel Thierry <michel.thierry@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

With the missing static const,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I had a look at restoring the use of engine->irq_shift in the irq
handlers, and while that does shrink the code abit, rearranging the code
brought even more savings. So I don't see a need to keep
engine->irq_shift around, and we can always bring it back but unlikely
if future gen do not need it.
-Chris

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a33171d82aee..dbfeff6c46a8 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -92,7 +92,6 @@  struct engine_info {
 		u32 gen : 8;
 		u32 base : 24;
 	} mmio_bases[MAX_MMIO_BASES];
-	unsigned irq_shift;
 };
 
 static const struct engine_info intel_engines[] = {
@@ -104,7 +103,6 @@  static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 0, .base = RENDER_RING_BASE }
 		},
-		.irq_shift = GEN8_RCS_IRQ_SHIFT,
 	},
 	[BCS] = {
 		.hw_id = BCS_HW,
@@ -114,7 +112,6 @@  static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 6, .base = BLT_RING_BASE }
 		},
-		.irq_shift = GEN8_BCS_IRQ_SHIFT,
 	},
 	[VCS] = {
 		.hw_id = VCS_HW,
@@ -126,7 +123,6 @@  static const struct engine_info intel_engines[] = {
 			{ .gen = 6, .base = GEN6_BSD_RING_BASE },
 			{ .gen = 4, .base = BSD_RING_BASE }
 		},
-		.irq_shift = GEN8_VCS1_IRQ_SHIFT,
 	},
 	[VCS2] = {
 		.hw_id = VCS2_HW,
@@ -137,7 +133,6 @@  static const struct engine_info intel_engines[] = {
 			{ .gen = 11, .base = GEN11_BSD2_RING_BASE },
 			{ .gen = 8, .base = GEN8_BSD2_RING_BASE }
 		},
-		.irq_shift = GEN8_VCS2_IRQ_SHIFT,
 	},
 	[VCS3] = {
 		.hw_id = VCS3_HW,
@@ -147,7 +142,6 @@  static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 11, .base = GEN11_BSD3_RING_BASE }
 		},
-		.irq_shift = 0, /* not used */
 	},
 	[VCS4] = {
 		.hw_id = VCS4_HW,
@@ -157,7 +151,6 @@  static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 11, .base = GEN11_BSD4_RING_BASE }
 		},
-		.irq_shift = 0, /* not used */
 	},
 	[VECS] = {
 		.hw_id = VECS_HW,
@@ -168,7 +161,6 @@  static const struct engine_info intel_engines[] = {
 			{ .gen = 11, .base = GEN11_VEBOX_RING_BASE },
 			{ .gen = 7, .base = VEBOX_RING_BASE }
 		},
-		.irq_shift = GEN8_VECS_IRQ_SHIFT,
 	},
 	[VECS2] = {
 		.hw_id = VECS2_HW,
@@ -178,7 +170,6 @@  static const struct engine_info intel_engines[] = {
 		.mmio_bases = {
 			{ .gen = 11, .base = GEN11_VEBOX2_RING_BASE }
 		},
-		.irq_shift = 0, /* not used */
 	},
 };
 
@@ -298,7 +289,6 @@  intel_engine_setup(struct drm_i915_private *dev_priv,
 		sizeof(engine->name));
 	engine->hw_id = engine->guc_id = info->hw_id;
 	engine->mmio_base = __engine_mmio_base(dev_priv, info->mmio_bases);
-	engine->irq_shift = info->irq_shift;
 	engine->class = info->class;
 	engine->instance = info->instance;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 999d5f2539d4..1bf8e16d7fa3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1573,6 +1573,14 @@  static u8 gtiir[] = {
 	[VECS] = 3,
 };
 
+unsigned int irq_shifts[] = {
+	[RCS] = GEN8_RCS_IRQ_SHIFT,
+	[BCS] = GEN8_BCS_IRQ_SHIFT,
+	[VCS] = GEN8_VCS1_IRQ_SHIFT,
+	[VCS2] = GEN8_VCS2_IRQ_SHIFT,
+	[VECS] = GEN8_VECS_IRQ_SHIFT,
+};
+
 static void enable_execlists(struct intel_engine_cs *engine)
 {
 	struct drm_i915_private *dev_priv = engine->i915;
@@ -1661,6 +1669,10 @@  static void reset_irq(struct intel_engine_cs *engine)
 	struct drm_i915_private *dev_priv = engine->i915;
 	int i;
 
+	/* TODO: correctly reset irqs for gen11 */
+	if (WARN_ON_ONCE(INTEL_GEN(engine->i915) >= 11))
+		return;
+
 	GEM_BUG_ON(engine->id >= ARRAY_SIZE(gtiir));
 
 	/*
@@ -1672,11 +1684,11 @@  static void reset_irq(struct intel_engine_cs *engine)
 	 */
 	for (i = 0; i < 2; i++) {
 		I915_WRITE(GEN8_GT_IIR(gtiir[engine->id]),
-			   GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift);
+			   GT_CONTEXT_SWITCH_INTERRUPT << irq_shifts[engine->id]);
 		POSTING_READ(GEN8_GT_IIR(gtiir[engine->id]));
 	}
 	GEM_BUG_ON(I915_READ(GEN8_GT_IIR(gtiir[engine->id])) &
-		   (GT_CONTEXT_SWITCH_INTERRUPT << engine->irq_shift));
+		   (GT_CONTEXT_SWITCH_INTERRUPT << irq_shifts[engine->id]));
 
 	clear_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted);
 }
@@ -2109,7 +2121,11 @@  logical_ring_default_vfuncs(struct intel_engine_cs *engine)
 static inline void
 logical_ring_default_irqs(struct intel_engine_cs *engine)
 {
-	unsigned shift = engine->irq_shift;
+	unsigned shift = 0;
+
+	if (INTEL_GEN(engine->i915) < 11)
+		shift = irq_shifts[engine->id];
+
 	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << shift;
 	engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
 }
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2e4408477ab5..3a7024fdad6f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1943,8 +1943,6 @@  static void intel_ring_init_semaphores(struct drm_i915_private *dev_priv,
 static void intel_ring_init_irq(struct drm_i915_private *dev_priv,
 				struct intel_engine_cs *engine)
 {
-	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT << engine->irq_shift;
-
 	if (INTEL_GEN(dev_priv) >= 6) {
 		engine->irq_enable = gen6_irq_enable;
 		engine->irq_disable = gen6_irq_disable;
@@ -2029,6 +2027,8 @@  int intel_init_render_ring_buffer(struct intel_engine_cs *engine)
 	if (HAS_L3_DPF(dev_priv))
 		engine->irq_keep_mask = GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
 
+	engine->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
+
 	if (INTEL_GEN(dev_priv) >= 6) {
 		engine->init_context = intel_rcs_ctx_init;
 		engine->emit_flush = gen7_render_ring_flush;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d8ddea0174ca..08265e312dc2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -330,7 +330,6 @@  struct intel_engine_cs {
 	u8 instance;
 	u32 context_size;
 	u32 mmio_base;
-	unsigned int irq_shift;
 
 	struct intel_ring *buffer;
 	struct intel_timeline *timeline;