Quoting Matthew Auld (2019-08-09 23:26:41) > From: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxx> > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > --- > 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel