Re: [Intel-gfx] [PATCH v4 30/38] drm: Compute tight evictions for drm_mm_scan

Daniel Vetter
2016-12-28T13:01:29Z

On Thu, Dec 22, 2016 at 08:36:33AM +0000, Chris Wilson wrote:
> Compute the minimal required hole during scan and only evict those nodes
> that overlap. This enables us to reduce the number of nodes we need to
> evict to the bare minimum.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Again, for next time around pls cc: driver maintainers too.
-Daniel

> ---
> drivers/gpu/drm/drm_mm.c | 60 +++++++++++++++++++++++++++------
> drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 2 +-
> drivers/gpu/drm/i915/i915_gem_evict.c | 3 +-
> drivers/gpu/drm/selftests/test-drm_mm.c | 10 +++---
> include/drm/drm_mm.h | 22 ++++++------
> 5 files changed, 71 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 1b5613bcb35e..189ab84c5a59 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -718,10 +718,10 @@ EXPORT_SYMBOL(drm_mm_replace_node);
> * @color: opaque tag value to use for the allocation
> * @start: start of the allowed range for the allocation
> * @end: end of the allowed range for the allocation
> + * @flags: flags to specify how the allocation will be performed afterwards
> *
> * This simply sets up the scanning routines with the parameters for the desired
> - * hole. Note that there's no need to specify allocation flags, since they only
> - * change the place a node is allocated from within a suitable hole.
> + * hole.
> *
> * Warning:
> * As long as the scan list is non-empty, no other operations than
> @@ -733,7 +733,8 @@ void drm_mm_scan_init_with_range(struct drm_mm_scan *scan,
> u64 alignment,
> unsigned long color,
> u64 start,
> - u64 end)
> + u64 end,
> + unsigned int flags)
> {
> DRM_MM_BUG_ON(start >= end);
> DRM_MM_BUG_ON(!size || size > end - start);
> @@ -744,6 +745,7 @@ void drm_mm_scan_init_with_range(struct drm_mm_scan *scan,
> scan->color = color;
> scan->alignment = alignment;
> scan->size = size;
> + scan->flags = flags;
>
> DRM_MM_BUG_ON(end <= start);
> scan->range_start = start;
> @@ -778,7 +780,7 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
> DRM_MM_BUG_ON(node->mm != mm);
> DRM_MM_BUG_ON(!node->allocated);
> DRM_MM_BUG_ON(node->scanned_block);
> - node->scanned_block = 1;
> + node->scanned_block = true;
> mm->scan_active++;
>
> hole = list_prev_entry(node, node_list);
> @@ -800,15 +802,53 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
>
> adj_start = max(col_start, scan->range_start);
> adj_end = min(col_end, scan->range_end);
> + if (adj_end <= adj_start || adj_end - adj_start < scan->size)
> + return false;
> +
> + if (scan->flags == DRM_MM_CREATE_TOP)
> + adj_start = adj_end - scan->size;
> +
> + if (scan->alignment) {
> + u64 rem;
> +
> + div64_u64_rem(adj_start, scan->alignment, &rem);
> + if (rem) {
> + adj_start -= rem;
> + if (scan->flags != DRM_MM_CREATE_TOP)
> + adj_start += scan->alignment;
> + if (adj_start < max(col_start, scan->range_start) ||
> + min(col_end, scan->range_end) - adj_start < scan->size)
> + return false;
> +
> + if (adj_end <= adj_start ||
> + adj_end - adj_start < scan->size)
> + return false;
> + }
> + }
>
> - if (check_free_hole(adj_start, adj_end,
> - scan->size, scan->alignment)) {
> + if (mm->color_adjust) {
> + /* If allocations need adjusting due to neighbouring colours,
> + * we do not have enough information to decide if we need
> + * to evict nodes on either side of [adj_start, adj_end].
> + * What almost works is
> + * hit_start = adj_start + (hole_start - col_start);
> + * hit_end = adj_start + scan->size + (hole_end - col_end);
> + * but because the decision is only made on the final hole,
> + * we may underestimate the required adjustments for an
> + * interior allocation.
> + */
> scan->hit_start = hole_start;
> scan->hit_end = hole_end;
> - return true;
> + } else {
> + scan->hit_start = adj_start;
> + scan->hit_end = adj_start + scan->size;
> }
>
> - return false;
> + DRM_MM_BUG_ON(scan->hit_start >= scan->hit_end);
> + DRM_MM_BUG_ON(scan->hit_start < hole_start);
> + DRM_MM_BUG_ON(scan->hit_end > hole_end);
> +
> + return true;
> }
> EXPORT_SYMBOL(drm_mm_scan_add_block);
>
> @@ -836,7 +876,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
>
> DRM_MM_BUG_ON(node->mm != scan->mm);
> DRM_MM_BUG_ON(!node->scanned_block);
> - node->scanned_block = 0;
> + node->scanned_block = false;
>
> DRM_MM_BUG_ON(!node->mm->scan_active);
> node->mm->scan_active--;
> @@ -846,7 +886,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
> prev_node->hole_follows = node->scanned_preceeds_hole;
> list_add(&node->node_list, &prev_node->node_list);
>
> - return (drm_mm_hole_node_end(node) > scan->hit_start &&
> + return (node->start + node->size > scan->hit_start &&
> node->start < scan->hit_end);
> }
> EXPORT_SYMBOL(drm_mm_scan_remove_block);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index fe1e886dcabb..2dae3169ce48 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -135,7 +135,7 @@ static int etnaviv_iommu_find_iova(struct etnaviv_iommu *mmu,
> }
>
> /* Try to retire some entries */
> - drm_mm_scan_init(&scan, &mmu->mm, size, 0, 0);
> + drm_mm_scan_init(&scan, &mmu->mm, size, 0, 0, 0);
>
> found = 0;
> INIT_LIST_HEAD(&list);
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index 77ded288534b..2741498cdf2b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -128,7 +128,8 @@ i915_gem_evict_something(struct i915_address_space *vm,
> */
> drm_mm_scan_init_with_range(&scan, &vm->mm,
> min_size, alignment, cache_level,
> - start, end);
> + start, end,
> + flags & PIN_HIGH ? DRM_MM_CREATE_TOP : 0);
>
> /* Retire before we search the active list. Although we have
> * reasonable accuracy in our retirement lists, we may have
> diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c b/drivers/gpu/drm/selftests/test-drm_mm.c
> index 997f2bc93b9b..1bbfc24342c5 100644
> --- a/drivers/gpu/drm/selftests/test-drm_mm.c
> +++ b/drivers/gpu/drm/selftests/test-drm_mm.c
> @@ -1199,7 +1199,7 @@ static bool evict_nothing(struct drm_mm *mm,
> struct drm_mm_node *node;
> unsigned int n;
>
> - drm_mm_scan_init(&scan, mm, 1, 0, 0);
> + drm_mm_scan_init(&scan, mm, 1, 0, 0, 0);
> for (n = 0; n < total_size; n++) {
> e = &nodes[n];
> list_add(&e->link, &evict_list);
> @@ -1246,7 +1246,7 @@ static bool evict_everything(struct drm_mm *mm,
> unsigned int n;
> int err;
>
> - drm_mm_scan_init(&scan, mm, total_size, 0, 0);
> + drm_mm_scan_init(&scan, mm, total_size, 0, 0, 0);
> for (n = 0; n < total_size; n++) {
> e = &nodes[n];
> list_add(&e->link, &evict_list);
> @@ -1296,7 +1296,8 @@ static int evict_something(struct drm_mm *mm,
>
> drm_mm_scan_init_with_range(&scan, mm,
> size, alignment, 0,
> - range_start, range_end);
> + range_start, range_end,
> + mode->create_flags);
> if (!evict_nodes(&scan,
> nodes, order, count,
> &evict_list))
> @@ -1874,7 +1875,8 @@ static int evict_color(struct drm_mm *mm,
>
> drm_mm_scan_init_with_range(&scan, mm,
> size, alignment, color,
> - range_start, range_end);
> + range_start, range_end,
> + mode->create_flags);
> if (!evict_nodes(&scan,
> nodes, order, count,
> &evict_list))
> diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
> index bae0f10da8e3..606336fc229a 100644
> --- a/include/drm/drm_mm.h
> +++ b/include/drm/drm_mm.h
> @@ -120,6 +120,7 @@ struct drm_mm_scan {
> struct drm_mm_node *prev_scanned_node;
>
> unsigned long color;
> + unsigned int flags;
> };
>
> /**
> @@ -388,11 +389,9 @@ __drm_mm_interval_first(const struct drm_mm *mm, u64 start, u64 last);
>
> void drm_mm_scan_init_with_range(struct drm_mm_scan *scan,
> struct drm_mm *mm,
> - u64 size,
> - u64 alignment,
> - unsigned long color,
> - u64 start,
> - u64 end);
> + u64 size, u64 alignment, unsigned long color,
> + u64 start, u64 end,
> + unsigned int flags);
>
> /**
> * drm_mm_scan_init - initialize lru scanning
> @@ -401,10 +400,10 @@ void drm_mm_scan_init_with_range(struct drm_mm_scan *scan,
> * @size: size of the allocation
> * @alignment: alignment of the allocation
> * @color: opaque tag value to use for the allocation
> + * @flags: flags to specify how the allocation will be performed afterwards
> *
> * This simply sets up the scanning routines with the parameters for the desired
> - * hole. Note that there's no need to specify allocation flags, since they only
> - * change the place a node is allocated from within a suitable hole.
> + * hole.
> *
> * Warning:
> * As long as the scan list is non-empty, no other operations than
> @@ -414,10 +413,13 @@ static inline void drm_mm_scan_init(struct drm_mm_scan *scan,
> struct drm_mm *mm,
> u64 size,
> u64 alignment,
> - unsigned long color)
> + unsigned long color,
> + unsigned int flags)
> {
> - drm_mm_scan_init_with_range(scan, mm, size, alignment, color,
> - 0, U64_MAX);
> + drm_mm_scan_init_with_range(scan, mm,
> + size, alignment, color,
> + 0, U64_MAX,
> + flags);
> }
>
> bool drm_mm_scan_add_block(struct drm_mm_scan *scan,
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org