| Message ID | 20240804091851.122186-2-david@davidgow.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix ttm small BAR placement handling | expand |
-----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of David Gow Sent: Sunday, August 4, 2024 2:19 AM To: Christian König <ckoenig.leichtzumerken@gmail.com>; Somalapuram Amaranath <Amaranath.Somalapuram@amd.com>; Thomas Hellström <thomas.hellstrom@linux.intel.com>; De Marchi, Lucas <lucas.demarchi@intel.com>; Andi Shyti <andi.shyti@linux.intel.com>; Thomas Zimmermann <tzimmermann@suse.de> Cc: David Gow <david@davidgow.net>; Jani Nikula <jani.nikula@linux.intel.com>; Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Tvrtko Ursulin <tursulin@ursulin.net>; Ville Syrjälä <ville.syrjala@linux.intel.com>; intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org Subject: [PATCH 1/2] drm/i915: Allow evicting to use the requested placement > > In commit a78a8da51b36 ("drm/ttm: replace busy placement with flags v6"), > the old system of having a separate placement list (for placements > which should be used without eviction) and a 'busy' placement list (for > placements which should be attempted if eviction is required) was > replaced with a new one where placements could be marked 'FALLBACK' (to > be attempted if eviction is required) or 'DESIRED' (to be attempted > first, but not if eviction is required). > > i915 had always included the requested placement in the list of > 'busy' placements: i.e., the placement could be used either if eviction > is required or not. But when the new system was put in place, the > requested (first) placement was marked 'DESIRED', so would never be used > if eviction became necessary. While a bug in the original commit > prevented this flag from working, when this was fixed in > 4a0e7b3c ("drm/i915: fix applying placement flag"), it caused long hangs > on DG2 systems with small BAR. > > Don't mark the requested placement DESIRED (or FALLBACK), allowing it to > be used in both situations. This matches the old behaviour, and resolves > the hangs. > > Thanks to Justin Brewer for bisecting the issue. > > Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") > Fixes: 4a0e7b3c3753 ("drm/i915: fix applying placement flag") > Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11255 > Signed-off-by: David Gow <david@davidgow.net> Thank you for the thorough write-up of this issue. Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > --- > drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > index e6f177183c0f..fb848fd8ba15 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c > @@ -165,7 +165,6 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, > i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] : > obj->mm.region, &places[0], obj->bo_offset, > obj->base.size, flags); > - places[0].flags |= TTM_PL_FLAG_DESIRED; > > /* Cache this on object? */ > for (i = 0; i < num_allowed; ++i) { > -- > 2.46.0 > >
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c index e6f177183c0f..fb848fd8ba15 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c @@ -165,7 +165,6 @@ i915_ttm_placement_from_obj(const struct drm_i915_gem_object *obj, i915_ttm_place_from_region(num_allowed ? obj->mm.placements[0] : obj->mm.region, &places[0], obj->bo_offset, obj->base.size, flags); - places[0].flags |= TTM_PL_FLAG_DESIRED; /* Cache this on object? */ for (i = 0; i < num_allowed; ++i) {
In commit a78a8da51b36 ("drm/ttm: replace busy placement with flags v6"), the old system of having a separate placement list (for placements which should be used without eviction) and a 'busy' placement list (for placements which should be attempted if eviction is required) was replaced with a new one where placements could be marked 'FALLBACK' (to be attempted if eviction is required) or 'DESIRED' (to be attempted first, but not if eviction is required). i915 had always included the requested placement in the list of 'busy' placements: i.e., the placement could be used either if eviction is required or not. But when the new system was put in place, the requested (first) placement was marked 'DESIRED', so would never be used if eviction became necessary. While a bug in the original commit prevented this flag from working, when this was fixed in 4a0e7b3c ("drm/i915: fix applying placement flag"), it caused long hangs on DG2 systems with small BAR. Don't mark the requested placement DESIRED (or FALLBACK), allowing it to be used in both situations. This matches the old behaviour, and resolves the hangs. Thanks to Justin Brewer for bisecting the issue. Fixes: a78a8da51b36 ("drm/ttm: replace busy placement with flags v6") Fixes: 4a0e7b3c3753 ("drm/i915: fix applying placement flag") Link: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11255 Signed-off-by: David Gow <david@davidgow.net> --- drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 1 - 1 file changed, 1 deletion(-)