Commit 92653f2a authored by David Gow's avatar David Gow Committed by Andi Shyti

drm/i915: Attempt to get pages without eviction first

In commit a78a8da5 ("drm/ttm: replace busy placement with flags v6"),
__i915_ttm_get_pages was updated to use flags instead of the separate
'busy' placement list. However, the behaviour was subtly changed.
Originally, the function would attempt to use the preferred placement
without eviction, and give an opportunity to restart the operation
before falling back to allowing eviction.

This was unintentionally changed, as the preferred placement was not
given the TTM_PL_FLAG_DESIRED flag, and so eviction could be triggered
in that first pass. This caused thrashing, and a significant performance
regression on DG2 systems with small BAR. For example, Minecraft and
Team Fortress 2 would drop to single-digit framerates.

Restore the original behaviour by marking the initial placement as
desired on that first attempt. Also, rework this to use a separate
struct ttm_palcement, as the individual placements are marked 'const',
so hot-patching the flags is even more dodgy than before.

Thanks to Justin Brewer for bisecting this.

Fixes: a78a8da5 ("drm/ttm: replace busy placement with flags v6")
Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11255Signed-off-by: default avatarDavid Gow <david@davidgow.net>
Reviewed-by: default avatarJonathan Cavitt <jonathan.cavitt@intel.com>
Reviewed-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
Signed-off-by: default avatarAndi Shyti <andi.shyti@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240804091851.122186-3-david@davidgow.net
parent 54bf0af9
...@@ -778,13 +778,16 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj, ...@@ -778,13 +778,16 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj,
.interruptible = true, .interruptible = true,
.no_wait_gpu = false, .no_wait_gpu = false,
}; };
int real_num_busy; struct ttm_placement initial_placement;
struct ttm_place initial_place;
int ret; int ret;
/* First try only the requested placement. No eviction. */ /* First try only the requested placement. No eviction. */
real_num_busy = placement->num_placement; initial_placement.num_placement = 1;
placement->num_placement = 1; memcpy(&initial_place, placement->placement, sizeof(struct ttm_place));
ret = ttm_bo_validate(bo, placement, &ctx); initial_place.flags |= TTM_PL_FLAG_DESIRED;
initial_placement.placement = &initial_place;
ret = ttm_bo_validate(bo, &initial_placement, &ctx);
if (ret) { if (ret) {
ret = i915_ttm_err_to_gem(ret); ret = i915_ttm_err_to_gem(ret);
/* /*
...@@ -799,7 +802,6 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj, ...@@ -799,7 +802,6 @@ static int __i915_ttm_get_pages(struct drm_i915_gem_object *obj,
* If the initial attempt fails, allow all accepted placements, * If the initial attempt fails, allow all accepted placements,
* evicting if necessary. * evicting if necessary.
*/ */
placement->num_placement = real_num_busy;
ret = ttm_bo_validate(bo, placement, &ctx); ret = ttm_bo_validate(bo, placement, &ctx);
if (ret) if (ret)
return i915_ttm_err_to_gem(ret); return i915_ttm_err_to_gem(ret);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment