[PATCH i-g-t v2 0/2] CONTRIBUTING: Add subscription info, patchwork link and lib guide

[PATCH i-g-t v2 0/2] CONTRIBUTING: Add subscription info, patchwork link and lib guide

[Kamil Konieczny]

Add info about subscription for igt-dev mailing list and a link
to patchwork where sended patches with test status could be seen.
Also add some general guidelines about libraries.

v2: rewritten second patch about lib functions

Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@gmail.com>
Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
Cc: Juha-Pekka Heikkila <juha-pekka.heikkila@intel.com>
Cc: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
Cc: Karthik B S <karthik.b.s@intel.com>
Cc: Petri Latvala <adrinael@adrinael.net>
Cc: "Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>

Kamil Konieczny (2):
  CONTRIBUTING: Add subscription and patchwork info
  CONTRIBUTING: Add guide about igt libraries

 CONTRIBUTING.md | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

-- 
2.49.0

[PATCH i-g-t v2 1/2] CONTRIBUTING: Add subscription and patchwork info

[Kamil Konieczny]

Add guide and link to subscription into mailing list and link
for patchwork.

Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Acked-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 CONTRIBUTING.md | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 81057a2ad..8370451d6 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -63,6 +63,13 @@ Sending Patches
 
   on its first invocation.
 
+- If you plan to contribute regularly, please subscribe to igt-dev mailinglist:
+  https://lists.freedesktop.org/mailman/listinfo/igt-dev
+  When you are not subscribed, please note that your contribution will take
+  more time to reach to mailing list. You could find out if it was delivered or
+  what is a testing status of your patches at page:
+  https://patchwork.freedesktop.org/project/igt/series/
+
 - Place relevant prefix in subject, for example when your change is in one
   testfile, use its name without '.c' nor '.h' suffix, like:
   tests/simple_test: short description
-- 
2.49.0

[PATCH i-g-t v2 2/2] CONTRIBUTING: Add guide about igt libraries

[Kamil Konieczny]

Add some general guide about adding new library function and
a few guides for their usage outside of tests.

Cc: Andrzej Hajda <andrzej.hajda@intel.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Acked-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
---
 CONTRIBUTING.md | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 8370451d6..d4efa61f5 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -38,6 +38,34 @@ The Code
 
 [igt-describe]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe
 
+IGT libraries
+-------------
+- Tests and benchmarks are the main usage of IGT libraries, so they
+  could use test specific macros/functions, for example igt_assert,
+  igt_require, igt_skip, igt_info or igt_debug.
+
+- New library function could be written when it will have at least two
+  different users, for example if it could be used by two or more tests.
+  In some cases single user can be accepted, when it is very likely it
+  will be used in future.
+
+- In a new library function():
+  if it uses some of the macros igt_assert/igt_require/igt_skip then
+  consider to write also __function() with the same functionality but
+  without them.
+
+- Libraries and igt_runner
+  Runner should not use lib functions. It is crucial for CI runs so using
+  libraries puts a risk of bringing changes meant for tests which in turn
+  could break runner.
+  Note: You will find places where igt_runner uses lib functions - this will
+  be on ToDo list to be fixed.
+
+- Libraries and tools/
+  Tools should try to not use lib functions. Any abnormal condition should
+  be simply reported by printf or fprintf to stdout/stderr and then tool
+  should exit gracefully. Do not use igt_abort nor igt_assert, igt_print,
+  igt_debug nor other testing/printing macros from igt lib/
 
 Sending Patches
 ---------------
-- 
2.49.0

Re: [PATCH i-g-t v2 1/2] CONTRIBUTING: Add subscription and patchwork info

[Dixit, Ashutosh]

On Mon, 31 Mar 2025 06:34:23 -0700, Kamil Konieczny wrote:
>
> Add guide and link to subscription into mailing list and link
> for patchwork.
>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Acked-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> ---
>  CONTRIBUTING.md | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 81057a2ad..8370451d6 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -63,6 +63,13 @@ Sending Patches
>
>    on its first invocation.
>
> +- If you plan to contribute regularly, please subscribe to igt-dev mailinglist:
> +  https://lists.freedesktop.org/mailman/listinfo/igt-dev
> +  When you are not subscribed, please note that your contribution will take
> +  more time to reach to mailing list. You could find out if it was delivered or
> +  what is a testing status of your patches at page:
> +  https://patchwork.freedesktop.org/project/igt/series/

I've heard that patchwork is likely to be phased out in the future and
replaced with some infrastructure around lore.kernel.org (see below).

Maybe we can mention that emails, as well as test results, can also be seen
at:

https://lore.kernel.org/igt-dev/

> +
>  - Place relevant prefix in subject, for example when your change is in one
>    testfile, use its name without '.c' nor '.h' suffix, like:
>    tests/simple_test: short description
> --
> 2.49.0
>

Re: [PATCH i-g-t v2 2/2] CONTRIBUTING: Add guide about igt libraries

[Dixit, Ashutosh]

On Mon, 31 Mar 2025 06:34:24 -0700, Kamil Konieczny wrote:
>
> Add some general guide about adding new library function and
> a few guides for their usage outside of tests.
>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Acked-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
> ---
>  CONTRIBUTING.md | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index 8370451d6..d4efa61f5 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -38,6 +38,34 @@ The Code
>
>  [igt-describe]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe
>
> +IGT libraries
> +-------------
> +- Tests and benchmarks are the main usage of IGT libraries, so they
> +  could use test specific macros/functions, for example igt_assert,
> +  igt_require, igt_skip, igt_info or igt_debug.
> +
> +- New library function could be written when it will have at least two
> +  different users, for example if it could be used by two or more tests.
> +  In some cases single user can be accepted, when it is very likely it
> +  will be used in future.
> +
> +- In a new library function():
> +  if it uses some of the macros igt_assert/igt_require/igt_skip then
> +  consider to write also __function() with the same functionality but
> +  without them.
> +
> +- Libraries and igt_runner
> +  Runner should not use lib functions. It is crucial for CI runs so using
> +  libraries puts a risk of bringing changes meant for tests which in turn
> +  could break runner.
> +  Note: You will find places where igt_runner uses lib functions - this will
> +  be on ToDo list to be fixed.
> +
> +- Libraries and tools/
> +  Tools should try to not use lib functions. Any abnormal condition should
> +  be simply reported by printf or fprintf to stdout/stderr and then tool
> +  should exit gracefully. Do not use igt_abort nor igt_assert, igt_print,
> +  igt_debug nor other testing/printing macros from igt lib/

Not sure about runner, but for tools, I'd rather have shared functions from
lib/ rather than duplicating code. So we should be more specific here about
which functions tools should not use. Maybe tools should not use 'igt_abort
nor igt_assert, igt_print, igt_debug' etc. or even using these might be ok,
as long as the tool works as expected?

Or maybe just say here that 'give some thought if you are planning to use
IGT lib code in tools, some IGT lib functions might be be appropriate in
tools'.

For example previously a lot of extra code was written in i915 perf
library, just to avoid using IGT lib/. I changed this approach in xe perf
lib and decided to use IGT lib/.



>
>  Sending Patches
>  ---------------
> --
> 2.49.0
>

Re: [PATCH i-g-t v2 1/2] CONTRIBUTING: Add subscription and patchwork info

[Kamil Konieczny]

Hi Dixit,,
On 2025-03-31 at 09:54:02 -0700, Dixit, Ashutosh wrote:
> On Mon, 31 Mar 2025 06:34:23 -0700, Kamil Konieczny wrote:
> >
> > Add guide and link to subscription into mailing list and link
> > for patchwork.
> >
> > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Acked-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> > ---
> >  CONTRIBUTING.md | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> > index 81057a2ad..8370451d6 100644
> > --- a/CONTRIBUTING.md
> > +++ b/CONTRIBUTING.md
> > @@ -63,6 +63,13 @@ Sending Patches
> >
> >    on its first invocation.
> >
> > +- If you plan to contribute regularly, please subscribe to igt-dev mailinglist:
> > +  https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > +  When you are not subscribed, please note that your contribution will take
> > +  more time to reach to mailing list. You could find out if it was delivered or
> > +  what is a testing status of your patches at page:
> > +  https://patchwork.freedesktop.org/project/igt/series/
> 
> I've heard that patchwork is likely to be phased out in the future and
> replaced with some infrastructure around lore.kernel.org (see below).

It will be updated then, now it should describe what we have.

> 
> Maybe we can mention that emails, as well as test results, can also be seen
> at:
> 
> https://lore.kernel.org/igt-dev/

Good point, thank you!

Regards,
Kamil

> 
> > +
> >  - Place relevant prefix in subject, for example when your change is in one
> >    testfile, use its name without '.c' nor '.h' suffix, like:
> >    tests/simple_test: short description
> > --
> > 2.49.0
> >

Re: [PATCH i-g-t v2 2/2] CONTRIBUTING: Add guide about igt libraries

[Kamil Konieczny]

Hi Dixit,,
On 2025-03-31 at 10:08:54 -0700, Dixit, Ashutosh wrote:
> On Mon, 31 Mar 2025 06:34:24 -0700, Kamil Konieczny wrote:
> >
> > Add some general guide about adding new library function and
> > a few guides for their usage outside of tests.
> >
> > Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> > Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > Acked-by: Katarzyna Piecielska <katarzyna.piecielska@intel.com>
> > ---
> >  CONTRIBUTING.md | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> > index 8370451d6..d4efa61f5 100644
> > --- a/CONTRIBUTING.md
> > +++ b/CONTRIBUTING.md
> > @@ -38,6 +38,34 @@ The Code
> >
> >  [igt-describe]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe
> >
> > +IGT libraries
> > +-------------
> > +- Tests and benchmarks are the main usage of IGT libraries, so they
> > +  could use test specific macros/functions, for example igt_assert,
> > +  igt_require, igt_skip, igt_info or igt_debug.
> > +
> > +- New library function could be written when it will have at least two
> > +  different users, for example if it could be used by two or more tests.
> > +  In some cases single user can be accepted, when it is very likely it
> > +  will be used in future.
> > +
> > +- In a new library function():
> > +  if it uses some of the macros igt_assert/igt_require/igt_skip then
> > +  consider to write also __function() with the same functionality but
> > +  without them.
> > +
> > +- Libraries and igt_runner
> > +  Runner should not use lib functions. It is crucial for CI runs so using
> > +  libraries puts a risk of bringing changes meant for tests which in turn
> > +  could break runner.
> > +  Note: You will find places where igt_runner uses lib functions - this will
> > +  be on ToDo list to be fixed.
> > +
> > +- Libraries and tools/
> > +  Tools should try to not use lib functions. Any abnormal condition should
> > +  be simply reported by printf or fprintf to stdout/stderr and then tool
> > +  should exit gracefully. Do not use igt_abort nor igt_assert, igt_print,
> > +  igt_debug nor other testing/printing macros from igt lib/
> 
> Not sure about runner, but for tools, I'd rather have shared functions from
> lib/ rather than duplicating code. So we should be more specific here about
> which functions tools should not use. Maybe tools should not use 'igt_abort
> nor igt_assert, igt_print, igt_debug' etc. or even using these might be ok,
> as long as the tool works as expected?
> 
> Or maybe just say here that 'give some thought if you are planning to use
> IGT lib code in tools, some IGT lib functions might be be appropriate in
> tools'.
> 
> For example previously a lot of extra code was written in i915 perf
> library, just to avoid using IGT lib/. I changed this approach in xe perf
> lib and decided to use IGT lib/.
> 

You are right, I will rewrite this.

Regards,
Kamil

> 
> 
> >
> >  Sending Patches
> >  ---------------
> > --
> > 2.49.0
> >