[Intel-gfx] [PATCH v2 34/37] drm/i915: Introduce GEM_OBJECT_SETPARAM with I915_PARAM_MEMORY_REGION
Daniel Vetter
daniel at ffwll.ch
Tue Jul 30 16:17:24 UTC 2019
On Thu, Jun 27, 2019 at 09:56:30PM +0100, Matthew Auld wrote:
> 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 | 12 ++
> 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 | 117 ++++++++++++++++++++
> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> include/uapi/drm/i915_drm.h | 27 +++++
> 6 files changed, 161 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 8a9787cf0cd0..157ca8247752 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -75,6 +75,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
>
> @@ -2357,6 +2358,17 @@ 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)
> +{
> + struct drm_i915_gem_context_param *args = data;
> +
> + if (args->param <= I915_CONTEXT_PARAM_MAX)
> + return i915_gem_context_setparam_ioctl(dev, data, file);
> +
> + return i915_gem_object_setparam_ioctl(dev, data, 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_context.h b/drivers/gpu/drm/i915/gem/i915_gem_context.h
> index 9691dd062f72..d5a9a63bb34c 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 691af388e4e7..bc95f449de50 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -551,6 +551,123 @@ 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 < ARRAY_SIZE(intel_region_map); ++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 > ARRAY_SIZE(intel_region_map))
> + return -EINVAL;
> +
> + 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);
> + ret = i915_gem_object_prepare_move(obj);
> + if (ret) {
> + DRM_ERROR("Cannot set memory region, object in use\n");
> + goto err;
So if all that's changed is the priority of allocations, but not the
overall list, will we allow this?
I think this will be needed for GL, where figuring out the usage pattern
of a given upload buffer is very much an observational thing in many
cases. And we might later change from a lmem, then smem priority order to
preferring smem (but still allowing lmem so that the uapi of the bo
doesn't change).
Also if we go with this would be nice to make that list of possible
allocations static, since I think the plan is to use that to limit what's
possible wrt uapi (stuff like mmap, but also pwrite/pread, and all that).
-Daniel
> + }
> +
> + if (args->size > ARRAY_SIZE(intel_region_map))
> + return -EINVAL;
> +
> + 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 (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);
> + 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 (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 1c3d5cb2893c..3d6fe993f26e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -3196,7 +3196,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 729e729e2282..5cf976e7608a 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
> /* 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.
> @@ -1595,11 +1597,36 @@ struct drm_i915_gem_context_param {
> * i915_context_engines_bond (I915_CONTEXT_ENGINES_EXT_BOND)
> */
> #define I915_CONTEXT_PARAM_ENGINES 0xa
> +
> +#define I915_CONTEXT_PARAM_MAX 0xffffffff
> /* Must be kept compact -- no holes and well documented */
>
> __u64 value;
> };
>
> +struct drm_i915_gem_object_param {
> + /** Handle for the object */
> + __u32 handle;
> +
> + __u32 size;
> +
> + /* Must be 1 */
> + __u32 object_class;
> +
> + /** Set the memory region for the object listed in preference order
> + * 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_PARAM_MEMORY_REGION 0x1
> + __u32 param;
> +
> + __u64 data;
> +};
> +
> /**
> * Context SSEU programming
> *
> --
> 2.20.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list