The first 5 patches are just cleanups to remove unused functions and handle ttm operation ctx nicer in driver move paths.
The last 5 patches have the goal to invert the operation of the move driver callback. Currently the core does some tt related moves itself and pass the drivers the rest. I'd like to have the driver decide things instead, so only use the fallback paths when a driver has no move callback.
Dave.
From: Dave Airlie airlied@redhat.com
this is unused
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/radeon/radeon_object.c | 15 --------------- drivers/gpu/drm/radeon/radeon_object.h | 3 --- 2 files changed, 18 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 0de267ab3913..689426dd8480 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -825,21 +825,6 @@ int radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo) return 0; }
-int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, bool no_wait) -{ - int r; - - r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL); - if (unlikely(r != 0)) - return r; - if (mem_type) - *mem_type = bo->tbo.mem.mem_type; - - r = ttm_bo_wait(&bo->tbo, true, no_wait); - ttm_bo_unreserve(&bo->tbo); - return r; -} - /** * radeon_bo_fence - add fence to buffer object * diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index e097a915277d..27cfb64057fe 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -133,9 +133,6 @@ static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo) return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); }
-extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, - bool no_wait); - extern int radeon_bo_create(struct radeon_device *rdev, unsigned long size, int byte_align, bool kernel, u32 domain, u32 flags,
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
this is unused
Signed-off-by: Dave Airlie airlied@redhat.com
Reviewed-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/radeon/radeon_object.c | 15 --------------- drivers/gpu/drm/radeon/radeon_object.h | 3 --- 2 files changed, 18 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 0de267ab3913..689426dd8480 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -825,21 +825,6 @@ int radeon_bo_fault_reserve_notify(struct ttm_buffer_object *bo) return 0; }
-int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type, bool no_wait) -{
- int r;
- r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL);
- if (unlikely(r != 0))
return r;
- if (mem_type)
*mem_type = bo->tbo.mem.mem_type;
- r = ttm_bo_wait(&bo->tbo, true, no_wait);
- ttm_bo_unreserve(&bo->tbo);
- return r;
-}
- /**
- radeon_bo_fence - add fence to buffer object
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h index e097a915277d..27cfb64057fe 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -133,9 +133,6 @@ static inline u64 radeon_bo_mmap_offset(struct radeon_bo *bo) return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); }
-extern int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
bool no_wait);
- extern int radeon_bo_create(struct radeon_device *rdev, unsigned long size, int byte_align, bool kernel, u32 domain, u32 flags,
From: Dave Airlie airlied@redhat.com
This wasn't used anywheere
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/qxl/qxl_object.h | 23 ----------------------- 1 file changed, 23 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index c7d79b20622e..09a5c818324d 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -58,29 +58,6 @@ static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo) return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); }
-static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type, - bool no_wait) -{ - int r; - - r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL); - if (unlikely(r != 0)) { - if (r != -ERESTARTSYS) { - struct drm_device *ddev = bo->tbo.base.dev; - - dev_err(ddev->dev, "%p reserve failed for wait\n", - bo); - } - return r; - } - if (mem_type) - *mem_type = bo->tbo.mem.mem_type; - - r = ttm_bo_wait(&bo->tbo, true, no_wait); - ttm_bo_unreserve(&bo->tbo); - return r; -} - extern int qxl_bo_create(struct qxl_device *qdev, unsigned long size, bool kernel, bool pinned, u32 domain,
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
This wasn't used anywheere
Signed-off-by: Dave Airlie airlied@redhat.com
Acked-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/qxl/qxl_object.h | 23 ----------------------- 1 file changed, 23 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index c7d79b20622e..09a5c818324d 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -58,29 +58,6 @@ static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo) return drm_vma_node_offset_addr(&bo->tbo.base.vma_node); }
-static inline int qxl_bo_wait(struct qxl_bo *bo, u32 *mem_type,
bool no_wait)
-{
- int r;
- r = ttm_bo_reserve(&bo->tbo, true, no_wait, NULL);
- if (unlikely(r != 0)) {
if (r != -ERESTARTSYS) {
struct drm_device *ddev = bo->tbo.base.dev;
dev_err(ddev->dev, "%p reserve failed for wait\n",
bo);
}
return r;
- }
- if (mem_type)
*mem_type = bo->tbo.mem.mem_type;
- r = ttm_bo_wait(&bo->tbo, true, no_wait);
- ttm_bo_unreserve(&bo->tbo);
- return r;
-}
- extern int qxl_bo_create(struct qxl_device *qdev, unsigned long size, bool kernel, bool pinned, u32 domain,
From: Dave Airlie airlied@redhat.com
Just pass it around move, and remove unused pieces
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/radeon/radeon_ttm.c | 34 +++++++++++++---------------- 1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 085f58e833d8..9ff8c81d7784 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -151,7 +151,7 @@ static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp) }
static int radeon_move_blit(struct ttm_buffer_object *bo, - bool evict, bool no_wait_gpu, + bool evict, struct ttm_resource *new_mem, struct ttm_resource *old_mem) { @@ -206,11 +206,10 @@ static int radeon_move_blit(struct ttm_buffer_object *bo, }
static int radeon_move_vram_ram(struct ttm_buffer_object *bo, - bool evict, bool interruptible, - bool no_wait_gpu, + bool evict, + struct ttm_operation_ctx *ctx, struct ttm_resource *new_mem) { - struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu }; struct ttm_resource *old_mem = &bo->mem; struct ttm_resource tmp_mem; struct ttm_place placements; @@ -227,7 +226,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, placements.lpfn = 0; placements.mem_type = TTM_PL_TT; placements.flags = TTM_PL_MASK_CACHING; - r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx); + r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx); if (unlikely(r)) { return r; } @@ -237,7 +236,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, goto out_cleanup; }
- r = ttm_tt_populate(bo->bdev, bo->ttm, &ctx); + r = ttm_tt_populate(bo->bdev, bo->ttm, ctx); if (unlikely(r)) { goto out_cleanup; } @@ -246,22 +245,21 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, if (unlikely(r)) { goto out_cleanup; } - r = radeon_move_blit(bo, true, no_wait_gpu, &tmp_mem, old_mem); + r = radeon_move_blit(bo, true, &tmp_mem, old_mem); if (unlikely(r)) { goto out_cleanup; } - r = ttm_bo_move_ttm(bo, &ctx, new_mem); + r = ttm_bo_move_ttm(bo, ctx, new_mem); out_cleanup: ttm_resource_free(bo, &tmp_mem); return r; }
static int radeon_move_ram_vram(struct ttm_buffer_object *bo, - bool evict, bool interruptible, - bool no_wait_gpu, + bool evict, + struct ttm_operation_ctx *ctx, struct ttm_resource *new_mem) { - struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu }; struct ttm_resource *old_mem = &bo->mem; struct ttm_resource tmp_mem; struct ttm_placement placement; @@ -278,15 +276,15 @@ static int radeon_move_ram_vram(struct ttm_buffer_object *bo, placements.lpfn = 0; placements.mem_type = TTM_PL_TT; placements.flags = TTM_PL_MASK_CACHING; - r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx); + r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx); if (unlikely(r)) { return r; } - r = ttm_bo_move_ttm(bo, &ctx, &tmp_mem); + r = ttm_bo_move_ttm(bo, ctx, &tmp_mem); if (unlikely(r)) { goto out_cleanup; } - r = radeon_move_blit(bo, true, no_wait_gpu, new_mem, old_mem); + r = radeon_move_blit(bo, true, new_mem, old_mem); if (unlikely(r)) { goto out_cleanup; } @@ -334,14 +332,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
if (old_mem->mem_type == TTM_PL_VRAM && new_mem->mem_type == TTM_PL_SYSTEM) { - r = radeon_move_vram_ram(bo, evict, ctx->interruptible, - ctx->no_wait_gpu, new_mem); + r = radeon_move_vram_ram(bo, evict, ctx, new_mem); } else if (old_mem->mem_type == TTM_PL_SYSTEM && new_mem->mem_type == TTM_PL_VRAM) { - r = radeon_move_ram_vram(bo, evict, ctx->interruptible, - ctx->no_wait_gpu, new_mem); + r = radeon_move_ram_vram(bo, evict, ctx, new_mem); } else { - r = radeon_move_blit(bo, evict, ctx->no_wait_gpu, + r = radeon_move_blit(bo, evict, new_mem, old_mem); }
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
Just pass it around move, and remove unused pieces
Signed-off-by: Dave Airlie airlied@redhat.com
Reviewed-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/radeon/radeon_ttm.c | 34 +++++++++++++---------------- 1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 085f58e833d8..9ff8c81d7784 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -151,7 +151,7 @@ static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp) }
static int radeon_move_blit(struct ttm_buffer_object *bo,
bool evict, bool no_wait_gpu,
{bool evict, struct ttm_resource *new_mem, struct ttm_resource *old_mem)
@@ -206,11 +206,10 @@ static int radeon_move_blit(struct ttm_buffer_object *bo, }
static int radeon_move_vram_ram(struct ttm_buffer_object *bo,
bool evict, bool interruptible,
bool no_wait_gpu,
bool evict,
{struct ttm_operation_ctx *ctx, struct ttm_resource *new_mem)
- struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu }; struct ttm_resource *old_mem = &bo->mem; struct ttm_resource tmp_mem; struct ttm_place placements;
@@ -227,7 +226,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, placements.lpfn = 0; placements.mem_type = TTM_PL_TT; placements.flags = TTM_PL_MASK_CACHING;
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx);
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx); if (unlikely(r)) { return r; }
@@ -237,7 +236,7 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, goto out_cleanup; }
- r = ttm_tt_populate(bo->bdev, bo->ttm, &ctx);
- r = ttm_tt_populate(bo->bdev, bo->ttm, ctx); if (unlikely(r)) { goto out_cleanup; }
@@ -246,22 +245,21 @@ static int radeon_move_vram_ram(struct ttm_buffer_object *bo, if (unlikely(r)) { goto out_cleanup; }
- r = radeon_move_blit(bo, true, no_wait_gpu, &tmp_mem, old_mem);
- r = radeon_move_blit(bo, true, &tmp_mem, old_mem); if (unlikely(r)) { goto out_cleanup; }
- r = ttm_bo_move_ttm(bo, &ctx, new_mem);
r = ttm_bo_move_ttm(bo, ctx, new_mem); out_cleanup: ttm_resource_free(bo, &tmp_mem); return r; }
static int radeon_move_ram_vram(struct ttm_buffer_object *bo,
bool evict, bool interruptible,
bool no_wait_gpu,
bool evict,
{struct ttm_operation_ctx *ctx, struct ttm_resource *new_mem)
- struct ttm_operation_ctx ctx = { interruptible, no_wait_gpu }; struct ttm_resource *old_mem = &bo->mem; struct ttm_resource tmp_mem; struct ttm_placement placement;
@@ -278,15 +276,15 @@ static int radeon_move_ram_vram(struct ttm_buffer_object *bo, placements.lpfn = 0; placements.mem_type = TTM_PL_TT; placements.flags = TTM_PL_MASK_CACHING;
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem, &ctx);
- r = ttm_bo_mem_space(bo, &placement, &tmp_mem, ctx); if (unlikely(r)) { return r; }
- r = ttm_bo_move_ttm(bo, &ctx, &tmp_mem);
- r = ttm_bo_move_ttm(bo, ctx, &tmp_mem); if (unlikely(r)) { goto out_cleanup; }
- r = radeon_move_blit(bo, true, no_wait_gpu, new_mem, old_mem);
- r = radeon_move_blit(bo, true, new_mem, old_mem); if (unlikely(r)) { goto out_cleanup; }
@@ -334,14 +332,12 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict,
if (old_mem->mem_type == TTM_PL_VRAM && new_mem->mem_type == TTM_PL_SYSTEM) {
r = radeon_move_vram_ram(bo, evict, ctx->interruptible,
ctx->no_wait_gpu, new_mem);
} else if (old_mem->mem_type == TTM_PL_SYSTEM && new_mem->mem_type == TTM_PL_VRAM) {r = radeon_move_vram_ram(bo, evict, ctx, new_mem);
r = radeon_move_ram_vram(bo, evict, ctx->interruptible,
ctx->no_wait_gpu, new_mem);
} else {r = radeon_move_ram_vram(bo, evict, ctx, new_mem);
r = radeon_move_blit(bo, evict, ctx->no_wait_gpu,
}r = radeon_move_blit(bo, evict, new_mem, old_mem);
From: Dave Airlie airlied@redhat.com
This just uses the ctx instead of passing bools and recreating it.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 48 +++++++++++++--------------- 1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index bcae4514952f..8a90b07f17a4 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -772,8 +772,9 @@ nouveau_bo_move_prep(struct nouveau_drm *drm, struct ttm_buffer_object *bo, }
static int -nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict, bool intr, - bool no_wait_gpu, struct ttm_resource *new_reg) +nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict, + struct ttm_operation_ctx *ctx, + struct ttm_resource *new_reg) { struct nouveau_drm *drm = nouveau_bdev(bo->bdev); struct nouveau_channel *chan = drm->ttm.chan; @@ -792,7 +793,7 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict, bool intr, }
mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING); - ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, intr); + ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, ctx->interruptible); if (ret == 0) { ret = drm->ttm.move(chan, bo, &bo->mem, new_reg); if (ret == 0) { @@ -879,10 +880,10 @@ nouveau_bo_move_init(struct nouveau_drm *drm) }
static int -nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr, - bool no_wait_gpu, struct ttm_resource *new_reg) +nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, + struct ttm_operation_ctx *ctx, + struct ttm_resource *new_reg) { - struct ttm_operation_ctx ctx = { intr, no_wait_gpu }; struct ttm_place placement_memtype = { .fpfn = 0, .lpfn = 0, @@ -898,11 +899,11 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr,
tmp_reg = *new_reg; tmp_reg.mm_node = NULL; - ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, &ctx); + ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, ctx); if (ret) return ret;
- ret = ttm_tt_populate(bo->bdev, bo->ttm, &ctx); + ret = ttm_tt_populate(bo->bdev, bo->ttm, ctx); if (ret) goto out;
@@ -910,21 +911,21 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr, if (ret) goto out;
- ret = nouveau_bo_move_m2mf(bo, true, intr, no_wait_gpu, &tmp_reg); + ret = nouveau_bo_move_m2mf(bo, true, ctx, &tmp_reg); if (ret) goto out;
- ret = ttm_bo_move_ttm(bo, &ctx, new_reg); + ret = ttm_bo_move_ttm(bo, ctx, new_reg); out: ttm_resource_free(bo, &tmp_reg); return ret; }
static int -nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict, bool intr, - bool no_wait_gpu, struct ttm_resource *new_reg) +nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict, + struct ttm_operation_ctx *ctx, + struct ttm_resource *new_reg) { - struct ttm_operation_ctx ctx = { intr, no_wait_gpu }; struct ttm_place placement_memtype = { .fpfn = 0, .lpfn = 0, @@ -940,15 +941,15 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict, bool intr,
tmp_reg = *new_reg; tmp_reg.mm_node = NULL; - ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, &ctx); + ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, ctx); if (ret) return ret;
- ret = ttm_bo_move_ttm(bo, &ctx, &tmp_reg); + ret = ttm_bo_move_ttm(bo, ctx, &tmp_reg); if (ret) goto out;
- ret = nouveau_bo_move_m2mf(bo, true, intr, no_wait_gpu, new_reg); + ret = nouveau_bo_move_m2mf(bo, true, ctx, new_reg); if (ret) goto out;
@@ -1059,17 +1060,14 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, /* Hardware assisted copy. */ if (drm->ttm.move) { if (new_reg->mem_type == TTM_PL_SYSTEM) - ret = nouveau_bo_move_flipd(bo, evict, - ctx->interruptible, - ctx->no_wait_gpu, new_reg); + ret = nouveau_bo_move_flipd(bo, evict, ctx, + new_reg); else if (old_reg->mem_type == TTM_PL_SYSTEM) - ret = nouveau_bo_move_flips(bo, evict, - ctx->interruptible, - ctx->no_wait_gpu, new_reg); + ret = nouveau_bo_move_flips(bo, evict, ctx, + new_reg); else - ret = nouveau_bo_move_m2mf(bo, evict, - ctx->interruptible, - ctx->no_wait_gpu, new_reg); + ret = nouveau_bo_move_m2mf(bo, evict, ctx, + new_reg); if (!ret) goto out; }
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
This just uses the ctx instead of passing bools and recreating it.
Signed-off-by: Dave Airlie airlied@redhat.com
Acked-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/nouveau/nouveau_bo.c | 48 +++++++++++++--------------- 1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index bcae4514952f..8a90b07f17a4 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -772,8 +772,9 @@ nouveau_bo_move_prep(struct nouveau_drm *drm, struct ttm_buffer_object *bo, }
static int -nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict, bool intr,
bool no_wait_gpu, struct ttm_resource *new_reg)
+nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict,
struct ttm_operation_ctx *ctx,
{ struct nouveau_drm *drm = nouveau_bdev(bo->bdev); struct nouveau_channel *chan = drm->ttm.chan;struct ttm_resource *new_reg)
@@ -792,7 +793,7 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict, bool intr, }
mutex_lock_nested(&cli->mutex, SINGLE_DEPTH_NESTING);
- ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, intr);
- ret = nouveau_fence_sync(nouveau_bo(bo), chan, true, ctx->interruptible); if (ret == 0) { ret = drm->ttm.move(chan, bo, &bo->mem, new_reg); if (ret == 0) {
@@ -879,10 +880,10 @@ nouveau_bo_move_init(struct nouveau_drm *drm) }
static int -nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr,
bool no_wait_gpu, struct ttm_resource *new_reg)
+nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict,
struct ttm_operation_ctx *ctx,
{struct ttm_resource *new_reg)
- struct ttm_operation_ctx ctx = { intr, no_wait_gpu }; struct ttm_place placement_memtype = { .fpfn = 0, .lpfn = 0,
@@ -898,11 +899,11 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr,
tmp_reg = *new_reg; tmp_reg.mm_node = NULL;
- ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, &ctx);
- ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, ctx); if (ret) return ret;
- ret = ttm_tt_populate(bo->bdev, bo->ttm, &ctx);
- ret = ttm_tt_populate(bo->bdev, bo->ttm, ctx); if (ret) goto out;
@@ -910,21 +911,21 @@ nouveau_bo_move_flipd(struct ttm_buffer_object *bo, bool evict, bool intr, if (ret) goto out;
- ret = nouveau_bo_move_m2mf(bo, true, intr, no_wait_gpu, &tmp_reg);
- ret = nouveau_bo_move_m2mf(bo, true, ctx, &tmp_reg); if (ret) goto out;
- ret = ttm_bo_move_ttm(bo, &ctx, new_reg);
ret = ttm_bo_move_ttm(bo, ctx, new_reg); out: ttm_resource_free(bo, &tmp_reg); return ret; }
static int
-nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict, bool intr,
bool no_wait_gpu, struct ttm_resource *new_reg)
+nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict,
struct ttm_operation_ctx *ctx,
{struct ttm_resource *new_reg)
- struct ttm_operation_ctx ctx = { intr, no_wait_gpu }; struct ttm_place placement_memtype = { .fpfn = 0, .lpfn = 0,
@@ -940,15 +941,15 @@ nouveau_bo_move_flips(struct ttm_buffer_object *bo, bool evict, bool intr,
tmp_reg = *new_reg; tmp_reg.mm_node = NULL;
- ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, &ctx);
- ret = ttm_bo_mem_space(bo, &placement, &tmp_reg, ctx); if (ret) return ret;
- ret = ttm_bo_move_ttm(bo, &ctx, &tmp_reg);
- ret = ttm_bo_move_ttm(bo, ctx, &tmp_reg); if (ret) goto out;
- ret = nouveau_bo_move_m2mf(bo, true, intr, no_wait_gpu, new_reg);
- ret = nouveau_bo_move_m2mf(bo, true, ctx, new_reg); if (ret) goto out;
@@ -1059,17 +1060,14 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, /* Hardware assisted copy. */ if (drm->ttm.move) { if (new_reg->mem_type == TTM_PL_SYSTEM)
ret = nouveau_bo_move_flipd(bo, evict,
ctx->interruptible,
ctx->no_wait_gpu, new_reg);
ret = nouveau_bo_move_flipd(bo, evict, ctx,
else if (old_reg->mem_type == TTM_PL_SYSTEM)new_reg);
ret = nouveau_bo_move_flips(bo, evict,
ctx->interruptible,
ctx->no_wait_gpu, new_reg);
ret = nouveau_bo_move_flips(bo, evict, ctx,
elsenew_reg);
ret = nouveau_bo_move_m2mf(bo, evict,
ctx->interruptible,
ctx->no_wait_gpu, new_reg);
ret = nouveau_bo_move_m2mf(bo, evict, ctx,
if (!ret) goto out; }new_reg);
From: Dave Airlie airlied@redhat.com
I'm thinking of pushing the wait into the drivers.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++-- drivers/gpu/drm/qxl/qxl_ttm.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++-- include/drm/ttm/ttm_bo_api.h | 5 +++++ 5 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 8a90b07f17a4..8d51cfca07c8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1038,7 +1038,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, struct nouveau_drm_tile *new_tile = NULL; int ret = 0;
- ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu); + ret = ttm_bo_wait_ctx(bo, ctx); if (ret) return ret;
@@ -1073,7 +1073,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, }
/* Fallback to software copy. */ - ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu); + ret = ttm_bo_wait_ctx(bo, ctx); if (ret == 0) ret = ttm_bo_move_memcpy(bo, ctx, new_reg);
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 01fe0c3a3d9a..2c35ca4270c6 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -160,7 +160,7 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict, struct ttm_resource *old_mem = &bo->mem; int ret;
- ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu); + ret = ttm_bo_wait_ctx(bo, ctx); if (ret) return ret;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 9ff8c81d7784..ea9ffa6198da 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -302,7 +302,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, struct ttm_resource *old_mem = &bo->mem; int r;
- r = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu); + r = ttm_bo_wait_ctx(bo, ctx); if (r) return r;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 1968df9743fc..bdee4df1f3f2 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -59,7 +59,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo, int ret;
if (old_mem->mem_type != TTM_PL_SYSTEM) { - ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu); + ret = ttm_bo_wait_ctx(bo, ctx);
if (unlikely(ret != 0)) { if (ret != -ERESTARTSYS) @@ -231,7 +231,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, unsigned long add = 0; int dir;
- ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu); + ret = ttm_bo_wait_ctx(bo, ctx); if (ret) return ret;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 6cbe59bc97ab..b840756dbcca 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -262,6 +262,11 @@ ttm_bo_get_unless_zero(struct ttm_buffer_object *bo) */ int ttm_bo_wait(struct ttm_buffer_object *bo, bool interruptible, bool no_wait);
+static inline int ttm_bo_wait_ctx(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx) +{ + return ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu); +} + /** * ttm_bo_mem_compat - Check if proposed placement is compatible with a bo *
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
I'm thinking of pushing the wait into the drivers.
Signed-off-by: Dave Airlie airlied@redhat.com
Reviewed-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++-- drivers/gpu/drm/qxl/qxl_ttm.c | 2 +- drivers/gpu/drm/radeon/radeon_ttm.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++-- include/drm/ttm/ttm_bo_api.h | 5 +++++ 5 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 8a90b07f17a4..8d51cfca07c8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1038,7 +1038,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, struct nouveau_drm_tile *new_tile = NULL; int ret = 0;
- ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
- ret = ttm_bo_wait_ctx(bo, ctx); if (ret) return ret;
@@ -1073,7 +1073,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, }
/* Fallback to software copy. */
- ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
- ret = ttm_bo_wait_ctx(bo, ctx); if (ret == 0) ret = ttm_bo_move_memcpy(bo, ctx, new_reg);
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c index 01fe0c3a3d9a..2c35ca4270c6 100644 --- a/drivers/gpu/drm/qxl/qxl_ttm.c +++ b/drivers/gpu/drm/qxl/qxl_ttm.c @@ -160,7 +160,7 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict, struct ttm_resource *old_mem = &bo->mem; int ret;
- ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
- ret = ttm_bo_wait_ctx(bo, ctx); if (ret) return ret;
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index 9ff8c81d7784..ea9ffa6198da 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -302,7 +302,7 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, struct ttm_resource *old_mem = &bo->mem; int r;
- r = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
- r = ttm_bo_wait_ctx(bo, ctx); if (r) return r;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c index 1968df9743fc..bdee4df1f3f2 100644 --- a/drivers/gpu/drm/ttm/ttm_bo_util.c +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c @@ -59,7 +59,7 @@ int ttm_bo_move_ttm(struct ttm_buffer_object *bo, int ret;
if (old_mem->mem_type != TTM_PL_SYSTEM) {
ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
ret = ttm_bo_wait_ctx(bo, ctx);
if (unlikely(ret != 0)) { if (ret != -ERESTARTSYS)
@@ -231,7 +231,7 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo, unsigned long add = 0; int dir;
- ret = ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
- ret = ttm_bo_wait_ctx(bo, ctx); if (ret) return ret;
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h index 6cbe59bc97ab..b840756dbcca 100644 --- a/include/drm/ttm/ttm_bo_api.h +++ b/include/drm/ttm/ttm_bo_api.h @@ -262,6 +262,11 @@ ttm_bo_get_unless_zero(struct ttm_buffer_object *bo) */ int ttm_bo_wait(struct ttm_buffer_object *bo, bool interruptible, bool no_wait);
+static inline int ttm_bo_wait_ctx(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx) +{
- return ttm_bo_wait(bo, ctx->interruptible, ctx->no_wait_gpu);
+}
- /**
- ttm_bo_mem_compat - Check if proposed placement is compatible with a bo
From: Dave Airlie airlied@redhat.com
This just consolidates the code making the flow easier to understand and also helps when moving move to the driver side.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/ttm/ttm_bo.c | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 5737b3fae1b3..993a87443c37 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -265,20 +265,18 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (ret) goto out_err; } - - if (bo->mem.mem_type == TTM_PL_SYSTEM) { - if (bdev->driver->move_notify) - bdev->driver->move_notify(bo, evict, mem); - bo->mem = *mem; - goto moved; - } }
if (bdev->driver->move_notify) bdev->driver->move_notify(bo, evict, mem);
- if (old_man->use_tt && new_man->use_tt) - ret = ttm_bo_move_ttm(bo, ctx, mem); + if (old_man->use_tt && new_man->use_tt) { + if (bo->mem.mem_type == TTM_PL_SYSTEM) { + ttm_bo_assign_mem(bo, mem); + ret = 0; + } else + ret = ttm_bo_move_ttm(bo, ctx, mem); + } else if (bdev->driver->move) ret = bdev->driver->move(bo, evict, ctx, mem); else @@ -294,7 +292,6 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, goto out_err; }
-moved: ctx->bytes_moved += bo->num_pages << PAGE_SHIFT; return 0;
From: Dave Airlie airlied@redhat.com
The core move code currently handles use_tt moves, for amdgpu this was being handled also in the driver, but not using the same paths.
If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index db5f761f37ec..d3bd2fd448be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, ttm_bo_move_null(bo, new_mem); return 0; } - if ((old_mem->mem_type == TTM_PL_TT && - new_mem->mem_type == TTM_PL_SYSTEM) || - (old_mem->mem_type == TTM_PL_SYSTEM && - new_mem->mem_type == TTM_PL_TT)) { - /* bind is enough */ + if (old_mem->mem_type == TTM_PL_SYSTEM && + new_mem->mem_type == TTM_PL_TT) { ttm_bo_move_null(bo, new_mem); return 0; } + + if (old_mem->mem_type == TTM_PL_TT && + new_mem->mem_type == TTM_PL_SYSTEM) + return ttm_bo_move_ttm(bo, ctx, new_mem); + if (old_mem->mem_type == AMDGPU_PL_GDS || old_mem->mem_type == AMDGPU_PL_GWS || old_mem->mem_type == AMDGPU_PL_OA ||
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
The core move code currently handles use_tt moves, for amdgpu this was being handled also in the driver, but not using the same paths.
If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index db5f761f37ec..d3bd2fd448be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, ttm_bo_move_null(bo, new_mem); return 0; }
- if ((old_mem->mem_type == TTM_PL_TT &&
new_mem->mem_type == TTM_PL_SYSTEM) ||
(old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT)) {
/* bind is enough */
- if (old_mem->mem_type == TTM_PL_SYSTEM &&
ttm_bo_move_null(bo, new_mem);new_mem->mem_type == TTM_PL_TT) {
I would feel better if we nuke ttm_bo_move_null() and always use ttm_bo_move_ttm().
Christian.
return 0;
}
- if (old_mem->mem_type == TTM_PL_TT &&
new_mem->mem_type == TTM_PL_SYSTEM)
return ttm_bo_move_ttm(bo, ctx, new_mem);
- if (old_mem->mem_type == AMDGPU_PL_GDS || old_mem->mem_type == AMDGPU_PL_GWS || old_mem->mem_type == AMDGPU_PL_OA ||
On Thu, 24 Sep 2020 at 00:46, Christian König christian.koenig@amd.com wrote:
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
The core move code currently handles use_tt moves, for amdgpu this was being handled also in the driver, but not using the same paths.
If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index db5f761f37ec..d3bd2fd448be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, ttm_bo_move_null(bo, new_mem); return 0; }
if ((old_mem->mem_type == TTM_PL_TT &&
new_mem->mem_type == TTM_PL_SYSTEM) ||
(old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT)) {
/* bind is enough */
if (old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT) { ttm_bo_move_null(bo, new_mem);
I would feel better if we nuke ttm_bo_move_null() and always use ttm_bo_move_ttm().
Any reason? The code path in the current move code pretty much is this.
The current move paths are
if (new_tt && old_tt) if old is system move null else move ttm else call driver move.
So I wanted to maintain that order. calling the move ttm will just make a pointless caching, populate, bind step.
But I'm going to think about it a bit more.
Dave.
On Thu, 24 Sep 2020 at 10:46, Dave Airlie airlied@gmail.com wrote:
On Thu, 24 Sep 2020 at 00:46, Christian König christian.koenig@amd.com wrote:
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
The core move code currently handles use_tt moves, for amdgpu this was being handled also in the driver, but not using the same paths.
If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index db5f761f37ec..d3bd2fd448be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, ttm_bo_move_null(bo, new_mem); return 0; }
if ((old_mem->mem_type == TTM_PL_TT &&
new_mem->mem_type == TTM_PL_SYSTEM) ||
(old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT)) {
/* bind is enough */
if (old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT) { ttm_bo_move_null(bo, new_mem);
I would feel better if we nuke ttm_bo_move_null() and always use ttm_bo_move_ttm().
Any reason? The code path in the current move code pretty much is this.
The current move paths are
if (new_tt && old_tt) if old is system move null else move ttm else call driver move.
So I wanted to maintain that order. calling the move ttm will just make a pointless caching, populate, bind step.
But I'm going to think about it a bit more.
I've looked into moving the code over to move_ttm to see if I could combine things better, but it doesn't really fall out that nicely.
I might try again on top of the refactoring once I'm gone further.
Dave.
Am 24.09.20 um 02:46 schrieb Dave Airlie:
On Thu, 24 Sep 2020 at 00:46, Christian König christian.koenig@amd.com wrote:
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
The core move code currently handles use_tt moves, for amdgpu this was being handled also in the driver, but not using the same paths.
If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index db5f761f37ec..d3bd2fd448be 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, ttm_bo_move_null(bo, new_mem); return 0; }
if ((old_mem->mem_type == TTM_PL_TT &&
new_mem->mem_type == TTM_PL_SYSTEM) ||
(old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT)) {
/* bind is enough */
if (old_mem->mem_type == TTM_PL_SYSTEM &&
new_mem->mem_type == TTM_PL_TT) { ttm_bo_move_null(bo, new_mem);
I would feel better if we nuke ttm_bo_move_null() and always use ttm_bo_move_ttm().
Any reason? The code path in the current move code pretty much is this.
The current move paths are
if (new_tt && old_tt) if old is system move null else move ttm else call driver move.
So I wanted to maintain that order. calling the move ttm will just make a pointless caching, populate, bind step.
Well we want to get rid of the bind/unbind stuff in TTM.
And I'm seriously thinking about getting rid of all the caching stuff.
So all of this should just go away rather soon.
Christian.
But I'm going to think about it a bit more.
Dave.
From: Dave Airlie airlied@redhat.com
The core move code currently handles use_tt moves, for radeon this was being handled also in the driver, but not using the same paths.
If moving between TT/SYSTEM (all the use_tt paths on radeon) use the core move function.
Eventually the core will be flipped over to calling the driver.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/radeon/radeon_ttm.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index ea9ffa6198da..df5cedb2b632 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -316,14 +316,16 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, bool evict, ttm_bo_move_null(bo, new_mem); return 0; } - if ((old_mem->mem_type == TTM_PL_TT && - new_mem->mem_type == TTM_PL_SYSTEM) || - (old_mem->mem_type == TTM_PL_SYSTEM && - new_mem->mem_type == TTM_PL_TT)) { - /* bind is enough */ + if (old_mem->mem_type == TTM_PL_SYSTEM && + new_mem->mem_type == TTM_PL_TT) { ttm_bo_move_null(bo, new_mem); return 0; } + + if (old_mem->mem_type == TTM_PL_TT && + new_mem->mem_type == TTM_PL_SYSTEM) + return ttm_bo_move_ttm(bo, ctx, new_mem); + if (!rdev->ring[radeon_copy_ring_index(rdev)].ready || rdev->asic->copy.copy == NULL) { /* use memcpy */
From: Dave Airlie airlied@redhat.com
The idea is to flip the core over to calling the driver always, so add support for moves here.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/nouveau/nouveau_bo.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 8d51cfca07c8..2c10a84b2cc0 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -1057,6 +1057,18 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict, goto out; }
+ if (old_reg->mem_type == TTM_PL_SYSTEM && + new_reg->mem_type == TTM_PL_TT) { + ttm_bo_move_null(bo, new_reg); + goto out; + } + + if (old_reg->mem_type == TTM_PL_TT && + new_reg->mem_type == TTM_PL_SYSTEM) { + ret = ttm_bo_move_ttm(bo, ctx, new_reg); + goto out; + } + /* Hardware assisted copy. */ if (drm->ttm.move) { if (new_reg->mem_type == TTM_PL_SYSTEM)
From: Dave Airlie airlied@redhat.com
Call the driver move function if it exists, otherwise use the fallback ttm/memcpy paths.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/ttm/ttm_bo.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 993a87443c37..3d9c62cdf38d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -229,6 +229,23 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk) } EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
+static int ttm_bo_move_fallback(struct ttm_buffer_object *bo, + struct ttm_operation_ctx *ctx, + struct ttm_resource *mem) +{ + struct ttm_resource_manager *old_man = ttm_manager_type(bo->bdev, bo->mem.mem_type); + struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev, mem->mem_type); + + if (old_man->use_tt && new_man->use_tt) { + if (bo->mem.mem_type == TTM_PL_SYSTEM) { + ttm_bo_assign_mem(bo, mem); + return 0; + } else + return ttm_bo_move_ttm(bo, ctx, mem); + } else + return ttm_bo_move_memcpy(bo, ctx, mem); +} + static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, struct ttm_resource *mem, bool evict, struct ttm_operation_ctx *ctx) @@ -270,17 +287,10 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (bdev->driver->move_notify) bdev->driver->move_notify(bo, evict, mem);
- if (old_man->use_tt && new_man->use_tt) { - if (bo->mem.mem_type == TTM_PL_SYSTEM) { - ttm_bo_assign_mem(bo, mem); - ret = 0; - } else - ret = ttm_bo_move_ttm(bo, ctx, mem); - } - else if (bdev->driver->move) + if (bdev->driver->move) ret = bdev->driver->move(bo, evict, ctx, mem); else - ret = ttm_bo_move_memcpy(bo, ctx, mem); + ret = ttm_bo_move_fallback(bo, ctx, mem);
if (ret) { if (bdev->driver->move_notify) {
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
Call the driver move function if it exists, otherwise use the fallback ttm/memcpy paths.
I would rather like to see the move callback made mandatory instead.
Christian.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/ttm/ttm_bo.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 993a87443c37..3d9c62cdf38d 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -229,6 +229,23 @@ void ttm_bo_bulk_move_lru_tail(struct ttm_lru_bulk_move *bulk) } EXPORT_SYMBOL(ttm_bo_bulk_move_lru_tail);
+static int ttm_bo_move_fallback(struct ttm_buffer_object *bo,
struct ttm_operation_ctx *ctx,
struct ttm_resource *mem)
+{
- struct ttm_resource_manager *old_man = ttm_manager_type(bo->bdev, bo->mem.mem_type);
- struct ttm_resource_manager *new_man = ttm_manager_type(bo->bdev, mem->mem_type);
- if (old_man->use_tt && new_man->use_tt) {
if (bo->mem.mem_type == TTM_PL_SYSTEM) {
ttm_bo_assign_mem(bo, mem);
return 0;
} else
return ttm_bo_move_ttm(bo, ctx, mem);
- } else
return ttm_bo_move_memcpy(bo, ctx, mem);
+}
- static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, struct ttm_resource *mem, bool evict, struct ttm_operation_ctx *ctx)
@@ -270,17 +287,10 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo, if (bdev->driver->move_notify) bdev->driver->move_notify(bo, evict, mem);
- if (old_man->use_tt && new_man->use_tt) {
if (bo->mem.mem_type == TTM_PL_SYSTEM) {
ttm_bo_assign_mem(bo, mem);
ret = 0;
} else
ret = ttm_bo_move_ttm(bo, ctx, mem);
- }
- else if (bdev->driver->move)
- if (bdev->driver->move) ret = bdev->driver->move(bo, evict, ctx, mem); else
ret = ttm_bo_move_memcpy(bo, ctx, mem);
ret = ttm_bo_move_fallback(bo, ctx, mem);
if (ret) { if (bdev->driver->move_notify) {
On Thu, 24 Sep 2020 at 00:45, Christian König christian.koenig@amd.com wrote:
Am 23.09.20 um 05:04 schrieb Dave Airlie:
From: Dave Airlie airlied@redhat.com
Call the driver move function if it exists, otherwise use the fallback ttm/memcpy paths.
I would rather like to see the move callback made mandatory instead.
Indeed this makes some stuff just easier to deal with.
Dave.
dri-devel@lists.freedesktop.org