| Message ID | 1436466554-24806-11-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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@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 */ >> + 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.
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); +} + 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)
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(-)