Re: [Intel-gfx] [PATCH] drm/i915/ttm: Fix incorrect assumptions about ttm_bo_validate() semantics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/18/21 12:53 PM, Matthew Auld wrote:
On Fri, 18 Jun 2021 at 09:31, Thomas Hellström
<thomas.hellstrom@xxxxxxxxxxxxxxx> wrote:
We have assumed that if the current placement was not the requested
placement, but instead one of the busy placements, a TTM move would have
been triggered. That is not the case.

So when we initially place LMEM objects in "Limbo", (that is system
placement without any pages allocated), to be able to defer clearing
objects until first get_pages(), the first get_pages() would happily keep
objects in system memory if that is one of the allowed placements. And
since we don't yet support i915 GEM system memory from TTM, everything
breaks apart.

So make sure we try the requested placement first, if no eviction is
needed. If that fails, retry with all allowed placements also allowing
evictions. Also make sure we handle TTM failure codes correctly.

Also temporarily (until we support i915 GEM system on TTM), restrict
allowed placements to the requested placement to avoid things falling
apart should LMEM be full.

Fixes: 38f28c0695c0 ("drm/i915/ttm: Calculate the object placement at get_pages time)
Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 61 +++++++++++++++++++++++--
  1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index df46535cca47..4bb0440f693c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -64,6 +64,30 @@ static struct ttm_placement i915_sys_placement = {
         .busy_placement = &sys_placement_flags,
  };

+static int i915_ttm_err_to_gem(int err)
+{
+       /* Fastpath */
+       if (likely(!err))
+               return 0;
+
+       switch (err) {
+       case -EBUSY:
+               /*
+                * TTM likes to convert -EDEADLK to -EBUSY, and wants us to
+                * restart the operation, since we don't record the contending
+                * lock. We use -EAGAIN to restart.
+                */
+               return -EAGAIN;
+       case -ENOSPC:
+               /* Memory type / region is full, and we can't evict. */
+               return -ENXIO;
ttm system will return -ENOMEM right?

Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>

Hmm, Yes, I suppose so. Will that need some mangling before handing over to GEM?

Thanks for reviewing!

Thomas





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux