| Message ID | 20220725114240.4844-4-Arunpravin.PaneerSelvam@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/6] drm/ttm: Add new callbacks to ttm res mgr | expand |
On 25/07/2022 12:42, Arunpravin Paneer Selvam wrote: > Implemented a new intersect and compatible callback function > fetching start offset from drm buddy allocator. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com> > --- > drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 43 +++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > index a5109548abc0..b5801c05bd41 100644 > --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c > @@ -178,6 +178,47 @@ static void i915_ttm_buddy_man_free(struct ttm_resource_manager *man, > kfree(bman_res); > } > > +static bool i915_ttm_buddy_man_intersect(struct ttm_resource_manager *man, > + struct ttm_resource *res, > + const struct ttm_place *place, > + size_t size) > +{ > + struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res); > + u32 start, num_pages = PFN_UP(size); > + struct drm_buddy_block *block; > + > + /* Check each drm buddy block individually */ > + list_for_each_entry(block, &bman_res->blocks, link) { > + start = drm_buddy_block_offset(block) >> PAGE_SHIFT; > + /* Don't evict BOs outside of the requested placement range */ > + if (place->fpfn >= (start + num_pages) || > + (place->lpfn && place->lpfn <= start)) > + return false; > + } > + > + return true; > +} This looks like a nice idea. We should be able to clean up i915_ttm_eviction_valuable() a fair bit I think, if we now call ttm_bo_eviction_valuable() at the end (like in amdgpu), and move the bits that are specific to buddy_man here? So something like: if (!place->fpfn && !place->lpfn) return true; if (!place->fpfn && place->lpfn == i915_buddy_man_visible_size(man)) return bman_res->used_visible_size > 0; /* Check each drm buddy block individually */ .... > + > +static bool i915_ttm_buddy_man_compatible(struct ttm_resource_manager *man, > + struct ttm_resource *res, > + const struct ttm_place *place, > + size_t size) Is it not possible to derive the size from res->num_pages? > +{ > + struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res); > + u32 start, num_pages = PFN_UP(size); > + struct drm_buddy_block *block; > + > + /* Check each drm buddy block individually */ > + list_for_each_entry(block, &bman_res->blocks, link) { > + start = drm_buddy_block_offset(block) >> PAGE_SHIFT; > + if (start < place->fpfn || > + (place->lpfn && (start + num_pages) > place->lpfn)) > + return false; > + } if (!place->fpfn && !place->lpfn) return true; if (!place->fpfn && place->lpfn == i915_buddy_man_visible_size(man)) return bman_res->used_visible_size == res->num_pages; /* Check each drm buddy block individually */ ... > + > + return true; > +} > + > static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man, > struct drm_printer *printer) > { > @@ -205,6 +246,8 @@ static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man, > static const struct ttm_resource_manager_func i915_ttm_buddy_manager_func = { > .alloc = i915_ttm_buddy_man_alloc, > .free = i915_ttm_buddy_man_free, > + .intersect = i915_ttm_buddy_man_intersect, s/intersect/intersects/ ? > + .compatible = i915_ttm_buddy_man_compatible, > .debug = i915_ttm_buddy_man_debug, > }; >
Hi Matthew, On 7/26/2022 4:11 PM, Matthew Auld wrote: > On 25/07/2022 12:42, Arunpravin Paneer Selvam wrote: >> Implemented a new intersect and compatible callback function >> fetching start offset from drm buddy allocator. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> Signed-off-by: Arunpravin Paneer Selvam >> <Arunpravin.PaneerSelvam@amd.com> >> --- >> drivers/gpu/drm/i915/i915_ttm_buddy_manager.c | 43 +++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c >> b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c >> index a5109548abc0..b5801c05bd41 100644 >> --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c >> +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c >> @@ -178,6 +178,47 @@ static void i915_ttm_buddy_man_free(struct >> ttm_resource_manager *man, >> kfree(bman_res); >> } >> +static bool i915_ttm_buddy_man_intersect(struct >> ttm_resource_manager *man, >> + struct ttm_resource *res, >> + const struct ttm_place *place, >> + size_t size) >> +{ >> + struct i915_ttm_buddy_resource *bman_res = >> to_ttm_buddy_resource(res); >> + u32 start, num_pages = PFN_UP(size); >> + struct drm_buddy_block *block; >> + >> + /* Check each drm buddy block individually */ >> + list_for_each_entry(block, &bman_res->blocks, link) { >> + start = drm_buddy_block_offset(block) >> PAGE_SHIFT; >> + /* Don't evict BOs outside of the requested placement range */ >> + if (place->fpfn >= (start + num_pages) || >> + (place->lpfn && place->lpfn <= start)) >> + return false; >> + } >> + >> + return true; >> +} > > This looks like a nice idea. We should be able to clean up > i915_ttm_eviction_valuable() a fair bit I think, if we now call > ttm_bo_eviction_valuable() at the end (like in amdgpu), and move the > bits that are specific to buddy_man here? > > So something like: > > if (!place->fpfn && !place->lpfn) > return true; > > if (!place->fpfn && > place->lpfn == i915_buddy_man_visible_size(man)) > return bman_res->used_visible_size > 0; > > /* Check each drm buddy block individually */ > .... > modified in v3 >> + >> +static bool i915_ttm_buddy_man_compatible(struct >> ttm_resource_manager *man, >> + struct ttm_resource *res, >> + const struct ttm_place *place, >> + size_t size) > > Is it not possible to derive the size from res->num_pages? I think it is possible, I will check with Christian for any specific case that might require ttm_buffer_object->base.size > >> +{ >> + struct i915_ttm_buddy_resource *bman_res = >> to_ttm_buddy_resource(res); >> + u32 start, num_pages = PFN_UP(size); >> + struct drm_buddy_block *block; >> + >> + /* Check each drm buddy block individually */ >> + list_for_each_entry(block, &bman_res->blocks, link) { >> + start = drm_buddy_block_offset(block) >> PAGE_SHIFT; >> + if (start < place->fpfn || >> + (place->lpfn && (start + num_pages) > place->lpfn)) >> + return false; >> + } > > if (!place->fpfn && !place->lpfn) > return true; > > if (!place->fpfn && > place->lpfn == i915_buddy_man_visible_size(man)) > return bman_res->used_visible_size == res->num_pages; > > /* Check each drm buddy block individually */ > ... > modified in v3 >> + >> + return true; >> +} >> + >> static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man, >> struct drm_printer *printer) >> { >> @@ -205,6 +246,8 @@ static void i915_ttm_buddy_man_debug(struct >> ttm_resource_manager *man, >> static const struct ttm_resource_manager_func >> i915_ttm_buddy_manager_func = { >> .alloc = i915_ttm_buddy_man_alloc, >> .free = i915_ttm_buddy_man_free, >> + .intersect = i915_ttm_buddy_man_intersect, > > s/intersect/intersects/ ? > okay. >> + .compatible = i915_ttm_buddy_man_compatible, >> .debug = i915_ttm_buddy_man_debug, >> };
diff --git a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c index a5109548abc0..b5801c05bd41 100644 --- a/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c +++ b/drivers/gpu/drm/i915/i915_ttm_buddy_manager.c @@ -178,6 +178,47 @@ static void i915_ttm_buddy_man_free(struct ttm_resource_manager *man, kfree(bman_res); } +static bool i915_ttm_buddy_man_intersect(struct ttm_resource_manager *man, + struct ttm_resource *res, + const struct ttm_place *place, + size_t size) +{ + struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res); + u32 start, num_pages = PFN_UP(size); + struct drm_buddy_block *block; + + /* Check each drm buddy block individually */ + list_for_each_entry(block, &bman_res->blocks, link) { + start = drm_buddy_block_offset(block) >> PAGE_SHIFT; + /* Don't evict BOs outside of the requested placement range */ + if (place->fpfn >= (start + num_pages) || + (place->lpfn && place->lpfn <= start)) + return false; + } + + return true; +} + +static bool i915_ttm_buddy_man_compatible(struct ttm_resource_manager *man, + struct ttm_resource *res, + const struct ttm_place *place, + size_t size) +{ + struct i915_ttm_buddy_resource *bman_res = to_ttm_buddy_resource(res); + u32 start, num_pages = PFN_UP(size); + struct drm_buddy_block *block; + + /* Check each drm buddy block individually */ + list_for_each_entry(block, &bman_res->blocks, link) { + start = drm_buddy_block_offset(block) >> PAGE_SHIFT; + if (start < place->fpfn || + (place->lpfn && (start + num_pages) > place->lpfn)) + return false; + } + + return true; +} + static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man, struct drm_printer *printer) { @@ -205,6 +246,8 @@ static void i915_ttm_buddy_man_debug(struct ttm_resource_manager *man, static const struct ttm_resource_manager_func i915_ttm_buddy_manager_func = { .alloc = i915_ttm_buddy_man_alloc, .free = i915_ttm_buddy_man_free, + .intersect = i915_ttm_buddy_man_intersect, + .compatible = i915_ttm_buddy_man_compatible, .debug = i915_ttm_buddy_man_debug, };