On 04/07/2019 10:58, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-07-04 10:51:44)
>>
>> 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@xxxxxxxxxxxxxxxxxx>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>>> ---
>>> 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".
>
> I was thinking of batch as in set of workarounds.
Ok :)
>>> + 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.
>
> Speaking my language :)
>
>>>
>>> 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.
>
> Expand upon that thought: where do you want this? I presume you are
> thinking of an igt helper for rcs0 -> class:RENDER, inst:0
I now see you already did it in the i915 patch:
+ seq_printf(m, "%s: Workarounds applied: %u\n",
+ engine->name, count);
So nothing really. IGT can be extended later if needed.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
