| Message ID | 20170325013010.36244-17-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 24, 2017 at 06:30:08PM -0700, Michel Thierry wrote: > Emit the required commands into the ring buffer for starting and > stopping the watchdog timer before/after batch buffer start during > batch buffer submission. > > v2: Support watchdog threshold per context engine, merge lri commands, > and move watchdog commands emission to emit_bb_start. Request space of > combined start_watchdog, bb_start and stop_watchdog to avoid any error > after emitting bb_start. > > Signed-off-by: Tomas Elf <tomas.elf@intel.com> > Signed-off-by: Ian Lister <ian.lister@intel.com> > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_context.h | 4 ++ > drivers/gpu/drm/i915/intel_lrc.c | 86 ++++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_ringbuffer.h | 4 ++ > 3 files changed, 92 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h > index 4af2ab94558b..88700bdbb4e1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.h > +++ b/drivers/gpu/drm/i915/i915_gem_context.h > @@ -150,6 +150,10 @@ struct i915_gem_context { > u32 *lrc_reg_state; > u64 lrc_desc; > int pin_count; > + /** watchdog_threshold: hw watchdog threshold value, > + * in clock counts > + */ > + u32 watchdog_threshold; > bool initialised; > } engine[I915_NUM_ENGINES]; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 73f8fbdcf1fb..2736f642dc76 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1319,6 +1319,8 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req, > const unsigned int flags) > { > u32 *cs; > + u32 num_dwords; > + bool watchdog_running = false; > int ret; > > /* Don't rely in hw updating PDPs, specially in lite-restore. > @@ -1338,10 +1340,29 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req, > req->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(req->engine); > } > > - cs = intel_ring_begin(req, 4); > + /* bb_start only */ > + num_dwords = 4; > + > + /* check if watchdog will be required */ > + if (req->ctx->engine[req->engine->id].watchdog_threshold != 0) { > + if (!req->engine->emit_start_watchdog || > + !req->engine->emit_stop_watchdog) > + return -EINVAL; A bit late! This will already be checked before we set the threshold. > + /* start_watchdog (6) + bb_start (4) + stop_watchdog (4) */ Don't mention bb_start since it isn't in the additional 10 and you just confused me! :) > + num_dwords += 10; > + watchdog_running = true; > + } > + > + cs = intel_ring_begin(req, num_dwords); > if (IS_ERR(cs)) > return PTR_ERR(cs); > > + if (watchdog_running) { > + /* Start watchdog timer */ > + cs = req->engine->emit_start_watchdog(req, cs); Are they going to be specialised vfuncs in the near future? i.e. something other than LRI. If the only advantage is for checking feature compatability, we could just a bit instead? > + } > + > /* FIXME(BDW): Address space and security selectors. */ > *cs++ = MI_BATCH_BUFFER_START_GEN8 | > (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | > @@ -1349,8 +1370,13 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req, > *cs++ = lower_32_bits(offset); > *cs++ = upper_32_bits(offset); > *cs++ = MI_NOOP; > - intel_ring_advance(req, cs); > > + if (watchdog_running) { > + /* Cancel watchdog timer */ > + cs = req->engine->emit_stop_watchdog(req, cs); > + } > + > + intel_ring_advance(req, cs); > return 0; > } > > @@ -1512,6 +1538,54 @@ static void gen8_watchdog_irq_handler(unsigned long data) > intel_uncore_forcewake_put(dev_priv, engine->fw_domains); > } > > +static u32 *gen8_emit_start_watchdog(struct drm_i915_gem_request *req, u32 *cs) > +{ > + struct intel_engine_cs *engine = req->engine; > + struct i915_gem_context *ctx = req->ctx; > + struct intel_context *ce = &ctx->engine[engine->id]; > + > + /* XXX: no watchdog support in BCS engine */ > + GEM_BUG_ON(engine->id == BCS); > + > + /* > + * watchdog register must never be programmed to zero. This would > + * cause the watchdog counter to exceed and not allow the engine to > + * go into IDLE state > + */ > + GEM_BUG_ON(ce->watchdog_threshold == 0); > + > + /* Set counter period */ > + *cs++ = MI_LOAD_REGISTER_IMM(2); > + *cs++ = i915_mmio_reg_offset(RING_THRESH(engine->mmio_base)); > + *cs++ = ce->watchdog_threshold; > + /* Start counter */ > + *cs++ = i915_mmio_reg_offset(RING_CNTR(engine->mmio_base)); > + *cs++ = GEN8_WATCHDOG_ENABLE; > + *cs++ = MI_NOOP; > + > + return cs; > +} > + > +static u32 *gen8_emit_stop_watchdog(struct drm_i915_gem_request *req, u32 *cs) > +{ > + struct intel_engine_cs *engine = req->engine; > + > + /* XXX: no watchdog support in BCS engine */ > + GEM_BUG_ON(engine->id == BCS); > + > + *cs++ = MI_LOAD_REGISTER_IMM(2); > + *cs++ = i915_mmio_reg_offset(RING_CNTR(engine->mmio_base)); > + > + if (engine->id == RCS) > + *cs++ = GEN8_RCS_WATCHDOG_DISABLE; > + else > + *cs++ = GEN8_XCS_WATCHDOG_DISABLE; > + > + *cs++ = MI_NOOP; 2 spare NOOPS, cancel out. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 4af2ab94558b..88700bdbb4e1 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -150,6 +150,10 @@ struct i915_gem_context { u32 *lrc_reg_state; u64 lrc_desc; int pin_count; + /** watchdog_threshold: hw watchdog threshold value, + * in clock counts + */ + u32 watchdog_threshold; bool initialised; } engine[I915_NUM_ENGINES]; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 73f8fbdcf1fb..2736f642dc76 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1319,6 +1319,8 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req, const unsigned int flags) { u32 *cs; + u32 num_dwords; + bool watchdog_running = false; int ret; /* Don't rely in hw updating PDPs, specially in lite-restore. @@ -1338,10 +1340,29 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req, req->ctx->ppgtt->pd_dirty_rings &= ~intel_engine_flag(req->engine); } - cs = intel_ring_begin(req, 4); + /* bb_start only */ + num_dwords = 4; + + /* check if watchdog will be required */ + if (req->ctx->engine[req->engine->id].watchdog_threshold != 0) { + if (!req->engine->emit_start_watchdog || + !req->engine->emit_stop_watchdog) + return -EINVAL; + + /* start_watchdog (6) + bb_start (4) + stop_watchdog (4) */ + num_dwords += 10; + watchdog_running = true; + } + + cs = intel_ring_begin(req, num_dwords); if (IS_ERR(cs)) return PTR_ERR(cs); + if (watchdog_running) { + /* Start watchdog timer */ + cs = req->engine->emit_start_watchdog(req, cs); + } + /* FIXME(BDW): Address space and security selectors. */ *cs++ = MI_BATCH_BUFFER_START_GEN8 | (flags & I915_DISPATCH_SECURE ? 0 : BIT(8)) | @@ -1349,8 +1370,13 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req, *cs++ = lower_32_bits(offset); *cs++ = upper_32_bits(offset); *cs++ = MI_NOOP; - intel_ring_advance(req, cs); + if (watchdog_running) { + /* Cancel watchdog timer */ + cs = req->engine->emit_stop_watchdog(req, cs); + } + + intel_ring_advance(req, cs); return 0; } @@ -1512,6 +1538,54 @@ static void gen8_watchdog_irq_handler(unsigned long data) intel_uncore_forcewake_put(dev_priv, engine->fw_domains); } +static u32 *gen8_emit_start_watchdog(struct drm_i915_gem_request *req, u32 *cs) +{ + struct intel_engine_cs *engine = req->engine; + struct i915_gem_context *ctx = req->ctx; + struct intel_context *ce = &ctx->engine[engine->id]; + + /* XXX: no watchdog support in BCS engine */ + GEM_BUG_ON(engine->id == BCS); + + /* + * watchdog register must never be programmed to zero. This would + * cause the watchdog counter to exceed and not allow the engine to + * go into IDLE state + */ + GEM_BUG_ON(ce->watchdog_threshold == 0); + + /* Set counter period */ + *cs++ = MI_LOAD_REGISTER_IMM(2); + *cs++ = i915_mmio_reg_offset(RING_THRESH(engine->mmio_base)); + *cs++ = ce->watchdog_threshold; + /* Start counter */ + *cs++ = i915_mmio_reg_offset(RING_CNTR(engine->mmio_base)); + *cs++ = GEN8_WATCHDOG_ENABLE; + *cs++ = MI_NOOP; + + return cs; +} + +static u32 *gen8_emit_stop_watchdog(struct drm_i915_gem_request *req, u32 *cs) +{ + struct intel_engine_cs *engine = req->engine; + + /* XXX: no watchdog support in BCS engine */ + GEM_BUG_ON(engine->id == BCS); + + *cs++ = MI_LOAD_REGISTER_IMM(2); + *cs++ = i915_mmio_reg_offset(RING_CNTR(engine->mmio_base)); + + if (engine->id == RCS) + *cs++ = GEN8_RCS_WATCHDOG_DISABLE; + else + *cs++ = GEN8_XCS_WATCHDOG_DISABLE; + + *cs++ = MI_NOOP; + + return cs; +} + /* * Reserve space for 2 NOOPs at the end of each request to be * used as a workaround for not being allowed to do lite @@ -1780,6 +1854,8 @@ int logical_render_ring_init(struct intel_engine_cs *engine) engine->emit_flush = gen8_emit_flush_render; engine->emit_breadcrumb = gen8_emit_breadcrumb_render; engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_render_sz; + engine->emit_start_watchdog = gen8_emit_start_watchdog; + engine->emit_stop_watchdog = gen8_emit_stop_watchdog; ret = intel_engine_create_scratch(engine, PAGE_SIZE); if (ret) @@ -1803,6 +1879,12 @@ int logical_xcs_ring_init(struct intel_engine_cs *engine) { logical_ring_setup(engine); + /* BCS engine does not have a watchdog-expired irq */ + if (engine->id != BCS) { + engine->emit_start_watchdog = gen8_emit_start_watchdog; + engine->emit_stop_watchdog = gen8_emit_stop_watchdog; + } + return logical_ring_init(engine); } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index fffe69f5aed2..34ca0c5ac5b9 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -275,6 +275,10 @@ struct intel_engine_cs { int (*emit_flush)(struct drm_i915_gem_request *request, u32 mode); + u32 * (*emit_start_watchdog)(struct drm_i915_gem_request *req, + u32 *cs); + u32 * (*emit_stop_watchdog)(struct drm_i915_gem_request *req, + u32 *cs); #define EMIT_INVALIDATE BIT(0) #define EMIT_FLUSH BIT(1) #define EMIT_BARRIER (EMIT_INVALIDATE | EMIT_FLUSH)