[Intel-gfx] [IGT] IGT/tests/firmware: Test firmware loading by reading debugfs

[Srivatsa, Anusha]

>-----Original Message-----
>From: Vivi, Rodrigo
>Sent: Friday, December 1, 2017 2:33 PM
>To: Srivatsa, Anusha <anusha.srivatsa at intel.com>
>Cc: intel-gfx at lists.freedesktop.org
>Subject: Re: [IGT] IGT/tests/firmware: Test firmware loading by reading debugfs
>
>On Fri, Dec 01, 2017 at 09:40:35PM +0000, Anusha Srivatsa wrote:
>> Read debugfs and sysfs entries to check for GuC loading conditions.
>>
>> The patch is still WIP. Once all check conditions are covered, it can
>> be extended to huc debugfs file too.
>>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa at intel.com>
>> ---
>>  tests/Makefile.sources |  1 +
>>  tests/test_firmware.c  | 96
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 97 insertions(+)
>>  create mode 100644 tests/test_firmware.c
>>
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources index
>> 0f4e39a..841fc54 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -234,6 +234,7 @@ TESTS_progs = \
>>  	template \
>>  	tools_test \
>>  	vgem_basic \
>> +	test_firmware \
>
>note that igt tests names are meaningful... <area>_<testname>
>
>so probably better to use fw_basic

OK... fw_basic or fw_debugfs?

>>  	vgem_slow \
>
>and also they are more or less sorted out here...
>but you added on the middle of a vgem group...

Oops .... will change the order.

>>  	$(NULL)
>>
>> diff --git a/tests/test_firmware.c b/tests/test_firmware.c new file
>> mode 100644 index 0000000..a5d621a
>> --- /dev/null
>> +++ b/tests/test_firmware.c
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Copyright (c) 2013 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> +obtaining a
>> + * copy of this software and associated documentation files (the
>> +"Software"),
>> + * to deal in the Software without restriction, including without
>> +limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> +sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom
>> +the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including
>> +the next
>> + * paragraph) shall be included in all copies or substantial portions
>> +of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> +EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
>EVENT
>> +SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
>DAMAGES
>> +OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> +ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> +OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors: Anusha Srivatsa
>> + *
>> + */
>> +
>> +#include "igt.h"
>> +#include "stdio.h"
>> +#include "stdlib.h"
>> +#include "igt_sysfs.h"
>> +#include "igt_debugfs.h"
>> +#include "i915_drm.h"
>> +#include "fcntl.h"
>> +
>> +IGT_TEST_DESCRIPTION("Read contents of debugfs to check for firmware
>> +status.");
>> +
>> +static void guc_load(int drm_fd, int debugfs, int gen) {
>> +	char firmware_buf[250];
>> +	float fw_version_wanted;
>> +	float fw_version_found;
>> +	int ret;
>> +	int enable_guc_loading;
>> +	FILE *fp;
>> +
>> +	igt_skip_on_f(gen < 6,
>> +		      "GuC not available on platforms prior to Skylake\n");
>> +
>> +	igt_sysfs_scanf(debugfs, "i915_enable_guc_loading", "%d",
>&enable_guc_loading);
>> +	igt_skip_on_f(!(enable_guc_loading < 1), "GuC loading not
>> +enabled\n");
>> +
>> +	ret = igt_debugfs_open(drm_fd, "i915_guc_load_status", O_RDONLY);
>> +	fp = fdopen(ret, "r");
>> +	igt_fail_on_f(ret < 0, "Not able to open the debugfs file\n");
>> +
>> +	igt_debugfs_read(drm_fd, "i915_guc_load_status", firmware_buf);
>> +
>> +	fseek(fp, 99, SEEK_CUR);
>
>This is risky/fragile imo... any small change on debugfs this will break...

I agree totally..... I am not very happy about it either. Any suggestions on how else I can scan it? 

>Have you consider the new kernel seftest thing to make the tests inside kernel
>itself without need for parse this debugfs interface?

Good point. Let me have a look at it.
Thanks a lot Rodrigo.

Anusha
>> +	fscanf(fp, "%f", &fw_version_wanted);
>> +	fseek(fp, 119, SEEK_SET);
>> +	fscanf(fp, "%f", &fw_version_found);
>> +
>> +	igt_fail_on_f((fw_version_wanted != fw_version_found),
>> +		      "Firmware version found not the version wanted\n");
>> +	igt_fail_on_f(strstr(firmware_buf, "fetch: NONE"),
>> +		      "Firmware not fetched\n");
>> +	igt_fail_on_f(strstr(firmware_buf, "fetch: SUCCESS") &&
>> +		      strstr(firmware_buf, "load: NONE "),
>> +		      "Firmware not loaded\n");
>> +}
>> +
>> +int drm_fd;
>> +int debugfs;
>> +int gen;
>> +
>> +igt_main
>> +{
>> +	igt_skip_on_simulation();
>> +	igt_fixture {
>> +		drm_fd = drm_open_driver(DRIVER_INTEL);
>> +		igt_require(drm_fd >= 0);
>> +
>> +		debugfs = igt_debugfs_dir(drm_fd);
>> +		gen = intel_gen(intel_get_drm_devid(drm_fd));
>> +	}
>> +
>> +	igt_subtest("guc-load-test")
>> +		guc_load(drm_fd, debugfs, gen);
>> +
>> +	igt_success();
>> +
>> +	igt_fixture {
>> +		close(debugfs);
>> +		close(drm_fd);
>> +	}
>> +}
>> --
>> 2.7.4
>>