Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Wajdeczko <michal.wajdeczko@intel.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v2 7/7] drm/xe/kunit: Allow to run tests against custom platform
Date: Thu, 4 Sep 2025 00:14:45 +0200	[thread overview]
Message-ID: <23b32a25-e157-435e-8b23-ccacb59be55f@intel.com> (raw)
In-Reply-To: <gwaifbohn7ocyslucdwns5mlqywlzte74x6ngop3cqilxc6oo5@capvsl2qcxz2>



On 9/2/2025 9:34 PM, Lucas De Marchi wrote:
> On Fri, Aug 29, 2025 at 07:19:22PM +0200, Michal Wajdeczko wrote:
>> We might want to run our KUnit tests against not defined yet
>> future platform that differs from the existing ones only by
>> new GDMID versions being used, but otherwise it is similar
>> to existing graphics and media IP.
>>
>> Add kunit-only "dut" modparam where we can specify GMDIDs to
>> be used by param generators:
>>
>>    $ tools/testing/kunit/kunit.py run ... \
>>        --kernel_args=xe.dut=LUNARLAKE:2025:2025 \
> 
> I don't think we should polute the module params for the xe module.

but this extra modparam would be only for CONFIG_DRM_XE_KUNIT_TEST
so the driver is already not in a production quality, do we still care?

> I'd rather have this in xe_test_mod.c and then it may call something
> from xe to set the behavior we want.

I'm afraid this could introduce cross-dependencies, maybe we should
move some code to xe_kunit_helpers.ko ?

until that time, maybe I just push patches 1-6 and try to experiment
with this last one patch separately if this extra modparm bothers you?

> 
> Another option is just not to be a module param and simply inspect the
> kernel command line. It should be fine if this is done from a test
> module rather than the real one.
> 
> Lucas De Marchi
> 
>>        "*xe_[wp][ac]*.*_??"
>>    ...
>>    KTAP version 1
>>    1..2
>>        KTAP version 1
>>        # Subtest: xe_pci
>>        # module: xe_test
>>        1..2
>>            KTAP version 1
>>            # Subtest: check_graphics_ip
>>            ok 1 20.25 Xe2_LPG
>>        ok 1 check_graphics_ip
>>            KTAP version 1
>>            # Subtest: check_media_ip
>>            ok 1 20.25 Xe2_LPM
>>        ok 2 check_media_ip
>>    # xe_pci: pass:2 fail:0 skip:0 total:2
>>    # Totals: pass:2 fail:0 skip:0 total:2
>>    ok 1 xe_pci
>>        KTAP version 1
>>        # Subtest: xe_wa
>>        # module: xe_test
>>        1..1
>>            KTAP version 1
>>            # Subtest: xe_wa_gt
>>            ok 1 LUNARLAKE 20.25(Xe2_LPG) A0 20.25(Xe2_LPM) A0
>>        ok 1 xe_wa_gt
>>    ok 2 xe_wa
>>
>> Suggested-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>> drivers/gpu/drm/xe/tests/xe_pci.c | 125 +++++++++++++++++++++++++++++-
>> 1 file changed, 123 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>> index f19b1a64128b..0ab3f71b0fb7 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_pci.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>> @@ -12,6 +12,91 @@
>> #include <kunit/test-bug.h>
>> #include <kunit/visibility.h>
>>
>> +/* defined as arrays to allow reuse param_gens macros */
>> +static struct xe_pci_fake_data dut_pci_data[1];
>> +static struct xe_ip dut_graphics_ip[1];
>> +static struct xe_ip dut_media_ip[1];
>> +
>> +static bool has_custom_dut(void)
>> +{
>> +    return dut_pci_data[0].graphics_verx100;
>> +}
>> +
>> +static enum xe_platform find_platform_by_name(const char *name)
>> +{
>> +    const struct xe_device_desc *desc;
>> +    const struct pci_device_id *ids;
>> +
>> +    for (ids = pciidlist; ids->driver_data; ids++) {
>> +        desc = (const void *)ids->driver_data;
>> +        if (!strcmp(name, desc->platform_name))
>> +            return desc->platform;
>> +    }
>> +    return XE_PLATFORM_UNINITIALIZED;
>> +}
>> +
>> +static const struct xe_ip *find_similar_graphics_ip(unsigned int verx100)
>> +{
>> +    size_t i;
>> +
>> +    for (i = 1; i < ARRAY_SIZE(graphics_ips); i++)
>> +        if (graphics_ips[i].verx100 > verx100)
>> +            break;
>> +    return &graphics_ips[i - 1];
>> +}
>> +
>> +static const struct xe_ip *find_similar_media_ip(unsigned int verx100)
>> +{
>> +    size_t i;
>> +
>> +    for (i = 1; i < ARRAY_SIZE(media_ips); i++)
>> +        if (media_ips[i].verx100 > verx100)
>> +            break;
>> +    return &media_ips[i - 1];
>> +}
>> +
>> +static int dut_param_set(const char *val, const struct kernel_param *kp)
>> +{
>> +    char buf[32], *s = buf, *tok;
>> +    const struct xe_ip *origin;
>> +
>> +    strscpy(buf, val);
>> +    tok = strsep(&s, ":");
>> +    dut_pci_data[0].platform = find_platform_by_name(tok);
>> +    if (dut_pci_data[0].platform < XE_METEORLAKE)
>> +        return -EINVAL;
>> +    tok = strsep(&s, ":");
>> +    if (kstrtouint(tok, 10, &dut_pci_data[0].graphics_verx100))
>> +        return -EINVAL;
>> +    tok = strsep(&s, ":");
>> +    if (kstrtouint(tok, 10, &dut_pci_data[0].media_verx100))
>> +        return -EINVAL;
>> +
>> +    dut_pci_data[0].subplatform = XE_SUBPLATFORM_NONE;
>> +    dut_pci_data[0].step.graphics = STEP_A0;
>> +    dut_pci_data[0].step.media = STEP_A0;
>> +
>> +    origin = find_similar_graphics_ip(dut_pci_data[0].graphics_verx100);
>> +    dut_graphics_ip[0].verx100 = dut_pci_data[0].graphics_verx100;
>> +    dut_graphics_ip[0].name = origin->name;
>> +    dut_graphics_ip[0].desc = origin->desc;
>> +
>> +    origin = find_similar_media_ip(dut_pci_data[0].media_verx100);
>> +    dut_media_ip[0].verx100 = dut_pci_data[0].media_verx100;
>> +    dut_media_ip[0].name = origin->name;
>> +    dut_media_ip[0].desc = origin->desc;
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct kernel_param_ops dut_param_ops = {
>> +    .set = dut_param_set,
>> +};
>> +
>> +module_param_cb(dut, &dut_param_ops, NULL, 0400);
>> +MODULE_PARM_DESC(dut, "Specify fake GMDID platform to be used during tests:\n"
>> +         "dut=<PLATFORM_NAME>:<GRAPHICS_VERx100>:<MEDIA_VERx100>\n");
>> +
>> #define PLATFORM_CASE(platform__, graphics_step__)                    \
>>     {                                        \
>>         .platform = XE_ ## platform__,                        \
>> @@ -64,6 +149,13 @@ static const struct xe_pci_fake_data cases[] = {
>>
>> KUNIT_ARRAY_PARAM(platform, cases, xe_pci_fake_data_desc);
>>
>> +static void xe_pci_fake_data_desc_nc(struct xe_pci_fake_data *param, char *desc)
>> +{
>> +    xe_pci_fake_data_desc(param, desc);
>> +}
>> +
>> +KUNIT_ARRAY_PARAM(dut_platform, dut_pci_data, xe_pci_fake_data_desc_nc);
>> +
>> /**
>>  * xe_pci_fake_data_gen_params - Generate struct xe_pci_fake_data parameters
>>  * @prev: the pointer to the previous parameter to iterate from or NULL
>> @@ -77,6 +169,8 @@ KUNIT_ARRAY_PARAM(platform, cases, xe_pci_fake_data_desc);
>>  */
>> const void *xe_pci_fake_data_gen_params(const void *prev, char *desc)
>> {
>> +    if (has_custom_dut())
>> +        return dut_platform_gen_params(prev, desc);
>>     return platform_gen_params(prev, desc);
>> }
>> EXPORT_SYMBOL_IF_KUNIT(xe_pci_fake_data_gen_params);
>> @@ -152,14 +246,14 @@ static const char *sriov_name(enum xe_sriov_mode mode)
>>
>> static const char *lookup_graphics_name(unsigned int verx100)
>> {
>> -    const struct xe_ip *ip = find_graphics_ip(verx100);
>> +    const struct xe_ip *ip = find_similar_graphics_ip(verx100);
>>
>>     return ip ? ip->name : "";
>> }
>>
>> static const char *lookup_media_name(unsigned int verx100)
>> {
>> -    const struct xe_ip *ip = find_media_ip(verx100);
>> +    const struct xe_ip *ip = find_similar_media_ip(verx100);
>>
>>     return ip ? ip->name : "";
>> }
>> @@ -207,6 +301,14 @@ static void xe_ip_kunit_desc(const struct xe_ip *param, char *desc)
>> KUNIT_ARRAY_PARAM(graphics_ip, graphics_ips, xe_ip_kunit_desc);
>> KUNIT_ARRAY_PARAM(media_ip, media_ips, xe_ip_kunit_desc);
>>
>> +static void xe_ip_kunit_desc_nc(struct xe_ip *param, char *desc)
>> +{
>> +    xe_ip_kunit_desc(param, desc);
>> +}
>> +
>> +KUNIT_ARRAY_PARAM(dut_graphics_ip, dut_graphics_ip, xe_ip_kunit_desc_nc);
>> +KUNIT_ARRAY_PARAM(dut_media_ip, dut_media_ip, xe_ip_kunit_desc_nc);
>> +
>> static void xe_pci_id_kunit_desc(const struct pci_device_id *param, char *desc)
>> {
>>     const struct xe_device_desc *dev_desc =
>> @@ -232,6 +334,8 @@ KUNIT_ARRAY_PARAM(pci_id, pciidlist, xe_pci_id_kunit_desc);
>>  */
>> const void *xe_pci_graphics_ip_gen_param(const void *prev, char *desc)
>> {
>> +    if (has_custom_dut())
>> +        return dut_graphics_ip_gen_params(prev, desc);
>>     return graphics_ip_gen_params(prev, desc);
>> }
>> EXPORT_SYMBOL_IF_KUNIT(xe_pci_graphics_ip_gen_param);
>> @@ -249,6 +353,8 @@ EXPORT_SYMBOL_IF_KUNIT(xe_pci_graphics_ip_gen_param);
>>  */
>> const void *xe_pci_media_ip_gen_param(const void *prev, char *desc)
>> {
>> +    if (has_custom_dut())
>> +        return dut_media_ip_gen_params(prev, desc);
>>     return media_ip_gen_params(prev, desc);
>> }
>> EXPORT_SYMBOL_IF_KUNIT(xe_pci_media_ip_gen_param);
>> @@ -292,6 +398,16 @@ static void fake_xe_info_probe_tile_count(struct xe_device *xe)
>>     /* Nothing to do, just use the statically defined value. */
>> }
>>
>> +static const struct xe_ip *find_dut_graphics_ip(unsigned int verx100)
>> +{
>> +    return dut_graphics_ip[0].verx100 == verx100 ? dut_graphics_ip : NULL;
>> +}
>> +
>> +static const struct xe_ip *find_dut_media_ip(unsigned int verx100)
>> +{
>> +    return dut_media_ip[0].verx100 == verx100 ? dut_media_ip : NULL;
>> +}
>> +
>> int xe_pci_fake_device_init(struct xe_device *xe)
>> {
>>     struct kunit *test = kunit_get_current_test();
>> @@ -332,6 +448,11 @@ int xe_pci_fake_device_init(struct xe_device *xe)
>>     kunit_activate_static_stub(test, xe_info_probe_tile_count,
>>                    fake_xe_info_probe_tile_count);
>>
>> +    if (has_custom_dut()) {
>> +        kunit_activate_static_stub(test, find_graphics_ip, find_dut_graphics_ip);
>> +        kunit_activate_static_stub(test, find_media_ip, find_dut_media_ip);
>> +    }
>> +
>>     xe_info_init_early(xe, desc, subplatform_desc);
>>     xe_info_init(xe, desc);
>>
>> -- 
>> 2.47.1
>>


  reply	other threads:[~2025-09-03 22:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-29 17:19 [PATCH v2 0/7] Misc xe/kunit improvements Michal Wajdeczko
2025-08-29 17:19 ` [PATCH v2 1/7] drm/xe: Allow to stub lookup for graphics and media IP Michal Wajdeczko
2025-09-02 18:13   ` Lucas De Marchi
2025-08-29 17:19 ` [PATCH v2 2/7] drm/xe/kunit: Update struct xe_pci_fake_data step declarations Michal Wajdeczko
2025-08-29 17:19 ` [PATCH v2 3/7] drm/xe/kunit: Introduce xe_pci_fake_data_desc() Michal Wajdeczko
2025-09-02 18:16   ` Lucas De Marchi
2025-08-29 17:19 ` [PATCH v2 4/7] drm/xe/kunit: Drop custom struct platform_test_case Michal Wajdeczko
2025-08-29 17:19 ` [PATCH v2 5/7] drm/xe/kunit: Promote fake platform parameter list Michal Wajdeczko
2025-08-29 17:19 ` [PATCH v2 6/7] drm/xe/kunit: Drop xe_wa_test_exit Michal Wajdeczko
2025-09-02 18:19   ` Lucas De Marchi
2025-08-29 17:19 ` [PATCH v2 7/7] drm/xe/kunit: Allow to run tests against custom platform Michal Wajdeczko
2025-09-02 19:34   ` Lucas De Marchi
2025-09-03 22:14     ` Michal Wajdeczko [this message]
2025-09-05 12:30       ` Lucas De Marchi
2025-08-29 18:57 ` ✗ CI.checkpatch: warning for Misc xe/kunit improvements (rev2) Patchwork
2025-08-29 18:58 ` ✓ CI.KUnit: success " Patchwork
2025-08-29 19:34 ` ✓ Xe.CI.BAT: " Patchwork
2025-08-30  7:26 ` ✗ Xe.CI.Full: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23b32a25-e157-435e-8b23-ccacb59be55f@intel.com \
    --to=michal.wajdeczko@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=lucas.demarchi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox