Current approach stops whole build process on a case that there is mistake
on documentation. Let's have entry on logs instead and continue.

Small fix also related multiline data e.g. on Description part.

Signed-off-by: Jari Tahvanainen <jari.tahvanainen@intel.com>
Signed-off-by: Karthik B S <karthik.b.s@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Andrzej Hajda authored 1 week ago and Kamil Konieczny committed 1 day ago

Parenthesis around macro body were added to follow C conventions.
For newer platforms they are not allowed in such context.
For iga64 they do not change anything and can be omitted.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Andrzej Hajda authored 1 week ago and Kamil Konieczny committed 1 day ago

Script incorrectly handled empty asms in case of the newest
platforms.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Andrzej Hajda authored 1 week ago and Kamil Konieczny committed 1 day ago

Script generated three redundant lines per compilation.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Peter Senna Tschudin authored 2 weeks ago and Kamil Konieczny committed 2 weeks ago

Updates scripts/run-tests.sh to include the following options:

 -f:        enable igt_facts on igt_runner
 -K <mode>: Linux Kernel kmemleak reports
            - once: run a kmemleak scan after all tests
            - each: run a kmemleak scan after each test

These options are simply relayed to igt_runner.

Cc: vitaly.prosyak@amd.com
Cc: christian.koenig@amd.com
Cc: alexander.deucher@amd.com
Cc: jesse.zhang@amd.com
Cc: harry.wentland@amd.com
Cc: zbigniew.kempczynski@intel.com
Cc: kamil.konieczny@linux.intel.com
Cc: ryszard.knop@intel.com
Cc: lucas.demarchi@intel.com
Cc: katarzyna.piecielska@intel.com
Reviewed-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@linux.intel.com>

Andrzej Hajda authored 3 months ago and Kamil Konieczny committed 3 months ago

The build process fails if the build dir is not a subdir of the project.

Fixes: cba1773f ("scripts/generate_iga64_codes: add iga64_macros.h to checksum calculation")
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Since generated code depends on iga64_macros.h assemblies should be
recompiled on it's change as well.

v2:
  - added warning to iga64_macros.h (Zbigniew)

Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>

Jean-François Dagenais authored 4 months ago and Kamil Konieczny committed 4 months ago

This script uses objcopy in order to dump a section. Objcopy is meant
for copying and expects a second argument. Failing that, it uses the
input file as output. Even though there's no intented change to the
file, this operation still re-writes the file completely.

In most cases, this re-writes the file as it was before. But in
cross-compilation cases, the "objcopy" program in the PATH used here
might be different than the toolchain which generated the ELF file.

In all cases, this causes a racy re-build of the .a, and in the worst
case, the objcopy re-written .o and .a files are actually incompatible
with the cross-linker used downstream in the ninja build causing failure
to find all symbols from the libs passed to this script.

```
../lib/intel_batchbuffer.c:763: undefined reference to
`gen8_gpgpu_fillfunc'
../lib/intel_batchbuffer.c:762: undefined reference to
`xehp_gpgpu_fillfunc'
[...]
```

The intent of the command was just to extract info from the LIBS, not
modify them. Using /dev/null as output file ensures the input files will
not be modified.

Signed-off-by: Jeff Dagenais <jeff.dagenais@gmail.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

George Zhang authored 4 months ago and Alex Hung committed 4 months ago

The default prune behaviour of the runner settings was changed.
This change is to maintain the original behaviour of this script
while also enabling the prune mode to be overriden.

v2: added m to getopts

v3: fixed bm: -> b:m: in getopts

Fixes: 997bac87 ("runner/settings: set PRUNE_KEEP_ALL as default")
Cc: Leo Li <sunpeng.li@amd.com>
Cc: Alex Hung <alex.hung@amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
Cc: Nicholas Choi <nicholas.choi@amd.com>
Signed-off-by: George Zhang <george.zhang@amd.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Logging out of not compiled tests looks like warning what is confusing.
Get rid of this printing to keep building steps clear.

Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Christoph Manszewski <christoph.manszewski@intel.com>
Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
Link: https://lore.kernel.org/r/20240923080520.39559-1-zbigniew.kempczynski@intel.com


Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

When tests are compiled conditionally and compilation flag is set
to false they have no binaries in the build directory. As testlist
is built from .c source file list, unnecessary warnings about missing
.testlist files are produced.

Avoid this warnings by skipping processing testlists for which binaries
weren't produced.

Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Cc: Christoph Manszewski <christoph.manszewski@intel.com>
Reviewed-by: Christoph Manszewski <christoph.manszewski@intel.com>
Link: https://lore.kernel.org/r/20240919081603.523299-1-zbigniew.kempczynski@intel.com


Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Kamil Konieczny authored 6 months ago and Zbigniew Kempczyński committed 6 months ago

Some tests could be added and compiled only when non-default
meson option is given. It will result in no testlist generated
for such tests but they could appear in our documentation.
Currently we report an error in such case.

Create a way to detect and report that and treat this as feature
and do not report it as an error.  This should allow to relax
processing of chamelium and other new tests.

Also while at this, print error information before actually
exiting with an error code, as previous prints were only
warnings.

Cc: Christoph Manszewski <christoph.manszewski@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Jari Tahvanainen <jari.tahvanainen@intel.com>
Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Christoph Manszewski <christoph.manszewski@intel.com>
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Link: https://lore.kernel.org/r/20240918113017.144687-12-christoph.manszewski@intel.com

PVC reports graphic version 12.60, which surprisingly maps to platform
12p72 in iga64, MTL maps 12.70 -> 12p71.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

Andrzej Hajda authored 8 months ago and Kamil Konieczny committed 8 months ago

With this patch adding iga64 assembly should be similar to
adding x86 assembly inline. Simple example:
    emit_iga64_code(shdr, set_exception, R"ASM(
        or (1|M0) cr0.1<1>:ud cr0.1<0;1,0>:ud ARG(0):ud
    )ASM", value);
Note presence of 'ARG(0)', it will be replaced by 'value' argument,
multiple arguments are possible.
More sophisticated examples in following patches.
How does it works:
1. Raw string literals (C++ feature available in gcc as extension):
   R"ASM(...)ASM" allows to use multiline/unescaped string literals.
   If for some reason they cannot be used we could always fallback to
   old ugly way of handling multiline strings with escape characters:
    emit_iga64_code(shdr, set_exception, "\n\
        or (1|M0) cr0.1<1>:ud cr0.1<0;1,0>:ud ARG(0):ud\n\
    ", value);
2. emit_iga64_code puts the assembly string into special linker section,
   and calls __emit_iga64_code with pointer to external variable
   which will contain code templates generated from the assembly for all
   supported platforms, remaining arguments are put to temporal array
   to eventually patch the code with positional arguments.
3. During build phase the linker section is scanned for assemblies.
   Every assembly is preprocessed with cpp, to replace ARG(x) macros with
   magic numbers, and to provide different code for different platforms
   if needed. Then output file is compiled with iga64, and then .c file
   is generated with global variables pointing to hexified iga64 codes.

v2:
 - fixed meson paths to script,
 - added check if compiler supports all platforms,
 - include assembly names in MD5 calculations,
 - use more specific name for MD5 sum
v3:
 - bump minimal meson version to kill "ERROR:  Expecting eol got id." bug
v4:
 - set minimal meson to 0.49.2 - builder uses it
v5:
 - revert back minimal ver of meson, instead use old syntax a.contains(b)
v6:
 - generate_iga64_codes moved to scripts dir,
 - added include guards to iga64_macros.h
v7:
 - use C++ style comments in generated file,
 - style fixes
v8:
 - added sanity check for assembly

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>

When main field name had whitespace, script incorrectly split it
into two separate words, for example when used with
"Mega feature" it treats it as two words: "Mega" and "feature".
This is not what we want it so fix it, also use "Mega feature"
in main call.

Also while at this, add additional verification step that
conflicting words are consistent and improve example of usage.

Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

The logic which sets the max_length is wrong: it shall always use
sheet[0], as this may be the only row on a table.

Yet, it might be possible that a caller to test_to_xls() would
have been sending a completely empty sheet. While this doesn't
occur currently, it doesn't hurt adding an explicit check,
reporting a warning if this is indeed the case.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Create script for checking if keywords used in descriptions
have any collisions, as we want them to appear in only one field.
For example, 'reset' word is used now in two different fields
Functionality and Sub-category:

tests/intel/xe_exec_reset.c: * Functionality: reset
tests/device_reset.c: * Sub-category: Reset

With this it will help to spot such potential problems.

Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Cc: Jari Tahvanainen <jari.tahvanainen@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

Pylint is too picky. Still, it is useful to run it, as sometimes
it could point to real issues.

So, let's make it happy by:
- disabling R0902, which is one of those "too many" warnings
  (IMO, it doesn't make sense to limit the number of times some
  thing can be used inside a code;

- disabling another warning about not using .items(). On that
  particular case, IMO it will make the code less clearer for
  reviewers and future maintainership, e. g.:

	test_nr = self.tests[testname].get("Test")

  and similar occurrences for self.tests[testname] is a lot clearer
  than:

	for testname, value in self.tests.items():
		...
		test_nr = value.get("Test")
		...
		value["subtests"][subtest][k] = val

	(and other similar occurrences)

  ok, a better name than "value" might help, but still the
  obvious choice would be "test", "test_testname" and such,
  which just makes the code more obfuscated.

  So, ignore the warning for good.

- remove a blank line before a docstring comment;

- remove a currently unused argument from update_files;

- fix indent on a single line.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

When there are common fields at the test and they were updated,
reflect the changes at the test level if all subtests have equal
values at the same field(s).

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Instead of using self.props plus an ingore_fields, create
a list of values to be updated and use it everywhere, as it
helps being consistent along the script about what fields
should be updated.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Such log is usually unneeded. Drop it.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

If the TEST field has the same value of the subtest one, ignore
it when updating it, as the subtest on is inherited.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

By default, the update logic will place new values at the end of
subtests.

Yet, if the field is already at the source file, just update
the value at the line.

Please notice that the logic needs to parse the entire subtest
description, as continuation lines may be present.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

If the last line before inserting a new value is a blank comment,
add the new fiels before that.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Eventually, a field may have been removed. Add support for
detecting it at the write logic.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Acked-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Print what value was not changed because it is duplicated.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Currently the last line of the spreadsheet is not processed.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Automatically creating/updating wildcards is not an easy task.
This script can't do it, so make it clearer that it will be
skipped.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Add a verbosity parameter to hide messages used mostly for debugging
purposes, as, on several cases, the output may be misleading.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

Add a short description of what function does.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

The current logic relies on dict = sorted(dict) to preserve the
sorted order. Ordered dicts were introduced only on python 3.7.

This is a silly requirement, as all we want is to sort the
dict.item() tuple. Change the logic to avoid the need of
checking for an specific python version.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

- add a new parameter to optionally generate JSON debug files;
- don't use default formatter;
- don't use args SUPRESS;
- don't use an empty epilog.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

Use autopep8 and some manual adjustments to make pylint
happier.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

Move the main code to a function, called only when the script
is executed directly from command line.

That allows the method to be re-used by other scripts if needed.

No functional changes.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>

Such change allows using the code on some other python script that
would be converting IGT doc test documentation into XLS files.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Acked-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Currently, some parmeters like Run Type are imported from files.
Don't add documentation for them by default, preserving the way
it was originally meant to be filled.

Acked-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

There are two functions there that aren't unused.

Drop the dead code.

Acked-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

There were some changes at test_list that caused it to stop
working. Fix them.

Acked-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Mauro Carvalho Chehab authored 1 year ago and Kamil Konieczny committed 1 year ago

IGT runner is case insensitive, so, for machine, it doesn't matter.

Yet, as dictionaries are case sensitive, better to place the tests
on a canonical form, e. g. in lower case.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>

Joining testlists shall take into account default values for GPUs
that are defined only on some JSON config files.

Add a logic to propagate such values.

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>