Add new extension to support setting an immutable-priority-list of potential placements, at creation time.
If we use the normal gem_create or gem_create_ext without the extensions/placements then we still get the old behaviour with only placing the object in system memory.
v2(Daniel & Jason): - Add a bunch of kernel-doc - Simplify design for placements extension
Testcase: igt/gem_create/create-ext-placement-sanity-check Testcase: igt/gem_create/create-ext-placement-each Testcase: igt/gem_create/create-ext-placement-all Signed-off-by: Matthew Auld matthew.auld@intel.com Signed-off-by: CQ Tang cq.tang@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Lionel Landwerlin lionel.g.landwerlin@linux.intel.com Cc: Jordan Justen jordan.l.justen@intel.com Cc: Daniel Vetter daniel.vetter@intel.com Cc: Kenneth Graunke kenneth@whitecape.org Cc: Jason Ekstrand jason@jlekstrand.net Cc: Dave Airlie airlied@gmail.com Cc: dri-devel@lists.freedesktop.org Cc: mesa-dev@lists.freedesktop.org --- drivers/gpu/drm/i915/gem/i915_gem_create.c | 215 ++++++++++++++++-- drivers/gpu/drm/i915/gem/i915_gem_object.c | 3 + .../gpu/drm/i915/gem/i915_gem_object_types.h | 6 + .../drm/i915/gem/selftests/i915_gem_mman.c | 26 +++ drivers/gpu/drm/i915/intel_memory_region.c | 16 ++ drivers/gpu/drm/i915/intel_memory_region.h | 4 + include/uapi/drm/i915_drm.h | 62 +++++ 7 files changed, 315 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_create.c b/drivers/gpu/drm/i915/gem/i915_gem_create.c index 90e9eb6601b5..895f1666a8d3 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_create.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c @@ -4,12 +4,47 @@ */
#include "gem/i915_gem_ioctls.h" +#include "gem/i915_gem_lmem.h" #include "gem/i915_gem_region.h"
#include "i915_drv.h" #include "i915_trace.h" #include "i915_user_extensions.h"
+static u32 object_max_page_size(struct drm_i915_gem_object *obj) +{ + u32 max_page_size = 0; + int i; + + for (i = 0; i < obj->mm.n_placements; i++) { + struct intel_memory_region *mr = obj->mm.placements[i]; + + GEM_BUG_ON(!is_power_of_2(mr->min_page_size)); + max_page_size = max_t(u32, max_page_size, mr->min_page_size); + } + + GEM_BUG_ON(!max_page_size); + return max_page_size; +} + +static void object_set_placements(struct drm_i915_gem_object *obj, + struct intel_memory_region **placements, + unsigned int n_placements) +{ + GEM_BUG_ON(!n_placements); + + if (n_placements == 1) { + struct intel_memory_region *mr = placements[0]; + struct drm_i915_private *i915 = mr->i915; + + obj->mm.placements = &i915->mm.regions[mr->id]; + obj->mm.n_placements = 1; + } else { + obj->mm.placements = placements; + obj->mm.n_placements = n_placements; + } +} + static int i915_gem_publish(struct drm_i915_gem_object *obj, struct drm_file *file, u64 *size_p, @@ -29,14 +64,12 @@ static int i915_gem_publish(struct drm_i915_gem_object *obj, }
static int -i915_gem_setup(struct drm_i915_gem_object *obj, - struct intel_memory_region *mr, - u64 size) +i915_gem_setup(struct drm_i915_gem_object *obj, u64 size) { + struct intel_memory_region *mr = obj->mm.placements[0]; int ret;
- GEM_BUG_ON(!is_power_of_2(mr->min_page_size)); - size = round_up(size, mr->min_page_size); + size = round_up(size, object_max_page_size(obj)); if (size == 0) return -EINVAL;
@@ -53,6 +86,7 @@ i915_gem_setup(struct drm_i915_gem_object *obj, GEM_BUG_ON(size != obj->base.size);
trace_i915_gem_object_create(obj); + return 0; }
@@ -62,6 +96,7 @@ i915_gem_dumb_create(struct drm_file *file, struct drm_mode_create_dumb *args) { struct drm_i915_gem_object *obj; + struct intel_memory_region *mr; enum intel_memory_type mem_type; int cpp = DIV_ROUND_UP(args->bpp, 8); u32 format; @@ -102,10 +137,10 @@ i915_gem_dumb_create(struct drm_file *file, if (!obj) return -ENOMEM;
- ret = i915_gem_setup(obj, - intel_memory_region_by_type(to_i915(dev), - mem_type), - args->size); + mr = intel_memory_region_by_type(to_i915(dev), mem_type); + object_set_placements(obj, &mr, 1); + + ret = i915_gem_setup(obj, args->size); if (ret) goto object_free;
@@ -129,6 +164,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, struct drm_i915_private *i915 = to_i915(dev); struct drm_i915_gem_create *args = data; struct drm_i915_gem_object *obj; + struct intel_memory_region *mr; int ret;
i915_gem_flush_free_objects(i915); @@ -137,10 +173,10 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data, if (!obj) return -ENOMEM;
- ret = i915_gem_setup(obj, - intel_memory_region_by_type(i915, - INTEL_MEMORY_SYSTEM), - args->size); + mr = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM); + object_set_placements(obj, &mr, 1); + + ret = i915_gem_setup(obj, args->size); if (ret) goto object_free;
@@ -156,7 +192,144 @@ struct create_ext { struct drm_i915_gem_object *vanilla_object; };
+static void repr_placements(char *buf, size_t size, + struct intel_memory_region **placements, + int n_placements) +{ + int i; + + buf[0] = '\0'; + + for (i = 0; i < n_placements; i++) { + struct intel_memory_region *mr = placements[i]; + int r; + + r = snprintf(buf, size, "\n %s -> { class: %d, inst: %d }", + mr->name, mr->type, mr->instance); + if (r >= size) + return; + + buf += r; + size -= r; + } +} + +static int set_placements(struct drm_i915_gem_create_ext_memory_regions *args, + struct create_ext *ext_data) +{ + struct drm_i915_private *i915 = ext_data->i915; + struct drm_i915_gem_memory_class_instance __user *uregions = + u64_to_user_ptr(args->regions); + struct drm_i915_gem_object *obj = ext_data->vanilla_object; + struct intel_memory_region **placements; + u32 mask; + int i, ret = 0; + + if (args->pad) { + drm_dbg(&i915->drm, "pad should be zero\n"); + ret = -EINVAL; + } + + if (!args->num_regions) { + drm_dbg(&i915->drm, "num_regions is zero\n"); + ret = -EINVAL; + } + + if (args->num_regions > ARRAY_SIZE(i915->mm.regions)) { + drm_dbg(&i915->drm, "num_regions is too large\n"); + ret = -EINVAL; + } + + if (ret) + return ret; + + placements = kmalloc_array(args->num_regions, + sizeof(struct intel_memory_region *), + GFP_KERNEL); + if (!placements) + return -ENOMEM; + + mask = 0; + for (i = 0; i < args->num_regions; i++) { + struct drm_i915_gem_memory_class_instance region; + struct intel_memory_region *mr; + + if (copy_from_user(®ion, uregions, sizeof(region))) { + ret = -EFAULT; + goto out_free; + } + + mr = intel_memory_region_lookup(i915, + region.memory_class, + region.memory_instance); + if (!mr || mr->private) { + drm_dbg(&i915->drm, "Device is missing region { class: %d, inst: %d } at index = %d\n", + region.memory_class, region.memory_instance, i); + ret = -EINVAL; + goto out_dump; + } + + if (mask & BIT(mr->id)) { + drm_dbg(&i915->drm, "Found duplicate placement %s -> { class: %d, inst: %d } at index = %d\n", + mr->name, region.memory_class, + region.memory_instance, i); + ret = -EINVAL; + goto out_dump; + } + + placements[i] = mr; + mask |= BIT(mr->id); + + ++uregions; + } + + if (obj->mm.placements) { + ret = -EINVAL; + goto out_dump; + } + + object_set_placements(obj, placements, args->num_regions); + if (args->num_regions == 1) + kfree(placements); + + return 0; + +out_dump: + if (1) { + char buf[256]; + + if (obj->mm.placements) { + repr_placements(buf, + sizeof(buf), + obj->mm.placements, + obj->mm.n_placements); + drm_dbg(&i915->drm, + "Placements were already set in previous EXT. Existing placements: %s\n", + buf); + } + + repr_placements(buf, sizeof(buf), placements, i); + drm_dbg(&i915->drm, "New placements(so far validated): %s\n", buf); + } + +out_free: + kfree(placements); + return ret; +} + +static int ext_set_placements(struct i915_user_extension __user *base, + void *data) +{ + struct drm_i915_gem_create_ext_memory_regions ext; + + if (copy_from_user(&ext, base, sizeof(ext))) + return -EFAULT; + + return set_placements(&ext, data); +} + static const i915_user_extension_fn create_extensions[] = { + [I915_GEM_CREATE_EXT_MEMORY_REGIONS] = ext_set_placements, };
/** @@ -172,6 +345,7 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, struct drm_i915_private *i915 = to_i915(dev); struct drm_i915_gem_create_ext *args = data; struct create_ext ext_data = { .i915 = i915 }; + struct intel_memory_region **placements_ext; struct drm_i915_gem_object *obj; int ret;
@@ -189,19 +363,26 @@ i915_gem_create_ext_ioctl(struct drm_device *dev, void *data, create_extensions, ARRAY_SIZE(create_extensions), &ext_data); + placements_ext = obj->mm.placements; if (ret) goto object_free;
- ret = i915_gem_setup(obj, - intel_memory_region_by_type(i915, - INTEL_MEMORY_SYSTEM), - args->size); + if (!placements_ext) { + struct intel_memory_region *mr = + intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM); + + object_set_placements(obj, &mr, 1); + } + + ret = i915_gem_setup(obj, args->size); if (ret) goto object_free;
return i915_gem_publish(obj, file, &args->size, &args->handle);
object_free: + if (obj->mm.n_placements > 1) + kfree(placements_ext); i915_gem_object_free(obj); return ret; } diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c index ea74cbca95be..28144410df86 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c @@ -249,6 +249,9 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915, if (obj->ops->release) obj->ops->release(obj);
+ if (obj->mm.n_placements > 1) + kfree(obj->mm.placements); + /* But keep the pointer alive for RCU-protected lookups */ call_rcu(&obj->rcu, __i915_gem_free_object_rcu); cond_resched(); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h index 8e485cb3343c..69d6e54bc569 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h @@ -219,6 +219,12 @@ struct drm_i915_gem_object { atomic_t pages_pin_count; atomic_t shrink_pin;
+ /** + * Priority list of potential placements for this object. + */ + struct intel_memory_region **placements; + int n_placements; + /** * Memory region for this object. */ diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c index 5cf6df49c333..05a3b29f545e 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -842,6 +842,24 @@ static bool can_mmap(struct drm_i915_gem_object *obj, enum i915_mmap_type type) return true; }
+static void object_set_placements(struct drm_i915_gem_object *obj, + struct intel_memory_region **placements, + unsigned int n_placements) +{ + GEM_BUG_ON(!n_placements); + + if (n_placements == 1) { + struct drm_i915_private *i915 = to_i915(obj->base.dev); + struct intel_memory_region *mr = placements[0]; + + obj->mm.placements = &i915->mm.regions[mr->id]; + obj->mm.n_placements = 1; + } else { + obj->mm.placements = placements; + obj->mm.n_placements = n_placements; + } +} + #define expand32(x) (((x) << 0) | ((x) << 8) | ((x) << 16) | ((x) << 24)) static int __igt_mmap(struct drm_i915_private *i915, struct drm_i915_gem_object *obj, @@ -950,6 +968,8 @@ static int igt_mmap(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj);
+ object_set_placements(obj, &mr, 1); + err = __igt_mmap(i915, obj, I915_MMAP_TYPE_GTT); if (err == 0) err = __igt_mmap(i915, obj, I915_MMAP_TYPE_WC); @@ -1068,6 +1088,8 @@ static int igt_mmap_access(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj);
+ object_set_placements(obj, &mr, 1); + err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_GTT); if (err == 0) err = __igt_mmap_access(i915, obj, I915_MMAP_TYPE_WB); @@ -1211,6 +1233,8 @@ static int igt_mmap_gpu(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj);
+ object_set_placements(obj, &mr, 1); + err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_GTT); if (err == 0) err = __igt_mmap_gpu(i915, obj, I915_MMAP_TYPE_WC); @@ -1354,6 +1378,8 @@ static int igt_mmap_revoke(void *arg) if (IS_ERR(obj)) return PTR_ERR(obj);
+ object_set_placements(obj, &mr, 1); + err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_GTT); if (err == 0) err = __igt_mmap_revoke(i915, obj, I915_MMAP_TYPE_WC); diff --git a/drivers/gpu/drm/i915/intel_memory_region.c b/drivers/gpu/drm/i915/intel_memory_region.c index 481a487faca6..d98e8b81d322 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.c +++ b/drivers/gpu/drm/i915/intel_memory_region.c @@ -28,6 +28,22 @@ static const struct { }, };
+struct intel_memory_region * +intel_memory_region_lookup(struct drm_i915_private *i915, + u16 class, u16 instance) +{ + struct intel_memory_region *mr; + int id; + + /* XXX: consider maybe converting to an rb tree at some point */ + for_each_memory_region(mr, i915, id) { + if (mr->type == class && mr->instance == instance) + return mr; + } + + return NULL; +} + struct intel_memory_region * intel_memory_region_by_type(struct drm_i915_private *i915, enum intel_memory_type mem_type) diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h index 7cd8e3d66a7f..d24ce5a0b30b 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.h +++ b/drivers/gpu/drm/i915/intel_memory_region.h @@ -97,6 +97,10 @@ struct intel_memory_region { } objects; };
+struct intel_memory_region * +intel_memory_region_lookup(struct drm_i915_private *i915, + u16 class, u16 instance); + int intel_memory_region_init_buddy(struct intel_memory_region *mem); void intel_memory_region_release_buddy(struct intel_memory_region *mem);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 47a47b87380f..d025f7da2735 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -2591,6 +2591,11 @@ struct drm_i915_gem_create_ext { * * The (page-aligned) allocated size for the object will be returned. * + * Note that for some devices we have might have further minimum + * page-size restrictions(larger than 4K), like for device local-memory. + * However in general the final size here should always reflect any + * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS + * extension to place the object in device local-memory. */ __u64 size; /** @@ -2611,10 +2616,67 @@ struct drm_i915_gem_create_ext { * If we don't supply any extensions then we get the same old gem_create * behaviour. * + * For I915_GEM_CREATE_EXT_MEMORY_REGIONS usage see + * struct drm_i915_gem_create_ext_memory_regions. */ +#define I915_GEM_CREATE_EXT_MEMORY_REGIONS 0 __u64 extensions; };
+/** + * struct drm_i915_gem_create_ext_memory_regions - The + * I915_GEM_CREATE_EXT_MEMORY_REGIONS extension. + * + * Set the object with the desired set of placements/regions in priority + * order. Each entry must be unique and supported by the device. + * + * This is provided as an array of struct drm_i915_gem_memory_class_instance, or + * an equivalent layout of class:instance pair encodings. See struct + * drm_i915_query_memory_regions and DRM_I915_QUERY_MEMORY_REGIONS for how to + * query the supported regions for a device. + * + * As an example, on discrete devices, if we wish to set the placement as + * device local-memory we can do something like: + * + * .. code-block:: C + * + * struct drm_i915_gem_memory_class_instance region_lmem = { + * .memory_class = I915_MEMORY_CLASS_DEVICE, + * .memory_instance = 0, + * }; + * struct drm_i915_gem_create_ext_memory_regions regions = { + * .base = { .name = I915_GEM_CREATE_EXT_MEMORY_REGIONS }, + * .regions = (uintptr_t)®ion_lmem, + * .num_regions = 1, + * }; + * struct drm_i915_gem_create_ext create_ext = { + * .size = 16 * PAGE_SIZE, + * .extensions = (uintptr_t)®ions, + * }; + * + * int err = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext); + * if (err) ... + * + * At which point we get the object handle in &drm_i915_gem_create_ext.handle, + * along with the final object size in &drm_i915_gem_create_ext.size, which + * should account for any rounding up, if required. + */ +struct drm_i915_gem_create_ext_memory_regions { + /** @base: Extension link. See struct i915_user_extension. */ + struct i915_user_extension base; + + /** @pad: MBZ */ + __u32 pad; + /** @num_regions: Number of elements in the @regions array. */ + __u32 num_regions; + /** + * @regions: The regions/placements array. + * + * An array of struct drm_i915_gem_memory_class_instance. + */ + __u64 regions; +}; + #if defined(__cplusplus) } #endif