On Thu, Aug 18, 2016 at 11:42:37PM +0100, Robert Bragg wrote: > Each metric set is given a sysfs entry like: > > /sys/class/drm/card0/metrics/<guid>/id > > This allows userspace to enumerate the specific sets that are available > for the current system. The 'id' file contains an unsigned integer that > can be used to open the associated metric set via > DRM_IOCTL_I915_PERF_OPEN. The <guid> is a globally unique ID for a > specific OA unit register configuration that can be reliably used by > userspace as a key to lookup corresponding counter meta data and > normalization equations. > > The guid registry is currently maintained as part of gputop along with > the XML metric set descriptions and code generation scripts, ref: > > https://github.com/rib/gputop > > gputop-data/guids.xml > > scripts/update-guids.py > > gputop-data/oa-*.xml > > scripts/i915-perf-kernelgen.py > > $ make -C gputop-data -f Makefile.xml SYSFS=1 WHITELIST=RenderBasic > > Signed-off-by: Robert Bragg <robert@xxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 +++++ > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > drivers/gpu/drm/i915/i915_oa_hsw.c | 45 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_oa_hsw.h | 4 ++++ > drivers/gpu/drm/i915/i915_perf.c | 46 ++++++++++++++++++++++++++++++++++++++ > 5 files changed, 104 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 92f668e..0f5f51b 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -1172,6 +1172,9 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) > * cannot run before the connectors are registered. > */ > intel_fbdev_initial_config_async(dev); > + > + /* Depends on sysfs having been initialized */ > + i915_perf_register(dev_priv); That's only true if drm_dev_register() returns 0. So this needs to be in that branch, presumably. > +int > +i915_perf_init_sysfs_hsw(struct drm_i915_private *dev_priv) > +{ > + int ret; > + > + ret = sysfs_create_group(dev_priv->perf.metrics_kobj, &group_render_basic); > + if (ret) > + goto error_render_basic; > + > + return 0; > + > +error_render_basic: > + return ret; > +} > + > +void > +i915_perf_deinit_sysfs_hsw(struct drm_i915_private *dev_priv) I'd like these to be consistent with the phase, i.e. perf_register_sysfs perf_unregister_sysfs > +void i915_perf_register(struct drm_i915_private *dev_priv) > +{ > + if (!dev_priv->perf.initialized) > + return; > + > + /* To be sure we're synchronized with an attempted > + * i915_perf_open_ioctl(); considering that we register after > + * being exposed to userspace. > + */ > + mutex_lock(&dev_priv->perf.lock); > + > + dev_priv->perf.metrics_kobj = > + kobject_create_and_add("metrics", > + &dev_priv->drm.primary->kdev->kobj); > + if (!dev_priv->perf.metrics_kobj) > + goto exit; > + > + if (i915_perf_init_sysfs_hsw(dev_priv)) { If you say hsw only, I expect to see a local check or comment saying we are on Haswell. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel