[Intel-gfx] [PATCH v8 5/6] drm/i915: add query uAPI

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Thu Jan 18 12:36:31 UTC 2018


On 17/01/2018 15:39, Lionel Landwerlin wrote:
> There are a number of information that are readable from hardware
> registers and that we would like to make accessible to userspace. One
> particular example is the topology of the execution units (how are
> execution units grouped in subslices and slices and also which ones
> have been fused off for die recovery).
> 
> At the moment the GET_PARAM ioctl covers some basic needs, but
> generally is only able to return a single value for each defined
> parameter. This is a bit problematic with topology descriptions which
> are array/maps of available units.
> 
> This change introduces a new ioctl that can deal with requests to fill
> structures of potentially variable lengths. The user is expected fill
> a query with length fields set at 0 on the first call, the kernel then
> sets the length fields to the their expected values. A second call to
> the kernel with length fields at their expected values will trigger a
> copy of the data to the pointed memory locations.
> 
> The scope of this uAPI is only to provide information to userspace,
> not to allow configuration of the device.
> 
> v2: Simplify dispatcher code iteration (Tvrtko)
>      Tweak uapi drm_i915_query_item structure (Tvrtko)
> 
> v3: Rename pad fields into flags (Chris)
>      Return error on flags field != 0 (Chris)
>      Only copy length back to userspace in drm_i915_query_item (Chris)
> 
> v4: Use array of functions instead of switch (Chris)
> 
> v5: More comments in uapi (Tvrtko)
>      Return query item errors in length field (All)
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile     |  1 +
>   drivers/gpu/drm/i915/i915_drv.c   |  1 +
>   drivers/gpu/drm/i915/i915_drv.h   |  3 ++
>   drivers/gpu/drm/i915/i915_query.c | 67 +++++++++++++++++++++++++++++++++++++++
>   include/uapi/drm/i915_drm.h       | 37 +++++++++++++++++++++
>   5 files changed, 109 insertions(+)
>   create mode 100644 drivers/gpu/drm/i915/i915_query.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 3bddd8a06806..b0415a3e2d59 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -69,6 +69,7 @@ i915-y += i915_cmd_parser.o \
>   	  i915_gem_timeline.o \
>   	  i915_gem_userptr.o \
>   	  i915_gemfs.o \
> +	  i915_query.o \
>   	  i915_trace_points.o \
>   	  i915_vma.o \
>   	  intel_breadcrumbs.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a4cf6858e39c..e7098cf07ef5 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2826,6 +2826,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
>   	DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW),
>   	DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>   	DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_UNLOCKED|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 c42015b05b47..b2615a88936e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3622,6 +3622,9 @@ extern void i915_perf_fini(struct drm_i915_private *dev_priv);
>   extern void i915_perf_register(struct drm_i915_private *dev_priv);
>   extern void i915_perf_unregister(struct drm_i915_private *dev_priv);
>   
> +/* i915_query.c */
> +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file);
> +
>   /* i915_suspend.c */
>   extern int i915_save_state(struct drm_i915_private *dev_priv);
>   extern int i915_restore_state(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> new file mode 100644
> index 000000000000..5aa886313cf9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -0,0 +1,67 @@
> +/*
> + * Copyright © 2017 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.
> + *
> + */
> +
> +#include "i915_drv.h"
> +#include <uapi/drm/i915_drm.h>
> +
> +static int (* const i915_query_funcs[])(struct drm_i915_private *dev_priv,
> +					struct drm_i915_query_item *query_item) = {
> +};
> +
> +int i915_query_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_i915_query *args = data;
> +	struct drm_i915_query_item __user *user_item_ptr =
> +		u64_to_user_ptr(args->items_ptr);
> +	u32 i;
> +
> +	if (args->flags != 0)
> +		return -EINVAL;
> +
> +	for (i = 0; i < args->num_items; i++, user_item_ptr++) {
> +		struct drm_i915_query_item item;
> +		u64 func_idx;
> +		int ret;
> +
> +		if (copy_from_user(&item, user_item_ptr, sizeof(item)))
> +			return -EFAULT;
> +
> +		if (item.query_id == 0)
> +			return -EINVAL;
> +
> +		func_idx = item.query_id - 1;
> +
> +		if (func_idx >= ARRAY_SIZE(i915_query_funcs))
> +			return -EINVAL;
> +
> +		ret = i915_query_funcs[func_idx](dev_priv, &item);
> +
> +		/* Only write the length back to userspace if they differ. */
> +		if (ret != item.length && put_user(ret, &user_item_ptr->length))
> +			return -EFAULT;
> +	}
> +
> +	return 0;
> +}
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 536ee4febd74..d94a00380bf6 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -318,6 +318,7 @@ typedef struct _drm_i915_sarea {
>   #define DRM_I915_PERF_OPEN		0x36
>   #define DRM_I915_PERF_ADD_CONFIG	0x37
>   #define DRM_I915_PERF_REMOVE_CONFIG	0x38
> +#define DRM_I915_QUERY			0x39
>   
>   #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
>   #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
> @@ -375,6 +376,7 @@ typedef struct _drm_i915_sarea {
>   #define DRM_IOCTL_I915_PERF_OPEN	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param)
>   #define DRM_IOCTL_I915_PERF_ADD_CONFIG	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config)
>   #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64)
> +#define DRM_IOCTL_I915_QUERY			DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query)
>   
>   /* Allow drivers to submit batchbuffers directly to hardware, relying
>    * on the security mechanisms provided by hardware.
> @@ -1613,6 +1615,41 @@ struct drm_i915_perf_oa_config {
>   	__u64 flex_regs_ptr;
>   };
>   
> +
> +struct drm_i915_query_item {
> +	__u64 query_id;
> +
> +	/*
> +	 * When set to zero by userspace, this is filled with the size of the
> +	 * data to be written at the data_ptr pointer. The kernel set this
> +	 * value to a negative value to signal an error on a particular query
> +	 * item.
> +	 */
> +	__s32 length;
> +
> +	/* Unused for now. */
> +	__u32 flags;
> +
> +	/*
> +	 * Data will be written at the location pointed by data_ptr when the
> +	 * value of length matches the length of the data to be written by the
> +	 * kernel.
> +	 */
> +	__u64 data_ptr;
> +};
> +
> +struct drm_i915_query {
> +	__u32 num_items;
> +
> +	/* Unused for now. */
> +	__u32 flags;
> +
> +	/*
> +	 * This point to an array of num_items drm_i915_query_item structures.
> +	 */
> +	__u64 items_ptr;
> +};
> +
>   #if defined(__cplusplus)
>   }
>   #endif
> 

Looks good to me.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko


More information about the Intel-gfx mailing list