| Message ID | 20181008151822.10519-2-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: serialized performance queries | expand |
Quoting Lionel Landwerlin (2018-10-08 16:18:20) > We would like to make use of perf in Vulkan. The Vulkan API is much > lower level than OpenGL, with applications directly exposed to the > concept of command buffers (pretty much equivalent to our batch > buffers). In Vulkan, queries are always limited in scope to a command > buffer. In OpenGL, the lack of command buffer concept meant that > queries' duration could span multiple command buffers. > > With that restriction gone in Vulkan, we would like to simplify > measuring performance just by measuring the deltas between the counter > snapshots written by 2 MI_RECORD_PERF_COUNT commands, rather than the > more complex scheme we currently have in the GL driver, using 2 > MI_RECORD_PERF_COUNT commands and doing some post processing on the > stream of OA reports, coming from the global OA buffer, to remove any > unrelated deltas in between the 2 MI_RECORD_PERF_COUNT. > > Disabling preemption only apply to a single context with which want to > query performance counters for and is considered a privileged > operation, by default protected by CAP_SYS_ADMIN. It is possible to > enable it for a normal user by disabling the paranoid stream setting. I'm am uncomfortable with disabling preemption like this. I suggest we at least have the preemption timeout in place first. -Chris
On 08/10/2018 16:35, Chris Wilson wrote: > Quoting Lionel Landwerlin (2018-10-08 16:18:20) >> We would like to make use of perf in Vulkan. The Vulkan API is much >> lower level than OpenGL, with applications directly exposed to the >> concept of command buffers (pretty much equivalent to our batch >> buffers). In Vulkan, queries are always limited in scope to a command >> buffer. In OpenGL, the lack of command buffer concept meant that >> queries' duration could span multiple command buffers. >> >> With that restriction gone in Vulkan, we would like to simplify >> measuring performance just by measuring the deltas between the counter >> snapshots written by 2 MI_RECORD_PERF_COUNT commands, rather than the >> more complex scheme we currently have in the GL driver, using 2 >> MI_RECORD_PERF_COUNT commands and doing some post processing on the >> stream of OA reports, coming from the global OA buffer, to remove any >> unrelated deltas in between the 2 MI_RECORD_PERF_COUNT. >> >> Disabling preemption only apply to a single context with which want to >> query performance counters for and is considered a privileged >> operation, by default protected by CAP_SYS_ADMIN. It is possible to >> enable it for a normal user by disabling the paranoid stream setting. > I'm am uncomfortable with disabling preemption like this. I suggest we > at least have the preemption timeout in place first. > -Chris > Sure, I'm not super happy about that either but that's the hardware we have to work with :/
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 8cbe58070561..c80655dbccc1 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -343,6 +343,7 @@ __create_hw_context(struct drm_i915_private *dev_priv, struct intel_context *ce = &ctx->__engine[n]; ce->gem_context = ctx; + ce->arb_enable = MI_ARB_ENABLE; } INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL); diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index f6d870b1f73e..295d53763ee0 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -171,6 +171,9 @@ struct i915_gem_context { int pin_count; const struct intel_context_ops *ops; + + /* arb_enable: Control preemption */ + u32 arb_enable; } __engine[I915_NUM_ENGINES]; /** ring_size: size for allocating the per-engine ring buffer */ diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 664b96bb65a3..e2a96b6844fe 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -354,6 +354,7 @@ struct perf_open_properties { u32 sample_flags; u64 single_context:1; + u64 context_disable_preemption:1; u64 ctx_handle; /* OA sampling state */ @@ -1360,6 +1361,12 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream) mutex_lock(&dev_priv->drm.struct_mutex); dev_priv->perf.oa.exclusive_stream = NULL; dev_priv->perf.oa.ops.disable_metric_set(dev_priv); + if (stream->ctx) { + struct intel_context *ce = + to_intel_context(stream->ctx, dev_priv->engine[RCS]); + + ce->arb_enable = MI_ARB_ENABLE; + } mutex_unlock(&dev_priv->drm.struct_mutex); free_oa_buffer(dev_priv); @@ -2099,6 +2106,13 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, goto err_enable; } + if (props->context_disable_preemption) { + struct intel_context *ce = + to_intel_context(stream->ctx, dev_priv->engine[RCS]); + + ce->arb_enable = MI_ARB_DISABLE; + } + stream->ops = &i915_oa_stream_ops; dev_priv->perf.oa.exclusive_stream = stream; @@ -2555,6 +2569,15 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, } } + if (props->context_disable_preemption) { + if (!props->single_context) { + DRM_DEBUG("preemption disable with no context\n"); + ret = -EINVAL; + goto err; + } + privileged_op = true; + } + /* * On Haswell the OA unit supports clock gating off for a specific * context and in this mode there's no visibility of metrics for the @@ -2569,8 +2592,10 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, * MI_REPORT_PERF_COUNT commands and so consider it a privileged op to * enable the OA unit by default. */ - if (IS_HASWELL(dev_priv) && specific_ctx) + if (IS_HASWELL(dev_priv) && specific_ctx && + !props->context_disable_preemption) { privileged_op = false; + } /* Similar to perf's kernel.perf_paranoid_cpu sysctl option * we check a dev.i915.perf_stream_paranoid sysctl option @@ -2579,7 +2604,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv, */ if (privileged_op && i915_perf_stream_paranoid && !capable(CAP_SYS_ADMIN)) { - DRM_DEBUG("Insufficient privileges to open system-wide i915 perf stream\n"); + DRM_DEBUG("Insufficient privileges to open i915 perf stream\n"); ret = -EACCES; goto err_ctx; } @@ -2771,6 +2796,9 @@ static int read_properties_unlocked(struct drm_i915_private *dev_priv, props->oa_periodic = true; props->oa_period_exponent = value; break; + case DRM_I915_PERF_PROP_HOLD_PREEMPTION: + props->context_disable_preemption = value != 0 ? 1 : 0; + break; case DRM_I915_PERF_PROP_MAX: MISSING_CASE(id); return -EINVAL; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index ff0e2b36cb8b..b240332838c1 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1896,7 +1896,7 @@ static int gen8_emit_bb_start(struct i915_request *rq, * * That satisfies both the GPGPU w/a and our heavy-handed paranoia. */ - *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE; + *cs++ = MI_ARB_ON_OFF | rq->hw_context->arb_enable; /* FIXME(BDW): Address space and security selectors. */ *cs++ = MI_BATCH_BUFFER_START_GEN8 | diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 298b2e197744..62f669030741 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -1540,6 +1540,14 @@ enum drm_i915_perf_property_id { */ DRM_I915_PERF_PROP_OA_EXPONENT, + /** + * Specifying this property is only valid when specify a context to + * filter with DRM_I915_PERF_PROP_CTX_HANDLE. Specifying this property + * will hold preemption of the particular context we want to gather + * performance data about. + */ + DRM_I915_PERF_PROP_HOLD_PREEMPTION, + DRM_I915_PERF_PROP_MAX /* non-ABI */ };
We would like to make use of perf in Vulkan. The Vulkan API is much lower level than OpenGL, with applications directly exposed to the concept of command buffers (pretty much equivalent to our batch buffers). In Vulkan, queries are always limited in scope to a command buffer. In OpenGL, the lack of command buffer concept meant that queries' duration could span multiple command buffers. With that restriction gone in Vulkan, we would like to simplify measuring performance just by measuring the deltas between the counter snapshots written by 2 MI_RECORD_PERF_COUNT commands, rather than the more complex scheme we currently have in the GL driver, using 2 MI_RECORD_PERF_COUNT commands and doing some post processing on the stream of OA reports, coming from the global OA buffer, to remove any unrelated deltas in between the 2 MI_RECORD_PERF_COUNT. Disabling preemption only apply to a single context with which want to query performance counters for and is considered a privileged operation, by default protected by CAP_SYS_ADMIN. It is possible to enable it for a normal user by disabling the paranoid stream setting. v2: Store preemption setting in intel_context (Chris) Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> --- drivers/gpu/drm/i915/i915_gem_context.c | 1 + drivers/gpu/drm/i915/i915_gem_context.h | 3 +++ drivers/gpu/drm/i915/i915_perf.c | 32 +++++++++++++++++++++++-- drivers/gpu/drm/i915/intel_lrc.c | 2 +- include/uapi/drm/i915_drm.h | 8 +++++++ 5 files changed, 43 insertions(+), 3 deletions(-)