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 <l...@intel.com>
>>> - 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 <arunpravin.paneersel...@amd.com>
>>
>> <snip>
>>
>>> +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.html&data=04%7C01%7Cchristian.koenig%40amd.com%7C56202fbe886f415c3b8308d9d1f5409c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637771673545453215%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ZnRmAQo%2BjX414hbqHigL4R18oBDKLIugUQIVcwhFI%2BY%3D&reserved=0
>
