Re: [Intel-gfx] [PATCH 04/22] drm/i915/region: support continuous allocations
>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Matthew Auld
>Sent: Friday, September 27, 2019 1:34 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: daniel.vet...@ffwll.ch
>Subject: [Intel-gfx] [PATCH 04/22] drm/i915/region: support continuous
>allocations
>
>Some kernel internal objects may need to be allocated as a continuous
Nit:
You refer to the "continuous block", but the then you create the "CONTIGUOUS"
allocations.
s/continuous/contiguous?
Mike
>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
>---
> .../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 | 3 +-
> drivers/gpu/drm/i915/intel_memory_region.c| 13 +-
> drivers/gpu/drm/i915/intel_memory_region.h| 3 +-
> .../drm/i915/selftests/intel_memory_region.c | 163 ++
> drivers/gpu/drm/i915/selftests/mock_region.c | 2 +-
> 8 files changed, 197 insertions(+), 9 deletions(-)
>
>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 d36c860c9c6f..7acd383f174f 100644
>--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
>@@ -117,6 +117,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 5c3bfc121921..b317a5c84144 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 = &obj->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(&mem->mm, block);
>+ block_size = min_t(u64, size,
>+ i915_buddy_block_size(&mem->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(&obj->mm.blocks);
> obj->mm.region = mem;
>+ obj->flags = flags;
> }
>
> void i915_gem_object_release_memory_region(struct
>drm_i915_gem_object *obj)
>@@ -115,6 +122,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 ebddc86d78
Re: [Intel-gfx] [PATCH 04/22] drm/i915/region: support continuous allocations
Quoting Matthew Auld (2019-09-27 18:33:51)
> struct drm_i915_gem_object *
> diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> index 4e1805aaeb99..f9fbf2865782 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
> @@ -471,7 +471,8 @@ static int igt_mock_memory_region_huge_pages(void *arg)
> unsigned int page_size = BIT(bit);
> resource_size_t phys;
>
> - obj = i915_gem_object_create_region(mem, page_size, 0);
> + obj = i915_gem_object_create_region(mem, page_size,
> + I915_BO_ALLOC_CONTIGUOUS);
Seems a good opportunity to test both?
> if (IS_ERR(obj)) {
> err = PTR_ERR(obj);
> goto out_destroy_device;
> diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
> b/drivers/gpu/drm/i915/intel_memory_region.c
> index e48d5c37c4df..7a66872d9eac 100644
> --- a/drivers/gpu/drm/i915/intel_memory_region.c
> +++ b/drivers/gpu/drm/i915/intel_memory_region.c
> @@ -47,8 +47,8 @@ __intel_memory_region_get_pages_buddy(struct
> intel_memory_region *mem,
> unsigned int flags,
> struct list_head *blocks)
> {
> - unsigned long n_pages = size >> ilog2(mem->mm.chunk_size);
> unsigned int min_order = 0;
> + unsigned long n_pages;
>
> GEM_BUG_ON(!IS_ALIGNED(size, mem->mm.chunk_size));
> GEM_BUG_ON(!list_empty(blocks));
> @@ -58,6 +58,13 @@ __intel_memory_region_get_pages_buddy(struct
> intel_memory_region *mem,
> ilog2(mem->mm.chunk_size);
> }
>
> + if (flags & I915_ALLOC_CONTIGUOUS) {
> + size = roundup_pow_of_two(size);
> + min_order = ilog2(size) - ilog2(mem->mm.chunk_size);
> + }
> +
> + n_pages = size >> ilog2(mem->mm.chunk_size);
> +
> mutex_lock(&mem->mm_lock);
>
> do {
> @@ -104,7 +111,9 @@ __intel_memory_region_get_block_buddy(struct
> intel_memory_region *mem,
> int ret;
>
> INIT_LIST_HEAD(&blocks);
> - ret = __intel_memory_region_get_pages_buddy(mem, size, 0, &blocks);
> + ret = __intel_memory_region_get_pages_buddy(mem, size,
> + I915_ALLOC_CONTIGUOUS,
> + &blocks);
This chunk looks odd. Quick explanation why we don't pass flags here?
> if (ret)
> return ERR_PTR(ret);
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 04/22] drm/i915/region: support continuous allocations
Some kernel internal objects may need to be allocated as a continuous
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
---
.../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 | 3 +-
drivers/gpu/drm/i915/intel_memory_region.c| 13 +-
drivers/gpu/drm/i915/intel_memory_region.h| 3 +-
.../drm/i915/selftests/intel_memory_region.c | 163 ++
drivers/gpu/drm/i915/selftests/mock_region.c | 2 +-
8 files changed, 197 insertions(+), 9 deletions(-)
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 d36c860c9c6f..7acd383f174f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -117,6 +117,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 5c3bfc121921..b317a5c84144 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 = &obj->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(&mem->mm, block);
+ block_size = min_t(u64, size,
+ i915_buddy_block_size(&mem->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(&obj->mm.blocks);
obj->mm.region = mem;
+ obj->flags = flags;
}
void i915_gem_object_release_memory_region(struct drm_i915_gem_object *obj)
@@ -115,6 +122,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,
struct sg_table *pages);
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);
void i915_gem_object_release_memory_region(struct drm_i915_gem_object *obj);
struct drm_i915_gem_object *
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 4e1805aaeb99..f9fbf2865782 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -471,7 +471,8 @@ static int igt_mock
