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

["Christian König" <christian.koenig@amd.com>]

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;
>   }
> -

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel