Message ID | 1434393394-21002-13-git-send-email-david.s.gordon@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote: > +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); That's scary since userspace depends on a few more DERRMR events (wait-for-scanline). Where will they end up? -Chris
On 16/06/15 10:24, Chris Wilson wrote: > On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote: >> +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); > > That's scary since userspace depends on a few more DERRMR events > (wait-for-scanline). Where will they end up? > -Chris This doesn't change any bits in DE_RRMR, or set the VBLANK or SCANLINE bits in the DE_GUCRMR, so those events should be unaffected. The GuC isn't interested in those, only in flip done. .Dave.
On Wed, Jun 17, 2015 at 09:20:44AM +0100, Dave Gordon wrote: > On 16/06/15 10:24, Chris Wilson wrote: > > On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote: > >> +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); > > > > That's scary since userspace depends on a few more DERRMR events > > (wait-for-scanline). Where will they end up? > > -Chris > > This doesn't change any bits in DE_RRMR, or set the VBLANK or SCANLINE > bits in the DE_GUCRMR, so those events should be unaffected. The GuC > isn't interested in those, only in flip done. Why does the guc care about flip_done? With atomic it'll get exactly none of those, ever ... -Daniel
On Wed, Jun 17, 2015 at 02:22:19PM +0200, Daniel Vetter wrote: > On Wed, Jun 17, 2015 at 09:20:44AM +0100, Dave Gordon wrote: > > On 16/06/15 10:24, Chris Wilson wrote: > > > On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote: > > >> +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); > > > > > > That's scary since userspace depends on a few more DERRMR events > > > (wait-for-scanline). Where will they end up? > > > -Chris > > > > This doesn't change any bits in DE_RRMR, or set the VBLANK or SCANLINE > > bits in the DE_GUCRMR, so those events should be unaffected. The GuC > > isn't interested in those, only in flip done. > > Why does the guc care about flip_done? With atomic it'll get exactly none > of those, ever ... Well I forgot that mmio writes also generate interrupts. Still strange that GuC is interested in this. Would be really interesting to know what GuC is up to here. -Daniel
On 17/06/15 13:41, Daniel Vetter wrote: > On Wed, Jun 17, 2015 at 02:22:19PM +0200, Daniel Vetter wrote: >> On Wed, Jun 17, 2015 at 09:20:44AM +0100, Dave Gordon wrote: >>> On 16/06/15 10:24, Chris Wilson wrote: >>>> On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote: >>>>> +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); >>>> >>>> That's scary since userspace depends on a few more DERRMR events >>>> (wait-for-scanline). Where will they end up? >>>> -Chris >>> >>> This doesn't change any bits in DE_RRMR, or set the VBLANK or SCANLINE >>> bits in the DE_GUCRMR, so those events should be unaffected. The GuC >>> isn't interested in those, only in flip done. >> >> Why does the guc care about flip_done? With atomic it'll get exactly none >> of those, ever ... > > Well I forgot that mmio writes also generate interrupts. Still strange > that GuC is interested in this. Would be really interesting to know what > GuC is up to here. > -Daniel Maybe Alex knows ... otherwise we can ask the GuC f/w team ... .Dave.
On 06/23/2015 04:33 AM, Dave Gordon wrote: > On 17/06/15 13:41, Daniel Vetter wrote: > > On Wed, Jun 17, 2015 at 02:22:19PM +0200, Daniel Vetter wrote: > >> On Wed, Jun 17, 2015 at 09:20:44AM +0100, Dave Gordon wrote: > >>> On 16/06/15 10:24, Chris Wilson wrote: > >>>> On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote: > >>>>> +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); > >>>> > >>>> That's scary since userspace depends on a few more DERRMR events > >>>> (wait-for-scanline). Where will they end up? > >>>> -Chris > >>> > >>> This doesn't change any bits in DE_RRMR, or set the VBLANK or SCANLINE > >>> bits in the DE_GUCRMR, so those events should be unaffected. The GuC > >>> isn't interested in those, only in flip done. > >> > >> Why does the guc care about flip_done? With atomic it'll get exactly none > >> of those, ever ... > > > > Well I forgot that mmio writes also generate interrupts. Still strange > > that GuC is interested in this. Would be really interesting to know what > > GuC is up to here. > > -Daniel > > Maybe Alex knows ... otherwise we can ask the GuC f/w team ... > The SLPC (Single Loop Power Control) within GuC needs these. However, to enable it or not is yet determined because architecture review is not done. Alex
On Tue, Jun 23, 2015 at 04:48:11PM -0700, Yu Dai wrote: > > > On 06/23/2015 04:33 AM, Dave Gordon wrote: > >On 17/06/15 13:41, Daniel Vetter wrote: > >> On Wed, Jun 17, 2015 at 02:22:19PM +0200, Daniel Vetter wrote: > >>> On Wed, Jun 17, 2015 at 09:20:44AM +0100, Dave Gordon wrote: > >>>> On 16/06/15 10:24, Chris Wilson wrote: > >>>>> On Mon, Jun 15, 2015 at 07:36:30PM +0100, Dave Gordon wrote: > >>>>>> +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); > >>>>> > >>>>> That's scary since userspace depends on a few more DERRMR events > >>>>> (wait-for-scanline). Where will they end up? > >>>>> -Chris > >>>> > >>>> This doesn't change any bits in DE_RRMR, or set the VBLANK or SCANLINE > >>>> bits in the DE_GUCRMR, so those events should be unaffected. The GuC > >>>> isn't interested in those, only in flip done. > >>> > >>> Why does the guc care about flip_done? With atomic it'll get exactly none > >>> of those, ever ... > >> > >> Well I forgot that mmio writes also generate interrupts. Still strange > >> that GuC is interested in this. Would be really interesting to know what > >> GuC is up to here. > > > >Maybe Alex knows ... otherwise we can ask the GuC f/w team ... > > > The SLPC (Single Loop Power Control) within GuC needs these. However, to > enable it or not is yet determined because architecture review is not done. Well if guc needs to know about display activity, the kernel can tell it directly. And it could even tell the guc when we've missed a frame, which is something the guc has absolutely no idea about since with atomic that's all implemented on the kernel side and never goes through the rings. Sounds like a "not engineerd for linux kernel" feature :( -Daniel
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 56f81de..e255253 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1638,12 +1638,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 @@ -5627,11 +5633,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 6e4667d..204777b 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_device *dev) { /* XXX: GT type based on PCI device ID? field seems unused by fw */ @@ -390,6 +437,7 @@ int intel_guc_ucode_load(struct drm_device *dev, bool wait) if (guc_fw->uc_fw_fetch_status == INTEL_UC_FIRMWARE_PENDING && !wait) return -EAGAIN; + direct_interrupts_to_host(dev_priv); i915_guc_submission_disable(dev); if (guc_fw->uc_fw_fetch_status == INTEL_UC_FIRMWARE_NONE) @@ -418,6 +466,7 @@ int intel_guc_ucode_load(struct drm_device *dev, bool wait) err = i915_guc_submission_enable(dev); if (err) goto fail; + direct_interrupts_to_guc(dev_priv); } return 0; @@ -426,6 +475,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); @@ -441,6 +491,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);