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@xxxxxxxxx>
> Signed-off-by: Ian Lister <ian.lister@xxxxxxxxx>
> Signed-off-by: Arun Siluvery <arun.siluvery@xxxxxxxxxxxxxxx>
> Signed-off-by: Michel Thierry <michel.thierry@xxxxxxxxx>
> ---
> 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
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
