Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Petri Latvala]

On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests to
> BAT. At the same time remove basic-s4 from the list, which is atm
> implicitly disabled via HIBERNATION=n in kconfig. We would need at least
> basic S4 coverage provided by basic-s4-devices, which requires
> HIBERNATION=y.
> 
> Signed-off-by: Imre Deak 
> ---
>  tests/intel-ci/fast-feedback.testlist | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/intel-ci/fast-feedback.testlist 
> b/tests/intel-ci/fast-feedback.testlist
> index f59ec98..efa7e86 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
>  igt@gem_exec_store@basic-render
>  igt@gem_exec_store@basic-vebox
>  igt@gem_exec_suspend@basic
> +igt@gem_exec_suspend@basic-s3-devices
>  igt@gem_exec_suspend@basic-s3
> -igt@gem_exec_suspend@basic-s4
> +igt@gem_exec_suspend@basic-s4-devices
>  igt@gem_flink_basic@bad-flink
>  igt@gem_flink_basic@bad-open
>  igt@gem_flink_basic@basic


I pushed this commit, but without adding basic-s3-devices. The needed
kernel config changes will be deployed after CI is using IGT builds
with this commit in, next week.


--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Ville Syrjälä]

On Tue, Oct 18, 2016 at 12:03:12PM +0300, Petri Latvala wrote:
> On Tue, Oct 18, 2016 at 09:34:49AM +0200, Daniel Vetter wrote:
> > Re S4: If it indeed improves coverage (i.e. calls our shutdown hooks and
> > all these S4-only callbacks) then adding it to BAT sounds reasonable.
> > Still there's the issue of where to get the machine time from. I really do
> > think you need to first trade in some speed-up here (or throw out some
> > other tests) before you can add more tests.
> 
> For the record, the runtimes of all tests in CI can be found in the
> result-runtimes.log file, within the test log directory.
> 
> Over 10s runtimes on a randomly selected machine are:
> 
> 31.92 igt@gem_ctx_switch@basic-default-heavy
> 22.91 igt@gem_busy@basic-hang-default
> 21.03 igt@kms_flip@basic-flip-vs-dpms
> 20.96 igt@kms_flip@basic-flip-vs-modeset
> 20.39 igt@gem_ctx_switch@basic-default
> 20.30 igt@gem_exec_create@basic
> 20.11 igt@gem_ctx_create@basic-files
> 19.64 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
> 18.93 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
> 18.91 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
> 18.76 igt@gem_exec_suspend@basic-s3
> 17.98 igt@kms_pipe_crc_basic@hang-read-crc-pipe-c
> 16.70 igt@kms_flip@basic-flip-vs-wf_vblank
> 14.17 igt@kms_pipe_crc_basic@hang-read-crc-pipe-a
> 14.14 igt@gem_exec_nop@basic-parallel
> 14.12 igt@gem_exec_nop@basic-series
> 13.76 igt@kms_pipe_crc_basic@hang-read-crc-pipe-b
> 12.70 igt@gem_ringfill@basic-default-hang
> 11.34 igt@drv_hangman@error-state-basic
> 10.91 igt@gem_sync@basic-many-each
> 10.81 igt@gem_close_race@basic-threads
> 10.73 igt@kms_flip@basic-plain-flip
> 10.33 igt@gem_sync@basic-store-each
> 10.29 igt@gem_sync@basic-store-all
> 10.11 igt@gem_sync@basic-each
> 10.08 igt@gem_sync@basic-all
> 
> Does any of these strike as a low-hanging fruit?

s/15/5/ for the S3 delay I'd say. If there's a machine that won't work
with 5 seconds, then we find out what's the lowest safe value and use
that instead. 15 seconds is excessive on all the machines I've seen.
And maybe make these delays configurabe via env variables so that I
don't have to keep patching igt when I want to test suspendy stuff ;)

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Petri Latvala]

On Tue, Oct 18, 2016 at 09:34:49AM +0200, Daniel Vetter wrote:
> Re S4: If it indeed improves coverage (i.e. calls our shutdown hooks and
> all these S4-only callbacks) then adding it to BAT sounds reasonable.
> Still there's the issue of where to get the machine time from. I really do
> think you need to first trade in some speed-up here (or throw out some
> other tests) before you can add more tests.

For the record, the runtimes of all tests in CI can be found in the
result-runtimes.log file, within the test log directory.

Over 10s runtimes on a randomly selected machine are:

31.92 igt@gem_ctx_switch@basic-default-heavy
22.91 igt@gem_busy@basic-hang-default
21.03 igt@kms_flip@basic-flip-vs-dpms
20.96 igt@kms_flip@basic-flip-vs-modeset
20.39 igt@gem_ctx_switch@basic-default
20.30 igt@gem_exec_create@basic
20.11 igt@gem_ctx_create@basic-files
19.64 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a
18.93 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b
18.91 igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c
18.76 igt@gem_exec_suspend@basic-s3
17.98 igt@kms_pipe_crc_basic@hang-read-crc-pipe-c
16.70 igt@kms_flip@basic-flip-vs-wf_vblank
14.17 igt@kms_pipe_crc_basic@hang-read-crc-pipe-a
14.14 igt@gem_exec_nop@basic-parallel
14.12 igt@gem_exec_nop@basic-series
13.76 igt@kms_pipe_crc_basic@hang-read-crc-pipe-b
12.70 igt@gem_ringfill@basic-default-hang
11.34 igt@drv_hangman@error-state-basic
10.91 igt@gem_sync@basic-many-each
10.81 igt@gem_close_race@basic-threads
10.73 igt@kms_flip@basic-plain-flip
10.33 igt@gem_sync@basic-store-each
10.29 igt@gem_sync@basic-store-all
10.11 igt@gem_sync@basic-each
10.08 igt@gem_sync@basic-all

Does any of these strike as a low-hanging fruit?


--
Petri Latvala
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Daniel Vetter]

On Mon, Oct 17, 2016 at 05:46:50PM +0300, Imre Deak wrote:
> On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
> > On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> > > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests
> > > to
> > > BAT. At the same time remove basic-s4 from the list, which is atm
> > > implicitly disabled via HIBERNATION=n in kconfig. We would need at
> > > least
> > > basic S4 coverage provided by basic-s4-devices, which requires
> > > HIBERNATION=y.
> > > 
> > > Signed-off-by: Imre Deak 
> > 
> > What's the impact on BAT runtime with this? 
> 
> I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
> 
> > Afaik we're already over budget ... Where do you safe the time to
> > afford this?
> 
> I didn't, but we don't have any S4 coverage in CI atm and it's the
> minimum that can be added. The S3-devices subtest is not critical,
> although it would be useful for cases where we wouldn't get any logs
> for the full S3 test.

Yeah, I don't think a reduced S3 test in BAT is useful if  we have the
full one already. It's a good test for debugging, but not within the very
tight constraints we have for BAT.

Re S4: If it indeed improves coverage (i.e. calls our shutdown hooks and
all these S4-only callbacks) then adding it to BAT sounds reasonable.
Still there's the issue of where to get the machine time from. I really do
think you need to first trade in some speed-up here (or throw out some
other tests) before you can add more tests.

And yes I know that's not fun, but until we have CI running a more
complete test set (after BAT has done the initial sanity checking, to
avoid wasting machine time on broken patches), that's the reality we have
:(

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Imre Deak]

On ma, 2016-10-17 at 18:01 +0300, Saarinen, Jani wrote:
> HI, 
> > -Original Message-
> > From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On
> > Behalf
> > Of Imre Deak
> > Sent: Monday, October 17, 2016 5:47 PM
> > To: Daniel Vetter <dan...@ffwll.ch>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] intel-ci: Add
> > gem_exec_suspend/basic-
> > S3/S4-devices to BAT
> > 
> > On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
> > > On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> > > > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices
> > > > subtests
> > > > to BAT. At the same time remove basic-s4 from the list, which
> > > > is atm
> > > > implicitly disabled via HIBERNATION=n in kconfig. We would need
> > > > at
> > > > least basic S4 coverage provided by basic-s4-devices, which
> > > > requires
> > > > HIBERNATION=y.
> > > > 
> > > > Signed-off-by: Imre Deak <imre.d...@intel.com>
> > > 
> > > What's the impact on BAT runtime with this?
> Are these also stable enough to be added so that we are not causing
> flip-flops?

Based on my tests, they are.

> > I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
> > 
> > > Afaik we're already over budget ... Where do you safe the time to
> > > afford this?
> > 
> > I didn't, but we don't have any S4 coverage in CI atm and it's the
> > minimum
> > that can be added. The S3-devices subtest is not critical, although
> > it would be
> > useful for cases where we wouldn't get any logs for the full S3
> > test.
> > 
> > --Imre
> > 
> > > -Daniel
> > > 
> > > > ---
> > > >  tests/intel-ci/fast-feedback.testlist | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/tests/intel-ci/fast-feedback.testlist
> > > > b/tests/intel-
> > > > ci/fast-feedback.testlist index f59ec98..efa7e86 100644
> > > > --- a/tests/intel-ci/fast-feedback.testlist
> > > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > > @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
> > > >  igt@gem_exec_store@basic-render
> > > >  igt@gem_exec_store@basic-vebox
> > > >  igt@gem_exec_suspend@basic
> > > > +igt@gem_exec_suspend@basic-s3-devices
> > > >  igt@gem_exec_suspend@basic-s3
> > > > -igt@gem_exec_suspend@basic-s4
> > > > +igt@gem_exec_suspend@basic-s4-devices
> > > >  igt@gem_flink_basic@bad-flink
> > > >  igt@gem_flink_basic@bad-open
> > > >  igt@gem_flink_basic@basic
> > > > --
> > > > 2.5.0
> > > > 
> 
> Jani Saarinen
> Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> 
> 
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Saarinen, Jani]

HI, 
> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Imre Deak
> Sent: Monday, October 17, 2016 5:47 PM
> To: Daniel Vetter <dan...@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-
> S3/S4-devices to BAT
> 
> On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
> > On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> > > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests
> > > to BAT. At the same time remove basic-s4 from the list, which is atm
> > > implicitly disabled via HIBERNATION=n in kconfig. We would need at
> > > least basic S4 coverage provided by basic-s4-devices, which requires
> > > HIBERNATION=y.
> > >
> > > Signed-off-by: Imre Deak <imre.d...@intel.com>
> >
> > What's the impact on BAT runtime with this?
Are these also stable enough to be added so that we are not causing flip-flops?

> 
> I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
> 
> > Afaik we're already over budget ... Where do you safe the time to
> > afford this?
> 
> I didn't, but we don't have any S4 coverage in CI atm and it's the minimum
> that can be added. The S3-devices subtest is not critical, although it would 
> be
> useful for cases where we wouldn't get any logs for the full S3 test.
> 
> --Imre
> 
> > -Daniel
> >
> > > ---
> > >  tests/intel-ci/fast-feedback.testlist | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-
> > > ci/fast-feedback.testlist index f59ec98..efa7e86 100644
> > > --- a/tests/intel-ci/fast-feedback.testlist
> > > +++ b/tests/intel-ci/fast-feedback.testlist
> > > @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
> > >  igt@gem_exec_store@basic-render
> > >  igt@gem_exec_store@basic-vebox
> > >  igt@gem_exec_suspend@basic
> > > +igt@gem_exec_suspend@basic-s3-devices
> > >  igt@gem_exec_suspend@basic-s3
> > > -igt@gem_exec_suspend@basic-s4
> > > +igt@gem_exec_suspend@basic-s4-devices
> > >  igt@gem_flink_basic@bad-flink
> > >  igt@gem_flink_basic@bad-open
> > >  igt@gem_flink_basic@basic
> > > --
> > > 2.5.0
> > >

Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo



___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Jani Nikula]

On Mon, 17 Oct 2016, Imre Deak  wrote:
> On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
>> On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
>> > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests
>> > to
>> > BAT. At the same time remove basic-s4 from the list, which is atm
>> > implicitly disabled via HIBERNATION=n in kconfig. We would need at
>> > least
>> > basic S4 coverage provided by basic-s4-devices, which requires
>> > HIBERNATION=y.
>> > 
>> > Signed-off-by: Imre Deak 
>> 
>> What's the impact on BAT runtime with this? 
>
> I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.
>
>> Afaik we're already over budget ... Where do you safe the time to
>> afford this?
>
> I didn't, but we don't have any S4 coverage in CI atm and it's the
> minimum that can be added. The S3-devices subtest is not critical,
> although it would be useful for cases where we wouldn't get any logs
> for the full S3 test.

If our CI was sufficiently clever, it could detect these patches to
change the test list, and let us all know what the impact is...

BR,
Jani.



>
> --Imre
>
>> -Daniel
>> 
>> > ---
>> >  tests/intel-ci/fast-feedback.testlist | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> > 
>> > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-
>> > ci/fast-feedback.testlist
>> > index f59ec98..efa7e86 100644
>> > --- a/tests/intel-ci/fast-feedback.testlist
>> > +++ b/tests/intel-ci/fast-feedback.testlist
>> > @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
>> >  igt@gem_exec_store@basic-render
>> >  igt@gem_exec_store@basic-vebox
>> >  igt@gem_exec_suspend@basic
>> > +igt@gem_exec_suspend@basic-s3-devices
>> >  igt@gem_exec_suspend@basic-s3
>> > -igt@gem_exec_suspend@basic-s4
>> > +igt@gem_exec_suspend@basic-s4-devices
>> >  igt@gem_flink_basic@bad-flink
>> >  igt@gem_flink_basic@bad-open
>> >  igt@gem_flink_basic@basic
>> > -- 
>> > 2.5.0
>> > 
>> > ___
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Imre Deak]

On ma, 2016-10-17 at 16:32 +0200, Daniel Vetter wrote:
> On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> > Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests
> > to
> > BAT. At the same time remove basic-s4 from the list, which is atm
> > implicitly disabled via HIBERNATION=n in kconfig. We would need at
> > least
> > basic S4 coverage provided by basic-s4-devices, which requires
> > HIBERNATION=y.
> > 
> > Signed-off-by: Imre Deak 
> 
> What's the impact on BAT runtime with this? 

I measured 8 sec for S3-devices and 9 sec for S4-devices on my APL.

> Afaik we're already over budget ... Where do you safe the time to
> afford this?

I didn't, but we don't have any S4 coverage in CI atm and it's the
minimum that can be added. The S3-devices subtest is not critical,
although it would be useful for cases where we wouldn't get any logs
for the full S3 test.

--Imre

> -Daniel
> 
> > ---
> >  tests/intel-ci/fast-feedback.testlist | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-
> > ci/fast-feedback.testlist
> > index f59ec98..efa7e86 100644
> > --- a/tests/intel-ci/fast-feedback.testlist
> > +++ b/tests/intel-ci/fast-feedback.testlist
> > @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
> >  igt@gem_exec_store@basic-render
> >  igt@gem_exec_store@basic-vebox
> >  igt@gem_exec_suspend@basic
> > +igt@gem_exec_suspend@basic-s3-devices
> >  igt@gem_exec_suspend@basic-s3
> > -igt@gem_exec_suspend@basic-s4
> > +igt@gem_exec_suspend@basic-s4-devices
> >  igt@gem_flink_basic@bad-flink
> >  igt@gem_flink_basic@bad-open
> >  igt@gem_flink_basic@basic
> > -- 
> > 2.5.0
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] intel-ci: Add gem_exec_suspend/basic-S3/S4-devices to BAT

[Daniel Vetter]

On Mon, Oct 17, 2016 at 03:39:04PM +0300, Imre Deak wrote:
> Add gem_exec_suspend/basic-s3-devices and basic-s4-devices subtests to
> BAT. At the same time remove basic-s4 from the list, which is atm
> implicitly disabled via HIBERNATION=n in kconfig. We would need at least
> basic S4 coverage provided by basic-s4-devices, which requires
> HIBERNATION=y.
> 
> Signed-off-by: Imre Deak 

What's the impact on BAT runtime with this? Afaik we're already over
budget ... Where do you safe the time to afford this?
-Daniel

> ---
>  tests/intel-ci/fast-feedback.testlist | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/intel-ci/fast-feedback.testlist 
> b/tests/intel-ci/fast-feedback.testlist
> index f59ec98..efa7e86 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -73,8 +73,9 @@ igt@gem_exec_store@basic-default
>  igt@gem_exec_store@basic-render
>  igt@gem_exec_store@basic-vebox
>  igt@gem_exec_suspend@basic
> +igt@gem_exec_suspend@basic-s3-devices
>  igt@gem_exec_suspend@basic-s3
> -igt@gem_exec_suspend@basic-s4
> +igt@gem_exec_suspend@basic-s4-devices
>  igt@gem_flink_basic@bad-flink
>  igt@gem_flink_basic@bad-open
>  igt@gem_flink_basic@basic
> -- 
> 2.5.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx