From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: intel-xe@lists.freedesktop.org
Subject: [Intel-xe] [PATCH v3 2/4] drm/xe/bo: Avoid creating a system resource when allocating a fresh VRAM bo
Date: Mon, 26 Jun 2023 20:17:39 +0200 [thread overview]
Message-ID: <20230626181741.32820-3-thomas.hellstrom@linux.intel.com> (raw)
In-Reply-To: <20230626181741.32820-1-thomas.hellstrom@linux.intel.com>
When creating a new bo, on the first move the bo->resource is typically
NULL. Our move callback rejected that instructing TTM to create a system
resource. In addition a struct ttm_tt with a page-vector was created,
although not populated with pages. Similarly when the clearing of VRAM
was complete, the system resource was put on a ghost object and freed
using the TTM delayed destroy mechanism.
This is a lot of pointless work. So avoid creating the system resource and
instead change the code to cope with a NULL bo->resource.
v2:
- Add some code comments (Matthew Brost)
v3:
- Fix a dereference of old_mem which might be NULL.
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
drivers/gpu/drm/xe/xe_bo.c | 48 +++++++++++++++++++++-----------------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index d2f4575d8548..73e06597c84d 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -479,7 +479,6 @@ static int xe_bo_trigger_rebind(struct xe_device *xe, struct xe_bo *bo,
* to unconditionally call unmap_attachment() when moving out to system.
*/
static int xe_bo_move_dmabuf(struct ttm_buffer_object *ttm_bo,
- struct ttm_resource *old_res,
struct ttm_resource *new_res)
{
struct dma_buf_attachment *attach = ttm_bo->base.import_attach;
@@ -564,6 +563,7 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
struct xe_device *xe = ttm_to_xe_device(ttm_bo->bdev);
struct xe_bo *bo = ttm_to_xe_bo(ttm_bo);
struct ttm_resource *old_mem = ttm_bo->resource;
+ u32 old_mem_type = old_mem ? old_mem->mem_type : XE_PL_SYSTEM;
struct ttm_tt *ttm = ttm_bo->ttm;
struct xe_tile *tile = NULL;
struct dma_fence *fence;
@@ -572,35 +572,29 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
bool needs_clear;
int ret = 0;
- if (!old_mem) {
- if (new_mem->mem_type != TTM_PL_SYSTEM) {
- hop->mem_type = TTM_PL_SYSTEM;
- hop->flags = TTM_PL_FLAG_TEMPORARY;
- ret = -EMULTIHOP;
- goto out;
- }
-
+ /* Bo creation path, moving to system or TT. No clearing required. */
+ if (!old_mem && ttm) {
ttm_bo_move_null(ttm_bo, new_mem);
- goto out;
+ return 0;
}
if (ttm_bo->type == ttm_bo_type_sg) {
ret = xe_bo_move_notify(bo, ctx);
if (!ret)
- ret = xe_bo_move_dmabuf(ttm_bo, old_mem, new_mem);
+ ret = xe_bo_move_dmabuf(ttm_bo, new_mem);
goto out;
}
tt_has_data = ttm && (ttm_tt_is_populated(ttm) ||
(ttm->page_flags & TTM_TT_FLAG_SWAPPED));
- move_lacks_source = !resource_is_vram(old_mem) && !tt_has_data;
+ move_lacks_source = !mem_type_is_vram(old_mem_type) && !tt_has_data;
needs_clear = (ttm && ttm->page_flags & TTM_TT_FLAG_ZERO_ALLOC) ||
(!ttm && ttm_bo->type == ttm_bo_type_device);
if ((move_lacks_source && !needs_clear) ||
- (old_mem->mem_type == XE_PL_SYSTEM &&
+ (old_mem_type == XE_PL_SYSTEM &&
new_mem->mem_type == XE_PL_TT)) {
ttm_bo_move_null(ttm_bo, new_mem);
goto out;
@@ -610,7 +604,7 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
* Failed multi-hop where the old_mem is still marked as
* TTM_PL_FLAG_TEMPORARY, should just be a dummy move.
*/
- if (old_mem->mem_type == XE_PL_TT &&
+ if (old_mem_type == XE_PL_TT &&
new_mem->mem_type == XE_PL_TT) {
ttm_bo_move_null(ttm_bo, new_mem);
goto out;
@@ -622,7 +616,7 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
goto out;
}
- if (old_mem->mem_type == XE_PL_TT &&
+ if (old_mem_type == XE_PL_TT &&
new_mem->mem_type == XE_PL_SYSTEM) {
long timeout = dma_resv_wait_timeout(ttm_bo->base.resv,
DMA_RESV_USAGE_BOOKKEEP,
@@ -637,8 +631,8 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
}
if (!move_lacks_source &&
- ((old_mem->mem_type == XE_PL_SYSTEM && resource_is_vram(new_mem)) ||
- (resource_is_vram(old_mem) &&
+ ((old_mem_type == XE_PL_SYSTEM && resource_is_vram(new_mem)) ||
+ (mem_type_is_vram(old_mem_type) &&
new_mem->mem_type == XE_PL_SYSTEM))) {
hop->fpfn = 0;
hop->lpfn = 0;
@@ -652,8 +646,8 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
tile = bo->tile;
else if (resource_is_vram(new_mem))
tile = mem_type_to_tile(xe, new_mem->mem_type);
- else if (resource_is_vram(old_mem))
- tile = mem_type_to_tile(xe, old_mem->mem_type);
+ else if (mem_type_is_vram(old_mem_type))
+ tile = mem_type_to_tile(xe, old_mem_type);
XE_BUG_ON(!tile);
XE_BUG_ON(!tile->migrate);
@@ -703,8 +697,20 @@ static int xe_bo_move(struct ttm_buffer_object *ttm_bo, bool evict,
xe_device_mem_access_put(xe);
goto out;
}
- ret = ttm_bo_move_accel_cleanup(ttm_bo, fence, evict, true,
- new_mem);
+ if (!move_lacks_source) {
+ ret = ttm_bo_move_accel_cleanup(ttm_bo, fence, evict,
+ true, new_mem);
+ } else {
+ /*
+ * ttm_bo_move_accel_cleanup() may blow up if
+ * bo->resource == NULL, so just attach the
+ * fence and set the new resource.
+ */
+ dma_resv_add_fence(ttm_bo->base.resv, fence,
+ DMA_RESV_USAGE_KERNEL);
+ ttm_bo_move_null(ttm_bo, new_mem);
+ }
+
dma_fence_put(fence);
}
--
2.40.1
next prev parent reply other threads:[~2023-06-26 18:18 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-26 18:17 [Intel-xe] [PATCH v3 0/4] drm/xe: Eviction fixes and optimizations Thomas Hellström
2023-06-26 18:17 ` [Intel-xe] [PATCH v3 1/4] drm/xe/bo: Fix swapin when moving to VRAM Thomas Hellström
2023-06-26 18:17 ` Thomas Hellström [this message]
2023-06-27 8:54 ` [Intel-xe] [PATCH v3 2/4] drm/xe/bo: Avoid creating a system resource when allocating a fresh VRAM bo Matthew Auld
2023-06-26 18:17 ` [Intel-xe] [PATCH v3 3/4] drm/xe/bo: Gracefully handle errors from ttm_bo_move_accel_cleanup() Thomas Hellström
2023-06-27 8:55 ` Matthew Auld
2023-06-26 18:17 ` [Intel-xe] [PATCH v3 4/4] drm/xe/bo: Evict VRAM to TT rather than to system Thomas Hellström
2023-06-26 18:20 ` [Intel-xe] ✓ CI.Patch_applied: success for drm/xe: Eviction fixes and optimizations (rev3) Patchwork
2023-06-26 18:20 ` [Intel-xe] ✓ CI.checkpatch: " Patchwork
2023-06-26 18:22 ` [Intel-xe] ✓ CI.KUnit: " Patchwork
2023-06-26 18:25 ` [Intel-xe] ✓ CI.Build: " Patchwork
2023-06-26 18:26 ` [Intel-xe] ✓ CI.Hooks: " Patchwork
2023-06-26 18:27 ` [Intel-xe] ✓ CI.checksparse: " Patchwork
2023-06-26 19:11 ` [Intel-xe] ○ CI.BAT: info " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230626181741.32820-3-thomas.hellstrom@linux.intel.com \
--to=thomas.hellstrom@linux.intel.com \
--cc=intel-xe@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox