On Mon, Aug 15, 2016 at 03:41:18PM +0100, Robert Bragg 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.
>
> No specific streams are supported yet so any attempt to open a stream
> will return an error.
>
> Signed-off-by: Robert Bragg <robert@xxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/Makefile | 3 +
> drivers/gpu/drm/i915/i915_drv.c | 6 +
> drivers/gpu/drm/i915/i915_drv.h | 92 +++++++++
> drivers/gpu/drm/i915/i915_perf.c | 430 +++++++++++++++++++++++++++++++++++++++
> include/uapi/drm/i915_drm.h | 67 ++++++
> 5 files changed, 598 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 6092f0e..9a2f1f9 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -106,6 +106,9 @@ i915-y += dvo_ch7017.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 83afdd0..f5209ff 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1169,6 +1169,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_init(dev_priv);
Then please call it i915_perf_register() and i915_perf_unregister()
respectively.
> + struct {
> + bool initialized;
This is bogus. We simply cannot allow userspace to access internals
before we set them up. As it stands you have no serialisation here, so
the protect is moot.
i915_perf_init() needs to be split up into the early initialisation
function to setup internals, and the i915_perf_register() function to
enable userspace (with however many steps you need in between).
> + struct list_head streams;
> + } perf;
> +
> /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */
> struct {
> int (*execbuf_submit)(struct i915_execbuffer_params *params,
> @@ -3421,6 +3506,9 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
>
> +int i915_perf_open_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file);
> +
> /* i915_gem_evict.c */
> int __must_check i915_gem_evict_something(struct drm_device *dev,
> struct i915_address_space *vm,
> @@ -3540,6 +3628,10 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
> u32 batch_len,
> bool is_master);
>
> +/* i915_perf.c */
> +extern void i915_perf_init(struct drm_i915_private *dev_priv);
> +extern void i915_perf_fini(struct drm_i915_private *dev_priv);
> +
> /* i915_suspend.c */
> extern int i915_save_state(struct drm_device *dev);
> extern int i915_restore_state(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> new file mode 100644
> index 0000000..d0ed645
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -0,0 +1,430 @@
> +/*
> + * Copyright © 2015-2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Robert Bragg <robert@xxxxxxxxxxxxx>
> + */
> +
> +#include <linux/anon_inodes.h>
> +#include <linux/sizes.h>
> +
> +#include "i915_drv.h"
> +
> +struct perf_open_properties {
> + u32 sample_flags;
> +
> + u64 single_context:1;
> + u64 ctx_handle;
> +};
> +
> +static ssize_t i915_perf_read_locked(struct i915_perf_stream *stream,
> + struct file *file,
> + char __user *buf,
> + size_t count,
> + loff_t *ppos)
> +{
> + struct i915_perf_read_state state = { count, 0, buf };
> + int ret = stream->ops->read(stream, &state);
> +
> + /* If we've successfully copied any data then reporting that
> + * takes precedence over any internal error status, so the
> + * data isn't lost
> + */
> + return state.read ? state.read : (ret ? ret : -EAGAIN);
return state.read ?: ret ?: -EAGAIN;
Alternatively you could follow the standard pattern for read. Dare I ask
what is going to go into state that needs the obfuscation?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
