Re: [Intel-gfx] [PATCH v3 03/21] drm/i915/region: support contiguous allocations

[Chris Wilson]

Quoting Matthew Auld (2019-10-04 18:04:34)
> @@ -98,10 +103,12 @@ i915_gem_object_get_pages_buddy(struct 
> drm_i915_gem_object *obj)
>  }
>  
>  void i915_gem_object_init_memory_region(struct drm_i915_gem_object *obj,
> -   struct intel_memory_region *mem)
> +   struct intel_memory_region *mem,
> +   unsigned long flags)
>  {
> INIT_LIST_HEAD(>mm.blocks);
> obj->mm.region = intel_memory_region_get(mem);
> +   obj->flags = flags;

This is odd as you don't necessarily have exclusive control of obj->flags

Looks like this may be a wart in the api.

> +static struct drm_i915_gem_object *
> +igt_object_create(struct intel_memory_region *mem,
> + struct list_head *objects,
> + u64 size,
> + unsigned int flags)
> +{
> +   struct drm_i915_gem_object *obj;
> +   int err;
> +
> +   obj = i915_gem_object_create_region(mem, size, flags);
> +   if (IS_ERR(obj))
> +   return obj;
> +
> +   err = i915_gem_object_pin_pages(obj);
> +   if (err)
> +   goto put;
> +
> +   list_add(>st_link, objects);
> +   return obj;
> +
> +put:
> +   i915_gem_object_put(obj);
> +   return ERR_PTR(err);
> +}
> +
> +static void igt_object_release(struct drm_i915_gem_object *obj)
> +{
> +   i915_gem_object_unpin_pages(obj);
> +   __i915_gem_object_put_pages(obj, I915_MM_NORMAL);
> +   i915_gem_object_put(obj);
> +   list_del(>st_link);

Did you just put the object before operating on it? Did you have a spare
reference lying around? :)
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v3 03/21] drm/i915/region: support contiguous allocations

[Matthew Auld]

Some kernel internal objects may need to be allocated as a contiguous
block, also thinking ahead the various kernel io_mapping interfaces seem
to expect it, although this is purely a limitation in the kernel
API...so perhaps something to be improved.

Signed-off-by: Matthew Auld 
Cc: Joonas Lahtinen 
Cc: Abdiel Janulgue 
Cc: Michael J Ruhl 
---
 drivers/gpu/drm/i915/gem/i915_gem_object.h|   6 +
 .../gpu/drm/i915/gem/i915_gem_object_types.h  |   4 +
 drivers/gpu/drm/i915/gem/i915_gem_region.c|  15 +-
 drivers/gpu/drm/i915/gem/i915_gem_region.h|   3 +-
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  71 
 drivers/gpu/drm/i915/intel_memory_region.c|   9 +-
 drivers/gpu/drm/i915/intel_memory_region.h|   3 +-
 .../drm/i915/selftests/intel_memory_region.c  | 165 ++
 drivers/gpu/drm/i915/selftests/mock_region.c  |   2 +-
 9 files changed, 239 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h 
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 086a9bf5adcc..dfd16d65630f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -139,6 +139,12 @@ i915_gem_object_is_readonly(const struct 
drm_i915_gem_object *obj)
return obj->base.vma_node.readonly;
 }
 
+static inline bool
+i915_gem_object_is_contiguous(const struct drm_i915_gem_object *obj)
+{
+   return obj->flags & I915_BO_ALLOC_CONTIGUOUS;
+}
+
 static inline bool
 i915_gem_object_type_has(const struct drm_i915_gem_object *obj,
 unsigned long flags)
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 11390586cfe1..c6a712cf7d7a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -119,6 +119,10 @@ struct drm_i915_gem_object {
 
I915_SELFTEST_DECLARE(struct list_head st_link);
 
+   unsigned long flags;
+#define I915_BO_ALLOC_CONTIGUOUS BIT(0)
+#define I915_BO_ALLOC_FLAGS (I915_BO_ALLOC_CONTIGUOUS)
+
/*
 * Is the object to be mapped as read-only to the GPU
 * Only honoured if hardware has relevant pte bit
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.c 
b/drivers/gpu/drm/i915/gem/i915_gem_region.c
index 5fc6e4540f82..04cb9f72945e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.c
@@ -23,10 +23,10 @@ i915_gem_object_get_pages_buddy(struct drm_i915_gem_object 
*obj)
 {
struct intel_memory_region *mem = obj->mm.region;
struct list_head *blocks = >mm.blocks;
-   unsigned int flags = I915_ALLOC_MIN_PAGE_SIZE;
resource_size_t size = obj->base.size;
resource_size_t prev_end;
struct i915_buddy_block *block;
+   unsigned int flags;
struct sg_table *st;
struct scatterlist *sg;
unsigned int sg_page_sizes;
@@ -42,6 +42,10 @@ i915_gem_object_get_pages_buddy(struct drm_i915_gem_object 
*obj)
return -ENOMEM;
}
 
+   flags = I915_ALLOC_MIN_PAGE_SIZE;
+   if (obj->flags & I915_BO_ALLOC_CONTIGUOUS)
+   flags |= I915_ALLOC_CONTIGUOUS;
+
ret = __intel_memory_region_get_pages_buddy(mem, size, flags, blocks);
if (ret)
goto err_free_sg;
@@ -56,7 +60,8 @@ i915_gem_object_get_pages_buddy(struct drm_i915_gem_object 
*obj)
list_for_each_entry(block, blocks, link) {
u64 block_size, offset;
 
-   block_size = i915_buddy_block_size(>mm, block);
+   block_size = min_t(u64, size,
+  i915_buddy_block_size(>mm, block));
offset = i915_buddy_block_offset(block);
 
GEM_BUG_ON(overflows_type(block_size, sg->length));
@@ -98,10 +103,12 @@ i915_gem_object_get_pages_buddy(struct drm_i915_gem_object 
*obj)
 }
 
 void i915_gem_object_init_memory_region(struct drm_i915_gem_object *obj,
-   struct intel_memory_region *mem)
+   struct intel_memory_region *mem,
+   unsigned long flags)
 {
INIT_LIST_HEAD(>mm.blocks);
obj->mm.region = intel_memory_region_get(mem);
+   obj->flags = flags;
 }
 
 void i915_gem_object_release_memory_region(struct drm_i915_gem_object *obj)
@@ -116,6 +123,8 @@ i915_gem_object_create_region(struct intel_memory_region 
*mem,
 {
struct drm_i915_gem_object *obj;
 
+   GEM_BUG_ON(flags & ~I915_BO_ALLOC_FLAGS);
+
if (!mem)
return ERR_PTR(-ENODEV);
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_region.h 
b/drivers/gpu/drm/i915/gem/i915_gem_region.h
index ebddc86d78f7..f2ff6f8bff74 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_region.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_region.h
@@ -17,7 +17,8 @@ void i915_gem_object_put_pages_buddy(struct 
drm_i915_gem_object *obj,