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

[Matthew Auld]

On 10/02/2023 11:03, Christian König wrote:
> 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 at intel.com>
>> Cc: Christian König <christian.koenig at 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 at amd.com>.

Should we go ahead and land this? (minus patch 3 since that is already 
fixed by vmware folks).

> 
> 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) ||
>