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

[Chris Wilson]

On Tue, Apr 25, 2017 at 10:30:07AM -0700, Lionel Landwerlin wrote:
> +static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv)
> +{
> +	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) {
> +		if (!ctx->engine[RCS].initialised)
> +			continue;
> +

You need to pin the context here, otherwise there is not guarrantee that
the lrc_reg_state exists, or map it directly.

> +		gen8_update_reg_state_unlocked(ctx,
> +					       ctx->engine[RCS].lrc_reg_state);
> +	}
> +
> +	mutex_unlock(&dev_priv->drm.struct_mutex);
> +
> +	/* Now update the current context.

You don't need to. The current context is the kernel context, it is
scratch and never used by userspace so no oa-reports.

> +	 *
> +	 * Note: Using MMIO to update per-context registers requires
> +	 * some extra care...
> +	 */
> +	ret = gen8_begin_ctx_mmio(dev_priv);
> +	if (ret) {
> +		DRM_ERROR("Failed to bring RCS out of idle to update current ctx OA state\n");
> +		return ret;
> +	}
> +
> +	I915_WRITE(GEN8_OACTXCONTROL, ((dev_priv->perf.oa.period_exponent <<
> +					GEN8_OA_TIMER_PERIOD_SHIFT) |
> +				      (dev_priv->perf.oa.periodic ?
> +				       GEN8_OA_TIMER_ENABLE : 0) |
> +				      GEN8_OA_COUNTER_RESUME));
> +
> +	config_oa_regs(dev_priv, dev_priv->perf.oa.flex_regs,
> +			dev_priv->perf.oa.flex_regs_len);
> +
> +	gen8_end_ctx_mmio(dev_priv);

This entire chunk can go and I don't need to critique
gen8_begin_ctx_mmio() -- it needs a bit of tlc.

This patch is not ready.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre