[Intel-gfx] [PATCH v2 34/37] drm/i915: Introduce GEM_OBJECT_SETPARAM with I915_PARAM_MEMORY_REGION
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Fri Jun 28 05:53:25 UTC 2019
On 27/06/2019 21:56, 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;
> + }
> +
> + 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;
What is this for? It's not used in the patch.
> +
> + /** 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.
> + */
These two structs only come in the next patch so I suspect the order of
the two needs swapping.
Regards,
Tvrtko
> +#define I915_PARAM_MEMORY_REGION 0x1
> + __u32 param;
> +
> + __u64 data;
> +};
> +
> /**
> * Context SSEU programming
> *
>
More information about the Intel-gfx
mailing list