Re: [PATCH 10/13 v4] drm/i915: Interrupt routing for GuC submission

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/07/15 16:33, O'Rourke, Tom wrote:
On Thu, Jul 09, 2015 at 07:29:11PM +0100, Dave Gordon wrote:
Turn on interrupt steering to route necessary interrupts to GuC.

v4:
     Rebased

Issue: VIZ-4884
Signed-off-by: Alex Dai <yu.dai@xxxxxxxxx>
Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_reg.h         | 11 +++++--
  drivers/gpu/drm/i915/intel_guc_loader.c | 51 +++++++++++++++++++++++++++++++++
  2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 63728c1..1c2072b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1664,12 +1664,18 @@ enum skl_disp_power_wells {
  #define GFX_MODE_GEN7	0x0229c
  #define RING_MODE_GEN7(ring)	((ring)->mmio_base+0x29c)
  #define   GFX_RUN_LIST_ENABLE		(1<<15)
+#define   GFX_INTERRUPT_STEERING	(1<<14)
  #define   GFX_TLB_INVALIDATE_EXPLICIT	(1<<13)
  #define   GFX_SURFACE_FAULT_ENABLE	(1<<12)
  #define   GFX_REPLAY_MODE		(1<<11)
  #define   GFX_PSMI_GRANULARITY		(1<<10)
  #define   GFX_PPGTT_ENABLE		(1<<9)

+#define   GFX_FORWARD_VBLANK_MASK	(3<<5)
+#define   GFX_FORWARD_VBLANK_NEVER	(0<<5)
+#define   GFX_FORWARD_VBLANK_ALWAYS	(1<<5)
+#define   GFX_FORWARD_VBLANK_COND	(2<<5)
+
  #define VLV_DISPLAY_BASE 0x180000
  #define VLV_MIPI_BASE VLV_DISPLAY_BASE

@@ -5683,11 +5689,12 @@ enum skl_disp_power_wells {
  #define GEN8_GT_IIR(which) (0x44308 + (0x10 * (which)))
  #define GEN8_GT_IER(which) (0x4430c + (0x10 * (which)))

-#define GEN8_BCS_IRQ_SHIFT 16
  #define GEN8_RCS_IRQ_SHIFT 0
-#define GEN8_VCS2_IRQ_SHIFT 16
+#define GEN8_BCS_IRQ_SHIFT 16
  #define GEN8_VCS1_IRQ_SHIFT 0
+#define GEN8_VCS2_IRQ_SHIFT 16
  #define GEN8_VECS_IRQ_SHIFT 0
+#define GEN8_WD_IRQ_SHIFT 16

  #define GEN8_DE_PIPE_ISR(pipe) (0x44400 + (0x10 * (pipe)))
  #define GEN8_DE_PIPE_IMR(pipe) (0x44404 + (0x10 * (pipe)))
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 25ba29f..2aa9227 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -79,6 +79,53 @@ const char *intel_guc_fw_status_repr(enum intel_guc_fw_status status)
  	}
  };

+static void direct_interrupts_to_host(struct drm_i915_private *dev_priv)
+{
+	struct intel_engine_cs *ring;
+	int i, irqs;
+
+	/* tell all command streamers NOT to forward interrupts and vblank to GuC */
+	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_NEVER);
+	irqs |= _MASKED_BIT_DISABLE(GFX_INTERRUPT_STEERING);
+	for_each_ring(ring, dev_priv, i)
+		I915_WRITE(RING_MODE_GEN7(ring), irqs);
+
+	/* tell DE to send nothing to GuC */
+	I915_WRITE(DE_GUCRMR, ~0);
+
+	/* route all GT interrupts to the host */
+	I915_WRITE(GUC_BCS_RCS_IER, 0);
+	I915_WRITE(GUC_VCS2_VCS1_IER, 0);
+	I915_WRITE(GUC_WD_VECS_IER, 0);
+}
+
+static void direct_interrupts_to_guc(struct drm_i915_private *dev_priv)
+{
+	struct intel_engine_cs *ring;
+	int i, irqs;
+
+	/* tell all command streamers to forward interrupts and vblank to GuC */
+	irqs = _MASKED_FIELD(GFX_FORWARD_VBLANK_MASK, GFX_FORWARD_VBLANK_ALWAYS);
+	irqs |= _MASKED_BIT_ENABLE(GFX_INTERRUPT_STEERING);
+	for_each_ring(ring, dev_priv, i)
+		I915_WRITE(RING_MODE_GEN7(ring), irqs);
+
+	/* tell DE to send (all) flip_done to GuC */
+	irqs = DERRMR_PIPEA_PRI_FLIP_DONE | DERRMR_PIPEA_SPR_FLIP_DONE |
+	       DERRMR_PIPEB_PRI_FLIP_DONE | DERRMR_PIPEB_SPR_FLIP_DONE |
+	       DERRMR_PIPEC_PRI_FLIP_DONE | DERRMR_PIPEC_SPR_FLIP_DONE;
+	/* Unmasked bits will cause GuC response message to be sent */
+	I915_WRITE(DE_GUCRMR, ~irqs);
+
+	/* route USER_INTERRUPT to Host, all others are sent to GuC. */
+	irqs = GT_RENDER_USER_INTERRUPT << GEN8_RCS_IRQ_SHIFT |
+	       GT_RENDER_USER_INTERRUPT << GEN8_BCS_IRQ_SHIFT;
+	/* These three registers have the same bit definitions */
>> +	I915_WRITE(GUC_BCS_RCS_IER, ~irqs);
>> +	I915_WRITE(GUC_VCS2_VCS1_IER, ~irqs);
>> +	I915_WRITE(GUC_WD_VECS_IER, ~irqs);
>> +}
>> +
>
[TOR:] Reliance on the registers having the same bit
definitions does not seem safe.  Each of the three
registers has shift constants defined.  I would expect
the shift constants for the second and third registers
to be used when writing those registers.

Also, GT_RENDER_USER_INTERRUPT seems to have been defined
for use with a different register than this set.

On the other hand, this code does actually write the
correct values.

The header file that defines GT_RENDER_USER_INTERRUPT, GEN8_RCS_IRQ_SHIFT et al. contains this comment:

/* 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.
 */

Per the second paragraph, we use these defines even though they appear to relate to a different (older) set of registers.

Also the BSpec doesn't actually contain separate definitions for these GuC-related interrupt control registers, but instead they all refer to the existing "GT Interrupt 0 Definition" page. So I think we're safe enough assuming for now that the H/W will continue to use the existing layout for all ISR/IMR/IIR/IER registers; that is, two 16-bit fields packed into each 32-bit register, with each field relating to a specific interrupt source, and corresponding bits in each of these fields controlling the same type of interrupt. But I did consider it an assumption that required a comment :)

If a future chip has a different set of interrupt registers or even just additional engines this code will need to be updated anyway, so the comment should alert anyone modifying this to check that the assumption is still valid.

So if you don't mind, I'm going to leave this unchanged.

.Dave.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux