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

["O'Rourke, Tom" <Tom.O'Rourke@intel.com>]

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@intel.com>
> Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
> ---
>  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 */
[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.

> +	I915_WRITE(GUC_BCS_RCS_IER, ~irqs);
> +	I915_WRITE(GUC_VCS2_VCS1_IER, ~irqs);
> +	I915_WRITE(GUC_WD_VECS_IER, ~irqs);
> +}
> +
>  static u32 get_gttype(struct drm_i915_private *dev_priv)
>  {
>  	/* XXX: GT type based on PCI device ID? field seems unused by fw */
> @@ -427,6 +474,7 @@ int intel_guc_ucode_load(struct drm_device *dev)
>  		intel_guc_fw_status_repr(guc_fw->guc_fw_fetch_status),
>  		intel_guc_fw_status_repr(guc_fw->guc_fw_load_status));
>  
> +	direct_interrupts_to_host(dev_priv);
>  	i915_guc_submission_disable(dev);
>  
>  	if (guc_fw->guc_fw_fetch_status == GUC_FIRMWARE_NONE)
> @@ -485,6 +533,7 @@ int intel_guc_ucode_load(struct drm_device *dev)
>  		err = i915_guc_submission_enable(dev);
>  		if (err)
>  			goto fail;
> +		direct_interrupts_to_guc(dev_priv);
>  	}
>  
>  	return 0;
> @@ -493,6 +542,7 @@ fail:
>  	if (guc_fw->guc_fw_load_status == GUC_FIRMWARE_PENDING)
>  		guc_fw->guc_fw_load_status = GUC_FIRMWARE_FAIL;
>  
> +	direct_interrupts_to_host(dev_priv);
>  	i915_guc_submission_disable(dev);
>  
>  	DRM_ERROR("Failed to initialize GuC, error %d\n", err);
> @@ -557,6 +607,7 @@ void intel_guc_ucode_fini(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>  
> +	direct_interrupts_to_host(dev_priv);
>  	i915_guc_submission_fini(dev);
>  
>  	if (guc_fw->guc_fw_obj)
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx