-----Original Message----- From: amd-gfx amd-gfx-bounces@lists.freedesktop.org On Behalf Of Matthew Auld Sent: Tuesday, January 4, 2022 7:32 PM To: Paneer Selvam, Arunpravin Arunpravin.PaneerSelvam@amd.com; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; amd-gfx@lists.freedesktop.org Cc: Deucher, Alexander Alexander.Deucher@amd.com; tzimmermann@suse.de; jani.nikula@linux.intel.com; Koenig, Christian Christian.Koenig@amd.com; daniel@ffwll.ch Subject: Re: [PATCH v6 2/6] drm: improve drm_buddy_alloc function
On 26/12/2021 22:24, Arunpravin wrote:
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@amd.com
<snip>
@@ -73,34 +83,16 @@ static int i915_ttm_buddy_man_alloc(struct ttm_resource_manager *man,
n_pages = size >> ilog2(mm->chunk_size);
- do {
struct drm_buddy_block *block;unsigned int order;order = fls(n_pages) - 1;GEM_BUG_ON(order > mm->max_order);GEM_BUG_ON(order < min_order);do {mutex_lock(&bman->lock);block = drm_buddy_alloc(mm, order);mutex_unlock(&bman->lock);if (!IS_ERR(block))break;if (order-- == min_order) {err = -ENOSPC;goto err_free_blocks;}} while (1);n_pages -= BIT(order);list_add_tail(&block->link, &bman_res->blocks);if (!n_pages)break;- } while (1);
- mutex_lock(&bman->lock);
- err = drm_buddy_alloc(mm, (u64)place->fpfn << PAGE_SHIFT,
(u64)place->lpfn << PAGE_SHIFT,place->lpfn will currently always be zero for i915, AFAIK. I assume here we want s/place->lpfn/lpfn/?
I replaced place->lpfn with lpfn as below
- err = drm_buddy_alloc(mm, (u64)place->fpfn << PAGE_SHIFT,
(u64)lpfn << PAGE_SHIFT,
AFAIK, we need to change only at above location, hope this fixes the firmware allocation issue.
Also something in this series is preventing i915 from loading on discrete devices, according to CI. Hopefully that is just the lpfn issue...which might explain seeing -EINVAL here[1] when allocating some vram for the firmware.
[1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fintel-gfx-...
(u64)n_pages << PAGE_SHIFT,min_page_size,&bman_res->blocks,bman_res->flags);mutex_unlock(&bman->lock);
if (unlikely(err))
goto err_free_blocks;*res = &bman_res->base; return 0;
@@ -266,10 +258,17 @@ int i915_ttm_buddy_man_reserve(struct ttm_resource_manager *man, { struct i915_ttm_buddy_manager *bman = to_buddy_manager(man); struct drm_buddy_mm *mm = &bman->mm;
unsigned long flags = 0; int ret;
flags |= DRM_BUDDY_RANGE_ALLOCATION;
mutex_lock(&bman->lock);
- ret = drm_buddy_alloc_range(mm, &bman->reserved, start, size);
ret = drm_buddy_alloc(mm, start,
start + size,size, mm->chunk_size,&bman->reserved,flags);mutex_unlock(&bman->lock);
return ret;
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h index fa644b512c2e..5ba490875f66 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.h @@ -20,6 +20,7 @@ struct drm_buddy_mm;
- @base: struct ttm_resource base class we extend
- @blocks: the list of struct i915_buddy_block for this resource/allocation
- @flags: DRM_BUDDY_*_ALLOCATION flags
- @mm: the struct i915_buddy_mm for this resource
- Extends the struct ttm_resource to manage an address space allocation with
@@ -28,6 +29,7 @@ struct drm_buddy_mm; struct i915_ttm_buddy_resource { struct ttm_resource base; struct list_head blocks;
- unsigned long flags; struct drm_buddy_mm *mm; };
diff --git a/include/drm/drm_buddy.h b/include/drm/drm_buddy.h index 09d73328c268..4368acaad222 100644 --- a/include/drm/drm_buddy.h +++ b/include/drm/drm_buddy.h @@ -13,15 +13,22 @@
#include <drm/drm_print.h>
-#define range_overflows(start, size, max) ({ \ +#define check_range_overflow(start, end, size, max) ({ \ typeof(start) start__ = (start); \
- typeof(end) end__ = (end);\ typeof(size) size__ = (size); \ typeof(max) max__ = (max); \ (void)(&start__ == &size__); \ (void)(&start__ == &max__); \
- start__ >= max__ || size__ > max__ - start__; \
- (void)(&start__ == &end__); \
- (void)(&end__ == &size__); \
- (void)(&end__ == &max__); \
- start__ >= max__ || end__ > max__ || \
- size__ > end__ - start__; \ })
+#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);
-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);
dri-devel@lists.freedesktop.org
-
Arunpravin