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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
