On Wed, Sep 14, 2016 at 3:42 PM, Emil Velikov emil.l.velikov@gmail.com wrote:
Hi Robert,
I think I've spotted one interesting, yet trivial bit.
On 14 September 2016 at 15:19, Robert Bragg robert@sixbynine.org wrote:
Adds base i915 perf infrastructure for Gen performance metrics.
This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of uint64 properties to configure a stream of metrics and returns a new fd usable with standard VFS system calls including read() to read typed and sized records; ioctl() to enable or disable capture and poll() to wait for data.
A stream is opened something like:
uint64_t properties[] = { /* Single context sampling */ DRM_I915_PERF_PROP_CTX_HANDLE, ctx_handle,
/* Include OA reports in samples */ DRM_I915_PERF_PROP_SAMPLE_OA, true, /* OA unit configuration */ DRM_I915_PERF_PROP_OA_METRICS_SET, metrics_set_id, DRM_I915_PERF_PROP_OA_FORMAT, report_format, DRM_I915_PERF_PROP_OA_EXPONENT, period_exponent,}; struct drm_i915_perf_open_param parm = { .flags = I915_PERF_FLAG_FD_CLOEXEC | I915_PERF_FLAG_FD_NONBLOCK | I915_PERF_FLAG_DISABLED, .properties_ptr = (uint64_t)properties, .num_properties = sizeof(properties) / 16, }; int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m);
Records read all start with a common { type, size } header with DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records contain an extensible number of fields and it's the DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that determine what's included in every sample.
If I'm understanding the above correctly the ioctl can only read user data and does not write to params, correct ?
--- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h
+#define DRM_IOCTL_I915_PERF_OPEN DRM_IOWR(DRM_COMMAND_BASE +
DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param)
If so, we seems to have a one letter too much in DRM_IOWR - should one use DRM_IOW/DRM_IOR ? Then again I'm not sure how many ioctls bother, so please don't read too much into my suggestion :-)
Ah, yep, good catch, I don't write back to the param struct any more.
The first iteration of this interface was even more closely modeled on the core linux perf interface where the param struct starts with a size member and in a case where userspace passes a structure that's smaller than expected the kernel returns an error but also writes back the expected size to inform userspace.
i915 perf moved to taking an array of u64 properties and no longer writes back a size member in the param struct like perf.
Thanks, - Robert
Regards, Emil