<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 22, 2016 at 1:31 PM, Daniel Vetter <span dir="ltr"><<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Nov 22, 2016 at 02:29:18PM +0100, Daniel Vetter wrote:<br>
> On Wed, Nov 09, 2016 at 08:00:06PM +0000, Matthew Auld wrote:<br>
> > On 7 November 2016 at 19:49, Robert Bragg <<a href="mailto:robert@sixbynine.org">robert@sixbynine.org</a>> wrote:<br>
> > > Adds base i915 perf infrastructure for Gen performance metrics.<br>
> > ><br>
> > > This adds a DRM_IOCTL_I915_PERF_OPEN ioctl that takes an array of uint64<br>
> > > properties to configure a stream of metrics and returns a new fd usable<br>
> > > with standard VFS system calls including read() to read typed and sized<br>
> > > records; ioctl() to enable or disable capture and poll() to wait for<br>
> > > data.<br>
> > ><br>
> > > A stream is opened something like:<br>
> > ><br>
> > > uint64_t properties[] = {<br>
> > > /* Single context sampling */<br>
> > > DRM_I915_PERF_PROP_CTX_HANDLE, ctx_handle,<br>
> > ><br>
> > > /* Include OA reports in samples */<br>
> > > DRM_I915_PERF_PROP_SAMPLE_OA, true,<br>
> > ><br>
> > > /* OA unit configuration */<br>
> > > DRM_I915_PERF_PROP_OA_METRICS_<wbr>SET, metrics_set_id,<br>
> > > DRM_I915_PERF_PROP_OA_FORMAT, report_format,<br>
> > > DRM_I915_PERF_PROP_OA_<wbr>EXPONENT, period_exponent,<br>
> > > };<br>
> > > struct drm_i915_perf_open_param parm = {<br>
> > > .flags = I915_PERF_FLAG_FD_CLOEXEC |<br>
> > > I915_PERF_FLAG_FD_NONBLOCK |<br>
> > > I915_PERF_FLAG_DISABLED,<br>
> > > .properties_ptr = (uint64_t)properties,<br>
> > > .num_properties = sizeof(properties) / 16,<br>
> > > };<br>
> > > int fd = drmIoctl(drm_fd, DRM_IOCTL_I915_PERF_OPEN, ¶m);<br>
> > ><br>
> > > Records read all start with a common { type, size } header with<br>
> > > DRM_I915_PERF_RECORD_SAMPLE being of most interest. Sample records<br>
> > > contain an extensible number of fields and it's the<br>
> > > DRM_I915_PERF_PROP_SAMPLE_xyz properties given when opening that<br>
> > > determine what's included in every sample.<br>
> > ><br>
> > > No specific streams are supported yet so any attempt to open a stream<br>
> > > will return an error.<br>
> > ><br>
> > > v2:<br>
> > > use i915_gem_context_get() - Chris Wilson<br>
> > > v3:<br>
> > > update read() interface to avoid passing state struct - Chris Wilson<br>
> > > fix some rebase fallout, with i915-perf init/deinit<br>
> > > v4:<br>
> > > s/DRM_IORW/DRM_IOW/ - Emil Velikov<br>
> > ><br>
> > > Signed-off-by: Robert Bragg <<a href="mailto:robert@sixbynine.org">robert@sixbynine.org</a>><br>
> > > Reviewed-by: Matthew Auld <<a href="mailto:matthew.auld@intel.com">matthew.auld@intel.com</a>><br>
> > > Reviewed-by: Sourab Gupta <<a href="mailto:sourab.gupta@intel.com">sourab.gupta@intel.com</a>><br>
> > Minor nit, there are a fair few DRM_ERROR's missing a new line.<br>
><br>
> Also, DRM_ERROR for userspace-triggerable failures is no good. igt<br>
> testcase are supposed to exercise all the invalid stuff, and would then<br>
> fail if you spam dmesg. Why was this not caught?<br>
><br>
> Fixup patch totally fine, but if this wasn't caught due to missing igt<br>
> that needs to be fixed, too.<br>
<br>
</div></div>Another nitpick for the future: Enabling new features first and then<br>
fixing up the fallout is the wrong way round, if someone bisects over this<br>
range mesa might blow up in really bad ways.<br>
<br>
Oh well, this has been out there for way too long, so meh.<br></blockquote><div><br></div><div>Fwiw I'm aware of this, and think I've ordered the patches correctly to avoid bisect problems in Mesa / userspace. This infrastructure patch should have no fallout to fix for userspace. The command parser changes that affect userspace were done before adding oacontrol usage to i915-perf and the cmd parser's EINVAL reporting for access failures was changed *before* removing oacontrol from the whitelist.<br><br></div><div>Did I overlook something in particular?<br></div><div><br></div><div>- Robert<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">-Daniel<br>
--<br>
Daniel Vetter<br>
Software Engineer, Intel Corporation<br>
<a href="http://blog.ffwll.ch" rel="noreferrer" target="_blank">http://blog.ffwll.ch</a><br>
</div></div></blockquote></div><br></div></div>