[Intel-gfx] uABI / Removing DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig

[Michael Sartain]

I'm writing to try and make a case for Tvrtko's "Remove
DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig" patch:

  https://lists.freedesktop.org/archives/intel-gfx/2018-June/169052.html

Including the tracing team maintainers because this conversation intersects
with their area. Also Pierre-Loup from Valve since he convinced me to start and
work on gpuvis*, helped designed it, and Valve and Steam game developers are
using it.

For background, Valve Software paid me to write gpuvis to use with Steam,
games, and the AMD open source driver starting in early 2017. Gpuvis is a tool
much like GPUView** on Windows. These have some similarities to KernelShark,
but with the added functionality of parsing specific gpu driver tracepoints and
graphing detailed information about GPU work such as submit time, SW queuing,
HW execution times, etc. We also use the ftrace trace_marker functionality to
allow userland applications to insert data which can also be shown, plotted,
graphed, etc.

Gpuvis is hooked into Valve's SteamVR platform. When the developer clicks a
checkbox, ftrace is started in the background capturing the last ~ 3-5 seconds
of trace data in kernel ring buffers. Hitting F6 captures that data using
trace-cmd, saves it, and then launches gpuvis. Here is a screenshot from a
capture with GPU info and userland print events:

  https://i.imgur.com/XhuMuZu.png

Since Intel also has an open source driver with similar tracepoints, I added
i915 support to gpuvis as well. There are eight tracepoints that the i915
driver currently exposes. Five are enabled all the time, three are exposed only
when the DRM_I915_LOW_LEVEL_TRACEPOINTS Kconfig option is enabled, which it
typically isn't by default. This essentially means that you have to rebuild the
Linux kernel to get the full functionality for Intel traces that you get with
AMD out of the box.

  * i915_request_queue
  * i915_request_add
  * i915_request_wait_begin
  * i915_request_wait_end
  * intel_engine_notify
  * i915_request_submit [LOW_LEVEL]
  * i915_request_in [LOW_LEVEL]
  * i915_request_out [LOW_LEVEL]

The patch Tvrtko submitted removes the DRM_I915_LOW_LEVEL_TRACEPOINTS ifdef and
brings full Intel tracing functionality without requiring kernel rebuilds.

The two main arguments I've heard against this patch are:

(a) Why would application developers care for seeing level of detail which they
    can't control?

Gpuvis is also for driver developers but even if not, please don't try to
withhold information from us game developers. Valve paid for this tool and
worked directly with driver devs to get it working *exactly* like all of them
wanted. Oftentimes traces of game capture glitches are sent to driver
developers, and conversations go back and forth between the two. Gpuvis can
also present data in an easier to digest format turning low level tracepoints
like amdgpu_cs_ioctl, amdgpu_sched_run_job, and fence_signaled into "SW queue",
"HW queue", and "Execution durations". If you ever do need to look into the
black box though, that data is also there.

(b) We can't expose tracepoints since they'll now be a stable uABI, and things
    will change in the future.

This argument intersects the discussion going on with tracepoints being
exposed as stable uABIs: What happens when these tracepoints disappear, change,
or are un-implementable in the future and break user space. Tracepoints limit
our future freedom, etc.

> The introduced interface will have to provide the information for years
> and kernel versions to come ...

And:

> We need to be cautious about inventing too many metrics here. As Joonas
> pointed out several times, once we add these into a uAPI, we are stuck with
> them, and that could stifle our freedom to tweak the GuC scheduling policies.

And:

> We don't have the option where you can make a promise that "we won't
> complain if the interface breaks". If gpuvis is to be bundled to some
> distro and the users would experience breakage when updating their
> kernel (say, due to security update), it'd be irrelevant what the
> gpuvis team's opinions are and kernel team would be forced to fix the
> fallout due to the kernel policies.

For what it's worth, I agree with Ingo on this subject:

  http://lkml.iu.edu/hypermail/linux/kernel/1011.2/00991.html

That post was from 2010, but I think it's still quite relevant to keep in mind.

Also:

 1) Valve, game developers, and AMD driver developers have already been using
    gpuvis with tracepoints for over 1.5 years now.
 2) The AMD and Intel tracepoints have *already* each changed and gpuvis was
    just updated to work with the new tracepoints.
 3) To be clear, "break" in this case just means the app showing unknown
    tracepoints without pretty graphs, falling back to something ~ KernelShark.
 4) Five of the eight Intel tracepoints gpuvis is using are already outside the
    LOW_LEVEL Kconfig. Are these stable uABIs?

But ok, instead of using the current already working Linux tracepoint system,
we're going to write a new userland API for this functionality. Something like:

> My initial idea would be to have DRM subsystem level profiling
> interface with versioning and vendor-specific extensions. Similar to
> i915 error capture, but from sysfs. So DRM drivers (when instructed to
> do so) would start capturing the profiling information in ring buffer
> and freeze it when the application asks to. Then it could be restarted
> after reading the previous log, similar to i915 error capture.
>
> With the DRM subsystem level profiling interface, if we can make the
> usermode drivers amend to the log information with their API level
> information. And further, if through debugging extensions we allow the
> user to amend to the log all the way from shader level, I think we've
> far bypassed ftrace's purpose of tracing what is going on inside the
> kernel.

Linux currently has ftrace and perf, and now we're going to write another
profiling subsystem that exposes Intel gpu data? This system sure sounds a lot
like perf/ftrace. And btw, I'm still going to have to use perf/ftrace to get to
data from the other subsystems like sched, ext4, and CPU counters, etc. Plus
getting Valve to update their use of trace_marker, and we'll also have a new
set of tools like "perf" and "trace-cmd" for this?

All because exposing tracepoints automatically introduce uABIs that can't ever
change?

Ftrace and perf are fantastic, stable, very well known, and documented with
ecosystems built around them. AMD already is doing exactly what we are asking
for with tracepoints, and Intel has tracepoints that work right now. Please,
there must be a way we can enable those and use the current tracing systems
without it introducing a stable tracepoint uABI which promises it'll never
change until the end of days?

Thank you.
 -Mike

Full disclosure: I'm currently working at Intel right now.

[*] https://github.com/mikesart/gpuvis
[**] https://graphics.stanford.edu/~mdfisher/GPUView.html