Re: [PATCH i-g-t v2 03/10] tests/intel/xe_drm_fdinfo: Add helper to read utilization for all classes

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Thu, Jul 11, 2024 at 08:40:46AM -0500, Lucas De Marchi wrote:
>On Tue, Jul 02, 2024 at 05:25:25PM GMT, Umesh Nerlige Ramappa wrote:
>>Add a helper that captures utilization for all classes.
>>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>>tests/intel/xe_drm_fdinfo.c | 20 ++++++++++++++++++++
>>1 file changed, 20 insertions(+)
>>
>>diff --git a/tests/intel/xe_drm_fdinfo.c b/tests/intel/xe_drm_fdinfo.c
>>index b64bef339..c697f3e53 100644
>>--- a/tests/intel/xe_drm_fdinfo.c
>>+++ b/tests/intel/xe_drm_fdinfo.c
>>@@ -42,6 +42,11 @@ IGT_TEST_DESCRIPTION("Read and verify drm client memory consumption and engine u
>>
>>#define BO_SIZE (65536)
>>
>>+struct pceu_cycles {
>>+	uint64_t cycles;
>>+	uint64_t total_cycles;
>>+};
>>+
>>static const char *engine_map[] = {
>>	"rcs",
>>	"bcs",
>>@@ -49,6 +54,21 @@ static const char *engine_map[] = {
>>	"vecs",
>>	"ccs",
>>};
>
>nit: missing newline here, otherwise:

will add

>
>>+static void read_engine_cycles(int xe, struct pceu_cycles *pceu)
>
>I think that would be better to give the compiler a hint about the
>size of the array:
>
>static void read_engine_cycles(int xe,
>			       struct pceu_cycles pceu[static DRM_XE_ENGINE_CLASS_COMPUTE + 1])

Never knew static could be placed there. Will add.

>
>(it would not be a bad idea to have a local __IGT_DRM_XE_ENGINE_CLASS_LAST
>too, but outside of the scope here)
>
>Otherwise,
>
>// Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

Thanks,
Umesh
>
>Lucas De Marchi
>
>>+{
>>+	struct drm_client_fdinfo info = { };
>>+	int class;
>>+
>>+	igt_assert(pceu);
>>+	igt_assert(igt_parse_drm_fdinfo(xe, &info, engine_map,
>>+					ARRAY_SIZE(engine_map), NULL, 0));
>>+
>>+	xe_for_each_engine_class(class) {
>>+		pceu[class].cycles = info.cycles[class];
>>+		pceu[class].total_cycles = info.total_cycles[class];
>>+	}
>>+}
>>+
>>/* Subtests */
>>static void test_active(int fd, struct drm_xe_engine *engine)
>>{
>>-- 
>>2.38.1
>>