Hi
Am 03.01.22 um 08:41 schrieb Christian König:
>
>
> Am 26.12.21 um 21:59 schrieb Arunpravin:
>> Hi Thomas
>>
>> On 16/12/21 5:05 pm, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 01.12.21 um 17:39 schrieb Arunpravin:
>>>> - Make drm_buddy_alloc a single function to handle
>>>> range allocation and non-range allocation demands
>>>>
>>>> - Implemented a new function alloc_range() which allocates
>>>> the requested power-of-two block comply with range limitations
>>>>
>>>> - Moved order computation and memory alignment logic from
>>>> i915 driver to drm buddy
>>>>
>>>> v2:
>>>> merged below changes to keep the build unbroken
>>>> - drm_buddy_alloc_range() becomes obsolete and may be removed
>>>> - enable ttm range allocation (fpfn / lpfn) support in i915 driver
>>>> - apply enhanced drm_buddy_alloc() function to i915 driver
>>>>
>>>> v3(Matthew Auld):
>>>> - Fix alignment issues and remove unnecessary list_empty check
>>>> - add more validation checks for input arguments
>>>> - make alloc_range() block allocations as bottom-up
>>>> - optimize order computation logic
>>>> - replace uint64_t with u64, which is preferred in the kernel
>>>>
>>>> v4(Matthew Auld):
>>>> - keep drm_buddy_alloc_range() function implementation for generic
>>>> actual range allocations
>>>> - keep alloc_range() implementation for end bias allocations
>>>>
>>>> Signed-off-by: Arunpravin <Arunpravin.PaneerSelvam at amd.com>
>> <SNIP>
>>
>>>> +#define DRM_BUDDY_RANGE_ALLOCATION (1 << 0)
>>>> +
>>>> struct drm_buddy_block {
>>>> #define DRM_BUDDY_HEADER_OFFSET GENMASK_ULL(63, 12)
>>>> #define DRM_BUDDY_HEADER_STATE GENMASK_ULL(11, 10)
>>>> @@ -132,12 +139,11 @@ int drm_buddy_init(struct drm_buddy_mm *mm,
>>>> u64 size, u64 chunk_size);
>>>> void drm_buddy_fini(struct drm_buddy_mm *mm);
>>>> -struct drm_buddy_block *
>>>> -drm_buddy_alloc(struct drm_buddy_mm *mm, unsigned int order);
>>> Just a style issue. The structure is called drm_buddy_mm. For
>>> consistency, I like to suggest to name all the public interfaces and
>>> defines drm_buddy_mm_* instead of just drm_buddy_*.
>>>
>> Thanks for the suggestion, I think renaming drm_buddy_* to
>> drm_buddy_mm_* creates a long name for the public interfaces, for
>> instance - drm_buddy_mm_alloc_blocks(),
>> discussing the style issue internally
>> @Matthew, @christian - please let me know your opinion
>
> I would prefer drm_buddy as prefix as well and I think we could rather
> drop the _mm postfix from the structure here.
I was mostly concerned about mismatching names for functions and
structures. If drm_buddy_ (without mm) for all naming is preferred, I
wouldn't mind.
Best regards
Thomas
>
> Cause what we try to manage is not necessary memory, but rather address
> space.
>
> Christian.
>
>>
>>>> -
>>>> -int drm_buddy_alloc_range(struct drm_buddy_mm *mm,
>>>> - struct list_head *blocks,
>>>> - u64 start, u64 size);
>>>> +int drm_buddy_alloc(struct drm_buddy_mm *mm,
>>>> + u64 start, u64 end, u64 size,
>>>> + u64 min_page_size,
>>>> + struct list_head *blocks,
>>>> + unsigned long flags);
>>>> void drm_buddy_free(struct drm_buddy_mm *mm, struct
>>>> drm_buddy_block *block);
>>> I'd call those *_alloc_blocks() and _free_blocks(). Right now it sounds
>>> as if they allocate and free instances of drm_buddy_mm.
>> can we call those drm_buddy_alloc_blocks() and drm_buddy_free_blocks()
>> Does this make sense?
>>> Best regards
>>> Thomas
>>>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20220106/06d5fa49/attachment.sig>