Re: [Intel-gfx] [PATCH 1/4] drm/gem-vram: handle NULL bo->resource in move callback

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

Am 08.02.23 um 15:53 schrieb Matthew Auld:
> The ttm BO now initially has NULL bo->resource, and leaves the driver
> the handle that. However it looks like we forgot to handle that for
> ttm_bo_move_memcpy() users, like with vram-gem, since it just silently
> returns zero. This seems to then trigger warnings like:
>
> WARNING: CPU: 0 PID: 1 at drivers/gpu/drm/drm_gem_vram_helper.c:255 drm_gem_vram_offset (??:?)
>
> Fix this by calling move_null() if the new resource is TTM_PL_SYSTEM,
> otherwise do the multi-hop sequence to ensure can safely call into
> ttm_bo_move_memcpy(), since it might also need to clear the memory.
> This should give the same behaviour as before.
>
> While we are here let's also treat calling ttm_bo_move_memcpy() with
> NULL bo->resource as programmer error, where expectation is that upper
> layers should now handle it.
>
> Fixes: 180253782038 ("drm/ttm: stop allocating dummy resources during BO creation")
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Christian König <christian.koenig@amd.com>

Oh, I wasn't aware that this broke at so many places. Especially radeon 
was tested earlier in the development of the patch set.

Thanks for looking into that, the radeon patch has my rb and the rest of 
the series is Acked-by: Christian König <christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/drm_gem_vram_helper.c | 11 +++++++++++
>   drivers/gpu/drm/ttm/ttm_bo_util.c     |  4 ++--
>   2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index d40b3edb52d0..0bea3df2a16d 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -916,6 +916,17 @@ static int bo_driver_move(struct ttm_buffer_object *bo,
>   {
>   	struct drm_gem_vram_object *gbo;
>   
> +	if (!bo->resource) {
> +		if (new_mem->mem_type != TTM_PL_SYSTEM) {
> +			hop->mem_type = TTM_PL_SYSTEM;
> +			hop->flags = TTM_PL_FLAG_TEMPORARY;
> +			return -EMULTIHOP;
> +		}
> +
> +		ttm_bo_move_null(bo, new_mem);
> +		return 0;
> +	}
> +
>   	gbo = drm_gem_vram_of_bo(bo);
>   
>   	return drm_gem_vram_bo_driver_move(gbo, evict, ctx, new_mem);
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index d9d2b0903b22..fd9fd3d15101 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -157,8 +157,8 @@ int ttm_bo_move_memcpy(struct ttm_buffer_object *bo,
>   	bool clear;
>   	int ret = 0;
>   
> -	if (!src_mem)
> -		return 0;
> +	if (WARN_ON(!src_mem))
> +		return -EINVAL;
>   
>   	src_man = ttm_manager_type(bdev, src_mem->mem_type);
>   	if (ttm && ((ttm->page_flags & TTM_TT_FLAG_SWAPPED) ||