All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.