Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] [RFC] drm/i915: make object creation avoid regions layering.
@ 2020-08-31 20:33 Dave Airlie
  2020-08-31 20:41 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork
  0 siblings, 1 reply; 2+ messages in thread
From: Dave Airlie @ 2020-08-31 20:33 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

From: Dave Airlie <airlied@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

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 2+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: make object creation avoid regions layering.
  2020-08-31 20:33 [Intel-gfx] [PATCH] [RFC] drm/i915: make object creation avoid regions layering Dave Airlie
@ 2020-08-31 20:41 ` Patchwork
  0 siblings, 0 replies; 2+ messages in thread
From: Patchwork @ 2020-08-31 20:41 UTC (permalink / raw)
  To: Dave Airlie; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: make object creation avoid regions layering.
URL   : https://patchwork.freedesktop.org/series/81197/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/intel_memory_region.o
In file included from drivers/gpu/drm/i915/intel_memory_region.c:302:0:
drivers/gpu/drm/i915/selftests/intel_memory_region.c: In function ‘igt_mock_fill’:
drivers/gpu/drm/i915/selftests/intel_memory_region.c:66:9: error: implicit declaration of function ‘i915_gem_object_create_region’; did you mean ‘i915_gem_object_create_stolen’? [-Werror=implicit-function-declaration]
   obj = i915_gem_object_create_region(mem, size, 0);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         i915_gem_object_create_stolen
drivers/gpu/drm/i915/selftests/intel_memory_region.c:66:7: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
   obj = i915_gem_object_create_region(mem, size, 0);
       ^
drivers/gpu/drm/i915/selftests/intel_memory_region.c: In function ‘igt_object_create’:
drivers/gpu/drm/i915/selftests/intel_memory_region.c:108:6: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
  obj = i915_gem_object_create_region(mem, size, flags);
      ^
drivers/gpu/drm/i915/selftests/intel_memory_region.c: In function ‘create_region_for_mapping’:
drivers/gpu/drm/i915/selftests/intel_memory_region.c:596:6: error: assignment makes pointer from integer without a cast [-Werror=int-conversion]
  obj = i915_gem_object_create_region(mr, size, 0);
      ^
In file included from drivers/gpu/drm/i915/intel_memory_region.c:303:0:
drivers/gpu/drm/i915/selftests/mock_region.c: At top level:
drivers/gpu/drm/i915/selftests/mock_region.c:49:3: error: ‘const struct intel_memory_region_ops’ has no member named ‘create_object’
  .create_object = mock_object_create,
   ^~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_region.c:49:19: error: excess elements in struct initializer [-Werror]
  .create_object = mock_object_create,
                   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/selftests/mock_region.c:49:19: note: (near initialization for ‘mock_region_ops’)
cc1: all warnings being treated as errors
scripts/Makefile.build:283: recipe for target 'drivers/gpu/drm/i915/intel_memory_region.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_memory_region.o] Error 1
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:500: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1788: recipe for target 'drivers' failed
make: *** [drivers] Error 2


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-08-31 20:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-31 20:33 [Intel-gfx] [PATCH] [RFC] drm/i915: make object creation avoid regions layering Dave Airlie
2020-08-31 20:41 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox