[PATCH] [RFC] drm/i915: make object creation avoid regions layering.
Dave Airlie
airlied at gmail.com
Mon Aug 31 20:33:43 UTC 2020
From: Dave Airlie <airlied at redhat.com>
This looked like indirect ptr for not much reason in the create
object path, I just wonder why it couldn't be simpler like this,
The tests aren't cleaned up but this was more of is this a good idea
test patch.
---
drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 16 ++++-------
drivers/gpu/drm/i915/gem/i915_gem_region.c | 33 +++++++++-------------
drivers/gpu/drm/i915/gem/i915_gem_region.h | 6 ++--
drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 22 ++++++---------
drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 24 ++++++----------
drivers/gpu/drm/i915/i915_gem.c | 29 ++++++++-----------
drivers/gpu/drm/i915/intel_memory_region.h | 5 ----
drivers/gpu/drm/i915/intel_region_lmem.c | 1 -
8 files changed, 50 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
index 932ee21e6609..710fb1158904 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
@@ -27,18 +27,14 @@ i915_gem_object_create_lmem(struct drm_i915_private *i915,
resource_size_t size,
unsigned int flags)
{
- return i915_gem_object_create_region(i915->mm.regions[INTEL_REGION_LMEM],
- size, flags);
-}
-
-struct drm_i915_gem_object *
-__i915_gem_lmem_object_create(struct intel_memory_region *mem,
- resource_size_t size,
- unsigned int flags)
-{
+ struct intel_memory_region *mem = i915->mm.regions[INTEL_REGION_LMEM];
static struct lock_class_key lock_class;
- struct drm_i915_private *i915 = mem->i915;
struct drm_i915_gem_object *obj;
+ int ret;
+
+ ret = i915_gem_object_pre_check(mem, &size, flags);
+ if (ret)
+ return ERR_PTR(ret);
obj = i915_gem_object_alloc();
if (!obj)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c b/drivers/gpu/drm/i915/gem/i915_gem_region.c
index 1515384d7e0e..0902b3790e70 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c
@@ -133,13 +133,12 @@ void i915_gem_object_release_memory_region(struct drm_i915_gem_object *obj)
intel_memory_region_put(mem);
}
-struct drm_i915_gem_object *
-i915_gem_object_create_region(struct intel_memory_region *mem,
- resource_size_t size,
+int i915_gem_object_pre_check(struct intel_memory_region *mem,
+ resource_size_t *size,
unsigned int flags)
{
- struct drm_i915_gem_object *obj;
+ GEM_BUG_ON(!is_power_of_2(mem->min_page_size));
/*
* NB: Our use of resource_size_t for the size stems from using struct
* resource for the mem->region. We might need to revisit this in the
@@ -148,13 +147,13 @@ i915_gem_object_create_region(struct intel_memory_region *mem,
GEM_BUG_ON(flags & ~I915_BO_ALLOC_FLAGS);
- if (!mem)
- return ERR_PTR(-ENODEV);
+ *size = round_up(*size, mem->min_page_size);
+ if (*size == 0)
+ return -EINVAL;
- size = round_up(size, mem->min_page_size);
-
- GEM_BUG_ON(!size);
- GEM_BUG_ON(!IS_ALIGNED(size, I915_GTT_MIN_ALIGNMENT));
+ /* For most of the ABI (e.g. mmap) we think in system pages */
+ GEM_BUG_ON(!IS_ALIGNED(*size, PAGE_SIZE));
+ GEM_BUG_ON(!IS_ALIGNED(*size, I915_GTT_MIN_ALIGNMENT));
/*
* XXX: There is a prevalence of the assumption that we fit the
@@ -163,15 +162,11 @@ i915_gem_object_create_region(struct intel_memory_region *mem,
* spot such a local variable, please consider fixing!
*/
- if (size >> PAGE_SHIFT > INT_MAX)
- return ERR_PTR(-E2BIG);
-
- if (overflows_type(size, obj->base.size))
- return ERR_PTR(-E2BIG);
+ if (*size >> PAGE_SHIFT > INT_MAX)
+ return -E2BIG;
- obj = mem->ops->create_object(mem, size, flags);
- if (!IS_ERR(obj))
- trace_i915_gem_object_create(obj);
+ if (overflows_type(*size, size_t))
+ return -E2BIG;
- return obj;
+ return 0;
}
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.h b/drivers/gpu/drm/i915/gem/i915_gem_region.h
index f2ff6f8bff74..584f3e91060c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.h
@@ -21,9 +21,7 @@ void i915_gem_object_init_memory_region(struct drm_i915_gem_object *obj,
unsigned long flags);
void i915_gem_object_release_memory_region(struct drm_i915_gem_object *obj);
-struct drm_i915_gem_object *
-i915_gem_object_create_region(struct intel_memory_region *mem,
- resource_size_t size,
+int i915_gem_object_pre_check(struct intel_memory_region *mem,
+ resource_size_t *size,
unsigned int flags);
-
#endif
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
index 38113d3c0138..a190933399d4 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
@@ -464,19 +464,22 @@ static int __create_shmem(struct drm_i915_private *i915,
return 0;
}
-static struct drm_i915_gem_object *
-create_shmem(struct intel_memory_region *mem,
- resource_size_t size,
- unsigned int flags)
+struct drm_i915_gem_object *
+i915_gem_object_create_shmem(struct drm_i915_private *i915,
+ resource_size_t size)
{
+ struct intel_memory_region *mem = i915->mm.regions[INTEL_REGION_SMEM];
static struct lock_class_key lock_class;
- struct drm_i915_private *i915 = mem->i915;
struct drm_i915_gem_object *obj;
struct address_space *mapping;
unsigned int cache_level;
gfp_t mask;
int ret;
+ int flags = 0;
+ ret = i915_gem_object_pre_check(mem, &size, flags);
+ if (ret)
+ return ERR_PTR(ret);
obj = i915_gem_object_alloc();
if (!obj)
return ERR_PTR(-ENOMEM);
@@ -529,14 +532,6 @@ create_shmem(struct intel_memory_region *mem,
return ERR_PTR(ret);
}
-struct drm_i915_gem_object *
-i915_gem_object_create_shmem(struct drm_i915_private *i915,
- resource_size_t size)
-{
- return i915_gem_object_create_region(i915->mm.regions[INTEL_REGION_SMEM],
- size, 0);
-}
-
/* Allocate a new GEM object and fill it with the supplied data */
struct drm_i915_gem_object *
i915_gem_object_create_shmem_from_data(struct drm_i915_private *dev_priv,
@@ -611,7 +606,6 @@ static void release_shmem(struct intel_memory_region *mem)
static const struct intel_memory_region_ops shmem_region_ops = {
.init = init_shmem,
.release = release_shmem,
- .create_object = create_shmem,
};
struct intel_memory_region *i915_gem_shmem_setup(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index e0f21f12d3ce..1dd4ccd020b1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -607,21 +607,22 @@ __i915_gem_object_create_stolen(struct intel_memory_region *mem,
return ERR_PTR(err);
}
-static struct drm_i915_gem_object *
-_i915_gem_object_create_stolen(struct intel_memory_region *mem,
- resource_size_t size,
- unsigned int flags)
+struct drm_i915_gem_object *
+i915_gem_object_create_stolen(struct drm_i915_private *i915,
+ resource_size_t size)
{
- struct drm_i915_private *i915 = mem->i915;
+ struct intel_memory_region *mem = i915->mm.regions[INTEL_REGION_STOLEN];
struct drm_i915_gem_object *obj;
struct drm_mm_node *stolen;
int ret;
+ int flags = I915_BO_ALLOC_CONTIGUOUS;
if (!drm_mm_initialized(&i915->mm.stolen))
return ERR_PTR(-ENODEV);
- if (size == 0)
- return ERR_PTR(-EINVAL);
+ ret = i915_gem_object_pre_check(mem, &size, flags);
+ if (ret)
+ return ERR_PTR(ret);
stolen = kzalloc(sizeof(*stolen), GFP_KERNEL);
if (!stolen)
@@ -646,14 +647,6 @@ _i915_gem_object_create_stolen(struct intel_memory_region *mem,
return obj;
}
-struct drm_i915_gem_object *
-i915_gem_object_create_stolen(struct drm_i915_private *i915,
- resource_size_t size)
-{
- return i915_gem_object_create_region(i915->mm.regions[INTEL_REGION_STOLEN],
- size, I915_BO_ALLOC_CONTIGUOUS);
-}
-
static int init_stolen(struct intel_memory_region *mem)
{
intel_memory_region_set_name(mem, "stolen");
@@ -673,7 +666,6 @@ static void release_stolen(struct intel_memory_region *mem)
static const struct intel_memory_region_ops i915_region_stolen_ops = {
.init = init_stolen,
.release = release_stolen,
- .create_object = _i915_gem_object_create_stolen,
};
struct intel_memory_region *i915_gem_stolen_setup(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9aa3066cb75d..1096c1818a66 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -45,6 +45,7 @@
#include "gem/i915_gem_ioctls.h"
#include "gem/i915_gem_mman.h"
#include "gem/i915_gem_region.h"
+#include "gem/i915_gem_lmem.h"
#include "gt/intel_engine_user.h"
#include "gt/intel_gt.h"
#include "gt/intel_gt_pm.h"
@@ -205,7 +206,8 @@ i915_gem_phys_pwrite(struct drm_i915_gem_object *obj,
static int
i915_gem_create(struct drm_file *file,
- struct intel_memory_region *mr,
+ struct drm_i915_private *i915,
+ enum intel_memory_type mem_type,
u64 *size_p,
u32 *handle_p)
{
@@ -214,16 +216,12 @@ i915_gem_create(struct drm_file *file,
u64 size;
int ret;
- GEM_BUG_ON(!is_power_of_2(mr->min_page_size));
- size = round_up(*size_p, mr->min_page_size);
- if (size == 0)
- return -EINVAL;
-
- /* For most of the ABI (e.g. mmap) we think in system pages */
- GEM_BUG_ON(!IS_ALIGNED(size, PAGE_SIZE));
-
- /* Allocate the new object */
- obj = i915_gem_object_create_region(mr, size, 0);
+ if (mem_type == INTEL_MEMORY_LOCAL)
+ obj = i915_gem_object_create_lmem(i915, size, 0);
+ else if (mem_type == INTEL_MEMORY_SYSTEM)
+ obj = i915_gem_object_create_shmem(i915, size);
+ else
+ obj = ERR_PTR(-ENODEV);
if (IS_ERR(obj))
return PTR_ERR(obj);
@@ -278,9 +276,8 @@ i915_gem_dumb_create(struct drm_file *file,
if (HAS_LMEM(to_i915(dev)))
mem_type = INTEL_MEMORY_LOCAL;
- return i915_gem_create(file,
- intel_memory_region_by_type(to_i915(dev),
- mem_type),
+ return i915_gem_create(file, to_i915(dev),
+ mem_type,
&args->size, &args->handle);
}
@@ -299,9 +296,7 @@ i915_gem_create_ioctl(struct drm_device *dev, void *data,
i915_gem_flush_free_objects(i915);
- return i915_gem_create(file,
- intel_memory_region_by_type(i915,
- INTEL_MEMORY_SYSTEM),
+ return i915_gem_create(file, i915, INTEL_MEMORY_SYSTEM,
&args->size, &args->handle);
}
diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h
index 232490d89a83..b2db98a89795 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.h
+++ b/drivers/gpu/drm/i915/intel_memory_region.h
@@ -61,11 +61,6 @@ struct intel_memory_region_ops {
int (*init)(struct intel_memory_region *mem);
void (*release)(struct intel_memory_region *mem);
-
- struct drm_i915_gem_object *
- (*create_object)(struct intel_memory_region *mem,
- resource_size_t size,
- unsigned int flags);
};
struct intel_memory_region {
diff --git a/drivers/gpu/drm/i915/intel_region_lmem.c b/drivers/gpu/drm/i915/intel_region_lmem.c
index 40d8f1a95df6..e1b14add006b 100644
--- a/drivers/gpu/drm/i915/intel_region_lmem.c
+++ b/drivers/gpu/drm/i915/intel_region_lmem.c
@@ -98,7 +98,6 @@ region_lmem_init(struct intel_memory_region *mem)
const struct intel_memory_region_ops intel_region_lmem_ops = {
.init = region_lmem_init,
.release = region_lmem_release,
- .create_object = __i915_gem_lmem_object_create,
};
struct intel_memory_region *
--
2.27.0
More information about the dri-devel
mailing list