[Intel-gfx] [PATCH] drm/i915/perf: allow holding preemption on filtered ctx

[Chris Wilson]

Quoting Lionel Landwerlin (2018-06-05 17:19:11)
> 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 with currently have in the GL driver, using 2
> MI_RECORD_PERF_COUNT commands and doing some post processing on the
> stream of OA reports to remove any unrelated deltas in the OA stream.
> 
> Disabling preemption only apply to the context with which want to
> query performance counters and is considered a privileged operation
> (disabled by default).

and protected by CAP_SYS_ADMIN (give or take it requiring root
privilege to let an ordinary user set it themselves).

Please make that clear in the commit log for the likes of myself who
panicked at the mere thought of being able to stop preemption (because
it's an outright priority inversion). Note that this will not escape a
fast reset timer so the batch had better be short, or they will end up
being banned.
 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 7d63c6d2f687..417ca93ea606 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2099,7 +2099,9 @@ 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 |

I'd prefer having it as a u32 enable/disable. Could it be intel_context
even, i.e. selectable on engine?

	*cs++ = MI_ARB_ON_OFF | rq->hw_context->arb_enable;
-Chris