[PATCH v7 01/11] drm/i915: Add i915 perf infrastructure

2016-10-25 Thread Matthew Auld
> +
> +/* Note we copy the properties from userspace outside of the i915 perf
> + * mutex to avoid an awkward lockdep with mmap_sem.
> + *
> + * Note this function only validates properties in isolation it doesn't
> + * validate that the combination of properties makes sense or that all
> + * properties necessary for a particular kind of stream have been set.
> + */
> +static int read_properties_unlocked(struct drm_i915_private *dev_priv,
> +   u64 __user *uprops,
> +   u32 n_props,
> +   struct perf_open_properties *props)
> +{
> +   u64 __user *uprop = uprops;
> +   int i;
> +
> +   memset(props, 0, sizeof(struct perf_open_properties));
> +
> +   if (!n_props) {
> +   DRM_ERROR("No i915 perf properties given");
> +   return -EINVAL;
> +   }
> +
> +   if (n_props > DRM_I915_PERF_PROP_MAX) {
> +   DRM_ERROR("More i915 perf properties specified than exist");
> +   return -EINVAL;
> +   }
> +
> +   for (i = 0; i < n_props; i++) {
> +   u64 id, value;
> +   int ret;
> +
> +   ret = get_user(id, (u64 __user *)uprop);
> +   if (ret)
> +   return ret;
> +
> +   ret = get_user(value, (u64 __user *)uprop + 1);
> +   if (ret)
> +   return ret;
Do we really need all of these __user casts, they seem redundant, no?

Otherwise looks good so:
Reviewed-by: Matthew Auld 


[PATCH v7 01/11] drm/i915: Add i915 perf infrastructure

2016-10-25 Thread Robert Bragg
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.

No specific streams are supported yet so any attempt to open a stream
will return an error.

v4:
s/DRM_IORW/DRM_IOW/ - Emil Velikov
v3:
update read() interface to avoid passing state struct - Chris Wilson
fix some rebase fallout, with i915-perf init/deinit
v2:
use i915_gem_context_get() - Chris Wilson

Signed-off-by: Robert Bragg 
---
 drivers/gpu/drm/i915/Makefile|   3 +
 drivers/gpu/drm/i915/i915_drv.c  |   4 +
 drivers/gpu/drm/i915/i915_drv.h  |  91 
 drivers/gpu/drm/i915/i915_perf.c | 443 +++
 include/uapi/drm/i915_drm.h  |  67 ++
 5 files changed, 608 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_perf.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 6123400..8d4e25f 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -113,6 +113,9 @@ i915-$(CONFIG_DRM_I915_CAPTURE_ERROR) += i915_gpu_error.o
 # virtual gpu code
 i915-y += i915_vgpu.o

+# perf code
+i915-y += i915_perf.o
+
 ifeq ($(CONFIG_DRM_I915_GVT),y)
 i915-y += intel_gvt.o
 include $(src)/gvt/Makefile
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 99e4e04..e99d14e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -836,6 +836,8 @@ static int i915_driver_init_early(struct drm_i915_private 
*dev_priv,

intel_detect_preproduction_hw(dev_priv);

+   i915_perf_init(dev_priv);
+
return 0;

 err_workqueues:
@@ -849,6 +851,7 @@ static int i915_driver_init_early(struct drm_i915_private 
*dev_priv,
  */
 static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
 {
+   i915_perf_fini(dev_priv);
i915_gem_load_cleanup(&dev_priv->drm);
i915_workqueues_cleanup(dev_priv);
 }
@@ -2554,6 +2557,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, 
DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, 
i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, 
i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, 
DRM_RENDER_ALLOW),
 };

 static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd3acab..fcc5958 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1764,6 +1764,84 @@ struct intel_wm_config {
bool sprites_scaled;
 };

+struct i915_perf_stream;
+
+struct i915_perf_stream_ops {
+   /* Enables the collection of HW samples, either in response to
+* I915_PERF_IOCTL_ENABLE or implicitly called when stream is
+* opened without I915_PERF_FLAG_DISABLED.
+*/
+   void (*enable)(struct i915_perf_stream *stream);
+
+   /* Disables the collection of HW samples, either in response to
+* I915_PERF_IOCTL_DISABLE or implicitly called before
+* destroying the stream.
+*/
+   void (*disable)(struct i915_perf_stream *stream);
+
+   /* Return: true if any i915 perf records are ready to read()
+* for this stream.
+*/
+   bool (*can_read)(struct i915_perf_stream *stream);
+
+   /* Call poll_wait, passing a wait queue that will be woken
+* once there is something ready to read() for the stream
+*/
+   void (*poll_wait)(struct i915_perf_stream *stream,
+