[igt-dev] [PATCH i-g-t] i915/gem_workarounds: Adapt to change in file format for per-engine wa

[Tvrtko Ursulin]

On 03/07/2019 17:59, Chris Wilson wrote:
> To reduce the assumptions of RCS0 in the kernel, we want to make the
> debugfs engine agnostic and so we need to adapt the igt parser for
> flexibility.
> 
> If we could just adequately simulate S3/S4 in the kernel we could forgo
> this test entirely...
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   tests/i915/gem_workarounds.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/i915/gem_workarounds.c b/tests/i915/gem_workarounds.c
> index 403863c0b..81c356f06 100644
> --- a/tests/i915/gem_workarounds.c
> +++ b/tests/i915/gem_workarounds.c
> @@ -249,6 +249,7 @@ igt_main
>   	igt_fixture {
>   		FILE *file;
>   		char *line = NULL;
> +		char *str;
>   		size_t line_size;
>   		int i, fd;
>   
> @@ -261,9 +262,13 @@ igt_main
>   
>   		fd = igt_debugfs_open(device, "i915_wa_registers", O_RDONLY);
>   		file = fdopen(fd, "r");
> -		igt_assert(getline(&line, &line_size, file) > 0);
> +		igt_require(getline(&line, &line_size, file) > 0);
>   		igt_debug("i915_wa_registers: %s", line);
> -		sscanf(line, "Workarounds applied: %d", &num_wa_regs);
> +
> +		/* We assume that the first batch is for rcs */

I think you mean "first line".

> +		str = strstr(line, "Workarounds applied:");
> +		igt_assert(str);
> +		sscanf(str, "Workarounds applied: %d", &num_wa_regs);
>   		igt_require(num_wa_regs > 0);

I have a suspicion all the above section could be simplified using 
fscanf and just checking return value and errno but that would be asking 
for too much work for what this is.

>   
>   		wa_regs = malloc(num_wa_regs * sizeof(*wa_regs));
> @@ -271,6 +276,9 @@ igt_main
>   
>   		i = 0;
>   		while (getline(&line, &line_size, file) > 0) {
> +			if (strstr(line, "Workarounds applied:"))
> +				break;
> +
>   			igt_debug("%s", line);
>   			if (sscanf(line, "0x%X: 0x%08X, mask: 0x%08X",
>   				   &wa_regs[i].addr,
> 

I just had a thought that since we are fiddling with this we should add 
engine names into strings. And/or class:instance pairs.

Regards,

Tvrtko