KUnit tests for TTM subsystem

KUnit tests for TTM subsystem

[Karolina Stolarek]

Hi all,

I'm working on KUnit tests for TTM subsystem (nothing RFC-worthy yet), 
with an aim to provide better test coverage for the code used by i915 
and Xe. Before digging deeper, I wanted to check if the priorities 
outlined here make sense and clarify a couple of things.

My plan is to focus on testing the higher layer structs to cover what's 
below, e.g. test ttm_pool functions by testing ttm_device_init() and 
ttm_tt_populate(). I want to split the work into a couple of batches:

1. Basic testing of structs (init/fini and reserve/unreserve), with an 
introduction of fake VRAM resource manager to test ttm_resource_manager 
init. Add some ttm_bo_validate() test stubs.

2. Eviction-specific testing with ttm_bo_validate() to trigger 
ttm_mem_evict_first(), possibly with a separate testing of 
ttm_resource_*_bulk_move() and ttm_bo(un)pin(). Add tests for 
ttm_resource_manager struct, including ttm_sys_man.

3. ttm_tt_(un)populate() testing, adding more coverage to what was 
implemented in (1) and (2).

4. Testing of ttm_bo_vm_ops and mmap/kmap/other features offered by 
ttm_bo_util (not quite clear on how to approach it; suggestions are 
welcome).

Is there something else I should pay attention to here? I can share more 
detailed plan listing specific functions and what tests could cover 
what, if needed.

Also, I have a question on how should I treat drm_gem_object when 
testing ttm_buffer_object. From what I understand, the majority of 
drivers initialize and use the object, but the TTM BO can work without 
it. Should I write the tests against TTM-backed gem objects or use TTM 
BOs with a dummy gem object embedded?

Many thanks,
Karolina

Re: KUnit tests for TTM subsystem

[Christian König]

Hi Karolina,

Am 16.05.23 um 16:02 schrieb Karolina Stolarek:
>
> Hi all,
>
> I'm working on KUnit tests for TTM subsystem (nothing RFC-worthy yet), 
> with an aim to provide better test coverage for the code used by i915 
> and Xe. Before digging deeper, I wanted to check if the priorities 
> outlined here make sense and clarify a couple of things.

oh, yes please finally somebody taking care of this :)

>
> My plan is to focus on testing the higher layer structs to cover 
> what's below, e.g. test ttm_pool functions by testing 
> ttm_device_init() and ttm_tt_populate(). I want to split the work into 
> a couple of batches:
>
> 1. Basic testing of structs (init/fini and reserve/unreserve), with an 
> introduction of fake VRAM resource manager to test 
> ttm_resource_manager init. Add some ttm_bo_validate() test stubs.
>
> 2. Eviction-specific testing with ttm_bo_validate() to trigger 
> ttm_mem_evict_first(), possibly with a separate testing of 
> ttm_resource_*_bulk_move() and ttm_bo(un)pin(). Add tests for 
> ttm_resource_manager struct, including ttm_sys_man.
>
> 3. ttm_tt_(un)populate() testing, adding more coverage to what was 
> implemented in (1) and (2).
>
> 4. Testing of ttm_bo_vm_ops and mmap/kmap/other features offered by 
> ttm_bo_util (not quite clear on how to approach it; suggestions are 
> welcome).
>
> Is there something else I should pay attention to here? I can share 
> more detailed plan listing specific functions and what tests could 
> cover what, if needed.

Sounds like a plan to me. But I suggest to start with the ttm_pool since 
that one is easy to test and initialize without the 
drm_device/drm_gem_object stuff etc... Write a patch for that, get it 
reviewed and upstream and then extend from there.

>
> Also, I have a question on how should I treat drm_gem_object when 
> testing ttm_buffer_object. From what I understand, the majority of 
> drivers initialize and use the object, but the TTM BO can work without 
> it. Should I write the tests against TTM-backed gem objects or use TTM 
> BOs with a dummy gem object embedded?

IIRC VMWGFX was the last one to not use the GEM object, but Zack 
implemented that a whole ago. So the GEM object is now mandatory.

It should be trivial to initialize. Just see the GEM unit tests how to 
come up with a dummy GEM driver and GEM objects. IIRC it was something 
like 10 lines of code for the EXEC unit test I've wrote.

Thanks,
Christian.

>
> Many thanks,
> Karolina

Re: KUnit tests for TTM subsystem

[Karolina Stolarek]

Hi Christian,

On 25.05.2023 10:30, Christian König wrote:
> Hi Karolina,
> 
> Am 16.05.23 um 16:02 schrieb Karolina Stolarek:
>>
>> Hi all,
>>
>> I'm working on KUnit tests for TTM subsystem (nothing RFC-worthy yet), 
>> with an aim to provide better test coverage for the code used by i915 
>> and Xe. Before digging deeper, I wanted to check if the priorities 
>> outlined here make sense and clarify a couple of things.
> 
> oh, yes please finally somebody taking care of this :)

:)

> 
>>
>> My plan is to focus on testing the higher layer structs to cover 
>> what's below, e.g. test ttm_pool functions by testing 
>> ttm_device_init() and ttm_tt_populate(). I want to split the work into 
>> a couple of batches:
>>
>> 1. Basic testing of structs (init/fini and reserve/unreserve), with an 
>> introduction of fake VRAM resource manager to test 
>> ttm_resource_manager init. Add some ttm_bo_validate() test stubs.
>>
>> 2. Eviction-specific testing with ttm_bo_validate() to trigger 
>> ttm_mem_evict_first(), possibly with a separate testing of 
>> ttm_resource_*_bulk_move() and ttm_bo(un)pin(). Add tests for 
>> ttm_resource_manager struct, including ttm_sys_man.
>>
>> 3. ttm_tt_(un)populate() testing, adding more coverage to what was 
>> implemented in (1) and (2).
>>
>> 4. Testing of ttm_bo_vm_ops and mmap/kmap/other features offered by 
>> ttm_bo_util (not quite clear on how to approach it; suggestions are 
>> welcome).
>>
>> Is there something else I should pay attention to here? I can share 
>> more detailed plan listing specific functions and what tests could 
>> cover what, if needed.
> 
> Sounds like a plan to me. But I suggest to start with the ttm_pool since 
> that one is easy to test and initialize without the 
> drm_device/drm_gem_object stuff etc... Write a patch for that, get it 
> reviewed and upstream and then extend from there.

Hmm, I can do it, ttm_pool has little dependency on other modules. IIRC, 
Thomas suggested that I should try to cover ttm_pool functions with 
tests from the higher level, but I don't mind writing independent tests.

As for drm_device init, I'm using functions from drm_kunit_helper.c and 
they work fine. I was able to write simple tests for ttm_device_init() 
with a dummy device.

If you don't mind, I can write some tests for ttm_pool, add some for 
ttm_device, and send an RFC. It's going to be quite a small patch 
series, but that fine, it'll make the review less painful :)

> 
>>
>> Also, I have a question on how should I treat drm_gem_object when 
>> testing ttm_buffer_object. From what I understand, the majority of 
>> drivers initialize and use the object, but the TTM BO can work without 
>> it. Should I write the tests against TTM-backed gem objects or use TTM 
>> BOs with a dummy gem object embedded?
> 
> IIRC VMWGFX was the last one to not use the GEM object, but Zack 
> implemented that a whole ago. So the GEM object is now mandatory.

OK, that makes sense, thanks. I'll go with GEM objects then.

> It should be trivial to initialize. Just see the GEM unit tests how to 
> come up with a dummy GEM driver and GEM objects. IIRC it was something 
> like 10 lines of code for the EXEC unit test I've wrote.

Right, thanks for the pointers, I'll check this out.

All the best,
Karolina

> 
> Thanks,
> Christian.
> 
>>
>> Many thanks,
>> Karolina
>