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

[Vodapalli, Ravi Kumar]

Hi,

Looks fine to me.

Reviewed-by:Ravi Kumar Vodapalli<ravi.kumar.vodapalli at intel.com>

Thanks,
Ravi Kumar V

On 12/8/2023 11:28 PM, Lucas De Marchi wrote:
> 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
>>