[Intel-gfx] [PATCH 11/15] drm/i915: Interrupt routing for GuC submission

Daniel Vetter daniel at ffwll.ch
Mon Jul 6 07:14:04 PDT 2015


On Fri, Jul 03, 2015 at 01:30:33PM +0100, Dave Gordon wrote:
> Turn on interrupt steering to route necessary interrupts to GuC.
> 
> Issue: VIZ-4884
> Signed-off-by: Alex Dai <yu.dai at intel.com>
> Signed-off-by: Dave Gordon <david.s.gordon at 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 eefb847..9f32c6c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1659,12 +1659,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
>  
> @@ -5677,11 +5683,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 eb76c34..ef5f3d5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -62,6 +62,53 @@
>  #define I915_SKL_GUC_UCODE "i915/skl_guc_ver3.bin"
>  MODULE_FIRMWARE(I915_SKL_GUC_UCODE);
>  
> +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);
> +}
> +
>  static u32 get_gttype(struct drm_i915_private *dev_priv)
>  {
>  	/* XXX: GT type based on PCI device ID? field seems unused by fw */
> @@ -417,6 +464,7 @@ int intel_guc_ucode_load(struct drm_device *dev)
>  		intel_uc_fw_status_repr(guc_fw->uc_fw_fetch_status),
>  		intel_uc_fw_status_repr(guc_fw->uc_fw_load_status));
>  
> +	direct_interrupts_to_host(dev_priv);
>  	i915_guc_submission_disable(dev);
>  
>  	if (guc_fw->uc_fw_fetch_status == INTEL_UC_FIRMWARE_NONE)
> @@ -450,6 +498,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);

This looks like still some remnants from when guc loading could fail and
we needed to switch between cpu and guc submission at runtime. Problem is
that you don't own the interrupt state at this point (since interrupts are
already enabled) and hence there could be problems with races.

Simplest solution would be to just set up the interrupt routing we want
statically at driver load time in the respective irq callbacks (looking at
guc-or-not-guc module option). That means we bake the guc submission y/n
decision into the driver load sequence very early, but it does simplify
things. And that's one of the reasons I don't like runtime fallbacks.
-Daniel

>  	}
>  
>  	return 0;
> @@ -458,6 +507,7 @@ fail:
>  	if (guc_fw->uc_fw_load_status == INTEL_UC_FIRMWARE_PENDING)
>  		guc_fw->uc_fw_load_status = INTEL_UC_FIRMWARE_FAIL;
>  
> +	direct_interrupts_to_host(dev_priv);
>  	i915_guc_submission_disable(dev);
>  
>  	DRM_ERROR("Failed to initialize GuC, error %d\n", err);
> @@ -473,6 +523,7 @@ void intel_guc_ucode_fini(struct drm_device *dev)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_uc_fw *guc_fw = &dev_priv->guc.guc_fw;
>  
> +	direct_interrupts_to_host(dev_priv);
>  	i915_guc_submission_fini(dev);
>  
>  	intel_uc_fw_fini(guc_fw);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the Intel-gfx mailing list