[Intel-gfx] [PATCH v9 12/15] drm/i915/perf: Add OA unit support for Gen 8+

[Chris Wilson]

On Mon, May 01, 2017 at 06:17:09PM -0700, Lionel Landwerlin wrote:

Focusing on the bit I know best and leaving the hw mumbo jumbo to one
side...

> +static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_engine_cs *engine = dev_priv->engine[RCS];
> +	struct i915_gem_context *ctx;
> +	int ret;
> +
> +	ret = i915_mutex_lock_interruptible(&dev_priv->drm);
> +	if (ret)
> +		return ret;
> +
> +	/* Switch away from any user context.  */
> +	ret = i915_gem_switch_to_kernel_context(dev_priv);
> +	if (ret) {
> +		mutex_unlock(&dev_priv->drm.struct_mutex);
> +		return ret;
> +	}
> +
> +	/* The OA register config is setup through the context image. This image
> +	 * might be written to by the GPU on context switch (in particular on
> +	 * lite-restore). This means we can't safely update a context's image,
> +	 * if this context is scheduled/submitted to run on the GPU.
> +	 *
> +	 * We could emit the OA register config through the batch buffer but
> +	 * this might leave small interval of time where the OA unit is
> +	 * configured at an invalid sampling period.
> +	 *
> +	 * So far the best way to work around this issue seems to be draining
> +	 * the GPU from any submitted work.
> +	 */
> +	ret = i915_gem_wait_for_idle(dev_priv,
> +				     I915_WAIT_INTERRUPTIBLE |
> +				     I915_WAIT_LOCKED);
> +	if (ret) {
> +		mutex_unlock(&dev_priv->drm.struct_mutex);
> +		return ret;
> +	}
> +
> +	/* Update all contexts now that we've stalled the submission. */
> +	list_for_each_entry(ctx, &dev_priv->context_list, link) {
> +		ret = engine->context_pin(engine, ctx);
> +		if (ret) {
> +			mutex_unlock(&dev_priv->drm.struct_mutex);
> +			return ret;
> +		}
> +
> +		gen8_update_reg_state_unlocked(ctx,
> +					       ctx->engine[RCS].lrc_reg_state);

engine->context_unpin is missing.

Since you do populate the OA settings for contexts initialised later,
you can skip the context_pin here (keeping the deferred initialisation
for currently unused contexts) and use

	list_for_each_entry(ctx, &dev_priv->context_list, link) {
		struct intel_context *ce = &ctx->engine[RCS];
		u32 *regs;

		if (!ce->state) /* OA settings will be set upon first use */
			continue;

		/* Hmm, export this from intel_lrc.c? */
		regs = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
		if (IS_ERR(regs)) {
			/* teardown? */
			mutex_unlock(&ctx->i915->drm.struct_mutex);
			return PTR_ERR(regs);
		}
		ce->state->obj->mm.dirty = true;
		regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
		/* Probably best since we don't really want to expose
		 * the LRC_STATE_PN trick.
		 */

		gen8_update_reg_state_unlocked(ctx, regs);

		i915_gem_object_unpin_map(ce->state->obj);
	}

-- 
Chris Wilson, Intel Open Source Technology Centre