[PATCH v1] tests/xe_debugfs: Improve test_gt subtest

[Gurram, Pravalika]

> -----Original Message-----
> From: De Marchi, Lucas <lucas.demarchi at intel.com>
> Sent: Thursday, January 9, 2025 10:33 PM
> To: Gurram, Pravalika <pravalika.gurram at intel.com>
> Cc: igt-dev at lists.freedesktop.org
> Subject: Re: [PATCH v1] tests/xe_debugfs: Improve test_gt subtest
> 
> On Thu, Jan 09, 2025 at 04:27:21PM +0530, Pravalika Gurram wrote:
> >Read the debugfs entries in the loop to improve the readability.
> >
> >Signed-off-by: Pravalika Gurram <pravalika.gurram at intel.com>
> >---
> > tests/intel/xe_debugfs.c | 84 +++++++---------------------------------
> > 1 file changed, 13 insertions(+), 71 deletions(-)
> >
> >diff --git a/tests/intel/xe_debugfs.c b/tests/intel/xe_debugfs.c index
> >bcbb5036a..3d8c17921 100644
> >--- a/tests/intel/xe_debugfs.c
> >+++ b/tests/intel/xe_debugfs.c
> >@@ -180,77 +180,19 @@ test_gt(int fd, int gt_id)
> > //		"guc_ct_selftest"
> > 	};
> >
> >-	sprintf(name, "gt%d/hw_engines", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/sa_info", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/steering", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/topology", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/pat", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/mocs", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/ggtt", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/register-save-restore", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/workarounds", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/default_lrc_rcs", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/default_lrc_ccs", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/default_lrc_bcs", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/default_lrc_vecs", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/default_lrc_vcs", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/hwconfig", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/uc/guc_info", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/uc/huc_info", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >-
> >-	sprintf(name, "gt%d/uc/guc_log", gt_id);
> >-	igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >-	igt_debugfs_dump(fd, name);
> >+	for (int i = 0; i < ARRAY_SIZE(expected_files); i++) {
> >+		sprintf(name, "gt%d/%s", gt_id, expected_files[i]);
> >+		igt_assert(igt_debugfs_exists(fd, name, O_RDONLY));
> >+		if (!strstr(name, "uc")) {
> 
> strstr will find any substring "uc". You could do a strcmp() instead, but I don't
> understand why we are doing this here.
> 
> I'd just remove 'uc' from the array and update validate_entries to ignore
> directories.
> 
> Lucas De Marchi
> 

Strcmp will not work here because sprintf will save  name = "gt0/uc" 
If we do strcmp(name,"gt0/uc") gt cannot be fixed 0  always correct it can change 
that's why I have added strstr

I did not understand which directories can be ignored you meant say "uc"
we don't need to dump "uc/*" data?

--Pravalika
> >+			igt_debugfs_dump(fd, name);
> >+		} else {
> >+			for (int j = 0; j < ARRAY_SIZE(expected_files_uc); j++)
> {
> >+				sprintf(name, "gt%d/uc/%s", gt_id,
> expected_files_uc[j]);
> >+				igt_assert(igt_debugfs_exists(fd, name,
> O_RDONLY));
> >+				igt_debugfs_dump(fd, name);
> >+			}
> >+		}
> >+	}
> >
> > 	sprintf(name, "/gt%d", gt_id);
> > 	validate_entries(fd, name, expected_files,
> >ARRAY_SIZE(expected_files));
> >--
> >2.34.1
> >