Re: [PATCH] [RFC/HACK] drm/ttm: avoid multihop moves in drivers.

Yes, please! That approach looks even better than what I had in mind.

A few comments below:

Am 20.10.20 um 06:16 schrieb Dave Airlie:

From: Dave Airlie airlied@redhat.com

[SNIP]

• /* don't go from system->vram in one hop,

•   report this back to the caller tell it
  

•   where to bounce this buffer through. */
  

• if ((old_mem->mem_type == TTM_PL_SYSTEM &&

•     new_mem->mem_type == TTM_PL_VRAM) ||
  

•    (old_mem->mem_type == TTM_PL_VRAM &&
  

•     new_mem->mem_type == TTM_PL_SYSTEM)) {
  

• new_mem->mem_type = TTM_PL_TT;
  

Not sure if that is such a good idea, new_mem can be some allocated memory in the VRAM domain at this moment.

Maybe instead give the callback a separate bounce argument instead.

• new_mem->placement = 0;
  

• return -EMULTIHOP;
  

Using EMULTIHOP here is a really nice idea.

[SNIP]

+static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo,

• 		     struct ttm_resource *mem,
  

• 		     struct ttm_operation_ctx *ctx)
  

+{

• struct ttm_place placement_memtype = {

• .fpfn = 0,
  

• .lpfn = 0,
  

• .mem_type = mem->mem_type,
  

• .flags = mem->placement,
  

• };
• struct ttm_placement bounce_placement;
• int ret;
• bounce_placement.num_placement = bounce_placement.num_busy_placement = 1;
• bounce_placement.placement = bounce_placement.busy_placement = &placement_memtype;
• /* find space in the bounce domain */
• ret = ttm_bo_mem_space(bo, &bounce_placement, mem, ctx);
• if (ret)

• return ret;
  

• /* move to the bounce domain */
• ret = ttm_bo_handle_move_mem(bo, mem, false, ctx);

Is this a recursion? I don't think it is, but I thought I better double check.

Regards, Christian.

• if (ret)

• return ret;
  

• return 0;

+}

• static int ttm_bo_move_buffer(struct ttm_buffer_object *bo, struct ttm_placement *placement, struct ttm_operation_ctx *ctx)

@@ -954,11 +984,18 @@ static int ttm_bo_move_buffer(struct ttm_buffer_object *bo, /* * Determine where to move the buffer. */ +bounce: ret = ttm_bo_mem_space(bo, placement, &mem, ctx); if (ret)

• goto out_unlock;
  

• return ret;
  

  ret = ttm_bo_handle_move_mem(bo, &mem, false, ctx);

-out_unlock:

• if (ret == -EMULTIHOP) {

• ret = ttm_bo_bounce_temp_buffer(bo, &mem, ctx);
  

• if (ret)
  

• 	return ret;
  

• /* try and move to final place now. */
  

• goto bounce;
  

• } if (ret) ttm_resource_free(bo, &mem); return ret;

@@ -1478,4 +1515,3 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object *bo) ttm_tt_destroy(bo->bdev, bo->ttm); bo->ttm = NULL; }