[igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Bhanuprakash Modem]

This reverts commit f6f5903348eff6510f8877043f9397a2a5cb94b3.

This patch is causing the regression in CI.

[1767/1775] Generating i915_tests.rst with a custom command
FAILED: docs/testplan/i915_tests.rst
./igt_doc.py --config ../tests/intel/i915_test_config.json --rest docs/testplan/i915_tests.rst --check-testlist --igt-build-path ./build
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted-signaled doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted-submitted doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted-submitted-signaled doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted-signaled doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted-submitted doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted-submitted-signaled doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-single-wait-all-available-unsubmitted doesn't exist on source files
Warning: Documented igt@syncobj_timeline@invalid-single-wait-available-unsubmitted doesn't exist on source files
Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted
Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted-signaled
Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted-submitted
Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted-submitted-signaled
Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-available-unsubmitted
Warning: Missing documentation for igt@syncobj_timeline@etime-single-wait-all-available-unsubmitted
Warning: Missing documentation for igt@syncobj_timeline@etime-single-wait-available-unsubmitted
Warning: Missing documentation for igt@syncobj_timeline@multi-wait-available-unsubmitted-signaled
Warning: Missing documentation for igt@syncobj_timeline@multi-wait-available-unsubmitted-submitted
Warning: Missing documentation for igt@syncobj_timeline@multi-wait-available-unsubmitted-submitted-signaled
Please refer: docs/test_documentation.md for more details
[1771/1775] Compiling C object runner/runner_test.p/runner_tests.c.o
ninja: build stopped: subcommand failed.

Cc: Erik Kurzinger <ekurzinger@nvidia.com>
Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Simon Ser <contact@emersion.fr>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 tests/syncobj_timeline.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/syncobj_timeline.c b/tests/syncobj_timeline.c
index 0422bca51..b4e1d093a 100644
--- a/tests/syncobj_timeline.c
+++ b/tests/syncobj_timeline.c
@@ -1782,7 +1782,7 @@ igt_main
 						WAIT_SIGNALED)) != 1)
 			continue;
 
-		if ((flags & WAIT_UNSUBMITTED) && !((flags & WAIT_FOR_SUBMIT) || (flags & WAIT_AVAILABLE)))
+		if ((flags & WAIT_UNSUBMITTED) && !(flags & WAIT_FOR_SUBMIT))
 			err = -EINVAL;
 		else if (!(flags & WAIT_SIGNALED) && !((flags & WAIT_SUBMITTED) && (flags & WAIT_AVAILABLE)))
 			err = -ETIME;
@@ -1851,7 +1851,7 @@ igt_main
 			continue;
 
 		err = 0;
-		if ((flags & WAIT_UNSUBMITTED) && !((flags & WAIT_FOR_SUBMIT) || (flags & WAIT_AVAILABLE))) {
+		if ((flags & WAIT_UNSUBMITTED) && !(flags & WAIT_FOR_SUBMIT)) {
 			err = -EINVAL;
 		} else if (flags & WAIT_ALL) {
 			if (flags & (WAIT_UNSUBMITTED | WAIT_SUBMITTED))
-- 
2.40.0

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Simon Ser]

NACK. Please fix your docs/testplan/i915_tests.rst instead.

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Juha-Pekka Heikkila]

Acked-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

On 13.11.2023 11.43, Bhanuprakash Modem wrote:
> This reverts commit f6f5903348eff6510f8877043f9397a2a5cb94b3.
> 
> This patch is causing the regression in CI.
> 
> [1767/1775] Generating i915_tests.rst with a custom command
> FAILED: docs/testplan/i915_tests.rst
> ./igt_doc.py --config ../tests/intel/i915_test_config.json --rest docs/testplan/i915_tests.rst --check-testlist --igt-build-path ./build
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted-signaled doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted-submitted doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-all-available-unsubmitted-submitted-signaled doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted-signaled doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted-submitted doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-multi-wait-available-unsubmitted-submitted-signaled doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-single-wait-all-available-unsubmitted doesn't exist on source files
> Warning: Documented igt@syncobj_timeline@invalid-single-wait-available-unsubmitted doesn't exist on source files
> Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted
> Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted-signaled
> Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted-submitted
> Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-all-available-unsubmitted-submitted-signaled
> Warning: Missing documentation for igt@syncobj_timeline@etime-multi-wait-available-unsubmitted
> Warning: Missing documentation for igt@syncobj_timeline@etime-single-wait-all-available-unsubmitted
> Warning: Missing documentation for igt@syncobj_timeline@etime-single-wait-available-unsubmitted
> Warning: Missing documentation for igt@syncobj_timeline@multi-wait-available-unsubmitted-signaled
> Warning: Missing documentation for igt@syncobj_timeline@multi-wait-available-unsubmitted-submitted
> Warning: Missing documentation for igt@syncobj_timeline@multi-wait-available-unsubmitted-submitted-signaled
> Please refer: docs/test_documentation.md for more details
> [1771/1775] Compiling C object runner/runner_test.p/runner_tests.c.o
> ninja: build stopped: subcommand failed.
> 
> Cc: Erik Kurzinger <ekurzinger@nvidia.com>
> Cc: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Simon Ser <contact@emersion.fr>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>   tests/syncobj_timeline.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/syncobj_timeline.c b/tests/syncobj_timeline.c
> index 0422bca51..b4e1d093a 100644
> --- a/tests/syncobj_timeline.c
> +++ b/tests/syncobj_timeline.c
> @@ -1782,7 +1782,7 @@ igt_main
>   						WAIT_SIGNALED)) != 1)
>   			continue;
>   
> -		if ((flags & WAIT_UNSUBMITTED) && !((flags & WAIT_FOR_SUBMIT) || (flags & WAIT_AVAILABLE)))
> +		if ((flags & WAIT_UNSUBMITTED) && !(flags & WAIT_FOR_SUBMIT))
>   			err = -EINVAL;
>   		else if (!(flags & WAIT_SIGNALED) && !((flags & WAIT_SUBMITTED) && (flags & WAIT_AVAILABLE)))
>   			err = -ETIME;
> @@ -1851,7 +1851,7 @@ igt_main
>   			continue;
>   
>   		err = 0;
> -		if ((flags & WAIT_UNSUBMITTED) && !((flags & WAIT_FOR_SUBMIT) || (flags & WAIT_AVAILABLE))) {
> +		if ((flags & WAIT_UNSUBMITTED) && !(flags & WAIT_FOR_SUBMIT)) {
>   			err = -EINVAL;
>   		} else if (flags & WAIT_ALL) {
>   			if (flags & (WAIT_UNSUBMITTED | WAIT_SUBMITTED))

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Juha-Pekka Heikkila]

On 13.11.2023 11.55, Simon Ser wrote:
> NACK. Please fix your docs/testplan/i915_tests.rst instead.

Let's do fixes outside of master branch where it doesn't bother others.

/Juha-Pekka

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Simon Ser]

On Monday, November 13th, 2023 at 10:59, Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> wrote:

> On 13.11.2023 11.55, Simon Ser wrote:
> 
> > NACK. Please fix your docs/testplan/i915_tests.rst instead.
> 
> Let's do fixes outside of master branch where it doesn't bother others.

Sorry, but no. This commit fixes a bug already. The CI regression is
completely unrelated, and Intel-specific, I really don't understand why
it happens in the first place.

Is it by design that actual bug fixes break Intel CI?

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Modem, Bhanuprakash]

On Mon-13-11-2023 03:25 pm, Simon Ser wrote:
> NACK. Please fix your docs/testplan/i915_tests.rst instead.

As "CI" won't pick the latest head, my patch can't go through the CI. 
And we can't guarantee that my fix may not end up in some other problem.

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Juha-Pekka Heikkila]

On 13.11.2023 12.02, Simon Ser wrote:
> On Monday, November 13th, 2023 at 10:59, Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> wrote:
> 
>> On 13.11.2023 11.55, Simon Ser wrote:
>>
>>> NACK. Please fix your docs/testplan/i915_tests.rst instead.
>>
>> Let's do fixes outside of master branch where it doesn't bother others.
> 
> Sorry, but no. This commit fixes a bug already. The CI regression is
> completely unrelated, and Intel-specific, I really don't understand why
> it happens in the first place.
> 
> Is it by design that actual bug fixes break Intel CI?

As is this is blocking others totally unrelated to this. This patch 
never passed ci in the first place hence never should've been merged. 
Let's fix it somewhere else than on everyone's build machines independently.

/Juha-Pekka

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Simon Ser]

On Monday, November 13th, 2023 at 11:06, Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> wrote:

> On 13.11.2023 12.02, Simon Ser wrote:
> 
> > On Monday, November 13th, 2023 at 10:59, Juha-Pekka Heikkila juhapekka.heikkila@gmail.com wrote:
> > 
> > > On 13.11.2023 11.55, Simon Ser wrote:
> > > 
> > > > NACK. Please fix your docs/testplan/i915_tests.rst instead.
> > > 
> > > Let's do fixes outside of master branch where it doesn't bother others.
> > 
> > Sorry, but no. This commit fixes a bug already. The CI regression is
> > completely unrelated, and Intel-specific, I really don't understand why
> > it happens in the first place.
> > 
> > Is it by design that actual bug fixes break Intel CI?
> 
> As is this is blocking others totally unrelated to this. This patch
> never passed ci in the first place hence never should've been merged.
> Let's fix it somewhere else than on everyone's build machines independently.

If we want a quick fix, we can disable the broken Intel CI, instead of
reverting a completely correct bugfix.

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Juha-Pekka Heikkila]

On 13.11.2023 12.30, Simon Ser wrote:
> On Monday, November 13th, 2023 at 11:06, Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> wrote:
> 
>> On 13.11.2023 12.02, Simon Ser wrote:
>>
>>> On Monday, November 13th, 2023 at 10:59, Juha-Pekka Heikkila juhapekka.heikkila@gmail.com wrote:
>>>
>>>> On 13.11.2023 11.55, Simon Ser wrote:
>>>>
>>>>> NACK. Please fix your docs/testplan/i915_tests.rst instead.
>>>>
>>>> Let's do fixes outside of master branch where it doesn't bother others.
>>>
>>> Sorry, but no. This commit fixes a bug already. The CI regression is
>>> completely unrelated, and Intel-specific, I really don't understand why
>>> it happens in the first place.
>>>
>>> Is it by design that actual bug fixes break Intel CI?
>>
>> As is this is blocking others totally unrelated to this. This patch
>> never passed ci in the first place hence never should've been merged.
>> Let's fix it somewhere else than on everyone's build machines independently.
> 
> If we want a quick fix, we can disable the broken Intel CI, instead of
> reverting a completely correct bugfix.

Simon, disabling CI doesn't universally fix the build for everyone. If 
you don't use testplan it's your story but it's not the story for all 
other igt users. Passing ci testing is the requirement for merging, 
that's why ci is running on patch sets. This reverted patch didn't even 
build in ci and after you merged it others started to see what ci 
already had reported about this patch.

/Juha-Pekka

Re: [igt-dev] [i-g-t] Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Simon Ser]

Apologies. Jani highlights that the bugfix does update the testlist
(dynamically generated), and I completely missed that. The CI failure
makes a lot more sense now.

[igt-dev] ✗ Fi.CI.BAT: failure for Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 277 bytes --]

== Series Details ==

Series: Revert "syncobj_timeline: don't expect EINVAL for WAIT_UNSUBMITTED, | WAIT_AVAILABLE"
URL   : https://patchwork.freedesktop.org/series/126326/
State : failure

== Summary ==

Series 126326 revision 1 was fully merged or fully failed: no git log



[-- Attachment #2: Type: text/html, Size: 760 bytes --]