glx-oml-sync-control-timing: Make test behavior more stable

Submitted by Illia Iorin on July 9, 2018, 7:09 p.m.

Details

Message ID 20180709190932.31609-1-illia.iorin@gloaballogic.com
State New
Headers show
Series "glx-oml-sync-control-timing: Make test behavior more stable" ( rev: 1 ) in Piglit

Not browsing as part of any series.

Commit Message

Illia Iorin July 9, 2018, 7:09 p.m.
The fourfold increase in the test failure condition
is due to its unstable work. The old condition was left,
but the test result was changed from fail to warn, added
fail condition.

Cc: Michel Dänzer <michel.daenzer@amd.com>
Cc: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Illia Iorin <illia.iorin@globallogic.com>
---
 tests/spec/glx_oml_sync_control/timing.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/tests/spec/glx_oml_sync_control/timing.c b/tests/spec/glx_oml_sync_control/timing.c
index ab7258940..6d810af52 100644
--- a/tests/spec/glx_oml_sync_control/timing.c
+++ b/tests/spec/glx_oml_sync_control/timing.c
@@ -332,7 +332,10 @@  draw(Display *dpy)
 				" does not match glXGetMscRateOML %fus\n",
 				msc_wallclock_duration_stats.mean,
 				expected_msc_wallclock_duration);
-			result = PIGLIT_FAIL;
+			piglit_merge_result(&result, PIGLIT_WARN);
+			if (fabs(expected_msc_wallclock_duration -
+			 	 msc_wallclock_duration_stats.mean) > 200)
+				result = PIGLIT_FAIL;
 		}
 	}
 

Comments

On Mon, Jul 9, 2018 at 9:09 PM, Illia Iorin <illia.iorin@gmail.com> wrote:
> The fourfold increase in the test failure condition
> is due to its unstable work. The old condition was left,
> but the test result was changed from fail to warn, added
> fail condition.
>

Hi, can you clarify what the commit message wrt. "fourfold increase...
due to its unstable work" means? I'd like to understand the observed
issue better.

thanks,
-mario

> Cc: Michel Dänzer <michel.daenzer@amd.com>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Signed-off-by: Illia Iorin <illia.iorin@globallogic.com>
> ---
>  tests/spec/glx_oml_sync_control/timing.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tests/spec/glx_oml_sync_control/timing.c b/tests/spec/glx_oml_sync_control/timing.c
> index ab7258940..6d810af52 100644
> --- a/tests/spec/glx_oml_sync_control/timing.c
> +++ b/tests/spec/glx_oml_sync_control/timing.c
> @@ -332,7 +332,10 @@ draw(Display *dpy)
>                                 " does not match glXGetMscRateOML %fus\n",
>                                 msc_wallclock_duration_stats.mean,
>                                 expected_msc_wallclock_duration);
> -                       result = PIGLIT_FAIL;
> +                       piglit_merge_result(&result, PIGLIT_WARN);
> +                       if (fabs(expected_msc_wallclock_duration -
> +                                msc_wallclock_duration_stats.mean) > 200)
> +                               result = PIGLIT_FAIL;
>                 }
>         }
>
> --
> 2.17.1
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
Hello,
Full explanation can be found in
https://bugs.freedesktop.org/show_bug.cgi?id=89437#c4
If this does not completely clarify the issue. Please specify the question.

Sincerely yours, Illia.

On Tue, Jul 10, 2018 at 12:38 AM Mario Kleiner <mario.kleiner.de@gmail.com>
wrote:

> On Mon, Jul 9, 2018 at 9:09 PM, Illia Iorin <illia.iorin@gmail.com> wrote:
> > The fourfold increase in the test failure condition
> > is due to its unstable work. The old condition was left,
> > but the test result was changed from fail to warn, added
> > fail condition.
> >
>
> Hi, can you clarify what the commit message wrt. "fourfold increase...
> due to its unstable work" means? I'd like to understand the observed
> issue better.
>
> thanks,
> -mario
>
> > Cc: Michel Dänzer <michel.daenzer@amd.com>
> > Cc: Emil Velikov <emil.velikov@collabora.com>
> > Signed-off-by: Illia Iorin <illia.iorin@globallogic.com>
> > ---
> >  tests/spec/glx_oml_sync_control/timing.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/spec/glx_oml_sync_control/timing.c
> b/tests/spec/glx_oml_sync_control/timing.c
> > index ab7258940..6d810af52 100644
> > --- a/tests/spec/glx_oml_sync_control/timing.c
> > +++ b/tests/spec/glx_oml_sync_control/timing.c
> > @@ -332,7 +332,10 @@ draw(Display *dpy)
> >                                 " does not match glXGetMscRateOML
> %fus\n",
> >                                 msc_wallclock_duration_stats.mean,
> >                                 expected_msc_wallclock_duration);
> > -                       result = PIGLIT_FAIL;
> > +                       piglit_merge_result(&result, PIGLIT_WARN);
> > +                       if (fabs(expected_msc_wallclock_duration -
> > +                                msc_wallclock_duration_stats.mean) >
> 200)
> > +                               result = PIGLIT_FAIL;
> >                 }
> >         }
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/piglit
>
ping

On Tue, Jul 10, 2018 at 2:25 PM Illia Iorin <illia.iorin@gmail.com> wrote:

> Hello,
> Full explanation can be found in
> https://bugs.freedesktop.org/show_bug.cgi?id=89437#c4
> If this does not completely clarify the issue. Please specify the question.
>
> Sincerely yours, Illia.
>
> On Tue, Jul 10, 2018 at 12:38 AM Mario Kleiner <mario.kleiner.de@gmail.com>
> wrote:
>
>> On Mon, Jul 9, 2018 at 9:09 PM, Illia Iorin <illia.iorin@gmail.com>
>> wrote:
>> > The fourfold increase in the test failure condition
>> > is due to its unstable work. The old condition was left,
>> > but the test result was changed from fail to warn, added
>> > fail condition.
>> >
>>
>> Hi, can you clarify what the commit message wrt. "fourfold increase...
>> due to its unstable work" means? I'd like to understand the observed
>> issue better.
>>
>> thanks,
>> -mario
>>
>> > Cc: Michel Dänzer <michel.daenzer@amd.com>
>> > Cc: Emil Velikov <emil.velikov@collabora.com>
>> > Signed-off-by: Illia Iorin <illia.iorin@globallogic.com>
>> > ---
>> >  tests/spec/glx_oml_sync_control/timing.c | 5 ++++-
>> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/tests/spec/glx_oml_sync_control/timing.c
>> b/tests/spec/glx_oml_sync_control/timing.c
>> > index ab7258940..6d810af52 100644
>> > --- a/tests/spec/glx_oml_sync_control/timing.c
>> > +++ b/tests/spec/glx_oml_sync_control/timing.c
>> > @@ -332,7 +332,10 @@ draw(Display *dpy)
>> >                                 " does not match glXGetMscRateOML
>> %fus\n",
>> >                                 msc_wallclock_duration_stats.mean,
>> >                                 expected_msc_wallclock_duration);
>> > -                       result = PIGLIT_FAIL;
>> > +                       piglit_merge_result(&result, PIGLIT_WARN);
>> > +                       if (fabs(expected_msc_wallclock_duration -
>> > +                                msc_wallclock_duration_stats.mean) >
>> 200)
>> > +                               result = PIGLIT_FAIL;
>> >                 }
>> >         }
>> >
>> > --
>> > 2.17.1
>> >
>> > _______________________________________________
>> > Piglit mailing list
>> > Piglit@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/piglit
>>
>
HI Illia,

Please don't top-post.

On 30 July 2018 at 10:51, Illia Iorin <illia.iorin@gmail.com> wrote:
> ping
>
> On Tue, Jul 10, 2018 at 2:25 PM Illia Iorin <illia.iorin@gmail.com> wrote:
>>
>> Hello,
>> Full explanation can be found in
Having a bug report is great, but the commit itself should be self sufficient.

>> https://bugs.freedesktop.org/show_bug.cgi?id=89437#c4
You can add this to the commit summary as
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89437

>> If this does not completely clarify the issue. Please specify the
>> question.
>>
>> Sincerely yours, Illia.
>>
>> On Tue, Jul 10, 2018 at 12:38 AM Mario Kleiner
>> <mario.kleiner.de@gmail.com> wrote:
>>>
>>> On Mon, Jul 9, 2018 at 9:09 PM, Illia Iorin <illia.iorin@gmail.com>
>>> wrote:
>>> > The fourfold increase in the test failure condition
>>> > is due to its unstable work. The old condition was left,
>>> > but the test result was changed from fail to warn, added
>>> > fail condition.
>>> >
>>>
>>> Hi, can you clarify what the commit message wrt. "fourfold increase...
>>> due to its unstable work" means? I'd like to understand the observed
>>> issue better.
>>>
As you can see from git log Michel has improved the first iteration (

The only problem reported as of comment 3 is an occasional failure on
the first iteration.
Something I believe is fixed with Michel's patch
375a0b8ac845e13c180dafc73cb4b8bc32fffb20

I'm struggling with your explanation in the bugzilla report. Can you
provide a bit more detail?

Thanks
Emil
Hello Emil,

On Mon, Jul 30, 2018 at 2:06 PM Emil Velikov <emil.l.velikov@gmail.com>
wrote:

> HI Illia,
>
> Please don't top-post.
>
> On 30 July 2018 at 10:51, Illia Iorin <illia.iorin@gmail.com> wrote:
> > ping
> >
> > On Tue, Jul 10, 2018 at 2:25 PM Illia Iorin <illia.iorin@gmail.com>
> wrote:
> >>
> >> Hello,
> >> Full explanation can be found in
> Having a bug report is great, but the commit itself should be self
> sufficient.
>
> >> https://bugs.freedesktop.org/show_bug.cgi?id=89437#c4
> You can add this to the commit summary as
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89437
>
> >> If this does not completely clarify the issue. Please specify the
> >> question.
> >>
> >> Sincerely yours, Illia.
> >>
> >> On Tue, Jul 10, 2018 at 12:38 AM Mario Kleiner
> >> <mario.kleiner.de@gmail.com> wrote:
> >>>
> >>> On Mon, Jul 9, 2018 at 9:09 PM, Illia Iorin <illia.iorin@gmail.com>
> >>> wrote:
> >>> > The fourfold increase in the test failure condition
> >>> > is due to its unstable work. The old condition was left,
> >>> > but the test result was changed from fail to warn, added
> >>> > fail condition.
> >>> >
> >>>
> >>> Hi, can you clarify what the commit message wrt. "fourfold increase...
> >>> due to its unstable work" means? I'd like to understand the observed
> >>> issue better.
> >>>
> As you can see from git log Michel has improved the first iteration (
>
> The only problem reported as of comment 3 is an occasional failure on
> the first iteration.
> Something I believe is fixed with Michel's patch
> 375a0b8ac845e13c180dafc73cb4b8bc32fffb20
>
> This test works  unstable at some resolutions. For example, you can run
test on all resolutions  supported by your display and see unexpected fails.


> I'm struggling with your explanation in the bugzilla report. Can you
> provide a bit more detail?
>
> As I’ve understood from source of Mesa all OML sync calls are redirected
to specific implementation of windows manager.
It is not mentioned what deviation is permissible in OML Spec. While I was
running test on different configurations of gpu arch, resolution and
windows manager I came to the conclusion 200 ns is the acceptable delay
that helps us to distinguish the error in mesa from the error in the window
manager. If we do not change the error condition this part of test do not
have sense, because it fails so often and unpredictable, and this “fail”
doesn’t show incorrect work of opengl implementation.

This workaround helps to catch an error which cause in the implementation
of openGL and not in the implementation of the window manager.

Thanks,
Illia.