[PATCH v12 06/10] drm/ttm/tests: Add tests with mock resource managers

[Karolina Stolarek]

On 3.06.2024 11:30, Thomas Hellström wrote:
> On Mon, 2024-06-03 at 10:28 +0200, Karolina Stolarek wrote:
>>>>>> diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> index 2f590bae53f8..2a2585b37118 100644
>>>>>> --- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> +++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
>>>>>> @@ -27,8 +27,42 @@ static int mock_move(struct
>>>>>> ttm_buffer_object
>>>>>> *bo,
>>>>>> bool evict,
>>>>>>     		     struct ttm_resource *new_mem,
>>>>>>     		     struct ttm_place *hop)
>>>>>>     {
>> (...)
>>>>>> +
>>>>>> +		if (ret)
>>>>>> +			return ret;
>>>>>> +
>>>>>> +		ttm_resource_free(bo, &bo->resource);
>>>>>> +		ttm_bo_assign_mem(bo, new_mem);
>>>>>> +		return 0;
>>>>>> +	}
>>>>>> +
>>>>>> +	return ttm_bo_move_memcpy(bo, ctx, new_mem);
>>>>>
>>>>> Do we hit this move_memcpy()? Since the mock manager doesn't
>>>>> actually
>>>>> reserve any memory to manager, I'd expect this to run into
>>>>> problems?
>>>>
>>>> We do. The mock manager has use_tt=true, so on move, we'd use
>>>> ttm_kmap_iter_tt_init() for src and dest and copy the pages. I'm
>>>> not
>>>> sure if that's the right approach, but it enables me to test if
>>>> ttm_operation_ctx's bytes_moved is correctly updated.
>>>
>>> Ah, ok. It's probably not a common use-case since with both src and
>>> dst
>>> having use_tt, IIRC ttm should keep the pages and their content
>>> mostly
>>> intact across a move. So you would memcpy the source on itself?
>>>
>>> But it would give some coverage of the copy code though.
>>
>> I dug around and it looks like, in the current scenario,
>> ttm_bo_move_memcpy() is just ttm_bo_move_sync_cleanup()
>> (ttm_resource_free + ttm_bo_assign_mem). That means I should revisit
>> the
>> definitions of move and mock manager... I'll try to simplify them.
>>
>> Do I understand correctly that we'd prefer to have a mock manager
>> with
>> user_tt=false?
> 
> Yes, but then you need to allocate a chunk of contigous memory for the
> mock manager to manage. And instead of using drm_buddy you'd have to
> use drm_mm to manage it, since the ttm_kmap_iter default iterators can
> only handle either
> a) Contigous memory regions as returned from the drm_mm manager.
> b) Fragmented memory regions as returned from the drm_buddy manager,
> but in that case, they currently only handle pci io memory.
> 
> So I'd suggest to go with the current code and mark as a TODO: to
> implement a) above.

I understand, thank you for your explanation, that would require some 
rewrite indeed. I'll follow your suggestion and include it in the TODO file.

All the best,
Karolina