[PATCH 24/28] drm/i915/uapi: implement object placement extension
Matthew Auld
matthew.auld at intel.com
Thu Apr 8 13:22:10 UTC 2021
Add new extension to support setting an immutable-priority-list of
potential placements, at creation time.
If we wish to set the placements/regions we can simply do:
struct drm_i915_gem_object_param region_param = { … }; /* Unchanged */
struct drm_i915_gem_create_ext_setparam setparam_region = {
.base = { .name = I915_GEM_CREATE_EXT_SETPARAM },
.param = region_param,
}
struct drm_i915_gem_create_ext create_ext = {
.size = 16 * PAGE_SIZE,
.extensions = (uintptr_t)&setparam_region,
};
int err = ioctl(fd, DRM_IOCTL_I915_GEM_CREATE_EXT, &create_ext);
if (err) ...
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.
One important change here is the returned size will now be rounded up to
the correct size, depending on the list of placements, where we might
have minimum page-size restrictions on some platforms when dealing with
device local-memory.
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 at intel.com>
Signed-off-by: CQ Tang <cq.tang at intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
---
drivers/gpu/drm/i915/gem/i915_gem_create.c | 210 ++++++++++++++++--
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 | 16 ++
7 files changed, 264 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 f12c64d8b40b..bf830b83d7e4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_create.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_create.c
@@ -4,12 +4,48 @@
*/
#include "gem/i915_gem_ioctls.h"
+#include "gem/i915_gem_lmem.h"
+#include "gem/i915_gem_object_blt.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 +65,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 +87,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;
}
@@ -61,6 +96,7 @@ i915_gem_dumb_create(struct drm_file *file,
struct drm_device *dev,
struct drm_mode_create_dumb *args)
{
+ struct intel_memory_region *stack[1];
struct drm_i915_gem_object *obj;
enum intel_memory_type mem_type;
int cpp = DIV_ROUND_UP(args->bpp, 8);
@@ -102,10 +138,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);
+ stack[0] = intel_memory_region_by_type(to_i915(dev), mem_type);
+ object_set_placements(obj, stack, 1);
+
+ ret = i915_gem_setup(obj, args->size);
if (ret)
goto object_free;
@@ -121,6 +157,130 @@ 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_object_param *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->data);
+ struct drm_i915_gem_object *obj = ext_data->vanilla_object;
+ struct intel_memory_region **placements;
+ u32 mask;
+ int i, ret = 0;
+
+ if (args->handle) {
+ DRM_DEBUG("Handle should be zero\n");
+ ret = -EINVAL;
+ }
+
+ if (!args->size) {
+ DRM_DEBUG("Size is zero\n");
+ ret = -EINVAL;
+ }
+
+ if (args->size > ARRAY_SIZE(i915->mm.regions)) {
+ DRM_DEBUG("Too many placements\n");
+ ret = -EINVAL;
+ }
+
+ if (ret)
+ return ret;
+
+ placements = kmalloc_array(args->size,
+ sizeof(struct intel_memory_region *),
+ GFP_KERNEL);
+ if (!placements)
+ return -ENOMEM;
+
+ mask = 0;
+ for (i = 0; i < args->size; 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_DEBUG("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_DEBUG("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->size);
+ if (args->size == 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_DEBUG("Placements were already set in previous SETPARAM. Existing placements: %s\n",
+ buf);
+ }
+
+ repr_placements(buf, sizeof(buf), placements, i);
+ DRM_DEBUG("New placements(so far validated): %s\n", buf);
+ }
+
+out_free:
+ kfree(placements);
+ return ret;
+}
+
static int __create_setparam(struct drm_i915_gem_object_param *args,
struct create_ext *ext_data)
{
@@ -129,6 +289,11 @@ static int __create_setparam(struct drm_i915_gem_object_param *args,
return -EINVAL;
}
+ switch (lower_32_bits(args->param)) {
+ case I915_PARAM_MEMORY_REGIONS:
+ return set_placements(args, ext_data);
+ }
+
return -EINVAL;
}
@@ -159,6 +324,8 @@ 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 intel_memory_region *stack[1];
struct drm_i915_gem_object *obj;
int ret;
@@ -176,19 +343,27 @@ 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) {
+ enum intel_memory_type mem_type = INTEL_MEMORY_SYSTEM;
+
+ stack[0] = intel_memory_region_by_type(i915, mem_type);
+ object_set_placements(obj, stack, 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;
}
@@ -205,6 +380,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 intel_memory_region *stack[1];
struct drm_i915_gem_object *obj;
int ret;
@@ -214,10 +390,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);
+ stack[0] = intel_memory_region_by_type(i915, INTEL_MEMORY_SYSTEM);
+ object_set_placements(obj, stack, 1);
+
+ ret = i915_gem_setup(obj, args->size);
if (ret)
goto object_free;
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 ac90b76a3fa0..5d08d923969a 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 5303efb9c871..c6a2642a2b0a 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1740,6 +1740,22 @@ struct drm_i915_gem_object_param {
*/
#define I915_OBJECT_PARAM (1ull<<32)
+/*
+ * I915_OBJECT_PARAM_MEMORY_REGIONS
+ *
+ * Set the data pointer with the desired set of placements in priority
+ * order(each entry must be unique and supported by the device), as an array of
+ * drm_i915_gem_memory_class_instance, or an equivalent layout of class:instance
+ * pair encodings. See DRM_I915_QUERY_MEMORY_REGIONS for how to query the
+ * supported regions.
+ *
+ * In this case the data pointer size should be the number of
+ * drm_i915_gem_memory_class_instance elements in the placements array.
+ *
+ */
+#define I915_PARAM_MEMORY_REGIONS 0
+#define I915_OBJECT_PARAM_MEMORY_REGIONS (I915_OBJECT_PARAM | \
+ I915_PARAM_MEMORY_REGIONS)
__u64 param;
/* Data value or pointer */
--
2.26.3
More information about the Intel-gfx-trybot
mailing list