[Intel-gfx] [PATCH v3 35/37] drm/i915: Introduce GEM_OBJECT_SETPARAM with I915_PARAM_MEMORY_REGION
Chris Wilson
chris at chris-wilson.co.uk
Sat Aug 10 11:54:54 UTC 2019
Quoting Matthew Auld (2019-08-09 23:26:41)
> From: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
>
> This call will specify which memory region an object should be placed.
>
> Note that changing the object's backing storage should be immediately
> done after an object is created or if it's not yet in use, otherwise
> this will fail on a busy object.
>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_context.c | 17 +++
> drivers/gpu/drm/i915/gem/i915_gem_context.h | 2 +
> drivers/gpu/drm/i915/gem/i915_gem_ioctls.h | 2 +
> drivers/gpu/drm/i915/gem/i915_gem_object.c | 115 ++++++++++++++++++++
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> include/uapi/drm/i915_drm.h | 23 ++++
> 6 files changed, 160 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index b407baaf0014..572033ac6e3b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -76,6 +76,7 @@
> #include "i915_globals.h"
> #include "i915_trace.h"
> #include "i915_user_extensions.h"
> +#include "i915_gem_ioctls.h"
>
> #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1
>
> @@ -2308,6 +2309,22 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> return ret;
> }
>
> +int i915_gem_setparam_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file)
This does not need to be in i915_gem_context.c
> +{
> + struct drm_i915_gem_context_param *args = data;
> +
switch(upper_32_bits(args->param)) {
> + case 0:
> + return i915_gem_context_setparam_ioctl(dev, data, file);
> + case 1:
> + return i915_gem_object_setparam_ioctl(dev, data, file);
> +
> + }
> + return -EINVAL;
> +}
> +
> int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file)
> {
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> index 106e2ccf7a4c..1cfcf1e6bbb9 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> @@ -157,6 +157,8 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> +int i915_gem_setparam_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file);
> int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> index 5abd5b2172f2..af7465bceebd 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_ioctls.h
> @@ -32,6 +32,8 @@ int i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> int i915_gem_mmap_offset_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_priv);
> +int i915_gem_object_setparam_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file_priv);
> int i915_gem_pread_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file);
> int i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 5982aeaaa2e3..52ea65f203a1 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -506,6 +506,121 @@ int __init i915_global_objects_init(void)
> return 0;
> }
>
> +static enum intel_region_id
> +__region_id(u32 region)
> +{
> + enum intel_region_id id;
> +
> + for (id = 0; id < INTEL_MEMORY_UKNOWN; ++id) {
> + if (intel_region_map[id] == region)
> + return id;
> + }
> +
> + return INTEL_MEMORY_UKNOWN;
> +}
> +
> +static int i915_gem_object_region_select(struct drm_i915_private *dev_priv,
> + struct drm_i915_gem_object_param *args,
> + struct drm_file *file,
> + struct drm_i915_gem_object *obj)
> +{
> + struct intel_context *ce = dev_priv->engine[BCS0]->kernel_context;
> + u32 __user *uregions = u64_to_user_ptr(args->data);
> + u32 uregions_copy[INTEL_MEMORY_UKNOWN];
> + int i, ret;
> +
> + if (args->size > INTEL_MEMORY_UKNOWN)
> + return -EINVAL;
args->size should be in bytes (as returned by the GETPARAM for this).
> + memset(uregions_copy, 0, sizeof(uregions_copy));
> + for (i = 0; i < args->size; i++) {
> + u32 region;
> +
> + ret = get_user(region, uregions);
> + if (ret)
> + return ret;
> +
> + uregions_copy[i] = region;
> + ++uregions;
> + }
> +
> + mutex_lock(&dev_priv->drm.struct_mutex);
Pardon?
> + ret = i915_gem_object_prepare_move(obj);
> + if (ret) {
> + DRM_ERROR("Cannot set memory region, object in use\n");
> + goto err;
> + }
> +
> + for (i = 0; i < args->size; i++) {
> + u32 region = uregions_copy[i];
> + enum intel_region_id id = __region_id(region);
> +
> + if (id == INTEL_MEMORY_UKNOWN) {
> + ret = -EINVAL;
> + goto err;
> + }
> +
> + ret = i915_gem_object_migrate(obj, ce, id);
> + if (!ret) {
> + if (!i915_gem_object_has_pages(obj) &&
> + MEMORY_TYPE_FROM_REGION(region) ==
> + INTEL_LMEM) {
> + /*
> + * TODO: this should be part of get_pages(),
> + * when async get_pages arrives
> + */
> + ret = i915_gem_object_fill_blt(obj, ce, 0);
> + if (ret) {
> + DRM_ERROR("Failed clearing the object\n");
> + goto err;
> + }
> +
> + i915_gem_object_lock(obj);
> + ret = i915_gem_object_set_to_cpu_domain(obj, false);
> + i915_gem_object_unlock(obj);
Pardon?
> + if (ret)
> + goto err;
> + }
> + break;
> + }
> + }
> +err:
> + mutex_unlock(&dev_priv->drm.struct_mutex);
> + return ret;
> +}
> +
> +int i915_gem_object_setparam_ioctl(struct drm_device *dev, void *data,
> + struct drm_file *file)
> +{
> +
> + struct drm_i915_gem_object_param *args = data;
> + struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_i915_gem_object *obj;
> + int ret;
> +
> + obj = i915_gem_object_lookup(file, args->handle);
> + if (!obj)
> + return -ENOENT;
> +
> + switch (lower_32_bits(args->param)) {
> + case I915_PARAM_MEMORY_REGION:
> + ret = i915_gem_object_region_select(dev_priv, args, file, obj);
> + if (ret) {
> + DRM_ERROR("Cannot set memory region, migration failed\n");
> + goto err;
> + }
> +
> + break;
> + default:
> + ret = -EINVAL;
> + break;
> + }
> +
> +err:
> + i915_gem_object_put(obj);
> + return ret;
> +}
> +
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> #include "selftests/huge_gem_object.c"
> #include "selftests/huge_pages.c"
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index f6a3daf696f6..845e80c2acc0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -2729,7 +2729,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_gem_context_reset_stats_ioctl, DRM_RENDER_ALLOW),
> 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_GEM_CONTEXT_SETPARAM, i915_gem_setparam_ioctl, DRM_RENDER_ALLOW),
> 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_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_RENDER_ALLOW),
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index fb84aed10825..75d79c17e91b 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -360,6 +360,7 @@ typedef struct _drm_i915_sarea {
> #define DRM_I915_GEM_VM_CREATE 0x3a
> #define DRM_I915_GEM_VM_DESTROY 0x3b
> #define DRM_I915_GEM_MMAP_OFFSET DRM_I915_GEM_MMAP_GTT
> +#define DRM_I915_GEM_OBJECT_SETPARAM DRM_I915_GEM_CONTEXT_SETPARAM
#define DRM_I915_GEM_SETPARAM 0x35
#define DRM_I915_GEM_GETPARAM 0x36
#define DRM_I915_GEM_CONTEXT_SETPARAM DRM_I915_GEM_SETPARAM
#define DRM_I915_GEM_CONTEXT_GETPARAM DRM_I915_GEM_GETPARAM
Make sure you do do both.
> /* Must be kept compact -- no holes */
>
> #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
> @@ -423,6 +424,7 @@ typedef struct _drm_i915_sarea {
> #define DRM_IOCTL_I915_GEM_VM_CREATE DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_CREATE, struct drm_i915_gem_vm_control)
> #define DRM_IOCTL_I915_GEM_VM_DESTROY DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_VM_DESTROY, struct drm_i915_gem_vm_control)
> #define DRM_IOCTL_I915_GEM_MMAP_OFFSET DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_MMAP_OFFSET, struct drm_i915_gem_mmap_offset)
> +#define DRM_IOCTL_I915_GEM_OBJECT_SETPARAM DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_OBJECT_SETPARAM, struct drm_i915_gem_object_param)
>
> /* Allow drivers to submit batchbuffers directly to hardware, relying
> * on the security mechanisms provided by hardware.
> @@ -1601,6 +1603,27 @@ struct drm_i915_gem_context_param {
> __u64 value;
> };
>
> +struct drm_i915_gem_object_param {
> + /** Handle for the object */
> + __u32 handle;
> +
> + __u32 size;
> +
> + /** Set the memory region for the object listed in preference order
/**
* Set
This is not kerneldoc anyway.
And the comment is not abutting the right variable.
> + * as an array of region ids within data. To force an object
> + * to a particular memory region, set the region as the sole entry.
> + *
> + * Valid region ids are derived from the id field of
> + * struct drm_i915_memory_region_info.
> + * See struct drm_i915_query_memory_region_info.
> + */
> +#define I915_OBJECT_PARAM (1ull<<32)
#define I915_OBJECT_PARAM_REGION (I915_OBJECT_PARAM | 0x1)
> + __u64 param;
> +
> + __u64 data;
Any particular reason for the divergent with the identical struct?
-Chris
More information about the dri-devel
mailing list