Re: [PATCH] drm/xe/tests: Add legacy IP descriptors to param generators

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 01 Jul 2025, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Fri, Jun 27, 2025 at 08:11:10PM +0200, Michal Wajdeczko wrote:
>>Recently introduced kunit parameter generators where based on
>>the existing arrays which have only GDMID-bsaed IP and didn't
>>take into account legacy IP definitions from pre-GMDID era.
>>
>>Add test only arrays with legacy IPs (as those will not change)
>>and extend param generators to start iterating over them.
>>
>> [ ] =================== xe_pci (2 subtests) ====================
>> [ ] ==================== check_graphics_ip  ====================
>> [ ] [PASSED] 12.00 Xe_LP
>> [ ] [PASSED] 12.10 Xe_LP+
>> [ ] [PASSED] 12.55 Xe_HPG
>> [ ] [PASSED] 12.60 Xe_HPC
>> [ ] [PASSED] 12.70 Xe_LPG
>> [ ] [PASSED] 12.71 Xe_LPG
>> [ ] [PASSED] 12.74 Xe_LPG+
>> [ ] [PASSED] 20.01 Xe2_HPG
>> [ ] [PASSED] 20.02 Xe2_HPG
>> [ ] [PASSED] 20.04 Xe2_LPG
>> [ ] [PASSED] 30.00 Xe3_LPG
>> [ ] [PASSED] 30.01 Xe3_LPG
>> [ ] [PASSED] 30.03 Xe3_LPG
>> [ ] ================ [PASSED] check_graphics_ip ================
>> [ ] ===================== check_media_ip  ======================
>> [ ] [PASSED] 12.00 Xe_M
>> [ ] [PASSED] 12.55 Xe_HPM
>> [ ] [PASSED] 13.00 Xe_LPM+
>> [ ] [PASSED] 13.01 Xe2_HPM
>> [ ] [PASSED] 20.00 Xe2_LPM
>> [ ] [PASSED] 30.00 Xe3_LPM
>> [ ] [PASSED] 30.02 Xe3_LPM
>> [ ] ================= [PASSED] check_media_ip ==================
>> [ ] ===================== [PASSED] xe_pci ======================
>>
>>Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>>Cc: Matt Roper <matthew.d.roper@intel.com>
>>---
>> drivers/gpu/drm/xe/tests/xe_pci.c | 33 +++++++++++++++++++++++++++++++
>> 1 file changed, 33 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c
>>index baccb657bd05..c5acee52b458 100644
>>--- a/drivers/gpu/drm/xe/tests/xe_pci.c
>>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c
>>@@ -12,12 +12,31 @@
>> #include <kunit/test-bug.h>
>> #include <kunit/visibility.h>
>>
>>+#define IS_ARRAY_ROW(arr, row) ((typeof(&(arr)[0]))(row) - (arr) < ARRAY_SIZE(arr))
>>+
>> static void xe_ip_kunit_desc(const struct xe_ip *param, char *desc)
>> {
>> 	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%u.%02u %s",
>> 		 param->verx100 / 100, param->verx100 % 100, param->name);
>> }
>>
>>+/* Pre-GMDID Graphics IPs */
>>+static const struct xe_ip legacy_graphics_ips[] = {
>>+	graphics_ip_xelp,
>>+	graphics_ip_xelpp,
>>+	graphics_ip_xehpg,
>>+	graphics_ip_xehpc,
>>+};
>>+
>>+/* Pre-GMDID Media IPs */
>>+static const struct xe_ip legacy_media_ips[] = {
>>+	media_ip_xem,
>>+	media_ip_xehpm,
>>+};
>>+
>>+KUNIT_ARRAY_PARAM(legacy_graphics_ip, legacy_graphics_ips, xe_ip_kunit_desc);
>>+KUNIT_ARRAY_PARAM(legacy_media_ip, legacy_media_ips, xe_ip_kunit_desc);
>>+
>> KUNIT_ARRAY_PARAM(graphics_ip, graphics_ips, xe_ip_kunit_desc);
>> KUNIT_ARRAY_PARAM(media_ip, media_ips, xe_ip_kunit_desc);
>>
>>@@ -34,6 +53,13 @@ KUNIT_ARRAY_PARAM(media_ip, media_ips, xe_ip_kunit_desc);
>>  */
>> const void *xe_pci_graphics_ip_gen_param(const void *prev, char *desc)
>> {
>>+	const void *next = legacy_graphics_ip_gen_params(prev, desc);
>>+
>>+	if (next)
>>+		return next;
>>+	if (IS_ARRAY_ROW(legacy_graphics_ips, prev))

I don't understand what "is array row" means.

BR,
Jani.

>
> I don't like much this way of iterating over multiple arrays, but for a
> test generator is probably fine. Maybe add a comment on top of
> 'Pre-GMDID Graphics IPs`:
>
> 	/*
> 	 * Mimic the way gmdid IPs are declared so the same param
> 	 * generator can be used for both
> 	 */
>
>
> anyway,
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>
> Lucas De Marchi
>
>>+		prev = NULL;
>>+
>> 	return graphics_ip_gen_params(prev, desc);
>> }
>> EXPORT_SYMBOL_IF_KUNIT(xe_pci_graphics_ip_gen_param);
>>@@ -51,6 +77,13 @@ EXPORT_SYMBOL_IF_KUNIT(xe_pci_graphics_ip_gen_param);
>>  */
>> const void *xe_pci_media_ip_gen_param(const void *prev, char *desc)
>> {
>>+	const void *next = legacy_media_ip_gen_params(prev, desc);
>>+
>>+	if (next)
>>+		return next;
>>+	if (IS_ARRAY_ROW(legacy_media_ips, prev))
>>+		prev = NULL;
>>+
>> 	return media_ip_gen_params(prev, desc);
>> }
>> EXPORT_SYMBOL_IF_KUNIT(xe_pci_media_ip_gen_param);
>>-- 
>>2.47.1
>>

-- 
Jani Nikula, Intel