Re: [PATCH v6 1/6] drm: move the buddy allocator from i915 into common drm

2022-01-09 Thread Arunpravin 



On 07/01/22 9:27 pm, Christian König wrote:
> Am 07.01.22 um 16:49 schrieb Matthew Auld:
>> On 26/12/2021 22:24, Arunpravin wrote:
>>> Move the base i915 buddy allocator code into drm
>>> - Move i915_buddy.h to include/drm
>>> - Move i915_buddy.c to drm root folder
>>> - Rename "i915" string with "drm" string wherever applicable
>>> - Rename "I915" string with "DRM" string wherever applicable
>>> - Fix header file dependencies
>>> - Fix alignment issues
>>> - add Makefile support for drm buddy
>>> - export functions and write kerneldoc description
>>> - Remove i915 selftest config check condition as buddy selftest
>>>    will be moved to drm selftest folder
>>>
>>> cleanup i915 buddy references in i915 driver module
>>> and replace with drm buddy
>>>
>>> v2:
>>>    - include header file in alphabetical order(Thomas)
>>>    - merged changes listed in the body section into a single patch
>>>  to keep the build intact(Christian, Jani)
>>>
>>> v3:
>>>    - make drm buddy a separate module(Thomas, Christian)
>>>
>>> v4:
>>>    - Fix build error reported by kernel test robot 
>>>    - removed i915 buddy selftest from i915_mock_selftests.h to
>>>  avoid build error
>>>    - removed selftests/i915_buddy.c file as we create a new set of
>>>  buddy test cases in drm/selftests folder
>>>
>>> v5:
>>>    - Fix merge conflict issue
>>>
>>> Signed-off-by: Arunpravin 
>>
>> 
>>
>>> +int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size)
>>> +{
>>> +    unsigned int i;
>>> +    u64 offset;
>>> +
>>> +    if (size < chunk_size)
>>> +    return -EINVAL;
>>> +
>>> +    if (chunk_size < PAGE_SIZE)
>>> +    return -EINVAL;
>>> +
>>> +    if (!is_power_of_2(chunk_size))
>>> +    return -EINVAL;
>>> +
>>> +    size = round_down(size, chunk_size);
>>> +
>>> +    mm->size = size;
>>> +    mm->avail = size;
>>> +    mm->chunk_size = chunk_size;
>>> +    mm->max_order = ilog2(size) - ilog2(chunk_size);
>>> +
>>> +    BUG_ON(mm->max_order > DRM_BUDDY_MAX_ORDER);
>>> +
>>> +    mm->slab_blocks = KMEM_CACHE(drm_buddy_block, 0);
>>> +    if (!mm->slab_blocks)
>>> +    return -ENOMEM;
>>
>> It looks like every KMEM_CACHE() also creates a debugfs entry? See the 
>> error here[1]. I guess because we end with multiple instances in i915. 
>> If so, is it possible to have a single KMEM_CACHE() as part of the 
>> buddy module, similar to what i915 was doing previously?
> 
> Oh, that is a really good point, this code here doesn't make to much sense.
> 
> The value of a KMEM_CACHE() is to allow speeding up allocation of the 
> same structure size between different drm_buddy object. If you allocate 
> one cache per drm_buddy that makes the whole functionality useless.
> 
> Please fix, this is actually a bug.
> 
> Christian.
> 

I fixed in v7 version

Thanks,
Arun
>>
>> [1] 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fintel-gfx-ci.01.org%2Ftree%2Fdrm-tip%2FTrybot_8217%2Fshard-skl4%2Figt%40i915_selftest%40mock%40memory_region.htmldata=04%7C01%7Cchristian.koenig%40amd.com%7C56202fbe886f415c3b8308d9d1f5409c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637771673545453215%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=ZnRmAQo%2BjX414hbqHigL4R18oBDKLIugUQIVcwhFI%2BY%3Dreserved=0
>

Re: [PATCH v6 1/6] drm: move the buddy allocator from i915 into common drm

2022-01-07 Thread Christian König 

Am 07.01.22 um 16:49 schrieb Matthew Auld:

On 26/12/2021 22:24, Arunpravin wrote:

Move the base i915 buddy allocator code into drm
- Move i915_buddy.h to include/drm
- Move i915_buddy.c to drm root folder
- Rename "i915" string with "drm" string wherever applicable
- Rename "I915" string with "DRM" string wherever applicable
- Fix header file dependencies
- Fix alignment issues
- add Makefile support for drm buddy
- export functions and write kerneldoc description
- Remove i915 selftest config check condition as buddy selftest
   will be moved to drm selftest folder

cleanup i915 buddy references in i915 driver module
and replace with drm buddy

v2:
   - include header file in alphabetical order(Thomas)
   - merged changes listed in the body section into a single patch
 to keep the build intact(Christian, Jani)

v3:
   - make drm buddy a separate module(Thomas, Christian)

v4:
   - Fix build error reported by kernel test robot 
   - removed i915 buddy selftest from i915_mock_selftests.h to
 avoid build error
   - removed selftests/i915_buddy.c file as we create a new set of
 buddy test cases in drm/selftests folder

v5:
   - Fix merge conflict issue

Signed-off-by: Arunpravin 





+int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size)
+{
+    unsigned int i;
+    u64 offset;
+
+    if (size < chunk_size)
+    return -EINVAL;
+
+    if (chunk_size < PAGE_SIZE)
+    return -EINVAL;
+
+    if (!is_power_of_2(chunk_size))
+    return -EINVAL;
+
+    size = round_down(size, chunk_size);
+
+    mm->size = size;
+    mm->avail = size;
+    mm->chunk_size = chunk_size;
+    mm->max_order = ilog2(size) - ilog2(chunk_size);
+
+    BUG_ON(mm->max_order > DRM_BUDDY_MAX_ORDER);
+
+    mm->slab_blocks = KMEM_CACHE(drm_buddy_block, 0);
+    if (!mm->slab_blocks)
+    return -ENOMEM;


It looks like every KMEM_CACHE() also creates a debugfs entry? See the 
error here[1]. I guess because we end with multiple instances in i915. 
If so, is it possible to have a single KMEM_CACHE() as part of the 
buddy module, similar to what i915 was doing previously?


Oh, that is a really good point, this code here doesn't make to much sense.

The value of a KMEM_CACHE() is to allow speeding up allocation of the 
same structure size between different drm_buddy object. If you allocate 
one cache per drm_buddy that makes the whole functionality useless.


Please fix, this is actually a bug.

Christian.



[1] 
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fintel-gfx-ci.01.org%2Ftree%2Fdrm-tip%2FTrybot_8217%2Fshard-skl4%2Figt%40i915_selftest%40mock%40memory_region.htmldata=04%7C01%7Cchristian.koenig%40amd.com%7C56202fbe886f415c3b8308d9d1f5409c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637771673545453215%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=ZnRmAQo%2BjX414hbqHigL4R18oBDKLIugUQIVcwhFI%2BY%3Dreserved=0

Re: [PATCH v6 1/6] drm: move the buddy allocator from i915 into common drm

2022-01-07 Thread Matthew Auld 

On 26/12/2021 22:24, Arunpravin wrote:

Move the base i915 buddy allocator code into drm
- Move i915_buddy.h to include/drm
- Move i915_buddy.c to drm root folder
- Rename "i915" string with "drm" string wherever applicable
- Rename "I915" string with "DRM" string wherever applicable
- Fix header file dependencies
- Fix alignment issues
- add Makefile support for drm buddy
- export functions and write kerneldoc description
- Remove i915 selftest config check condition as buddy selftest
   will be moved to drm selftest folder

cleanup i915 buddy references in i915 driver module
and replace with drm buddy

v2:
   - include header file in alphabetical order(Thomas)
   - merged changes listed in the body section into a single patch
 to keep the build intact(Christian, Jani)

v3:
   - make drm buddy a separate module(Thomas, Christian)

v4:
   - Fix build error reported by kernel test robot 
   - removed i915 buddy selftest from i915_mock_selftests.h to
 avoid build error
   - removed selftests/i915_buddy.c file as we create a new set of
 buddy test cases in drm/selftests folder

v5:
   - Fix merge conflict issue

Signed-off-by: Arunpravin 





+int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size)
+{
+   unsigned int i;
+   u64 offset;
+
+   if (size < chunk_size)
+   return -EINVAL;
+
+   if (chunk_size < PAGE_SIZE)
+   return -EINVAL;
+
+   if (!is_power_of_2(chunk_size))
+   return -EINVAL;
+
+   size = round_down(size, chunk_size);
+
+   mm->size = size;
+   mm->avail = size;
+   mm->chunk_size = chunk_size;
+   mm->max_order = ilog2(size) - ilog2(chunk_size);
+
+   BUG_ON(mm->max_order > DRM_BUDDY_MAX_ORDER);
+
+   mm->slab_blocks = KMEM_CACHE(drm_buddy_block, 0);
+   if (!mm->slab_blocks)
+   return -ENOMEM;


It looks like every KMEM_CACHE() also creates a debugfs entry? See the 
error here[1]. I guess because we end with multiple instances in i915. 
If so, is it possible to have a single KMEM_CACHE() as part of the buddy 
module, similar to what i915 was doing previously?


[1] 
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_8217/shard-skl4/igt@i915_selftest@mock@memory_region.html

[PATCH v6 1/6] drm: move the buddy allocator from i915 into common drm

2021-12-26 Thread Arunpravin 
Move the base i915 buddy allocator code into drm
- Move i915_buddy.h to include/drm
- Move i915_buddy.c to drm root folder
- Rename "i915" string with "drm" string wherever applicable
- Rename "I915" string with "DRM" string wherever applicable
- Fix header file dependencies
- Fix alignment issues
- add Makefile support for drm buddy
- export functions and write kerneldoc description
- Remove i915 selftest config check condition as buddy selftest
  will be moved to drm selftest folder

cleanup i915 buddy references in i915 driver module
and replace with drm buddy

v2:
  - include header file in alphabetical order(Thomas)
  - merged changes listed in the body section into a single patch
to keep the build intact(Christian, Jani)

v3:
  - make drm buddy a separate module(Thomas, Christian)

v4:
  - Fix build error reported by kernel test robot 
  - removed i915 buddy selftest from i915_mock_selftests.h to
avoid build error
  - removed selftests/i915_buddy.c file as we create a new set of
buddy test cases in drm/selftests folder

v5:
  - Fix merge conflict issue

Signed-off-by: Arunpravin 
---
 drivers/gpu/drm/Kconfig   |   6 +
 drivers/gpu/drm/Makefile  |   2 +
 drivers/gpu/drm/drm_buddy.c   | 516 
 drivers/gpu/drm/i915/Kconfig  |   1 +
 drivers/gpu/drm/i915/Makefile |   1 -
 drivers/gpu/drm/i915/i915_buddy.c | 466 ---
 drivers/gpu/drm/i915/i915_buddy.h | 143 
 drivers/gpu/drm/i915/i915_module.c|   3 -
 drivers/gpu/drm/i915/i915_scatterlist.c   |  11 +-
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.c |  33 +-
 drivers/gpu/drm/i915/i915_ttm_buddy_manager.h |   4 +-
 drivers/gpu/drm/i915/selftests/i915_buddy.c   | 787 --
 .../drm/i915/selftests/i915_mock_selftests.h  |   1 -
 .../drm/i915/selftests/intel_memory_region.c  |  13 +-
 include/drm/drm_buddy.h   | 151 
 15 files changed, 707 insertions(+), 1431 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_buddy.c
 delete mode 100644 drivers/gpu/drm/i915/i915_buddy.c
 delete mode 100644 drivers/gpu/drm/i915/i915_buddy.h
 delete mode 100644 drivers/gpu/drm/i915/selftests/i915_buddy.c
 create mode 100644 include/drm/drm_buddy.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index b1f22e457fd0..b85f7ffae621 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -198,6 +198,12 @@ config DRM_TTM
  GPU memory types. Will be enabled automatically if a device driver
  uses it.
 
+config DRM_BUDDY
+   tristate
+   depends on DRM
+   help
+ A page based buddy allocator
+
 config DRM_VRAM_HELPER
tristate
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 301a44dc18e3..ff0286eca254 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -42,6 +42,8 @@ obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
 drm_shmem_helper-y := drm_gem_shmem_helper.o
 obj-$(CONFIG_DRM_GEM_SHMEM_HELPER) += drm_shmem_helper.o
 
+obj-$(CONFIG_DRM_BUDDY) += drm_buddy.o
+
 drm_vram_helper-y := drm_gem_vram_helper.o
 obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
 
diff --git a/drivers/gpu/drm/drm_buddy.c b/drivers/gpu/drm/drm_buddy.c
new file mode 100644
index ..9340a4b61c5a
--- /dev/null
+++ b/drivers/gpu/drm/drm_buddy.c
@@ -0,0 +1,516 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2021 Intel Corporation
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+
+static struct drm_buddy_block *drm_block_alloc(struct drm_buddy_mm *mm,
+  struct drm_buddy_block *parent,
+  unsigned int order,
+  u64 offset)
+{
+   struct drm_buddy_block *block;
+
+   BUG_ON(order > DRM_BUDDY_MAX_ORDER);
+
+   block = kmem_cache_zalloc(mm->slab_blocks, GFP_KERNEL);
+   if (!block)
+   return NULL;
+
+   block->header = offset;
+   block->header |= order;
+   block->parent = parent;
+
+   BUG_ON(block->header & DRM_BUDDY_HEADER_UNUSED);
+   return block;
+}
+
+static void drm_block_free(struct drm_buddy_mm *mm,
+  struct drm_buddy_block *block)
+{
+   kmem_cache_free(mm->slab_blocks, block);
+}
+
+static void mark_allocated(struct drm_buddy_block *block)
+{
+   block->header &= ~DRM_BUDDY_HEADER_STATE;
+   block->header |= DRM_BUDDY_ALLOCATED;
+
+   list_del(>link);
+}
+
+static void mark_free(struct drm_buddy_mm *mm,
+ struct drm_buddy_block *block)
+{
+   block->header &= ~DRM_BUDDY_HEADER_STATE;
+   block->header |= DRM_BUDDY_FREE;
+
+   list_add(>link,
+>free_list[drm_buddy_block_order(block)]);
+}
+
+static void mark_split(struct drm_buddy_block *block)
+{
+   block->header &=