[Intel-xe] [PATCH 1/2] drm/xe: Group normal kunit tests in a single module

[Lucas De Marchi]

On Thu, Dec 07, 2023 at 05:11:10PM +0100, Mauro Carvalho Chehab wrote:
>On Wed, 22 Nov 2023 12:31:46 -0800
>Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>
>> Creating one module for each compilation unit to be tested seems
>> excessive as the number of tests increase. Group them all in a single
>> kunit test module called xe_test.ko.
>>
>> The tests requiring the physical device, aka "live" tests, are still
>> kept in separate modules since they are normally triggered via igt,
>> and not via kunit.py. After igt is converted, those can be merged in
>> a single module as well.
>>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>>  drivers/gpu/drm/xe/tests/Makefile      |  7 ++++++-
>>  drivers/gpu/drm/xe/tests/xe_pci_test.c |  5 -----
>>  drivers/gpu/drm/xe/tests/xe_rtp_test.c |  5 -----
>>  drivers/gpu/drm/xe/tests/xe_test_mod.c | 10 ++++++++++
>>  drivers/gpu/drm/xe/tests/xe_wa_test.c  |  5 -----
>>  5 files changed, 16 insertions(+), 16 deletions(-)
>>  create mode 100644 drivers/gpu/drm/xe/tests/xe_test_mod.c
>>
>> diff --git a/drivers/gpu/drm/xe/tests/Makefile b/drivers/gpu/drm/xe/tests/Makefile
>> index 39d8a0892274..9d1d88af8b2f 100644
>> --- a/drivers/gpu/drm/xe/tests/Makefile
>> +++ b/drivers/gpu/drm/xe/tests/Makefile
>> @@ -1,10 +1,15 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>
>> +# "live" kunit tests
>>  obj-$(CONFIG_DRM_XE_KUNIT_TEST) += \
>>  	xe_bo_test.o \
>>  	xe_dma_buf_test.o \
>>  	xe_migrate_test.o \
>> -	xe_mocs_test.o \
>> +	xe_mocs_test.o
>> +
>> +# Normal kunit tests
>> +obj-$(CONFIG_DRM_XE_KUNIT_TEST) += xe_test.o
>> +xe_test-y = xe_test_mod.o \
>>  	xe_pci_test.o \
>>  	xe_rtp_test.o \
>>  	xe_wa_test.o
>> diff --git a/drivers/gpu/drm/xe/tests/xe_pci_test.c b/drivers/gpu/drm/xe/tests/xe_pci_test.c
>> index daf652517608..59e29a148478 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_pci_test.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_pci_test.c
>> @@ -67,8 +67,3 @@ static struct kunit_suite xe_pci_test_suite = {
>>  };
>>
>>  kunit_test_suite(xe_pci_test_suite);
>> -
>> -MODULE_AUTHOR("Intel Corporation");
>> -MODULE_LICENSE("GPL");
>> -MODULE_DESCRIPTION("xe_pci kunit test");
>> -MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
>> diff --git a/drivers/gpu/drm/xe/tests/xe_rtp_test.c b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
>> index a1d204133cc1..83b0ba097a3e 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_rtp_test.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_rtp_test.c
>> @@ -310,8 +310,3 @@ static struct kunit_suite xe_rtp_test_suite = {
>>  };
>>
>>  kunit_test_suite(xe_rtp_test_suite);
>> -
>> -MODULE_AUTHOR("Intel Corporation");
>> -MODULE_LICENSE("GPL");
>> -MODULE_DESCRIPTION("xe_rtp kunit test");
>> -MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
>> diff --git a/drivers/gpu/drm/xe/tests/xe_test_mod.c b/drivers/gpu/drm/xe/tests/xe_test_mod.c
>> new file mode 100644
>> index 000000000000..875f3e6f965e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/tests/xe_test_mod.c
>> @@ -0,0 +1,10 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +#include <linux/module.h>
>> +
>> +MODULE_AUTHOR("Intel Corporation");
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("xe kunit tests");
>> +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
>> diff --git a/drivers/gpu/drm/xe/tests/xe_wa_test.c b/drivers/gpu/drm/xe/tests/xe_wa_test.c
>> index 01ea974591ea..4eb8e1e8a76c 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_wa_test.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_wa_test.c
>> @@ -135,8 +135,3 @@ static struct kunit_suite xe_rtp_test_suite = {
>>  };
>>
>>  kunit_test_suite(xe_rtp_test_suite);
>> -
>> -MODULE_AUTHOR("Intel Corporation");
>> -MODULE_LICENSE("GPL");
>> -MODULE_DESCRIPTION("xe_wa kunit test");
>> -MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
>
>AFAIKT, if you want to have multiple test groups at the same file,
>you should use, instead, a different macro, pointing to all
>test suites that will belong to the module, e. g.:
>
>kunit_test_suites(
>	&xe_pci_test_suite,
>	...
>	&xe_rtp_test_suite);

having multiple kunit_test_suite should be fine as it will only add the
symbol to a special section, .kunit_test_suites, and when the objects are
linked together there will be one .kunit_test_suites section with all of
them. We'd need to force using  kunit_test_suites() if we needed to
enforce the ordering (although it's still up to the linker if it will be
top to bottom or bottom to top)

I think kunit_test_suites is useful if the suites are on the same
compilation unit, but we don't really need to export the symbols just to
group them in a single place.

>
>Regards,
>Mauro
>