[PATCH 2/6] drm/xe/kunit: Introduce xe_pci_fake_data_desc()

[Michal Wajdeczko]

On 8/18/2025 6:18 PM, Lucas De Marchi wrote:
> On Fri, Aug 01, 2025 at 08:13:46PM +0200, Michal Wajdeczko wrote:
>> We already use struct xe_pci_fake_data to provide custom config of
>> the fake PCI device and soon we will be using thios struct also as
> 
>                         ^^^ typo
> 
>> direct parameter for the parameterized Xe KUnit tests.
>>
>> Add function to generate description based on that config data.
>>
>> For platform or subplatform name lookup pciidlist which already
>> have definitions of all supported platforms.
>>
>> Examples:
>>
>>  TIGERLAKE
>>  TIGERLAKE A0
>>  TIGERLAKE SR-IOV PF
>>  ...
>>  PANTHERLAKE
>>  PANTHERLAKE SR-IOV PF
>>  PANTHERLAKE 3000 3000
>>  PANTHERLAKE 3000 A0 3000 A0
>>  PANTHERLAKE 3000 A0 3000 A0 SR-IOV VF
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> ---
>> drivers/gpu/drm/xe/tests/xe_pci.c      | 101 +++++++++++++++++++++++++
>> drivers/gpu/drm/xe/tests/xe_pci_test.h |   1 +
>> 2 files changed, 102 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>> index 81bc86647499..5c6403785984 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>> @@ -12,6 +12,107 @@
>> #include <kunit/test-bug.h>
>> #include <kunit/visibility.h>
>>
>> +static const struct xe_device_desc *lookup_desc(enum xe_platform p)
>> +{
>> +    const struct pci_device_id *ids = pciidlist;
>> +    const struct xe_device_desc *desc;
>> +
>> +    for (; ids->vendor || ids->subvendor || ids->class_mask; ids++) {
> 
> why ids->class_mask? for xe we should never match with vendor set.
> 
>> +        desc = (const void *)ids->driver_data;
>> +        if (desc->platform == p)
>> +            return desc;
>> +    }
>> +    return NULL;
>> +}
>> +
>> +static const struct xe_subplatform_desc *lookup_sub_desc(enum xe_platform p, enum xe_subplatform s)
>> +{
>> +    const struct xe_device_desc *desc = lookup_desc(p);
>> +    const struct xe_subplatform_desc *spd;
>> +
>> +    if (desc && desc->subplatforms)
>> +        for (spd = desc->subplatforms; spd->subplatform; spd++)
>> +            if (spd->subplatform == s)
>> +                return spd;
>> +    return NULL;
>> +}
>> +
>> +static const char *lookup_platform_name(enum xe_platform p)
>> +{
>> +    const struct xe_device_desc *desc = lookup_desc(p);
>> +
>> +    return desc ? desc->platform_name : "INVALID";
>> +}
>> +
>> +static const char *__lookup_subplatform_name(enum xe_platform p, enum xe_subplatform s)
>> +{
>> +    const struct xe_subplatform_desc *desc = lookup_sub_desc(p, s);
>> +
>> +    return desc ? desc->name : "INVALID";
>> +}
>> +
>> +static const char *lookup_subplatform_name(enum xe_platform p, enum xe_subplatform s)
>> +{
>> +    return s == XE_SUBPLATFORM_NONE ? "" : __lookup_subplatform_name(p, s);
>> +}
>> +
>> +static const char *subplatform_prefix(enum xe_subplatform s)
>> +{
>> +    return s == XE_SUBPLATFORM_NONE ? "" : " ";
>> +}
>> +
>> +static const char *step_prefix(enum xe_step step)
>> +{
>> +    return step == STEP_NONE ? "" : " ";
>> +}
>> +
>> +static const char *step_name(enum xe_step step)
>> +{
>> +    return step == STEP_NONE ? "" : xe_step_name(step);
>> +}
>> +
>> +static const char *sriov_prefix(enum xe_sriov_mode mode)
>> +{
>> +    return mode <= XE_SRIOV_MODE_NONE ? "" : " ";
>> +}
>> +
>> +static const char *sriov_name(enum xe_sriov_mode mode)
>> +{
>> +    return mode <= XE_SRIOV_MODE_NONE ? "" : xe_sriov_mode_to_string(mode);
>> +}
>> +
>> +/**
>> + * xe_pci_fake_data_desc - Describe struct xe_pci_fake_data parameter
>> + * @param: the &struct xe_pci_fake_data parameter to describe
>> + * @desc: output buffer with minimum size of KUNIT_PARAM_DESC_SIZE
>> + *
>> + * This function prepares description of the struct xe_pci_fake_data parameter.
>> + *
>> + * It is tailored for use in parameterized KUnit tests where parameter generator
>> + * is based on the struct xe_pci_fake_data arrays.
>> + */
>> +void xe_pci_fake_data_desc(const struct xe_pci_fake_data *param, char *desc)
>> +{
>> +    if (param->graphics_verx100 || param->media_verx100)
>> +        snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s%s%s %u%s%s %u%s%s%s%s",
>> +             lookup_platform_name(param->platform),
>> +             subplatform_prefix(param->subplatform),
>> +             lookup_subplatform_name(param->platform, param->subplatform),
>> +             param->graphics_verx100,
>> +             step_prefix(param->step.graphics), step_name(param->step.graphics),
>> +             param->media_verx100,
>> +             step_prefix(param->step.media), step_name(param->step.media),
>> +             sriov_prefix(param->sriov_mode), sriov_name(param->sriov_mode));
>> +    else
>> +        snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s%s%s%s%s%s%s",
>> +             lookup_platform_name(param->platform),
>> +             subplatform_prefix(param->subplatform),
>> +             lookup_subplatform_name(param->platform, param->subplatform),
>> +             step_prefix(param->step.graphics), step_name(param->step.graphics),
>> +             sriov_prefix(param->sriov_mode), sriov_name(param->sriov_mode));
>> +}
>> +EXPORT_SYMBOL_IF_KUNIT(xe_pci_fake_data_desc);
> 
> our dep here to be in xe.ko seems to be pciidlist. Would it make sense
> to simply export that instead and then move all this into

while we could export pciidlist this will not allow us to drop completely
tests/xe_pci.c file as we have other param generators there that require
access to full array definitions (like graphics_ips)

> drivers/gpu/drm/xe/tests/xe_pci_test.c? There some potential name

but tests/xe_pci_test.c should contain just test code, no reusable
helpers that will export

or did you mean tests/xe_kunit_helpers.c? but that's xe.ko too

> clashes. tests/xe_pci.c is more to export what's needed for the tests
> than to add helper functions like these.

but this _are_ helpers for other tests, not just the current xe_pci ones
in tests/xe_pci_test.c

> 
> Lucas De Marchi
> 
>> +
>> static void xe_ip_kunit_desc(const struct xe_ip *param, char *desc)
>> {
>>     snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%u.%02u %s",
>> diff --git a/drivers/gpu/drm/xe/tests/xe_pci_test.h b/drivers/gpu/drm/xe/tests/xe_pci_test.h
>> index 32452bd65bff..1195839999ab 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_pci_test.h
>> +++ b/drivers/gpu/drm/xe/tests/xe_pci_test.h

btw, this file has the confusing name ...

maybe we should rename:

	tests/xe_pci.c      to xe_pci_kunit_helpers.c
	tests/xe_pci_test.h to xe_pci_kunit_helpers.h

to make it clear what they are for and avoid clash with original xe_pci.c
and xe_pci_test.c ?


>> @@ -24,6 +24,7 @@ struct xe_pci_fake_data {
>> };
>>
>> int xe_pci_fake_device_init(struct xe_device *xe);
>> +void xe_pci_fake_data_desc(const struct xe_pci_fake_data *param, char *desc);
>>
>> const void *xe_pci_graphics_ip_gen_param(const void *prev, char *desc);
>> const void *xe_pci_media_ip_gen_param(const void *prev, char *desc);
>> -- 
>> 2.47.1
>>