On Wed, 2024-11-20 at 11:51 +0100, Christian König wrote: > Am 15.11.24 um 16:01 schrieb Thomas Hellström: > > Make the interface more symmetric by providing and using a > > ttm_resource_cursor_init(). > > > > v10: > > - Fix a stray newline (Matthew Brost) > > - Update kerneldoc (Matthew Brost) > > > > Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx> > > Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx> > > Reviewed-by: Christian König <christian.koenig@xxxxxxx> > > Did you plan to merge this through drm-misc-next or the XE branch? > > If through drm-misc-next then I would go ahead and push this patch > since > that is really a stand alone cleanup. I was planning to merge it all through drm-xe-next, so I'll hold off merging that patch. Thanks, Thomas > > Regards, > Christian. > > > --- > > drivers/gpu/drm/ttm/ttm_bo.c | 3 ++- > > drivers/gpu/drm/ttm/ttm_bo_util.c | 3 ++- > > drivers/gpu/drm/ttm/ttm_resource.c | 35 ++++++++++++++++++++----- > > ----- > > include/drm/ttm/ttm_resource.h | 11 +++++----- > > 4 files changed, 34 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c > > b/drivers/gpu/drm/ttm/ttm_bo.c > > index 48c5365efca1..06d6a452c4f4 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > > @@ -450,7 +450,8 @@ int ttm_bo_evict_first(struct ttm_device *bdev, > > struct ttm_resource_manager *man > > int ret = 0; > > > > spin_lock(&bdev->lru_lock); > > - res = ttm_resource_manager_first(man, &cursor); > > + ttm_resource_cursor_init(&cursor, man); > > + res = ttm_resource_manager_first(&cursor); > > ttm_resource_cursor_fini(&cursor); > > if (!res) { > > ret = -ENOENT; > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c > > b/drivers/gpu/drm/ttm/ttm_bo_util.c > > index d939925efa81..917096bd5f68 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c > > @@ -865,7 +865,8 @@ s64 ttm_lru_walk_for_evict(struct ttm_lru_walk > > *walk, struct ttm_device *bdev, > > s64 lret; > > > > spin_lock(&bdev->lru_lock); > > - ttm_resource_manager_for_each_res(man, &cursor, res) { > > + ttm_resource_cursor_init(&cursor, man); > > + ttm_resource_manager_for_each_res(&cursor, res) { > > struct ttm_buffer_object *bo = res->bo; > > bool bo_needs_unlock = false; > > bool bo_locked = false; > > diff --git a/drivers/gpu/drm/ttm/ttm_resource.c > > b/drivers/gpu/drm/ttm/ttm_resource.c > > index a87665eb28a6..e19360cc7930 100644 > > --- a/drivers/gpu/drm/ttm/ttm_resource.c > > +++ b/drivers/gpu/drm/ttm/ttm_resource.c > > @@ -81,6 +81,23 @@ static void ttm_bulk_move_drop_cursors(struct > > ttm_lru_bulk_move *bulk) > > ttm_resource_cursor_clear_bulk(cursor); > > } > > > > +/** > > + * ttm_resource_cursor_init() - Initialize a struct > > ttm_resource_cursor > > + * @cursor: The cursor to initialize. > > + * @man: The resource manager. > > + * > > + * Initialize the cursor before using it for iteration. > > + */ > > +void ttm_resource_cursor_init(struct ttm_resource_cursor *cursor, > > + struct ttm_resource_manager *man) > > +{ > > + cursor->priority = 0; > > + cursor->man = man; > > + ttm_lru_item_init(&cursor->hitch, TTM_LRU_HITCH); > > + INIT_LIST_HEAD(&cursor->bulk_link); > > + INIT_LIST_HEAD(&cursor->hitch.link); > > +} > > + > > /** > > * ttm_resource_cursor_fini() - Finalize the LRU list cursor > > usage > > * @cursor: The struct ttm_resource_cursor to finalize. > > @@ -593,7 +610,6 @@ ttm_resource_cursor_check_bulk(struct > > ttm_resource_cursor *cursor, > > /** > > * ttm_resource_manager_first() - Start iterating over the > > resources > > * of a resource manager > > - * @man: resource manager to iterate over > > * @cursor: cursor to record the position > > * > > * Initializes the cursor and starts iterating. When done > > iterating, > > @@ -602,17 +618,16 @@ ttm_resource_cursor_check_bulk(struct > > ttm_resource_cursor *cursor, > > * Return: The first resource from the resource manager. > > */ > > struct ttm_resource * > > -ttm_resource_manager_first(struct ttm_resource_manager *man, > > - struct ttm_resource_cursor *cursor) > > +ttm_resource_manager_first(struct ttm_resource_cursor *cursor) > > { > > - lockdep_assert_held(&man->bdev->lru_lock); > > + struct ttm_resource_manager *man = cursor->man; > > > > - cursor->priority = 0; > > - cursor->man = man; > > - ttm_lru_item_init(&cursor->hitch, TTM_LRU_HITCH); > > - INIT_LIST_HEAD(&cursor->bulk_link); > > - list_add(&cursor->hitch.link, &man->lru[cursor- > > >priority]); > > + if (WARN_ON_ONCE(!man)) > > + return NULL; > > + > > + lockdep_assert_held(&man->bdev->lru_lock); > > > > + list_move(&cursor->hitch.link, &man->lru[cursor- > > >priority]); > > return ttm_resource_manager_next(cursor); > > } > > > > @@ -648,8 +663,6 @@ ttm_resource_manager_next(struct > > ttm_resource_cursor *cursor) > > ttm_resource_cursor_clear_bulk(cursor); > > } > > > > - ttm_resource_cursor_fini(cursor); > > - > > return NULL; > > } > > > > diff --git a/include/drm/ttm/ttm_resource.h > > b/include/drm/ttm/ttm_resource.h > > index be034be56ba1..e1f3b95d73b6 100644 > > --- a/include/drm/ttm/ttm_resource.h > > +++ b/include/drm/ttm/ttm_resource.h > > @@ -325,6 +325,9 @@ struct ttm_resource_cursor { > > unsigned int priority; > > }; > > > > +void ttm_resource_cursor_init(struct ttm_resource_cursor *cursor, > > + struct ttm_resource_manager *man); > > + > > void ttm_resource_cursor_fini(struct ttm_resource_cursor > > *cursor); > > > > /** > > @@ -456,8 +459,7 @@ void ttm_resource_manager_debug(struct > > ttm_resource_manager *man, > > struct drm_printer *p); > > > > struct ttm_resource * > > -ttm_resource_manager_first(struct ttm_resource_manager *man, > > - struct ttm_resource_cursor *cursor); > > +ttm_resource_manager_first(struct ttm_resource_cursor *cursor); > > struct ttm_resource * > > ttm_resource_manager_next(struct ttm_resource_cursor *cursor); > > > > @@ -466,14 +468,13 @@ ttm_lru_first_res_or_null(struct list_head > > *head); > > > > /** > > * ttm_resource_manager_for_each_res - iterate over all resources > > - * @man: the resource manager > > * @cursor: struct ttm_resource_cursor for the current position > > * @res: the current resource > > * > > * Iterate over all the evictable resources in a resource > > manager. > > */ > > -#define ttm_resource_manager_for_each_res(man, cursor, > > res) \ > > - for (res = ttm_resource_manager_first(man, cursor); > > res; \ > > +#define ttm_resource_manager_for_each_res(cursor, res) \ > > + for (res = ttm_resource_manager_first(cursor); res; \ > > res = ttm_resource_manager_next(cursor)) > > > > struct ttm_kmap_iter * >